On Mon, 21 Oct 2019 22:28:57 +0200
Jens Freimann <jfreim...@redhat.com> wrote:
> On Mon, Oct 21, 2019 at 01:01:22PM -0600, Alex Williamson wrote:
> >On Mon, 21 Oct 2019 20:45:46 +0200
> >Jens Freimann <jfreim...@redhat.com> wrote:
> >
> >> On Mon, Oct 21, 2019 at 08:58:23AM -0600, Alex Williamson wrote:
> >> >On Fri, 18 Oct 2019 22:20:31 +0200
> >> >Jens Freimann <jfreim...@redhat.com> wrote:
> >> >
> >> >> This patch adds a net_failover_pair_id property to PCIDev which is
> >> >> used to link the primary device in a failover pair (the PCI dev) to
> >> >> a standby (a virtio-net-pci) device.
> >> >>
> >> >> It only supports ethernet devices. Also currently it only supports
> >> >> PCIe devices. QEMU will exit with an error message otherwise.
> >> >
> >> >Doesn't the PCIe device also need to be hotpluggable? We can have PCIe
> >> >devices attached to the root bus where we don't currently support
> >> >hotplug. Thanks,
> >>
> >> How do I recognize such a device? hotpluggable in DeviceClass?
> >
> >I wouldn't expect it to be a device property, it's more of a function
> >of whether the bus that it's attached to supports hotplug, so probably
> >something related to the bus controller. Thanks,
>
> IIUC this is checked for in qdev_device_add() by this:
>
> if (qdev_hotplug && bus && !qbus_is_hotpluggable(bus)) {
> error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name);
> return NULL;
> }
>
> So qemu will fail with 'Bus pcie.0 does not allow hotplug' when we try
> to hotplug the device. I tried a primary with bus=pcie.0 and it aborted.
> Aborting qemu is not nice so I'll make it print a error message
> QERR_BUS_NO_HOTPLUG instead but let it continue. This will be
> a change in the virtio-net patch, not in this one.
qdev_hotplug is only set to true after qdev_machine_creation_done(), so
if we start a VM with cold-plugged primary and failover, wouldn't the
user only learn the configuration is invalid at the time they try to
use it? I agree that the above should prevent a device from being
hot-added to the bus, but I don't think that's our starting position,
is it? Thanks,
Alex
