"Michael S. Tsirkin" <mst@xxxxxxxxxx> writes: > 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? 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. [...] -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list