Re: an issue for device hot-unplug

[Yu Zhang <yu.zhang@ionos.com>]

[-- Attachment #1: Type: text/plain, Size: 3648 bytes --]

> this patch targets corner case of early boot where
> guest hasn't initialized ACPI subsystem yet and 'broken'
> management asking to unplug device too early which leads
> to device stuck in being unplugged state due to regression
> in QEMU.
> However, It doesn't apply to fully booted guest.

by adding a few debug lines I see that in
acpi_pcihp_device_unplug_request_cb(),

    pdev->qdev.pending_deleted_event = true;

was executed, which then directly triggered the error in:

void qmp_device_del(const char *id, Error **errp)
{
    DeviceState *dev = find_device_state(id, errp);
    if (dev != NULL) {
        if (dev->pending_deleted_event &&
            (dev->pending_deleted_expires_ms == 0 ||
             dev->pending_deleted_expires_ms >
qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL))) {
            error_setg(errp, "Device %s is already in the "
                             "process of unplug", id);
            return;
        }

        qdev_unplug(dev, errp);
    }
}

In QEMU code, there are 6 lines where this flag is changed:

hw/core/qdev.c:564:        dev->pending_deleted_event = false;
hw/core/qdev.c:601:        dev->pending_deleted_event = true;
hw/acpi/pcihp.c:219:                    qdev->pending_deleted_event = false;
hw/acpi/pcihp.c:359:    pdev->qdev.pending_deleted_event = true;
hw/pci/pcie.c:516:        dev->qdev.pending_deleted_event = false;
hw/pci/pcie.c:573:    dev->pending_deleted_event = true;

Considering the complexity of the code, the logic for setting and clearing
this flag
seems not quite straightforward. I doubt that the setting of
pending_deleted_event in
acpi_pcihp_device_unplug_request_cb() is the appropriate approach to
accomplish its target.


On Tue, Apr 4, 2023 at 2:25 PM Igor Mammedov <imammedo@redhat.com> wrote:

> On Tue, 4 Apr 2023 08:45:54 +0200
> Jinpu Wang <jinpu.wang@ionos.com> wrote:
>
> > Hi Yu,
> >
> > On Mon, Apr 3, 2023 at 6:59 PM Yu Zhang <yu.zhang@ionos.com> wrote:
> > >
> > > Dear Laurent,
> > >
> > > Thank you for your quick reply. We used qemu-7.1, but it is
> reproducible with qemu from v6.2 to the recent v8.0 release candidates.
> > > I found that it's introduced by the commit  9323f892b39 (between
> v6.2.0-rc2 and v6.2.0-rc3).
> > >
> > > If it doesn't break anything else, it suffices to remove the line
> below from acpi_pcihp_device_unplug_request_cb():
> > >
> > >     pdev->qdev.pending_deleted_event = true;
> > >
> > > but you may have a reason to keep it. First of all, I'll open a bug in
> the bug tracker and let you know.
> > >
> > > Best regards,
> > > Yu Zhang
> > This patch from Igor Mammedov seems relevant,
> >
> https://lore.kernel.org/qemu-devel/20230403131833-mutt-send-email-mst@kernel.org/T/#t
>
> this patch targets corner case of early boot where
> guest hasn't initialized ACPI subsystem yet and 'broken'
> management asking to unplug device too early which leads
> to device stuck in being unplugged state due to regression
> in QEMU.
> However, It doesn't apply to fully booted guest.
>
> [...]
>
> > >> > The purpose is for detecting the end of the PCI device hot-unplug.
> However, we feel the
> > >> > error confusing. How is it possible that a disk "is already in the
> process of unplug"
> > >> > during the first hot-unplug attempt? So far as I know, the issue
> was also encountered by
> > >> > libvirt, but they simply ignored it:
> > >> >
> > >> > https://bugzilla.redhat.com/show_bug.cgi?id=1878659
> > >> > <https://bugzilla.redhat.com/show_bug.cgi?id=1878659>
> see my other reply email/BZ comment 17.
>
> [...]
>
>

[-- Attachment #2: Type: text/html, Size: 4997 bytes --]