Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties

2016-09-22 Thread Daniel P. Berrange
On Thu, Sep 22, 2016 at 10:36:45AM +0200, Markus Armbruster wrote:
> "Lin Ma"  writes:
> 
>  Markus Armbruster  2016/9/20 星期二 上午 1:13 >>>
> >>Andreas Färber  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,
  _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 

Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties

2016-09-22 Thread Daniel P. Berrange
On Thu, Sep 22, 2016 at 02:03:39PM +0200, Markus Armbruster wrote:
> "Daniel P. Berrange"  writes:
> >
> > IMHO we should go further and leverage QAPI schema to auto-generate all
> > the tedious boilerplate code for QOM objects
> >
> > eg, consider the crypto/secret.c object file.
> >
> > We could declare it as
> >
> > { 'object': 'QCryptoSecret',
> >   'parent': 'Object',
> >   'properties': {
> >  'format': 'QCryptoSecretFormat',
> >  'data': 'str',
> >  'file': 'str',
> >  'keyid': 'str',
> >  'iv': 'str'
> >  } }
> >
> > Based on that it would have enough knowledge to generate
> >
> >  - struct QCryptoSecret  definition + typedef
> >  - struct QCryptoSecretClass definition + typedef
> >  - TYPE_CRYPTO_SECRET macro
> >  - QCRYPT_SECRET() cast macro
> >  - Setters & getters aka
> >  qcrypto_secret_prop_set_format
> >  qcrypto_secret_prop_get_format
> >  qcrypto_secret_prop_set_data
> >  qcrypto_secret_prop_get_data
> >  qcrypto_secret_prop_set_file
> >  qcrypto_secret_prop_get_file
> >  qcrypto_secret_prop_set_keyid
> >  qcrypto_secret_prop_get_keyid
> >  qcrypto_secret_prop_set_iv
> >  qcrypto_secret_prop_get_iv
> >  - qcrypto_secret_finalize
> >  - qcrypto_secret_class_init
> >  - TypeInfo qcrypto_secret_info variable
> >  - qcrypto_secret_register_types() method
> >  - type_init(qcrypto_secret_register_types);
> >
> > That'd massively reduce the work to create new objects, to just
> > filling in the semantically useful logic
> 
> Promising idea.  Sadly, the existing QAPI queue is eating all my QAPI
> cycles.

Indeed, I've no time either. Perhaps this is something nice for a "lucky"
GSoC / OutReachy student :-) Copying Stefan

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

2016-09-22 Thread Markus Armbruster
"Daniel P. Berrange"  writes:

> On Thu, Sep 22, 2016 at 01:12:22PM +0200, Markus Armbruster wrote:
>> "Daniel P. Berrange"  writes:
>> 
>> > On Thu, Sep 22, 2016 at 10:36:45AM +0200, Markus Armbruster wrote:
>> >> 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.
>> >
>> > FWIW, we could easily optimize handling of description strings by
>> > applying the same trick that GLib has done for GObject property
>> > descriptions.
>> >
>> > Almost certainly every call to object_property_set_description is
>> > going to be passing a string literal, not a dynamically allocated
>> > string. So we take advantage of that and in fact mandate that it
>> > is a string literal, and thus avoid the strdup() of description.
>> >
>> > We can place a fun game to enforce this at compile time thus:
>> >
>> >  - Rename object_property_set_description() to
>> >object_property_set_description_internal()
>> >
>> >  - Add the macro
>> >
>> >   #define  object_property_set_description(obj, name, desc, errp) \
>> >  object_property_set_description_internal(obj, name, "" desc "", errp)
>> 
>> Cute :)
>> 
>> > None the less, we really should make an effort to switch things
>> > over to use class properties instead of instance properties, as
>> > its going to save us allocating a 64 byte struct per property
>> > per instance
>> 
>> Yes, please.
>> 
>> Related: a way to define a bunch of properties as *data*, i.e. an array
>> of property descriptions, commonly static.  Reasoning about static data
>> is so much easier than reasoning about code.
>
> IMHO we should go further and leverage QAPI schema to auto-generate all
> the tedious boilerplate code for QOM objects
>
> eg, consider the crypto/secret.c object file.
>
> We could declare it as
>
> { 'object': 'QCryptoSecret',
>   'parent': 'Object',
>   'properties': {
>  'format': 'QCryptoSecretFormat',
>  'data': 'str',
>  'file': 'str',
>  'keyid': 'str',
>  'iv': 'str'
>  } }
>
> Based on that it would have enough knowledge to generate
>
>  - struct QCryptoSecret  definition + typedef
>  - struct QCryptoSecretClass definition + typedef
>  - TYPE_CRYPTO_SECRET macro
>  - QCRYPT_SECRET() cast macro
>  - Setters & getters aka
>  qcrypto_secret_prop_set_format
>  qcrypto_secret_prop_get_format
>  qcrypto_secret_prop_set_data
>  qcrypto_secret_prop_get_data
>  qcrypto_secret_prop_set_file
>  qcrypto_secret_prop_get_file
>  qcrypto_secret_prop_set_keyid
>  qcrypto_secret_prop_get_keyid
>  qcrypto_secret_prop_set_iv
>  qcrypto_secret_prop_get_iv
>  - qcrypto_secret_finalize
>  - qcrypto_secret_class_init
>  - TypeInfo qcrypto_secret_info variable
>  - qcrypto_secret_register_types() method
>  - type_init(qcrypto_secret_register_types);
>
> That'd massively reduce the work to create new objects, to just
> filling in the semantically useful logic

