Re: [PATCH 6/6] pcie: expire pending delete

[Michael S. Tsirkin]

On Mon, Oct 11, 2021 at 02:05:04PM +0200, Gerd Hoffmann wrote:
> Add an expire time for pending delete, once the time is over allow
> pressing the attention button again.
> 
> This makes pcie hotplug behave more like acpi hotplug, where one can
> try sending an 'device_del' monitor command again in case the guest
> didn't respond to the first attempt.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  include/hw/qdev-core.h | 1 +
>  hw/pci/pcie.c          | 2 ++
>  softmmu/qdev-monitor.c | 4 +++-
>  3 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 4ff19c714bd8..d082a9a00aca 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -180,6 +180,7 @@ struct DeviceState {
>      char *canonical_path;
>      bool realized;
>      bool pending_deleted_event;
> +    int64_t pending_deleted_expires_ms;
>      QemuOpts *opts;
>      int hotplugged;
>      bool allow_unplug_during_migration;
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index f3ac04399969..477c8776aa27 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -549,6 +549,8 @@ void pcie_cap_slot_unplug_request_cb(HotplugHandler 
> *hotplug_dev,
>      }
>  
>      dev->pending_deleted_event = true;
> +    dev->pending_deleted_expires_ms =
> +        qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 5000; /* 5 secs */
>  
>      /* In case user cancel the operation of multi-function hot-add,
>       * remove the function that is unexposed to guest individually,


Well this will be barely enough, right?

        Once the Power
        Indicator begins blinking, a 5-second abort interval exists during 
which a second depression of the
        Attention Button cancels the operation.

So I guess it needs to be more. Problem is of course if guest is
busy because of interrupts and whatnot, it might not get to
handling that in time ...


> diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
> index 0705f008466d..5e7960c52a0a 100644
> --- a/softmmu/qdev-monitor.c
> +++ b/softmmu/qdev-monitor.c
> @@ -910,7 +910,9 @@ void qmp_device_del(const char *id, Error **errp)
>  {
>      DeviceState *dev = find_device_state(id, errp);
>      if (dev != NULL) {
> -        if (dev->pending_deleted_event) {
> +        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;
> -- 
> 2.31.1