[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] qdev property: cleanup
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH] qdev property: cleanup |
Date: |
Fri, 15 Apr 2016 12:06:32 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.1 |
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