Re: [Qemu-devel] [PATCH v4 5/8] qom: add object_new_with_props / object_new_withpropv constructors

["Andreas Färber" <afaerber@suse.de>]

Am 20.05.2015 um 17:18 schrieb Daniel P. Berrange:
> On Wed, May 20, 2015 at 11:44:19AM -0300, Eduardo Habkost wrote:
>> On Tue, May 19, 2015 at 06:11:05PM +0200, Paolo Bonzini wrote:
>>> On 19/05/2015 17:55, Daniel P. Berrange wrote:
>>>> Paolo told me on previous posting that object_property_add_child()
>>>> holds a reference on 'obj' for as long as it is registered in the
>>>> object hierarchy composition. So it sufficient to rely on that long
>>>> term reference, and let the caller dispose of the object by calling
>>>> object_unparent(obj) when finally done.
>>>
>>> For an example of the same pattern:
>>>
>>> DeviceState *qdev_try_create(BusState *bus, const char *type)
>>> {
>>>     DeviceState *dev;
>>>
>>>     if (object_class_by_name(type) == NULL) {
>>>         return NULL;
>>>     }
>>>     dev = DEVICE(object_new(type));
>>>     if (!dev) {
>>>         return NULL;
>>>     }
>>>
>>>     if (!bus) {
>>>         bus = sysbus_get_default();
>>>     }
>>>
>>>     qdev_set_parent_bus(dev, bus);
>>>     object_unref(OBJECT(dev));
>>>     return dev;
>>> }
>>>
>>> Effectively this is idea as GObject's "floating reference".
>>> qdev_set_parent_bus (in qdev_try_create) and object_property_add_child
>>> (in Daniel's patches) "sink" the floating reference by doing
>>> object_unref.  If we had floating references, the object would be
>>> returned to the caller unref'ed anyway.
>>
>> I was agreeing with Andreas at first (because it would make the
>> reference ownership rules simpler and easier to understand), until I
>> noticed that every call of qdev_try_create() and object_resolve_path()
>> in the code would need an additional object_unref() call if we didn't
>> use this pattern.
>>
>> But it bothers me that this exceptional behavior is not documented on
>> neither qdev_try_create() or object_resolve_path().
>>
>>>
>>> Of course, the reference can go away via QMP.  But that will only happen
>>> after the caller would have called object_unref itself.
>>
>> But the caller won't ever call object_unref() because it doesn't own any
>> reference, right? In this case, can we clarify the rules about how long
>> can callers safely expect the object to stay around? Can the object be
>> disposed in another thread? Can it be disposed only when some specific
>> events happen?
> 
> In the inline docs for object_new_with_props I wrote
> 
>   * The returned object will have one stable reference maintained
>   * for as long as it is present in the object hierarchy.
> 
> We could expand it to explicitly say that 'object_unparent' is required
> to remove the object from the hierarchy and free it.

I've already queued this series - let's hold off changes for a bit. :)

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)