qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH v3 5/6] qapi: Add support for aliases


From: Kevin Wolf
Subject: [PATCH v3 5/6] qapi: Add support for aliases
Date: Thu, 12 Aug 2021 18:11:30 +0200

Introduce alias definitions for object types (structs and unions). This
allows using the same QAPI type and visitor for many syntax variations
that exist in the external representation, like between QMP and the
command line. It also provides a new tool for evolving the schema while
maintaining backwards compatibility during a deprecation period.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 docs/devel/qapi-code-gen.rst           | 104 +++++++++++++++++++++-
 docs/sphinx/qapidoc.py                 |   2 +-
 scripts/qapi/expr.py                   |  47 +++++++++-
 scripts/qapi/schema.py                 | 116 +++++++++++++++++++++++--
 scripts/qapi/types.py                  |   4 +-
 scripts/qapi/visit.py                  |  34 +++++++-
 tests/qapi-schema/test-qapi.py         |   7 +-
 tests/qapi-schema/double-type.err      |   2 +-
 tests/qapi-schema/unknown-expr-key.err |   2 +-
 9 files changed, 297 insertions(+), 21 deletions(-)

diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
index 26c62b0e7b..c0883507a8 100644
--- a/docs/devel/qapi-code-gen.rst
+++ b/docs/devel/qapi-code-gen.rst
@@ -262,7 +262,8 @@ Syntax::
                'data': MEMBERS,
                '*base': STRING,
                '*if': COND,