Promising idea.  Sadly, the existing QAPI queue is eating all my QAPI
cycles.



Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties

2016-09-22 Thread Daniel P. Berrange
On Thu, Sep 22, 2016 at 01:12:22PM +0200, Markus Armbruster wrote:
> "Daniel P. Berrange"  writes:
> 
> > On Thu, Sep 22, 2016 at 10:36:45AM +0200, Markus Armbruster wrote:
> >> 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.
> >
> > FWIW, we could easily optimize handling of description strings by
> > applying the same trick that GLib has done for GObject property
> > descriptions.
> >
> > Almost certainly every call to object_property_set_description is
> > going to be passing a string literal, not a dynamically allocated
> > string. So we take advantage of that and in fact mandate that it
> > is a string literal, and thus avoid the strdup() of description.
> >
> > We can place a fun game to enforce this at compile time thus:
> >
> >  - Rename object_property_set_description() to
> >object_property_set_description_internal()
> >
> >  - Add the macro
> >
> >   #define  object_property_set_description(obj, name, desc, errp) \
> >  object_property_set_description_internal(obj, name, "" desc "", errp)
> 
> Cute :)
> 
> > None the less, we really should make an effort to switch things
> > over to use class properties instead of instance properties, as
> > its going to save us allocating a 64 byte struct per property
> > per instance
> 
> Yes, please.
> 
> Related: a way to define a bunch of properties as *data*, i.e. an array
> of property descriptions, commonly static.  Reasoning about static data
> is so much easier than reasoning about code.

IMHO we should go further and leverage QAPI schema to auto-generate all
the tedious boilerplate code for QOM objects

eg, consider the crypto/secret.c object file.

We could declare it as

{ 'object': 'QCryptoSecret',
  'parent': 'Object',
  'properties': {
 'format': 'QCryptoSecretFormat',
 'data': 'str',
 'file': 'str',
 'keyid': 'str',
 'iv': 'str'
 } }

Based on that it would have enough knowledge to generate

 - struct QCryptoSecret  definition + typedef
 - struct QCryptoSecretClass definition + typedef
 - TYPE_CRYPTO_SECRET macro
 - QCRYPT_SECRET() cast macro
 - Setters & getters aka
 qcrypto_secret_prop_set_format
 qcrypto_secret_prop_get_format
 qcrypto_secret_prop_set_data
 qcrypto_secret_prop_get_data
 qcrypto_secret_prop_set_file
 qcrypto_secret_prop_get_file
 qcrypto_secret_prop_set_keyid
 qcrypto_secret_prop_get_keyid
 qcrypto_secret_prop_set_iv
 qcrypto_secret_prop_get_iv
 - qcrypto_secret_finalize
 - qcrypto_secret_class_init
 - TypeInfo qcrypto_secret_info variable
 - qcrypto_secret_register_types() method
 - type_init(qcrypto_secret_register_types);

That'd massively reduce the work to create new objects, to just
filling in the semantically useful logic

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

2016-09-22 Thread Markus Armbruster
"Daniel P. Berrange"  writes:

> On Thu, Sep 22, 2016 at 10:36:45AM +0200, Markus Armbruster wrote:
>> 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.
>
> FWIW, we could easily optimize handling of description strings by
> applying the same trick that GLib has done for GObject property
> descriptions.
>
> Almost certainly every call to object_property_set_description is
> going to be passing a string literal, not a dynamically allocated
> string. So we take advantage of that and in fact mandate that it
> is a string literal, and thus avoid the strdup() of description.
>
> We can place a fun game to enforce this at compile time thus:
>
>  - Rename object_property_set_description() to
>object_property_set_description_internal()
>
>  - Add the macro
>
>   #define  object_property_set_description(obj, name, desc, errp) \
>  object_property_set_description_internal(obj, name, "" desc "", errp)

Cute :)

> None the less, we really should make an effort to switch things
> over to use class properties instead of instance properties, as
> its going to save us allocating a 64 byte struct per property
> per instance

Yes, please.

Related: a way to define a bunch of properties as *data*, i.e. an array
of property descriptions, commonly static.  Reasoning about static data
is so much easier than reasoning about code.



Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties

2016-09-22 Thread Daniel P. Berrange
On Thu, Sep 22, 2016 at 10:36:45AM +0200, Markus Armbruster wrote:
> 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.

FWIW, we could easily optimize handling of description strings by
applying the same trick that GLib has done for GObject property
descriptions.

Almost certainly every call to object_property_set_description is
going to be passing a string literal, not a dynamically allocated
string. So we take advantage of that and in fact mandate that it
is a string literal, and thus avoid the strdup() of description.

We can place a fun game to enforce this at compile time thus:

 - Rename object_property_set_description() to
   object_property_set_description_internal()

 - Add the macro

  #define  object_property_set_description(obj, name, desc, errp) \
 object_property_set_description_internal(obj, name, "" desc "", errp)


None the less, we really should make an effort to switch things
over to use class properties instead of instance properties, as
its going to save us allocating a 64 byte struct per property
per instance


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

