On Thu, Mar 14, 2013 at 09:06:15AM +0100, Markus Armbruster wrote: > "Michael S. Tsirkin" <mst@xxxxxxxxxx> writes: > > > 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@xxxxxxxxxx> > > --- > > QMP/qmp-events.txt | 16 ++++++++++++++++ > > hw/qdev.c | 12 ++++++++++++ > > include/monitor/monitor.h | 1 + > > monitor.c | 1 + > > qapi-schema.json | 4 +++- > > 5 files changed, 33 insertions(+), 1 deletion(-) > > > > diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt > > index b2698e4..24cf3e8 100644 > > --- a/QMP/qmp-events.txt > > +++ b/QMP/qmp-events.txt > > @@ -136,6 +136,22 @@ 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, optional) > > If there are no members present, do we get an event without member > "data", or do we get one with an empty member? > > { "event": "DEVICE_DELETED", > "timestamp": { "seconds": 1265044230, "microseconds": 450486 } } > > > { "event": "DEVICE_DELETED", > "data": { }, > "timestamp": { "seconds": 1265044230, "microseconds": 450486 } } > > There's no precedence, as this is the first event with data where all > data members are optional. We get one without data. We currently have events with data and some members, and events without data, but not events with an empty data, I didn't see a need to invent one with an empty data. You? > > + > > +{ "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..bebc44d 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; > > @@ -760,6 +761,7 @@ static void device_unparent(Object *obj) > > DeviceState *dev = DEVICE(obj); > > DeviceClass *dc = DEVICE_GET_CLASS(dev); > > BusState *bus; > > + QObject *event_data; > > > > while (dev->num_child_bus) { > > bus = QLIST_FIRST(&dev->child_bus); > > @@ -778,6 +780,16 @@ static void device_unparent(Object *obj) > > object_unref(OBJECT(dev->parent_bus)); > > dev->parent_bus = NULL; > > } > > + > > + if (dev->id) { > > + event_data = qobject_from_jsonf("{ 'device': %s }", dev->id); > > + } else { > > + event_data = NULL; > > + } > > + monitor_protocol_event(QEVENT_DEVICE_DELETED, event_data); > > + if (event_data) { > > + qobject_decref(event_data); > > + } > > You make this unconditional in 3/3. Actually, unconditional should work > just fine even here. No need to respin just for that. > > Answering my doc question: we get an event without member "data". > > Is that what we want? Does it matter? > > } > > > > static void device_class_init(ObjectClass *class, void *data) > > 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 > > ## > > What do you mean by "Guest reset will automatically complete removal for > all devices"? -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list