From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:51483) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UHazc-00054A-Og for qemu-devel@nongnu.org; Mon, 18 Mar 2013 10:24:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UHazX-0008Uz-KL for qemu-devel@nongnu.org; Mon, 18 Mar 2013 10:24:52 -0400 Received: from e23smtp04.au.ibm.com ([202.81.31.146]:44542) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UHazX-0008U7-2u for qemu-devel@nongnu.org; Mon, 18 Mar 2013 10:24:47 -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 00:14:19 +1000 Received: from d23relay03.au.ibm.com (d23relay03.au.ibm.com [9.190.235.21]) by d23dlp02.au.ibm.com (Postfix) with ESMTP id 5EB072BB0051 for ; Tue, 19 Mar 2013 01:24:34 +1100 (EST) Received: from d23av03.au.ibm.com (d23av03.au.ibm.com [9.190.234.97]) by d23relay03.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r2IEOTsL6226210 for ; Tue, 19 Mar 2013 01:24:30 +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 r2IEOWR9015983 for ; Tue, 19 Mar 2013 01:24:33 +1100 From: Anthony Liguori In-Reply-To: <50e744fbae4b08dc4ec33d5d44acc83da7170391.1363264726.git.mst@redhat.com> References: <50e744fbae4b08dc4ec33d5d44acc83da7170391.1363264726.git.mst@redhat.com> Date: Mon, 18 Mar 2013 09:24:16 -0500 Message-ID: <87zjy0946n.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: "Michael S. Tsirkin" , Markus Armbruster Cc: Kevin Wolf , Eduardo Habkost , libvir-list@redhat.com, Stefan Hajnoczi , qemu-devel@nongnu.org, Luiz Capitulino , Gerd Hoffmann , Paolo Bonzini , Andreas =?us-ascii?B?PT91dGYtOD9RP0Y9QzM9QTRyYmVy Pz0=?= "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. Regards, Anthony Liguori > object_unref(obj); > + g_free(path); > } > > static void object_deinit(Object *obj, TypeImpl *type) > -- > MST