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

Il 18/03/2013 15:24, Anthony Liguori ha scritto: > "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. Yes, seems okay. Especially if you think of object_property_del_child as the base class's implementation of unparent.

Paolo