On Thu, 30 Sep 2021, Laurent Vivier wrote: > Failover needs to detect the end of the PCI unplug to start migration > after the VFIO card has been unplugged. > > To do that, a flag is set in pcie_cap_slot_unplug_request_cb() and reset in > pcie_unplug_device(). > > But since > 17858a169508 ("hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35") > we have switched to ACPI unplug and these functions are not called anymore > and the flag not set. So failover migration is not able to detect if card > is really unplugged and acts as it's done as soon as it's started. So it > doesn't wait the end of the unplug to start the migration. We don't see any > problem when we test that because ACPI unplug is faster than PCIe native > hotplug and when the migration really starts the unplug operation is > already done. > > See c000a9bd06ea ("pci: mark device having guest unplug request pending") > a99c4da9fc2a ("pci: mark devices partially unplugged") > > Signed-off-by: Laurent Vivier <lviv...@redhat.com> Modulo the comment suggestion below, Reviewed-by: Ani Sinha <a...@anisinha.ca> > --- > > Notes: > v2: move partially_hotplugged to acpi_pcihp_eject_slot() > > hw/acpi/pcihp.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c > index f610a25d2ef9..7bbf13492a4f 100644 > --- a/hw/acpi/pcihp.c > +++ b/hw/acpi/pcihp.c > @@ -222,9 +222,13 @@ static void acpi_pcihp_eject_slot(AcpiPciHpState *s, > unsigned bsel, unsigned slo > PCIDevice *dev = PCI_DEVICE(qdev); > if (PCI_SLOT(dev->devfn) == slot) { > if (!acpi_pcihp_pc_no_hotplug(s, dev)) { > - hotplug_ctrl = qdev_get_hotplug_handler(qdev); > - hotplug_handler_unplug(hotplug_ctrl, qdev, &error_abort); > - object_unparent(OBJECT(qdev)); > + if (dev->partially_hotplugged) { Please add a comment here as to why you are skipping full unplug and what this flag signifies and where it is set from. > + qdev->pending_deleted_event = false; > + } else { > + hotplug_ctrl = qdev_get_hotplug_handler(qdev); > + hotplug_handler_unplug(hotplug_ctrl, qdev, &error_abort); > + object_unparent(OBJECT(qdev)); > + } > } > } > } > @@ -396,6 +400,7 @@ void acpi_pcihp_device_unplug_request_cb(HotplugHandler > *hotplug_dev, > return; > } > > + pdev->qdev.pending_deleted_event = true; > s->acpi_pcihp_pci_status[bsel].down |= (1U << slot); > acpi_send_event(DEVICE(hotplug_dev), ACPI_PCI_HOTPLUG_STATUS); > } > -- > 2.31.1 > >