Re: [Qemu-devel] [RFC PATCH 02/15] qdev: store DeviceState's canonical path to use when unparenting

[Paolo Bonzini <pbonzini@redhat.com>]

On 29/04/2015 21:20, Michael Roth wrote:
> If the parent is finalized as a result of object_unparent(), it
> will still be attached to the composition tree at the time any
> children are unparented as a result of that same call to
> object_unparent(). However, in some cases, object_unparent()
> will complete without finalizing the parent device, due to
> lingering references that won't be released till some time later.
> One such example is if the parent has MemoryRegion children (which
> take a ref on their parent), who in turn have AddressSpace's (which
> take a ref on their regions), since those AddressSpaces get cleaned
> up asynchronously by the RCU thread.
> 
> In this case qdev:device_unparent() may be called for a child Device
> that no longer has a path to the root/machine container, causing
> object_get_canonical_path() to assert.

This doesn't seem right.  Unparent callbacks are _not_ called when you 
finalize, they are called in post-order as soon as you unplug a device 
(the call tree is object_unparent ==> device_unparent(parent) ==> 
bus_unparent(parent->bus) ==> device_unparent(parent->bus->child[0]) 
and so on).

DEVICE_DELETED is called after a device's children have been 
unparented.  It could be called after a bus is dead though.  Could it 
be that the patch you want is simply this:

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 6e6a65d..46019c4 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -1241,11 +1241,6 @@ static void device_unparent(Object *obj)
         bus = QLIST_FIRST(&dev->child_bus);
         object_unparent(OBJECT(bus));
     }
-    if (dev->parent_bus) {
-        bus_remove_child(dev->parent_bus, dev);
-        object_unref(OBJECT(dev->parent_bus));
-        dev->parent_bus = NULL;
-    }
 
     /* Only send event if the device had been completely realized */
     if (dev->pending_deleted_event) {
@@ -1254,6 +1249,12 @@ static void device_unparent(Object *obj)
         qapi_event_send_device_deleted(!!dev->id, dev->id, path, &error_abort);
         g_free(path);
     }
+
+    if (dev->parent_bus) {
+        bus_remove_child(dev->parent_bus, dev);
+        object_unref(OBJECT(dev->parent_bus));
+        dev->parent_bus = NULL;
+    }
 }
 
 static void device_class_init(ObjectClass *class, void *data)

?

Paolo