qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered


From: Daniel P. Berrange
Subject: Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes
Date: Mon, 7 Sep 2015 09:46:04 +0100
User-agent: Mutt/1.5.23 (2014-03-12)

On Fri, Sep 04, 2015 at 11:38:06PM +0200, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Aug 26, 2015 at 2:03 PM, Daniel P. Berrange <address@hidden> 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 = strlen(name);
> > +
> > +    if (name_len >= 3 && !memcmp(name + name_len - 3, "[*]", 4)) {
> > +        int i;
> > +        ObjectProperty *ret;
> > +        char *name_no_array = 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.

Actually I skipped object_property_del() intentionally. Classes should
be immutable once defined, so deleting a property from a class would
not be appropriate.

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 :|



reply via email to

[Prev in Thread] Current Thread [Next in Thread]