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

[Markus Armbruster]

"Lin Ma" <address@hidden> writes:

>>>> Markus Armbruster <address@hidden> 2016/9/20 星期二 上午 1:13 >>>
>>Andreas Färber <address@hidden> 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<subtype>' where subtype is a qdev
>>>> #             device type name.  Child properties create the composition 
>>>> tree.
>>>> #
>>>> #      3) A link type in the form 'link<subtype>' 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 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.

Unfortunately, I can't see a "property type" abstraction in the code.
Perhaps Andreas or Paolo can provide some guidance.

> If this way makes sense, Then Should I add a member description for
> ObjectPropertyInfo as well?

I suspect you won't even need ObjectPropertyInfo here once you've fused
your loops.

ObjectPropertyInfo is a QAPI/QMP thing.  Adding a description to it
needs to make sense for QMP.  I haven't thought about it.

>>Other ideas?