From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:41597) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UDHcc-0002bm-Jv for qemu-devel@nongnu.org; Wed, 06 Mar 2013 11:55:25 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UDHcR-0007kE-E0 for qemu-devel@nongnu.org; Wed, 06 Mar 2013 11:55:18 -0500 Received: from mx1.redhat.com ([209.132.183.28]:29685) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UDHcR-0007jR-5q for qemu-devel@nongnu.org; Wed, 06 Mar 2013 11:55:07 -0500 Date: Wed, 6 Mar 2013 18:55:21 +0200 From: "Michael S. Tsirkin" Message-ID: <20130306165521.GA429@redhat.com> References: <20130306164954.GA331@redhat.com> <51377435.3080100@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <51377435.3080100@redhat.com> Subject: Re: [Qemu-devel] [PATCHv2] qdev: DEVICE_DELETED event List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Kevin Wolf , Anthony Liguori , Eduardo Habkost , Stefan Hajnoczi , Markus Armbruster , qemu-devel@nongnu.org, Gerd Hoffmann , laine@redhat.com, Luiz Capitulino , Andreas =?iso-8859-1?Q?F=E4rber?= On Wed, Mar 06, 2013 at 05:52:05PM +0100, Paolo Bonzini wrote: > Il 06/03/2013 17:49, Michael S. Tsirkin ha scritto: > > 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 > > --- > > > > Changes from v1: > > - move to device_unparent > > - address comments by Andreas and Eric > > > > Andreas also suggested a more generic object-deleted event, > > I'm not sure how useful that is so let's add what we already need, for > > devices with an id and wait and see what's necessary for non-device > > objects? > > > > QMP/qmp-events.txt | 15 +++++++++++++++ > > hw/qdev.c | 7 +++++++ > > include/monitor/monitor.h | 1 + > > monitor.c | 1 + > > qapi-schema.json | 4 +++- > > 5 files changed, 27 insertions(+), 1 deletion(-) > > > > diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt > > index b2698e4..f2f115a 100644 > > --- a/QMP/qmp-events.txt > > +++ b/QMP/qmp-events.txt > > @@ -136,6 +136,21 @@ 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) > > + > > +{ "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..d603f4f 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; > > @@ -761,6 +762,12 @@ static void device_unparent(Object *obj) > > DeviceClass *dc = DEVICE_GET_CLASS(dev); > > BusState *bus; > > > > + if (dev->id) { > > + QObject *data = qobject_from_jsonf("{ 'device': %s }", dev->id); > > + monitor_protocol_event(QEVENT_DEVICE_DELETED, data); > > + qobject_decref(data); > > + } > > Do this at the end of device_unparent, so that parents are reported > after their children. > > Paolo Hmm yes it seems cleaner, though we'd need to copy the id as the device can go away. Doing this after while (dev->num_child_bus) { bus = QLIST_FIRST(&dev->child_bus); qbus_free(bus); } would be enough, isn't it? > > while (dev->num_child_bus) { > > bus = QLIST_FIRST(&dev->child_bus); > > qbus_free(bus); > > 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 > > ## > >