2016-09-22 Thread Markus Armbruster
"Lin Ma"  writes:

 Markus Armbruster  2016/9/20 星期二 上午 1:13 >>>
>>Andreas Färber  writes:
>>
>>> Hi Lin and Markus,
>>>
>>> Am 19.09.2016 um 13:58 schrieb Markus Armbruster:
>>[...]
 You're messing with struct EnumProperty because you want more help than
 what ObjectPropertyInfo can provice.
 
 Digression on the meaning of ObjectPropertyInfo.  This is its
 definition:
 
 ##
 # @ObjectPropertyInfo:
 #
 # @name: the name of the property
 #
 # @type: the type of the property.  This will typically come in one of four
 #  forms:
 #
 #  1) A primitive type such as 'u8', 'u16', 'bool', 'str', or 'double'.
 # These types are mapped to the appropriate JSON type.
 #
 #  2) A child type in the form 'child' where subtype is a qdev
 # device type name.  Child properties create the composition 
 tree.
 #
 #  3) A link type in the form 'link' where subtype is a qdev
 # device type name.  Link properties form the device model 
 graph.
 #
 # Since: 1.2
 ##
 { 'struct': 'ObjectPropertyInfo',
   'data': { 'name': 'str', 'type': 'str' } }
 
 @type is documented to be either a primitive type, a child type or a
 link.  "Primitive type" isn't defined.  The examples given suggest it's
 a QAPI built-in type.  If that's correct, clause 1) does not cover
 enumeration types.  However, it doesn't seem to be correct: I observ
 'string', not 'str'.  Paolo, Andreas, any idea what @type is supposed to
 mean?
 
 End of digression.
 
 All ObjectPropertyInfo gives you is two strings: a name and a type.  If
 you need more information than that, you have to add a proper interface
 to get it!  Say a function that takes an object and a property name, and
 returns information about the property's type.  Probably should be two
 functions, one that maps QOM object and property name to the property's
 QOM type, one that maps a QOM type to QOM type information.
 
 In other words, you need QOM object and type introspection.  Paolo,
 Andreas, if that already exists in some form, please point us to it.
>>>
>>> Could you say what exactly you want to introspect here?
>>
>>Context: Lin wants to implement "-object TYPE-NAME,help", similar to
>>"-device DRIVER-NAME,help", i.e. list the available properties of
>>TYPE-NAME.
>>
>>His proposed patch tries to give better help for enumeration types by
>>listing the acceptable values.  The code that does it is an unacceptable
>>hack, though.  We're trying to figure out a way to provide such help
>>cleanly.
>>
>>One way to do it would be introspecting QOM *types*.  Find the
>>property's type, figure out what kind of type it is, if it's an
>>enumeration type, find its members, ...
>>
>>> Both ObjectClass and Object have a list of properties that together form
>>> the list of properties that can be set on an instance. So you'll need to
>>> instantiate the object like I think we do for devices. Then you can
>>> recursively enumerate the properties to get their names and types (and
>>> possibly put them into a new list for alphabetical sorting if desired).
>>
>>Lin's code uses object_new() to instantiate a dummy object,
>>object_property_iter_init() and object_property_iter_next() to find the
>>properties.  Sounds okay so far, doesn't it?
>>
>>Hmm, ObjectProperty has members name, type, description.  Could
>>description be useful?  I guess it's set with
>>object_property_set_description().  I can see only a few dozen calls,
>>which makes me suspect it's null more often than not.
>
> 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 

Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties

2016-09-21 Thread Lin Ma


