[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCHv2] qdev: DEVICE_DELETED event
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCHv2] qdev: DEVICE_DELETED event |
Date: |
Wed, 06 Mar 2013 17:57:17 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130219 Thunderbird/17.0.3 |
Il 06/03/2013 17:55, Michael S. Tsirkin ha scritto:
> 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?
>>>
>>> QMP/qmp-events.txt | 15 +++++++++++++++
>>> hw/qdev.c | 7 +++++++
>>> include/monitor/monitor.h | 1 +
>>> monitor.c | 1 +
>>> qapi-schema.json | 4 +++-
>>> 5 files changed, 27 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
>>> index b2698e4..f2f115a 100644
>>> --- a/QMP/qmp-events.txt
>>> +++ b/QMP/qmp-events.txt
>>> @@ -136,6 +136,21 @@ Example:
>>> Note: The "ready to complete" status is always reset by a BLOCK_JOB_ERROR
>>> event.
>>>
>>> +DEVICE_DELETED
>>> +-----------------
>>> +
>>> +Emitted whenever the device removal completion is acknowledged
>>> +by the guest. At this point, it's safe to reuse the specified device ID.
>>> +Device removal can be initiated by the guest or by HMP/QMP commands.
>>> +
>>> +Data:
>>> +
>>> +- "device": device name (json-string)
>>> +
>>> +{ "event": "DEVICE_DELETED",
>>> + "data": { "device": "virtio-net-pci-0" },
>>> + "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
>>> +
>>> DEVICE_TRAY_MOVED
>>> -----------------
>>>
>>> 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?
Yes, but don't worry - the device cannot go away, it is protected while
device_unparent is called (see object_unparent in qom/object.c). So put
it wherever you prefer.
Paolo
>>> while (dev->num_child_bus) {
>>> bus = QLIST_FIRST(&dev->child_bus);
>>> qbus_free(bus);
>>> diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
>>> index 87fb49c..b868760 100644
>>> --- a/include/monitor/monitor.h
>>> +++ b/include/monitor/monitor.h
>>> @@ -39,6 +39,7 @@ typedef enum MonitorEvent {
>>> QEVENT_BLOCK_JOB_CANCELLED,
>>> QEVENT_BLOCK_JOB_ERROR,
>>> QEVENT_BLOCK_JOB_READY,
>>> + QEVENT_DEVICE_DELETED,
>>> QEVENT_DEVICE_TRAY_MOVED,
>>> QEVENT_SUSPEND,
>>> QEVENT_SUSPEND_DISK,
>>> diff --git a/monitor.c b/monitor.c
>>> index 32a6e74..2a5e7b6 100644
>>> --- a/monitor.c
>>> +++ b/monitor.c
>>> @@ -457,6 +457,7 @@ static const char *monitor_event_names[] = {
>>> [QEVENT_BLOCK_JOB_CANCELLED] = "BLOCK_JOB_CANCELLED",
>>> [QEVENT_BLOCK_JOB_ERROR] = "BLOCK_JOB_ERROR",
>>> [QEVENT_BLOCK_JOB_READY] = "BLOCK_JOB_READY",
>>> + [QEVENT_DEVICE_DELETED] = "DEVICE_DELETED",
>>> [QEVENT_DEVICE_TRAY_MOVED] = "DEVICE_TRAY_MOVED",
>>> [QEVENT_SUSPEND] = "SUSPEND",
>>> [QEVENT_SUSPEND_DISK] = "SUSPEND_DISK",
>>> diff --git a/qapi-schema.json b/qapi-schema.json
>>> index 28b070f..bb361e1 100644
>>> --- a/qapi-schema.json
>>> +++ b/qapi-schema.json
>>> @@ -2354,7 +2354,9 @@
>>> # Notes: When this command completes, the device may not be removed from
>>> the
>>> # guest. Hot removal is an operation that requires guest
>>> cooperation.
>>> # This command merely requests that the guest begin the hot removal
>>> -# process.
>>> +# process. Completion of the device removal process is signaled
>>> with a
>>> +# DEVICE_DELETED event. Guest reset will automatically complete
>>> removal
>>> +# for all devices.
>>> #
>>> # Since: 0.14.0
>>> ##
>>>