From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52334) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZYwDK-0006NR-F0 for qemu-devel@nongnu.org; Mon, 07 Sep 2015 09:12:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZYwDH-0007VK-9r for qemu-devel@nongnu.org; Mon, 07 Sep 2015 09:12:02 -0400 Received: from mx2.suse.de ([195.135.220.15]:40719) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZYwDH-0007VC-4D for qemu-devel@nongnu.org; Mon, 07 Sep 2015 09:11:59 -0400 References: <1440590594-5514-1-git-send-email-berrange@redhat.com> <1440590594-5514-2-git-send-email-berrange@redhat.com> <20150907084604.GA29882@redhat.com> From: =?UTF-8?Q?Andreas_F=c3=a4rber?= Message-ID: <55ED8D1C.6020404@suse.de> Date: Mon, 7 Sep 2015 15:11:56 +0200 MIME-Version: 1.0 In-Reply-To: <20150907084604.GA29882@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" Cc: =?UTF-8?Q?Marc-Andr=c3=a9_Lureau?= , QEMU , Markus Armbruster Am 07.09.2015 um 10:46 schrieb Daniel P. Berrange: > On Fri, Sep 04, 2015 at 11:38:06PM +0200, Marc-Andr=C3=A9 Lureau wrote: >> On Wed, Aug 26, 2015 at 2:03 PM, Daniel P. Berrange wrote: >>> +ObjectProperty * >>> +object_class_property_add(ObjectClass *klass, >>> + const char *name, >>> + const char *type, >>> + ObjectPropertyAccessor *get, >>> + ObjectPropertyAccessor *set, >>> + ObjectPropertyRelease *release, >>> + void *opaque, >>> + Error **errp) >>> +{ >>> + ObjectProperty *prop; >>> + size_t name_len =3D strlen(name); >>> + >>> + if (name_len >=3D 3 && !memcmp(name + name_len - 3, "[*]", 4)) { >>> + int i; >>> + ObjectProperty *ret; >>> + char *name_no_array =3D g_strdup(name); >>> + >> >> I question the need for dynamic/array property name registered in >> classes. What would be more useful is an array property instead. It >> would help to introspect classes for dynamic "children[*]" case. >> object_property_add_child() could verify/check against the class >> declaration, and grow the instance properties list (like it does now, >> but it would be only for instances of children[] items). On >> introspection of classes, the class "children[*]" property would be >> visible, but would be hidden when introspecting the instance, and you >> wouldn't be able to lookup that "array" property. >> >> It seems relatively straightforward to deal with the link<> case, by >> storing the offset of the "child" pointer. This seems fine if limited >> to a single link<> (it should probably check the prop is not of the >> name[*] style already), ex: >> https://gist.github.com/elmarco/905241b683fb9c5f2a08 >> >> Your patches looks good to me in general but object_property_del() >> should be fixed, since the prop find may belong to the class. >=20 > Actually I skipped object_property_del() intentionally. Classes should > be immutable once defined, so deleting a property from a class would > not be appropriate. Agreed, I don't see a use case either. Can you propose a sentence to amend the commit message with? Then I would apply this patch to my upcoming qom-next pull, then the unreviewed rest could go through their respective maintainers. Regards, Andreas --=20 SUSE Linux GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany GF: Felix Imend=C3=B6rffer, Jane Smithard, Graham Norton; HRB 21284 (AG N= =C3=BCrnberg)