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

"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. > + > +{ "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? > } > > 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"?