>>> Markus Armbruster  2016/9/20 星期二 上午 1:13 >>>
>Andreas Färber  writes:
>
>> Hi Lin and Markus,
>>
>> Am 19.09.2016 um 13:58 schrieb Markus Armbruster:
>[...]
>>> You're messing with struct EnumProperty because you want more help than
>>> what ObjectPropertyInfo can provice.
>>> 
>>> Digression on the meaning of ObjectPropertyInfo.  This is its
>>> definition:
>>> 
>>> ##
>>> # @ObjectPropertyInfo:
>>> #
>>> # @name: the name of the property
>>> #
>>> # @type: the type of the property.  This will typically come in one of four
>>> #   forms:
>>> #
>>> #   1) A primitive type such as 'u8', 'u16', 'bool', 'str', or 'double'.
>>> #  These types are mapped to the appropriate JSON type.
>>> #
>>> #   2) A child type in the form 'child' where subtype is a qdev
>>> #  device type name.  Child properties create the composition 
>>> tree.
>>> #
>>> #   3) A link type in the form 'link' where subtype is a qdev
>>> #  device type name.  Link properties form the device model 
>>> graph.
>>> #
>>> # Since: 1.2
>>> ##
>>> { 'struct': 'ObjectPropertyInfo',
>>>   'data': { 'name': 'str', 'type': 'str' } }
>>> 
>>> @type is documented to be either a primitive type, a child type or a
>>> link.  "Primitive type" isn't defined.  The examples given suggest it's
>>> a QAPI built-in type.  If that's correct, clause 1) does not cover
>>> enumeration types.  However, it doesn't seem to be correct: I observ
>>> 'string', not 'str'.  Paolo, Andreas, any idea what @type is supposed to
>>> mean?
>>> 
>>> End of digression.
>>> 
>>> All ObjectPropertyInfo gives you is two strings: a name and a type.  If
>>> you need more information than that, you have to add a proper interface
>>> to get it!  Say a function that takes an object and a property name, and
>>> returns information about the property's type.  Probably should be two
>>> functions, one that maps QOM object and property name to the property's
>>> QOM type, one that maps a QOM type to QOM type information.
>>> 
>>> In other words, you need QOM object and type introspection.  Paolo,
>>> Andreas, if that already exists in some form, please point us to it.
>>
>> Could you say what exactly you want to introspect here?
>
>Context: Lin wants to implement "-object TYPE-NAME,help", similar to
>"-device DRIVER-NAME,help", i.e. list the available properties of
>TYPE-NAME.
>
>His proposed patch tries to give better help for enumeration types by
>listing the acceptable values.  The code that does it is an unacceptable
>hack, though.  We're trying to figure out a way to provide such help
>cleanly.
>
>One way to do it would be introspecting QOM *types*.  Find the
>property's type, figure out what kind of type it is, if it's an
>enumeration type, find its members, ...
>
>> Both ObjectClass and Object have a list of properties that together form
>> the list of properties that can be set on an instance. So you'll need to
>> instantiate the object like I think we do for devices. Then you can
>> recursively enumerate the properties to get their names and types (and
>> possibly put them into a new list for alphabetical sorting if desired).
>
>Lin's code uses object_new() to instantiate a dummy object,
>object_property_iter_init() and object_property_iter_next() to find the
>properties.  Sounds okay so far, doesn't it?
>
>Hmm, ObjectProperty has members name, type, description.  Could
>description be useful?  I guess it's set with
>object_property_set_description().  I can see only a few dozen calls,
>which makes me suspect it's null more often than not.

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, 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_cl
ass_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.
Any suggestions?
 
If this way makes sense, Then Should I add a member description for
ObjectPropertyInfo as well?
 
>
>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.
>
>Other ideas?




Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties

2016-09-19 Thread Markus Armbruster
Andreas Färber  writes:

> Hi Lin and Markus,
>
> Am 19.09.2016 um 13:58 schrieb Markus Armbruster:
[...]
>> You're messing with struct EnumProperty because you want more help than
>> what ObjectPropertyInfo can provice.
>> 
>> Digression on the meaning of ObjectPropertyInfo.  This is its
>> definition:
>> 
>> ##
>> # @ObjectPropertyInfo:
>> #
>> # @name: the name of the property
>> #
>> # @type: the type of the property.  This will typically come in one of four
>> #forms:
>> #
>> #1) A primitive type such as 'u8', 'u16', 'bool', 'str', or 'double'.
>> #   These types are mapped to the appropriate JSON type.
>> #
>> #2) A child type in the form 'child' where subtype is a qdev
>> #   device type name.  Child properties create the composition tree.
>> #
>> #3) A link type in the form 'link' where subtype is a qdev
>> #   device type name.  Link properties form the device model graph.
>> #
>> # Since: 1.2
>> ##
>> { 'struct': 'ObjectPropertyInfo',
>>   'data': { 'name': 'str', 'type': 'str' } }
>> 
>> @type is documented to be either a primitive type, a child type or a
>> link.  "Primitive type" isn't defined.  The examples given suggest it's
>> a QAPI built-in type.  If that's correct, clause 1) does not cover
>> enumeration types.  However, it doesn't seem to be correct: I observ
>> 'string', not 'str'.  Paolo, Andreas, any idea what @type is supposed to
>> mean?
>> 
>> End of digression.
>> 
>> All ObjectPropertyInfo gives you is two strings: a name and a type.  If
>> you need more information than that, you have to add a proper interface
>> to get it!  Say a function that takes an object and a property name, and
>> returns information about the property's type.  Probably should be two
>> functions, one that maps QOM object and property name to the property's
>> QOM type, one that maps a QOM type to QOM type information.
>> 
>> In other words, you need QOM object and type introspection.  Paolo,
>> Andreas, if that already exists in some form, please point us to it.
>
> Could you say what exactly you want to introspect here?

Context: Lin wants to implement "-object TYPE-NAME,help", similar to
"-device DRIVER-NAME,help", i.e. list the available properties of
TYPE-NAME.

His proposed patch tries to give better help for enumeration types by
listing the acceptable values.  The code that does it is an unacceptable
hack, though.  We're trying to figure out a way to provide such help
cleanly.

One way to do it would be introspecting QOM *types*.  Find the
property's type, figure out what kind of type it is, if it's an
enumeration type, find its members, ...

> Both ObjectClass and Object have a list of properties that together form
> the list of properties that can be set on an instance. So you'll need to
> instantiate the object like I think we do for devices. Then you can
> recursively enumerate the properties to get their names and types (and
> possibly put them into a new list for alphabetical sorting if desired).

Lin's code uses object_new() to instantiate a dummy object,
object_property_iter_init() and object_property_iter_next() to find the
properties.  Sounds okay so far, doesn't it?

Hmm, ObjectProperty has members name, type, description.  Could
description be useful?  I guess it's set with
object_property_set_description().  I can see only a few dozen calls,
which makes me suspect it's null more often than not.

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.

