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

[Paolo Bonzini <pbonzini@redhat.com>]

Il 11/06/2014 15:35, Peter Crosthwaite ha scritto:
> On Wed, Jun 11, 2014 at 10:19 PM, Paolo Bonzini <pbonzini@redhat.com> 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 <pbonzini@redhat.com>
>> ---
>>  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
>>
>>
>>