[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 4/6] s390x/pci: Ignore the unplug call if we
From: |
David Hildenbrand |
Subject: |
Re: [Qemu-devel] [PATCH v2 4/6] s390x/pci: Ignore the unplug call if we already have a release_timer |
Date: |
Wed, 16 Jan 2019 10:38:05 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.1 |
On 15.01.19 23:53, Collin Walling wrote:
> 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.
No, there is no such thing. E.g. on some architectures it might even be
desirable to send multiple unplug requests to the guest. Say you want to
remove a DIMM. Could be that the guest cannot remove it right now, so it
NACKs the requests. Later you want to retry, so you send another
requests. This means we cannot really track pending unplug requests (and
might not even want to do so).
Regarding hotplug_handler_unplug(), the expectation is that once the
function returns without an error, the device was removed (or is ready
to be removed). So here, tracking a pending removal is not necessary.
See patch #5 where I properly convert what we have on s390x pci to
unplug requests.
>
>> 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 <address@hidden>
>> ---
>> 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.
Thanks!
>
> Reviewed-by: Collin Walling <address@hidden>
>
--
Thanks,
David / dhildenb
- [Qemu-devel] [PATCH v2 0/6] s390x/pci: hotplug handler fixes and reworks, David Hildenbrand, 2019/01/14
- [Qemu-devel] [PATCH v2 1/6] s390x/pci: Use hotplug_dev instead of looking up the host bridge, David Hildenbrand, 2019/01/14
- [Qemu-devel] [PATCH v2 2/6] s390x/pci: Move some hotplug checks to the pre_plug handler, David Hildenbrand, 2019/01/14
- [Qemu-devel] [PATCH v2 6/6] s390x/pci: Unplug remaining devices on pcihost reset, David Hildenbrand, 2019/01/14
- [Qemu-devel] [PATCH v2 4/6] s390x/pci: Ignore the unplug call if we already have a release_timer, David Hildenbrand, 2019/01/14
- [Qemu-devel] [PATCH v2 3/6] s390x/pci: Always delete and free the release_timer, David Hildenbrand, 2019/01/14
- [Qemu-devel] [PATCH v2 5/6] s390x/pci: Introduce unplug requests and split unplug handler, David Hildenbrand, 2019/01/14
- Re: [Qemu-devel] [PATCH v2 0/6] s390x/pci: hotplug handler fixes and reworks, Cornelia Huck, 2019/01/16