Re: [Qemu-devel] [PATCH qom v1 1/1] qom: object: remove parent pointer when unparenting

[Paolo Bonzini <pbonzini@redhat.com>]

Il 11/06/2014 13:28, Andreas Färber ha scritto:
> Am 11.06.2014 12:19, schrieb Peter Crosthwaite:
>> On Wed, Jun 11, 2014 at 6:13 PM, Andreas Färber <afaerber@suse.de> wrote:
>>> Am 27.05.2014 02:39, schrieb Peter Crosthwaite:
>>>> Certain parts of the QOM framework test this pointer to determine if
>>>> an object is parented. Nuke it when the object is unparented to allow
>>>> for reuse of an object after unparenting.
>>>>
>>>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>>> ---
>>>>
>>>>  qom/object.c | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/qom/object.c b/qom/object.c
>>>> index e42b254..8319e89 100644
>>>> --- a/qom/object.c
>>>> +++ b/qom/object.c
>>>> @@ -402,6 +402,7 @@ void object_unparent(Object *obj)
>>>>      if (obj->parent) {
>>>>          object_property_del_child(obj->parent, obj, NULL);
>>>>      }
>>>> +    obj->parent = NULL;
>>>>      object_unref(obj);
>>>>  }
>>>>
>>>
>>> This looks okay to me, and it might also help the segfault on hot-unplug
>>> Stefan and Kevin reported before I went on travels.
>>>
>>
>> Welcome back.
>>
>>> Any objection to moving this one line up into the if?
>>>
>>
>> No problem. Will respin.
>
> I've done so myself, but now I wonder why we are checking obj->parent at
> all there after we already return if !obj->parent? Is this to guard
> against ObjectClass::unparent() changing Object::parent? Either way, the
> two variants you posted and I suggested should be fine.

Yes, in case unparent might already end up removing the property.

I have a patch that moves the unparent call to 
object_finalize_child_property and only removes the property here.  The 
patch would apply anyway, so I'm okay with it.

Paolo