Re: [PATCH 02/11] pci: add option for net failover

[Alex Williamson]

On Mon, 21 Oct 2019 22:28:57 +0200
Jens Freimann <address@hidden> 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 <address@hidden> 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 <address@hidden> 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