diff mbox series

[v2,3/3] qapi: allow unions to contain further unions

Message ID 20230223134027.2294640-4-berrange@redhat.com (mailing list archive)
State New, archived
Headers show
Series qapi: allow unions to contain further unions | expand

Commit Message

Daniel P. Berrangé Feb. 23, 2023, 1:40 p.m. UTC
This extends the QAPI schema validation to permit unions inside unions,
provided the checks for clashing fields pass.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 scripts/qapi/schema.py                        |  6 +-
 tests/qapi-schema/meson.build                 |  2 +
 tests/qapi-schema/qapi-schema-test.json       | 32 ++++++++++
 tests/qapi-schema/qapi-schema-test.out        | 29 ++++++++++
 .../union-invalid-union-subfield.err          |  2 +
 .../union-invalid-union-subfield.json         | 30 ++++++++++
 .../union-invalid-union-subfield.out          |  0
 .../union-invalid-union-subtype.err           |  2 +
 .../union-invalid-union-subtype.json          | 29 ++++++++++
 .../union-invalid-union-subtype.out           |  0
 tests/unit/test-qobject-input-visitor.c       | 47 +++++++++++++++
 tests/unit/test-qobject-output-visitor.c      | 58 +++++++++++++++++++
 12 files changed, 234 insertions(+), 3 deletions(-)
 create mode 100644 tests/qapi-schema/union-invalid-union-subfield.err
 create mode 100644 tests/qapi-schema/union-invalid-union-subfield.json
 create mode 100644 tests/qapi-schema/union-invalid-union-subfield.out
 create mode 100644 tests/qapi-schema/union-invalid-union-subtype.err
 create mode 100644 tests/qapi-schema/union-invalid-union-subtype.json
 create mode 100644 tests/qapi-schema/union-invalid-union-subtype.out

Comments

Het Gala March 7, 2023, 3:53 a.m. UTC | #1
Hi,


On 23/02/23 7:10 pm, Daniel P. Berrangé wrote:
> This extends the QAPI schema validation to permit unions inside unions,
> provided the checks for clashing fields pass.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   scripts/qapi/schema.py                        |  6 +-
>   tests/qapi-schema/meson.build                 |  2 +
>   tests/qapi-schema/qapi-schema-test.json       | 32 ++++++++++
>   tests/qapi-schema/qapi-schema-test.out        | 29 ++++++++++
>   .../union-invalid-union-subfield.err          |  2 +
>   .../union-invalid-union-subfield.json         | 30 ++++++++++
>   .../union-invalid-union-subfield.out          |  0
>   .../union-invalid-union-subtype.err           |  2 +
>   .../union-invalid-union-subtype.json          | 29 ++++++++++
>   .../union-invalid-union-subtype.out           |  0
>   tests/unit/test-qobject-input-visitor.c       | 47 +++++++++++++++
>   tests/unit/test-qobject-output-visitor.c      | 58 +++++++++++++++++++
>   12 files changed, 234 insertions(+), 3 deletions(-)
>   create mode 100644 tests/qapi-schema/union-invalid-union-subfield.err
>   create mode 100644 tests/qapi-schema/union-invalid-union-subfield.json
>   create mode 100644 tests/qapi-schema/union-invalid-union-subfield.out
>   create mode 100644 tests/qapi-schema/union-invalid-union-subtype.err
>   create mode 100644 tests/qapi-schema/union-invalid-union-subtype.json
>   create mode 100644 tests/qapi-schema/union-invalid-union-subtype.out
>
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index 6c481ab0c0..5c4457f789 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -465,9 +465,10 @@ def check(self, schema):
>       # on behalf of info, which is not necessarily self.info
>       def check_clash(self, info, seen):
>           assert self._checked
> -        assert not self.variants       # not implemented
>           for m in self.members:
>               m.check_clash(info, seen)
> +        if self.variants:
> +            self.variants.check_clash(info, seen)
>   
>       def connect_doc(self, doc=None):
>           super().connect_doc(doc)
> @@ -652,8 +653,7 @@ def check(self, schema, seen):
>                           self.info,
>                           "branch '%s' is not a value of %s"
>                           % (v.name, self.tag_member.type.describe()))
> -                if (not isinstance(v.type, QAPISchemaObjectType)
> -                        or v.type.variants):
> +                if not isinstance(v.type, QAPISchemaObjectType):
>                       raise QAPISemError(
>                           self.info,
>                           "%s cannot use %s"
> diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
> index d85b14f28c..1591eb322b 100644
> --- a/tests/qapi-schema/meson.build
> +++ b/tests/qapi-schema/meson.build
> @@ -194,6 +194,8 @@ schemas = [
>     'union-invalid-data.json',
>     'union-invalid-discriminator.json',
>     'union-invalid-if-discriminator.json',
> +  'union-invalid-union-subfield.json',
> +  'union-invalid-union-subtype.json',
>     'union-no-base.json',
>     'union-optional-discriminator.json',
>     'union-string-discriminator.json',
> diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
> index ba7302f42b..40f1a3d88d 100644
> --- a/tests/qapi-schema/qapi-schema-test.json
> +++ b/tests/qapi-schema/qapi-schema-test.json
> @@ -114,6 +114,38 @@
>   { 'struct': 'UserDefC',
>     'data': { 'string1': 'str', 'string2': 'str' } }
>   
> +# this tests that unions can contain other unions in their branches
> +{ 'enum': 'TestUnionEnum',
> +  'data': [ 'value-a', 'value-b' ] }
> +
> +{ 'enum': 'TestUnionEnumA',
> +  'data': [ 'value-a1', 'value-a2' ] }
> +
> +{ 'struct': 'TestUnionTypeA1',
> +  'data': { 'integer': 'int',
> +            'name': 'str'} }
> +
> +{ 'struct': 'TestUnionTypeA2',
> +  'data': { 'integer': 'int',
> +            'size': 'int' } }
> +
> +{ 'union': 'TestUnionTypeA',
> +  'base': { 'type-a': 'TestUnionEnumA' },
> +  'discriminator': 'type-a',
> +  'data': { 'value-a1': 'TestUnionTypeA1',
> +            'value-a2': 'TestUnionTypeA2' } }
> +
> +{ 'struct': 'TestUnionTypeB',
> +  'data': { 'integer': 'int',
> +            'onoff': 'bool' } }
> +
> +{ 'union': 'TestUnionInUnion',
> +  'base': { 'type': 'TestUnionEnum' },
> +  'discriminator': 'type',
> +  'data': { 'value-a': 'TestUnionTypeA',
> +            'value-b': 'TestUnionTypeB' } }
> +
> +
>   # for testing use of 'number' within alternates
>   { 'alternate': 'AltEnumBool', 'data': { 'e': 'EnumOne', 'b': 'bool' } }
>   { 'alternate': 'AltEnumNum', 'data': { 'e': 'EnumOne', 'n': 'number' } }
> diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
> index 043d75c655..9fe1038944 100644
> --- a/tests/qapi-schema/qapi-schema-test.out
> +++ b/tests/qapi-schema/qapi-schema-test.out
> @@ -105,6 +105,35 @@ alternate UserDefAlternate
>   object UserDefC
>       member string1: str optional=False
>       member string2: str optional=False
> +enum TestUnionEnum
> +    member value-a
> +    member value-b
> +enum TestUnionEnumA
> +    member value-a1
> +    member value-a2
> +object TestUnionTypeA1
> +    member integer: int optional=False
> +    member name: str optional=False
> +object TestUnionTypeA2
> +    member integer: int optional=False
> +    member size: int optional=False
> +object q_obj_TestUnionTypeA-base
> +    member type-a: TestUnionEnumA optional=False
> +object TestUnionTypeA
> +    base q_obj_TestUnionTypeA-base
> +    tag type-a
> +    case value-a1: TestUnionTypeA1
> +    case value-a2: TestUnionTypeA2
> +object TestUnionTypeB
> +    member integer: int optional=False
> +    member onoff: bool optional=False
> +object q_obj_TestUnionInUnion-base
> +    member type: TestUnionEnum optional=False
> +object TestUnionInUnion
> +    base q_obj_TestUnionInUnion-base
> +    tag type
> +    case value-a: TestUnionTypeA
> +    case value-b: TestUnionTypeB
>   alternate AltEnumBool
>       tag type
>       case e: EnumOne
> diff --git a/tests/qapi-schema/union-invalid-union-subfield.err b/tests/qapi-schema/union-invalid-union-subfield.err
> new file mode 100644
> index 0000000000..43574dea79
> --- /dev/null
> +++ b/tests/qapi-schema/union-invalid-union-subfield.err
> @@ -0,0 +1,2 @@
> +union-invalid-union-subfield.json: In union 'TestUnion':
> +union-invalid-union-subfield.json:25: member 'teeth' of type 'TestTypeFish' collides with base member 'teeth' of type 'TestUnion-base'
> diff --git a/tests/qapi-schema/union-invalid-union-subfield.json b/tests/qapi-schema/union-invalid-union-subfield.json
> new file mode 100644
> index 0000000000..e1639d3a96
> --- /dev/null
> +++ b/tests/qapi-schema/union-invalid-union-subfield.json
> @@ -0,0 +1,30 @@
> +# Clash between common member and union variant's variant member
> +# Base's member 'teeth' clashes with TestTypeFish's
> +
> +{ 'enum': 'TestEnum',
> +  'data': [ 'animals', 'plants' ] }
> +
> +{ 'enum': 'TestAnimals',
> +  'data': [ 'fish', 'birds'] }
> +
> +{ 'struct': 'TestTypeFish',
> +  'data': { 'scales': 'int', 'teeth': 'int' } }
> +
> +{ 'struct': 'TestTypeBirds',
> +  'data': { 'feathers': 'int' } }
> +
> +{ 'union': 'TestTypeAnimals',
> +  'base': { 'atype': 'TestAnimals' },
> +  'discriminator': 'atype',
> +  'data': { 'fish': 'TestTypeFish',
> +            'birds': 'TestTypeBirds' } }
> +
> +{ 'struct': 'TestTypePlants',
> +  'data': { 'integer': 'int' } }
> +
> +{ 'union': 'TestUnion',
> +  'base': { 'type': 'TestEnum',
> +            'teeth': 'int' },
> +  'discriminator': 'type',
> +  'data': { 'animals': 'TestTypeAnimals',
> +            'plants': 'TestTypePlants' } }
> diff --git a/tests/qapi-schema/union-invalid-union-subfield.out b/tests/qapi-schema/union-invalid-union-subfield.out
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/tests/qapi-schema/union-invalid-union-subtype.err b/tests/qapi-schema/union-invalid-union-subtype.err
> new file mode 100644
> index 0000000000..e45f330cec
> --- /dev/null
> +++ b/tests/qapi-schema/union-invalid-union-subtype.err
> @@ -0,0 +1,2 @@
> +union-invalid-union-subtype.json: In union 'TestUnion':
> +union-invalid-union-subtype.json:25: base member 'type' of type 'TestTypeA-base' collides with base member 'type' of type 'TestUnion-base'
> diff --git a/tests/qapi-schema/union-invalid-union-subtype.json b/tests/qapi-schema/union-invalid-union-subtype.json
> new file mode 100644
> index 0000000000..ce1de51d8d
> --- /dev/null
> +++ b/tests/qapi-schema/union-invalid-union-subtype.json
> @@ -0,0 +1,29 @@
> +# Clash between common member and union variant's common member
> +# Base's member 'type' clashes with TestTypeA's
> +
> +{ 'enum': 'TestEnum',
> +  'data': [ 'value-a', 'value-b' ] }
> +
> +{ 'enum': 'TestEnumA',
> +  'data': [ 'value-a1', 'value-a2' ] }
> +
> +{ 'struct': 'TestTypeA1',
> +  'data': { 'integer': 'int' } }
> +
> +{ 'struct': 'TestTypeA2',
> +  'data': { 'integer': 'int' } }
> +
> +{ 'union': 'TestTypeA',
> +  'base': { 'type': 'TestEnumA' },
> +  'discriminator': 'type',
> +  'data': { 'value-a1': 'TestTypeA1',
> +            'value-a2': 'TestTypeA2' } }
> +
> +{ 'struct': 'TestTypeB',
> +  'data': { 'integer': 'int' } }
> +
> +{ 'union': 'TestUnion',
> +  'base': { 'type': 'TestEnum' },
> +  'discriminator': 'type',
> +  'data': { 'value-a': 'TestTypeA',
> +            'value-b': 'TestTypeB' } }
> diff --git a/tests/qapi-schema/union-invalid-union-subtype.out b/tests/qapi-schema/union-invalid-union-subtype.out
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/tests/unit/test-qobject-input-visitor.c b/tests/unit/test-qobject-input-visitor.c
> index 77fbf985be..9b3e2dbe14 100644
> --- a/tests/unit/test-qobject-input-visitor.c
> +++ b/tests/unit/test-qobject-input-visitor.c
> @@ -706,6 +706,51 @@ static void test_visitor_in_union_flat(TestInputVisitorData *data,
>       g_assert(&base->enum1 == &tmp->enum1);
>   }
>   
> +static void test_visitor_in_union_in_union(TestInputVisitorData *data,
> +                                           const void *unused)
> +{
> +    Visitor *v;
> +    g_autoptr(TestUnionInUnion) tmp = NULL;
> +
> +    v = visitor_input_test_init(data,
> +                                "{ 'type': 'value-a', "
> +                                "  'type-a': 'value-a1', "
> +                                "  'integer': 2, "
> +                                "  'name': 'fish' }");
> +
> +    visit_type_TestUnionInUnion(v, NULL, &tmp, &error_abort);
> +    g_assert_cmpint(tmp->type, ==, TEST_UNION_ENUM_VALUE_A);
> +    g_assert_cmpint(tmp->u.value_a.type_a, ==, TEST_UNION_ENUMA_VALUE_A1);
> +    g_assert_cmpint(tmp->u.value_a.u.value_a1.integer, ==, 2);
> +    g_assert_cmpint(strcmp(tmp->u.value_a.u.value_a1.name, "fish"), ==, 0);
> +
> +    qapi_free_TestUnionInUnion(tmp);
> +
> +    v = visitor_input_test_init(data,
> +                                "{ 'type': 'value-a', "
> +                                "  'type-a': 'value-a2', "
> +                                "  'integer': 1729, "
> +                                "  'size': 87539319 }");
> +
> +    visit_type_TestUnionInUnion(v, NULL, &tmp, &error_abort);
> +    g_assert_cmpint(tmp->type, ==, TEST_UNION_ENUM_VALUE_A);
> +    g_assert_cmpint(tmp->u.value_a.type_a, ==, TEST_UNION_ENUMA_VALUE_A2);
> +    g_assert_cmpint(tmp->u.value_a.u.value_a2.integer, ==, 1729);
> +    g_assert_cmpint(tmp->u.value_a.u.value_a2.size, ==, 87539319);
> +
> +    qapi_free_TestUnionInUnion(tmp);
> +
> +    v = visitor_input_test_init(data,
> +                                "{ 'type': 'value-b', "
> +                                "  'integer': 1729, "
> +                                "  'onoff': true }");
> +
> +    visit_type_TestUnionInUnion(v, NULL, &tmp, &error_abort);
> +    g_assert_cmpint(tmp->type, ==, TEST_UNION_ENUM_VALUE_B);
> +    g_assert_cmpint(tmp->u.value_b.integer, ==, 1729);
> +    g_assert_cmpint(tmp->u.value_b.onoff, ==, true);
> +}
> +
>   static void test_visitor_in_alternate(TestInputVisitorData *data,
>                                         const void *unused)
>   {
> @@ -1216,6 +1261,8 @@ int main(int argc, char **argv)
>                              NULL, test_visitor_in_null);
>       input_visitor_test_add("/visitor/input/union-flat",
>                              NULL, test_visitor_in_union_flat);
> +    input_visitor_test_add("/visitor/input/union-in-union",
> +                           NULL, test_visitor_in_union_in_union);
>       input_visitor_test_add("/visitor/input/alternate",
>                              NULL, test_visitor_in_alternate);
>       input_visitor_test_add("/visitor/input/errors",
> diff --git a/tests/unit/test-qobject-output-visitor.c b/tests/unit/test-qobject-output-visitor.c
> index 7f054289fe..1535b3ad17 100644
> --- a/tests/unit/test-qobject-output-visitor.c
> +++ b/tests/unit/test-qobject-output-visitor.c
> @@ -352,6 +352,62 @@ static void test_visitor_out_union_flat(TestOutputVisitorData *data,
>       qapi_free_UserDefFlatUnion(tmp);
>   }
>   
> +static void test_visitor_out_union_in_union(TestOutputVisitorData *data,
> +                                            const void *unused)
> +{
> +    QDict *qdict;
> +
> +    TestUnionInUnion *tmp = g_new0(TestUnionInUnion, 1);
> +    tmp->type = TEST_UNION_ENUM_VALUE_A;
> +    tmp->u.value_a.type_a = TEST_UNION_ENUMA_VALUE_A1;
> +    tmp->u.value_a.u.value_a1.integer = 42;
> +    tmp->u.value_a.u.value_a1.name = g_strdup("fish");
> +
> +    visit_type_TestUnionInUnion(data->ov, NULL, &tmp, &error_abort);
> +    qdict = qobject_to(QDict, visitor_get(data));
> +    g_assert(qdict);
> +    g_assert_cmpstr(qdict_get_str(qdict, "type"), ==, "value-a");
> +    g_assert_cmpstr(qdict_get_str(qdict, "type-a"), ==, "value-a1");
> +    g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 42);
> +    g_assert_cmpstr(qdict_get_str(qdict, "name"), ==, "fish");
> +
> +    qapi_free_TestUnionInUnion(tmp);
> +
> +
> +    visitor_reset(data);
> +    tmp = g_new0(TestUnionInUnion, 1);
> +    tmp->type = TEST_UNION_ENUM_VALUE_A;
> +    tmp->u.value_a.type_a = TEST_UNION_ENUMA_VALUE_A2;
> +    tmp->u.value_a.u.value_a2.integer = 1729;
> +    tmp->u.value_a.u.value_a2.size = 87539319;
> +
> +    visit_type_TestUnionInUnion(data->ov, NULL, &tmp, &error_abort);
> +    qdict = qobject_to(QDict, visitor_get(data));
> +    g_assert(qdict);
> +    g_assert_cmpstr(qdict_get_str(qdict, "type"), ==, "value-a");
> +    g_assert_cmpstr(qdict_get_str(qdict, "type-a"), ==, "value-a2");
> +    g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 1729);
> +    g_assert_cmpint(qdict_get_int(qdict, "size"), ==, 87539319);
> +
> +    qapi_free_TestUnionInUnion(tmp);
> +
> +
> +    visitor_reset(data);
> +    tmp = g_new0(TestUnionInUnion, 1);
> +    tmp->type = TEST_UNION_ENUM_VALUE_B;
> +    tmp->u.value_b.integer = 1729;
> +    tmp->u.value_b.onoff = true;
> +
> +    visit_type_TestUnionInUnion(data->ov, NULL, &tmp, &error_abort);
> +    qdict = qobject_to(QDict, visitor_get(data));
> +    g_assert(qdict);
> +    g_assert_cmpstr(qdict_get_str(qdict, "type"), ==, "value-b");
> +    g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 1729);
> +    g_assert_cmpint(qdict_get_bool(qdict, "onoff"), ==, true);
> +
> +    qapi_free_TestUnionInUnion(tmp);
> +}
> +
>   static void test_visitor_out_alternate(TestOutputVisitorData *data,
>                                          const void *unused)
>   {
> @@ -586,6 +642,8 @@ int main(int argc, char **argv)
>                               &out_visitor_data, test_visitor_out_list_qapi_free);
>       output_visitor_test_add("/visitor/output/union-flat",
>                               &out_visitor_data, test_visitor_out_union_flat);
> +    output_visitor_test_add("/visitor/output/union-in-union",
> +                            &out_visitor_data, test_visitor_out_union_in_union);
>       output_visitor_test_add("/visitor/output/alternate",
>                               &out_visitor_data, test_visitor_out_alternate);
>       output_visitor_test_add("/visitor/output/null",

The changes look good to me. I have tried to built migrate QAPI changes 
on top of Daniel's v1 changes, and it was successful.

Once Markus and other maintainers approves this patchset, I can post v4 
version of 'migrate' qapi changes on top of this change.

Looking forward for replies on this patchset :)

Regards,
Het Gala
Markus Armbruster March 17, 2023, 3:48 p.m. UTC | #2
Daniel P. Berrangé <berrange@redhat.com> writes:

> This extends the QAPI schema validation to permit unions inside unions,
> provided the checks for clashing fields pass.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  scripts/qapi/schema.py                        |  6 +-
>  tests/qapi-schema/meson.build                 |  2 +
>  tests/qapi-schema/qapi-schema-test.json       | 32 ++++++++++
>  tests/qapi-schema/qapi-schema-test.out        | 29 ++++++++++
>  .../union-invalid-union-subfield.err          |  2 +
>  .../union-invalid-union-subfield.json         | 30 ++++++++++
>  .../union-invalid-union-subfield.out          |  0
>  .../union-invalid-union-subtype.err           |  2 +
>  .../union-invalid-union-subtype.json          | 29 ++++++++++
>  .../union-invalid-union-subtype.out           |  0
>  tests/unit/test-qobject-input-visitor.c       | 47 +++++++++++++++
>  tests/unit/test-qobject-output-visitor.c      | 58 +++++++++++++++++++
>  12 files changed, 234 insertions(+), 3 deletions(-)
>  create mode 100644 tests/qapi-schema/union-invalid-union-subfield.err
>  create mode 100644 tests/qapi-schema/union-invalid-union-subfield.json
>  create mode 100644 tests/qapi-schema/union-invalid-union-subfield.out
>  create mode 100644 tests/qapi-schema/union-invalid-union-subtype.err
>  create mode 100644 tests/qapi-schema/union-invalid-union-subtype.json
>  create mode 100644 tests/qapi-schema/union-invalid-union-subtype.out
>
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index 6c481ab0c0..5c4457f789 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -465,9 +465,10 @@ def check(self, schema):
>      # on behalf of info, which is not necessarily self.info
>      def check_clash(self, info, seen):
>          assert self._checked
> -        assert not self.variants       # not implemented
>          for m in self.members:
>              m.check_clash(info, seen)
> +        if self.variants:
> +            self.variants.check_clash(info, seen)
>  
>      def connect_doc(self, doc=None):
>          super().connect_doc(doc)
> @@ -652,8 +653,7 @@ def check(self, schema, seen):
>                          self.info,
>                          "branch '%s' is not a value of %s"
>                          % (v.name, self.tag_member.type.describe()))
> -                if (not isinstance(v.type, QAPISchemaObjectType)
> -                        or v.type.variants):
> +                if not isinstance(v.type, QAPISchemaObjectType):
>                      raise QAPISemError(
>                          self.info,
>                          "%s cannot use %s"

I stared at the code some to convince myself this is complete.

> diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
> index d85b14f28c..1591eb322b 100644
> --- a/tests/qapi-schema/meson.build
> +++ b/tests/qapi-schema/meson.build
> @@ -194,6 +194,8 @@ schemas = [
>    'union-invalid-data.json',
>    'union-invalid-discriminator.json',
>    'union-invalid-if-discriminator.json',
> +  'union-invalid-union-subfield.json',
> +  'union-invalid-union-subtype.json',
>    'union-no-base.json',
>    'union-optional-discriminator.json',
>    'union-string-discriminator.json',
> diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
> index ba7302f42b..40f1a3d88d 100644
> --- a/tests/qapi-schema/qapi-schema-test.json
> +++ b/tests/qapi-schema/qapi-schema-test.json
> @@ -114,6 +114,38 @@
>  { 'struct': 'UserDefC',
>    'data': { 'string1': 'str', 'string2': 'str' } }
>  
> +# this tests that unions can contain other unions in their branches
> +{ 'enum': 'TestUnionEnum',
> +  'data': [ 'value-a', 'value-b' ] }
> +
> +{ 'enum': 'TestUnionEnumA',
> +  'data': [ 'value-a1', 'value-a2' ] }
> +
> +{ 'struct': 'TestUnionTypeA1',
> +  'data': { 'integer': 'int',
> +            'name': 'str'} }
> +
> +{ 'struct': 'TestUnionTypeA2',
> +  'data': { 'integer': 'int',
> +            'size': 'int' } }
> +
> +{ 'union': 'TestUnionTypeA',
> +  'base': { 'type-a': 'TestUnionEnumA' },
> +  'discriminator': 'type-a',
> +  'data': { 'value-a1': 'TestUnionTypeA1',
> +            'value-a2': 'TestUnionTypeA2' } }
> +
> +{ 'struct': 'TestUnionTypeB',
> +  'data': { 'integer': 'int',
> +            'onoff': 'bool' } }
> +
> +{ 'union': 'TestUnionInUnion',
> +  'base': { 'type': 'TestUnionEnum' },
> +  'discriminator': 'type',
> +  'data': { 'value-a': 'TestUnionTypeA',
> +            'value-b': 'TestUnionTypeB' } }
> +
> +
>  # for testing use of 'number' within alternates
>  { 'alternate': 'AltEnumBool', 'data': { 'e': 'EnumOne', 'b': 'bool' } }
>  { 'alternate': 'AltEnumNum', 'data': { 'e': 'EnumOne', 'n': 'number' } }
> diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
> index 043d75c655..9fe1038944 100644
> --- a/tests/qapi-schema/qapi-schema-test.out
> +++ b/tests/qapi-schema/qapi-schema-test.out
> @@ -105,6 +105,35 @@ alternate UserDefAlternate
>  object UserDefC
>      member string1: str optional=False
>      member string2: str optional=False
> +enum TestUnionEnum
> +    member value-a
> +    member value-b
> +enum TestUnionEnumA
> +    member value-a1
> +    member value-a2
> +object TestUnionTypeA1
> +    member integer: int optional=False
> +    member name: str optional=False
> +object TestUnionTypeA2
> +    member integer: int optional=False
> +    member size: int optional=False
> +object q_obj_TestUnionTypeA-base
> +    member type-a: TestUnionEnumA optional=False
> +object TestUnionTypeA
> +    base q_obj_TestUnionTypeA-base
> +    tag type-a
> +    case value-a1: TestUnionTypeA1
> +    case value-a2: TestUnionTypeA2
> +object TestUnionTypeB
> +    member integer: int optional=False
> +    member onoff: bool optional=False
> +object q_obj_TestUnionInUnion-base
> +    member type: TestUnionEnum optional=False
> +object TestUnionInUnion
> +    base q_obj_TestUnionInUnion-base
> +    tag type
> +    case value-a: TestUnionTypeA
> +    case value-b: TestUnionTypeB
>  alternate AltEnumBool
>      tag type
>      case e: EnumOne

Looks good to me.  I also inspected the generated code; no complaints.

> diff --git a/tests/qapi-schema/union-invalid-union-subfield.err b/tests/qapi-schema/union-invalid-union-subfield.err
> new file mode 100644
> index 0000000000..43574dea79
> --- /dev/null
> +++ b/tests/qapi-schema/union-invalid-union-subfield.err
> @@ -0,0 +1,2 @@
> +union-invalid-union-subfield.json: In union 'TestUnion':
> +union-invalid-union-subfield.json:25: member 'teeth' of type 'TestTypeFish' collides with base member 'teeth' of type 'TestUnion-base'

Bad error message, see my review of PATCH 1.

> diff --git a/tests/qapi-schema/union-invalid-union-subfield.json b/tests/qapi-schema/union-invalid-union-subfield.json
> new file mode 100644
> index 0000000000..e1639d3a96
> --- /dev/null
> +++ b/tests/qapi-schema/union-invalid-union-subfield.json
> @@ -0,0 +1,30 @@
> +# Clash between common member and union variant's variant member
> +# Base's member 'teeth' clashes with TestTypeFish's
> +
> +{ 'enum': 'TestEnum',
> +  'data': [ 'animals', 'plants' ] }
> +
> +{ 'enum': 'TestAnimals',
> +  'data': [ 'fish', 'birds'] }
> +
> +{ 'struct': 'TestTypeFish',
> +  'data': { 'scales': 'int', 'teeth': 'int' } }
> +
> +{ 'struct': 'TestTypeBirds',
> +  'data': { 'feathers': 'int' } }
> +
> +{ 'union': 'TestTypeAnimals',
> +  'base': { 'atype': 'TestAnimals' },
> +  'discriminator': 'atype',
> +  'data': { 'fish': 'TestTypeFish',
> +            'birds': 'TestTypeBirds' } }
> +
> +{ 'struct': 'TestTypePlants',
> +  'data': { 'integer': 'int' } }
> +
> +{ 'union': 'TestUnion',
> +  'base': { 'type': 'TestEnum',
> +            'teeth': 'int' },
> +  'discriminator': 'type',
> +  'data': { 'animals': 'TestTypeAnimals',
> +            'plants': 'TestTypePlants' } }
> diff --git a/tests/qapi-schema/union-invalid-union-subfield.out b/tests/qapi-schema/union-invalid-union-subfield.out
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/tests/qapi-schema/union-invalid-union-subtype.err b/tests/qapi-schema/union-invalid-union-subtype.err
> new file mode 100644
> index 0000000000..e45f330cec
> --- /dev/null
> +++ b/tests/qapi-schema/union-invalid-union-subtype.err
> @@ -0,0 +1,2 @@
> +union-invalid-union-subtype.json: In union 'TestUnion':
> +union-invalid-union-subtype.json:25: base member 'type' of type 'TestTypeA-base' collides with base member 'type' of type 'TestUnion-base'

Likewise.

> diff --git a/tests/qapi-schema/union-invalid-union-subtype.json b/tests/qapi-schema/union-invalid-union-subtype.json
> new file mode 100644
> index 0000000000..ce1de51d8d
> --- /dev/null
> +++ b/tests/qapi-schema/union-invalid-union-subtype.json
> @@ -0,0 +1,29 @@
> +# Clash between common member and union variant's common member
> +# Base's member 'type' clashes with TestTypeA's
> +
> +{ 'enum': 'TestEnum',
> +  'data': [ 'value-a', 'value-b' ] }
> +
> +{ 'enum': 'TestEnumA',
> +  'data': [ 'value-a1', 'value-a2' ] }
> +
> +{ 'struct': 'TestTypeA1',
> +  'data': { 'integer': 'int' } }
> +
> +{ 'struct': 'TestTypeA2',
> +  'data': { 'integer': 'int' } }
> +
> +{ 'union': 'TestTypeA',
> +  'base': { 'type': 'TestEnumA' },
> +  'discriminator': 'type',
> +  'data': { 'value-a1': 'TestTypeA1',
> +            'value-a2': 'TestTypeA2' } }
> +
> +{ 'struct': 'TestTypeB',
> +  'data': { 'integer': 'int' } }
> +
> +{ 'union': 'TestUnion',
> +  'base': { 'type': 'TestEnum' },
> +  'discriminator': 'type',
> +  'data': { 'value-a': 'TestTypeA',
> +            'value-b': 'TestTypeB' } }
> diff --git a/tests/qapi-schema/union-invalid-union-subtype.out b/tests/qapi-schema/union-invalid-union-subtype.out
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/tests/unit/test-qobject-input-visitor.c b/tests/unit/test-qobject-input-visitor.c
> index 77fbf985be..9b3e2dbe14 100644
> --- a/tests/unit/test-qobject-input-visitor.c
> +++ b/tests/unit/test-qobject-input-visitor.c
> @@ -706,6 +706,51 @@ static void test_visitor_in_union_flat(TestInputVisitorData *data,
>      g_assert(&base->enum1 == &tmp->enum1);
>  }
>  
> +static void test_visitor_in_union_in_union(TestInputVisitorData *data,
> +                                           const void *unused)
> +{
> +    Visitor *v;
> +    g_autoptr(TestUnionInUnion) tmp = NULL;
> +
> +    v = visitor_input_test_init(data,
> +                                "{ 'type': 'value-a', "
> +                                "  'type-a': 'value-a1', "
> +                                "  'integer': 2, "
> +                                "  'name': 'fish' }");
> +
> +    visit_type_TestUnionInUnion(v, NULL, &tmp, &error_abort);
> +    g_assert_cmpint(tmp->type, ==, TEST_UNION_ENUM_VALUE_A);
> +    g_assert_cmpint(tmp->u.value_a.type_a, ==, TEST_UNION_ENUMA_VALUE_A1);
> +    g_assert_cmpint(tmp->u.value_a.u.value_a1.integer, ==, 2);
> +    g_assert_cmpint(strcmp(tmp->u.value_a.u.value_a1.name, "fish"), ==, 0);
> +
> +    qapi_free_TestUnionInUnion(tmp);
> +
> +    v = visitor_input_test_init(data,
> +                                "{ 'type': 'value-a', "
> +                                "  'type-a': 'value-a2', "
> +                                "  'integer': 1729, "
> +                                "  'size': 87539319 }");
> +
> +    visit_type_TestUnionInUnion(v, NULL, &tmp, &error_abort);
> +    g_assert_cmpint(tmp->type, ==, TEST_UNION_ENUM_VALUE_A);
> +    g_assert_cmpint(tmp->u.value_a.type_a, ==, TEST_UNION_ENUMA_VALUE_A2);
> +    g_assert_cmpint(tmp->u.value_a.u.value_a2.integer, ==, 1729);
> +    g_assert_cmpint(tmp->u.value_a.u.value_a2.size, ==, 87539319);
> +
> +    qapi_free_TestUnionInUnion(tmp);
> +
> +    v = visitor_input_test_init(data,
> +                                "{ 'type': 'value-b', "
> +                                "  'integer': 1729, "
> +                                "  'onoff': true }");
> +
> +    visit_type_TestUnionInUnion(v, NULL, &tmp, &error_abort);
> +    g_assert_cmpint(tmp->type, ==, TEST_UNION_ENUM_VALUE_B);
> +    g_assert_cmpint(tmp->u.value_b.integer, ==, 1729);
> +    g_assert_cmpint(tmp->u.value_b.onoff, ==, true);
> +}
> +
>  static void test_visitor_in_alternate(TestInputVisitorData *data,
>                                        const void *unused)
>  {
> @@ -1216,6 +1261,8 @@ int main(int argc, char **argv)
>                             NULL, test_visitor_in_null);
>      input_visitor_test_add("/visitor/input/union-flat",
>                             NULL, test_visitor_in_union_flat);
> +    input_visitor_test_add("/visitor/input/union-in-union",
> +                           NULL, test_visitor_in_union_in_union);
>      input_visitor_test_add("/visitor/input/alternate",
>                             NULL, test_visitor_in_alternate);
>      input_visitor_test_add("/visitor/input/errors",
> diff --git a/tests/unit/test-qobject-output-visitor.c b/tests/unit/test-qobject-output-visitor.c
> index 7f054289fe..1535b3ad17 100644
> --- a/tests/unit/test-qobject-output-visitor.c
> +++ b/tests/unit/test-qobject-output-visitor.c
> @@ -352,6 +352,62 @@ static void test_visitor_out_union_flat(TestOutputVisitorData *data,
>      qapi_free_UserDefFlatUnion(tmp);
>  }
>  
> +static void test_visitor_out_union_in_union(TestOutputVisitorData *data,
> +                                            const void *unused)
> +{
> +    QDict *qdict;
> +
> +    TestUnionInUnion *tmp = g_new0(TestUnionInUnion, 1);
> +    tmp->type = TEST_UNION_ENUM_VALUE_A;
> +    tmp->u.value_a.type_a = TEST_UNION_ENUMA_VALUE_A1;
> +    tmp->u.value_a.u.value_a1.integer = 42;
> +    tmp->u.value_a.u.value_a1.name = g_strdup("fish");
> +
> +    visit_type_TestUnionInUnion(data->ov, NULL, &tmp, &error_abort);
> +    qdict = qobject_to(QDict, visitor_get(data));
> +    g_assert(qdict);
> +    g_assert_cmpstr(qdict_get_str(qdict, "type"), ==, "value-a");
> +    g_assert_cmpstr(qdict_get_str(qdict, "type-a"), ==, "value-a1");
> +    g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 42);
> +    g_assert_cmpstr(qdict_get_str(qdict, "name"), ==, "fish");
> +
> +    qapi_free_TestUnionInUnion(tmp);
> +
> +
> +    visitor_reset(data);
> +    tmp = g_new0(TestUnionInUnion, 1);
> +    tmp->type = TEST_UNION_ENUM_VALUE_A;
> +    tmp->u.value_a.type_a = TEST_UNION_ENUMA_VALUE_A2;
> +    tmp->u.value_a.u.value_a2.integer = 1729;
> +    tmp->u.value_a.u.value_a2.size = 87539319;
> +
> +    visit_type_TestUnionInUnion(data->ov, NULL, &tmp, &error_abort);
> +    qdict = qobject_to(QDict, visitor_get(data));
> +    g_assert(qdict);
> +    g_assert_cmpstr(qdict_get_str(qdict, "type"), ==, "value-a");
> +    g_assert_cmpstr(qdict_get_str(qdict, "type-a"), ==, "value-a2");
> +    g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 1729);
> +    g_assert_cmpint(qdict_get_int(qdict, "size"), ==, 87539319);
> +
> +    qapi_free_TestUnionInUnion(tmp);
> +
> +
> +    visitor_reset(data);
> +    tmp = g_new0(TestUnionInUnion, 1);
> +    tmp->type = TEST_UNION_ENUM_VALUE_B;
> +    tmp->u.value_b.integer = 1729;
> +    tmp->u.value_b.onoff = true;
> +
> +    visit_type_TestUnionInUnion(data->ov, NULL, &tmp, &error_abort);
> +    qdict = qobject_to(QDict, visitor_get(data));
> +    g_assert(qdict);
> +    g_assert_cmpstr(qdict_get_str(qdict, "type"), ==, "value-b");
> +    g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 1729);
> +    g_assert_cmpint(qdict_get_bool(qdict, "onoff"), ==, true);
> +
> +    qapi_free_TestUnionInUnion(tmp);
> +}
> +
>  static void test_visitor_out_alternate(TestOutputVisitorData *data,
>                                         const void *unused)
>  {
> @@ -586,6 +642,8 @@ int main(int argc, char **argv)
>                              &out_visitor_data, test_visitor_out_list_qapi_free);
>      output_visitor_test_add("/visitor/output/union-flat",
>                              &out_visitor_data, test_visitor_out_union_flat);
> +    output_visitor_test_add("/visitor/output/union-in-union",
> +                            &out_visitor_data, test_visitor_out_union_in_union);
>      output_visitor_test_add("/visitor/output/alternate",
>                              &out_visitor_data, test_visitor_out_alternate);
>      output_visitor_test_add("/visitor/output/null",

Reviewed-by: Markus Armbruster <armbru@redhat.com>
diff mbox series

Patch

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 6c481ab0c0..5c4457f789 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -465,9 +465,10 @@  def check(self, schema):
     # on behalf of info, which is not necessarily self.info
     def check_clash(self, info, seen):
         assert self._checked
-        assert not self.variants       # not implemented
         for m in self.members:
             m.check_clash(info, seen)
+        if self.variants:
+            self.variants.check_clash(info, seen)
 
     def connect_doc(self, doc=None):
         super().connect_doc(doc)
@@ -652,8 +653,7 @@  def check(self, schema, seen):
                         self.info,
                         "branch '%s' is not a value of %s"
                         % (v.name, self.tag_member.type.describe()))
-                if (not isinstance(v.type, QAPISchemaObjectType)
-                        or v.type.variants):
+                if not isinstance(v.type, QAPISchemaObjectType):
                     raise QAPISemError(
                         self.info,
                         "%s cannot use %s"
diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
index d85b14f28c..1591eb322b 100644
--- a/tests/qapi-schema/meson.build
+++ b/tests/qapi-schema/meson.build
@@ -194,6 +194,8 @@  schemas = [
   'union-invalid-data.json',
   'union-invalid-discriminator.json',
   'union-invalid-if-discriminator.json',
+  'union-invalid-union-subfield.json',
+  'union-invalid-union-subtype.json',
   'union-no-base.json',
   'union-optional-discriminator.json',
   'union-string-discriminator.json',
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index ba7302f42b..40f1a3d88d 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -114,6 +114,38 @@ 
 { 'struct': 'UserDefC',
   'data': { 'string1': 'str', 'string2': 'str' } }
 
+# this tests that unions can contain other unions in their branches
+{ 'enum': 'TestUnionEnum',
+  'data': [ 'value-a', 'value-b' ] }
+
+{ 'enum': 'TestUnionEnumA',
+  'data': [ 'value-a1', 'value-a2' ] }
+
+{ 'struct': 'TestUnionTypeA1',
+  'data': { 'integer': 'int',
+            'name': 'str'} }
+
+{ 'struct': 'TestUnionTypeA2',
+  'data': { 'integer': 'int',
+            'size': 'int' } }
+
+{ 'union': 'TestUnionTypeA',
+  'base': { 'type-a': 'TestUnionEnumA' },
+  'discriminator': 'type-a',
+  'data': { 'value-a1': 'TestUnionTypeA1',
+            'value-a2': 'TestUnionTypeA2' } }
+
+{ 'struct': 'TestUnionTypeB',
+  'data': { 'integer': 'int',
+            'onoff': 'bool' } }
+
+{ 'union': 'TestUnionInUnion',
+  'base': { 'type': 'TestUnionEnum' },
+  'discriminator': 'type',
+  'data': { 'value-a': 'TestUnionTypeA',
+            'value-b': 'TestUnionTypeB' } }
+
+
 # for testing use of 'number' within alternates
 { 'alternate': 'AltEnumBool', 'data': { 'e': 'EnumOne', 'b': 'bool' } }
 { 'alternate': 'AltEnumNum', 'data': { 'e': 'EnumOne', 'n': 'number' } }
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 043d75c655..9fe1038944 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -105,6 +105,35 @@  alternate UserDefAlternate
 object UserDefC
     member string1: str optional=False
     member string2: str optional=False
+enum TestUnionEnum
+    member value-a
+    member value-b
+enum TestUnionEnumA
+    member value-a1
+    member value-a2
+object TestUnionTypeA1
+    member integer: int optional=False
+    member name: str optional=False
+object TestUnionTypeA2
+    member integer: int optional=False
+    member size: int optional=False
+object q_obj_TestUnionTypeA-base
+    member type-a: TestUnionEnumA optional=False
+object TestUnionTypeA
+    base q_obj_TestUnionTypeA-base
+    tag type-a
+    case value-a1: TestUnionTypeA1
+    case value-a2: TestUnionTypeA2
+object TestUnionTypeB
+    member integer: int optional=False
+    member onoff: bool optional=False
+object q_obj_TestUnionInUnion-base
+    member type: TestUnionEnum optional=False
+object TestUnionInUnion
+    base q_obj_TestUnionInUnion-base
+    tag type
+    case value-a: TestUnionTypeA
+    case value-b: TestUnionTypeB
 alternate AltEnumBool
     tag type
     case e: EnumOne
diff --git a/tests/qapi-schema/union-invalid-union-subfield.err b/tests/qapi-schema/union-invalid-union-subfield.err
new file mode 100644
index 0000000000..43574dea79
--- /dev/null
+++ b/tests/qapi-schema/union-invalid-union-subfield.err
@@ -0,0 +1,2 @@ 
+union-invalid-union-subfield.json: In union 'TestUnion':
+union-invalid-union-subfield.json:25: member 'teeth' of type 'TestTypeFish' collides with base member 'teeth' of type 'TestUnion-base'
diff --git a/tests/qapi-schema/union-invalid-union-subfield.json b/tests/qapi-schema/union-invalid-union-subfield.json
new file mode 100644
index 0000000000..e1639d3a96
--- /dev/null
+++ b/tests/qapi-schema/union-invalid-union-subfield.json
@@ -0,0 +1,30 @@ 
+# Clash between common member and union variant's variant member
+# Base's member 'teeth' clashes with TestTypeFish's
+
+{ 'enum': 'TestEnum',
+  'data': [ 'animals', 'plants' ] }
+
+{ 'enum': 'TestAnimals',
+  'data': [ 'fish', 'birds'] }
+
+{ 'struct': 'TestTypeFish',
+  'data': { 'scales': 'int', 'teeth': 'int' } }
+
+{ 'struct': 'TestTypeBirds',
+  'data': { 'feathers': 'int' } }
+
+{ 'union': 'TestTypeAnimals',
+  'base': { 'atype': 'TestAnimals' },
+  'discriminator': 'atype',
+  'data': { 'fish': 'TestTypeFish',
+            'birds': 'TestTypeBirds' } }
+
+{ 'struct': 'TestTypePlants',
+  'data': { 'integer': 'int' } }
+
+{ 'union': 'TestUnion',
+  'base': { 'type': 'TestEnum',
+            'teeth': 'int' },
+  'discriminator': 'type',
+  'data': { 'animals': 'TestTypeAnimals',
+            'plants': 'TestTypePlants' } }
diff --git a/tests/qapi-schema/union-invalid-union-subfield.out b/tests/qapi-schema/union-invalid-union-subfield.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/union-invalid-union-subtype.err b/tests/qapi-schema/union-invalid-union-subtype.err
new file mode 100644
index 0000000000..e45f330cec
--- /dev/null
+++ b/tests/qapi-schema/union-invalid-union-subtype.err
@@ -0,0 +1,2 @@ 
+union-invalid-union-subtype.json: In union 'TestUnion':
+union-invalid-union-subtype.json:25: base member 'type' of type 'TestTypeA-base' collides with base member 'type' of type 'TestUnion-base'
diff --git a/tests/qapi-schema/union-invalid-union-subtype.json b/tests/qapi-schema/union-invalid-union-subtype.json
new file mode 100644
index 0000000000..ce1de51d8d
--- /dev/null
+++ b/tests/qapi-schema/union-invalid-union-subtype.json
@@ -0,0 +1,29 @@ 
+# Clash between common member and union variant's common member
+# Base's member 'type' clashes with TestTypeA's
+
+{ 'enum': 'TestEnum',
+  'data': [ 'value-a', 'value-b' ] }
+
+{ 'enum': 'TestEnumA',
+  'data': [ 'value-a1', 'value-a2' ] }
+
+{ 'struct': 'TestTypeA1',
+  'data': { 'integer': 'int' } }
+
+{ 'struct': 'TestTypeA2',
+  'data': { 'integer': 'int' } }
+
+{ 'union': 'TestTypeA',
+  'base': { 'type': 'TestEnumA' },
+  'discriminator': 'type',
+  'data': { 'value-a1': 'TestTypeA1',
+            'value-a2': 'TestTypeA2' } }
+
+{ 'struct': 'TestTypeB',
+  'data': { 'integer': 'int' } }
+
+{ 'union': 'TestUnion',
+  'base': { 'type': 'TestEnum' },
+  'discriminator': 'type',
+  'data': { 'value-a': 'TestTypeA',
+            'value-b': 'TestTypeB' } }
diff --git a/tests/qapi-schema/union-invalid-union-subtype.out b/tests/qapi-schema/union-invalid-union-subtype.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/unit/test-qobject-input-visitor.c b/tests/unit/test-qobject-input-visitor.c
index 77fbf985be..9b3e2dbe14 100644
--- a/tests/unit/test-qobject-input-visitor.c
+++ b/tests/unit/test-qobject-input-visitor.c
@@ -706,6 +706,51 @@  static void test_visitor_in_union_flat(TestInputVisitorData *data,
     g_assert(&base->enum1 == &tmp->enum1);
 }
 
+static void test_visitor_in_union_in_union(TestInputVisitorData *data,
+                                           const void *unused)
+{
+    Visitor *v;
+    g_autoptr(TestUnionInUnion) tmp = NULL;
+
+    v = visitor_input_test_init(data,
+                                "{ 'type': 'value-a', "
+                                "  'type-a': 'value-a1', "
+                                "  'integer': 2, "
+                                "  'name': 'fish' }");
+
+    visit_type_TestUnionInUnion(v, NULL, &tmp, &error_abort);
+    g_assert_cmpint(tmp->type, ==, TEST_UNION_ENUM_VALUE_A);
+    g_assert_cmpint(tmp->u.value_a.type_a, ==, TEST_UNION_ENUMA_VALUE_A1);
+    g_assert_cmpint(tmp->u.value_a.u.value_a1.integer, ==, 2);
+    g_assert_cmpint(strcmp(tmp->u.value_a.u.value_a1.name, "fish"), ==, 0);
+
+    qapi_free_TestUnionInUnion(tmp);
+
+    v = visitor_input_test_init(data,
+                                "{ 'type': 'value-a', "
+                                "  'type-a': 'value-a2', "
+                                "  'integer': 1729, "
+                                "  'size': 87539319 }");
+
+    visit_type_TestUnionInUnion(v, NULL, &tmp, &error_abort);
+    g_assert_cmpint(tmp->type, ==, TEST_UNION_ENUM_VALUE_A);
+    g_assert_cmpint(tmp->u.value_a.type_a, ==, TEST_UNION_ENUMA_VALUE_A2);
+    g_assert_cmpint(tmp->u.value_a.u.value_a2.integer, ==, 1729);
+    g_assert_cmpint(tmp->u.value_a.u.value_a2.size, ==, 87539319);
+
+    qapi_free_TestUnionInUnion(tmp);
+
+    v = visitor_input_test_init(data,
+                                "{ 'type': 'value-b', "
+                                "  'integer': 1729, "
+                                "  'onoff': true }");
+
+    visit_type_TestUnionInUnion(v, NULL, &tmp, &error_abort);
+    g_assert_cmpint(tmp->type, ==, TEST_UNION_ENUM_VALUE_B);
+    g_assert_cmpint(tmp->u.value_b.integer, ==, 1729);
+    g_assert_cmpint(tmp->u.value_b.onoff, ==, true);
+}
+
 static void test_visitor_in_alternate(TestInputVisitorData *data,
                                       const void *unused)
 {
@@ -1216,6 +1261,8 @@  int main(int argc, char **argv)
                            NULL, test_visitor_in_null);
     input_visitor_test_add("/visitor/input/union-flat",
                            NULL, test_visitor_in_union_flat);
+    input_visitor_test_add("/visitor/input/union-in-union",
+                           NULL, test_visitor_in_union_in_union);
     input_visitor_test_add("/visitor/input/alternate",
                            NULL, test_visitor_in_alternate);
     input_visitor_test_add("/visitor/input/errors",
diff --git a/tests/unit/test-qobject-output-visitor.c b/tests/unit/test-qobject-output-visitor.c
index 7f054289fe..1535b3ad17 100644
--- a/tests/unit/test-qobject-output-visitor.c
+++ b/tests/unit/test-qobject-output-visitor.c
@@ -352,6 +352,62 @@  static void test_visitor_out_union_flat(TestOutputVisitorData *data,
     qapi_free_UserDefFlatUnion(tmp);
 }
 
+static void test_visitor_out_union_in_union(TestOutputVisitorData *data,
+                                            const void *unused)
+{
+    QDict *qdict;
+
+    TestUnionInUnion *tmp = g_new0(TestUnionInUnion, 1);
+    tmp->type = TEST_UNION_ENUM_VALUE_A;
+    tmp->u.value_a.type_a = TEST_UNION_ENUMA_VALUE_A1;
+    tmp->u.value_a.u.value_a1.integer = 42;
+    tmp->u.value_a.u.value_a1.name = g_strdup("fish");
+
+    visit_type_TestUnionInUnion(data->ov, NULL, &tmp, &error_abort);
+    qdict = qobject_to(QDict, visitor_get(data));
+    g_assert(qdict);
+    g_assert_cmpstr(qdict_get_str(qdict, "type"), ==, "value-a");
+    g_assert_cmpstr(qdict_get_str(qdict, "type-a"), ==, "value-a1");
+    g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 42);
+    g_assert_cmpstr(qdict_get_str(qdict, "name"), ==, "fish");
+
+    qapi_free_TestUnionInUnion(tmp);
+
+
+    visitor_reset(data);
+    tmp = g_new0(TestUnionInUnion, 1);
+    tmp->type = TEST_UNION_ENUM_VALUE_A;
+    tmp->u.value_a.type_a = TEST_UNION_ENUMA_VALUE_A2;
+    tmp->u.value_a.u.value_a2.integer = 1729;
+    tmp->u.value_a.u.value_a2.size = 87539319;
+
+    visit_type_TestUnionInUnion(data->ov, NULL, &tmp, &error_abort);
+    qdict = qobject_to(QDict, visitor_get(data));
+    g_assert(qdict);
+    g_assert_cmpstr(qdict_get_str(qdict, "type"), ==, "value-a");
+    g_assert_cmpstr(qdict_get_str(qdict, "type-a"), ==, "value-a2");
+    g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 1729);
+    g_assert_cmpint(qdict_get_int(qdict, "size"), ==, 87539319);
+
+    qapi_free_TestUnionInUnion(tmp);
+
+
+    visitor_reset(data);
+    tmp = g_new0(TestUnionInUnion, 1);
+    tmp->type = TEST_UNION_ENUM_VALUE_B;
+    tmp->u.value_b.integer = 1729;
+    tmp->u.value_b.onoff = true;
+
+    visit_type_TestUnionInUnion(data->ov, NULL, &tmp, &error_abort);
+    qdict = qobject_to(QDict, visitor_get(data));
+    g_assert(qdict);
+    g_assert_cmpstr(qdict_get_str(qdict, "type"), ==, "value-b");
+    g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 1729);
+    g_assert_cmpint(qdict_get_bool(qdict, "onoff"), ==, true);
+
+    qapi_free_TestUnionInUnion(tmp);
+}
+
 static void test_visitor_out_alternate(TestOutputVisitorData *data,
                                        const void *unused)
 {
@@ -586,6 +642,8 @@  int main(int argc, char **argv)
                             &out_visitor_data, test_visitor_out_list_qapi_free);
     output_visitor_test_add("/visitor/output/union-flat",
                             &out_visitor_data, test_visitor_out_union_flat);
+    output_visitor_test_add("/visitor/output/union-in-union",
+                            &out_visitor_data, test_visitor_out_union_in_union);
     output_visitor_test_add("/visitor/output/alternate",
                             &out_visitor_data, test_visitor_out_alternate);
     output_visitor_test_add("/visitor/output/null",