Marc-André Lureau <[email protected]> writes:

> Hi
>
> On Wed, Jul 29, 2026 at 11:29 AM Markus Armbruster via qemu
> development <[email protected]> wrote:
>> Before I dive into individual patches, let me try to work out what the
>> series does as a whole.
>>
>> > This series adds:
>> >   - A new QAPITypeInfo struct that pairs a property with its QAPI schema
>> >     type name, enum lookup table, and list-element type.
>>
>> Peeking at the code, I see that ObjectProperty gains a member @qapi_type
>> pointing to its QAPITypeInfo.
>>
>> It is null when the ObjectProperty doesn't have a QAPI type.
>>
>> If it's non-null, then ObjectProperty members @name and @type are
>> redundant with qapi_type.name and .type.
>>
>> Correct?
>
> Almost. @name is the property name (e.g. "policy"), not the type name.
> But @type becomes redundant with qapi_type->name when qapi_type is set
> (object_property_add_qapi derive prop->type from qapi_type->name).
>
> Also @type carries additional information for some property kinds that
> @qapi_type doesn't cover: child<T> and link<T> embed the linked
> object's QOM type name in the type string. qapi_type can't represent
> that atm.

Can child and link properties have non-null @qapi_type?

>> Would "every ObjectProperty has a QAPI type" be a reasonable goal for
>> the future?
>
> Maybe? we would need to address the child<>/link<> gap. At least we
> can make raw object_property_add() deprecated/static in object.c after
> this series

Interesting!

>> >   - A QAPI code generator (qapi-type-infos) that emits a QAPITypeInfo
>> >     instance for every schema-defined type, including the mapping
>> >     between internal C names and the schema name visible to clients.
>>
>> Peeking at the code, I find:
>>
>> * The type
>>
>>     typedef struct QAPITypeInfo {
>>         const char *name;
>>         const char *schema_name;
>>         const QEnumLookup *lookup;
>>         const struct QAPITypeInfo *list;
>>     } QAPITypeInfo;
>>
>> * A T_type_info for each QAPI type T, including built-in types.
>>
>> * T_type_info member @name is T's QAPI name, i.e. "T".
>>
>> * T_type_info member @schema_name is T's masked name used in
>>   query-qmp-schema output, null when T is elided there.
>>
>> * T_type_info member @list points to TList_type_info when that exists,
>>   else it's null.
>>
>> * T_type_info member @lookup points to T_lookup when T is an enum, else
>>   it's null.
>>
>> Correct?
>
> Yes
>
>> >   - A "qapi-type" field in the ObjectPropertyInfo and
>> >     ObjectPropertyValue QMP structs, populated from the QAPITypeInfo
>> >     when present giving clients a cross-reference into query-qmp-schema
>> >     output.
>>
>> To be precise: when ObjectPropertyInfo member @type is "T", then member
>> @qapi-type is T_type_info.qapi-type.  Correct?
>
> @qapi-type is T_type_info.schema_name, the masked name

Right.

>> If .qapi-type is non-null, you can use it to look up precise type
>> information via QAPI introspection, i.e. query-qmp-schema.
>>
>> Correct?
>
> Yes
>
>>
>> Possible problem: query-qmp-schema covers only types that are actually
>> used in QMP.  But the above technique additionally wants QOM property
>> types.  I haven't checked what your series does about this, if anything.
>
> Most types used as QOM properties are also used by QMP commands. But
> it's true that types not used by QMP get schame_name = NULL atm.

So the problem is real, and to reap the full benefit of your work, we
need to solve it.  Not necessarily right away.

> Should we have a new pragma? Even if we have conditions, we may end up
> with unused types in the build, but that shouldn't be a big issue. Or
> we would need a more complicated several step build.

No need not worry about the how right now.

>> ObjectPropertyInfo is only used with QMP command handlers.  It is
>> computed from ObjectProperty.
>>
>> >   - Conversion of all PropertyInfo definitions from the old
>> >     .type/.enum_table strings to the new .qapi_type pointer.
>>
>> The above is QOM, this is qdev.
>>
>> Like ObjectProperty, PropertyInfo gains a member @qapi_type pointing to
>> its QAPITypeInfo.  Howver, this one cannot be null.
>>
>> PropertyInfo members @type and @enum_table are dropped, because they are
>> redundant with qapi_type.type and .lookup.
>>
>> Correct?
>
> Almost. Every PropertyInfo has a non-NULL qapi_type, except the array
> ones (created with DEFINE_PROP_ARRAY_INFO) which leave .qapi_type NULL
> and derive it at registration time from
> .element_info->qapi_type->list.

Can you point me to the code setting it?  Would save me the digging.

>                                 Either way, the resulting
> ObjectProperty always ends up with a non-NULL qapi_type.
>
> PropertyInfo members @type and @enum_table are dropped, because they
> are redundant with qapi_type->name and ->lookup.

Yes.

>> >   - Replacement of the generic qdev_prop_array with typed per-element
>> >     array PropertyInfos, removing the arrayinfo/arrayfieldsize
>> >     indirection from struct Property.
>>
>> Before the series, an array-valued Property's @info member is
>> @qdev_prop_array.  @qdev_prop_array provides no information on the array
>> elements.  Instead, Property member @arrayinfo points to the
>> PropertyInfo for the elements, and @arrayfieldsize is the size of an
>> element.
>>
>> Your series makes PropertyInfo array-capable: new members @element_info
>> and @element_size are the elements' PropertyInfo and size.  It then adds
>> a proper PropertyInfo for each such property, and drops
>> @qdev_prop_array.
>>
>> Correct?
>
> Yes
>
>>
>> This could perhaps be spun out and merged separately to reduce the size
>> of future respins.
>
> I can split it out if it helps

Splitting off self-contained parts of a big series can help if they can
be merged quicker than the entire series.  We'll see.

>> Not mentioned:
>>
>>     - New QOM property creation functions for creating properties of
>>       QAPI type.  These take a QAPITypeInfo.
>>
>>     - Convert some properties to use them.
>>
>> >   - Removal of the deprecated PropertyInfo.type and .enum_table fields,
>> >     and of the old object_property_add_enum/add_tm APIs.
>> >
>> > Along the way, a few pre-existing type mismatches in property
>> > definitions are fixed, the "struct tm" RTC property is replaced with a
>> > proper QAPI StructTm type etc. Introducing more specific types or a
>> > "typedef" to QAPI could help provide better associated type informations
>> > than plain "str" in many cases, for example.
>>
>> Examples?
>>
>
> Properties using str_type_info that carry structured values with their
> own validation: macaddr ("52:54:00:12:34:56"), PCI addresses
> ("04:00.0"), netdev names, chardev names, drive names, UUID strings. A
> QAPI "typedef" (or newtype) for e.g. MacAddr, PciDevAddr, UUID could
> let tools know these aren't arbitrary strings (see "hw/nvdimm: convert
> UUID property to QAPI-aware registration" patch).

Two separate kinds:

1. Strings we need to parse: PCI addresses, MAC addresses, UUIDs, ...

   Parsing maps the string to something else, usually some object that
   isn't a string.

   We avoid string parsing in QAPI/QMP whenever practical.  You quoted
   several examples where we don't.

2. IDs of things we need to resolve

   Resolving maps the string to the object it names.

In both cases, we map strings to objects.

QAPI is unaware of this.  It simply uses strings in generated C.

What if QAPI was aware?  If the schema specified the object type, and
how to map from string to it?  This is a step beyond a mere typedef.
The mapping would move from handwritten code to generated code.
Generated interfaces would use the object types instead of strings.

Not now, of course.

>> > Comments welcome!
>> >
>> > Signed-off-by: Marc-André Lureau <[email protected]>
>>


Reply via email to