qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH v5] qom, qmp, hmp, qapi: create qom-type-prop-li


From: Eduardo Habkost
Subject: Re: [Qemu-devel] [PATCH v5] qom, qmp, hmp, qapi: create qom-type-prop-list for class properties
Date: Tue, 26 Jan 2016 15:26:35 -0200
User-agent: Mutt/1.5.23 (2014-03-12)

On Tue, Jan 26, 2016 at 03:51:21PM +0000, Daniel P. Berrange wrote:
> 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.

The existing implementation has its limitations, but we can
address those limitations without exporting a new API that return
arbitrarily different results (that aren't even a superset of the
existing API).

About the existing qmp_device_list_properties() limitations:

cannot_destroy_with_object_finalize_yet is supposed to eventually
go away. If there are use cases that depend on listing properties
for cannot_destroy_with_object_finalize_yet classes, we can fix
that.

The TYPE_DEVICE requirement can be removed, as long as the
non-device QOM classes are object_new()-safe like the existing
cannot_destroy_with_object_finalize_yet=false device classes
(they are supposed to be).

About having to instantiate objects: if optimizing that is so
important, we can gradually convert the existing classes to use
class-properties. While we convert them, we can even have a
doesnt_need_to_instantiate_object_to_query_properties flag to
indicate classes that were already converted. No need to export a
new API.

Abstract classes are harder, but if they are important we can
make them a special case inside the existing implementation
instead of having two APIs.

                             * * *

So, now we have enumerated the current API limitations. Can we
enumerate the real world use cases that are affected by them, so
we know which ones we need to address first?

-- 
Eduardo



reply via email to

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