"Michael S. Tsirkin" <mst@redhat.com> writes:
On Thu, Mar 14, 2013 at 09:06:15AM +0100, Markus Armbruster wrote:
"Michael S. Tsirkin" <mst@redhat.com> 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@redhat.com> --- 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?
Donning my downstream hat for a minute. Upstream after this series is fully applied: * event always has data * data always has member path * data may have member device Downstream after backport of just 1/3: * event may have data * data always has member device Alternative downstream: event always has data, data * event always has data * data may have member device Smaller difference to upstream. Would libvirt care?
+ +{ "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?
It doesn't matter upstream, because the anomaly created by 1/3 gets removed by 3/3. It matters downstream if 1/3 gets backported without 3/3. I will not withhold my upstream ACK because of such downstream concerns. [...]