Re: [Qemu-devel] [PATCH v2 4/6] s390x/pci: Ignore the unplug call if we already have a release_timer

[Collin Walling <walling@linux.ibm.com>]

On 1/14/19 5:31 AM, David Hildenbrand wrote:
> ... otherwise two successive calls to qdev_unplug() (e.g. by an impatient
> user) will effectively overwrite pbdev->release_timer, resulting in a
> memory leak. We are already processing the unplug.
> 

Does QEMU not have a way to detect if a device is already in the process 
of being unplugged? Seems like not having that kind of protection could 
cause many problems.

Perhaps that effort would be arduous.

> If there is already a release_timer, the unplug will be performed after
> the timeout.
> 
> Can be easily triggered by
> (hmp) device_add virtio-mouse-pci,id=test
> (hmp) stop
> (hmp) device_del test
> (hmp) device_del test
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>   hw/s390x/s390-pci-bus.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index 59325cae3b..34a9cb2a80 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -972,6 +972,9 @@ static void s390_pcihost_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
>       case ZPCI_FS_STANDBY:
>           break;
>       default:
> +        if (pbdev->release_timer) {
> +            return;
> +        }
>           s390_pci_generate_plug_event(HP_EVENT_DECONFIGURE_REQUEST,
>                                        pbdev->fh, pbdev->fid);
>           pbdev->release_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
> 

Looks good to me.

Reviewed-by: Collin Walling <walling@linux.ibm.com>