Re: [Qemu-devel] [PATCH] qdev: DEVICE_DELETED event

["Andreas Färber" <afaerber@suse.de>]

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.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

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"
>  
>  int qdev_hotplug = 0;
>  static bool qdev_hot_added = 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 = qobject_from_jsonf("{ 'device': %s }", dev->id);
> +        monitor_protocol_event(QEVENT_DEVICE_DELETED, data);
> +        qobject_decref(data);
> +    }
>      object_unparent(OBJECT(dev));
>  }
>  

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

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg