Re: [PATCH v2 03/53] qapi: add type-infos generator
Marc-André Lureau <[email protected]>
| Newsgroups | org.nongnu.qemu-devel |
|---|---|
| Message-ID | <CAMxuvazV=Fq6kYQ_1iPrLUHHxF2CwYTd7EbHnE-W5csT-pOsgQ@mail.gmail.com> |
Hi On Thu, Aug 6, 2026 at 6:06 PM Markus Armbruster <[email protected]> wrote: > > 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 | 90 +++++++++++++++++++++ > > meson.build | 1 + > > scripts/qapi/type_infos.py | 182 +++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 273 insertions(+) > > > > diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst > > index 3a632b4a648..bb2e8d9c675 100644 > > --- a/docs/devel/qapi-code-gen.rst > > +++ b/docs/devel/qapi-code-gen.rst > > @@ -2103,3 +2103,93 @@ Example:: > > })); > > > > [Uninteresting stuff omitted...] > > + > > + > > +Code generated for type information > > +----------------------------------- > > + > > +The following files are created: > > + > > + ``$(prefix)qapi-type-infos.c`` > > + A ``QAPITypeInfo`` instance for each schema-defined type, providing > > + a mapping between the C type name and the schema name used by > > + introspection, along with optional enum lookup table and list-element > > + pointers. > > + > > + ``$(prefix)qapi-type-infos.h`` > > + Declarations for the above type info instances > > + > > +Each ``QAPITypeInfo`` struct has the following fields: > > + > > +``name`` > > + The C identifier of the type (e.g. ``"UserDefOne"``). > > Isn't this the name used in the QAPI schema source? correct > > > + > > +``schema_name`` > > + The masked name used in ``query-qmp-schema`` output, or ``NULL`` > > + for built-in types whose schema name equals their C name. This > > + allows management tools to cross-reference a QOM property's > > + ``qapi-type`` against the introspection schema. > > I believe it can also be null when query-qmp-schema omits the type. > More on that below. Yes, that's a current limitation. Only types reachable from QMP are in the introspection schema. I will try to address this > > > + > > +``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 list type's > > + ``QAPITypeInfo``. ``NULL`` when no list type exists. > > Confusing mix of "array" and "list" in the same sentence. Suggest "the > array type's". > > QAPI can't decide whether to call the thing an array (like JSON) or a > list (like the C implementation). Unfortunate. > docs/devel/qapi-code-gen.rst generally sticks to array, but there are a > few lapses. fixed > > > + > > +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 schema type. > > You're using "schema type" like you do above, i.e. for the masked type > name. Correct? I mean the type in general > > > The ``qom-list`` and > > +``device-list-properties`` QMP commands then expose the ``qapi-type`` > > +field, giving management tools a formal type reference into the > > +introspection schema. > > As far as I understand, we connect a property of QAPI type to the type's > QAPITypeInfo, the commands you mentioned then use it to emit the masked > type name, so that management applications can use that to find a > property's type in the output of query-qmp-schema. > > Problem (already discussed elsewhere): only works when the type is used > in QMP. If it isn't, we still have QAPITypeInfo, but its .schema_name > is null. This needs to be documented as a known shortcoming. ok > > Other uses of QAPITypeInfo may emerge in the future. We might also > replace it by something that supports additional uses; see my ramblings > in review of PATCH 01. In this series, however, connecting QOM > properties to their masked QAPI type name is their only use. I > understand we emit QAPITypeInfo for any QAPI type that could be used as > QOM property type. Dead data unless it is actually used as QOM property > type. This is acceptable; we already generate similarly dead visitors. > But it's worth mentioning in the commit message, I think. > > > + > > +Implicit types (names starting with ``q_``) are skipped. > > This isn't quite correct. Array types are also implicitly defined, and > their names don't start with 'q_'. > > Possible fixes: > > * Drop the sentence (only if we believe it's not needed) > > * Drop the parenthesis, add "other than arrays" or something like that. > > * Positive instead of negative: only user-defined and built-in types. > This could perhaps be worked into the text under "The following files > are created". May want to add why: only these may be used as property > types. rephrased, thanks > > > + > > +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", > > + .schema_name = "1", > > + .list = &UserDefOneList_type_info, > > + }; > > + > > + const QAPITypeInfo UserDefOneList_type_info = { > > + .name = "UserDefOneList", > > + .schema_name = "[1]", > > + }; > > + > > + [Uninteresting stuff omitted...] > > + > > +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 > > diff --git a/meson.build b/meson.build > > index 19e123423b5..f135eddad12 100644 > > --- a/meson.build > > +++ b/meson.build > > @@ -3487,6 +3487,7 @@ qapi_gen_depends = [ meson.current_source_dir() / 'scripts/qapi/__init__.py', > > meson.current_source_dir() / 'scripts/qapi/parser.py', > > meson.current_source_dir() / 'scripts/qapi/schema.py', > > meson.current_source_dir() / 'scripts/qapi/source.py', > > + meson.current_source_dir() / 'scripts/qapi/type_infos.py', > > meson.current_source_dir() / 'scripts/qapi/types.py', > > meson.current_source_dir() / 'scripts/qapi/visit.py', > > meson.current_source_dir() / 'scripts/qapi-gen.py' > > diff --git a/scripts/qapi/type_infos.py b/scripts/qapi/type_infos.py > > new file mode 100644 > > index 00000000000..dc7e2bace9e > > --- /dev/null > > +++ b/scripts/qapi/type_infos.py > > Please use '-' instead of '_' in filenames for local consistency. It has to use _ for import, afaik > > > @@ -0,0 +1,182 @@ > > +""" > > +QAPI type info generator > > + > > +SPDX-License-Identifier: GPL-2.0-or-later > > +""" > > + > > +from typing import ( > > + Dict, > > + List, > > + Optional, > > + Set, > > +) > > + > > +from .common import c_name, mcgen > > +from .gen import QAPISchemaModularCVisitor, ifcontext > > +from .schema import ( > > + QAPISchema, > > + QAPISchemaAlternatives, > > + QAPISchemaBranches, > > + QAPISchemaEnumMember, > > + QAPISchemaFeature, > > + QAPISchemaIfCond, > > + QAPISchemaObjectType, > > + QAPISchemaObjectTypeMember, > > + QAPISchemaType, > > + QAPISchemaVisitor, > > +) > > +from .source import QAPISourceInfo > > + > > + > > +class _ArrayTypeCollector(QAPISchemaVisitor): > > + def __init__(self) -> None: > > + self.list_types: Set[str] = set() > > + > > + def visit_array_type(self, > > + name: str, > > + info: Optional[QAPISourceInfo], > > + ifcond: QAPISchemaIfCond, > > + element_type: QAPISchemaType) -> None: > > + self.list_types.add(name) > > + > > + > > +class QAPISchemaGenTypeInfoVisitor(QAPISchemaModularCVisitor): > > + > > + def __init__(self, prefix: str, name_map: Dict[str, str], > > + list_types: Set[str]): > > + super().__init__( > > + prefix, 'qapi-type-infos', > > + ' * Schema-defined QAPI type info', > > + ' * Built-in QAPI type info', __doc__) > > + self._name_map = name_map > > + self._list_types = list_types > > + > > + def _begin_builtin_module(self) -> None: > > + self._genc.preamble_add(mcgen(''' > > +#include "qemu/osdep.h" > > +#include "qapi/qapi-builtin-types.h" > > +#include "qapi/qapi-builtin-type-infos.h" > > +''')) > > + self._genh.preamble_add(mcgen(''' > > +#include "qapi/qapi-type-info.h" > > +''')) > > + > > + def _begin_user_module(self, name: str) -> None: > > + type_infos = self._module_basename('qapi-type-infos', name) > > + types = self._module_basename('qapi-types', name) > > + self._genc.preamble_add(mcgen(''' > > +#include "qemu/osdep.h" > > +#include "%(types)s.h" > > +#include "%(type_infos)s.h" > > +''', > > + types=types, > > + type_infos=type_infos)) > > + self._genh.preamble_add(mcgen(''' > > +#include "qapi/qapi-builtin-type-infos.h" > > +''')) > > + > > + def _gen_type_info(self, name: str, > > + ifcond: Optional[QAPISchemaIfCond] = None, > > + with_lookup: bool = False, > > + with_list: bool = False, > > + schema_name: Optional[str] = None) -> None: > > + c_id = c_name(name + '_type_info') > > + lookup = '' > > + if with_lookup: > > + lookup = mcgen(''' > > + .lookup = &%(c_name)s_lookup, > > +''', > > + c_name=c_name(name)) > > + list_ref = '' > > + if with_list: > > + list_ref = mcgen(''' > > + .list = &%(list_id)s, > > +''', > > + list_id=c_name(name + 'List_type_info')) > > + if schema_name is None: > > + masked = self._name_map.get(name) > > + schema_name = '"%s"' % masked if masked is not None else 'NULL' > > + else: > > + schema_name = '"%s"' % schema_name > > + with ifcontext(ifcond or QAPISchemaIfCond(), > > + self._genh, self._genc): > > + self._genh.add(mcgen(''' > > + > > +extern const QAPITypeInfo %(c_id)s; > > +''', > > + c_id=c_id)) > > + self._genc.add(mcgen(''' > > + > > +const QAPITypeInfo %(c_id)s = { > > + .name = "%(name)s", > > + .schema_name = %(schema_name)s, > > +%(lookup)s%(list_ref)s}; > > This substitutes values of mcgen() into mcgen(). That's not how it > wants to be used. Please ... > > > +''', > > + c_id=c_id, name=name, > > + schema_name=schema_name, > > + lookup=lookup, > > + list_ref=list_ref)) > > ... try something like > > self._genh.add(mcgen(''' > > extern const QAPITypeInfo %(c_id)s; > ''', > c_id=c_id)) > self._genc.add(mcgen(''' > > const QAPITypeInfo %(c_id)s = { > .name = "%(name)s", > .schema_name = %(schema_name)s, > ''', > c_id=c_id, name=name, > schema_name=schema_name)) > > if with_lookup: > self._genc.add(mcgen(''' > .lookup = &%(c_name)s_lookup, > ''', > c_name=c_name(name)) > > and so forth. ok > > > + > > + def _has_list(self, name: str) -> bool: > > + return name + 'List' in self._list_types > > Simpler, I think: > > return self._schema.lookup_type(name + 'List') > > where self._schema is set in .visit_begin(). ok > > > + > > + def visit_builtin_type(self, > > + name: str, > > + info: Optional[QAPISourceInfo], > > + json_type: str) -> None: > > + self._gen_type_info(name, with_list=self._has_list(name)) > > + > > + def visit_enum_type(self, > > + name: str, > > + info: Optional[QAPISourceInfo], > > + ifcond: QAPISchemaIfCond, > > + features: List[QAPISchemaFeature], > > + members: List[QAPISchemaEnumMember], > > + prefix: Optional[str]) -> None: > > + self._gen_type_info(name, ifcond, with_lookup=True, > > + with_list=self._has_list(name)) > > + > > + def visit_object_type(self, > > + name: str, > > + info: Optional[QAPISourceInfo], > > + ifcond: QAPISchemaIfCond, > > + features: List[QAPISchemaFeature], > > + base: Optional[QAPISchemaObjectType], > > + members: List[QAPISchemaObjectTypeMember], > > + branches: Optional[QAPISchemaBranches]) -> None: > > + if name.startswith('q_'): > > Duplicates QAPISchemaObjectType.is_implicit(). Similar code in types.py > and visit.py. Not this patch's fault; moving on. > > > + return > > + self._gen_type_info(name, ifcond, > > + with_list=self._has_list(name)) > > + > > + def visit_array_type(self, > > + name: str, > > + info: Optional[QAPISourceInfo], > > + ifcond: QAPISchemaIfCond, > > + element_type: QAPISchemaType) -> None: > > + elem_schema = self._name_map.get(element_type.name, > > + element_type.name) > > + self._gen_type_info(name, ifcond, > > + schema_name='[' + elem_schema + ']') > > + > > + def visit_alternate_type(self, > > + name: str, > > + info: Optional[QAPISourceInfo], > > + ifcond: QAPISchemaIfCond, > > + features: List[QAPISchemaFeature], > > + alternatives: QAPISchemaAlternatives) -> None: > > + self._gen_type_info(name, ifcond, > > + with_list=self._has_list(name)) > > + > > + > > +def gen_type_infos(schema: QAPISchema, > > + output_dir: str, > > + prefix: str, > > + opt_builtins: bool, > > + name_map: Dict[str, str]) -> None: > > + collector = _ArrayTypeCollector() > > + schema.visit(collector) > > + vis = QAPISchemaGenTypeInfoVisitor(prefix, name_map, > > + collector.list_types) > > + schema.visit(vis) > > + vis.write(output_dir, opt_builtins) >