[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: |
Daniel P. Berrange |
Subject: |
Re: [Qemu-devel] [PATCH v4 5/8] qom: add object_new_with_props / object_new_withpropv constructors |
Date: |
Wed, 20 May 2015 17:10:37 +0100 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Wed, May 20, 2015 at 01:06:21PM -0300, Eduardo Habkost wrote:
> 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.
> > >
> > > 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.
It seems that there is implicit assumption existing in QEMU, that objects
will only be deleted from the hierarchy by code running in the main event
loop thread. So if a method in the main event loop has got a pointer to
the object it can assume it is safe to use. If anything spawns off a
background thread, or passes the object pointer to a non-main loop thread,
then it should first have acquired an extra reference on the object, and
that thread would have to release the reference when done. I guess this
needs documenting somewhere for clarity
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
- [Qemu-devel] [PATCH v4 7/8] qom: add a object_property_add_enum helper method, (continued)
- [Qemu-devel] [PATCH v4 7/8] qom: add a object_property_add_enum helper method, Daniel P. Berrange, 2015/05/13
- [Qemu-devel] [PATCH v4 5/8] qom: add object_new_with_props / object_new_withpropv constructors, Daniel P. Berrange, 2015/05/13
- 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, 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 <=
- 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, 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