Other ideas?



Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties

2016-09-19 Thread Andreas Färber
Hi Lin and Markus,

Am 19.09.2016 um 13:58 schrieb Markus Armbruster:
> This is about QOM use.  Cc: Andreas in case he has smart ideas.
> Andreas, you may want to skip ahead to "EnumProperty".
> 
> "Lin Ma"  writes:
> 
> Markus Armbruster  2016/9/12 星期一 下午 11:42 >>>
>>> Lin Ma  writes:
>>>
 '-object help' prints available user creatable backends.
 '-object $typename,help' prints relevant properties.

 Signed-off-by: Lin Ma 
 ---
  include/qom/object_interfaces.h |   2 +
  qemu-options.hx |   7 ++-
  qom/object_interfaces.c | 112 
 
  vl.c   |   5 ++
  4 files changed, 125 insertions(+), 1 deletion(-)

> [...]
 diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
 index bf59846..4ee8643 100644
 --- a/qom/object_interfaces.c
 +++ b/qom/object_interfaces.c
 @@ -5,6 +5,7 @@
  #include "qapi-visit.h"
  #include "qapi/qmp-output-visitor.h"
  #include "qapi/opts-visitor.h"
 +#include "qemu/help_option.h"
  
  void user_creatable_complete(Object *obj, Error **errp)
  {
 @@ -212,6 +213,117 @@ void user_creatable_del(const char *id, Error **errp)
  object_unparent(obj);
  }
  
 +int user_creatable_help_func(void *opaque, QemuOpts *opts, Error **errp)
 +{
 +Visitor *v;
 +char *type = NULL;
 +Error *local_err = NULL;
 +
>>>
>>> Why this blank line?
>>>
>> I'll remove it.
>>
 +int i;
 +char *values = NULL;
 +Object *obj;
 +ObjectPropertyInfoList *props = NULL;
 +ObjectProperty *prop;
 +ObjectPropertyIterator iter;
 +ObjectPropertyInfoList *start;
 +
 +struct EnumProperty {
 +  const char * const *strings;
 +  int (*get)(Object *, Error **);
 +  void (*set)(Object *, int, Error **);
 +} *enumprop;
>>>
>>> Copied from object.c.  Big no-no.  Whatever you do with this probably
>>> belongs into object.c instead.
>>>
>> Yes, this way is ugly.
>> What I want to do is parsing the enum <-> string mappings through the 
>> EnumProperty
>> to make the output more reasonable.
>> It should be parsed in object.c, But I can't assume the size of enum str 
>> list, then can't
>> pre-alloc it in user_creatable_help_func. So far I havn't figured out a good 
>> way to return
>> a string array that including the enum str list to user_creatable_help_func 
>> for printing.
>> May I have your suggestions?
> 
> See below.
> 
 +
 +   
>>  v = opts_visitor_new(opts);
 +visit_start_struct(v, NULL, NULL, 0, _err);
 +if (local_err) {
 +  goto out;
 +}
 +
 +visit_type_str(v, "qom-type", , _err);
 +if (local_err) {
 +  goto out_visit;
 +}
 +
 +if (type && is_help_option(type)) {
>>>
>>> I think the Options visitor is overkill.  Much simpler:
>>>
>>>type = qemu_opt_get(opts, "qom-type");
>>>if (type && is_help_option(type)) {
>>>
>> Yes, it is much simpler, I'll use this instead of visitor code.
>>
 +  printf("Available object backend types:\n");
 +  GSList *list = object_class_get_list(TYPE_USER_CREATABLE, false);
 +  while (list) {
 +  const char *name;
 +  name = object_class_get_name(OBJECT_CLASS(list->data));
 +  if (strcmp(name, TYPE_USER_CREATABLE)) {
>>>
>>> Why do you need to skip TYPE_USER_CREATABLE?
>>>
>>> Hmm...
>>>
>>>$ qemu-system-x86_64 -object user-creatable,id=foo
>>>**
>>>ERROR:/work/armbru/qemu/qom/object.c:361:object_initialize_with_type: 
>>> assertion failed (type->instance_size >= sizeof(Object)): (0 >= 40)
>>>Aborted (core dumped)
>>>
>>> Should this type be abstract?
>>>
>> Yes, The object type user-creatable is abstract, But obviously it missed the 
>> abstract property.
>> I'll add it in next patch(patch 1/2), Then I dont need to skip it at here 
>> anymore.
> 
> Yes, please.
> 
 +  printf("%s\n", name);
 +  }
 +  list = list->next;
 +  }
>>>
>>> Recommend to keep the loop control in one place:
>>>
>>>for (list = object_class_get_list(TYPE_USER_CREATABLE, 
>>> false);
>>> list;
>>> list = list->next) {
>>>...
>>>}
>>>
>>> If you hate multi-line for (...), you can do
>>>
>>>GSList *head = object_class_get_list(TYPE_USER_CREATABLE, 
>>> false);
>>>
>>>for (list = head; list; list->next) {
>>>...
>>>}
>>>
>> Will do it.
>>
 +  g_slist_free(list);
 +  goto out_visit;
 +}
 +
 +if (!type || !qemu_opt_has_help_opt(opts)) {

Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties

2016-09-19 Thread Markus Armbruster
This is about QOM use.  Cc: Andreas in case he has smart ideas.
Andreas, you may want to skip ahead to "EnumProperty".

"Lin Ma"  writes:

 Markus Armbruster  2016/9/12 星期一 下午 11:42 >>>
>>Lin Ma  writes:
>>
>>> '-object help' prints available user creatable backends.
>>> '-object $typename,help' prints relevant properties.
>>>
>>> Signed-off-by: Lin Ma 
>>> ---
>>>  include/qom/object_interfaces.h |   2 +
>>>  qemu-options.hx  |   7 ++-
>>>  qom/object_interfaces.c  | 112 
>>>  vl.c|   5 ++
>>>  4 files changed, 125 insertions(+), 1 deletion(-)
>>>
[...]
>>> diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
>>> index bf59846..4ee8643 100644
>>> --- a/qom/object_interfaces.c
>>> +++ b/qom/object_interfaces.c
>>> @@ -5,6 +5,7 @@
>>>  #include "qapi-visit.h"
>>>  #include "qapi/qmp-output-visitor.h"
>>>  #include "qapi/opts-visitor.h"
>>> +#include "qemu/help_option.h"
>>>  
>>>  void user_creatable_complete(Object *obj, Error **errp)
>>>  {
>>> @@ -212,6 +213,117 @@ void user_creatable_del(const char *id, Error **errp)
>>>   object_unparent(obj);
>>>  }
>>>  
>>> +int user_creatable_help_func(void *opaque, QemuOpts *opts, Error **errp)
>>> +{
>>> +Visitor *v;
>>> +char *type = NULL;
>>> +Error *local_err = NULL;
>>> +
>>
>>Why this blank line?
>>
> I'll remove it.
>
>>> +int i;
>>> +char *values = NULL;
>>> +Object *obj;
>>> +ObjectPropertyInfoList *props = NULL;
>>> +ObjectProperty *prop;
>>> +ObjectPropertyIterator iter;
>>> +ObjectPropertyInfoList *start;
>>> +
>>> +struct EnumProperty {
>>> +   const char * const *strings;
>>> +   int (*get)(Object *, Error **);
>>> +   void (*set)(Object *, int, Error **);
>>> +} *enumprop;
>>
>>Copied from object.c.  Big no-no.  Whatever you do with this probably
>>belongs into object.c instead.
>>
> Yes, this way is ugly.
> What I want to do is parsing the enum <-> string mappings through the 
> EnumProperty
> to make the output more reasonable.
> It should be parsed in object.c, But I can't assume the size of enum str 
> list, then can't
> pre-alloc it in user_creatable_help_func. So far I havn't figured out a good 
> way to return
> a string array that including the enum str list to user_creatable_help_func 
> for printing.
> May I have your suggestions?

See below.

>>> +
>>> +   
>  v = opts_visitor_new(opts);
>>> +visit_start_struct(v, NULL, NULL, 0, _err);
>>> +if (local_err) {
>>> +   goto out;
>>> +}
>>> +
>>> +visit_type_str(v, "qom-type", , _err);
>>> +if (local_err) {
>>> +   goto out_visit;
>>> +}
>>> +
>>> +if (type && is_help_option(type)) {
>>
>>I think the Options visitor is overkill.  Much simpler:
>>
>> type = qemu_opt_get(opts, "qom-type");
>> if (type && is_help_option(type)) {
>>
> Yes, it is much simpler, I'll use this instead of visitor code.
>
>>> +   printf("Available object backend types:\n");
>>> +   GSList *list = object_class_get_list(TYPE_USER_CREATABLE, false);
>>> +   while (list) {
>>> +   const char *name;
>>> +   name = object_class_get_name(OBJECT_CLASS(list->data));
>>> +   if (strcmp(name, TYPE_USER_CREATABLE)) {
>>
>>Why do you need to skip TYPE_USER_CREATABLE?
>>
>>Hmm...
>>
>>$ qemu-system-x86_64 -object user-creatable,id=foo
>>**
>>ERROR:/work/armbru/qemu/qom/object.c:361:object_initialize_with_type: 
>> assertion failed (type->instance_size >= sizeof(Object)): (0 >= 40)
>>Aborted (core dumped)
>>
>>Should this type be abstract?
>>
> Yes, The object type user-creatable is abstract, But obviously it missed the 
> abstract property.
> I'll add it in next patch(patch 1/2), Then I dont need to skip it at here 
> anymore.

Yes, please.

>>> +   printf("%s\n", name);
>>> +   }
>>> +   list = list->next;
>>> +   }
>>
>>Recommend to keep the loop control in one place:
>>
>> for (list = object_class_get_list(TYPE_USER_CREATABLE, 
>> false);
>>  list;
>>  list = list->next) {
>> ...
>> }
>>
>>If you hate multi-line for (...), you can do
>>
>> GSList *head = object_class_get_list(TYPE_USER_CREATABLE, 
>> false);
>>
>> for (list = head; list; list->next) {
>> ...
>> }
>>
> Will do it.
>
>>> +   g_slist_free(list);
>>> +   goto out_visit;
>>> +}
>>> +
>>> +if (!type || !qemu_opt_has_help_opt(opts)) {
>>> +   visit_end_struct(v, NULL);
>>> +   return 0;
>>> +}
>>> +
>>> +if (!object_class_by_name(type)) {
>>> +   printf("invalid object type: %s\n", type);
>>> +   goto out_visit;
>>> +}
>>> +obj = 

[Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties

2016-09-17 Thread Lin Ma


>>> "Daniel P. Berrange"  2016/9/12 星期一 下午 11:56 >>>
>On Sun, Sep 11, 2016 at 01:53:01PM +0800, Lin Ma wrote:
>> '-object help' prints available user creatable backends.
>> '-object $typename,help' prints relevant properties.
>> 
>> Signed-off-by: Lin Ma 
>> ---
>>  include/qom/object_interfaces.h |   2 +
>>  qemu-options.hx   |   7 ++-
>>  qom/object_interfaces.c   | 112 
>>  vl.c |   5 ++
>>  4 files changed, 125 insertions(+), 1 deletion(-)
>
>> diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
>> index bf59846..4ee8643 100644
>> --- a/qom/object_interfaces.c
>> +++ b/qom/object_interfaces.c
>> @@ -5,6 +5,7 @@
>>  #include "qapi-visit.h"
>>  #include "qapi/qmp-output-visitor.h"
>>  #include "qapi/opts-visitor.h"
>> +#include "qemu/help_option.h"
>>  
>>  void user_creatable_complete(Object *obj, Error **errp)
>>  {
>> @@ -212,6 +213,117 @@ void user_creatable_del(const char *id, Error **errp)
>>object_unparent(obj);
>>  }
>>  
>> +int user_creatable_help_func(void *opaque, QemuOpts *opts, Error **errp)
>> +{
>> +Visitor *v;
>> +char *type = NULL;
>> +Error *local_err = NULL;
>> +
>> +int i;
>> +char *values = NULL;
>> +Object *obj;
>> +ObjectPropertyInfoList *props = NULL;
>> +ObjectProperty *prop;
>> +ObjectPropertyIterator iter;
>> +ObjectPropertyInfoList *start;
>> +
>> +struct EnumProperty {
>> +const char * const *strings;
>> +int (*get)(Object *, Error **);
>> +void (*set)(Object *, int, Error **);
>> +} *enumprop;
>
>Ewww, this struct is declared privately in object.c and
>you're declaring it here so you can poke at private
>data belonging to the object internal impl. This is
>not ok in any way.
>
Yes, this way is ugly.
What I want to do is parsing the enum <-> string mappings through the 
EnumProperty
to make the output more reasonable.
It should be parsed in object.c, But I can't assume the size of enum str list, 
then can't
pre-alloc it in user_creatable_help_func. So far I havn't figured out a good 
way to return
a string array that including the enum str list to user_creatable_help_func for 
printing.
May I have your suggestions?

>> +v = opts_visitor_new(opts);
>> +visit_start_struct(v, NULL, NULL, 0, _err);
>> +if (local_err) {
>> +goto out;
>> +}
>> +
>> +visit_type_str(v, "qom-type", , _err);
>> +if (local_err) {
>> +goto out_visit;
>> +}
>> +
>> +if (type && is_help_option(type)) {
>> +printf("Available object backend types:\n");
>> +GSList *list = object_class_get_list(TYPE_USER_CREATABLE, false);
>> +while (list) {
>> +const char *name;
>> +name = object_class_get_name(OBJECT_CLASS(list->data));
>> +if (strcmp(name, TYPE_USER_CREATABLE)) {
>> +printf("%s\n", name);
>> +}
>> +list = list->next;
>> +}
>> +g_slist_free(list);
>> +goto out_visit;
>> +}
>> +
>> +if (!type || !qemu_opt_has_help_opt(opts)) {
>> +visit_end_struct(v, NULL);
>> +return 0;
>> +}
>> +
>> +if (!object_class_by_name(type)) {
>> +printf("invalid object type: %s\n", type);
>> +goto out_visit;
>> +}
>> +obj = object_new(type);
>> +object_property_iter_init(, obj);
>> +
>> +while ((prop = object_property_iter_next())) {
>> +ObjectPropertyInfoList *entry = g_malloc0(sizeof(*entry));
>> +entry->value = g_malloc0(sizeof(ObjectPropertyInfo));
>> +entry->next = props;
>> +props = entry;
>> +entry->value->name = g_strdup(prop->name);
>> +i = 0;
>> +enumprop = prop->opaque;
>> +if (!g_str_equal(prop->type, "string") && \
>> +!g_str_equal(prop->type, "bool") && \
>> +!g_str_equal(prop->type, "struct tm") && \
>> +!g_str_equal(prop->type, "int") && \
>> +!g_str_equal(prop->
type, "uint8") && \
>> +!g_str_equal(prop->type, "uint16") && \
>> +!g_str_equal(prop->type, "uint32") && \
>> +!g_str_equal(prop->type, "uint64")) {
>
>It is absolutely *not* safe to assume that the result of
>this condition is an enum.
>
Yes, It does not make sense and is not safe. Writing this way becasue
I can't figure out a way to judge whether the type is an enum or not.
May I have your ideas?

>> +while (enumprop->strings[i] != NULL) {
>> +if (i != 0) {
>> +values = g_strdup_printf("%s/%s",
>> +
>> values, enumprop->strings[i]);
>
>Leaking the old memory for 'values'.
>
Ok, I'll fix it.
>> +} else {
>> + 

[Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties

2016-09-17 Thread Lin Ma


>>> Markus Armbruster  2016/9/12 星期一 下午 11:42 >>>
>Lin Ma  writes:
>
>> '-object help' prints available user creatable backends.
>> '-object $typename,help' prints relevant properties.
>>
>> Signed-off-by: Lin Ma 
>> ---
>>  include/qom/object_interfaces.h |   2 +
>>  qemu-options.hx   |   7 ++-
>>  qom/object_interfaces.c   | 112 
>>  vl.c |   5 ++
>>  4 files changed, 125 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/qom/object_interfaces.h 
>> b/include/qom/object_interfaces.h
>> index 8b17f4d..197cd5f 100644
>> --- a/include/qom/object_interfaces.h
>> +++ b/include/qom/object_interfaces.h
>> @@ -165,4 +165,6 @@ int user_creatable_add_opts_foreach(void *opaque,
>>   */
>>  void user_creatable_del(const char *id, Error **errp);
>>  
>> +int user_creatable_help_func(void *opaque, QemuOpts *opts, Error **errp);
>> +
>>  #endif
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index a71aaf8..fade53c 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -3752,7 +3752,9 @@ DEF("object", HAS_ARG, QEMU_OPTION_object,
>>"  create a new object of type TYPENAME setting 
>> properties\n"
>>"  in the order they are specified.  Note that 
>> the 'id'\n"
>>"  property must be set.  These objects are 
>> placed in the\n"
>> -"  '/objects' path.\n",
>> +"  '/objects' path.\n"
>> +"  Use '-object help' to print available 
>> backend types and\n"
>> +"  '-object typename,help' to print relevant 
>> properties.\n",
>>QEMU_ARCH_ALL)
>>  STEXI
>>  @item -object @var{typename}[,@var{prop1}=@var{value1},...]
>> @@ -3762,6 +3764,9 @@ in the order they are specified.  Note that the 'id'
>>  property must be set.  These objects are placed in the
>>  '/objects' path.
>>  
>> +Use '-object help' to print available backend types and
>> +'-object typename,help' to print relevant properties.
>> +
>
>Make that
>
>  +Use @code{-object help} to print available backend types and
>  +@code{-object @var{typename},help} to print relevant properties.
>  +
>
Ok.

>>  @table @option
>>  
>>  @item -object 
>> memory-backend-file,id=@var{id},size=@var{size},mem-path=@var{dir},share=@var{on|off}
>> diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
>> index bf59846..4ee8643 100644
>> --- a/qom/object_interfaces.c
>> +++ b/qom/object_interfaces.c
>> @@ -5,6 +5,7 @@
>>  #include "qapi-visit.h"
>>  #include "qapi/qmp-output-visitor.h"
>>  #include "qapi/opts-visitor.h"
>> +#include "qemu/help_option.h"
>>  
>>  void user_creatable_complete(Object *obj, Error **errp)
>>  {
>> @@ -212,6 +213,117 @@ void user_creatable_del(const char *id, Error **errp)
>>object_unparent(obj);
>>  }
>>  
>> +int user_creatable_help_func(void *opaque, QemuOpts *opts, Error **errp)
>> +{
>> +Visitor *v;
>> +char *type = NULL;
>> +Error *local_err = NULL;
>> +
>
>Why this blank line?
>
I'll remove it.

>> +int i;
>> +char *values = NULL;
>> +Object *obj;
>> +ObjectPropertyInfoList *props = NULL;
>> +ObjectProperty *prop;
>> +ObjectPropertyIterator iter;
>> +ObjectPropertyInfoList *start;
>> +
>> +struct EnumProperty {
>> +const char * const *strings;
>> +int (*get)(Object *, Error **);
>> +void (*set)(Object *, int, Error **);
>> +} *enumprop;
>
>Copied from object.c.  Big no-no.  Whatever you do with this probably
>belongs into object.c instead.
>
Yes, this way is ugly.
What I want to do is parsing the enum <-> string mappings through the 
EnumProperty
to make the output more reasonable.
It should be parsed in object.c, But I can't assume the size of enum str list, 
then can't
pre-alloc it in user_creatable_help_func. So far I havn't figured out a good 
way to return
a string array that including the enum str list to user_creatable_help_func for 
printing.
May I have your suggestions?

>> +
>> +
 v = opts_visitor_new(opts);
>> +visit_start_struct(v, NULL, NULL, 0, _err);
>> +if (local_err) {
>> +goto out;
>> +}
>> +
>> +visit_type_str(v, "qom-type", , _err);
>> +if (local_err) {
>> +goto out_visit;
>> +}
>> +
>> +if (type && is_help_option(type)) {
>
>I think the Options visitor is overkill.  Much simpler:
>
>  type = qemu_opt_get(opts, "qom-type");
>  if (type && is_help_option(type)) {
>
Yes, it is much simpler, I'll use this instead of visitor code.

>> +printf("Available object backend types:\n");
>> +GSList *list = object_class_get_list(TYPE_USER_CREATABLE, false);
>> +while (list) {
>> +const char *name;
>> +name = object_class_get_name(OBJECT_CLASS(list->data));
>> +