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

[Peter Crosthwaite]

On Thu, Jun 12, 2014 at 12:01 AM, Paolo Bonzini <address@hidden> wrote:
> 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.
>

Ok it's adding up for me now.

Reviewed-by: Peter Crosthwaite <address@hidden>

Regards,
Peter

> 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
>>>
>>>
>>>
>
>