Re: [libvirt] [Qemu-devel] [PATCH v6 1/3] qdev: DEVICE_DELETED event

On Thu, Mar 14, 2013 at 09:06:15AM +0100, Markus Armbruster wrote: > "Michael S. Tsirkin" <mst(a)redhat.com&gt; 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(a)redhat.com&gt; > > --- > > 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?