Re: [libvirt] [Qemu-devel] [PATCH v8 2/3] qom: pass original path to unparent method

"Michael S. Tsirkin" <mst(a)redhat.com&gt; writes: > We need to know the original path since unparenting loses this state. > > Signed-off-by: Michael S. Tsirkin <mst(a)redhat.com&gt; > --- > 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.