From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:42560) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UDHef-00053V-Jr for qemu-devel@nongnu.org; Wed, 06 Mar 2013 11:57:27 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UDHee-0008Vo-7q for qemu-devel@nongnu.org; Wed, 06 Mar 2013 11:57:25 -0500 Received: from mx1.redhat.com ([209.132.183.28]:5883) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UDHed-0008Vb-W9 for qemu-devel@nongnu.org; Wed, 06 Mar 2013 11:57:24 -0500 Message-ID: <5137756D.4050100@redhat.com> Date: Wed, 06 Mar 2013 17:57:17 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <20130306164954.GA331@redhat.com> <51377435.3080100@redhat.com> <20130306165521.GA429@redhat.com> In-Reply-To: <20130306165521.GA429@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCHv2] qdev: DEVICE_DELETED event List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: Kevin Wolf , Anthony Liguori , Eduardo Habkost , Stefan Hajnoczi , Markus Armbruster , qemu-devel@nongnu.org, Gerd Hoffmann , laine@redhat.com, Luiz Capitulino , =?ISO-8859-1?Q?Andreas_F=E4rber?= Il 06/03/2013 17:55, Michael S. Tsirkin ha scritto: > 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? Yes, but don't worry - the device cannot go away, it is protected while device_unparent is called (see object_unparent in qom/object.c). So put it wherever you prefer. Paolo >>> 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 >>> ## >>>