| Message ID | 20130306164954.GA331@redhat.com |
|---|---|
| State | New |
| Headers | show |
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 <mst@redhat.com> >>> --- >>> >>> 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
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); + } + 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 ##
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 <mst@redhat.com> --- 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(-)