From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34519) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yv6ao-0005so-Fw for qemu-devel@nongnu.org; Wed, 20 May 2015 12:11:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yv6al-0001RP-92 for qemu-devel@nongnu.org; Wed, 20 May 2015 12:11:38 -0400 Received: from cantor2.suse.de ([195.135.220.15]:59743 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yv6al-0001Qf-0f for qemu-devel@nongnu.org; Wed, 20 May 2015 12:11:35 -0400 Message-ID: <555CB235.7020904@suse.de> Date: Wed, 20 May 2015 18:11:33 +0200 From: =?windows-1252?Q?Andreas_F=E4rber?= MIME-Version: 1.0 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> In-Reply-To: <20150520160621.GL17796@thinpad.lan.raisama.net> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 5/8] qom: add object_new_with_props / object_new_withpropv constructors List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: Paolo Bonzini , qemu-devel@nongnu.org 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) =3D=3D NULL) { >>>> return NULL; >>>> } >>>> dev =3D DEVICE(object_new(type)); >>>> if (!dev) { >>>> return NULL; >>>> } >>>> >>>> if (!bus) { >>>> bus =3D 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_chi= ld >>>> (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 ha= ppen >>>> 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 lo= ng >>> can callers safely expect the object to stay around? Can the object b= e >>> disposed in another thread? Can it be disposed only when some specifi= c >>> 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 require= d >> to remove the object from the hierarchy and free it. >=20 > 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. qdev is a different matter - there's too many instances to fix properly right away. The current code serves to balance the ref count for hot-unplug etc. to work. But it is not a template to copy for new QOM code IMO. Regards, Andreas --=20 SUSE Linux GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Felix Imend=F6rffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB 21284 (AG N=FCrnberg)