From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53226) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZYwIZ-0008OI-Tt for qemu-devel@nongnu.org; Mon, 07 Sep 2015 09:17:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZYwIW-0000xg-NB for qemu-devel@nongnu.org; Mon, 07 Sep 2015 09:17:27 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49038) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZYwIW-0000xV-G0 for qemu-devel@nongnu.org; Mon, 07 Sep 2015 09:17:24 -0400 Date: Mon, 7 Sep 2015 14:17:20 +0100 From: "Daniel P. Berrange" Message-ID: <20150907131720.GL29882@redhat.com> References: <1440590594-5514-1-git-send-email-berrange@redhat.com> <1440590594-5514-2-git-send-email-berrange@redhat.com> <20150907084604.GA29882@redhat.com> <55ED8D1C.6020404@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <55ED8D1C.6020404@suse.de> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes Reply-To: "Daniel P. Berrange" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andreas =?utf-8?Q?F=C3=A4rber?= Cc: =?utf-8?Q?Marc-Andr=C3=A9?= Lureau , QEMU , Markus Armbruster On Mon, Sep 07, 2015 at 03:11:56PM +0200, Andreas F=C3=A4rber wrote: > 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 wrot= e: > >> 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 yo= u > >> 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 limite= d > >> 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 shoul= d > > be immutable once defined, so deleting a property from a class would > > not be appropriate. >=20 > Agreed, I don't see a use case either. >=20 > 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 unreviewe= d > rest could go through their respective maintainers. "Supporting for deletion of properties registered on classes is omitted, since class definitions must be immutable once created" Before you queue it though, I was going to repost with the support for magic "[*]" property expansion removed, unless you think that is really needed. It doesn't do anything you can't do explicitly so I figure its cleaner to not add this magic to the class imp too, as its known to suffer poor scalability. Regards, Daniel --=20 |: http://berrange.com -o- http://www.flickr.com/photos/dberrange= / :| |: http://libvirt.org -o- http://virt-manager.or= g :| |: http://autobuild.org -o- http://search.cpan.org/~danberr= / :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vn= c :|