Re: [PATCH v3] failover: fix unplug pending detection

[Laurent Vivier <lvivier@redhat.com>]

On 01/10/2021 12:45, Gerd Hoffmann wrote:
> On Fri, Oct 01, 2021 at 10:25:02AM +0200, 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().
> 
>> +    /*
>> +     * pending_deleted_event is used by virtio-net failover to detect the
>> +     * end of the unplug operation, the flag is set to false in
>> +     * acpi_pcihp_eject_slot() when the operation is completed.
>> +     */
>> +    pdev->qdev.pending_deleted_event = true;
> 
> This has the side effect of blocking a second 'device_del' command.

This problem is not introduced by this patch as it only adds the code we already have in 
PCIe Native hotplug to ACPI hotplug (but see below).

> So, in case the first time didn't work (for example due to the guest not
> listening because grub just doesn't do that), you can try a second time
> once the linux kernel is up'n'running.
> 
> I suspect this patch will break that (didn't actually test though).

I think the solution to this problem is to not check for pending_deleted_event value in 
qmp_device_del().

But this has been explicitly added by:

commit cce8944cc9efab47d4bf29cfffb3470371c3541b
Author: Julia Suvorova <jusual@redhat.com>
Date:   Thu Feb 20 17:55:56 2020 +0100

     qdev-monitor: Forbid repeated device_del

     Device unplug can be done asynchronously. Thus, sending the second
     device_del before the previous unplug is complete may lead to
     unexpected results. On PCIe devices, this cancels the hot-unplug
     process.

     Signed-off-by: Julia Suvorova <jusual@redhat.com>
     Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
     Message-Id: <20200220165556.39388-1-jusual@redhat.com>
     Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

So do you mean ACPI differs from PCIe Native hotplug in this case?

Thanks,
Laurent