From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:39867) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UDEqX-0007fe-Sa for qemu-devel@nongnu.org; Wed, 06 Mar 2013 08:57:35 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UDEqS-0002VX-V7 for qemu-devel@nongnu.org; Wed, 06 Mar 2013 08:57:29 -0500 Received: from cantor2.suse.de ([195.135.220.15]:41532 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UDEqS-0002VL-Lc for qemu-devel@nongnu.org; Wed, 06 Mar 2013 08:57:24 -0500 Message-ID: <51374B42.2090405@suse.de> Date: Wed, 06 Mar 2013 14:57:22 +0100 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <20130306130055.GA29446@redhat.com> In-Reply-To: <20130306130055.GA29446@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] qdev: DEVICE_DELETED event List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" , Paolo Bonzini Cc: Kevin Wolf , Anthony Liguori , Eduardo Habkost , Stefan Hajnoczi , Markus Armbruster , qemu-devel@nongnu.org, Gerd Hoffmann , laine@redhat.com, Luiz Capitulino Am 06.03.2013 14:00, schrieb Michael S. Tsirkin: > libvirt has a long-standing bug: when removing the device, > it can request removal but does not know when does the > removal complete. Add an event so we can fix this in a robust way. >=20 > Signed-off-by: Michael S. Tsirkin Sounds like a good idea to me. :) [...] > diff --git a/hw/qdev.c b/hw/qdev.c > index 689cd54..f30d251 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" > =20 > int qdev_hotplug =3D 0; > static bool qdev_hot_added =3D false; > @@ -267,6 +268,11 @@ void qdev_init_nofail(DeviceState *dev) > /* Unlink device from bus and free the structure. */ > void qdev_free(DeviceState *dev) > { > + if (dev->id) { > + QObject *data =3D qobject_from_jsonf("{ 'device': %s }", dev->= id); > + monitor_protocol_event(QEVENT_DEVICE_DELETED, data); > + qobject_decref(data); > + } > object_unparent(OBJECT(dev)); > } > =20 I'm pretty sure this is the wrong place to fire the notification. We should rather do this when the device is actually deleted - which qdev_free() does *not* actually guarantee, as criticized in the s390x and unref'ing contexts. I would suggest to place your code into device_unparent() instead. Another thing to consider is what data to pass to the event: Not all devices have an ID. We should still have a canonical path when we fire this event in either qdev_free() or in device_unparent() before the if (dev->parent_bus) block though. That would be a question for Anthony, not having a use case for the event I am indifferent there. Further, thinking of objects such as virtio-rng backends or future blockdev/chardev objects, might it make sense to turn this into a generic object deletion event rather than a device event? Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg