I'm reviewing just the documentation right now. I'll look at the remainder later.
Documentation updates are commonly at least as hard as generator changes, if not harder. Please don't be discouraged by my remarks! Marc-André Lureau <[email protected]> writes: > New QAPISchemaGenTypeInfoVisitor produces per-module > qapi-type-infos-*.h/c files. Each file declares QAPITypeInfo constants > pairing the QAPI type name with its masked introspection name. > > Signed-off-by: Marc-André Lureau <[email protected]> > --- > docs/devel/qapi-code-gen.rst | 107 +++++++++++++++++++++++++- > meson.build | 1 + > scripts/qapi/backend.py | 2 + > scripts/qapi/introspect.py | 2 +- > scripts/qapi/type_infos.py | 178 > +++++++++++++++++++++++++++++++++++++++++++ > 5 files changed, 285 insertions(+), 5 deletions(-) > > diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst > index b1cc5b5f0db1..d176238fc2ef 100644 > --- a/docs/devel/qapi-code-gen.rst > +++ b/docs/devel/qapi-code-gen.rst > @@ -1247,10 +1247,10 @@ meaningful type names instead. This is in section "Client JSON Protocol introspection", which explains query-qmp-schema. A bit more context: Command and event names are part of the wire ABI, but type names are not. Therefore, the SchemaInfo for types have auto-generated meaningless names. For readability, the examples in this section use meaningful type names instead. > Optional member "features" exposes the entity's feature strings as a > JSON array of strings. > > -To examine a type, start with a command or event using it, then follow > -references by name. > - > -QAPI schema definitions not reachable that way are omitted. > +To examine a type used by QMP, start with a command or event using it, > +then follow references by name. User-defined and array types are also > +included when they are not reachable from a command or event, as they > +may also be used by QOM. The "To examine a type ..." sentence tries to tell readers not to look up types by name. Perhaps we can do without, because how would you even do that? It also prepares for "QAPI schema definitions not reachable that way are omitted." But you delete that. The deletion should go into "[PATCH 04] qapi: register all introspectable types, not just QMP-reachable ones". The final sentence talks about types not reachable from commands or events, and hints at QOM use. I feel the former is a distraction here. Regarding the latter: we need to say more about QOM introspection than "[types] may also be used by QOM". Before this series, query-qmp-schema is the only QAPI introspection game in town, and this section explains it. Afterwards, there's also qom-list, qom-list-properties, qom-list-get. I fear doing them justice will require some rewriting. Maybe just add a FIXME comment for now? Two spaces between sentences for local consistency, please. > The SchemaInfo for a command has meta-type "command", and variant > members "arg-type", "ret-type" and "allow-oob". On the wire, the > @@ -2101,3 +2101,102 @@ Example:: > })); > > [Uninteresting stuff omitted...] > + > + > +Code generated for type information > +----------------------------------- > + > +Type info is generated for user-defined and array types. With option > +``--builtins``, it is also generated for built-in types and the internal > +``QType`` enum. QType is also a built-in type. It isn't reall internal: while it's not currently used in the QAPI schema, it it is used outside the QAPI subsystem, e.g. in qobject/qobject.h. > + Other implicit types are skipped. The term "implicit type" is not really defined. It's only mentioned under "Client JSON Protocol introspection". Do we need this sentence? What about this: Type information is generated for user-defined types and their array types. With option ``--builtins``, it is also generated for the built-in types. By the way, we don't put `` around --builtins elsewhere. Should we? > + > +The following files are created: > + > + ``$(prefix)qapi-type-infos.c`` > + A ``QAPITypeInfo`` instance for each type listed above, providing > + a mapping between the QAPI type name and the masked name used by > + introspection, along with optional enum lookup table and array > + type pointers. > + > + ``$(prefix)qapi-type-infos.h`` > + Declarations for the above type info instances > + > +Each ``QAPITypeInfo`` struct has the following fields: > + > +``name`` > + The QAPI type name (e.g. ``"UserDefOne"``). QOM uses this as the > + property type string. Is "QOM uses this" relevant here? Permit me a digression. QAPI introspection masks type names, because they're not ABI. Exposing things not meant to be ABI invites misuse, and then misuse becomes accidental ABI. QOM exposes unmasked type names. I don't like it. > + > +``masked_name`` > + For user-defined types, the masked name used in > + ``query-qmp-schema`` output. For array types, the corresponding > + bracketed introspection name. Built-in types use their QAPI name; It's "the masked name used in query-qmp-schema output" for *all* types, not just user-defined types. An array's masked name is indeed the element's masked name enclosed in brackets, but this document carefully avoids to specify that. Closest we get is this under "Client JSON Protocol introspection": For convenience, the array's name may resemble the element type; however, clients should examine member "element-type" instead of making assumptions based on parsing member "name". Let's not specify more. > + note that introspection canonicalizes the integer built-in types > + to ``int``. Yes. Also under "Client JSON Protocol introspection": The QAPI schema supports a number of integer types that only differ in how they map to C. They are identical as far as SchemaInfo is concerned. Therefore, they get all mapped to a single type "int" in SchemaInfo. Do we need to repeat it here? > + Internal types not present in introspection use > + ``NULL``. Currently only QType. Could we make that not a special case? I.e. .masked_name = "QType" like for other built-in types. > + > +``lookup`` > + For enum types, a pointer to the corresponding ``QEnumLookup`` > + table. ``NULL`` for non-enum types. > + > +``list`` > + For types that have an array variant, a pointer to the array type's > + ``QAPITypeInfo``. ``NULL`` when no array type exists. > + > +These type info instances are used by QOM property registration > +functions (``object_property_add_qapi()``, > +``object_class_property_add_qapi_enum()``, etc.) to associate each > +property with its QAPI type. The ``qom-list`` and > +``device-list-properties`` QMP commands then expose the ``qapi-type`` > +field, giving management tools a formal type reference they can look > +up in the introspection schema. Alright, you talk a bit more about QOM introspection here. Fine for now, but it really needs comprehensive treatment similar to what we do for query-qmp-schema. But we can do that later. > + > +All user-defined and array types are included in introspection, even when > +they are not reachable from a command or event. Their type info therefore > +always provides the introspection cross-reference. I'm not sure I understand this paragraph. What are you trying to convey? > + > +Example:: > + > + $ cat qapi-generated/example-qapi-type-infos.h > + [Uninteresting stuff omitted...] > + > + #ifndef EXAMPLE_QAPI_TYPE_INFOS_H > + #define EXAMPLE_QAPI_TYPE_INFOS_H > + > + #include "qapi/qapi-builtin-type-infos.h" > + > + extern const QAPITypeInfo UserDefOne_type_info; > + > + extern const QAPITypeInfo UserDefOneList_type_info; > + > + #endif /* EXAMPLE_QAPI_TYPE_INFOS_H */ > + $ cat qapi-generated/example-qapi-type-infos.c > + [Uninteresting stuff omitted...] > + > + const QAPITypeInfo UserDefOne_type_info = { > + .name = "UserDefOne", > + .masked_name = "1", > + .list = &UserDefOneList_type_info, > + }; > + > + const QAPITypeInfo UserDefOneList_type_info = { > + .name = "UserDefOneList", > + .masked_name = "[1]", > + }; > + > + [Uninteresting stuff omitted...] For me, it actually generates masked names "0" and "[0]". Is the mapping to masked names stable? > + > +For a modular QAPI schema (see section `Include directives`_), code for > +each sub-module SUBDIR/SUBMODULE.json is actually generated into :: > + > + SUBDIR/$(prefix)qapi-type-infos-SUBMODULE.h > + SUBDIR/$(prefix)qapi-type-infos-SUBMODULE.c > + > +If qapi-gen.py is run with option --builtins, additional files are > +created: > + > + ``qapi-builtin-type-infos.h`` > + Type info instances for built-in types > + > + ``qapi-builtin-type-infos.c`` > + Definitions for the above type info instances [...]
