[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 5/8] qom: add object_new_with_props / object_
From: |
Andreas Färber |
Subject: |
Re: [Qemu-devel] [PATCH v4 5/8] qom: add object_new_with_props / object_new_withpropv constructors |
Date: |
Wed, 20 May 2015 18:24:47 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 |
Am 20.05.2015 um 18:22 schrieb Eduardo Habkost:
> On Wed, May 20, 2015 at 06:11:33PM +0200, Andreas Färber wrote:
>> Am 20.05.2015 um 18:06 schrieb Eduardo Habkost:
>>> On Wed, May 20, 2015 at 04:18:03PM +0100, Daniel P. Berrange wrote:
>>>> 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.changes
>>>>>
>>>>> 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.
>>>
>>> What's missing to me is some clarification on how long it is safe to
>>> assume that the object won't be removed from the hierarchy by other
>>> code.
>>
>> There is no guarantee. Therefore any caller of object_resolve_path()
>> must ref and unref as needed if doing more than a single operation, such
>> as setting a link target.
>
> So, does that mean zero guarantee, or guarantee for "a single
> operation"? It can't be both.
>
> (And how exactly "a single operation" should be defined?)
I see your point, after all object_ref() is a single instruction, too.
We could of course change it to return a ref'ed object, but that would
require a lot of code review and probably unrefs. Mostly for objects
that don't go away, but still.
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)
- Re: [Qemu-devel] [PATCH v4 5/8] qom: add object_new_with_props / object_new_withpropv constructors, (continued)
- Re: [Qemu-devel] [PATCH v4 5/8] qom: add object_new_with_props / object_new_withpropv constructors, Daniel P. Berrange, 2015/05/19
- Re: [Qemu-devel] [PATCH v4 5/8] qom: add object_new_with_props / object_new_withpropv constructors, Andreas Färber, 2015/05/19
- Re: [Qemu-devel] [PATCH v4 5/8] qom: add object_new_with_props / object_new_withpropv constructors, Paolo Bonzini, 2015/05/19
- Re: [Qemu-devel] [PATCH v4 5/8] qom: add object_new_with_props / object_new_withpropv constructors, Eduardo Habkost, 2015/05/20
- Re: [Qemu-devel] [PATCH v4 5/8] qom: add object_new_with_props / object_new_withpropv constructors, Daniel P. Berrange, 2015/05/20
- Re: [Qemu-devel] [PATCH v4 5/8] qom: add object_new_with_props / object_new_withpropv constructors, Andreas Färber, 2015/05/20
- Re: [Qemu-devel] [PATCH v4 5/8] qom: add object_new_with_props / object_new_withpropv constructors, Eduardo Habkost, 2015/05/20
- Re: [Qemu-devel] [PATCH v4 5/8] qom: add object_new_with_props / object_new_withpropv constructors, Daniel P. Berrange, 2015/05/20
- Re: [Qemu-devel] [PATCH v4 5/8] qom: add object_new_with_props / object_new_withpropv constructors, Andreas Färber, 2015/05/20
- Re: [Qemu-devel] [PATCH v4 5/8] qom: add object_new_with_props / object_new_withpropv constructors, Eduardo Habkost, 2015/05/20
- Re: [Qemu-devel] [PATCH v4 5/8] qom: add object_new_with_props / object_new_withpropv constructors,
Andreas Färber <=
- Re: [Qemu-devel] [PATCH v4 5/8] qom: add object_new_with_props / object_new_withpropv constructors, Eduardo Habkost, 2015/05/20