From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:57928) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UHcYW-0006pP-VM for qemu-devel@nongnu.org; Mon, 18 Mar 2013 12:05:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UHcYV-0005BK-0G for qemu-devel@nongnu.org; Mon, 18 Mar 2013 12:05:00 -0400 Received: from e23smtp04.au.ibm.com ([202.81.31.146]:44058) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UHcYU-0005B4-FG for qemu-devel@nongnu.org; Mon, 18 Mar 2013 12:04:58 -0400 Received: from /spool/local by e23smtp04.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 19 Mar 2013 01:54:28 +1000 Received: from d23relay04.au.ibm.com (d23relay04.au.ibm.com [9.190.234.120]) by d23dlp02.au.ibm.com (Postfix) with ESMTP id 5A9FC2BB0050 for ; Tue, 19 Mar 2013 03:04:43 +1100 (EST) Received: from d23av03.au.ibm.com (d23av03.au.ibm.com [9.190.234.97]) by d23relay04.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r2IFpnrw49676480 for ; Tue, 19 Mar 2013 02:51:52 +1100 Received: from d23av03.au.ibm.com (loopback [127.0.0.1]) by d23av03.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r2IG4dEe008806 for ; Tue, 19 Mar 2013 03:04:40 +1100 From: Anthony Liguori In-Reply-To: <51472CC0.8010706@redhat.com> References: <50e744fbae4b08dc4ec33d5d44acc83da7170391.1363264726.git.mst@redhat.com> <87zjy0946n.fsf@codemonkey.ws> <51472CC0.8010706@redhat.com> Date: Mon, 18 Mar 2013 11:04:29 -0500 Message-ID: <878v5kitiq.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [PATCH v8 2/3] qom: pass original path to unparent method List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Kevin Wolf , Eduardo Habkost , "Michael S. Tsirkin" , libvir-list@redhat.com, Stefan Hajnoczi , Markus Armbruster , qemu-devel@nongnu.org, Gerd Hoffmann , Luiz Capitulino , Andreas =?utf-8?Q?F=C3=A4rber?= Paolo Bonzini writes: > Il 18/03/2013 15:24, Anthony Liguori ha scritto: >> "Michael S. Tsirkin" writes: >> >>> We need to know the original path since unparenting loses this state. >>> >>> Signed-off-by: Michael S. Tsirkin >>> --- >>> hw/qdev.c | 4 ++-- >>> include/qom/object.h | 3 ++- >>> qom/object.c | 4 +++- >>> 3 files changed, 7 insertions(+), 4 deletions(-) >>> >>> diff --git a/hw/qdev.c b/hw/qdev.c >>> index 741af96..64546cf 100644 >>> --- a/hw/qdev.c >>> +++ b/hw/qdev.c >>> @@ -436,7 +436,7 @@ static void qbus_realize(BusState *bus, DeviceState *parent, const char *name) >>> } >>> } >>> >>> -static void bus_unparent(Object *obj) >>> +static void bus_unparent(Object *obj, const char *path) >>> { >>> BusState *bus = BUS(obj); >>> BusChild *kid; >>> @@ -756,7 +756,7 @@ static void device_class_base_init(ObjectClass *class, void *data) >>> klass->props = NULL; >>> } >>> >>> -static void device_unparent(Object *obj) >>> +static void device_unparent(Object *obj, const char *path) >>> { >>> DeviceState *dev = DEVICE(obj); >>> DeviceClass *dc = DEVICE_GET_CLASS(dev); >>> diff --git a/include/qom/object.h b/include/qom/object.h >>> index cf094e7..f0790d4 100644 >>> --- a/include/qom/object.h >>> +++ b/include/qom/object.h >>> @@ -330,11 +330,12 @@ typedef struct ObjectProperty >>> /** >>> * ObjectUnparent: >>> * @obj: the object that is being removed from the composition tree >>> + * @path: canonical path that object had if any >>> * >>> * Called when an object is being removed from the QOM composition tree. >>> * The function should remove any backlinks from children objects to @obj. >>> */ >>> -typedef void (ObjectUnparent)(Object *obj); >>> +typedef void (ObjectUnparent)(Object *obj, const char *path); >>> >>> /** >>> * ObjectFree: >>> diff --git a/qom/object.c b/qom/object.c >>> index 3d638ff..21c9da4 100644 >>> --- a/qom/object.c >>> +++ b/qom/object.c >>> @@ -362,14 +362,16 @@ static void object_property_del_child(Object *obj, Object *child, Error **errp) >>> >>> void object_unparent(Object *obj) >>> { >>> + gchar *path = object_get_canonical_path(obj); >>> object_ref(obj); >>> if (obj->parent) { >>> object_property_del_child(obj->parent, obj, NULL); >>> } >>> if (obj->class->unparent) { >>> - (obj->class->unparent)(obj); >>> + (obj->class->unparent)(obj, path); >>> } >> >> I think you should actually just move this call above >> if (obj->parent) { object_parent_del_child(...); }. >> >> There's no harm AFAICT in doing this and it seems more logical to me to >> have destruction flow start with the subclass and move up to the base >> class. >> >> This avoids needing a hack like this because the object is still in a >> reasonable state when unparent is called. >> >> Paolo, do you see anything wrong with this? I looked at the commit you >> added this in and it doesn't look like it would be a problem. > > Yes, seems okay. Especially if you think of object_property_del_child > as the base class's implementation of unparent. Cool, Michael can you update your patch? Should simplify it quite a bit. Regards, Anthony Liguori > > Paolo