Re: [Qemu-devel] [PATCH v3 2/5] virtio_net: Add support for "Data Path Switching" during Live Migration.

[si-wei liu]

On 01/09/2019 07:56 AM, Michael S. Tsirkin wrote:
> On Mon, Jan 07, 2019 at 05:29:41PM -0500, Venu Busireddy wrote:
> > diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> > index 80d42e1..2a3ffd3 100644
> > --- a/hw/acpi/pcihp.c
> > +++ b/hw/acpi/pcihp.c
> > @@ -176,6 +176,25 @@ static void acpi_pcihp_eject_slot(AcpiPciHpState *s, 
> > unsigned bsel, unsigned slo
> >       }
> >   }
> >   
> > +static void acpi_pcihp_cleanup_failover_primary(AcpiPciHpState *s, int bsel)
> > +{
> > +    BusChild *kid, *next;
> > +    PCIBus *bus = acpi_pcihp_find_hotplug_bus(s, bsel);
> > +
> > +    if (!bus) {
> > +        return;
> > +    }
> > +    QTAILQ_FOREACH_SAFE(kid, &bus->qbus.children, sibling, next) {
> > +        DeviceState *qdev = kid->child;
> > +        PCIDevice *pdev = PCI_DEVICE(qdev);
> > +        int slot = PCI_SLOT(pdev->devfn);
> > +
> > +        if (pdev->failover_primary) {
> > +            s->acpi_pcihp_pci_status[bsel].down |= (1U << slot);
> > +        }
> > +    }
> > +}
> > +
> >   static void acpi_pcihp_update_hotplug_bus(AcpiPciHpState *s, int bsel)
> >   {
> >       BusChild *kid, *next;
> So the result here will be that device will be deleted completely,
> and will not reappear after guest reboot.
The management stack will replug the VF until seeing the STANDBY_CHANGED 
"enabled" event after guest driver finishes feature negotiation and sets 
driver_ok.

> I don't think this is what we wanted.
> I think we wanted a special state that will hide device from guest until
> guest acks the failover bit.
What do we get by hiding? On the next reboot after system reset guest 
may load an older OS instance without standby advertised. The VF can't 
be plugged out then?

The model we adopt here doesn't pair virtio with VF in the QEMU level. 
If the VF isn't being used by guest, it would make sense to notify 
management to release VF anyways.

> > @@ -207,6 +226,14 @@ static void acpi_pcihp_update(AcpiPciHpState *s)
> >       int i;
> >   
> >       for (i = 0; i < ACPI_PCIHP_MAX_HOTPLUG_BUS; ++i) {
> > +        /*
> > +         * Set the acpi_pcihp_pci_status[].down bits of all the
> > +         * failover_primary devices so that the devices are ejected
> > +         * from the guest. We can't use the qdev_unplug() as well as the
> > +         * hotplug_handler to unplug the devices, because the guest may
> > +         * not be in a state to cooperate.
> > +         */
> > +        acpi_pcihp_cleanup_failover_primary(s, i);
> >           acpi_pcihp_update_hotplug_bus(s, i);
> >       }
> >   }
> I really don't want acpi to know anything about failover.
>
> All that needs to happen is sending a device delete request
> to guest. Should work with any hotplug removal:
> pci standard,acpi, etc.
As the code comments above indicated, there was issue uncovered that the 
guest may not be in a state to respond to interrupt during reboot. 
Actually management stack running fast enough is supposed to do this 
graceful hot plug removal upon receiving the STANDBY_CHANGED "disabled" 
event. However, if management stack's unable to do so, the code here 
makes sure the VF can be deleted and won't be seen by an older kernel 
after reboot.

-Siwei