[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCHv2] qdev: DEVICE_DELETED event
From: |
Andreas Färber |
Subject: |
Re: [Qemu-devel] [PATCHv2] qdev: DEVICE_DELETED event |
Date: |
Wed, 06 Mar 2013 18:03:37 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130215 Thunderbird/17.0.3 |
Am 06.03.2013 17:55, schrieb Michael S. Tsirkin:
> On Wed, Mar 06, 2013 at 05:52:05PM +0100, Paolo Bonzini wrote:
>> Il 06/03/2013 17:49, Michael S. Tsirkin ha scritto:
>>> libvirt has a long-standing bug: when removing the device,
>>> it can request removal but does not know when the
>>> removal completes. Add an event so we can fix this in a robust way.
>>>
>>> Signed-off-by: Michael S. Tsirkin <address@hidden>
>>> ---
>>>
>>> Changes from v1:
>>> - move to device_unparent
>>> - address comments by Andreas and Eric
>>>
>>> Andreas also suggested a more generic object-deleted event,
>>> I'm not sure how useful that is so let's add what we already need, for
>>> devices with an id and wait and see what's necessary for non-device
>>> objects?
Fine with me, just wanted to bring that up for consideration since it
impacts not only where we call it but also what data we can provide.
>>> diff --git a/hw/qdev.c b/hw/qdev.c
>>> index 689cd54..d603f4f 100644
>>> --- a/hw/qdev.c
>>> +++ b/hw/qdev.c
>>> @@ -29,6 +29,7 @@
>>> #include "sysemu/sysemu.h"
>>> #include "qapi/error.h"
>>> #include "qapi/visitor.h"
>>> +#include "qapi/qmp/qjson.h"
>>>
>>> int qdev_hotplug = 0;
>>> static bool qdev_hot_added = false;
>>> @@ -761,6 +762,12 @@ static void device_unparent(Object *obj)
>>> DeviceClass *dc = DEVICE_GET_CLASS(dev);
>>> BusState *bus;
>>>
>>> + if (dev->id) {
>>> + QObject *data = qobject_from_jsonf("{ 'device': %s }", dev->id);
>>> + monitor_protocol_event(QEVENT_DEVICE_DELETED, data);
>>> + qobject_decref(data);
>>> + }
>>
>> Do this at the end of device_unparent, so that parents are reported
>> after their children.
>>
>> Paolo
>
> Hmm yes it seems cleaner, though we'd need to copy the
> id as the device can go away.
>
> Doing this after
> while (dev->num_child_bus) {
> bus = QLIST_FIRST(&dev->child_bus);
> qbus_free(bus);
> }
> would be enough, isn't it?
I had implicitly suggested after dev->realized and before
dev->parent_bus. The reason being that what is inside the if
(dev->realized) {} block actually still needs to be moved into an
unrealize function, and realized = false might still do device-specific
cleanups that could consume time. I'll try to cook something up tonight,
shouldn't collide with your change.
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg