All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/8] qapi: Add support for aliases
@ 2021-09-17 16:13 Kevin Wolf
  2021-09-17 16:13 ` [PATCH v4 1/8] qapi: Add interfaces for alias support to Visitor Kevin Wolf
                   ` (9 more replies)
  0 siblings, 10 replies; 13+ messages in thread
From: Kevin Wolf @ 2021-09-17 16:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, jsnow, armbru

This series introduces alias definitions for QAPI object types (structs
and unions).

This allows using the same QAPI type and visitor even when the syntax
has some variations between different external interfaces such as QMP
and the command line.

It also provides a new tool for evolving the schema while maintaining
backwards compatibility (possibly during a deprecation period).

The first user is intended to be a QAPIfied -chardev command line
option, for which I'll send a separate series. A git tag is available
that contains both this series and the chardev changes that make use of
it:

    https://repo.or.cz/qemu/kevin.git qapi-alias-chardev-v4

v4:
- Make sure to keep a defined order of aliases in StackObject.aliases
- Added patch 4 to allow for better error messages when wildcard aliases
  provide a second value for a member, which may or may not be consumed
  elsewhere.
- Resolve chained aliases only once instead of just checking that they
  can be resolved while looking for matching aliases, and actually
  resolving them at the end. This is not only a code simplification, but
  actually necessary for correct error messages on conflicts.
- Separate schema.py cleanup patch by Markus ('qapi: Revert an
  accidental change from list to view object')
- Fixed alias name checks in the QAPI generator
- Changed check_path() to avoid modifying its 'path' parameter
- Some more test cases
- Coding style fixes
- Documentation improvements

v3:
- Mention the new functions in the big comment in visitor.h. However,
  since the comment is about users of the visitor rather than the
  generated code, it seems like to wrong place to go into details.
- Updated commit message for patch 3 ('Simplify full_name_nth() ...')
- Patch 4 ('qapi: Apply aliases in qobject-input-visitor'):
    - Multiple matching wildcard aliases are considered conflicting now
    - Improved comments for several functions
    - Renamed bool *implicit_object into *is_alias_prefix, which
      describes better what it is rather than what it is used for
    - Simplified alias_present() into input_present()
    - Fixed potential use of wrong StackObject in error message
- Patch 5 ('qapi: Add support for aliases'):
    - Made QAPISchemaAlias a QAPISchemaMember
    - Check validity of alias source paths (must exist in at least one
      variant, no optional objects in the path of a wildcard alias, no
      alias loops)
- Many new tests cases, both positive and negative, including unit tests
  of the generated visit functions
- Coding style changes
- Rebased documentation (.txt -> .rst conversion in master)

v2:
- Renamed 'alias' to 'name' in all data structures describing aliases
- Tons of new or changed comments and other documentation
- Be more explicit that empty 'source' is invalid and assert it
- Fixed full_name_so() for lists (added a parameter to tell the function
  whether the name of a list member or the list itself is meant)
- Changed some QAPI generator error messages
- Assert the type of parameters in QAPISchemaAlias.__init__()

Kevin Wolf (7):
  qapi: Add interfaces for alias support to Visitor
  qapi: Remember alias definitions in qobject-input-visitor
  qapi: Simplify full_name_nth() in qobject-input-visitor
  qapi: Store Error in StackObject.h for qobject-input-visitor
  qapi: Apply aliases in qobject-input-visitor
  qapi: Add support for aliases
  tests/qapi-schema: Test cases for aliases

Markus Armbruster (1):
  qapi: Revert an accidental change from list to view object

 docs/devel/qapi-code-gen.rst                  | 109 ++++-
 docs/sphinx/qapidoc.py                        |   2 +-
 include/qapi/visitor-impl.h                   |  12 +
 include/qapi/visitor.h                        |  59 ++-
 qapi/qapi-visit-core.c                        |  22 +
 qapi/qobject-input-visitor.c                  | 452 ++++++++++++++++--
 tests/unit/test-qmp-cmds.c                    |  10 +
 tests/unit/test-qobject-input-visitor.c       | 271 +++++++++++
 scripts/qapi/expr.py                          |  54 ++-
 scripts/qapi/schema.py                        | 117 ++++-
 scripts/qapi/types.py                         |   4 +-
 scripts/qapi/visit.py                         |  34 +-
 tests/qapi-schema/test-qapi.py                |   7 +-
 tests/qapi-schema/alias-bad-type.err          |   2 +
 tests/qapi-schema/alias-bad-type.json         |   3 +
 tests/qapi-schema/alias-bad-type.out          |   0
 tests/qapi-schema/alias-missing-source.err    |   2 +
 tests/qapi-schema/alias-missing-source.json   |   3 +
 tests/qapi-schema/alias-missing-source.out    |   0
 tests/qapi-schema/alias-name-bad-type.err     |   2 +
 tests/qapi-schema/alias-name-bad-type.json    |   3 +
 tests/qapi-schema/alias-name-bad-type.out     |   0
 tests/qapi-schema/alias-name-conflict.err     |   2 +
 tests/qapi-schema/alias-name-conflict.json    |   4 +
 tests/qapi-schema/alias-name-conflict.out     |   0
 tests/qapi-schema/alias-recursive.err         |   2 +
 tests/qapi-schema/alias-recursive.json        |   4 +
 tests/qapi-schema/alias-recursive.out         |   0
 tests/qapi-schema/alias-source-bad-type.err   |   2 +
 tests/qapi-schema/alias-source-bad-type.json  |   3 +
 tests/qapi-schema/alias-source-bad-type.out   |   0
 .../alias-source-elem-bad-type.err            |   2 +
 .../alias-source-elem-bad-type.json           |   3 +
 .../alias-source-elem-bad-type.out            |   0
 tests/qapi-schema/alias-source-empty.err      |   2 +
 tests/qapi-schema/alias-source-empty.json     |   3 +
 tests/qapi-schema/alias-source-empty.out      |   0
 .../alias-source-inexistent-variants.err      |   2 +
 .../alias-source-inexistent-variants.json     |  12 +
 .../alias-source-inexistent-variants.out      |   0
 tests/qapi-schema/alias-source-inexistent.err |   2 +
 .../qapi-schema/alias-source-inexistent.json  |   3 +
 tests/qapi-schema/alias-source-inexistent.out |   0
 .../alias-source-non-object-path.err          |   2 +
 .../alias-source-non-object-path.json         |   3 +
 .../alias-source-non-object-path.out          |   0
 .../alias-source-non-object-wildcard.err      |   2 +
 .../alias-source-non-object-wildcard.json     |   3 +
 .../alias-source-non-object-wildcard.out      |   0
 ...lias-source-optional-wildcard-indirect.err |   2 +
 ...ias-source-optional-wildcard-indirect.json |   6 +
 ...lias-source-optional-wildcard-indirect.out |   0
 .../alias-source-optional-wildcard.err        |   2 +
 .../alias-source-optional-wildcard.json       |   5 +
 .../alias-source-optional-wildcard.out        |   0
 tests/qapi-schema/alias-unknown-key.err       |   3 +
 tests/qapi-schema/alias-unknown-key.json      |   3 +
 tests/qapi-schema/alias-unknown-key.out       |   0
 tests/qapi-schema/aliases-bad-type.err        |   2 +
 tests/qapi-schema/aliases-bad-type.json       |   3 +
 tests/qapi-schema/aliases-bad-type.out        |   0
 tests/qapi-schema/meson.build                 |  16 +
 tests/qapi-schema/qapi-schema-test.json       |  35 ++
 tests/qapi-schema/qapi-schema-test.out        |  44 ++
 tests/qapi-schema/unknown-expr-key.err        |   2 +-
 65 files changed, 1290 insertions(+), 57 deletions(-)
 create mode 100644 tests/qapi-schema/alias-bad-type.err
 create mode 100644 tests/qapi-schema/alias-bad-type.json
 create mode 100644 tests/qapi-schema/alias-bad-type.out
 create mode 100644 tests/qapi-schema/alias-missing-source.err
 create mode 100644 tests/qapi-schema/alias-missing-source.json
 create mode 100644 tests/qapi-schema/alias-missing-source.out
 create mode 100644 tests/qapi-schema/alias-name-bad-type.err
 create mode 100644 tests/qapi-schema/alias-name-bad-type.json
 create mode 100644 tests/qapi-schema/alias-name-bad-type.out
 create mode 100644 tests/qapi-schema/alias-name-conflict.err
 create mode 100644 tests/qapi-schema/alias-name-conflict.json
 create mode 100644 tests/qapi-schema/alias-name-conflict.out
 create mode 100644 tests/qapi-schema/alias-recursive.err
 create mode 100644 tests/qapi-schema/alias-recursive.json
 create mode 100644 tests/qapi-schema/alias-recursive.out
 create mode 100644 tests/qapi-schema/alias-source-bad-type.err
 create mode 100644 tests/qapi-schema/alias-source-bad-type.json
 create mode 100644 tests/qapi-schema/alias-source-bad-type.out
 create mode 100644 tests/qapi-schema/alias-source-elem-bad-type.err
 create mode 100644 tests/qapi-schema/alias-source-elem-bad-type.json
 create mode 100644 tests/qapi-schema/alias-source-elem-bad-type.out
 create mode 100644 tests/qapi-schema/alias-source-empty.err
 create mode 100644 tests/qapi-schema/alias-source-empty.json
 create mode 100644 tests/qapi-schema/alias-source-empty.out
 create mode 100644 tests/qapi-schema/alias-source-inexistent-variants.err
 create mode 100644 tests/qapi-schema/alias-source-inexistent-variants.json
 create mode 100644 tests/qapi-schema/alias-source-inexistent-variants.out
 create mode 100644 tests/qapi-schema/alias-source-inexistent.err
 create mode 100644 tests/qapi-schema/alias-source-inexistent.json
 create mode 100644 tests/qapi-schema/alias-source-inexistent.out
 create mode 100644 tests/qapi-schema/alias-source-non-object-path.err
 create mode 100644 tests/qapi-schema/alias-source-non-object-path.json
 create mode 100644 tests/qapi-schema/alias-source-non-object-path.out
 create mode 100644 tests/qapi-schema/alias-source-non-object-wildcard.err
 create mode 100644 tests/qapi-schema/alias-source-non-object-wildcard.json
 create mode 100644 tests/qapi-schema/alias-source-non-object-wildcard.out
 create mode 100644 tests/qapi-schema/alias-source-optional-wildcard-indirect.err
 create mode 100644 tests/qapi-schema/alias-source-optional-wildcard-indirect.json
 create mode 100644 tests/qapi-schema/alias-source-optional-wildcard-indirect.out
 create mode 100644 tests/qapi-schema/alias-source-optional-wildcard.err
 create mode 100644 tests/qapi-schema/alias-source-optional-wildcard.json
 create mode 100644 tests/qapi-schema/alias-source-optional-wildcard.out
 create mode 100644 tests/qapi-schema/alias-unknown-key.err
 create mode 100644 tests/qapi-schema/alias-unknown-key.json
 create mode 100644 tests/qapi-schema/alias-unknown-key.out
 create mode 100644 tests/qapi-schema/aliases-bad-type.err
 create mode 100644 tests/qapi-schema/aliases-bad-type.json
 create mode 100644 tests/qapi-schema/aliases-bad-type.out

-- 
2.31.1



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v4 1/8] qapi: Add interfaces for alias support to Visitor
  2021-09-17 16:13 [PATCH v4 0/8] qapi: Add support for aliases Kevin Wolf
@ 2021-09-17 16:13 ` Kevin Wolf
  2021-09-17 16:13 ` [PATCH v4 2/8] qapi: Remember alias definitions in qobject-input-visitor Kevin Wolf
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Kevin Wolf @ 2021-09-17 16:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, jsnow, armbru

This adds functions to the Visitor interface that can be used to define
aliases and alias scopes.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 include/qapi/visitor-impl.h | 12 ++++++++
 include/qapi/visitor.h      | 59 ++++++++++++++++++++++++++++++++++---
 qapi/qapi-visit-core.c      | 22 ++++++++++++++
 3 files changed, 89 insertions(+), 4 deletions(-)

diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 3b950f6e3d..704c5ad2d9 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -119,6 +119,18 @@ struct Visitor
     /* Optional */
     bool (*deprecated)(Visitor *v, const char *name);
 
+    /*
+     * Optional; intended for input visitors. If not given, aliases are
+     * ignored.
+     */
+    void (*define_alias)(Visitor *v, const char *name, const char **source);
+
+    /* Must be set if define_alias is set */
+    void (*start_alias_scope)(Visitor *v);
+
+    /* Must be set if define_alias is set */
+    void (*end_alias_scope)(Visitor *v);
+
     /* Must be set */
     VisitorType type;
 
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index b3c9ef7a81..3bf0f4dad2 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -220,10 +220,17 @@
  * </example>
  *
  * This file provides helpers for use by the generated
- * visit_type_FOO(): visit_optional() for the 'has_member' field
- * associated with optional 'member' in the C struct,
- * visit_next_list() for advancing through a FooList linked list, and
- * visit_is_input() for cleaning up on failure.
+ * visit_type_FOO():
+ *
+ * - visit_optional() for the 'has_member' field associated with
+ *   optional 'member' in the C struct,
+ * - visit_next_list() for advancing through a FooList linked list
+ * - visit_is_input() for cleaning up on failure
+ * - visit_define_alias() for defining alternative names for object
+ *   members in input visitors
+ * - visit_start/end_alias_scope() to limit the scope of aliases
+ *   within a single input object (e.g. aliases defined in the base
+ *   struct should not provide values for the parent struct)
  */
 
 /*** Useful types ***/
@@ -477,6 +484,50 @@ bool visit_deprecated_accept(Visitor *v, const char *name, Error **errp);
  */
 bool visit_deprecated(Visitor *v, const char *name);
 
+/*
+ * Defines a new alias rule.
+ *
+ * If @name is non-NULL, the member called @name in the external
+ * representation of the currently visited object is defined as an
+ * alias for the member described by @source.  It is not allowed to
+ * call this function when the currently visited type is not an
+ * object.
+ *
+ * If @name is NULL, all members of the object described by @source
+ * are considered to have alias members with the same key in the
+ * currently visited object.
+ *
+ * @source is a NULL-terminated non-empty array of names that describe
+ * the path to a member, starting from the currently visited object.
+ * All elements in @source except the last one should describe
+ * objects.  If an intermediate element refers to a member with a
+ * non-object type, the alias won't work (this case can legitimately
+ * happen in unions where an alias only makes sense for one branch,
+ * but not for another).
+ *
+ * The alias stays valid until the current alias scope ends.
+ * visit_start/end_struct() implicitly start/end an alias scope.
+ * Additionally, visit_start/end_alias_scope() can be used to explicitly
+ * create a nested alias scope.
+ */
+void visit_define_alias(Visitor *v, const char *name, const char **source);
+
+/*
+ * Begins an explicit alias scope.
+ *
+ * Alias definitions after here will only stay valid until the
+ * corresponding visit_end_alias_scope() is called.
+ */
+void visit_start_alias_scope(Visitor *v);
+
+/*
+ * Ends an explicit alias scope.
+ *
+ * Alias definitions between the correspoding visit_start_alias_scope()
+ * call and here go out of scope and won't apply in later code any more.
+ */
+void visit_end_alias_scope(Visitor *v);
+
 /*
  * Visit an enum value.
  *
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index a641adec51..79df6901ae 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -153,6 +153,28 @@ bool visit_deprecated(Visitor *v, const char *name)
     return true;
 }
 
+void visit_define_alias(Visitor *v, const char *name, const char **source)
+{
+    assert(source[0] != NULL);
+    if (v->define_alias) {
+        v->define_alias(v, name, source);
+    }
+}
+
+void visit_start_alias_scope(Visitor *v)
+{
+    if (v->start_alias_scope) {
+        v->start_alias_scope(v);
+    }
+}
+
+void visit_end_alias_scope(Visitor *v)
+{
+    if (v->end_alias_scope) {
+        v->end_alias_scope(v);
+    }
+}
+
 bool visit_is_input(Visitor *v)
 {
     return v->type == VISITOR_INPUT;
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v4 2/8] qapi: Remember alias definitions in qobject-input-visitor
  2021-09-17 16:13 [PATCH v4 0/8] qapi: Add support for aliases Kevin Wolf
  2021-09-17 16:13 ` [PATCH v4 1/8] qapi: Add interfaces for alias support to Visitor Kevin Wolf
@ 2021-09-17 16:13 ` Kevin Wolf
  2021-09-17 16:13 ` [PATCH v4 3/8] qapi: Simplify full_name_nth() " Kevin Wolf
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Kevin Wolf @ 2021-09-17 16:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, jsnow, armbru

This makes qobject-input-visitor remember the currently valid aliases in
each StackObject. It doesn't actually allow using the aliases yet.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qapi/qobject-input-visitor.c | 153 +++++++++++++++++++++++++++++++++++
 1 file changed, 153 insertions(+)

diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index 04b790412e..44de05ad0a 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -30,6 +30,50 @@
 #include "qemu/cutils.h"
 #include "qemu/option.h"
 
+/*
+ * Describes an alias that is relevant for the current StackObject,
+ * either because it aliases a member of the currently visited object
+ * or because it aliases a member of a nested object.
+ *
+ * When processing a nested object, all InputVisitorAlias objects that
+ * are relevant for the nested object are propagated, i.e. copied with
+ * the name of the nested object removed from @source.
+ */
+typedef struct InputVisitorAlias {
+    /* StackObject in which the alias was defined */
+    struct StackObject *alias_so;
+
+    /*
+     * Alias name as defined for @alias_so.
+     * NULL means that this is a wildcard alias, i.e. all members of
+     * @src get an alias in @alias_so with the same name.
+     */
+    const char *name;
+
+    /*
+     * NULL-terminated array representing a path to the source member
+     * that the alias refers to.
+     *
+     * Must contain at least one non-NULL element if @alias is not NULL.
+     *
+     * If it contains no non-NULL element, @alias_so must be different
+     * from the StackObject which contains this InputVisitorAlias in
+     * its aliases list.  In this case, all elements in the currently
+     * visited object have an alias with the same name in @alias_so.
+     */
+    const char **src;
+
+    /*
+     * The alias remains valid as long as the StackObject which
+     * contains this InputVisitorAlias in its aliases list has
+     * StackObject.alias_scope_nesting >= InputVisitorAlias.scope_nesting
+     * or until the whole StackObject is removed.
+     */
+    int scope_nesting;
+
+    QSIMPLEQ_ENTRY(InputVisitorAlias) next;
+} InputVisitorAlias;
+
 typedef struct StackObject {
     const char *name;            /* Name of @obj in its parent, if any */
     QObject *obj;                /* QDict or QList being visited */
@@ -39,6 +83,14 @@ typedef struct StackObject {
     const QListEntry *entry;    /* If @obj is QList: unvisited tail */
     unsigned index;             /* If @obj is QList: list index of @entry */
 
+    /*
+     * Aliases for members in the visited object or nested objects.
+     * Ordered so that the more locally defined alias comes first.
+     */
+    QSIMPLEQ_HEAD(, InputVisitorAlias) aliases;
+
+    int alias_scope_nesting;    /* Number of open alias scopes */
+
     QSLIST_ENTRY(StackObject) node; /* parent */
 } StackObject;
 
@@ -205,6 +257,45 @@ static const char *qobject_input_get_keyval(QObjectInputVisitor *qiv,
     return qstring_get_str(qstr);
 }
 
+/*
+ * Propagate aliases from the parent StackObject @src to its direct
+ * child StackObject @dst, which is representing the child struct @name.
+ *
+ * Every alias whose source path begins with @dst->name and which still
+ * applies in @dst (i.e. it is either a wildcard alias or has at least
+ * one more source path element) is propagated to @dst with the first
+ * element (i.e. @dst->name) removed from the source path.
+ */
+static void propagate_aliases(StackObject *dst, StackObject *src)
+{
+    InputVisitorAlias *a;
+    InputVisitorAlias *propagated_alias;
+
+    QSIMPLEQ_FOREACH(a, &src->aliases, next) {
+        if (!a->src[0] || strcmp(a->src[0], dst->name)) {
+            continue;
+        }
+
+        /*
+         * If this is not a wildcard alias, but a->src[1] is NULL,
+         * then it referred to a->name in src and doesn't apply inside
+         * dst any more.
+         */
+        if (a->name && !a->src[1]) {
+            continue;
+        }
+
+        propagated_alias = g_new(InputVisitorAlias, 1);
+        *propagated_alias = (InputVisitorAlias) {
+            .name       = a->name,
+            .alias_so   = a->alias_so,
+            .src        = &a->src[1],
+        };
+
+        QSIMPLEQ_INSERT_TAIL(&dst->aliases, propagated_alias, next);
+    }
+}
+
 static const QListEntry *qobject_input_push(QObjectInputVisitor *qiv,
                                             const char *name,
                                             QObject *obj, void *qapi)
@@ -219,6 +310,7 @@ static const QListEntry *qobject_input_push(QObjectInputVisitor *qiv,
     tos->name = name;
     tos->obj = obj;
     tos->qapi = qapi;
+    QSIMPLEQ_INIT(&tos->aliases);
 
     if (qdict) {
         h = g_hash_table_new(g_str_hash, g_str_equal);
@@ -228,6 +320,9 @@ static const QListEntry *qobject_input_push(QObjectInputVisitor *qiv,
             g_hash_table_insert(h, (void *)qdict_entry_key(entry), NULL);
         }
         tos->h = h;
+        if (!QSLIST_EMPTY(&qiv->stack)) {
+            propagate_aliases(tos, QSLIST_FIRST(&qiv->stack));
+        }
     } else {
         assert(qlist);
         tos->entry = qlist_first(qlist);
@@ -259,10 +354,17 @@ static bool qobject_input_check_struct(Visitor *v, Error **errp)
 
 static void qobject_input_stack_object_free(StackObject *tos)
 {
+    InputVisitorAlias *a;
+
     if (tos->h) {
         g_hash_table_unref(tos->h);
     }
 
+    while ((a = QSIMPLEQ_FIRST(&tos->aliases))) {
+        QSIMPLEQ_REMOVE_HEAD(&tos->aliases, next);
+        g_free(a);
+    }
+
     g_free(tos);
 }
 
@@ -276,6 +378,54 @@ static void qobject_input_pop(Visitor *v, void **obj)
     qobject_input_stack_object_free(tos);
 }
 
+static void qobject_input_start_alias_scope(Visitor *v)
+{
+    QObjectInputVisitor *qiv = to_qiv(v);
+    StackObject *tos = QSLIST_FIRST(&qiv->stack);
+
+    tos->alias_scope_nesting++;
+}
+
+static void qobject_input_end_alias_scope(Visitor *v)
+{
+    QObjectInputVisitor *qiv = to_qiv(v);
+    StackObject *tos = QSLIST_FIRST(&qiv->stack);
+    InputVisitorAlias *a, *next;
+
+    assert(tos->alias_scope_nesting > 0);
+    tos->alias_scope_nesting--;
+
+    QSIMPLEQ_FOREACH_SAFE(a, &tos->aliases, next, next) {
+        if (a->scope_nesting > tos->alias_scope_nesting) {
+            QSIMPLEQ_REMOVE(&tos->aliases, a, InputVisitorAlias, next);
+            g_free(a);
+        }
+    }
+}
+
+static void qobject_input_define_alias(Visitor *v, const char *name,
+                                       const char **source)
+{
+    QObjectInputVisitor *qiv = to_qiv(v);
+    StackObject *tos = QSLIST_FIRST(&qiv->stack);
+    InputVisitorAlias *alias = g_new(InputVisitorAlias, 1);
+
+    /*
+     * The source path can become empty during alias propagation for
+     * wildcard aliases, but not when defining an alias (it would map
+     * all names onto themselves, which doesn't make sense).
+     */
+    assert(source[0]);
+
+    *alias = (InputVisitorAlias) {
+        .name       = name,
+        .alias_so   = tos,
+        .src        = source,
+    };
+
+    QSIMPLEQ_INSERT_HEAD(&tos->aliases, alias, next);
+}
+
 static bool qobject_input_start_struct(Visitor *v, const char *name, void **obj,
                                        size_t size, Error **errp)
 {
@@ -717,6 +867,9 @@ static QObjectInputVisitor *qobject_input_visitor_base_new(QObject *obj)
     v->visitor.start_alternate = qobject_input_start_alternate;
     v->visitor.optional = qobject_input_optional;
     v->visitor.deprecated_accept = qobject_input_deprecated_accept;
+    v->visitor.define_alias = qobject_input_define_alias;
+    v->visitor.start_alias_scope = qobject_input_start_alias_scope;
+    v->visitor.end_alias_scope = qobject_input_end_alias_scope;
     v->visitor.free = qobject_input_free;
 
     v->root = qobject_ref(obj);
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v4 3/8] qapi: Simplify full_name_nth() in qobject-input-visitor
  2021-09-17 16:13 [PATCH v4 0/8] qapi: Add support for aliases Kevin Wolf
  2021-09-17 16:13 ` [PATCH v4 1/8] qapi: Add interfaces for alias support to Visitor Kevin Wolf
  2021-09-17 16:13 ` [PATCH v4 2/8] qapi: Remember alias definitions in qobject-input-visitor Kevin Wolf
@ 2021-09-17 16:13 ` Kevin Wolf
  2021-09-17 16:13 ` [PATCH v4 4/8] qapi: Store Error in StackObject.h for qobject-input-visitor Kevin Wolf
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Kevin Wolf @ 2021-09-17 16:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, jsnow, armbru

Instead of counting how many elements from the top of the stack we need
to ignore until we find the thing we're interested in, we can just
directly pass the StackObject pointer because all callers already know
it.

We only need a different way now to tell if we want to know the name of
something contained in the given StackObject or of the StackObject
itself. Make this explicit with a new boolean parameter.

This makes the function easier to use in cases where we have the
StackObject, but don't know how many steps down the stack it is. The
following patches will introduce such a caller.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qapi/qobject-input-visitor.c | 43 ++++++++++++++++++++----------------
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index 44de05ad0a..3eb3b34894 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -115,20 +115,20 @@ static QObjectInputVisitor *to_qiv(Visitor *v)
 }
 
 /*
- * Find the full name of something @qiv is currently visiting.
- * @qiv is visiting something named @name in the stack of containers
- * @qiv->stack.
- * If @n is zero, return its full name.
- * If @n is positive, return the full name of the @n-th container
- * counting from the top.  The stack of containers must have at least
- * @n elements.
- * The returned string is valid until the next full_name_nth(@v) or
- * destruction of @v.
+ * Find the full name of a member in @so which @qiv is currently
+ * visiting.  If the currently visited thing is an object, @name is
+ * the (local) name of the member to describe.  If it is a list, @name
+ * is ignored and the current index (so->index) is included.
+ *
+ * If @skip_member is true, find the full name of @so itself instead.
+ * @name must be NULL then.
+ *
+ * The returned string is valid until the next full_name_so(@qiv) or
+ * destruction of @qiv.
  */
-static const char *full_name_nth(QObjectInputVisitor *qiv, const char *name,
-                                 int n)
+static const char *full_name_so(QObjectInputVisitor *qiv, const char *name,
+                                bool skip_member, StackObject *so)
 {
-    StackObject *so;
     char buf[32];
 
     if (qiv->errname) {
@@ -137,10 +137,14 @@ static const char *full_name_nth(QObjectInputVisitor *qiv, const char *name,
         qiv->errname = g_string_new("");
     }
 
-    QSLIST_FOREACH(so , &qiv->stack, node) {
-        if (n) {
-            n--;
-        } else if (qobject_type(so->obj) == QTYPE_QDICT) {
+    if (skip_member && so) {
+        assert(name == NULL);
+        name = so->name;
+        so = QSLIST_NEXT(so, node);
+    }
+
+    for (; so; so = QSLIST_NEXT(so, node)) {
+        if (qobject_type(so->obj) == QTYPE_QDICT) {
             g_string_prepend(qiv->errname, name ?: "<anonymous>");
             g_string_prepend_c(qiv->errname, '.');
         } else {
@@ -151,7 +155,6 @@ static const char *full_name_nth(QObjectInputVisitor *qiv, const char *name,
         }
         name = so->name;
     }
-    assert(!n);
 
     if (name) {
         g_string_prepend(qiv->errname, name);
@@ -166,7 +169,9 @@ static const char *full_name_nth(QObjectInputVisitor *qiv, const char *name,
 
 static const char *full_name(QObjectInputVisitor *qiv, const char *name)
 {
-    return full_name_nth(qiv, name, 0);
+    StackObject *tos = QSLIST_FIRST(&qiv->stack);
+
+    return full_name_so(qiv, name, false, tos);
 }
 
 static QObject *qobject_input_try_get_object(QObjectInputVisitor *qiv,
@@ -513,7 +518,7 @@ static bool qobject_input_check_list(Visitor *v, Error **errp)
 
     if (tos->entry) {
         error_setg(errp, "Only %u list elements expected in %s",
-                   tos->index + 1, full_name_nth(qiv, NULL, 1));
+                   tos->index + 1, full_name_so(qiv, NULL, true, tos));
         return false;
     }
     return true;
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v4 4/8] qapi: Store Error in StackObject.h for qobject-input-visitor
  2021-09-17 16:13 [PATCH v4 0/8] qapi: Add support for aliases Kevin Wolf
                   ` (2 preceding siblings ...)
  2021-09-17 16:13 ` [PATCH v4 3/8] qapi: Simplify full_name_nth() " Kevin Wolf
@ 2021-09-17 16:13 ` Kevin Wolf
  2021-09-17 16:13 ` [PATCH v4 5/8] qapi: Apply aliases in qobject-input-visitor Kevin Wolf
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Kevin Wolf @ 2021-09-17 16:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, jsnow, armbru

StackObject.h is a GHashTable that stores all keys in the input that
haven't been consumed yet. If qobject_input_check_struct() still finds
any entry, an error is returned because the input was unexpected. The
value of hash table entries is currently unused (always NULL).

The next patch implements, amongst others, wildcard aliases that can
lead to situations where it can't be decided immediately if a value is
still used elsewhere or if a conflict (two values for one member) needs
to be reported.

Allow it to store an Error object in the hash table so that a good error
message is used for qobject_input_check_struct() if the value isn't
consumed elsewhere, but no error is reported if some other object does
consume the value.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qapi/qobject-input-visitor.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index 3eb3b34894..90ebd2fe95 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -79,7 +79,14 @@ typedef struct StackObject {
     QObject *obj;                /* QDict or QList being visited */
     void *qapi; /* sanity check that caller uses same pointer */
 
-    GHashTable *h;              /* If @obj is QDict: unvisited keys */
+    /*
+     * If @obj is QDict:
+     * Keys are the unvisited keys. Values are Error objects to be
+     * returned in qobject_input_check_struct() if the value was not
+     * consumed, or NULL for a default error.
+     */
+    GHashTable *h;
+
     const QListEntry *entry;    /* If @obj is QList: unvisited tail */
     unsigned index;             /* If @obj is QList: list index of @entry */
 
@@ -318,7 +325,8 @@ static const QListEntry *qobject_input_push(QObjectInputVisitor *qiv,
     QSIMPLEQ_INIT(&tos->aliases);
 
     if (qdict) {
-        h = g_hash_table_new(g_str_hash, g_str_equal);
+        h = g_hash_table_new_full(g_str_hash, g_str_equal, NULL,
+                                  (GDestroyNotify) error_free);
         for (entry = qdict_first(qdict);
              entry;
              entry = qdict_next(qdict, entry)) {
@@ -345,13 +353,19 @@ static bool qobject_input_check_struct(Visitor *v, Error **errp)
     StackObject *tos = QSLIST_FIRST(&qiv->stack);
     GHashTableIter iter;
     const char *key;
+    Error *err;
 
     assert(tos && !tos->entry);
 
     g_hash_table_iter_init(&iter, tos->h);
-    if (g_hash_table_iter_next(&iter, (void **)&key, NULL)) {
-        error_setg(errp, "Parameter '%s' is unexpected",
-                   full_name(qiv, key));
+    if (g_hash_table_iter_next(&iter, (void **)&key, (void **)&err)) {
+        if (err) {
+            g_hash_table_steal(tos->h, key);
+            error_propagate(errp, err);
+        } else {
+            error_setg(errp, "Parameter '%s' is unexpected",
+                       full_name(qiv, key));
+        }
         return false;
     }
     return true;
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v4 5/8] qapi: Apply aliases in qobject-input-visitor
  2021-09-17 16:13 [PATCH v4 0/8] qapi: Add support for aliases Kevin Wolf
                   ` (3 preceding siblings ...)
  2021-09-17 16:13 ` [PATCH v4 4/8] qapi: Store Error in StackObject.h for qobject-input-visitor Kevin Wolf
@ 2021-09-17 16:13 ` Kevin Wolf
  2021-09-17 16:13 ` [PATCH v4 6/8] qapi: Revert an accidental change from list to view object Kevin Wolf
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Kevin Wolf @ 2021-09-17 16:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, jsnow, armbru

When looking for an object in a struct in the external representation,
check not only the currently visited struct, but also whether an alias
in the current StackObject matches and try to fetch the value from the
alias then. Providing two values for the same object through different
aliases is an error.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qapi/qobject-input-visitor.c | 232 +++++++++++++++++++++++++++++++++--
 1 file changed, 223 insertions(+), 9 deletions(-)

diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index 90ebd2fe95..bf302a6e07 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -109,6 +109,9 @@ struct QObjectInputVisitor {
     QObject *root;
     bool keyval;                /* Assume @root made with keyval_parse() */
 
+    /* For visiting objects where all members are from aliases */
+    QDict *empty_qdict;
+
     /* Stack of objects being visited (all entries will be either
      * QDict or QList). */
     QSLIST_HEAD(, StackObject) stack;
@@ -181,9 +184,194 @@ static const char *full_name(QObjectInputVisitor *qiv, const char *name)
     return full_name_so(qiv, name, false, tos);
 }
 
+static bool find_object_member(QObjectInputVisitor *qiv,
+                               StackObject **so, const char **name,
+                               bool *is_alias_prefix, Error **errp);
+
+/*
+ * Check whether the member @name in the object visited by @so can be
+ * specified in the input by using the alias described by @a (which
+ * must be an alias contained in so->aliases).
+ *
+ * If @name is only a prefix of the alias source, but doesn't match
+ * immediately, false is returned and *is_alias_prefix is set to true
+ * if it is non-NULL.  In all other cases, *is_alias_prefix is left
+ * unchanged.
+ */
+static bool alias_source_matches(QObjectInputVisitor *qiv,
+                                 StackObject *so, InputVisitorAlias *a,
+                                 const char *name, bool *is_alias_prefix)
+{
+    if (a->src[0] == NULL) {
+        assert(a->name == NULL);
+        return true;
+    }
+
+    if (!strcmp(a->src[0], name)) {
+        if (a->name && a->src[1] == NULL) {
+            /*
+             * We're matching an exact member, the source for this alias is
+             * immediately in @so.
+             */
+            return true;
+        } else if (is_alias_prefix) {
+            /*
+             * We're only looking at a prefix of the source path for the alias.
+             * If the input contains no object of the requested name, we will
+             * implicitly create an empty one so that the alias can still be
+             * used.
+             *
+             * We want to create the implicit object only if the alias is
+             * actually used, but we can't tell here for wildcard aliases (only
+             * a later visitor call will determine this). This means that
+             * wildcard aliases must never have optional keys in their source
+             * path. The QAPI generator checks this condition.
+             */
+            const char *alias_name = a->name;
+            StackObject *alias_so = a->alias_so;
+            if (!a->name || find_object_member(qiv, &alias_so, &alias_name,
+                                               NULL, NULL)) {
+                *is_alias_prefix = true;
+            }
+        }
+    }
+
+    return false;
+}
+
+/*
+ * Find the place in the input where the value for the object member
+ * @name in @so is specified, considering applicable aliases.
+ *
+ * If a value could be found, true is returned and @so and @name are
+ * updated to identify the key name and StackObject where the value
+ * can be found in the input.  (This is either unchanged or the
+ * alias_so/name of an alias.)  The value of @is_alias_prefix on
+ * return is undefined in this case.
+ *
+ * If no value could be found in the input, false is returned and @so
+ * and @name are set to NULL.  This is not an error and @errp remains
+ * unchanged.  If @is_alias_prefix is non-NULL, it is set to true if
+ * the given name is a prefix of the source path of an alias for which
+ * a value may be present in the input.  It is set to false otherwise.
+ *
+ * If an error occurs (e.g. two values are specified for the member
+ * through different names), false is returned and @errp is set.  The
+ * value of @is_alias_prefix is set to false and @name is set to NULL
+ * on return in this case.
+ */
+static bool find_object_member(QObjectInputVisitor *qiv,
+                               StackObject **so, const char **name,
+                               bool *is_alias_prefix, Error **errp)
+{
+    QDict *qdict = qobject_to(QDict, (*so)->obj);
+    const char *found_name = NULL;
+    StackObject *found_so = NULL;
+    bool found_is_wildcard = false;
+    InputVisitorAlias *a;
+
+    if (is_alias_prefix) {
+        *is_alias_prefix = false;
+    }
+
+    /* Directly present in the container */
+    if (qdict_haskey(qdict, *name)) {
+        found_name = *name;
+        found_so = *so;
+    }
+
+    /*
+     * Find aliases whose source path matches @name in this StackObject. We can
+     * then get the value with the key a->name from a->alias_so.
+     */
+    QSIMPLEQ_FOREACH(a, &(*so)->aliases, next) {
+        /*
+         * For single-member aliases, an alias name is specified in the
+         * alias definition. For wildcard aliases, the alias has the same
+         * name as the member in the source object, i.e. *name.
+         */
+        const char *alias_name = a->name ?: *name;
+        StackObject *alias_so = a->alias_so;
+
+        if (!alias_source_matches(qiv, *so, a, *name, is_alias_prefix)) {
+            continue;
+        }
+
+        /*
+         * Check whether the alias is present in the input
+         * (potentially through chained aliases) and update @alias_so
+         * and @alias_name accordingly if so.
+         *
+         * The QAPI generator makes sure that aliases cannot form loops,
+         * so the recursion is guaranteed to terminate.
+         */
+        if (!find_object_member(qiv, &alias_so, &alias_name, NULL, NULL)) {
+            continue;
+        }
+
+        /* Conflict: The input has multiple values for the member */
+        if (found_name) {
+            g_autofree char *name1 =
+                g_strdup(full_name_so(qiv, found_name, false, found_so));
+            g_autofree char *name2 =
+                g_strdup(full_name_so(qiv, alias_name, false, alias_so));
+            Error *local_err = NULL;
+
+            error_setg(&local_err, "Value for parameter '%s' is specified "
+                       "both as '%s' and '%s'",
+                       full_name_so(qiv, *name, false, *so), name1, name2);
+
+            if (!a->name) {
+                /*
+                 * Less local wildcard alias: Assume the value belongs
+                 * elsewhere and ignore it, but store the error in case
+                 * it stays unused.
+                 */
+                g_hash_table_replace(alias_so->h, (void *) alias_name,
+                                     local_err);
+                continue;
+            } else if (found_is_wildcard) {
+                /*
+                 * Override a previously found wildcard alias with a
+                 * single-member alias. Store the error in case the
+                 * value for the wildcard alias isn't used elsewhere.
+                 */
+                g_hash_table_replace(found_so->h, (void *) found_name,
+                                     local_err);
+            } else {
+                /* Any other conflict is definitely an error */
+                error_propagate(errp, local_err);
+                if (is_alias_prefix) {
+                    *is_alias_prefix = false;
+                }
+                *name = NULL;
+                return false;
+            }
+        }
+
+        found_name = alias_name;
+        found_so = alias_so;
+        found_is_wildcard = !a->name;
+    }
+
+    *so = found_so;
+    *name = found_name;
+
+    if (!found_name) {
+        return false;
+    }
+
+    /*
+     * Every source can be used only once. If a value in the input
+     * would end up being used twice through aliases, we'll fail the
+     * second access.
+     */
+    return g_hash_table_contains(found_so->h, found_name);
+}
+
 static QObject *qobject_input_try_get_object(QObjectInputVisitor *qiv,
                                              const char *name,
-                                             bool consume)
+                                             bool consume, Error **errp)
 {
     StackObject *tos;
     QObject *qobj;
@@ -201,10 +389,31 @@ static QObject *qobject_input_try_get_object(QObjectInputVisitor *qiv,
     assert(qobj);
 
     if (qobject_type(qobj) == QTYPE_QDICT) {
-        assert(name);
-        ret = qdict_get(qobject_to(QDict, qobj), name);
-        if (tos->h && consume && ret) {
-            bool removed = g_hash_table_remove(tos->h, name);
+        StackObject *so = tos;
+        const char *key = name;
+        bool is_alias_prefix;
+
+        assert(key);
+        if (!find_object_member(qiv, &so, &key, &is_alias_prefix, errp)) {
+            if (is_alias_prefix) {
+                /*
+                 * The member is not present in the input, but
+                 * something inside of it might still be given through
+                 * an alias. Pretend there was an empty object in the
+                 * input.
+                 */
+                if (!qiv->empty_qdict) {
+                    qiv->empty_qdict = qdict_new();
+                }
+                return QOBJECT(qiv->empty_qdict);
+            } else {
+                return NULL;
+            }
+        }
+        ret = qdict_get(qobject_to(QDict, so->obj), key);
+        assert(ret != NULL);
+        if (so->h && consume) {
+            bool removed = g_hash_table_remove(so->h, key);
             assert(removed);
         }
     } else {
@@ -230,9 +439,10 @@ static QObject *qobject_input_get_object(QObjectInputVisitor *qiv,
                                          const char *name,
                                          bool consume, Error **errp)
 {
-    QObject *obj = qobject_input_try_get_object(qiv, name, consume);
+    ERRP_GUARD();
+    QObject *obj = qobject_input_try_get_object(qiv, name, consume, errp);
 
-    if (!obj) {
+    if (!obj && !*errp) {
         error_setg(errp, QERR_MISSING_PARAMETER, full_name(qiv, name));
     }
     return obj;
@@ -823,13 +1033,16 @@ static bool qobject_input_type_size_keyval(Visitor *v, const char *name,
 static void qobject_input_optional(Visitor *v, const char *name, bool *present)
 {
     QObjectInputVisitor *qiv = to_qiv(v);
-    QObject *qobj = qobject_input_try_get_object(qiv, name, false);
+    Error *local_err = NULL;
+    QObject *qobj = qobject_input_try_get_object(qiv, name, false, &local_err);
 
-    if (!qobj) {
+    /* If there was an error, let the caller try and run into the error */
+    if (!qobj && !local_err) {
         *present = false;
         return;
     }
 
+    error_free(local_err);
     *present = true;
 }
 
@@ -862,6 +1075,7 @@ static void qobject_input_free(Visitor *v)
         qobject_input_stack_object_free(tos);
     }
 
+    qobject_unref(qiv->empty_qdict);
     qobject_unref(qiv->root);
     if (qiv->errname) {
         g_string_free(qiv->errname, TRUE);
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v4 6/8] qapi: Revert an accidental change from list to view object
  2021-09-17 16:13 [PATCH v4 0/8] qapi: Add support for aliases Kevin Wolf
                   ` (4 preceding siblings ...)
  2021-09-17 16:13 ` [PATCH v4 5/8] qapi: Apply aliases in qobject-input-visitor Kevin Wolf
@ 2021-09-17 16:13 ` Kevin Wolf
  2021-09-17 16:13 ` [PATCH v4 7/8] qapi: Add support for aliases Kevin Wolf
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Kevin Wolf @ 2021-09-17 16:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, jsnow, armbru

From: Markus Armbruster <armbru@redhat.com>

A long time ago, commit 23a4b2c6f1 "qapi: Eliminate
QAPISchemaObjectType.check() variable members" replaced the manual
building of the list of members by seen.values(), where @seen is an
OrderedDict mapping names to members.  The list is then stored in
self.members.

With Python 2, this is an innocent change: seen.values() returns "a
copy of the dictionary’s list of values".

With Python 3, it returns a dictionary view object instad.  These
"provide a dynamic view on the dictionary’s entries, which means that
when the dictionary changes, the view reflects these changes."

Commit 23a4b2c6f1 predates the first mention of Python 3 in
scripts/qapi/ by years.  If we had wanted a view object then, we'd
have used seen.viewvalues().

The accidental change of self.members from list to view object keeps
@seen alive longer.  Not wanted, but harmless enough.  I believe
that's all.

However, the change is in the next commit's way, which wants to mess
with self.members.  Revert it.

All other uses of .values() in scripts/qapi/ are of the form

    for ... in dict.values():

where the change to view object is just fine.  Same for .keys() and
.items().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 scripts/qapi/schema.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 3d72c7dfc9..87f80f8de2 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -440,7 +440,7 @@ 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)
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v4 7/8] qapi: Add support for aliases
  2021-09-17 16:13 [PATCH v4 0/8] qapi: Add support for aliases Kevin Wolf
                   ` (5 preceding siblings ...)
  2021-09-17 16:13 ` [PATCH v4 6/8] qapi: Revert an accidental change from list to view object Kevin Wolf
@ 2021-09-17 16:13 ` Kevin Wolf
  2021-09-17 16:13 ` [PATCH v4 8/8] tests/qapi-schema: Test cases " Kevin Wolf
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Kevin Wolf @ 2021-09-17 16:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, jsnow, armbru

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           | 109 ++++++++++++++++++++++-
 docs/sphinx/qapidoc.py                 |   2 +-
 scripts/qapi/expr.py                   |  54 +++++++++++-
 scripts/qapi/schema.py                 | 115 +++++++++++++++++++++++--
 scripts/qapi/types.py                  |   4 +-
 scripts/qapi/visit.py                  |  34 +++++++-
 tests/qapi-schema/test-qapi.py         |   7 +-
 tests/qapi-schema/unknown-expr-key.err |   2 +-
 8 files changed, 308 insertions(+), 19 deletions(-)

diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
index ced7a5ffe1..722a8e62f7 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
 ---------------
@@ -894,6 +903,100 @@ 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.
+
+This allows using the same QAPI type and visitor even when the syntax
+has some variations between different external interfaces such as QMP
+and the command line.  Note that aliases are not visible in the schema
+introspection yet, which may make their use in QMP unpractical for now.
+
+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.  Optional objects may not be used in the path of a wildcard
+alias.
+
+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.
+
+Wildcard aliases can lead to situations where the same name could either
+refer to a member of a nested object (through the wildcard alias) or to
+a local member.  In this case, the wildcard alias can't be used to
+specify a value for the member of the nested object.
+
+
 Documentation comments
 ----------------------
 
diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
index d791b59492..18bf6cf4b8 100644
--- a/docs/sphinx/qapidoc.py
+++ b/docs/sphinx/qapidoc.py
@@ -315,7 +315,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 90bde501b0..a9dfa9252e 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -447,6 +447,51 @@ 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)
+
+            permissive = a['name'] in info.pragma.member_name_exceptions
+            check_name_lower(a['name'], info, source,
+                             permit_upper=permissive,
+                             permit_underscore=permissive)
+            desc = "alias '%s'" % a['name']
+        else:
+            desc = "wildcard alias"
+
+        if not isinstance(a['source'], list):
+            raise QAPISemError(
+                info, f"{desc} member 'source' must be an array")
+        if not a['source']:
+            raise QAPISemError(
+                info, f"{desc} member 'source' must not be empty")
+
+        source = f"element in {desc} member 'source'"
+        for s in a['source']:
+            check_name_is_str(s, info, source)
+
+
 def check_enum(expr: _JSONObject, info: QAPISourceInfo) -> None:
     """
     Normalize and validate this expression as an ``enum`` definition.
@@ -500,6 +545,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:
@@ -526,6 +572,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")
 
@@ -665,7 +713,8 @@ 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)
@@ -676,7 +725,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 87f80f8de2..611ea14f72 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -145,7 +145,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,
@@ -391,7 +391,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
@@ -409,6 +409,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
@@ -440,12 +443,18 @@ def check(self, schema):
         for m in self.local_members:
             m.check(schema)
             m.check_clash(self.info, seen)
+
+        # Make a copy before aliases are added to @seen
         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, a.source, members)
+
         self.members = members  # mark completed
 
     # Check that the members of this type do not cause duplicate JSON members,
@@ -457,6 +466,68 @@ def check_clash(self, info, seen):
         for m in self.members:
             m.check_clash(info, seen)
 
+    def check_path(self, alias, path, members=None, local_aliases_seen=()):
+        assert isinstance(path, list)
+
+        if not path:
+            return
+
+        for a in self.aliases:
+            if a.name == path[0]:
+                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]))
+
+                self.check_path(alias, a.source + path[1:], members,
+                                (*local_aliases_seen, a))
+                return
+
+        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 == path[0]:
+                # Wildcard aliases can only accept object types in the whole
+                # path; for single-member aliases, the last element can be
+                # any type
+                nested_path = path[1:]
+                need_obj = (alias.name is None) or nested_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 nested_path:
+                    m.type.check_path(alias, nested_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:
+                    v.type.check_path(alias, path)
+                    return
+                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
@@ -501,7 +572,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)
@@ -666,7 +737,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):
@@ -732,6 +803,29 @@ 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 not self.name:
+            return "wildcard alias"
+        return super().describe(info)
+
+
 class QAPISchemaObjectTypeMember(QAPISchemaMember):
     def __init__(self, name, info, typ, optional, ifcond=None, features=None):
         super().__init__(name, info, ifcond)
@@ -999,6 +1093,12 @@ def _make_features(self, features, info):
                                   QAPISchemaIfCond(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,
                                      QAPISchemaIfCond(v.get('if')))
@@ -1075,11 +1175,12 @@ def _def_struct_type(self, expr, info, doc):
         base = expr.get('base')
         data = expr['data']
         ifcond = QAPISchemaIfCond(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)
@@ -1098,6 +1199,7 @@ def _def_union_type(self, expr, info, doc):
         data = expr['data']
         base = expr.get('base')
         ifcond = QAPISchemaIfCond(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
@@ -1126,7 +1228,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 831294fe42..e8bf097eb9 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -19,6 +19,7 @@
 from .gen import QAPISchemaModularCVisitor, ifcontext
 from .schema import (
     QAPISchema,
+    QAPISchemaAlias,
     QAPISchemaEnumMember,
     QAPISchemaFeature,
     QAPISchemaIfCond,
@@ -327,7 +328,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 9d9196a143..679758fead 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -24,6 +24,7 @@
 from .gen import QAPISchemaModularCVisitor, ifcontext
 from .schema import (
     QAPISchema,
+    QAPISchemaAlias,
     QAPISchemaEnumMember,
     QAPISchemaEnumType,
     QAPISchemaFeature,
@@ -59,7 +60,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)
@@ -67,6 +69,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"' % element for element 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)) {
@@ -147,6 +167,11 @@ def gen_visit_object_members(name: str,
     }
 ''')
 
+    if aliases:
+        ret += mcgen('''
+    visit_end_alias_scope(v);
+''')
+
     ret += mcgen('''
     return true;
 }
@@ -375,14 +400,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 73cffae2b6..c3d92fd51f 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/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



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v4 8/8] tests/qapi-schema: Test cases for aliases
  2021-09-17 16:13 [PATCH v4 0/8] qapi: Add support for aliases Kevin Wolf
                   ` (6 preceding siblings ...)
  2021-09-17 16:13 ` [PATCH v4 7/8] qapi: Add support for aliases Kevin Wolf
@ 2021-09-17 16:13 ` Kevin Wolf
  2021-10-01  9:28 ` [PATCH v4 0/8] qapi: Add support " Kevin Wolf
  2021-10-26 21:23 ` John Snow
  9 siblings, 0 replies; 13+ messages in thread
From: Kevin Wolf @ 2021-09-17 16:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, jsnow, armbru

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/unit/test-qmp-cmds.c                    |  10 +
 tests/unit/test-qobject-input-visitor.c       | 271 ++++++++++++++++++
 tests/qapi-schema/alias-bad-type.err          |   2 +
 tests/qapi-schema/alias-bad-type.json         |   3 +
 tests/qapi-schema/alias-bad-type.out          |   0
 tests/qapi-schema/alias-missing-source.err    |   2 +
 tests/qapi-schema/alias-missing-source.json   |   3 +
 tests/qapi-schema/alias-missing-source.out    |   0
 tests/qapi-schema/alias-name-bad-type.err     |   2 +
 tests/qapi-schema/alias-name-bad-type.json    |   3 +
 tests/qapi-schema/alias-name-bad-type.out     |   0
 tests/qapi-schema/alias-name-conflict.err     |   2 +
 tests/qapi-schema/alias-name-conflict.json    |   4 +
 tests/qapi-schema/alias-name-conflict.out     |   0
 tests/qapi-schema/alias-recursive.err         |   2 +
 tests/qapi-schema/alias-recursive.json        |   4 +
 tests/qapi-schema/alias-recursive.out         |   0
 tests/qapi-schema/alias-source-bad-type.err   |   2 +
 tests/qapi-schema/alias-source-bad-type.json  |   3 +
 tests/qapi-schema/alias-source-bad-type.out   |   0
 .../alias-source-elem-bad-type.err            |   2 +
 .../alias-source-elem-bad-type.json           |   3 +
 .../alias-source-elem-bad-type.out            |   0
 tests/qapi-schema/alias-source-empty.err      |   2 +
 tests/qapi-schema/alias-source-empty.json     |   3 +
 tests/qapi-schema/alias-source-empty.out      |   0
 .../alias-source-inexistent-variants.err      |   2 +
 .../alias-source-inexistent-variants.json     |  12 +
 .../alias-source-inexistent-variants.out      |   0
 tests/qapi-schema/alias-source-inexistent.err |   2 +
 .../qapi-schema/alias-source-inexistent.json  |   3 +
 tests/qapi-schema/alias-source-inexistent.out |   0
 .../alias-source-non-object-path.err          |   2 +
 .../alias-source-non-object-path.json         |   3 +
 .../alias-source-non-object-path.out          |   0
 .../alias-source-non-object-wildcard.err      |   2 +
 .../alias-source-non-object-wildcard.json     |   3 +
 .../alias-source-non-object-wildcard.out      |   0
 ...lias-source-optional-wildcard-indirect.err |   2 +
 ...ias-source-optional-wildcard-indirect.json |   6 +
 ...lias-source-optional-wildcard-indirect.out |   0
 .../alias-source-optional-wildcard.err        |   2 +
 .../alias-source-optional-wildcard.json       |   5 +
 .../alias-source-optional-wildcard.out        |   0
 tests/qapi-schema/alias-unknown-key.err       |   3 +
 tests/qapi-schema/alias-unknown-key.json      |   3 +
 tests/qapi-schema/alias-unknown-key.out       |   0
 tests/qapi-schema/aliases-bad-type.err        |   2 +
 tests/qapi-schema/aliases-bad-type.json       |   3 +
 tests/qapi-schema/aliases-bad-type.out        |   0
 tests/qapi-schema/meson.build                 |  16 ++
 tests/qapi-schema/qapi-schema-test.json       |  35 +++
 tests/qapi-schema/qapi-schema-test.out        |  44 +++
 53 files changed, 473 insertions(+)
 create mode 100644 tests/qapi-schema/alias-bad-type.err
 create mode 100644 tests/qapi-schema/alias-bad-type.json
 create mode 100644 tests/qapi-schema/alias-bad-type.out
 create mode 100644 tests/qapi-schema/alias-missing-source.err
 create mode 100644 tests/qapi-schema/alias-missing-source.json
 create mode 100644 tests/qapi-schema/alias-missing-source.out
 create mode 100644 tests/qapi-schema/alias-name-bad-type.err
 create mode 100644 tests/qapi-schema/alias-name-bad-type.json
 create mode 100644 tests/qapi-schema/alias-name-bad-type.out
 create mode 100644 tests/qapi-schema/alias-name-conflict.err
 create mode 100644 tests/qapi-schema/alias-name-conflict.json
 create mode 100644 tests/qapi-schema/alias-name-conflict.out
 create mode 100644 tests/qapi-schema/alias-recursive.err
 create mode 100644 tests/qapi-schema/alias-recursive.json
 create mode 100644 tests/qapi-schema/alias-recursive.out
 create mode 100644 tests/qapi-schema/alias-source-bad-type.err
 create mode 100644 tests/qapi-schema/alias-source-bad-type.json
 create mode 100644 tests/qapi-schema/alias-source-bad-type.out
 create mode 100644 tests/qapi-schema/alias-source-elem-bad-type.err
 create mode 100644 tests/qapi-schema/alias-source-elem-bad-type.json
 create mode 100644 tests/qapi-schema/alias-source-elem-bad-type.out
 create mode 100644 tests/qapi-schema/alias-source-empty.err
 create mode 100644 tests/qapi-schema/alias-source-empty.json
 create mode 100644 tests/qapi-schema/alias-source-empty.out
 create mode 100644 tests/qapi-schema/alias-source-inexistent-variants.err
 create mode 100644 tests/qapi-schema/alias-source-inexistent-variants.json
 create mode 100644 tests/qapi-schema/alias-source-inexistent-variants.out
 create mode 100644 tests/qapi-schema/alias-source-inexistent.err
 create mode 100644 tests/qapi-schema/alias-source-inexistent.json
 create mode 100644 tests/qapi-schema/alias-source-inexistent.out
 create mode 100644 tests/qapi-schema/alias-source-non-object-path.err
 create mode 100644 tests/qapi-schema/alias-source-non-object-path.json
 create mode 100644 tests/qapi-schema/alias-source-non-object-path.out
 create mode 100644 tests/qapi-schema/alias-source-non-object-wildcard.err
 create mode 100644 tests/qapi-schema/alias-source-non-object-wildcard.json
 create mode 100644 tests/qapi-schema/alias-source-non-object-wildcard.out
 create mode 100644 tests/qapi-schema/alias-source-optional-wildcard-indirect.err
 create mode 100644 tests/qapi-schema/alias-source-optional-wildcard-indirect.json
 create mode 100644 tests/qapi-schema/alias-source-optional-wildcard-indirect.out
 create mode 100644 tests/qapi-schema/alias-source-optional-wildcard.err
 create mode 100644 tests/qapi-schema/alias-source-optional-wildcard.json
 create mode 100644 tests/qapi-schema/alias-source-optional-wildcard.out
 create mode 100644 tests/qapi-schema/alias-unknown-key.err
 create mode 100644 tests/qapi-schema/alias-unknown-key.json
 create mode 100644 tests/qapi-schema/alias-unknown-key.out
 create mode 100644 tests/qapi-schema/aliases-bad-type.err
 create mode 100644 tests/qapi-schema/aliases-bad-type.json
 create mode 100644 tests/qapi-schema/aliases-bad-type.out

diff --git a/tests/unit/test-qmp-cmds.c b/tests/unit/test-qmp-cmds.c
index 83efa39720..8187371691 100644
--- a/tests/unit/test-qmp-cmds.c
+++ b/tests/unit/test-qmp-cmds.c
@@ -77,6 +77,16 @@ void qmp_test_command_cond_features3(Error **errp)
 {
 }
 
+void qmp_test_aliases0(bool has_as0, AliasStruct0 *as0,
+                       bool has_as1, AliasStruct1 *as1,
+                       bool has_as2, AliasStruct2 *as2,
+                       bool has_as3, AliasStruct3 *as3,
+                       bool has_afu, AliasFlatUnion *afu,
+                       bool has_asu, AliasSimpleUnion *asu,
+                       Error **errp)
+{
+}
+
 UserDefTwo *qmp_user_def_cmd2(UserDefOne *ud1a,
                               bool has_udb1, UserDefOne *ud1b,
                               Error **errp)
diff --git a/tests/unit/test-qobject-input-visitor.c b/tests/unit/test-qobject-input-visitor.c
index e41b91a2a6..8a6fe94077 100644
--- a/tests/unit/test-qobject-input-visitor.c
+++ b/tests/unit/test-qobject-input-visitor.c
@@ -952,6 +952,265 @@ static void test_visitor_in_list_union_number(TestInputVisitorData *data,
     g_string_free(gstr_list, true);
 }
 
+static void test_visitor_in_alias_struct_local(TestInputVisitorData *data,
+                                               const void *unused)
+{
+    AliasStruct1 *tmp = NULL;
+    Error *err = NULL;
+    Visitor *v;
+
+    /* Can still specify the real member name with alias support */
+    v = visitor_input_test_init(data, "{ 'foo': 42 }");
+    visit_type_AliasStruct1(v, NULL, &tmp, &error_abort);
+    g_assert_cmpint(tmp->foo, ==, 42);
+    qapi_free_AliasStruct1(tmp);
+
+    /* The alias is a working alternative */
+    v = visitor_input_test_init(data, "{ 'bar': 42 }");
+    visit_type_AliasStruct1(v, NULL, &tmp, &error_abort);
+    g_assert_cmpint(tmp->foo, ==, 42);
+    qapi_free_AliasStruct1(tmp);
+
+    /* But you can't use both at the same time */
+    v = visitor_input_test_init(data, "{ 'foo': 5, 'bar': 42 }");
+    visit_type_AliasStruct1(v, NULL, &tmp, &err);
+    error_free_or_abort(&err);
+}
+
+static void test_visitor_in_alias_struct_nested(TestInputVisitorData *data,
+                                                const void *unused)
+{
+    AliasStruct2 *tmp = NULL;
+    Error *err = NULL;
+    Visitor *v;
+
+    /* Can still specify the real member names with alias support */
+    v = visitor_input_test_init(data, "{ 'nested': { 'foo': 42 } }");
+    visit_type_AliasStruct2(v, NULL, &tmp, &error_abort);
+    g_assert_cmpint(tmp->nested->foo, ==, 42);
+    qapi_free_AliasStruct2(tmp);
+
+    /* The inner alias is a working alternative */
+    v = visitor_input_test_init(data, "{ 'nested': { 'bar': 42 } }");
+    visit_type_AliasStruct2(v, NULL, &tmp, &error_abort);
+    g_assert_cmpint(tmp->nested->foo, ==, 42);
+    qapi_free_AliasStruct2(tmp);
+
+    /* So is the outer alias */
+    v = visitor_input_test_init(data, "{ 'bar': 42 }");
+    visit_type_AliasStruct2(v, NULL, &tmp, &error_abort);
+    g_assert_cmpint(tmp->nested->foo, ==, 42);
+    qapi_free_AliasStruct2(tmp);
+
+    /* You can't use more than one option at the same time */
+    v = visitor_input_test_init(data, "{ 'bar': 5, 'nested': { 'foo': 42 } }");
+    visit_type_AliasStruct2(v, NULL, &tmp, &err);
+    error_free_or_abort(&err);
+
+    v = visitor_input_test_init(data, "{ 'bar': 5, 'nested': { 'bar': 42 } }");
+    visit_type_AliasStruct2(v, NULL, &tmp, &err);
+    error_free_or_abort(&err);
+
+    v = visitor_input_test_init(data, "{ 'nested': { 'foo': 42, 'bar': 42 } }");
+    visit_type_AliasStruct2(v, NULL, &tmp, &err);
+    error_free_or_abort(&err);
+
+    v = visitor_input_test_init(data, "{ 'bar': 5, "
+                                      "  'nested': { 'foo': 42, 'bar': 42 } }");
+    visit_type_AliasStruct2(v, NULL, &tmp, &err);
+    error_free_or_abort(&err);
+}
+
+static void test_visitor_in_alias_wildcard(TestInputVisitorData *data,
+                                           const void *unused)
+{
+    AliasStruct3 *tmp = NULL;
+    Error *err = NULL;
+    Visitor *v;
+
+    /* Can still specify the real member names with alias support */
+    v = visitor_input_test_init(data, "{ 'nested': { 'foo': 42 } }");
+    visit_type_AliasStruct3(v, NULL, &tmp, &error_abort);
+    g_assert_cmpint(tmp->nested->foo, ==, 42);
+    qapi_free_AliasStruct3(tmp);
+
+    /* The wildcard alias makes it work on the top level */
+    v = visitor_input_test_init(data, "{ 'foo': 42 }");
+    visit_type_AliasStruct3(v, NULL, &tmp, &error_abort);
+    g_assert_cmpint(tmp->nested->foo, ==, 42);
+    qapi_free_AliasStruct3(tmp);
+
+    /* It makes the inner alias available, too */
+    v = visitor_input_test_init(data, "{ 'bar': 42 }");
+    visit_type_AliasStruct3(v, NULL, &tmp, &error_abort);
+    g_assert_cmpint(tmp->nested->foo, ==, 42);
+    qapi_free_AliasStruct3(tmp);
+
+    /* You can't use more than one option at the same time */
+    v = visitor_input_test_init(data, "{ 'foo': 42, 'nested': { 'foo': 42 } }");
+    visit_type_AliasStruct3(v, NULL, &tmp, &err);
+    error_free_or_abort(&err);
+
+    v = visitor_input_test_init(data, "{ 'bar': 42, 'nested': { 'foo': 42 } }");
+    visit_type_AliasStruct3(v, NULL, &tmp, &err);
+    error_free_or_abort(&err);
+
+    v = visitor_input_test_init(data, "{ 'foo': 42, 'nested': { 'bar': 42 } }");
+    visit_type_AliasStruct3(v, NULL, &tmp, &err);
+    error_free_or_abort(&err);
+
+    v = visitor_input_test_init(data, "{ 'bar': 42, 'nested': { 'bar': 42 } }");
+    visit_type_AliasStruct3(v, NULL, &tmp, &err);
+    error_free_or_abort(&err);
+
+    v = visitor_input_test_init(data, "{ 'foo': 42, 'bar': 42 }");
+    visit_type_AliasStruct3(v, NULL, &tmp, &err);
+    error_free_or_abort(&err);
+}
+
+static void test_visitor_in_alias_flat_union(TestInputVisitorData *data,
+                                             const void *unused)
+{
+    AliasFlatUnion *tmp = NULL;
+    Error *err = NULL;
+    Visitor *v;
+
+    /* Can still specify the real member name with alias support */
+    v = visitor_input_test_init(data, "{ 'tag': 'drei' }");
+    visit_type_AliasFlatUnion(v, NULL, &tmp, &error_abort);
+    g_assert_cmpint(tmp->tag, ==, FEATURE_ENUM1_DREI);
+    qapi_free_AliasFlatUnion(tmp);
+
+    /* Use alias for a base member (the discriminator even) */
+    v = visitor_input_test_init(data, "{ 'variant': 'zwei' }");
+    visit_type_AliasFlatUnion(v, NULL, &tmp, &error_abort);
+    g_assert_cmpint(tmp->tag, ==, FEATURE_ENUM1_ZWEI);
+    qapi_free_AliasFlatUnion(tmp);
+
+    /* Use alias for a variant member */
+    v = visitor_input_test_init(data, "{ 'tag': 'eins', 'bar': 42 }");
+    visit_type_AliasFlatUnion(v, NULL, &tmp, &error_abort);
+    g_assert_cmpint(tmp->tag, ==, FEATURE_ENUM1_EINS);
+    g_assert_cmpint(tmp->u.eins.foo, ==, 42);
+    qapi_free_AliasFlatUnion(tmp);
+
+    /* Both together */
+    v = visitor_input_test_init(data, "{ 'variant': 'eins', 'bar': 42 }");
+    visit_type_AliasFlatUnion(v, NULL, &tmp, &error_abort);
+    g_assert_cmpint(tmp->tag, ==, FEATURE_ENUM1_EINS);
+    g_assert_cmpint(tmp->u.eins.foo, ==, 42);
+    qapi_free_AliasFlatUnion(tmp);
+
+    /* You can't use more than one option at the same time for each alias */
+    v = visitor_input_test_init(data, "{ 'variant': 'zwei', 'tag': 'drei' }");
+    visit_type_AliasFlatUnion(v, NULL, &tmp, &err);
+    error_free_or_abort(&err);
+
+    v = visitor_input_test_init(data, "{ 'tag': 'eins', 'foo': 6, 'bar': 9 }");
+    visit_type_AliasFlatUnion(v, NULL, &tmp, &err);
+    error_free_or_abort(&err);
+}
+
+static void test_visitor_in_alias_simple_union(TestInputVisitorData *data,
+                                               const void *unused)
+{
+    AliasSimpleUnion *tmp = NULL;
+    Error *err = NULL;
+    Visitor *v;
+
+    /* Can still specify the real member name with alias support */
+    v = visitor_input_test_init(data, "{ 'type': 'eins', "
+                                      "  'data': { 'foo': 42 } }");
+    visit_type_AliasSimpleUnion(v, NULL, &tmp, &error_abort);
+    g_assert_cmpint(tmp->type, ==, ALIAS_SIMPLE_UNION_KIND_EINS);
+    g_assert_cmpint(tmp->u.eins.data->foo, ==, 42);
+    qapi_free_AliasSimpleUnion(tmp);
+
+    /* 'type' can be aliased */
+    v = visitor_input_test_init(data, "{ 'tag': 'eins', "
+                                      "  'data': { 'foo': 42 } }");
+    visit_type_AliasSimpleUnion(v, NULL, &tmp, &error_abort);
+    g_assert_cmpint(tmp->type, ==, ALIAS_SIMPLE_UNION_KIND_EINS);
+    g_assert_cmpint(tmp->u.eins.data->foo, ==, 42);
+    qapi_free_AliasSimpleUnion(tmp);
+
+    /* The wildcard alias makes it work on the top level */
+    v = visitor_input_test_init(data, "{ 'type': 'eins', 'foo': 42 }");
+    visit_type_AliasSimpleUnion(v, NULL, &tmp, &error_abort);
+    g_assert_cmpint(tmp->type, ==, ALIAS_SIMPLE_UNION_KIND_EINS);
+    g_assert_cmpint(tmp->u.eins.data->foo, ==, 42);
+    qapi_free_AliasSimpleUnion(tmp);
+
+    /* It makes the inner alias available, too */
+    v = visitor_input_test_init(data, "{ 'type': 'eins', 'bar': 42 }");
+    visit_type_AliasSimpleUnion(v, NULL, &tmp, &error_abort);
+    g_assert_cmpint(tmp->type, ==, ALIAS_SIMPLE_UNION_KIND_EINS);
+    g_assert_cmpint(tmp->u.eins.data->foo, ==, 42);
+    qapi_free_AliasSimpleUnion(tmp);
+
+    /* You can't use more than one option at the same time for each alias */
+    v = visitor_input_test_init(data, "{ 'type': 'eins', 'tag': 'eins' }");
+    visit_type_AliasSimpleUnion(v, NULL, &tmp, &err);
+    error_free_or_abort(&err);
+
+    v = visitor_input_test_init(data, "{ 'type': 'eins', "
+                                      "  'bar': 123, "
+                                      "  'data': { 'foo': 312 } }");
+    visit_type_AliasSimpleUnion(v, NULL, &tmp, &err);
+    error_free_or_abort(&err);
+}
+
+static void test_visitor_in_alias_nested_wildcard(TestInputVisitorData *data,
+                                                  const void *unused)
+{
+    AliasSimpleUnion *tmp = NULL;
+    Error *err = NULL;
+    Visitor *v;
+
+    /* Can still specify the real member name with alias support */
+    v = visitor_input_test_init(data, "{ 'type': 'drei', 'data': { "
+                                      "    'nested': { 'foo': 42 } } }");
+    visit_type_AliasSimpleUnion(v, NULL, &tmp, &error_abort);
+    g_assert_cmpint(tmp->type, ==, ALIAS_SIMPLE_UNION_KIND_DREI);
+    g_assert_cmpint(tmp->u.drei.data->nested->foo, ==, 42);
+    qapi_free_AliasSimpleUnion(tmp);
+
+    /* The combined wildcard aliases make it work on the top level */
+    v = visitor_input_test_init(data, "{ 'type': 'drei', 'foo': 42 }");
+    visit_type_AliasSimpleUnion(v, NULL, &tmp, &error_abort);
+    g_assert_cmpint(tmp->type, ==, ALIAS_SIMPLE_UNION_KIND_DREI);
+    g_assert_cmpint(tmp->u.drei.data->nested->foo, ==, 42);
+    qapi_free_AliasSimpleUnion(tmp);
+
+    /* Or by having one of the levels explicit */
+    v = visitor_input_test_init(data, "{ 'type': 'drei', "
+                                      "  'data': { 'foo': 42 } }");
+    visit_type_AliasSimpleUnion(v, NULL, &tmp, &error_abort);
+    g_assert_cmpint(tmp->type, ==, ALIAS_SIMPLE_UNION_KIND_DREI);
+    g_assert_cmpint(tmp->u.drei.data->nested->foo, ==, 42);
+    qapi_free_AliasSimpleUnion(tmp);
+
+    v = visitor_input_test_init(data, "{ 'type': 'drei', "
+                                      "  'nested': { 'foo': 42 } }");
+    visit_type_AliasSimpleUnion(v, NULL, &tmp, &error_abort);
+    g_assert_cmpint(tmp->type, ==, ALIAS_SIMPLE_UNION_KIND_DREI);
+    g_assert_cmpint(tmp->u.drei.data->nested->foo, ==, 42);
+    qapi_free_AliasSimpleUnion(tmp);
+
+    /* But you can't provide two values */
+    v = visitor_input_test_init(data, "{ 'type': 'drei', "
+                                      "  'data': { 'nested': { 'foo': 42 } }, "
+                                      "  'foo': 1234 }");
+    visit_type_AliasSimpleUnion(v, NULL, &tmp, &err);
+    error_free_or_abort(&err);
+
+    v = visitor_input_test_init(data, "{ 'type': 'drei', "
+                                      "  'data': { 'foo': 42 }, "
+                                      "  'foo': 1234 }");
+    visit_type_AliasSimpleUnion(v, NULL, &tmp, &err);
+    error_free_or_abort(&err);
+}
+
 static void input_visitor_test_add(const char *testpath,
                                    const void *user_data,
                                    void (*test_func)(TestInputVisitorData *data,
@@ -1350,6 +1609,18 @@ int main(int argc, char **argv)
                            NULL, test_visitor_in_list_union_string);
     input_visitor_test_add("/visitor/input/list_union/number",
                            NULL, test_visitor_in_list_union_number);
+    input_visitor_test_add("/visitor/input/alias/struct-local",
+                           NULL, test_visitor_in_alias_struct_local);
+    input_visitor_test_add("/visitor/input/alias/struct-nested",
+                           NULL, test_visitor_in_alias_struct_nested);
+    input_visitor_test_add("/visitor/input/alias/wildcard",
+                           NULL, test_visitor_in_alias_wildcard);
+    input_visitor_test_add("/visitor/input/alias/flat-union",
+                           NULL, test_visitor_in_alias_flat_union);
+    input_visitor_test_add("/visitor/input/alias/simple-union",
+                           NULL, test_visitor_in_alias_simple_union);
+    input_visitor_test_add("/visitor/input/alias/nested-wildcard",
+                           NULL, test_visitor_in_alias_nested_wildcard);
     input_visitor_test_add("/visitor/input/fail/struct",
                            NULL, test_visitor_in_fail_struct);
     input_visitor_test_add("/visitor/input/fail/struct-nested",
diff --git a/tests/qapi-schema/alias-bad-type.err b/tests/qapi-schema/alias-bad-type.err
new file mode 100644
index 0000000000..820e18ed9c
--- /dev/null
+++ b/tests/qapi-schema/alias-bad-type.err
@@ -0,0 +1,2 @@
+alias-bad-type.json: In struct 'AliasStruct0':
+alias-bad-type.json:1: 'aliases' members must be objects
diff --git a/tests/qapi-schema/alias-bad-type.json b/tests/qapi-schema/alias-bad-type.json
new file mode 100644
index 0000000000..0aa5d206fe
--- /dev/null
+++ b/tests/qapi-schema/alias-bad-type.json
@@ -0,0 +1,3 @@
+{ 'struct': 'AliasStruct0',
+  'data': { 'foo': 'int' },
+  'aliases': [ 'must be an object' ] }
diff --git a/tests/qapi-schema/alias-bad-type.out b/tests/qapi-schema/alias-bad-type.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/alias-missing-source.err b/tests/qapi-schema/alias-missing-source.err
new file mode 100644
index 0000000000..8b7d601fbf
--- /dev/null
+++ b/tests/qapi-schema/alias-missing-source.err
@@ -0,0 +1,2 @@
+alias-missing-source.json: In struct 'AliasStruct0':
+alias-missing-source.json:1: 'aliases' member misses key 'source'
diff --git a/tests/qapi-schema/alias-missing-source.json b/tests/qapi-schema/alias-missing-source.json
new file mode 100644
index 0000000000..b6c91a9488
--- /dev/null
+++ b/tests/qapi-schema/alias-missing-source.json
@@ -0,0 +1,3 @@
+{ 'struct': 'AliasStruct0',
+  'data': { 'foo': 'int' },
+  'aliases': [ { 'name': 'bar' } ] }
diff --git a/tests/qapi-schema/alias-missing-source.out b/tests/qapi-schema/alias-missing-source.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/alias-name-bad-type.err b/tests/qapi-schema/alias-name-bad-type.err
new file mode 100644
index 0000000000..489f45ff9b
--- /dev/null
+++ b/tests/qapi-schema/alias-name-bad-type.err
@@ -0,0 +1,2 @@
+alias-name-bad-type.json: In struct 'AliasStruct0':
+alias-name-bad-type.json:1: alias member 'name' requires a string name
diff --git a/tests/qapi-schema/alias-name-bad-type.json b/tests/qapi-schema/alias-name-bad-type.json
new file mode 100644
index 0000000000..17442d5939
--- /dev/null
+++ b/tests/qapi-schema/alias-name-bad-type.json
@@ -0,0 +1,3 @@
+{ 'struct': 'AliasStruct0',
+  'data': { 'foo': 'int' },
+  'aliases': [ { 'name': ['bar'], 'source': ['foo'] } ] }
diff --git a/tests/qapi-schema/alias-name-bad-type.out b/tests/qapi-schema/alias-name-bad-type.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/alias-name-conflict.err b/tests/qapi-schema/alias-name-conflict.err
new file mode 100644
index 0000000000..d5825a0285
--- /dev/null
+++ b/tests/qapi-schema/alias-name-conflict.err
@@ -0,0 +1,2 @@
+alias-name-conflict.json: In struct 'AliasStruct0':
+alias-name-conflict.json:1: alias 'bar' collides with member 'bar'
diff --git a/tests/qapi-schema/alias-name-conflict.json b/tests/qapi-schema/alias-name-conflict.json
new file mode 100644
index 0000000000..bdb5bd4eab
--- /dev/null
+++ b/tests/qapi-schema/alias-name-conflict.json
@@ -0,0 +1,4 @@
+{ 'struct': 'AliasStruct0',
+  'data': { 'foo': 'int',
+            'bar': 'int' },
+  'aliases': [ { 'name': 'bar', 'source': ['foo'] } ] }
diff --git a/tests/qapi-schema/alias-name-conflict.out b/tests/qapi-schema/alias-name-conflict.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/alias-recursive.err b/tests/qapi-schema/alias-recursive.err
new file mode 100644
index 0000000000..127ce019a8
--- /dev/null
+++ b/tests/qapi-schema/alias-recursive.err
@@ -0,0 +1,2 @@
+alias-recursive.json: In struct 'AliasStruct0':
+alias-recursive.json:1: alias 'baz' resolving to 'bar' makes 'bar' an alias for itself
diff --git a/tests/qapi-schema/alias-recursive.json b/tests/qapi-schema/alias-recursive.json
new file mode 100644
index 0000000000..e25b935324
--- /dev/null
+++ b/tests/qapi-schema/alias-recursive.json
@@ -0,0 +1,4 @@
+{ 'struct': 'AliasStruct0',
+  'data': { 'foo': 'int' },
+  'aliases': [ { 'name': 'bar', 'source': ['baz'] },
+               { 'name': 'baz', 'source': ['bar'] } ] }
diff --git a/tests/qapi-schema/alias-recursive.out b/tests/qapi-schema/alias-recursive.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/alias-source-bad-type.err b/tests/qapi-schema/alias-source-bad-type.err
new file mode 100644
index 0000000000..ed4b903b41
--- /dev/null
+++ b/tests/qapi-schema/alias-source-bad-type.err
@@ -0,0 +1,2 @@
+alias-source-bad-type.json: In struct 'AliasStruct0':
+alias-source-bad-type.json:1: alias 'bar' member 'source' must be an array
diff --git a/tests/qapi-schema/alias-source-bad-type.json b/tests/qapi-schema/alias-source-bad-type.json
new file mode 100644
index 0000000000..d6a7430ee3
--- /dev/null
+++ b/tests/qapi-schema/alias-source-bad-type.json
@@ -0,0 +1,3 @@
+{ 'struct': 'AliasStruct0',
+  'data': { 'foo': 'int' },
+  'aliases': [ { 'name': 'bar', 'source': 'foo' } ] }
diff --git a/tests/qapi-schema/alias-source-bad-type.out b/tests/qapi-schema/alias-source-bad-type.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/alias-source-elem-bad-type.err b/tests/qapi-schema/alias-source-elem-bad-type.err
new file mode 100644
index 0000000000..f7a1ee2cb7
--- /dev/null
+++ b/tests/qapi-schema/alias-source-elem-bad-type.err
@@ -0,0 +1,2 @@
+alias-source-elem-bad-type.json: In struct 'AliasStruct0':
+alias-source-elem-bad-type.json:1: element in alias 'bar' member 'source' requires a string name
diff --git a/tests/qapi-schema/alias-source-elem-bad-type.json b/tests/qapi-schema/alias-source-elem-bad-type.json
new file mode 100644
index 0000000000..1d08f56492
--- /dev/null
+++ b/tests/qapi-schema/alias-source-elem-bad-type.json
@@ -0,0 +1,3 @@
+{ 'struct': 'AliasStruct0',
+  'data': { 'foo': 'int' },
+  'aliases': [ { 'name': 'bar', 'source': ['foo', true] } ] }
diff --git a/tests/qapi-schema/alias-source-elem-bad-type.out b/tests/qapi-schema/alias-source-elem-bad-type.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/alias-source-empty.err b/tests/qapi-schema/alias-source-empty.err
new file mode 100644
index 0000000000..368b7ffa96
--- /dev/null
+++ b/tests/qapi-schema/alias-source-empty.err
@@ -0,0 +1,2 @@
+alias-source-empty.json: In struct 'AliasStruct0':
+alias-source-empty.json:1: alias 'bar' member 'source' must not be empty
diff --git a/tests/qapi-schema/alias-source-empty.json b/tests/qapi-schema/alias-source-empty.json
new file mode 100644
index 0000000000..74b529de4a
--- /dev/null
+++ b/tests/qapi-schema/alias-source-empty.json
@@ -0,0 +1,3 @@
+{ 'struct': 'AliasStruct0',
+  'data': { 'foo': 'int' },
+  'aliases': [ { 'name': 'bar', 'source': [] } ] }
diff --git a/tests/qapi-schema/alias-source-empty.out b/tests/qapi-schema/alias-source-empty.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/alias-source-inexistent-variants.err b/tests/qapi-schema/alias-source-inexistent-variants.err
new file mode 100644
index 0000000000..a5d4a4c334
--- /dev/null
+++ b/tests/qapi-schema/alias-source-inexistent-variants.err
@@ -0,0 +1,2 @@
+alias-source-inexistent-variants.json: In union 'AliasStruct0':
+alias-source-inexistent-variants.json:7: alias 'test' has a source path that does not exist in any variant of union type 'AliasStruct0'
diff --git a/tests/qapi-schema/alias-source-inexistent-variants.json b/tests/qapi-schema/alias-source-inexistent-variants.json
new file mode 100644
index 0000000000..6328095b86
--- /dev/null
+++ b/tests/qapi-schema/alias-source-inexistent-variants.json
@@ -0,0 +1,12 @@
+{ 'enum': 'Variants',
+  'data': [ 'a', 'b' ] }
+{ 'struct': 'Variant0',
+  'data': { 'foo': 'int' } }
+{ 'struct': 'Variant1',
+  'data': { 'bar': 'int' } }
+{ 'union': 'AliasStruct0',
+  'base': { 'type': 'Variants' },
+  'discriminator': 'type',
+  'data': { 'a': 'Variant0',
+            'b': 'Variant1' },
+  'aliases': [ { 'name': 'test', 'source': ['baz'] } ] }
diff --git a/tests/qapi-schema/alias-source-inexistent-variants.out b/tests/qapi-schema/alias-source-inexistent-variants.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/alias-source-inexistent.err b/tests/qapi-schema/alias-source-inexistent.err
new file mode 100644
index 0000000000..2d65d3f588
--- /dev/null
+++ b/tests/qapi-schema/alias-source-inexistent.err
@@ -0,0 +1,2 @@
+alias-source-inexistent.json: In struct 'AliasStruct0':
+alias-source-inexistent.json:1: alias 'bar' has inexistent source
diff --git a/tests/qapi-schema/alias-source-inexistent.json b/tests/qapi-schema/alias-source-inexistent.json
new file mode 100644
index 0000000000..5106d3609f
--- /dev/null
+++ b/tests/qapi-schema/alias-source-inexistent.json
@@ -0,0 +1,3 @@
+{ 'struct': 'AliasStruct0',
+  'data': { 'foo': 'int' },
+  'aliases': [ { 'name': 'bar', 'source': ['baz'] } ] }
diff --git a/tests/qapi-schema/alias-source-inexistent.out b/tests/qapi-schema/alias-source-inexistent.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/alias-source-non-object-path.err b/tests/qapi-schema/alias-source-non-object-path.err
new file mode 100644
index 0000000000..b3c748350f
--- /dev/null
+++ b/tests/qapi-schema/alias-source-non-object-path.err
@@ -0,0 +1,2 @@
+alias-source-non-object-path.json: In struct 'AliasStruct0':
+alias-source-non-object-path.json:1: alias 'bar' has non-object 'foo' in its source path
diff --git a/tests/qapi-schema/alias-source-non-object-path.json b/tests/qapi-schema/alias-source-non-object-path.json
new file mode 100644
index 0000000000..808a3e6281
--- /dev/null
+++ b/tests/qapi-schema/alias-source-non-object-path.json
@@ -0,0 +1,3 @@
+{ 'struct': 'AliasStruct0',
+  'data': { 'foo': 'int' },
+  'aliases': [ { 'name': 'bar', 'source': ['foo', 'baz'] } ] }
diff --git a/tests/qapi-schema/alias-source-non-object-path.out b/tests/qapi-schema/alias-source-non-object-path.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/alias-source-non-object-wildcard.err b/tests/qapi-schema/alias-source-non-object-wildcard.err
new file mode 100644
index 0000000000..4adc0d2281
--- /dev/null
+++ b/tests/qapi-schema/alias-source-non-object-wildcard.err
@@ -0,0 +1,2 @@
+alias-source-non-object-wildcard.json: In struct 'AliasStruct0':
+alias-source-non-object-wildcard.json:1: wildcard alias has non-object 'foo' in its source path
diff --git a/tests/qapi-schema/alias-source-non-object-wildcard.json b/tests/qapi-schema/alias-source-non-object-wildcard.json
new file mode 100644
index 0000000000..59ce1081ef
--- /dev/null
+++ b/tests/qapi-schema/alias-source-non-object-wildcard.json
@@ -0,0 +1,3 @@
+{ 'struct': 'AliasStruct0',
+  'data': { 'foo': 'int' },
+  'aliases': [ { 'source': ['foo'] } ] }
diff --git a/tests/qapi-schema/alias-source-non-object-wildcard.out b/tests/qapi-schema/alias-source-non-object-wildcard.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/alias-source-optional-wildcard-indirect.err b/tests/qapi-schema/alias-source-optional-wildcard-indirect.err
new file mode 100644
index 0000000000..b58b8ff00f
--- /dev/null
+++ b/tests/qapi-schema/alias-source-optional-wildcard-indirect.err
@@ -0,0 +1,2 @@
+alias-source-optional-wildcard-indirect.json: In struct 'AliasStruct0':
+alias-source-optional-wildcard-indirect.json:3: wildcard alias has optional object member 'nested' in its source path
diff --git a/tests/qapi-schema/alias-source-optional-wildcard-indirect.json b/tests/qapi-schema/alias-source-optional-wildcard-indirect.json
new file mode 100644
index 0000000000..fcf04969dc
--- /dev/null
+++ b/tests/qapi-schema/alias-source-optional-wildcard-indirect.json
@@ -0,0 +1,6 @@
+{ 'struct': 'Nested',
+  'data': { 'foo': 'int' } }
+{ 'struct': 'AliasStruct0',
+  'data': { '*nested': 'Nested' },
+  'aliases': [ { 'name': 'nested-alias', 'source': ['nested'] },
+               { 'source': ['nested-alias'] } ] }
diff --git a/tests/qapi-schema/alias-source-optional-wildcard-indirect.out b/tests/qapi-schema/alias-source-optional-wildcard-indirect.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/alias-source-optional-wildcard.err b/tests/qapi-schema/alias-source-optional-wildcard.err
new file mode 100644
index 0000000000..e39200bd3d
--- /dev/null
+++ b/tests/qapi-schema/alias-source-optional-wildcard.err
@@ -0,0 +1,2 @@
+alias-source-optional-wildcard.json: In struct 'AliasStruct0':
+alias-source-optional-wildcard.json:3: wildcard alias has optional object member 'nested' in its source path
diff --git a/tests/qapi-schema/alias-source-optional-wildcard.json b/tests/qapi-schema/alias-source-optional-wildcard.json
new file mode 100644
index 0000000000..1a315f2ae0
--- /dev/null
+++ b/tests/qapi-schema/alias-source-optional-wildcard.json
@@ -0,0 +1,5 @@
+{ 'struct': 'Nested',
+  'data': { 'foo': 'int' } }
+{ 'struct': 'AliasStruct0',
+  'data': { '*nested': 'Nested' },
+  'aliases': [ { 'source': ['nested'] } ] }
diff --git a/tests/qapi-schema/alias-source-optional-wildcard.out b/tests/qapi-schema/alias-source-optional-wildcard.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/alias-unknown-key.err b/tests/qapi-schema/alias-unknown-key.err
new file mode 100644
index 0000000000..c7b8cb9498
--- /dev/null
+++ b/tests/qapi-schema/alias-unknown-key.err
@@ -0,0 +1,3 @@
+alias-unknown-key.json: In struct 'AliasStruct0':
+alias-unknown-key.json:1: 'aliases' member has unknown key 'known'
+Valid keys are 'name', 'source'.
diff --git a/tests/qapi-schema/alias-unknown-key.json b/tests/qapi-schema/alias-unknown-key.json
new file mode 100644
index 0000000000..cdb8fc3d07
--- /dev/null
+++ b/tests/qapi-schema/alias-unknown-key.json
@@ -0,0 +1,3 @@
+{ 'struct': 'AliasStruct0',
+  'data': { 'foo': 'int' },
+  'aliases': [ { 'name': 'bar', 'source': ['foo'], 'known': false } ] }
diff --git a/tests/qapi-schema/alias-unknown-key.out b/tests/qapi-schema/alias-unknown-key.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/aliases-bad-type.err b/tests/qapi-schema/aliases-bad-type.err
new file mode 100644
index 0000000000..7ffe789ec0
--- /dev/null
+++ b/tests/qapi-schema/aliases-bad-type.err
@@ -0,0 +1,2 @@
+aliases-bad-type.json: In struct 'AliasStruct0':
+aliases-bad-type.json:1: 'aliases' must be an array
diff --git a/tests/qapi-schema/aliases-bad-type.json b/tests/qapi-schema/aliases-bad-type.json
new file mode 100644
index 0000000000..4bbf6d6b20
--- /dev/null
+++ b/tests/qapi-schema/aliases-bad-type.json
@@ -0,0 +1,3 @@
+{ 'struct': 'AliasStruct0',
+  'data': { 'foo': 'int' },
+  'aliases': 'this must be an array' }
diff --git a/tests/qapi-schema/aliases-bad-type.out b/tests/qapi-schema/aliases-bad-type.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
index 6b2a4ce41a..ed0166fa0b 100644
--- a/tests/qapi-schema/meson.build
+++ b/tests/qapi-schema/meson.build
@@ -3,6 +3,22 @@ test_env.set('PYTHONPATH', meson.source_root() / 'scripts')
 test_env.set('PYTHONIOENCODING', 'utf-8')
 
 schemas = [
+  'alias-bad-type.json',
+  'aliases-bad-type.json',
+  'alias-missing-source.json',
+  'alias-name-bad-type.json',
+  'alias-name-conflict.json',
+  'alias-recursive.json',
+  'alias-source-bad-type.json',
+  'alias-source-elem-bad-type.json',
+  'alias-source-empty.json',
+  'alias-source-inexistent.json',
+  'alias-source-inexistent-variants.json',
+  'alias-source-non-object-path.json',
+  'alias-source-non-object-wildcard.json',
+  'alias-source-optional-wildcard.json',
+  'alias-source-optional-wildcard-indirect.json',
+  'alias-unknown-key.json',
   'alternate-any.json',
   'alternate-array.json',
   'alternate-base.json',
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index b6c36a9eee..349f88bea3 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -348,3 +348,38 @@
 
 { 'event': 'TEST_EVENT_FEATURES1',
   'features': [ 'deprecated' ] }
+
+# test  'aliases'
+
+{ 'struct': 'AliasStruct0',
+  'data': { 'foo': 'int' },
+  'aliases': [] }
+{ 'struct': 'AliasStruct1',
+  'data': { 'foo': 'int' },
+  'aliases': [ { 'name': 'bar', 'source': ['foo'] } ] }
+{ 'struct': 'AliasStruct2',
+  'data': { 'nested': 'AliasStruct1' },
+  'aliases': [ { 'name': 'bar', 'source': ['nested', 'foo'] } ] }
+{ 'struct': 'AliasStruct3',
+  'data': { 'nested': 'AliasStruct1' },
+  'aliases': [ { 'source': ['nested'] } ] }
+
+{ 'union': 'AliasFlatUnion',
+  'base': { 'tag': 'FeatureEnum1' },
+  'discriminator': 'tag',
+  'data': { 'eins': 'FeatureStruct1' },
+  'aliases': [ { 'name': 'variant', 'source': ['tag'] },
+               { 'name': 'bar', 'source': ['foo'] } ] }
+{ 'union': 'AliasSimpleUnion',
+  'data': { 'eins': 'AliasStruct1',
+            'drei': 'AliasStruct3' },
+  'aliases': [ { 'source': ['data'] },
+               { 'name': 'tag', 'source': ['type'] } ] }
+
+{ 'command': 'test-aliases0',
+  'data': { '*as0': 'AliasStruct0',
+            '*as1': 'AliasStruct1',
+            '*as2': 'AliasStruct2',
+            '*as3': 'AliasStruct3',
+            '*afu': 'AliasFlatUnion',
+            '*asu': 'AliasSimpleUnion' } }
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index d557fe2d89..87374e11ee 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -453,6 +453,50 @@ event TEST_EVENT_FEATURES0 FeatureStruct1
 event TEST_EVENT_FEATURES1 None
     boxed=False
     feature deprecated
+object AliasStruct0
+    member foo: int optional=False
+object AliasStruct1
+    member foo: int optional=False
+    alias bar -> foo
+object AliasStruct2
+    member nested: AliasStruct1 optional=False
+    alias bar -> nested.foo
+object AliasStruct3
+    member nested: AliasStruct1 optional=False
+    alias * -> nested.*
+object q_obj_AliasFlatUnion-base
+    member tag: FeatureEnum1 optional=False
+object AliasFlatUnion
+    base q_obj_AliasFlatUnion-base
+    alias variant -> tag
+    alias bar -> foo
+    tag tag
+    case eins: FeatureStruct1
+    case zwei: q_empty
+    case drei: q_empty
+object q_obj_AliasStruct1-wrapper
+    member data: AliasStruct1 optional=False
+object q_obj_AliasStruct3-wrapper
+    member data: AliasStruct3 optional=False
+enum AliasSimpleUnionKind
+    member eins
+    member drei
+object AliasSimpleUnion
+    member type: AliasSimpleUnionKind optional=False
+    alias * -> data.*
+    alias tag -> type
+    tag type
+    case eins: q_obj_AliasStruct1-wrapper
+    case drei: q_obj_AliasStruct3-wrapper
+object q_obj_test-aliases0-arg
+    member as0: AliasStruct0 optional=True
+    member as1: AliasStruct1 optional=True
+    member as2: AliasStruct2 optional=True
+    member as3: AliasStruct3 optional=True
+    member afu: AliasFlatUnion optional=True
+    member asu: AliasSimpleUnion optional=True
+command test-aliases0 q_obj_test-aliases0-arg -> None
+    gen=True success_response=True boxed=False oob=False preconfig=False
 module include/sub-module.json
 include sub-sub-module.json
 object SecondArrayRef
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 0/8] qapi: Add support for aliases
  2021-09-17 16:13 [PATCH v4 0/8] qapi: Add support for aliases Kevin Wolf
                   ` (7 preceding siblings ...)
  2021-09-17 16:13 ` [PATCH v4 8/8] tests/qapi-schema: Test cases " Kevin Wolf
@ 2021-10-01  9:28 ` Kevin Wolf
  2021-10-26 21:23 ` John Snow
  9 siblings, 0 replies; 13+ messages in thread
From: Kevin Wolf @ 2021-10-01  9:28 UTC (permalink / raw)
  To: qemu-devel; +Cc: jsnow, armbru

Am 17.09.2021 um 18:13 hat Kevin Wolf geschrieben:
> This series introduces alias definitions for QAPI object types (structs
> and unions).
> 
> This allows using the same QAPI type and visitor even when the syntax
> has some variations between different external interfaces such as QMP
> and the command line.
> 
> It also provides a new tool for evolving the schema while maintaining
> backwards compatibility (possibly during a deprecation period).
> 
> The first user is intended to be a QAPIfied -chardev command line
> option, for which I'll send a separate series. A git tag is available
> that contains both this series and the chardev changes that make use of
> it:
> 
>     https://repo.or.cz/qemu/kevin.git qapi-alias-chardev-v4
> 
> v4:
> - Make sure to keep a defined order of aliases in StackObject.aliases
> - Added patch 4 to allow for better error messages when wildcard aliases
>   provide a second value for a member, which may or may not be consumed
>   elsewhere.
> - Resolve chained aliases only once instead of just checking that they
>   can be resolved while looking for matching aliases, and actually
>   resolving them at the end. This is not only a code simplification, but
>   actually necessary for correct error messages on conflicts.
> - Separate schema.py cleanup patch by Markus ('qapi: Revert an
>   accidental change from list to view object')
> - Fixed alias name checks in the QAPI generator
> - Changed check_path() to avoid modifying its 'path' parameter
> - Some more test cases
> - Coding style fixes
> - Documentation improvements

ping



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 0/8] qapi: Add support for aliases
  2021-09-17 16:13 [PATCH v4 0/8] qapi: Add support for aliases Kevin Wolf
                   ` (8 preceding siblings ...)
  2021-10-01  9:28 ` [PATCH v4 0/8] qapi: Add support " Kevin Wolf
@ 2021-10-26 21:23 ` John Snow
  2021-10-27 10:31   ` Kevin Wolf
  9 siblings, 1 reply; 13+ messages in thread
From: John Snow @ 2021-10-26 21:23 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: qemu-devel, Markus Armbruster

[-- Attachment #1: Type: text/plain, Size: 11380 bytes --]

On Fri, Sep 17, 2021 at 12:13 PM Kevin Wolf <kwolf@redhat.com> wrote:

> This series introduces alias definitions for QAPI object types (structs
> and unions).
>
> This allows using the same QAPI type and visitor even when the syntax
> has some variations between different external interfaces such as QMP
> and the command line.
>
>
I'm absurdly late to the party here, and I'm afraid my involvement may only
stall your progress even further, but ... can you fill me in on the
slightly-less-higher-level details here?

I'm curious as to the nature of "has some variations" and how the aliases
help alleviate them. Do you accomplish that compatibility by using
different names for different fields of the struct?

so e.g. x.foo can be used as an alias for x.bar, but both map ultimately
onto 'x.foo'? Or does this series provide for some more fundamental
mechanical changes to map one type onto another?


> It also provides a new tool for evolving the schema while maintaining
> backwards compatibility (possibly during a deprecation period).
>
> The first user is intended to be a QAPIfied -chardev command line
> option, for which I'll send a separate series. A git tag is available
> that contains both this series and the chardev changes that make use of
> it:
>
>     https://repo.or.cz/qemu/kevin.git qapi-alias-chardev-v4
>
>
v4:
> - Make sure to keep a defined order of aliases in StackObject.aliases
> - Added patch 4 to allow for better error messages when wildcard aliases
>   provide a second value for a member, which may or may not be consumed
>   elsewhere.
> - Resolve chained aliases only once instead of just checking that they
>   can be resolved while looking for matching aliases, and actually
>   resolving them at the end. This is not only a code simplification, but
>   actually necessary for correct error messages on conflicts.
> - Separate schema.py cleanup patch by Markus ('qapi: Revert an
>   accidental change from list to view object')
> - Fixed alias name checks in the QAPI generator
> - Changed check_path() to avoid modifying its 'path' parameter
> - Some more test cases
> - Coding style fixes
> - Documentation improvements
>
> v3:
> - Mention the new functions in the big comment in visitor.h. However,
>   since the comment is about users of the visitor rather than the
>   generated code, it seems like to wrong place to go into details.
> - Updated commit message for patch 3 ('Simplify full_name_nth() ...')
> - Patch 4 ('qapi: Apply aliases in qobject-input-visitor'):
>     - Multiple matching wildcard aliases are considered conflicting now
>     - Improved comments for several functions
>     - Renamed bool *implicit_object into *is_alias_prefix, which
>       describes better what it is rather than what it is used for
>     - Simplified alias_present() into input_present()
>     - Fixed potential use of wrong StackObject in error message
> - Patch 5 ('qapi: Add support for aliases'):
>     - Made QAPISchemaAlias a QAPISchemaMember
>     - Check validity of alias source paths (must exist in at least one
>       variant, no optional objects in the path of a wildcard alias, no
>       alias loops)
> - Many new tests cases, both positive and negative, including unit tests
>   of the generated visit functions
> - Coding style changes
> - Rebased documentation (.txt -> .rst conversion in master)
>
> v2:
> - Renamed 'alias' to 'name' in all data structures describing aliases
> - Tons of new or changed comments and other documentation
> - Be more explicit that empty 'source' is invalid and assert it
> - Fixed full_name_so() for lists (added a parameter to tell the function
>   whether the name of a list member or the list itself is meant)
> - Changed some QAPI generator error messages
> - Assert the type of parameters in QAPISchemaAlias.__init__()
>
> Kevin Wolf (7):
>   qapi: Add interfaces for alias support to Visitor
>   qapi: Remember alias definitions in qobject-input-visitor
>   qapi: Simplify full_name_nth() in qobject-input-visitor
>   qapi: Store Error in StackObject.h for qobject-input-visitor
>   qapi: Apply aliases in qobject-input-visitor
>   qapi: Add support for aliases
>   tests/qapi-schema: Test cases for aliases
>
> Markus Armbruster (1):
>   qapi: Revert an accidental change from list to view object
>
>  docs/devel/qapi-code-gen.rst                  | 109 ++++-
>  docs/sphinx/qapidoc.py                        |   2 +-
>  include/qapi/visitor-impl.h                   |  12 +
>  include/qapi/visitor.h                        |  59 ++-
>  qapi/qapi-visit-core.c                        |  22 +
>  qapi/qobject-input-visitor.c                  | 452 ++++++++++++++++--
>  tests/unit/test-qmp-cmds.c                    |  10 +
>  tests/unit/test-qobject-input-visitor.c       | 271 +++++++++++
>  scripts/qapi/expr.py                          |  54 ++-
>  scripts/qapi/schema.py                        | 117 ++++-
>  scripts/qapi/types.py                         |   4 +-
>  scripts/qapi/visit.py                         |  34 +-
>  tests/qapi-schema/test-qapi.py                |   7 +-
>  tests/qapi-schema/alias-bad-type.err          |   2 +
>  tests/qapi-schema/alias-bad-type.json         |   3 +
>  tests/qapi-schema/alias-bad-type.out          |   0
>  tests/qapi-schema/alias-missing-source.err    |   2 +
>  tests/qapi-schema/alias-missing-source.json   |   3 +
>  tests/qapi-schema/alias-missing-source.out    |   0
>  tests/qapi-schema/alias-name-bad-type.err     |   2 +
>  tests/qapi-schema/alias-name-bad-type.json    |   3 +
>  tests/qapi-schema/alias-name-bad-type.out     |   0
>  tests/qapi-schema/alias-name-conflict.err     |   2 +
>  tests/qapi-schema/alias-name-conflict.json    |   4 +
>  tests/qapi-schema/alias-name-conflict.out     |   0
>  tests/qapi-schema/alias-recursive.err         |   2 +
>  tests/qapi-schema/alias-recursive.json        |   4 +
>  tests/qapi-schema/alias-recursive.out         |   0
>  tests/qapi-schema/alias-source-bad-type.err   |   2 +
>  tests/qapi-schema/alias-source-bad-type.json  |   3 +
>  tests/qapi-schema/alias-source-bad-type.out   |   0
>  .../alias-source-elem-bad-type.err            |   2 +
>  .../alias-source-elem-bad-type.json           |   3 +
>  .../alias-source-elem-bad-type.out            |   0
>  tests/qapi-schema/alias-source-empty.err      |   2 +
>  tests/qapi-schema/alias-source-empty.json     |   3 +
>  tests/qapi-schema/alias-source-empty.out      |   0
>  .../alias-source-inexistent-variants.err      |   2 +
>  .../alias-source-inexistent-variants.json     |  12 +
>  .../alias-source-inexistent-variants.out      |   0
>  tests/qapi-schema/alias-source-inexistent.err |   2 +
>  .../qapi-schema/alias-source-inexistent.json  |   3 +
>  tests/qapi-schema/alias-source-inexistent.out |   0
>  .../alias-source-non-object-path.err          |   2 +
>  .../alias-source-non-object-path.json         |   3 +
>  .../alias-source-non-object-path.out          |   0
>  .../alias-source-non-object-wildcard.err      |   2 +
>  .../alias-source-non-object-wildcard.json     |   3 +
>  .../alias-source-non-object-wildcard.out      |   0
>  ...lias-source-optional-wildcard-indirect.err |   2 +
>  ...ias-source-optional-wildcard-indirect.json |   6 +
>  ...lias-source-optional-wildcard-indirect.out |   0
>  .../alias-source-optional-wildcard.err        |   2 +
>  .../alias-source-optional-wildcard.json       |   5 +
>  .../alias-source-optional-wildcard.out        |   0
>  tests/qapi-schema/alias-unknown-key.err       |   3 +
>  tests/qapi-schema/alias-unknown-key.json      |   3 +
>  tests/qapi-schema/alias-unknown-key.out       |   0
>  tests/qapi-schema/aliases-bad-type.err        |   2 +
>  tests/qapi-schema/aliases-bad-type.json       |   3 +
>  tests/qapi-schema/aliases-bad-type.out        |   0
>  tests/qapi-schema/meson.build                 |  16 +
>  tests/qapi-schema/qapi-schema-test.json       |  35 ++
>  tests/qapi-schema/qapi-schema-test.out        |  44 ++
>  tests/qapi-schema/unknown-expr-key.err        |   2 +-
>  65 files changed, 1290 insertions(+), 57 deletions(-)
>  create mode 100644 tests/qapi-schema/alias-bad-type.err
>  create mode 100644 tests/qapi-schema/alias-bad-type.json
>  create mode 100644 tests/qapi-schema/alias-bad-type.out
>  create mode 100644 tests/qapi-schema/alias-missing-source.err
>  create mode 100644 tests/qapi-schema/alias-missing-source.json
>  create mode 100644 tests/qapi-schema/alias-missing-source.out
>  create mode 100644 tests/qapi-schema/alias-name-bad-type.err
>  create mode 100644 tests/qapi-schema/alias-name-bad-type.json
>  create mode 100644 tests/qapi-schema/alias-name-bad-type.out
>  create mode 100644 tests/qapi-schema/alias-name-conflict.err
>  create mode 100644 tests/qapi-schema/alias-name-conflict.json
>  create mode 100644 tests/qapi-schema/alias-name-conflict.out
>  create mode 100644 tests/qapi-schema/alias-recursive.err
>  create mode 100644 tests/qapi-schema/alias-recursive.json
>  create mode 100644 tests/qapi-schema/alias-recursive.out
>  create mode 100644 tests/qapi-schema/alias-source-bad-type.err
>  create mode 100644 tests/qapi-schema/alias-source-bad-type.json
>  create mode 100644 tests/qapi-schema/alias-source-bad-type.out
>  create mode 100644 tests/qapi-schema/alias-source-elem-bad-type.err
>  create mode 100644 tests/qapi-schema/alias-source-elem-bad-type.json
>  create mode 100644 tests/qapi-schema/alias-source-elem-bad-type.out
>  create mode 100644 tests/qapi-schema/alias-source-empty.err
>  create mode 100644 tests/qapi-schema/alias-source-empty.json
>  create mode 100644 tests/qapi-schema/alias-source-empty.out
>  create mode 100644 tests/qapi-schema/alias-source-inexistent-variants.err
>  create mode 100644 tests/qapi-schema/alias-source-inexistent-variants.json
>  create mode 100644 tests/qapi-schema/alias-source-inexistent-variants.out
>  create mode 100644 tests/qapi-schema/alias-source-inexistent.err
>  create mode 100644 tests/qapi-schema/alias-source-inexistent.json
>  create mode 100644 tests/qapi-schema/alias-source-inexistent.out
>  create mode 100644 tests/qapi-schema/alias-source-non-object-path.err
>  create mode 100644 tests/qapi-schema/alias-source-non-object-path.json
>  create mode 100644 tests/qapi-schema/alias-source-non-object-path.out
>  create mode 100644 tests/qapi-schema/alias-source-non-object-wildcard.err
>  create mode 100644 tests/qapi-schema/alias-source-non-object-wildcard.json
>  create mode 100644 tests/qapi-schema/alias-source-non-object-wildcard.out
>  create mode 100644
> tests/qapi-schema/alias-source-optional-wildcard-indirect.err
>  create mode 100644
> tests/qapi-schema/alias-source-optional-wildcard-indirect.json
>  create mode 100644
> tests/qapi-schema/alias-source-optional-wildcard-indirect.out
>  create mode 100644 tests/qapi-schema/alias-source-optional-wildcard.err
>  create mode 100644 tests/qapi-schema/alias-source-optional-wildcard.json
>  create mode 100644 tests/qapi-schema/alias-source-optional-wildcard.out
>  create mode 100644 tests/qapi-schema/alias-unknown-key.err
>  create mode 100644 tests/qapi-schema/alias-unknown-key.json
>  create mode 100644 tests/qapi-schema/alias-unknown-key.out
>  create mode 100644 tests/qapi-schema/aliases-bad-type.err
>  create mode 100644 tests/qapi-schema/aliases-bad-type.json
>  create mode 100644 tests/qapi-schema/aliases-bad-type.out
>
> --
> 2.31.1
>
>

[-- Attachment #2: Type: text/html, Size: 13414 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 0/8] qapi: Add support for aliases
  2021-10-26 21:23 ` John Snow
@ 2021-10-27 10:31   ` Kevin Wolf
  2021-10-27 15:34     ` John Snow
  0 siblings, 1 reply; 13+ messages in thread
From: Kevin Wolf @ 2021-10-27 10:31 UTC (permalink / raw)
  To: John Snow; +Cc: qemu-devel, Markus Armbruster

Am 26.10.2021 um 23:23 hat John Snow geschrieben:
> On Fri, Sep 17, 2021 at 12:13 PM Kevin Wolf <kwolf@redhat.com> wrote:
> 
> > This series introduces alias definitions for QAPI object types (structs
> > and unions).
> >
> > This allows using the same QAPI type and visitor even when the syntax
> > has some variations between different external interfaces such as QMP
> > and the command line.
> >
> >
> I'm absurdly late to the party here, and I'm afraid my involvement may only
> stall your progress even further, but ...

I'm not sure if it's worth investing much of your time in this. After a
year of discussing implementation details, Markus decided that he
doesn't like the whole approach, so the series is probably dead in this
shape. Maybe parts of it (possibly even large parts) can be saved and
used in a modified approach, depending on how radically different the
new approach is.

Markus promised that he'll think of alternative approaches to solve the
problem. I'm waiting for that before I waste more time implementing
something else that is also dead from the start.

> can you fill me in on the slightly-less-higher-level details here?
> 
> I'm curious as to the nature of "has some variations" and how the aliases
> help alleviate them. Do you accomplish that compatibility by using
> different names for different fields of the struct?

> so e.g. x.foo can be used as an alias for x.bar, but both map ultimately
> onto 'x.foo'? Or does this series provide for some more fundamental
> mechanical changes to map one type onto another?

I would recommend reading the changes to docs/devel/qapi-code-gen.rst in
patch 7, which explain the implemented mechanism.

For -chardev, most of "some varations" are different names for the same
member of a struct, or nesting in QMP that you don't want to have on the
command line. I went through all cases in one of the last messages in
the v3 thread, but I think these are the two important categories of
cases.

The C types stay the same as they have always been, aliases just provide
an alternative way to specify the same thing in the input.

> > It also provides a new tool for evolving the schema while maintaining
> > backwards compatibility (possibly during a deprecation period).
> >
> > The first user is intended to be a QAPIfied -chardev command line
> > option, for which I'll send a separate series. A git tag is available
> > that contains both this series and the chardev changes that make use of
> > it:
> >
> >     https://repo.or.cz/qemu/kevin.git qapi-alias-chardev-v4

You may also want to have a look at this, and specifically
qapi/char.json, to see how I used aliases in practice.

Kevin



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 0/8] qapi: Add support for aliases
  2021-10-27 10:31   ` Kevin Wolf
@ 2021-10-27 15:34     ` John Snow
  0 siblings, 0 replies; 13+ messages in thread
From: John Snow @ 2021-10-27 15:34 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: qemu-devel, Markus Armbruster

[-- Attachment #1: Type: text/plain, Size: 3057 bytes --]

On Wed, Oct 27, 2021 at 6:31 AM Kevin Wolf <kwolf@redhat.com> wrote:

> Am 26.10.2021 um 23:23 hat John Snow geschrieben:
> > On Fri, Sep 17, 2021 at 12:13 PM Kevin Wolf <kwolf@redhat.com> wrote:
> >
> > > This series introduces alias definitions for QAPI object types (structs
> > > and unions).
> > >
> > > This allows using the same QAPI type and visitor even when the syntax
> > > has some variations between different external interfaces such as QMP
> > > and the command line.
> > >
> > >
> > I'm absurdly late to the party here, and I'm afraid my involvement may
> only
> > stall your progress even further, but ...
>
> I'm not sure if it's worth investing much of your time in this. After a
>

ACK, understood. Still, I had this on my review pile for a while and I want
to know what problem we're trying to solve, and why Markus wasn't
enthralled by it.


> year of discussing implementation details, Markus decided that he
> doesn't like the whole approach, so the series is probably dead in this
> shape. Maybe parts of it (possibly even large parts) can be saved and
> used in a modified approach, depending on how radically different the
> new approach is.
>
> Markus promised that he'll think of alternative approaches to solve the
> problem. I'm waiting for that before I waste more time implementing
> something else that is also dead from the start.
>
> > can you fill me in on the slightly-less-higher-level details here?
> >
> > I'm curious as to the nature of "has some variations" and how the aliases
> > help alleviate them. Do you accomplish that compatibility by using
> > different names for different fields of the struct?
>
> > so e.g. x.foo can be used as an alias for x.bar, but both map ultimately
> > onto 'x.foo'? Or does this series provide for some more fundamental
> > mechanical changes to map one type onto another?
>
> I would recommend reading the changes to docs/devel/qapi-code-gen.rst in
> patch 7, which explain the implemented mechanism.
>
> For -chardev, most of "some varations" are different names for the same
> member of a struct, or nesting in QMP that you don't want to have on the
> command line. I went through all cases in one of the last messages in
> the v3 thread, but I think these are the two important categories of
> cases.
>
> The C types stay the same as they have always been, aliases just provide
> an alternative way to specify the same thing in the input.
>
>
Thanks very much for the pointers!


> > > It also provides a new tool for evolving the schema while maintaining
> > > backwards compatibility (possibly during a deprecation period).
> > >
> > > The first user is intended to be a QAPIfied -chardev command line
> > > option, for which I'll send a separate series. A git tag is available
> > > that contains both this series and the chardev changes that make use of
> > > it:
> > >
> > >     https://repo.or.cz/qemu/kevin.git qapi-alias-chardev-v4
>
> You may also want to have a look at this, and specifically
> qapi/char.json, to see how I used aliases in practice.
>
> Kevin
>
>

[-- Attachment #2: Type: text/html, Size: 4386 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-10-27 16:07 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-17 16:13 [PATCH v4 0/8] qapi: Add support for aliases Kevin Wolf
2021-09-17 16:13 ` [PATCH v4 1/8] qapi: Add interfaces for alias support to Visitor Kevin Wolf
2021-09-17 16:13 ` [PATCH v4 2/8] qapi: Remember alias definitions in qobject-input-visitor Kevin Wolf
2021-09-17 16:13 ` [PATCH v4 3/8] qapi: Simplify full_name_nth() " Kevin Wolf
2021-09-17 16:13 ` [PATCH v4 4/8] qapi: Store Error in StackObject.h for qobject-input-visitor Kevin Wolf
2021-09-17 16:13 ` [PATCH v4 5/8] qapi: Apply aliases in qobject-input-visitor Kevin Wolf
2021-09-17 16:13 ` [PATCH v4 6/8] qapi: Revert an accidental change from list to view object Kevin Wolf
2021-09-17 16:13 ` [PATCH v4 7/8] qapi: Add support for aliases Kevin Wolf
2021-09-17 16:13 ` [PATCH v4 8/8] tests/qapi-schema: Test cases " Kevin Wolf
2021-10-01  9:28 ` [PATCH v4 0/8] qapi: Add support " Kevin Wolf
2021-10-26 21:23 ` John Snow
2021-10-27 10:31   ` Kevin Wolf
2021-10-27 15:34     ` John Snow

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.