From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54083) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ynocq-0001OV-27 for qemu-devel@nongnu.org; Thu, 30 Apr 2015 09:35:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ynocp-0005Gr-4j for qemu-devel@nongnu.org; Thu, 30 Apr 2015 09:35:35 -0400 Message-ID: <55422F91.4050504@redhat.com> Date: Thu, 30 Apr 2015 15:35:13 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1430335224-6716-1-git-send-email-mdroth@linux.vnet.ibm.com> <1430335224-6716-3-git-send-email-mdroth@linux.vnet.ibm.com> In-Reply-To: <1430335224-6716-3-git-send-email-mdroth@linux.vnet.ibm.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [RFC PATCH 02/15] qdev: store DeviceState's canonical path to use when unparenting List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Roth , qemu-devel@nongnu.org Cc: "Michael S. Tsirkin" , aik@ozlabs.ru, qemu-ppc@nongnu.org, bharata@linux.vnet.ibm.com, nfont@linux.vnet.ibm.com, david@gibson.dropbear.id.au 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