[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all a
From: |
Daniel P. Berrange |
Subject: |
Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties |
Date: |
Thu, 22 Sep 2016 14:01:37 +0100 |
User-agent: |
Mutt/1.7.0 (2016-08-17) |
On Thu, Sep 22, 2016 at 10:36:45AM +0200, Markus Armbruster wrote:
> "Lin Ma" <address@hidden> writes:
>
> >>>> Markus Armbruster <address@hidden> 2016/9/20 星期二 上午 1:13 >>>
> >>Andreas Färber <address@hidden> writes:
> > Saving acceptable values of enumeration types into member description
> > of ObjectProperty is a good idea.
> >
> > The member description of ObjectProperty instance of any user-creatable
> > object is NULL so far,
>
> It's null until set with object_property_set_description(). We do that
> for a few properties, and may do it more.
>
> > If I use object_property_set_description() to fill
> > the
> > acceptable values of enumeration type into the description in function
> > object_property_add_enum and object_class_property_add_enum, Then I
> > can use this description to judge whether a ObjectProperty instance' type
> > is enumeration or not in function user_creatable_help_func. In this case, If
> > member description is not NULL, it means this ObjectProperty instance is
> > an enumeration.
>
> No. If you need to decide in user_creatable_help_func() whether a
> property has an enumeration type, something's wrong at the
> infrastructure level.
>
> > Any suggestions?
>
> Yes:
>
> >>When it's null we could still fall back to a description of the type.
> >>Does such a thing exist? Enumeration types could provide one listing
> >>their values.
>
> Don't make up a description in user_creatable_help_func(), improve the
> description infrastructure and its use so you get more useful ones
> there.
>
> The existing description infrastructure is just Property member
> description and object_property_set_description(). Rarely used, so
> description is generally null.
>
> Calling object_property_set_description() more often could be helpful,
> but to come up with a sensible description string, you need to know what
> the property does. Needs to be left to people actually familiar with
> the objects.
>
> Aside: historically, we add properties to *instances*. All the property
> meta-data gets duplicated for every instance, including property
> descriptions. This is more flexible than adding the meta-data to the
> class. The flexibility is rarely needed, but the price in wasted memory
> is always paid. Only since commit 16bf7f5, we can add it to classes.
> Adding lots of helpful property descriptions would increase the cost of
> instance properties further.
>
> What you could perhaps do is adding a *type* description. For enums,
> that would show the set of acceptable values. Then if the property has
> no description, fall back to the description of its type.
I don't think we need to invent anything new. We can use the existing
property description facility and auto-generate the string containing
the permitted values thus:
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index dabc42e..0446839 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -202,9 +202,11 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
self._btin += gen_enum(name, values, prefix)
if do_builtins:
self.defn += gen_enum_lookup(name, values, prefix)
+ self._btin += gen_enum_value_str(name, values)
else:
self._fwdecl += gen_enum(name, values, prefix)
self.defn += gen_enum_lookup(name, values, prefix)
+ self._fwdecl += gen_enum_value_str(name, values)
def visit_array_type(self, name, info, element_type):
if isinstance(element_type, QAPISchemaBuiltinType):
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 21bc32f..d11c414 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1649,6 +1649,15 @@ const char *const %(c_name)s_lookup[] = {
return ret
+def gen_enum_value_str(name, values):
+ return mcgen('''
+
+#define %(c_name)s_value_str "%(value_str)s"
+''',
+ c_name=c_name(name),
+ value_str=", ".join(["'%s'" % c for c in values]))
+
+
def gen_enum(name, values, prefix=None):
# append automatically generated _MAX value
enum_values = values + ['_MAX']
Now, when registering an enum property do something like this:
object_class_property_add_enum(oc, "format",
"QCryptoSecretFormat",
QCryptoSecretFormat_lookup,
qcrypto_secret_prop_get_format,
qcrypto_secret_prop_set_format,
NULL);
object_class_property_set_description(oc, "format",
"Data format, one of "
QCryptoSecretFormat_value_str,
&error_abort);
So that description ends up being
"Data format, one of 'base64', 'plain'"
It would be nicer if the object_property_add_* methods just
accepted a 'const char *description' too. As well as being
more efficient that a second method call which has to search
for the ObjectProperty struct, potentially reporting errors,
it will also encourage people to actually provide descriptions
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 :|
- Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties, (continued)
- Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties, Markus Armbruster, 2016/09/19
- Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties, Andreas Färber, 2016/09/19
- Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties, Markus Armbruster, 2016/09/19
- Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties, Lin Ma, 2016/09/21
- Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties, Markus Armbruster, 2016/09/22
- Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties, Daniel P. Berrange, 2016/09/22
- Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties, Markus Armbruster, 2016/09/22
- Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties, Daniel P. Berrange, 2016/09/22
- Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties, Markus Armbruster, 2016/09/22
- Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties, Daniel P. Berrange, 2016/09/22
- Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties,
Daniel P. Berrange <=
Re: [Qemu-devel] [PATCH v2] object: Add 'help' option for all available backends and properties, Daniel P. Berrange, 2016/09/12