Re: [Qemu-devel] [PATCH v5] qom, qmp, hmp, qapi: create qom-type-prop-list for class properties

[Daniel P. Berrange]

On Tue, Jan 26, 2016 at 01:35:38PM -0200, Eduardo Habkost wrote:
> On Mon, Jan 25, 2016 at 11:24:47AM +0300, Valentin Rakush wrote:
> > This patch adds support for qom-type-prop-list command to list object
> > class properties. A later patch will use this functionality to
> > implement x86_64-cpu properties.
> > 
> > Signed-off-by: Valentin Rakush <address@hidden>
> > Cc: Luiz Capitulino <address@hidden>
> > Cc: Eric Blake <address@hidden>
> > Cc: Markus Armbruster <address@hidden>
> > Cc: Andreas Färber <address@hidden>
> > Cc: Daniel P. Berrange <address@hidden>
> > Cc: Eduardo Habkost <address@hidden>
> > ---
> [...]
> > diff --git a/qmp.c b/qmp.c
> > index 53affe2..baf25c0 100644
> > --- a/qmp.c
> > +++ b/qmp.c
> > @@ -460,6 +460,37 @@ ObjectTypeInfoList *qmp_qom_list_types(bool 
> > has_implements,
> >      return ret;
> >  }
> >  
> > +ObjectPropertyInfoList *qmp_qom_type_prop_list(const char *typename, Error 
> > **errp)
> > +{
> > +    ObjectClass *klass;
> > +    ObjectPropertyInfoList *props = NULL;
> > +    ObjectProperty *prop;
> > +    ObjectPropertyIterator iter;
> > +
> > +    klass = object_class_by_name(typename);
> > +    if (!klass) {
> > +        error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
> > +                  "Object class '%s' not found", typename);
> > +        return NULL;
> > +    }
> > +
> > +    object_class_property_iter_init(&iter, klass);
> > +    while ((prop = object_property_iter_next(&iter))) {
> > +        ObjectPropertyInfoList *entry = g_new0(ObjectPropertyInfoList, 1);
> > +
> > +        if (entry) {
> > +            entry->value = g_new0(ObjectPropertyInfo, 1);
> > +            entry->next = props;
> > +            props = entry;
> > +
> > +            entry->value->name = g_strdup(prop->name);
> > +            entry->value->type = g_strdup(prop->type);
> > +        }
> > +    }
> > +
> > +    return props;
> > +}
> > +
> 
> We already have "-device <type>,help", and it uses a completely
> different mechanism for listing properties. There's no reason for
> having two arbitrarily different APIs for listing properties
> returning different results.
> 
> If qmp_device_list_properties() is not enough for you, please
> clarify why, so we can consider improving it.

qmp_device_list_properties() has to actually instantiate an instance
of objects it is reporting properties against, since it is reporting
properties registered against object instances. In fact it only
reports properties against things which are TYPE_DEVICE - it'll refuse
to report other object types. Having to instantiate objects is inherantly
limiting to the command because there are some objects that cannot be
instantiated for this purpose. eg abstract objects and objects marked
"cannot_destroy_with_object_finalize_yet". Finally there is also a
performance and memory overhead in having to instantiate objects which
is best avoided.

This new API is reporting properties that are statically registered
against the *class* rather than than object instance. It is guaranteed
that you can always report these properties for any class without any
restrictions, nor any chance of side effects during instantiation.

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