Re: [Qemu-devel] commit da57febfed "qdev: give all devices a canonical path" broke usb_del

[Michael Tokarev <mjt@tls.msk.ru>]

On 08.08.2012 16:18, Michael Tokarev wrote:
> While dealing with USB issues today I noticed that
> usb_del monitor command is broken, attempting to
> delete any usb device immediately results in assertion
> failure:
> 
> (qemu) usb_del 0.1
> ERROR:qom/object.c:408:object_delete: assertion failed: (obj->ref == 0)
> Aborted
> 
> 
> I bisected this issue to commit:
> 
> commit da57febfed7bad11be79f047b59719c38abd0712
> Author: Paolo Bonzini <pbonzini@redhat.com>
> Date:   Tue Mar 27 18:38:47 2012 +0200
> 
>     qdev: give all devices a canonical path
> 
>     A strong limitation of QOM right now is that unconverted ports
>     (e.g. all...) do not give a canonical path to devices that are
>     part of the board.  This in turn makes it impossible to replace
>     PROP_PTR with a QOM link for example.
> 
>     Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
>     Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>     Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> 
> 
> The problem is still present in current qemu/master git.
> 
> I'm not sure what to do with this.

Well.  This commit adds this code:

@@ -152,6 +152,16 @@ int qdev_init(DeviceState *dev)
+
+    if (!OBJECT(dev)->parent) {
+        static int unattached_count = 0;
+        gchar *name = g_strdup_printf("device[%d]", unattached_count++);
+
+        object_property_add_child(container_get("/unattached"), name,
+                                  OBJECT(dev), NULL);
+        g_free(name);
+    }

ie, there's now extra call to object_property_add_child(),
which does object_ref().  So no doubt there's no a new
assertion failure: (obj->ref == 0).

Once again, patch description does not reflect what is
actually done by the patch.  Can we please stop this
practice of accepting patches with desrciption not
reflecting reality?

Back to the point: should there be a call to object_unref()
somewhere?

Thanks,

/mjt