From: Eduardo Habkost <ehabkost@redhat.com>
To: "Daniel P. Berrange" <berrange@redhat.com>
Cc: qemu-devel@nongnu.org, armbru@redhat.com, lcapitulino@redhat.com,
afaerber@suse.de, asmetanin@virtuozzo.com, den@openvz.org,
Valentin Rakush <valentin.rakush@gmail.com>
Subject: Re: [Qemu-devel] [PATCH v5] qom, qmp, hmp, qapi: create qom-type-prop-list for class properties
Date: Wed, 27 Jan 2016 13:09:37 -0200 [thread overview]
Message-ID: <20160127150937.GQ3869@thinpad.lan.raisama.net> (raw)
In-Reply-To: <20160126221913.GA13460@redhat.com>
On Tue, Jan 26, 2016 at 10:19:13PM +0000, Daniel P. Berrange wrote:
> On Tue, Jan 26, 2016 at 03:26:35PM -0200, Eduardo Habkost wrote:
> > 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 <valentin.rakush@gmail.com>
> > > > > Cc: Luiz Capitulino <lcapitulino@redhat.com>
> > > > > Cc: Eric Blake <eblake@redhat.com>
> > > > > Cc: Markus Armbruster <armbru@redhat.com>
> > > > > Cc: Andreas Färber <afaerber@suse.de>
> > > > > Cc: Daniel P. Berrange <berrange@redhat.com>
> > > > > Cc: Eduardo Habkost <ehabkost@redhat.com>
> > > > > ---
> > > > [...]
> > > > > 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?
>
> Being able to list properties of arbitrary non-device objects is
> really the critical thing that's missing right now, with abstract
> types a close second.
About abstract types: I thought we didn't export any class
hierarchy information. Should we do it? I guess we wouldn't want
clients to make assumptions about the class hierarchy.
--
Eduardo
next prev parent reply other threads:[~2016-01-27 15:09 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-25 8:24 [Qemu-devel] [PATCH v5] qom, qmp, hmp, qapi: create qom-type-prop-list for class properties Valentin Rakush
2016-01-26 15:35 ` Eduardo Habkost
2016-01-26 15:42 ` Eric Blake
2016-01-26 15:51 ` Daniel P. Berrange
2016-01-26 17:26 ` Eduardo Habkost
2016-01-26 22:19 ` Daniel P. Berrange
2016-01-27 9:53 ` Valentin Rakush
2016-01-27 15:09 ` Eduardo Habkost [this message]
2016-01-27 15:23 ` Daniel P. Berrange
2016-01-29 10:03 ` Valentin Rakush
2016-01-29 15:28 ` Eduardo Habkost
2016-01-31 13:40 ` Valentin Rakush
2016-02-02 15:55 ` Eduardo Habkost
2016-02-07 14:52 ` Valentin Rakush
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160127150937.GQ3869@thinpad.lan.raisama.net \
--to=ehabkost@redhat.com \
--cc=afaerber@suse.de \
--cc=armbru@redhat.com \
--cc=asmetanin@virtuozzo.com \
--cc=berrange@redhat.com \
--cc=den@openvz.org \
--cc=lcapitulino@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=valentin.rakush@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).