Re: [Qemu-devel] [RFC PATCH 03/13] qom: move unparenting to the child property's release callback

[Paolo Bonzini]

Il 11/06/2014 15:35, Peter Crosthwaite ha scritto:
> On Wed, Jun 11, 2014 at 10:19 PM, Paolo Bonzini <address@hidden> wrote:
> > This ensures that the unparent callback is called automatically
> > when the parent object is finalized.
> >
> > Note that there's no need to keep a reference neither in
> > object_unparent nor in object_finalize_child_property.  The
> > reference held by the child property itself will do.
> >
> > Signed-off-by: Paolo Bonzini <address@hidden>
> > ---
> >  qom/object.c | 14 ++++----------
> >  1 file changed, 4 insertions(+), 10 deletions(-)
> >
> > diff --git a/qom/object.c b/qom/object.c
> > index 01087e5..69a95d6 100644
> > --- a/qom/object.c
> > +++ b/qom/object.c
> > @@ -386,19 +386,9 @@ static void object_property_del_child(Object *obj, Object 
> > *child, Error **errp)
> >
> >  void object_unparent(Object *obj)
> >  {
> > -    if (!obj->parent) {
> > -        return;
> > -    }
> > -
> > -    object_ref(obj);
> > -    if (obj->class->unparent) {
> > -        (obj->class->unparent)(obj);
> > -    }
>
> This changes this publicly visible API significantly, in that unparent
> from the childs point-of-view is now invisible in cases where you are
> doing a standalone object_unparent() call.
>
> I'm not sure why parent finalisation is the only occasion on which a
> genuine and complete unparenting is allowed? I think this API should
> remain complete should a QOM user wish to re-arrange child
> relationships seperate from any finalisations.

No, this is moved to object_finalize_child_property, i.e. the release 
callback of child<> properties.  object_property_del_child calls it.

The change is that, before, the unparent callback was only invoked if 
you explicitly called object_unparent.  Now it's always called as part 
of finalize, even if you unparent the outer object but not the inner 
ones.  I find it more intuitive.

Note that this is not yet needed in these 13 patches.

Paolo

> >      if (obj->parent) {
> >          object_property_del_child(obj->parent, obj, NULL);
> > -        obj->parent = NULL;
> >      }
> > -    object_unref(obj);
> >  }
> >
> >  static void object_deinit(Object *obj, TypeImpl *type)
> > @@ -1017,6 +1007,10 @@ static void object_finalize_child_property(Object *obj, 
> > const char *name,
> >  {
> >      Object *child = opaque;
> >
> > +    if (child->class->unparent) {
> > +        (child->class->unparent)(child);
> > +    }
> > +    child->parent = NULL;
>
> Can you just leave object_unparent() mostly as-is and call it from
> here? It's already guarded against repeated calls should there be
> users calling it manually before parent finalisations.
>
> Regards,
> Peter
>
> >      object_unref(child);
> >  }
> >
> > --
> > 1.8.3.1