[libvirt] [Qemu-devel] [PATCH v6 1/3] qdev: DEVICE_DELETED event
Markus Armbruster
armbru at redhat.com
Thu Mar 14 12:24:02 UTC 2013
"Michael S. Tsirkin" <mst at redhat.com> writes:
> On Thu, Mar 14, 2013 at 09:06:15AM +0100, Markus Armbruster wrote:
>> "Michael S. Tsirkin" <mst at 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 at 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.
[...]
More information about the libvir-list
mailing list