Re: [Qemu-devel] [PATCH] qdev property: cleanup

[Paolo Bonzini <pbonzini@redhat.com>]

On 12/04/2016 10:20, Markus Armbruster wrote:
> QOM replaced the static DeviceInfo.  The static qdev property arrays
> remain, but they're now stored into their DeviceClass's member props
> dynamically, by the TypeInfo's class_init() method.
> 
> The qdev properties so stored get mapped to QOM properties in
> device_initfn().  Two, in fact: a legacy property and a static property.
> qdev_property_add_legacy() converts to the former, and
> qdev_property_add_static() converts to the latter.

So far so good. :)

> I don't understand the difference between "legacy" and "static" well
> enough to explain it.  I also don't really understand what other kinds
> of QOM properties exist.

The "legacy" properties are needed for "info qtree".  For example, a PCI
address "01.0" is the number 8 in the normal property, and the string
"01.0" in the legacy property.

The normal property does accept a string, and converts it into the
number, but if you want the "human-readable" string back you need to use
the legacy properties.

>> original: *The actual type of the property and the field depends on
>> the property type*
>>
>> Using two same word 'property' is ambiguous and hard for newbie to
>> distinguish. The 1st 'property' should mean a QOM property. and the
>> 2nd 'property', I think the original author`s meaning is: qdev
>> property.

The type of the QOM property depends on the PropertyInfo and it's stored
in prop->info->name.

QType is, generally speaking, a JSON type name (int, bool, string, list,
dictionary).  Instead, the QOM property type is more complex; it can be:
1) a QAPI type name 2) link<QOM-TYPE> 3) child<QOM-TYPE>.

QAPI type names in turns are a superset of JSON type names.  For example
the QType for an enumeration could be QTYPE_QSTRING (yes, it has
prop->qtype equal to QTYPE_QINT now, but that's an accident).  The QAPI
type name instead could be "LostTickPolicy".  In the future, the latter
would also be introspectable through the "query-qmp-schema".

There is no reason for prop->qtype to be QTYPE_QINT in the case of a
string.  It would be easy to fix.  For example you could remove
prop->qtype and add a new prop->info->qtype that contains one of
QTYPE_QINT, QTYPE_QBOOL or, for enumerations, QTYPE_QSTRING.

> I agree the wording of the comment is suboptimal.
> 
> The thing being added is a static QOM property.  Its type and so forth
> are taken from the qdev property @prop.
> 
>>           But, what is the qdev property *type*? cannot find 'type'
>> field in the definition except a *qtype*

It is prop->info->name: PropertyInfo is a description of the property,
and the name of the PropertyInfo becomes the type of the QOM property.

>> struct Property {
>>     const char   *name;
>>     PropertyInfo *info;
>>     ptrdiff_t    offset;
>>     uint8_t      bitnr;
>>     QType        qtype;
>>     int64_t      defval;
>>     int          arrayoffset;
>>     PropertyInfo *arrayinfo;
>>     int          arrayfieldsize;
>> };
>>
>> And *the actual type of the field* depends on the qtype

It doesn't depend on the qtype.  It is stored there for convenience.

> Here's my try rewording the comment (sticking to GTK-Doc conventions
> even though I dislike them):
> 
> /**
>  * @qdev_property_add_static:
>  * @dev: Device to add the property to
>  * @prop: The qdev property definition
>  * @err: location to store error information
>  *
>  * Add a static QOM property to @dev for qdev property @prop.
>  * On error, store error in @errp.
>  */
> 
> Paolo, is it correct?

Yes, it is.  You can add also "The type of the QOM property is derived
from prop->info", which is what Cao Jin really wanted to convey in his
patch.

Paolo