-               '*features': FEATURES }
+               '*features': FEATURES,
+               '*aliases': ALIASES }
     MEMBERS = { MEMBER, ... }
     MEMBER = STRING : TYPE-REF
            | STRING : { 'type': TYPE-REF,
@@ -312,6 +313,9 @@ the schema`_ below for more on this.
 The optional 'features' member specifies features.  See Features_
 below for more on this.
 
+The optional 'aliases' member specifies aliases.  See Aliases_ below
+for more on this.
+
 
 Union types
 -----------
@@ -321,13 +325,15 @@ Syntax::
     UNION = { 'union': STRING,
               'data': BRANCHES,
               '*if': COND,
-              '*features': FEATURES }
+              '*features': FEATURES,
+              '*aliases': ALIASES }
           | { 'union': STRING,
               'data': BRANCHES,
               'base': ( MEMBERS | STRING ),
               'discriminator': STRING,
               '*if': COND,
-              '*features': FEATURES }
+              '*features': FEATURES,
+              '*aliases': ALIASES }
     BRANCHES = { BRANCH, ... }
     BRANCH = STRING : TYPE-REF
            | STRING : { 'type': TYPE-REF, '*if': COND }
@@ -437,6 +443,9 @@ the schema`_ below for more on this.
 The optional 'features' member specifies features.  See Features_
 below for more on this.
 
+The optional 'aliases' member specifies aliases.  See Aliases_ below
+for more on this.
+
 
 Alternate types
 ---------------
@@ -888,6 +897,95 @@ shows a conditional entity only when the condition is 
satisfied in
 this particular build.
 
 
+Aliases
+-------
+
+Object types, including structs and unions, can contain alias
+definitions.
+
+Aliases define alternative member names that may be used in wire input
+to provide a value for a member in the same object or in a nested
+object.
+
+Syntax::
+
+    ALIASES = [ ALIAS, ... ]
+    ALIAS = { '*name': STRING,
+              'source': [ STRING, ... ] }
+
+If ``name`` is present, then the single member referred to by ``source``
+is made accessible with the name given by ``name`` in the type where the
+alias definition is specified.
+
+If ``name`` is not present, then this is a wildcard alias and all
+members in the object referred to by ``source`` are made accessible in
+the type where the alias definition is specified with the same name as
+they have in ``source``.
+
+``source`` is a non-empty list of member names representing the path to
+an object member. The first name is resolved in the same object.  Each
+subsequent member is resolved in the object named by the preceding
+member.
+
+Do not use optional objects in the path of a wildcard alias unless there
+is no semantic difference between an empty object and an absent object.
+Absent objects are implicitly turned into empty ones if an alias could
+apply and provide a value in the nested object, which is always the case
+for wildcard aliases.
+
+Example: Alternative name for a member in the same object ::
+
+ { 'struct': 'File',
+   'data': { 'path': 'str' },
+   'aliases': [ { 'name': 'filename', 'source': ['path'] } ] }
+
+The member ``path`` may instead be given through its alias ``filename``
+in input.
+
+Example: Alias for a member in a nested object ::
+
+ { 'struct': 'A',
+   'data': { 'zahl': 'int' } }
+ { 'struct': 'B',
+   'data': { 'drei': 'A' } }
+ { 'struct': 'C',
+   'data': { 'zwei': 'B' } }
+ { 'struct': 'D',
+   'data': { 'eins': 'C' },
+   'aliases': [ { 'name': 'number',
+                  'source': ['eins', 'zwei', 'drei', 'zahl' ] },
+                { 'name': 'the_B',
+                  'source': ['eins','zwei'] } ] }
+
+With this definition, each of the following inputs for ``D`` mean the
+same::
+
+ { 'eins': { 'zwei': { 'drei': { 'zahl': 42 } } } }
+
+ { 'the_B': { 'drei': { 'zahl': 42 } } }
+
+ { 'number': 42 }
+
+Example: Flattening a simple union with a wildcard alias that maps all
+members of ``data`` to the top level ::
+
+ { 'union': 'SocketAddress',
+   'data': {
+     'inet': 'InetSocketAddress',
+     'unix': 'UnixSocketAddress' },
+   'aliases': [ { 'source': ['data'] } ] }
+
+Aliases are transitive: ``source`` may refer to another alias name.  In
+this case, the alias is effectively an alternative name for the source
+of the other alias.
+
+In order to accommodate unions where variants differ in structure, it
+is allowed to use a path that doesn't necessarily match an existing
+member in every variant; in this case, the alias remains unused.  The
+QAPI generator checks that there is at least one branch for which an
+alias could match.
+
+
 Documentation comments
 ----------------------
 
diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
index 87c67ab23f..68340b8529 100644
--- a/docs/sphinx/qapidoc.py
+++ b/docs/sphinx/qapidoc.py
@@ -313,7 +313,7 @@ def visit_enum_type(self, name, info, ifcond, features, 
members, prefix):
                       + self._nodes_for_if_section(ifcond))
 
     def visit_object_type(self, name, info, ifcond, features,
-                          base, members, variants):
+                          base, members, variants, aliases):
         doc = self._cur_doc
         if base and base.is_implicit():
             base = None
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index cf98923fa6..054fef8d8e 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -430,6 +430,45 @@ def check_features(features: Optional[object],
         check_if(feat, info, source)
 
 
+def check_aliases(aliases: Optional[object],
+                  info: QAPISourceInfo) -> None:
+    """
+    Normalize and validate the ``aliases`` member.
+
+    :param aliases: The aliases member value to validate.
+    :param info: QAPI schema source file information.
+
+    :raise QAPISemError: When ``aliases`` fails validation.
+    :return: None, ``aliases`` is normalized in-place as needed.
+    """
+
+    if aliases is None:
+        return
+    if not isinstance(aliases, list):
+        raise QAPISemError(info, "'aliases' must be an array")
+    for a in aliases:
+        if not isinstance(a, dict):
+            raise QAPISemError(info, "'aliases' members must be objects")
+        check_keys(a, info, "'aliases' member", ['source'], ['name'])
+
+        if 'name' in a:
+            source = "alias member 'name'"
+            check_name_is_str(a['name'], info, source)
+            check_name_str(a['name'], info, source)
+
+        if not isinstance(a['source'], list):
+            raise QAPISemError(info,
+                "alias member 'source' must be an array")
+        if not a['source']:
+            raise QAPISemError(info,
+                "alias member 'source' must not be empty")
+
+        source = "member of alias member 'source'"
+        for s in a['source']:
+            check_name_is_str(s, info, source)
+            check_name_str(s, info, source)
+
+
 def check_enum(expr: _JSONObject, info: QAPISourceInfo) -> None:
     """
     Normalize and validate this expression as an ``enum`` definition.
@@ -483,6 +522,7 @@ def check_struct(expr: _JSONObject, info: QAPISourceInfo) 
-> None:
 
     check_type(members, info, "'data'", allow_dict=name)
     check_type(expr.get('base'), info, "'base'")
+    check_aliases(expr.get('aliases'), info)
 
 
 def check_union(expr: _JSONObject, info: QAPISourceInfo) -> None:
@@ -509,6 +549,8 @@ def check_union(expr: _JSONObject, info: QAPISourceInfo) -> 
None:
             raise QAPISemError(info, "'discriminator' requires 'base'")
         check_name_is_str(discriminator, info, "'discriminator'")
 
+    check_aliases(expr.get('aliases'), info)
+
     if not isinstance(members, dict):
         raise QAPISemError(info, "'data' must be an object")
 
@@ -653,7 +695,7 @@ def check_exprs(exprs: List[_JSONObject]) -> 
List[_JSONObject]:
         elif meta == 'union':
             check_keys(expr, info, meta,
                        ['union', 'data'],
-                       ['base', 'discriminator', 'if', 'features'])
+                       ['base', 'discriminator', 'if', 'features', 'aliases'])
             normalize_members(expr.get('base'))
             normalize_members(expr['data'])
             check_union(expr, info)
@@ -664,7 +706,8 @@ def check_exprs(exprs: List[_JSONObject]) -> 
List[_JSONObject]:
             check_alternate(expr, info)
         elif meta == 'struct':
             check_keys(expr, info, meta,
-                       ['struct', 'data'], ['base', 'if', 'features'])
+                       ['struct', 'data'],
+                       ['base', 'if', 'features', 'aliases'])
             normalize_members(expr['data'])
             check_struct(expr, info)
         elif meta == 'command':
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index d1d27ff7ee..fc75635f4e 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -118,7 +118,7 @@ def visit_array_type(self, name, info, ifcond, 
element_type):
         pass
 
     def visit_object_type(self, name, info, ifcond, features,
-                          base, members, variants):
+                          base, members, variants, aliases):
         pass
 
     def visit_object_type_flat(self, name, info, ifcond, features,
@@ -364,7 +364,7 @@ def describe(self):
 
 class QAPISchemaObjectType(QAPISchemaType):
     def __init__(self, name, info, doc, ifcond, features,
-                 base, local_members, variants):
+                 base, local_members, variants, aliases=None):
         # struct has local_members, optional base, and no variants
         # flat union has base, variants, and no local_members
         # simple union has local_members, variants, and no base
@@ -382,6 +382,9 @@ def __init__(self, name, info, doc, ifcond, features,
         self.local_members = local_members
         self.variants = variants
         self.members = None
+        self.aliases = aliases or []
+        for a in self.aliases:
+            a.set_defined_in(name)
 
     def check(self, schema):
         # This calls another type T's .check() exactly when the C
@@ -413,12 +416,16 @@ def check(self, schema):
         for m in self.local_members:
             m.check(schema)
             m.check_clash(self.info, seen)
-        members = seen.values()
+        members = list(seen.values())
 
         if self.variants:
             self.variants.check(schema, seen)
             self.variants.check_clash(self.info, seen)
 
+        for a in self.aliases:
+            a.check_clash(self.info, seen)
+            self.check_path(a, list(a.source), members)
+
         self.members = members  # mark completed
 
     # Check that the members of this type do not cause duplicate JSON members,
@@ -430,6 +437,68 @@ def check_clash(self, info, seen):
         for m in self.members:
             m.check_clash(info, seen)
 
+    # Deletes elements from path, so pass a copy if you still need them
+    def check_path(self, alias, path, members=None, local_aliases_seen=()):
+        assert isinstance(path, list)
+
+        if not path:
+            return
+        first = path.pop(0)
+
+        for a in self.aliases:
+            if a.name == first:
+                if a in local_aliases_seen:
+                    raise QAPISemError(
+                        self.info,
+                        "%s resolving to '%s' makes '%s' an alias for itself"
+                        % (a.describe(self.info), a.source[0], a.source[0]))
+
+                path = a.source + path
+                return self.check_path(alias, path, members,
+                                       (*local_aliases_seen, a))
+
+        if members is None:
+            assert self.members is not None
+            members = self.members
+        else:
+            assert isinstance(members, list)
+
+        for m in members:
+            if m.name == first:
+                # Wildcard aliases can only accept object types in the whole
+                # path; for single-member aliases, the last element can be
+                # any type
+                need_obj = (alias.name is None) or path
+                if need_obj and not isinstance(m.type, QAPISchemaObjectType):
+                    raise QAPISemError(
+                        self.info,
+                        "%s has non-object '%s' in its source path"
+                        % (alias.describe(self.info), m.name))
+                if alias.name is None and m.optional:
+                    raise QAPISemError(
+                        self.info,
+                        "%s has optional object %s in its source path"
+                        % (alias.describe(self.info), m.describe(self.info)))
+                if path:
+                    m.type.check_path(alias, path)
+                return
+
+        # It is sufficient that the path is valid in at least one variant
+        if self.variants:
+            for v in self.variants.variants:
+                try:
+                    return v.type.check_path(alias, [first, *path])
+                except QAPISemError:
+                    pass
+            raise QAPISemError(
+                self.info,
+                "%s has a source path that does not exist in any variant of %s"
+                % (alias.describe(self.info), self.describe()))
+
+        raise QAPISemError(
+            self.info,
+            "%s has inexistent source" % alias.describe(self.info))
+
     def connect_doc(self, doc=None):
         super().connect_doc(doc)
         doc = doc or self.doc
@@ -474,7 +543,7 @@ def visit(self, visitor):
         super().visit(visitor)
         visitor.visit_object_type(
             self.name, self.info, self.ifcond, self.features,
-            self.base, self.local_members, self.variants)
+            self.base, self.local_members, self.variants, self.aliases)
         visitor.visit_object_type_flat(
             self.name, self.info, self.ifcond, self.features,
             self.members, self.variants)
@@ -639,7 +708,7 @@ def check_clash(self, info, seen):
 
 
 class QAPISchemaMember:
-    """ Represents object members, enum members and features """
+    """ Represents object members, enum members, features and aliases """
     role = 'member'
 
     def __init__(self, name, info, ifcond=None):
@@ -705,6 +774,30 @@ class QAPISchemaFeature(QAPISchemaMember):
     role = 'feature'
 
 
+class QAPISchemaAlias(QAPISchemaMember):
+    role = 'alias'
+
+    def __init__(self, name, info, source):
+        assert name is None or isinstance(name, str)
+        assert source
+        for member in source:
+            assert isinstance(member, str)
+
+        super().__init__(name or '*', info)
+        self.name = name
+        self.source = source
+
+    def check_clash(self, info, seen):
+        if self.name:
+            super().check_clash(info, seen)
+
+    def describe(self, info):
+        if self.name:
+            return super().describe(info)
+        else:
+            return "wildcard alias"
+
+
 class QAPISchemaObjectTypeMember(QAPISchemaMember):
     def __init__(self, name, info, typ, optional, ifcond=None, features=None):
         super().__init__(name, info, ifcond)
@@ -971,6 +1064,12 @@ def _make_features(self, features, info):
         return [QAPISchemaFeature(f['name'], info, f.get('if'))
                 for f in features]
 
+    def _make_aliases(self, aliases, info):
+        if aliases is None:
+            return []
+        return [QAPISchemaAlias(a.get('name'), info, a['source'])
+                for a in aliases]
+
     def _make_enum_members(self, values, info):
         return [QAPISchemaEnumMember(v['name'], info, v.get('if'))
                 for v in values]
@@ -1045,11 +1144,12 @@ def _def_struct_type(self, expr, info, doc):
         base = expr.get('base')
         data = expr['data']
         ifcond = expr.get('if')
+        aliases = self._make_aliases(expr.get('aliases'), info)
         features = self._make_features(expr.get('features'), info)
         self._def_entity(QAPISchemaObjectType(
             name, info, doc, ifcond, features, base,
             self._make_members(data, info),
-            None))
+            None, aliases))
 
     def _make_variant(self, case, typ, ifcond, info):
         return QAPISchemaVariant(case, info, typ, ifcond)
@@ -1068,6 +1168,7 @@ def _def_union_type(self, expr, info, doc):
         data = expr['data']
         base = expr.get('base')
         ifcond = expr.get('if')
+        aliases = self._make_aliases(expr.get('aliases'), info)
         features = self._make_features(expr.get('features'), info)
         tag_name = expr.get('discriminator')
         tag_member = None
@@ -1092,7 +1193,8 @@ def _def_union_type(self, expr, info, doc):
             QAPISchemaObjectType(name, info, doc, ifcond, features,
                                  base, members,
                                  QAPISchemaVariants(
-                                     tag_name, info, tag_member, variants)))
+                                     tag_name, info, tag_member, variants),
+                                 aliases))
 
     def _def_alternate_type(self, expr, info, doc):
         name = expr['alternate']
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index 20d572a23a..3bc451baa9 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -25,6 +25,7 @@
 from .gen import QAPISchemaModularCVisitor, ifcontext
 from .schema import (
     QAPISchema,
+    QAPISchemaAlias,
     QAPISchemaEnumMember,
     QAPISchemaFeature,
     QAPISchemaObjectType,
@@ -332,7 +333,8 @@ def visit_object_type(self,
                           features: List[QAPISchemaFeature],
                           base: Optional[QAPISchemaObjectType],
                           members: List[QAPISchemaObjectTypeMember],
-                          variants: Optional[QAPISchemaVariants]) -> None:
+                          variants: Optional[QAPISchemaVariants],
+                          aliases: List[QAPISchemaAlias]) -> None:
         # Nothing to do for the special empty builtin
         if name == 'q_empty':
             return
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index 9e96f3c566..0aa0764755 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -26,6 +26,7 @@
 from .gen import QAPISchemaModularCVisitor, ifcontext
 from .schema import (
     QAPISchema,
+    QAPISchemaAlias,
     QAPISchemaEnumMember,
     QAPISchemaEnumType,
     QAPISchemaFeature,
@@ -60,7 +61,8 @@ def gen_visit_members_decl(name: str) -> str:
 def gen_visit_object_members(name: str,
                              base: Optional[QAPISchemaObjectType],
                              members: List[QAPISchemaObjectTypeMember],
-                             variants: Optional[QAPISchemaVariants]) -> str:
+                             variants: Optional[QAPISchemaVariants],
+                             aliases: List[QAPISchemaAlias]) -> str:
     ret = mcgen('''
 
 bool visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)
@@ -68,6 +70,24 @@ def gen_visit_object_members(name: str,
 ''',
                 c_name=c_name(name))
 
+    if aliases:
+        ret += mcgen('''
+    visit_start_alias_scope(v);
+''')
+
+    for a in aliases:
+        if a.name:
+            name = '"%s"' % a.name
+        else:
+            name = "NULL"
+
+        source = ", ".join('"%s"' % x for x in a.source)
+
+        ret += mcgen('''
+    visit_define_alias(v, %(name)s, (const char * []) { %(source)s, NULL });
+''',
+                     name=name, source=source)
+
     if base:
         ret += mcgen('''
     if (!visit_type_%(c_type)s_members(v, (%(c_type)s *)obj, errp)) {
@@ -148,6 +168,11 @@ def gen_visit_object_members(name: str,
     }
 ''')
 
+    if aliases:
+        ret += mcgen('''
+    visit_end_alias_scope(v);
+''')
+
     ret += mcgen('''
     return true;
 }
@@ -376,14 +401,15 @@ def visit_object_type(self,
                           features: List[QAPISchemaFeature],
                           base: Optional[QAPISchemaObjectType],
                           members: List[QAPISchemaObjectTypeMember],
-                          variants: Optional[QAPISchemaVariants]) -> None:
+                          variants: Optional[QAPISchemaVariants],
+                          aliases: List[QAPISchemaAlias]) -> None:
         # Nothing to do for the special empty builtin
         if name == 'q_empty':
             return
         with ifcontext(ifcond, self._genh, self._genc):
             self._genh.add(gen_visit_members_decl(name))
-            self._genc.add(gen_visit_object_members(name, base,
-                                                    members, variants))
+            self._genc.add(gen_visit_object_members(
+                name, base, members, variants, aliases))
             # TODO Worth changing the visitor signature, so we could
             # directly use rather than repeat type.is_implicit()?
             if not name.startswith('q_'):
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index f1c4deb9a5..376630901b 100755
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -47,7 +47,7 @@ def visit_array_type(self, name, info, ifcond, element_type):
         self._print_if(ifcond)
 
     def visit_object_type(self, name, info, ifcond, features,
-                          base, members, variants):
+                          base, members, variants, aliases):
         print('object %s' % name)
         if base:
             print('    base %s' % base.name)
@@ -56,6 +56,11 @@ def visit_object_type(self, name, info, ifcond, features,
                   % (m.name, m.type.name, m.optional))
             self._print_if(m.ifcond, 8)
             self._print_features(m.features, indent=8)
+        for a in aliases:
+            if a.name:
+                print('    alias %s -> %s' % (a.name, '.'.join(a.source)))
+            else:
+                print('    alias * -> %s.*' % '.'.join(a.source))
         self._print_variants(variants)
         self._print_if(ifcond)
         self._print_features(features)
diff --git a/tests/qapi-schema/double-type.err 
b/tests/qapi-schema/double-type.err
index 576e716197..c382e61d88 100644
--- a/tests/qapi-schema/double-type.err
+++ b/tests/qapi-schema/double-type.err
@@ -1,3 +1,3 @@
 double-type.json: In struct 'Bar':
 double-type.json:2: struct has unknown key 'command'
-Valid keys are 'base', 'data', 'features', 'if', 'struct'.
+Valid keys are 'aliases', 'base', 'data', 'features', 'if', 'struct'.
diff --git a/tests/qapi-schema/unknown-expr-key.err 
b/tests/qapi-schema/unknown-expr-key.err
index f2538e3ce7..354916968f 100644
--- a/tests/qapi-schema/unknown-expr-key.err
+++ b/tests/qapi-schema/unknown-expr-key.err
@@ -1,3 +1,3 @@
 unknown-expr-key.json: In struct 'Bar':
 unknown-expr-key.json:2: struct has unknown keys 'bogus', 'phony'
-Valid keys are 'base', 'data', 'features', 'if', 'struct'.
+Valid keys are 'aliases', 'base', 'data', 'features', 'if', 'struct'.
-- 
2.31.1




reply via email to

[Prev in Thread] Current Thread [Next in Thread]