From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34269) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yv6Zy-0004UU-8u for qemu-devel@nongnu.org; Wed, 20 May 2015 12:10:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yv6Zu-0001Ee-Nr for qemu-devel@nongnu.org; Wed, 20 May 2015 12:10:46 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34048) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yv6Zu-0001EU-8Q for qemu-devel@nongnu.org; Wed, 20 May 2015 12:10:42 -0400 Date: Wed, 20 May 2015 17:10:37 +0100 From: "Daniel P. Berrange" Message-ID: <20150520161037.GB23989@redhat.com> References: <1431533649-23115-1-git-send-email-berrange@redhat.com> <1431533649-23115-6-git-send-email-berrange@redhat.com> <555B5C2E.5050903@suse.de> <20150519155528.GF8535@redhat.com> <555B6099.4060803@redhat.com> <20150520144419.GE28075@thinpad.lan.raisama.net> <20150520151803.GA23989@redhat.com> <20150520160621.GL17796@thinpad.lan.raisama.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20150520160621.GL17796@thinpad.lan.raisama.net> Subject: Re: [Qemu-devel] [PATCH v4 5/8] qom: add object_new_with_props / object_new_withpropv constructors Reply-To: "Daniel P. Berrange" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: Paolo Bonzini , Andreas =?utf-8?Q?F=C3=A4rber?= , qemu-devel@nongnu.org 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 :|