* [PATCH 01/22] qapi: Tidy up unusual line breaks
2021-09-13 12:39 [PATCH 00/22] qapi: Remove simple unions from the schema language Markus Armbruster
@ 2021-09-13 12:39 ` Markus Armbruster
2021-09-13 13:29 ` Marc-André Lureau
2021-09-13 12:39 ` [PATCH 02/22] qapi: Stop enforcing "type name should not end in 'Kind' Markus Armbruster
` (21 subsequent siblings)
22 siblings, 1 reply; 54+ messages in thread
From: Markus Armbruster @ 2021-09-13 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, eblake, marcandre.lureau
Break lines between members instead of within members.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
docs/devel/qapi-code-gen.rst | 12 +++++------
tests/qapi-schema/doc-good.json | 4 ++--
tests/qapi-schema/enum-if-invalid.json | 4 ++--
tests/qapi-schema/qapi-schema-test.json | 28 ++++++++++++-------------
4 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
index ced7a5ffe1..b154eae82e 100644
--- a/docs/devel/qapi-code-gen.rst
+++ b/docs/devel/qapi-code-gen.rst
@@ -859,9 +859,9 @@ longhand form of MEMBER.
Example: a struct type with unconditional member 'foo' and conditional
member 'bar' ::
- { 'struct': 'IfStruct', 'data':
- { 'foo': 'int',
- 'bar': { 'type': 'int', 'if': 'IFCOND'} } }
+ { 'struct': 'IfStruct',
+ 'data': { 'foo': 'int',
+ 'bar': { 'type': 'int', 'if': 'IFCOND'} } }
A union's discriminator may not be conditional.
@@ -871,9 +871,9 @@ the longhand form of ENUM-VALUE_.
Example: an enum type with unconditional value 'foo' and conditional
value 'bar' ::
- { 'enum': 'IfEnum', 'data':
- [ 'foo',
- { 'name' : 'bar', 'if': 'IFCOND' } ] }
+ { 'enum': 'IfEnum',
+ 'data': [ 'foo',
+ { 'name' : 'bar', 'if': 'IFCOND' } ] }
Likewise, features can be conditional. This requires the longhand
form of FEATURE_.
diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
index e0027e4cf6..cbf5c56c4b 100644
--- a/tests/qapi-schema/doc-good.json
+++ b/tests/qapi-schema/doc-good.json
@@ -60,8 +60,8 @@
#
# @two is undocumented
##
-{ 'enum': 'Enum', 'data':
- [ { 'name': 'one', 'if': 'IFONE' }, 'two' ],
+{ 'enum': 'Enum',
+ 'data': [ { 'name': 'one', 'if': 'IFONE' }, 'two' ],
'features': [ 'enum-feat' ],
'if': 'IFCOND' }
diff --git a/tests/qapi-schema/enum-if-invalid.json b/tests/qapi-schema/enum-if-invalid.json
index 60bd0ef1d7..6bd20041f3 100644
--- a/tests/qapi-schema/enum-if-invalid.json
+++ b/tests/qapi-schema/enum-if-invalid.json
@@ -1,3 +1,3 @@
# check invalid 'if' type
-{ 'enum': 'TestIfEnum', 'data':
- [ 'foo', { 'name' : 'bar', 'if': { 'val': 'foo' } } ] }
+{ 'enum': 'TestIfEnum',
+ 'data': [ 'foo', { 'name' : 'bar', 'if': { 'val': 'foo' } } ] }
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index b6c36a9eee..3c43e14e22 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -220,27 +220,27 @@
# test 'if' condition handling
-{ 'struct': 'TestIfStruct', 'data':
- { 'foo': 'int',
- 'bar': { 'type': 'int', 'if': 'TEST_IF_STRUCT_BAR'} },
+{ 'struct': 'TestIfStruct',
+ 'data': { 'foo': 'int',
+ 'bar': { 'type': 'int', 'if': 'TEST_IF_STRUCT_BAR'} },
'if': 'TEST_IF_STRUCT' }
-{ 'enum': 'TestIfEnum', 'data':
- [ 'foo', { 'name' : 'bar', 'if': 'TEST_IF_ENUM_BAR' } ],
+{ 'enum': 'TestIfEnum',
+ 'data': [ 'foo', { 'name' : 'bar', 'if': 'TEST_IF_ENUM_BAR' } ],
'if': 'TEST_IF_ENUM' }
-{ 'union': 'TestIfUnion', 'data':
- { 'foo': 'TestStruct',
- 'bar': { 'type': 'str', 'if': 'TEST_IF_UNION_BAR'} },
+{ 'union': 'TestIfUnion',
+ 'data': { 'foo': 'TestStruct',
+ 'bar': { 'type': 'str', 'if': 'TEST_IF_UNION_BAR'} },
'if': { 'all': ['TEST_IF_UNION', 'TEST_IF_STRUCT'] } }
{ 'command': 'test-if-union-cmd',
'data': { 'union-cmd-arg': 'TestIfUnion' },
'if': { 'all': ['TEST_IF_UNION', 'TEST_IF_STRUCT'] } }
-{ 'alternate': 'TestIfAlternate', 'data':
- { 'foo': 'int',
- 'bar': { 'type': 'TestStruct', 'if': 'TEST_IF_ALT_BAR'} },
+{ 'alternate': 'TestIfAlternate',
+ 'data': { 'foo': 'int',
+ 'bar': { 'type': 'TestStruct', 'if': 'TEST_IF_ALT_BAR'} },
'if': { 'all': ['TEST_IF_ALT', 'TEST_IF_STRUCT'] } }
{ 'command': 'test-if-alternate-cmd',
@@ -256,9 +256,9 @@
{ 'command': 'test-cmd-return-def-three', 'returns': 'UserDefThree' }
-{ 'event': 'TEST_IF_EVENT', 'data':
- { 'foo': 'TestIfStruct',
- 'bar': { 'type': ['TestIfEnum'], 'if': 'TEST_IF_EVT_BAR' } },
+{ 'event': 'TEST_IF_EVENT',
+ 'data': { 'foo': 'TestIfStruct',
+ 'bar': { 'type': ['TestIfEnum'], 'if': 'TEST_IF_EVT_BAR' } },
'if': { 'all': ['TEST_IF_EVT', 'TEST_IF_STRUCT'] } }
{ 'event': 'TEST_IF_EVENT2', 'data': {},
--
2.31.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH 01/22] qapi: Tidy up unusual line breaks
2021-09-13 12:39 ` [PATCH 01/22] qapi: Tidy up unusual line breaks Markus Armbruster
@ 2021-09-13 13:29 ` Marc-André Lureau
2021-09-13 13:54 ` Markus Armbruster
0 siblings, 1 reply; 54+ messages in thread
From: Marc-André Lureau @ 2021-09-13 13:29 UTC (permalink / raw)
To: Markus Armbruster; +Cc: John Snow, Blake, Eric, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 5133 bytes --]
Hi
On Mon, Sep 13, 2021 at 4:39 PM Markus Armbruster <armbru@redhat.com> wrote:
> Break lines between members instead of within members.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
I wish we would just automated tools to format files. With that git 2.23
feature, no more excuses :):
https://www.moxio.com/blog/43/ignoring-bulk-change-commits-with-git-blame
Sadly, our mixed json/py format doesn't get parsed after "blake" reformats
it (strings or trailing commas).
---
> docs/devel/qapi-code-gen.rst | 12 +++++------
> tests/qapi-schema/doc-good.json | 4 ++--
> tests/qapi-schema/enum-if-invalid.json | 4 ++--
> tests/qapi-schema/qapi-schema-test.json | 28 ++++++++++++-------------
> 4 files changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
> index ced7a5ffe1..b154eae82e 100644
> --- a/docs/devel/qapi-code-gen.rst
> +++ b/docs/devel/qapi-code-gen.rst
> @@ -859,9 +859,9 @@ longhand form of MEMBER.
> Example: a struct type with unconditional member 'foo' and conditional
> member 'bar' ::
>
> - { 'struct': 'IfStruct', 'data':
> - { 'foo': 'int',
> - 'bar': { 'type': 'int', 'if': 'IFCOND'} } }
> + { 'struct': 'IfStruct',
> + 'data': { 'foo': 'int',
> + 'bar': { 'type': 'int', 'if': 'IFCOND'} } }
>
> A union's discriminator may not be conditional.
>
> @@ -871,9 +871,9 @@ the longhand form of ENUM-VALUE_.
> Example: an enum type with unconditional value 'foo' and conditional
> value 'bar' ::
>
> - { 'enum': 'IfEnum', 'data':
> - [ 'foo',
> - { 'name' : 'bar', 'if': 'IFCOND' } ] }
> + { 'enum': 'IfEnum',
> + 'data': [ 'foo',
> + { 'name' : 'bar', 'if': 'IFCOND' } ] }
>
> Likewise, features can be conditional. This requires the longhand
> form of FEATURE_.
> diff --git a/tests/qapi-schema/doc-good.json
> b/tests/qapi-schema/doc-good.json
> index e0027e4cf6..cbf5c56c4b 100644
> --- a/tests/qapi-schema/doc-good.json
> +++ b/tests/qapi-schema/doc-good.json
> @@ -60,8 +60,8 @@
> #
> # @two is undocumented
> ##
> -{ 'enum': 'Enum', 'data':
> - [ { 'name': 'one', 'if': 'IFONE' }, 'two' ],
> +{ 'enum': 'Enum',
> + 'data': [ { 'name': 'one', 'if': 'IFONE' }, 'two' ],
> 'features': [ 'enum-feat' ],
> 'if': 'IFCOND' }
>
> diff --git a/tests/qapi-schema/enum-if-invalid.json
> b/tests/qapi-schema/enum-if-invalid.json
> index 60bd0ef1d7..6bd20041f3 100644
> --- a/tests/qapi-schema/enum-if-invalid.json
> +++ b/tests/qapi-schema/enum-if-invalid.json
> @@ -1,3 +1,3 @@
> # check invalid 'if' type
> -{ 'enum': 'TestIfEnum', 'data':
> - [ 'foo', { 'name' : 'bar', 'if': { 'val': 'foo' } } ] }
> +{ 'enum': 'TestIfEnum',
> + 'data': [ 'foo', { 'name' : 'bar', 'if': { 'val': 'foo' } } ] }
> diff --git a/tests/qapi-schema/qapi-schema-test.json
> b/tests/qapi-schema/qapi-schema-test.json
> index b6c36a9eee..3c43e14e22 100644
> --- a/tests/qapi-schema/qapi-schema-test.json
> +++ b/tests/qapi-schema/qapi-schema-test.json
> @@ -220,27 +220,27 @@
>
> # test 'if' condition handling
>
> -{ 'struct': 'TestIfStruct', 'data':
> - { 'foo': 'int',
> - 'bar': { 'type': 'int', 'if': 'TEST_IF_STRUCT_BAR'} },
> +{ 'struct': 'TestIfStruct',
> + 'data': { 'foo': 'int',
> + 'bar': { 'type': 'int', 'if': 'TEST_IF_STRUCT_BAR'} },
> 'if': 'TEST_IF_STRUCT' }
>
> -{ 'enum': 'TestIfEnum', 'data':
> - [ 'foo', { 'name' : 'bar', 'if': 'TEST_IF_ENUM_BAR' } ],
> +{ 'enum': 'TestIfEnum',
> + 'data': [ 'foo', { 'name' : 'bar', 'if': 'TEST_IF_ENUM_BAR' } ],
> 'if': 'TEST_IF_ENUM' }
>
> -{ 'union': 'TestIfUnion', 'data':
> - { 'foo': 'TestStruct',
> - 'bar': { 'type': 'str', 'if': 'TEST_IF_UNION_BAR'} },
> +{ 'union': 'TestIfUnion',
> + 'data': { 'foo': 'TestStruct',
> + 'bar': { 'type': 'str', 'if': 'TEST_IF_UNION_BAR'} },
> 'if': { 'all': ['TEST_IF_UNION', 'TEST_IF_STRUCT'] } }
>
> { 'command': 'test-if-union-cmd',
> 'data': { 'union-cmd-arg': 'TestIfUnion' },
> 'if': { 'all': ['TEST_IF_UNION', 'TEST_IF_STRUCT'] } }
>
> -{ 'alternate': 'TestIfAlternate', 'data':
> - { 'foo': 'int',
> - 'bar': { 'type': 'TestStruct', 'if': 'TEST_IF_ALT_BAR'} },
> +{ 'alternate': 'TestIfAlternate',
> + 'data': { 'foo': 'int',
> + 'bar': { 'type': 'TestStruct', 'if': 'TEST_IF_ALT_BAR'} },
> 'if': { 'all': ['TEST_IF_ALT', 'TEST_IF_STRUCT'] } }
>
> { 'command': 'test-if-alternate-cmd',
> @@ -256,9 +256,9 @@
>
> { 'command': 'test-cmd-return-def-three', 'returns': 'UserDefThree' }
>
> -{ 'event': 'TEST_IF_EVENT', 'data':
> - { 'foo': 'TestIfStruct',
> - 'bar': { 'type': ['TestIfEnum'], 'if': 'TEST_IF_EVT_BAR' } },
> +{ 'event': 'TEST_IF_EVENT',
> + 'data': { 'foo': 'TestIfStruct',
> + 'bar': { 'type': ['TestIfEnum'], 'if': 'TEST_IF_EVT_BAR' } },
> 'if': { 'all': ['TEST_IF_EVT', 'TEST_IF_STRUCT'] } }
>
> { 'event': 'TEST_IF_EVENT2', 'data': {},
> --
> 2.31.1
>
>
[-- Attachment #2: Type: text/html, Size: 7949 bytes --]
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [PATCH 01/22] qapi: Tidy up unusual line breaks
2021-09-13 13:29 ` Marc-André Lureau
@ 2021-09-13 13:54 ` Markus Armbruster
2021-09-13 13:59 ` Marc-André Lureau
0 siblings, 1 reply; 54+ messages in thread
From: Markus Armbruster @ 2021-09-13 13:54 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: John Snow, Blake, Eric, qemu-devel
Marc-André Lureau <marcandre.lureau@redhat.com> writes:
> Hi
>
> On Mon, Sep 13, 2021 at 4:39 PM Markus Armbruster <armbru@redhat.com> wrote:
>
>> Break lines between members instead of within members.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>
>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> I wish we would just automated tools to format files. With that git 2.23
> feature, no more excuses :):
> https://www.moxio.com/blog/43/ignoring-bulk-change-commits-with-git-blame
Interesting.
> Sadly, our mixed json/py format doesn't get parsed after "blake" reformats
> it (strings or trailing commas).
Naming QAPI schema files .json even though their contents isn't was a
mistake.
We discussed possible improvements in the thread below
Message-ID: <87ime52wxd.fsf@dusky.pond.sub.org>
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08325.html
Too much to read, but there's a summary:
Message-ID: <877dt5ofoi.fsf@dusky.pond.sub.org>
https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg02553.html
The least invasive way to achieve formatting automation could be
switching from bastardized JSON to proper subset of Python.
What's "blake"?
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [PATCH 01/22] qapi: Tidy up unusual line breaks
2021-09-13 13:54 ` Markus Armbruster
@ 2021-09-13 13:59 ` Marc-André Lureau
0 siblings, 0 replies; 54+ messages in thread
From: Marc-André Lureau @ 2021-09-13 13:59 UTC (permalink / raw)
To: Markus Armbruster; +Cc: John Snow, Blake, Eric, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1623 bytes --]
Hi
On Mon, Sep 13, 2021 at 5:55 PM Markus Armbruster <armbru@redhat.com> wrote:
> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>
> > Hi
> >
> > On Mon, Sep 13, 2021 at 4:39 PM Markus Armbruster <armbru@redhat.com>
> wrote:
> >
> >> Break lines between members instead of within members.
> >>
> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >>
> >
> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > I wish we would just automated tools to format files. With that git 2.23
> > feature, no more excuses :):
> >
> https://www.moxio.com/blog/43/ignoring-bulk-change-commits-with-git-blame
>
> Interesting.
>
Yeah, unfortunately you would have to add the commit rev after it's merged.
> > Sadly, our mixed json/py format doesn't get parsed after "blake"
> reformats
> > it (strings or trailing commas).
>
> Naming QAPI schema files .json even though their contents isn't was a
> mistake.
>
> We discussed possible improvements in the thread below
>
> Message-ID: <87ime52wxd.fsf@dusky.pond.sub.org>
> https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08325.html
>
> Too much to read, but there's a summary:
>
> Message-ID: <877dt5ofoi.fsf@dusky.pond.sub.org>
> https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg02553.html
>
> The least invasive way to achieve formatting automation could be
> switching from bastardized JSON to proper subset of Python.
>
ok
> What's "blake"?
>
Sorry, don't know why I always remember "blake" when it is simply "black" (
https://github.com/psf/black)
[-- Attachment #2: Type: text/html, Size: 3290 bytes --]
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH 02/22] qapi: Stop enforcing "type name should not end in 'Kind'
2021-09-13 12:39 [PATCH 00/22] qapi: Remove simple unions from the schema language Markus Armbruster
2021-09-13 12:39 ` [PATCH 01/22] qapi: Tidy up unusual line breaks Markus Armbruster
@ 2021-09-13 12:39 ` Markus Armbruster
2021-09-13 14:40 ` Eric Blake
2021-09-13 12:39 ` [PATCH 03/22] qapi: Convert simple union KeyValue to flat one Markus Armbruster
` (20 subsequent siblings)
22 siblings, 1 reply; 54+ messages in thread
From: Markus Armbruster @ 2021-09-13 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, eblake, marcandre.lureau
I'm about to convert simple unions to flat unions, then drop simple
union support. The conversion involves making the implict enum types
explicit. To reduce churn, I'd like to name them exactly like the
implicit types they replace. However, these names are reserved for
the generator's use. They won't be once simple unions are gone. Stop
enforcing this naming rule now rather than then.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
scripts/qapi/expr.py | 6 +++---
tests/qapi-schema/meson.build | 1 -
tests/qapi-schema/reserved-type-kind.err | 2 --
tests/qapi-schema/reserved-type-kind.json | 2 --
tests/qapi-schema/reserved-type-kind.out | 0
5 files changed, 3 insertions(+), 8 deletions(-)
delete mode 100644 tests/qapi-schema/reserved-type-kind.err
delete mode 100644 tests/qapi-schema/reserved-type-kind.json
delete mode 100644 tests/qapi-schema/reserved-type-kind.out
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 90bde501b0..91959ee79a 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -171,7 +171,7 @@ def check_defn_name_str(name: str, info: QAPISourceInfo, meta: str) -> None:
- 'event' names adhere to `check_name_upper()`.
- 'command' names adhere to `check_name_lower()`.
- Else, meta is a type, and must pass `check_name_camel()`.
- These names must not end with ``Kind`` nor ``List``.
+ These names must not end with ``List``.
:param name: Name to check.
:param info: QAPI schema source file information.
@@ -187,9 +187,9 @@ def check_defn_name_str(name: str, info: QAPISourceInfo, meta: str) -> None:
permit_underscore=name in info.pragma.command_name_exceptions)
else:
check_name_camel(name, info, meta)
- if name.endswith('Kind') or name.endswith('List'):
+ if name.endswith('List'):
raise QAPISemError(
- info, "%s name should not end in '%s'" % (meta, name[-4:]))
+ info, "%s name should not end in 'List'" % meta)
def check_keys(value: _JSONObject,
diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
index 6b2a4ce41a..0798e94042 100644
--- a/tests/qapi-schema/meson.build
+++ b/tests/qapi-schema/meson.build
@@ -168,7 +168,6 @@ schemas = [
'reserved-member-q.json',
'reserved-member-u.json',
'reserved-member-underscore.json',
- 'reserved-type-kind.json',
'reserved-type-list.json',
'returns-alternate.json',
'returns-array-bad.json',
diff --git a/tests/qapi-schema/reserved-type-kind.err b/tests/qapi-schema/reserved-type-kind.err
deleted file mode 100644
index d8fb769f9d..0000000000
--- a/tests/qapi-schema/reserved-type-kind.err
+++ /dev/null
@@ -1,2 +0,0 @@
-reserved-type-kind.json: In enum 'UnionKind':
-reserved-type-kind.json:2: enum name should not end in 'Kind'
diff --git a/tests/qapi-schema/reserved-type-kind.json b/tests/qapi-schema/reserved-type-kind.json
deleted file mode 100644
index 9ecaba12bc..0000000000
--- a/tests/qapi-schema/reserved-type-kind.json
+++ /dev/null
@@ -1,2 +0,0 @@
-# we reject types that would conflict with implicit union enum
-{ 'enum': 'UnionKind', 'data': [ 'oops' ] }
diff --git a/tests/qapi-schema/reserved-type-kind.out b/tests/qapi-schema/reserved-type-kind.out
deleted file mode 100644
index e69de29bb2..0000000000
--
2.31.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH 02/22] qapi: Stop enforcing "type name should not end in 'Kind'
2021-09-13 12:39 ` [PATCH 02/22] qapi: Stop enforcing "type name should not end in 'Kind' Markus Armbruster
@ 2021-09-13 14:40 ` Eric Blake
0 siblings, 0 replies; 54+ messages in thread
From: Eric Blake @ 2021-09-13 14:40 UTC (permalink / raw)
To: Markus Armbruster; +Cc: marcandre.lureau, jsnow, qemu-devel
On Mon, Sep 13, 2021 at 02:39:12PM +0200, Markus Armbruster wrote:
> I'm about to convert simple unions to flat unions, then drop simple
> union support. The conversion involves making the implict enum types
> explicit. To reduce churn, I'd like to name them exactly like the
> implicit types they replace. However, these names are reserved for
> the generator's use. They won't be once simple unions are gone. Stop
> enforcing this naming rule now rather than then.
Sounds like a good plan!
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> scripts/qapi/expr.py | 6 +++---
> tests/qapi-schema/meson.build | 1 -
> tests/qapi-schema/reserved-type-kind.err | 2 --
> tests/qapi-schema/reserved-type-kind.json | 2 --
> tests/qapi-schema/reserved-type-kind.out | 0
> 5 files changed, 3 insertions(+), 8 deletions(-)
> delete mode 100644 tests/qapi-schema/reserved-type-kind.err
> delete mode 100644 tests/qapi-schema/reserved-type-kind.json
> delete mode 100644 tests/qapi-schema/reserved-type-kind.out
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH 03/22] qapi: Convert simple union KeyValue to flat one
2021-09-13 12:39 [PATCH 00/22] qapi: Remove simple unions from the schema language Markus Armbruster
2021-09-13 12:39 ` [PATCH 01/22] qapi: Tidy up unusual line breaks Markus Armbruster
2021-09-13 12:39 ` [PATCH 02/22] qapi: Stop enforcing "type name should not end in 'Kind' Markus Armbruster
@ 2021-09-13 12:39 ` Markus Armbruster
2021-09-13 14:45 ` Eric Blake
2021-09-14 7:15 ` Gerd Hoffmann
2021-09-13 12:39 ` [PATCH 04/22] qapi: Convert simple union InputEvent " Markus Armbruster
` (19 subsequent siblings)
22 siblings, 2 replies; 54+ messages in thread
From: Markus Armbruster @ 2021-09-13 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, eblake, Gerd Hoffmann, marcandre.lureau
Simple unions predate flat unions. Having both complicates the QAPI
schema language and the QAPI generator. We haven't been using simple
unions in new code for a long time, because they are less flexible and
somewhat awkward on the wire.
To prepare for their removal, convert simple union KeyValue to an
equivalent flat one. Adds some boilerplate to the schema, which is a
bit ugly, but a lot easier to maintain than the simple union feature.
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qapi/ui.json | 30 ++++++++++++++++++++++++++++--
1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/qapi/ui.json b/qapi/ui.json
index b2cf7a6759..a6b0dce876 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -824,6 +824,30 @@
'ac_home', 'ac_back', 'ac_forward', 'ac_refresh', 'ac_bookmarks',
'lang1', 'lang2' ] }
+##
+# @KeyValueKind:
+#
+# Since: 6.1
+##
+{ 'enum': 'KeyValueKind',
+ 'data': [ 'number', 'qcode' ] }
+
+##
+# @IntWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'IntWrapper',
+ 'data': { 'data': 'int' } }
+
+##
+# @QKeyCodeWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'QKeyCodeWrapper',
+ 'data': { 'data': 'QKeyCode' } }
+
##
# @KeyValue:
#
@@ -832,9 +856,11 @@
# Since: 1.3
##
{ 'union': 'KeyValue',
+ 'base': { 'type': 'KeyValueKind' },
+ 'discriminator': 'type',
'data': {
- 'number': 'int',
- 'qcode': 'QKeyCode' } }
+ 'number': 'IntWrapper',
+ 'qcode': 'QKeyCodeWrapper' } }
##
# @send-key:
--
2.31.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH 03/22] qapi: Convert simple union KeyValue to flat one
2021-09-13 12:39 ` [PATCH 03/22] qapi: Convert simple union KeyValue to flat one Markus Armbruster
@ 2021-09-13 14:45 ` Eric Blake
2021-09-14 4:54 ` Markus Armbruster
2021-09-14 7:15 ` Gerd Hoffmann
1 sibling, 1 reply; 54+ messages in thread
From: Eric Blake @ 2021-09-13 14:45 UTC (permalink / raw)
To: Markus Armbruster; +Cc: marcandre.lureau, jsnow, qemu-devel, Gerd Hoffmann
On Mon, Sep 13, 2021 at 02:39:13PM +0200, Markus Armbruster wrote:
> Simple unions predate flat unions. Having both complicates the QAPI
> schema language and the QAPI generator. We haven't been using simple
> unions in new code for a long time, because they are less flexible and
> somewhat awkward on the wire.
>
> To prepare for their removal, convert simple union KeyValue to an
> equivalent flat one. Adds some boilerplate to the schema, which is a
> bit ugly, but a lot easier to maintain than the simple union feature.
>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> qapi/ui.json | 30 ++++++++++++++++++++++++++++--
> 1 file changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/qapi/ui.json b/qapi/ui.json
> index b2cf7a6759..a6b0dce876 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -824,6 +824,30 @@
> 'ac_home', 'ac_back', 'ac_forward', 'ac_refresh', 'ac_bookmarks',
> 'lang1', 'lang2' ] }
>
> +##
> +# @KeyValueKind:
> +#
> +# Since: 6.1
6.2 now? Or should this be...
> +
> ##
> # @KeyValue:
> #
> @@ -832,9 +856,11 @@
> # Since: 1.3
...1.3, since the type has been around by that name already (albeit
implicitly) since that older release?
> ##
> { 'union': 'KeyValue',
> + 'base': { 'type': 'KeyValueKind' },
> + 'discriminator': 'type',
> 'data': {
> - 'number': 'int',
> - 'qcode': 'QKeyCode' } }
> + 'number': 'IntWrapper',
> + 'qcode': 'QKeyCodeWrapper' } }
>
I'll trust your decision on the documentation issue; the conversion
itself is sane, so I'm fine with:
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [PATCH 03/22] qapi: Convert simple union KeyValue to flat one
2021-09-13 14:45 ` Eric Blake
@ 2021-09-14 4:54 ` Markus Armbruster
0 siblings, 0 replies; 54+ messages in thread
From: Markus Armbruster @ 2021-09-14 4:54 UTC (permalink / raw)
To: Eric Blake
Cc: marcandre.lureau, Gerd Hoffmann, jsnow, Markus Armbruster, qemu-devel
Eric Blake <eblake@redhat.com> writes:
> On Mon, Sep 13, 2021 at 02:39:13PM +0200, Markus Armbruster wrote:
>> Simple unions predate flat unions. Having both complicates the QAPI
>> schema language and the QAPI generator. We haven't been using simple
>> unions in new code for a long time, because they are less flexible and
>> somewhat awkward on the wire.
>>
>> To prepare for their removal, convert simple union KeyValue to an
>> equivalent flat one. Adds some boilerplate to the schema, which is a
>> bit ugly, but a lot easier to maintain than the simple union feature.
>>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> qapi/ui.json | 30 ++++++++++++++++++++++++++++--
>> 1 file changed, 28 insertions(+), 2 deletions(-)
>>
>> diff --git a/qapi/ui.json b/qapi/ui.json
>> index b2cf7a6759..a6b0dce876 100644
>> --- a/qapi/ui.json
>> +++ b/qapi/ui.json
>> @@ -824,6 +824,30 @@
>> 'ac_home', 'ac_back', 'ac_forward', 'ac_refresh', 'ac_bookmarks',
>> 'lang1', 'lang2' ] }
>>
>> +##
>> +# @KeyValueKind:
>> +#
>> +# Since: 6.1
>
> 6.2 now? Or should this be...
Yes. Can't count :)
>> +
>> ##
>> # @KeyValue:
>> #
>> @@ -832,9 +856,11 @@
>> # Since: 1.3
>
> ...1.3, since the type has been around by that name already (albeit
> implicitly) since that older release?
I'll change it to 1.3.
My first version had KeyValueType here. Then I found the renaming of
its uses in C code tedious, and realized I could avoid it by unreserving
*Kind type names early. I forgot to adjust the Since tag.
>> ##
>> { 'union': 'KeyValue',
>> + 'base': { 'type': 'KeyValueKind' },
>> + 'discriminator': 'type',
>> 'data': {
>> - 'number': 'int',
>> - 'qcode': 'QKeyCode' } }
>> + 'number': 'IntWrapper',
>> + 'qcode': 'QKeyCodeWrapper' } }
>>
>
> I'll trust your decision on the documentation issue; the conversion
> itself is sane, so I'm fine with:
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
Thanks!
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 03/22] qapi: Convert simple union KeyValue to flat one
2021-09-13 12:39 ` [PATCH 03/22] qapi: Convert simple union KeyValue to flat one Markus Armbruster
2021-09-13 14:45 ` Eric Blake
@ 2021-09-14 7:15 ` Gerd Hoffmann
1 sibling, 0 replies; 54+ messages in thread
From: Gerd Hoffmann @ 2021-09-14 7:15 UTC (permalink / raw)
To: Markus Armbruster; +Cc: jsnow, eblake, qemu-devel, marcandre.lureau
On Mon, Sep 13, 2021 at 02:39:13PM +0200, Markus Armbruster wrote:
> Simple unions predate flat unions. Having both complicates the QAPI
> schema language and the QAPI generator. We haven't been using simple
> unions in new code for a long time, because they are less flexible and
> somewhat awkward on the wire.
>
> To prepare for their removal, convert simple union KeyValue to an
> equivalent flat one. Adds some boilerplate to the schema, which is a
> bit ugly, but a lot easier to maintain than the simple union feature.
>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH 04/22] qapi: Convert simple union InputEvent to flat one
2021-09-13 12:39 [PATCH 00/22] qapi: Remove simple unions from the schema language Markus Armbruster
` (2 preceding siblings ...)
2021-09-13 12:39 ` [PATCH 03/22] qapi: Convert simple union KeyValue to flat one Markus Armbruster
@ 2021-09-13 12:39 ` Markus Armbruster
2021-09-13 14:46 ` Eric Blake
2021-09-14 7:15 ` Gerd Hoffmann
2021-09-13 12:39 ` [PATCH 05/22] qapi: Convert simple union TpmTypeOptions " Markus Armbruster
` (18 subsequent siblings)
22 siblings, 2 replies; 54+ messages in thread
From: Markus Armbruster @ 2021-09-13 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, eblake, Gerd Hoffmann, marcandre.lureau
Simple unions predate flat unions. Having both complicates the QAPI
schema language and the QAPI generator. We haven't been using simple
unions in new code for a long time, because they are less flexible and
somewhat awkward on the wire.
To prepare for their removal, convert simple union InputEvent to an
equivalent flat one. Adds some boilerplate to the schema, which is a
bit ugly, but a lot easier to maintain than the simple union feature.
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qapi/ui.json | 42 ++++++++++++++++++++++++++++++++++++++----
1 file changed, 38 insertions(+), 4 deletions(-)
diff --git a/qapi/ui.json b/qapi/ui.json
index a6b0dce876..fe10d69431 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -960,6 +960,38 @@
'data' : { 'axis' : 'InputAxis',
'value' : 'int' } }
+##
+# @InputEventKind:
+#
+# Since: 6.1
+##
+{ 'enum': 'InputEventKind',
+ 'data': [ 'key', 'btn', 'rel', 'abs' ] }
+
+##
+# @InputKeyEventWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'InputKeyEventWrapper',
+ 'data': { 'data': 'InputKeyEvent' } }
+
+##
+# @InputBtnEventWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'InputBtnEventWrapper',
+ 'data': { 'data': 'InputBtnEvent' } }
+
+##
+# @InputMoveEventWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'InputMoveEventWrapper',
+ 'data': { 'data': 'InputMoveEvent' } }
+
##
# @InputEvent:
#
@@ -975,10 +1007,12 @@
# Since: 2.0
##
{ 'union' : 'InputEvent',
- 'data' : { 'key' : 'InputKeyEvent',
- 'btn' : 'InputBtnEvent',
- 'rel' : 'InputMoveEvent',
- 'abs' : 'InputMoveEvent' } }
+ 'base': { 'type': 'InputEventKind' },
+ 'discriminator': 'type',
+ 'data' : { 'key' : 'InputKeyEventWrapper',
+ 'btn' : 'InputBtnEventWrapper',
+ 'rel' : 'InputMoveEventWrapper',
+ 'abs' : 'InputMoveEventWrapper' } }
##
# @input-send-event:
--
2.31.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH 04/22] qapi: Convert simple union InputEvent to flat one
2021-09-13 12:39 ` [PATCH 04/22] qapi: Convert simple union InputEvent " Markus Armbruster
@ 2021-09-13 14:46 ` Eric Blake
2021-09-14 4:55 ` Markus Armbruster
2021-09-14 7:15 ` Gerd Hoffmann
1 sibling, 1 reply; 54+ messages in thread
From: Eric Blake @ 2021-09-13 14:46 UTC (permalink / raw)
To: Markus Armbruster; +Cc: marcandre.lureau, jsnow, qemu-devel, Gerd Hoffmann
On Mon, Sep 13, 2021 at 02:39:14PM +0200, Markus Armbruster wrote:
> Simple unions predate flat unions. Having both complicates the QAPI
> schema language and the QAPI generator. We haven't been using simple
> unions in new code for a long time, because they are less flexible and
> somewhat awkward on the wire.
>
> To prepare for their removal, convert simple union InputEvent to an
> equivalent flat one. Adds some boilerplate to the schema, which is a
> bit ugly, but a lot easier to maintain than the simple union feature.
>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> qapi/ui.json | 42 ++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 38 insertions(+), 4 deletions(-)
Same question as in 3/22:
>
> diff --git a/qapi/ui.json b/qapi/ui.json
> index a6b0dce876..fe10d69431 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -960,6 +960,38 @@
> 'data' : { 'axis' : 'InputAxis',
> 'value' : 'int' } }
>
> +##
> +# @InputEventKind:
> +#
> +# Since: 6.1
This should either be 6.2, or...
> ##
> # @InputEvent:
> #
> @@ -975,10 +1007,12 @@
> # Since: 2.0
...2.0.
> ##
> { 'union' : 'InputEvent',
> - 'data' : { 'key' : 'InputKeyEvent',
> - 'btn' : 'InputBtnEvent',
> - 'rel' : 'InputMoveEvent',
> - 'abs' : 'InputMoveEvent' } }
> + 'base': { 'type': 'InputEventKind' },
> + 'discriminator': 'type',
> + 'data' : { 'key' : 'InputKeyEventWrapper',
> + 'btn' : 'InputBtnEventWrapper',
> + 'rel' : 'InputMoveEventWrapper',
> + 'abs' : 'InputMoveEventWrapper' } }
But as with that patch, I trust your decision on docs, and the
conversion itself is sane.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [PATCH 04/22] qapi: Convert simple union InputEvent to flat one
2021-09-13 14:46 ` Eric Blake
@ 2021-09-14 4:55 ` Markus Armbruster
0 siblings, 0 replies; 54+ messages in thread
From: Markus Armbruster @ 2021-09-14 4:55 UTC (permalink / raw)
To: Eric Blake
Cc: marcandre.lureau, Gerd Hoffmann, jsnow, Markus Armbruster, qemu-devel
Eric Blake <eblake@redhat.com> writes:
> On Mon, Sep 13, 2021 at 02:39:14PM +0200, Markus Armbruster wrote:
>> Simple unions predate flat unions. Having both complicates the QAPI
>> schema language and the QAPI generator. We haven't been using simple
>> unions in new code for a long time, because they are less flexible and
>> somewhat awkward on the wire.
>>
>> To prepare for their removal, convert simple union InputEvent to an
>> equivalent flat one. Adds some boilerplate to the schema, which is a
>> bit ugly, but a lot easier to maintain than the simple union feature.
>>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> qapi/ui.json | 42 ++++++++++++++++++++++++++++++++++++++----
>> 1 file changed, 38 insertions(+), 4 deletions(-)
>
> Same question as in 3/22:
>
>>
>> diff --git a/qapi/ui.json b/qapi/ui.json
>> index a6b0dce876..fe10d69431 100644
>> --- a/qapi/ui.json
>> +++ b/qapi/ui.json
>> @@ -960,6 +960,38 @@
>> 'data' : { 'axis' : 'InputAxis',
>> 'value' : 'int' } }
>>
>> +##
>> +# @InputEventKind:
>> +#
>> +# Since: 6.1
>
> This should either be 6.2, or...
>
>> ##
>> # @InputEvent:
>> #
>> @@ -975,10 +1007,12 @@
>> # Since: 2.0
>
> ...2.0.
Same answer: 2.0.
>> ##
>> { 'union' : 'InputEvent',
>> - 'data' : { 'key' : 'InputKeyEvent',
>> - 'btn' : 'InputBtnEvent',
>> - 'rel' : 'InputMoveEvent',
>> - 'abs' : 'InputMoveEvent' } }
>> + 'base': { 'type': 'InputEventKind' },
>> + 'discriminator': 'type',
>> + 'data' : { 'key' : 'InputKeyEventWrapper',
>> + 'btn' : 'InputBtnEventWrapper',
>> + 'rel' : 'InputMoveEventWrapper',
>> + 'abs' : 'InputMoveEventWrapper' } }
>
> But as with that patch, I trust your decision on docs, and the
> conversion itself is sane.
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
Thanks!
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 04/22] qapi: Convert simple union InputEvent to flat one
2021-09-13 12:39 ` [PATCH 04/22] qapi: Convert simple union InputEvent " Markus Armbruster
2021-09-13 14:46 ` Eric Blake
@ 2021-09-14 7:15 ` Gerd Hoffmann
1 sibling, 0 replies; 54+ messages in thread
From: Gerd Hoffmann @ 2021-09-14 7:15 UTC (permalink / raw)
To: Markus Armbruster; +Cc: jsnow, eblake, qemu-devel, marcandre.lureau
On Mon, Sep 13, 2021 at 02:39:14PM +0200, Markus Armbruster wrote:
> Simple unions predate flat unions. Having both complicates the QAPI
> schema language and the QAPI generator. We haven't been using simple
> unions in new code for a long time, because they are less flexible and
> somewhat awkward on the wire.
>
> To prepare for their removal, convert simple union InputEvent to an
> equivalent flat one. Adds some boilerplate to the schema, which is a
> bit ugly, but a lot easier to maintain than the simple union feature.
>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH 05/22] qapi: Convert simple union TpmTypeOptions to flat one
2021-09-13 12:39 [PATCH 00/22] qapi: Remove simple unions from the schema language Markus Armbruster
` (3 preceding siblings ...)
2021-09-13 12:39 ` [PATCH 04/22] qapi: Convert simple union InputEvent " Markus Armbruster
@ 2021-09-13 12:39 ` Markus Armbruster
2021-09-13 16:32 ` Stefan Berger
2021-09-13 12:39 ` [PATCH 06/22] qapi: Convert simple union MemoryDeviceInfo " Markus Armbruster
` (17 subsequent siblings)
22 siblings, 1 reply; 54+ messages in thread
From: Markus Armbruster @ 2021-09-13 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, Stefan Berger, eblake, marcandre.lureau
Simple unions predate flat unions. Having both complicates the QAPI
schema language and the QAPI generator. We haven't been using simple
unions in new code for a long time, because they are less flexible and
somewhat awkward on the wire.
To prepare for their removal, convert simple union TpmTypeOptions to
an equivalent flat one, with existing enum TpmType replacing implicit
enum TpmTypeOptionsKind. Adds some boilerplate to the schema, which
is a bit ugly, but a lot easier to maintain than the simple union
feature.
Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qapi/tpm.json | 24 ++++++++++++++++++++++--
backends/tpm/tpm_emulator.c | 2 +-
backends/tpm/tpm_passthrough.c | 2 +-
monitor/hmp-cmds.c | 8 ++++----
4 files changed, 28 insertions(+), 8 deletions(-)
diff --git a/qapi/tpm.json b/qapi/tpm.json
index f4dde2f646..b3ade008bf 100644
--- a/qapi/tpm.json
+++ b/qapi/tpm.json
@@ -99,6 +99,24 @@
{ 'struct': 'TPMEmulatorOptions', 'data': { 'chardev' : 'str' },
'if': 'CONFIG_TPM' }
+##
+# @TPMPassthroughOptionsWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'TPMPassthroughOptionsWrapper',
+ 'data': { 'data': 'TPMPassthroughOptions' },
+ 'if': 'CONFIG_TPM' }
+
+##
+# @TPMEmulatorOptionsWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'TPMEmulatorOptionsWrapper',
+ 'data': { 'data': 'TPMEmulatorOptions' },
+ 'if': 'CONFIG_TPM' }
+
##
# @TpmTypeOptions:
#
@@ -110,8 +128,10 @@
# Since: 1.5
##
{ 'union': 'TpmTypeOptions',
- 'data': { 'passthrough' : 'TPMPassthroughOptions',
- 'emulator': 'TPMEmulatorOptions' },
+ 'base': { 'type': 'TpmType' },
+ 'discriminator': 'type',
+ 'data': { 'passthrough' : 'TPMPassthroughOptionsWrapper',
+ 'emulator': 'TPMEmulatorOptionsWrapper' },
'if': 'CONFIG_TPM' }
##
diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
index f8095d23d5..87d061e9bb 100644
--- a/backends/tpm/tpm_emulator.c
+++ b/backends/tpm/tpm_emulator.c
@@ -623,7 +623,7 @@ static TpmTypeOptions *tpm_emulator_get_tpm_options(TPMBackend *tb)
TPMEmulator *tpm_emu = TPM_EMULATOR(tb);
TpmTypeOptions *options = g_new0(TpmTypeOptions, 1);
- options->type = TPM_TYPE_OPTIONS_KIND_EMULATOR;
+ options->type = TPM_TYPE_EMULATOR;
options->u.emulator.data = QAPI_CLONE(TPMEmulatorOptions, tpm_emu->options);
return options;
diff --git a/backends/tpm/tpm_passthrough.c b/backends/tpm/tpm_passthrough.c
index 21b7459183..d5558fae6c 100644
--- a/backends/tpm/tpm_passthrough.c
+++ b/backends/tpm/tpm_passthrough.c
@@ -321,7 +321,7 @@ static TpmTypeOptions *tpm_passthrough_get_tpm_options(TPMBackend *tb)
{
TpmTypeOptions *options = g_new0(TpmTypeOptions, 1);
- options->type = TPM_TYPE_OPTIONS_KIND_PASSTHROUGH;
+ options->type = TPM_TYPE_PASSTHROUGH;
options->u.passthrough.data = QAPI_CLONE(TPMPassthroughOptions,
TPM_PASSTHROUGH(tb)->options);
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index e00255f7ee..d6858407ad 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -925,10 +925,10 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
c, TpmModel_str(ti->model));
monitor_printf(mon, " \\ %s: type=%s",
- ti->id, TpmTypeOptionsKind_str(ti->options->type));
+ ti->id, TpmType_str(ti->options->type));
switch (ti->options->type) {
- case TPM_TYPE_OPTIONS_KIND_PASSTHROUGH:
+ case TPM_TYPE_PASSTHROUGH:
tpo = ti->options->u.passthrough.data;
monitor_printf(mon, "%s%s%s%s",
tpo->has_path ? ",path=" : "",
@@ -936,11 +936,11 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
tpo->has_cancel_path ? ",cancel-path=" : "",
tpo->has_cancel_path ? tpo->cancel_path : "");
break;
- case TPM_TYPE_OPTIONS_KIND_EMULATOR:
+ case TPM_TYPE_EMULATOR:
teo = ti->options->u.emulator.data;
monitor_printf(mon, ",chardev=%s", teo->chardev);
break;
- case TPM_TYPE_OPTIONS_KIND__MAX:
+ case TPM_TYPE__MAX:
break;
}
monitor_printf(mon, "\n");
--
2.31.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH 05/22] qapi: Convert simple union TpmTypeOptions to flat one
2021-09-13 12:39 ` [PATCH 05/22] qapi: Convert simple union TpmTypeOptions " Markus Armbruster
@ 2021-09-13 16:32 ` Stefan Berger
0 siblings, 0 replies; 54+ messages in thread
From: Stefan Berger @ 2021-09-13 16:32 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel
Cc: jsnow, Stefan Berger, eblake, marcandre.lureau
On 9/13/21 8:39 AM, Markus Armbruster wrote:
> Simple unions predate flat unions. Having both complicates the QAPI
> schema language and the QAPI generator. We haven't been using simple
> unions in new code for a long time, because they are less flexible and
> somewhat awkward on the wire.
>
> To prepare for their removal, convert simple union TpmTypeOptions to
> an equivalent flat one, with existing enum TpmType replacing implicit
> enum TpmTypeOptionsKind. Adds some boilerplate to the schema, which
> is a bit ugly, but a lot easier to maintain than the simple union
> feature.
>
> Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Acked-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
> qapi/tpm.json | 24 ++++++++++++++++++++++--
> backends/tpm/tpm_emulator.c | 2 +-
> backends/tpm/tpm_passthrough.c | 2 +-
> monitor/hmp-cmds.c | 8 ++++----
> 4 files changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/qapi/tpm.json b/qapi/tpm.json
> index f4dde2f646..b3ade008bf 100644
> --- a/qapi/tpm.json
> +++ b/qapi/tpm.json
> @@ -99,6 +99,24 @@
> { 'struct': 'TPMEmulatorOptions', 'data': { 'chardev' : 'str' },
> 'if': 'CONFIG_TPM' }
>
> +##
> +# @TPMPassthroughOptionsWrapper:
> +#
> +# Since: 6.1
> +##
> +{ 'struct': 'TPMPassthroughOptionsWrapper',
> + 'data': { 'data': 'TPMPassthroughOptions' },
> + 'if': 'CONFIG_TPM' }
> +
> +##
> +# @TPMEmulatorOptionsWrapper:
> +#
> +# Since: 6.1
> +##
> +{ 'struct': 'TPMEmulatorOptionsWrapper',
> + 'data': { 'data': 'TPMEmulatorOptions' },
> + 'if': 'CONFIG_TPM' }
> +
> ##
> # @TpmTypeOptions:
> #
> @@ -110,8 +128,10 @@
> # Since: 1.5
> ##
> { 'union': 'TpmTypeOptions',
> - 'data': { 'passthrough' : 'TPMPassthroughOptions',
> - 'emulator': 'TPMEmulatorOptions' },
> + 'base': { 'type': 'TpmType' },
> + 'discriminator': 'type',
> + 'data': { 'passthrough' : 'TPMPassthroughOptionsWrapper',
> + 'emulator': 'TPMEmulatorOptionsWrapper' },
> 'if': 'CONFIG_TPM' }
>
> ##
> diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
> index f8095d23d5..87d061e9bb 100644
> --- a/backends/tpm/tpm_emulator.c
> +++ b/backends/tpm/tpm_emulator.c
> @@ -623,7 +623,7 @@ static TpmTypeOptions *tpm_emulator_get_tpm_options(TPMBackend *tb)
> TPMEmulator *tpm_emu = TPM_EMULATOR(tb);
> TpmTypeOptions *options = g_new0(TpmTypeOptions, 1);
>
> - options->type = TPM_TYPE_OPTIONS_KIND_EMULATOR;
> + options->type = TPM_TYPE_EMULATOR;
> options->u.emulator.data = QAPI_CLONE(TPMEmulatorOptions, tpm_emu->options);
>
> return options;
> diff --git a/backends/tpm/tpm_passthrough.c b/backends/tpm/tpm_passthrough.c
> index 21b7459183..d5558fae6c 100644
> --- a/backends/tpm/tpm_passthrough.c
> +++ b/backends/tpm/tpm_passthrough.c
> @@ -321,7 +321,7 @@ static TpmTypeOptions *tpm_passthrough_get_tpm_options(TPMBackend *tb)
> {
> TpmTypeOptions *options = g_new0(TpmTypeOptions, 1);
>
> - options->type = TPM_TYPE_OPTIONS_KIND_PASSTHROUGH;
> + options->type = TPM_TYPE_PASSTHROUGH;
> options->u.passthrough.data = QAPI_CLONE(TPMPassthroughOptions,
> TPM_PASSTHROUGH(tb)->options);
>
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index e00255f7ee..d6858407ad 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -925,10 +925,10 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
> c, TpmModel_str(ti->model));
>
> monitor_printf(mon, " \\ %s: type=%s",
> - ti->id, TpmTypeOptionsKind_str(ti->options->type));
> + ti->id, TpmType_str(ti->options->type));
>
> switch (ti->options->type) {
> - case TPM_TYPE_OPTIONS_KIND_PASSTHROUGH:
> + case TPM_TYPE_PASSTHROUGH:
> tpo = ti->options->u.passthrough.data;
> monitor_printf(mon, "%s%s%s%s",
> tpo->has_path ? ",path=" : "",
> @@ -936,11 +936,11 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
> tpo->has_cancel_path ? ",cancel-path=" : "",
> tpo->has_cancel_path ? tpo->cancel_path : "");
> break;
> - case TPM_TYPE_OPTIONS_KIND_EMULATOR:
> + case TPM_TYPE_EMULATOR:
> teo = ti->options->u.emulator.data;
> monitor_printf(mon, ",chardev=%s", teo->chardev);
> break;
> - case TPM_TYPE_OPTIONS_KIND__MAX:
> + case TPM_TYPE__MAX:
> break;
> }
> monitor_printf(mon, "\n");
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH 06/22] qapi: Convert simple union MemoryDeviceInfo to flat one
2021-09-13 12:39 [PATCH 00/22] qapi: Remove simple unions from the schema language Markus Armbruster
` (4 preceding siblings ...)
2021-09-13 12:39 ` [PATCH 05/22] qapi: Convert simple union TpmTypeOptions " Markus Armbruster
@ 2021-09-13 12:39 ` Markus Armbruster
2021-09-13 12:39 ` [PATCH 07/22] qapi: Convert simple union ChardevBackend " Markus Armbruster
` (16 subsequent siblings)
22 siblings, 0 replies; 54+ messages in thread
From: Markus Armbruster @ 2021-09-13 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, eblake, Eduardo Habkost, marcandre.lureau
Simple unions predate flat unions. Having both complicates the QAPI
schema language and the QAPI generator. We haven't been using simple
unions in new code for a long time, because they are less flexible and
somewhat awkward on the wire.
To prepare for their removal, convert simple union MemoryDeviceInfo to
an equivalent flat one. Adds some boilerplate to the schema, which is
a bit ugly, but a lot easier to maintain than the simple union
feature.
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qapi/machine.json | 42 ++++++++++++++++++++++++++++++++++++++----
1 file changed, 38 insertions(+), 4 deletions(-)
diff --git a/qapi/machine.json b/qapi/machine.json
index 157712f006..b3bdcccb72 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1194,6 +1194,38 @@
}
}
+##
+# @MemoryDeviceInfoKind:
+#
+# Since: 6.1
+##
+{ 'enum': 'MemoryDeviceInfoKind',
+ 'data': [ 'dimm', 'nvdimm', 'virtio-pmem', 'virtio-mem' ] }
+
+##
+# @PCDIMMDeviceInfoWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'PCDIMMDeviceInfoWrapper',
+ 'data': { 'data': 'PCDIMMDeviceInfo' } }
+
+##
+# @VirtioPMEMDeviceInfoWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'VirtioPMEMDeviceInfoWrapper',
+ 'data': { 'data': 'VirtioPMEMDeviceInfo' } }
+
+##
+# @VirtioMEMDeviceInfoWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'VirtioMEMDeviceInfoWrapper',
+ 'data': { 'data': 'VirtioMEMDeviceInfo' } }
+
##
# @MemoryDeviceInfo:
#
@@ -1205,10 +1237,12 @@
# Since: 2.1
##
{ 'union': 'MemoryDeviceInfo',
- 'data': { 'dimm': 'PCDIMMDeviceInfo',
- 'nvdimm': 'PCDIMMDeviceInfo',
- 'virtio-pmem': 'VirtioPMEMDeviceInfo',
- 'virtio-mem': 'VirtioMEMDeviceInfo'
+ 'base': { 'type': 'MemoryDeviceInfoKind' },
+ 'discriminator': 'type',
+ 'data': { 'dimm': 'PCDIMMDeviceInfoWrapper',
+ 'nvdimm': 'PCDIMMDeviceInfoWrapper',
+ 'virtio-pmem': 'VirtioPMEMDeviceInfoWrapper',
+ 'virtio-mem': 'VirtioMEMDeviceInfoWrapper'
}
}
--
2.31.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH 07/22] qapi: Convert simple union ChardevBackend to flat one
2021-09-13 12:39 [PATCH 00/22] qapi: Remove simple unions from the schema language Markus Armbruster
` (5 preceding siblings ...)
2021-09-13 12:39 ` [PATCH 06/22] qapi: Convert simple union MemoryDeviceInfo " Markus Armbruster
@ 2021-09-13 12:39 ` Markus Armbruster
2021-09-13 12:39 ` [PATCH 08/22] qapi: Convert simple union SocketAddressLegacy " Markus Armbruster
` (15 subsequent siblings)
22 siblings, 0 replies; 54+ messages in thread
From: Markus Armbruster @ 2021-09-13 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, Paolo Bonzini, eblake, marcandre.lureau
Simple unions predate flat unions. Having both complicates the QAPI
schema language and the QAPI generator. We haven't been using simple
unions in new code for a long time, because they are less flexible and
somewhat awkward on the wire.
To prepare for their removal, convert simple union ChardevBackend to
an equivalent flat one. Adds some boilerplate to the schema, which is
a bit ugly, but a lot easier to maintain than the simple union
feature.
Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qapi/char.json | 169 +++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 148 insertions(+), 21 deletions(-)
diff --git a/qapi/char.json b/qapi/char.json
index 9b18ee3305..69c136c138 100644
--- a/qapi/char.json
+++ b/qapi/char.json
@@ -407,6 +407,131 @@
'base': 'ChardevCommon',
'if': 'CONFIG_SPICE_PROTOCOL' }
+##
+# @ChardevBackendKind:
+#
+# Since: 6.1
+##
+{ 'enum': 'ChardevBackendKind',
+ 'data': [ 'file',
+ 'serial',
+ 'parallel',
+ 'pipe',
+ 'socket',
+ 'udp',
+ 'pty',
+ 'null',
+ 'mux',
+ 'msmouse',
+ 'wctablet',
+ 'braille',
+ 'testdev',
+ 'stdio',
+ 'console',
+ { 'name': 'spicevmc', 'if': 'CONFIG_SPICE' },
+ { 'name': 'spiceport', 'if': 'CONFIG_SPICE' },
+ { 'name': 'qemu-vdagent', 'if': 'CONFIG_SPICE_PROTOCOL' },
+ 'vc',
+ 'ringbuf',
+ # next one is just for compatibility
+ 'memory' ] }
+
+##
+# @ChardevFileWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'ChardevFileWrapper',
+ 'data': { 'data': 'ChardevFile' } }
+
+##
+# @ChardevHostdevWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'ChardevHostdevWrapper',
+ 'data': { 'data': 'ChardevHostdev' } }
+
+##
+# @ChardevSocketWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'ChardevSocketWrapper',
+ 'data': { 'data': 'ChardevSocket' } }
+
+##
+# @ChardevUdpWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'ChardevUdpWrapper',
+ 'data': { 'data': 'ChardevUdp' } }
+
+##
+# @ChardevCommonWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'ChardevCommonWrapper',
+ 'data': { 'data': 'ChardevCommon' } }
+
+##
+# @ChardevMuxWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'ChardevMuxWrapper',
+ 'data': { 'data': 'ChardevMux' } }
+
+##
+# @ChardevStdioWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'ChardevStdioWrapper',
+ 'data': { 'data': 'ChardevStdio' } }
+
+##
+# @ChardevSpiceChannelWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'ChardevSpiceChannelWrapper',
+ 'data': { 'data': 'ChardevSpiceChannel' } }
+
+##
+# @ChardevSpicePortWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'ChardevSpicePortWrapper',
+ 'data': { 'data': 'ChardevSpicePort' } }
+
+##
+# @ChardevQemuVDAgentWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'ChardevQemuVDAgentWrapper',
+ 'data': { 'data': 'ChardevQemuVDAgent' } }
+
+##
+# @ChardevVCWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'ChardevVCWrapper',
+ 'data': { 'data': 'ChardevVC' } }
+
+##
+# @ChardevRingbufWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'ChardevRingbufWrapper',
+ 'data': { 'data': 'ChardevRingbuf' } }
+
##
# @ChardevBackend:
#
@@ -415,31 +540,33 @@
# Since: 1.4 (testdev since 2.2, wctablet since 2.9, vdagent since 6.1)
##
{ 'union': 'ChardevBackend',
- 'data': { 'file': 'ChardevFile',
- 'serial': 'ChardevHostdev',
- 'parallel': 'ChardevHostdev',
- 'pipe': 'ChardevHostdev',
- 'socket': 'ChardevSocket',
- 'udp': 'ChardevUdp',
- 'pty': 'ChardevCommon',
- 'null': 'ChardevCommon',
- 'mux': 'ChardevMux',
- 'msmouse': 'ChardevCommon',
- 'wctablet': 'ChardevCommon',
- 'braille': 'ChardevCommon',
- 'testdev': 'ChardevCommon',
- 'stdio': 'ChardevStdio',
- 'console': 'ChardevCommon',
- 'spicevmc': { 'type': 'ChardevSpiceChannel',
+ 'base': { 'type': 'ChardevBackendKind' },
+ 'discriminator': 'type',
+ 'data': { 'file': 'ChardevFileWrapper',
+ 'serial': 'ChardevHostdevWrapper',
+ 'parallel': 'ChardevHostdevWrapper',
+ 'pipe': 'ChardevHostdevWrapper',
+ 'socket': 'ChardevSocketWrapper',
+ 'udp': 'ChardevUdpWrapper',
+ 'pty': 'ChardevCommonWrapper',
+ 'null': 'ChardevCommonWrapper',
+ 'mux': 'ChardevMuxWrapper',
+ 'msmouse': 'ChardevCommonWrapper',
+ 'wctablet': 'ChardevCommonWrapper',
+ 'braille': 'ChardevCommonWrapper',
+ 'testdev': 'ChardevCommonWrapper',
+ 'stdio': 'ChardevStdioWrapper',
+ 'console': 'ChardevCommonWrapper',
+ 'spicevmc': { 'type': 'ChardevSpiceChannelWrapper',
'if': 'CONFIG_SPICE' },
- 'spiceport': { 'type': 'ChardevSpicePort',
+ 'spiceport': { 'type': 'ChardevSpicePortWrapper',
'if': 'CONFIG_SPICE' },
- 'qemu-vdagent': { 'type': 'ChardevQemuVDAgent',
+ 'qemu-vdagent': { 'type': 'ChardevQemuVDAgentWrapper',
'if': 'CONFIG_SPICE_PROTOCOL' },
- 'vc': 'ChardevVC',
- 'ringbuf': 'ChardevRingbuf',
+ 'vc': 'ChardevVCWrapper',
+ 'ringbuf': 'ChardevRingbufWrapper',
# next one is just for compatibility
- 'memory': 'ChardevRingbuf' } }
+ 'memory': 'ChardevRingbufWrapper' } }
##
# @ChardevReturn:
--
2.31.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH 08/22] qapi: Convert simple union SocketAddressLegacy to flat one
2021-09-13 12:39 [PATCH 00/22] qapi: Remove simple unions from the schema language Markus Armbruster
` (6 preceding siblings ...)
2021-09-13 12:39 ` [PATCH 07/22] qapi: Convert simple union ChardevBackend " Markus Armbruster
@ 2021-09-13 12:39 ` Markus Armbruster
2021-09-13 12:39 ` [PATCH 09/22] qapi: Convert simple union ImageInfoSpecific " Markus Armbruster
` (14 subsequent siblings)
22 siblings, 0 replies; 54+ messages in thread
From: Markus Armbruster @ 2021-09-13 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, eblake, Daniel P. Berrangé, marcandre.lureau
Simple unions predate flat unions. Having both complicates the QAPI
schema language and the QAPI generator. We haven't been using simple
unions in new code for a long time, because they are less flexible and
somewhat awkward on the wire.
To prepare for their removal, convert simple union SocketAddressLegacy
to an equivalent flat one, with existing enum SocketAddressType
replacing implicit enum type SocketAddressLegacyKind. Adds some
boilerplate to the schema, which is a bit ugly, but a lot easier to
maintain than the simple union feature.
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qapi/sockets.json | 46 +++++++++++++++++++++++++++++++++++-------
chardev/char-socket.c | 6 +++---
chardev/char-udp.c | 4 ++--
tests/unit/test-yank.c | 6 +++---
util/qemu-sockets.c | 8 ++++----
5 files changed, 51 insertions(+), 19 deletions(-)
diff --git a/qapi/sockets.json b/qapi/sockets.json
index 7866dc27d6..21ead16bb3 100644
--- a/qapi/sockets.json
+++ b/qapi/sockets.json
@@ -110,6 +110,38 @@
'cid': 'str',
'port': 'str' } }
+##
+# @InetSocketAddressWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'InetSocketAddressWrapper',
+ 'data': { 'data': 'InetSocketAddress' } }
+
+##
+# @UnixSocketAddressWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'UnixSocketAddressWrapper',
+ 'data': { 'data': 'UnixSocketAddress' } }
+
+##
+# @VsockSocketAddressWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'VsockSocketAddressWrapper',
+ 'data': { 'data': 'VsockSocketAddress' } }
+
+##
+# @StringWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'StringWrapper',
+ 'data': { 'data': 'String' } }
+
##
# @SocketAddressLegacy:
#
@@ -117,18 +149,18 @@
#
# Note: This type is deprecated in favor of SocketAddress. The
# difference between SocketAddressLegacy and SocketAddress is that the
-# latter is a flat union rather than a simple union. Flat is nicer
-# because it avoids nesting on the wire, i.e. that form has fewer {}.
-
+# latter is has fewer {} on the wire.
#
# Since: 1.3
##
{ 'union': 'SocketAddressLegacy',
+ 'base': { 'type': 'SocketAddressType' },
+ 'discriminator': 'type',
'data': {
- 'inet': 'InetSocketAddress',
- 'unix': 'UnixSocketAddress',
- 'vsock': 'VsockSocketAddress',
- 'fd': 'String' } }
+ 'inet': 'InetSocketAddressWrapper',
+ 'unix': 'UnixSocketAddressWrapper',
+ 'vsock': 'VsockSocketAddressWrapper',
+ 'fd': 'StringWrapper' } }
##
# @SocketAddressType:
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index c43668cc15..836cfa0bc2 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -1520,7 +1520,7 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
addr = g_new0(SocketAddressLegacy, 1);
if (path) {
UnixSocketAddress *q_unix;
- addr->type = SOCKET_ADDRESS_LEGACY_KIND_UNIX;
+ addr->type = SOCKET_ADDRESS_TYPE_UNIX;
q_unix = addr->u.q_unix.data = g_new0(UnixSocketAddress, 1);
q_unix->path = g_strdup(path);
#ifdef CONFIG_LINUX
@@ -1530,7 +1530,7 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
q_unix->abstract = abstract;
#endif
} else if (host) {
- addr->type = SOCKET_ADDRESS_LEGACY_KIND_INET;
+ addr->type = SOCKET_ADDRESS_TYPE_INET;
addr->u.inet.data = g_new(InetSocketAddress, 1);
*addr->u.inet.data = (InetSocketAddress) {
.host = g_strdup(host),
@@ -1543,7 +1543,7 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
.ipv6 = qemu_opt_get_bool(opts, "ipv6", 0),
};
} else if (fd) {
- addr->type = SOCKET_ADDRESS_LEGACY_KIND_FD;
+ addr->type = SOCKET_ADDRESS_TYPE_FD;
addr->u.fd.data = g_new(String, 1);
addr->u.fd.data->str = g_strdup(fd);
} else {
diff --git a/chardev/char-udp.c b/chardev/char-udp.c
index 16b5dbce58..6756e69924 100644
--- a/chardev/char-udp.c
+++ b/chardev/char-udp.c
@@ -165,7 +165,7 @@ static void qemu_chr_parse_udp(QemuOpts *opts, ChardevBackend *backend,
qemu_chr_parse_common(opts, qapi_ChardevUdp_base(udp));
addr = g_new0(SocketAddressLegacy, 1);
- addr->type = SOCKET_ADDRESS_LEGACY_KIND_INET;
+ addr->type = SOCKET_ADDRESS_TYPE_INET;
addr->u.inet.data = g_new(InetSocketAddress, 1);
*addr->u.inet.data = (InetSocketAddress) {
.host = g_strdup(host),
@@ -180,7 +180,7 @@ static void qemu_chr_parse_udp(QemuOpts *opts, ChardevBackend *backend,
if (has_local) {
udp->has_local = true;
addr = g_new0(SocketAddressLegacy, 1);
- addr->type = SOCKET_ADDRESS_LEGACY_KIND_INET;
+ addr->type = SOCKET_ADDRESS_TYPE_INET;
addr->u.inet.data = g_new(InetSocketAddress, 1);
*addr->u.inet.data = (InetSocketAddress) {
.host = g_strdup(localaddr),
diff --git a/tests/unit/test-yank.c b/tests/unit/test-yank.c
index 2383d2908c..e6c036a64d 100644
--- a/tests/unit/test-yank.c
+++ b/tests/unit/test-yank.c
@@ -88,7 +88,7 @@ static void char_change_test(gconstpointer opaque)
.type = CHARDEV_BACKEND_KIND_SOCKET,
.u.socket.data = &(ChardevSocket) {
.addr = &(SocketAddressLegacy) {
- .type = SOCKET_ADDRESS_LEGACY_KIND_INET,
+ .type = SOCKET_ADDRESS_TYPE_INET,
.u.inet.data = &addr->u.inet
},
.has_server = true,
@@ -102,7 +102,7 @@ static void char_change_test(gconstpointer opaque)
.type = CHARDEV_BACKEND_KIND_UDP,
.u.udp.data = &(ChardevUdp) {
.remote = &(SocketAddressLegacy) {
- .type = SOCKET_ADDRESS_LEGACY_KIND_UNIX,
+ .type = SOCKET_ADDRESS_TYPE_UNIX,
.u.q_unix.data = &(UnixSocketAddress) {
.path = (char *)""
}
@@ -114,7 +114,7 @@ static void char_change_test(gconstpointer opaque)
.type = CHARDEV_BACKEND_KIND_SOCKET,
.u.socket.data = &(ChardevSocket) {
.addr = &(SocketAddressLegacy) {
- .type = SOCKET_ADDRESS_LEGACY_KIND_INET,
+ .type = SOCKET_ADDRESS_TYPE_INET,
.u.inet.data = &(InetSocketAddress) {
.host = (char *)"127.0.0.1",
.port = (char *)"0"
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index c5043999e9..72216ef980 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -1455,22 +1455,22 @@ SocketAddress *socket_address_flatten(SocketAddressLegacy *addr_legacy)
addr = g_new(SocketAddress, 1);
switch (addr_legacy->type) {
- case SOCKET_ADDRESS_LEGACY_KIND_INET:
+ case SOCKET_ADDRESS_TYPE_INET:
addr->type = SOCKET_ADDRESS_TYPE_INET;
QAPI_CLONE_MEMBERS(InetSocketAddress, &addr->u.inet,
addr_legacy->u.inet.data);
break;
- case SOCKET_ADDRESS_LEGACY_KIND_UNIX:
+ case SOCKET_ADDRESS_TYPE_UNIX:
addr->type = SOCKET_ADDRESS_TYPE_UNIX;
QAPI_CLONE_MEMBERS(UnixSocketAddress, &addr->u.q_unix,
addr_legacy->u.q_unix.data);
break;
- case SOCKET_ADDRESS_LEGACY_KIND_VSOCK:
+ case SOCKET_ADDRESS_TYPE_VSOCK:
addr->type = SOCKET_ADDRESS_TYPE_VSOCK;
QAPI_CLONE_MEMBERS(VsockSocketAddress, &addr->u.vsock,
addr_legacy->u.vsock.data);
break;
- case SOCKET_ADDRESS_LEGACY_KIND_FD:
+ case SOCKET_ADDRESS_TYPE_FD:
addr->type = SOCKET_ADDRESS_TYPE_FD;
QAPI_CLONE_MEMBERS(String, &addr->u.fd, addr_legacy->u.fd.data);
break;
--
2.31.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH 09/22] qapi: Convert simple union ImageInfoSpecific to flat one
2021-09-13 12:39 [PATCH 00/22] qapi: Remove simple unions from the schema language Markus Armbruster
` (7 preceding siblings ...)
2021-09-13 12:39 ` [PATCH 08/22] qapi: Convert simple union SocketAddressLegacy " Markus Armbruster
@ 2021-09-13 12:39 ` Markus Armbruster
2021-09-16 15:01 ` Hanna Reitz
2021-09-13 12:39 ` [PATCH 10/22] qapi: Convert simple union TransactionAction " Markus Armbruster
` (13 subsequent siblings)
22 siblings, 1 reply; 54+ messages in thread
From: Markus Armbruster @ 2021-09-13 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, jsnow, Hanna Reitz, eblake, marcandre.lureau
Simple unions predate flat unions. Having both complicates the QAPI
schema language and the QAPI generator. We haven't been using simple
unions in new code for a long time, because they are less flexible and
somewhat awkward on the wire.
To prepare for their removal, convert simple union ImageInfoSpecific
to an equivalent flat one. Adds some boilerplate to the schema, which
is a bit ugly, but a lot easier to maintain than the simple union
feature.
Implicit enum ImageInfoSpecificKind becomes explicit. It duplicates
part of enum BlockdevDriver. We could reuse BlockdevDriver instead.
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Hanna Reitz <hreitz@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qapi/block-core.json | 56 ++++++++++++++++++++++++++++++++++++++------
1 file changed, 49 insertions(+), 7 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index c8ce1d9d5d..b2858b144f 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -139,6 +139,49 @@
'*encryption-format': 'RbdImageEncryptionFormat'
} }
+##
+# @ImageInfoSpecificKind:
+#
+# Since: 6.1
+##
+{ 'enum': 'ImageInfoSpecificKind',
+ 'data': [ 'qcow2', 'vmdk', 'luks', 'rbd' ] }
+
+##
+# @ImageInfoSpecificQCow2Wrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'ImageInfoSpecificQCow2Wrapper',
+ 'data': { 'data': 'ImageInfoSpecificQCow2' } }
+
+##
+# @ImageInfoSpecificVmdkWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'ImageInfoSpecificVmdkWrapper',
+ 'data': { 'data': 'ImageInfoSpecificVmdk' } }
+
+##
+# @ImageInfoSpecificLUKSWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'ImageInfoSpecificLUKSWrapper',
+ 'data': { 'data': 'QCryptoBlockInfoLUKS' } }
+# If we need to add block driver specific parameters for
+# LUKS in future, then we'll subclass QCryptoBlockInfoLUKS
+# to define a ImageInfoSpecificLUKS
+
+##
+# @ImageInfoSpecificRbdWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'ImageInfoSpecificRbdWrapper',
+ 'data': { 'data': 'ImageInfoSpecificRbd' } }
+
##
# @ImageInfoSpecific:
#
@@ -147,14 +190,13 @@
# Since: 1.7
##
{ 'union': 'ImageInfoSpecific',
+ 'base': { 'type': 'ImageInfoSpecificKind' },
+ 'discriminator': 'type',
'data': {
- 'qcow2': 'ImageInfoSpecificQCow2',
- 'vmdk': 'ImageInfoSpecificVmdk',
- # If we need to add block driver specific parameters for
- # LUKS in future, then we'll subclass QCryptoBlockInfoLUKS
- # to define a ImageInfoSpecificLUKS
- 'luks': 'QCryptoBlockInfoLUKS',
- 'rbd': 'ImageInfoSpecificRbd'
+ 'qcow2': 'ImageInfoSpecificQCow2Wrapper',
+ 'vmdk': 'ImageInfoSpecificVmdkWrapper',
+ 'luks': 'ImageInfoSpecificLUKSWrapper',
+ 'rbd': 'ImageInfoSpecificRbdWrapper'
} }
##
--
2.31.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH 09/22] qapi: Convert simple union ImageInfoSpecific to flat one
2021-09-13 12:39 ` [PATCH 09/22] qapi: Convert simple union ImageInfoSpecific " Markus Armbruster
@ 2021-09-16 15:01 ` Hanna Reitz
0 siblings, 0 replies; 54+ messages in thread
From: Hanna Reitz @ 2021-09-16 15:01 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: Kevin Wolf, jsnow, eblake, marcandre.lureau
On 13.09.21 14:39, Markus Armbruster wrote:
> Simple unions predate flat unions. Having both complicates the QAPI
> schema language and the QAPI generator. We haven't been using simple
> unions in new code for a long time, because they are less flexible and
> somewhat awkward on the wire.
>
> To prepare for their removal, convert simple union ImageInfoSpecific
> to an equivalent flat one. Adds some boilerplate to the schema, which
> is a bit ugly, but a lot easier to maintain than the simple union
> feature.
>
> Implicit enum ImageInfoSpecificKind becomes explicit. It duplicates
> part of enum BlockdevDriver. We could reuse BlockdevDriver instead.
>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Hanna Reitz <hreitz@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> qapi/block-core.json | 56 ++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 49 insertions(+), 7 deletions(-)
Acked-by: Hanna Reitz <hreitz@redhat.com>
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH 10/22] qapi: Convert simple union TransactionAction to flat one
2021-09-13 12:39 [PATCH 00/22] qapi: Remove simple unions from the schema language Markus Armbruster
` (8 preceding siblings ...)
2021-09-13 12:39 ` [PATCH 09/22] qapi: Convert simple union ImageInfoSpecific " Markus Armbruster
@ 2021-09-13 12:39 ` Markus Armbruster
2021-09-13 14:53 ` Eric Blake
2021-09-16 15:01 ` Hanna Reitz
2021-09-13 12:39 ` [PATCH 11/22] tests/qapi-schema: Prepare for simple union UserDefListUnion removal Markus Armbruster
` (12 subsequent siblings)
22 siblings, 2 replies; 54+ messages in thread
From: Markus Armbruster @ 2021-09-13 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, jsnow, Hanna Reitz, eblake, marcandre.lureau
Simple unions predate flat unions. Having both complicates the QAPI
schema language and the QAPI generator. We haven't been using simple
unions in new code for a long time, because they are less flexible and
somewhat awkward on the wire.
To prepare for their removal, convert simple union TransactionAction
to an equivalent flat one. Adds some boilerplate to the schema, which
is a bit ugly, but a lot easier to maintain than the simple union
feature.
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Hanna Reitz <hreitz@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qapi/transaction.json | 111 +++++++++++++++++++++++++++++++++++++-----
1 file changed, 99 insertions(+), 12 deletions(-)
diff --git a/qapi/transaction.json b/qapi/transaction.json
index 894258d9e2..d7fc73d7df 100644
--- a/qapi/transaction.json
+++ b/qapi/transaction.json
@@ -38,6 +38,91 @@
{ 'enum': 'ActionCompletionMode',
'data': [ 'individual', 'grouped' ] }
+##
+# @TransactionActionKind:
+#
+# Since: 6.1
+##
+{ 'enum': 'TransactionActionKind',
+ 'data': [ 'abort', 'block-dirty-bitmap-add', 'block-dirty-bitmap-remove',
+ 'block-dirty-bitmap-clear', 'block-dirty-bitmap-enable',
+ 'block-dirty-bitmap-disable', 'block-dirty-bitmap-merge',
+ 'blockdev-backup', 'blockdev-snapshot',
+ 'blockdev-snapshot-internal-sync', 'blockdev-snapshot-sync',
+ 'drive-backup' ] }
+
+##
+# @AbortWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'AbortWrapper',
+ 'data': { 'data': 'Abort' } }
+
+##
+# @BlockDirtyBitmapAddWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'BlockDirtyBitmapAddWrapper',
+ 'data': { 'data': 'BlockDirtyBitmapAdd' } }
+
+##
+# @BlockDirtyBitmapWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'BlockDirtyBitmapWrapper',
+ 'data': { 'data': 'BlockDirtyBitmap' } }
+
+##
+# @BlockDirtyBitmapMergeWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'BlockDirtyBitmapMergeWrapper',
+ 'data': { 'data': 'BlockDirtyBitmapMerge' } }
+
+##
+# @BlockdevBackupWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'BlockdevBackupWrapper',
+ 'data': { 'data': 'BlockdevBackup' } }
+
+##
+# @BlockdevSnapshotWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'BlockdevSnapshotWrapper',
+ 'data': { 'data': 'BlockdevSnapshot' } }
+
+##
+# @BlockdevSnapshotInternalWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'BlockdevSnapshotInternalWrapper',
+ 'data': { 'data': 'BlockdevSnapshotInternal' } }
+
+##
+# @BlockdevSnapshotSyncWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'BlockdevSnapshotSyncWrapper',
+ 'data': { 'data': 'BlockdevSnapshotSync' } }
+
+##
+# @DriveBackupWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'DriveBackupWrapper',
+ 'data': { 'data': 'DriveBackup' } }
+
##
# @TransactionAction:
#
@@ -60,19 +145,21 @@
# Since: 1.1
##
{ 'union': 'TransactionAction',
+ 'base': { 'type': 'TransactionActionKind' },
+ 'discriminator': 'type',
'data': {
- 'abort': 'Abort',
- 'block-dirty-bitmap-add': 'BlockDirtyBitmapAdd',
- 'block-dirty-bitmap-remove': 'BlockDirtyBitmap',
- 'block-dirty-bitmap-clear': 'BlockDirtyBitmap',
- 'block-dirty-bitmap-enable': 'BlockDirtyBitmap',
- 'block-dirty-bitmap-disable': 'BlockDirtyBitmap',
- 'block-dirty-bitmap-merge': 'BlockDirtyBitmapMerge',
- 'blockdev-backup': 'BlockdevBackup',
- 'blockdev-snapshot': 'BlockdevSnapshot',
- 'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal',
- 'blockdev-snapshot-sync': 'BlockdevSnapshotSync',
- 'drive-backup': 'DriveBackup'
+ 'abort': 'AbortWrapper',
+ 'block-dirty-bitmap-add': 'BlockDirtyBitmapAddWrapper',
+ 'block-dirty-bitmap-remove': 'BlockDirtyBitmapWrapper',
+ 'block-dirty-bitmap-clear': 'BlockDirtyBitmapWrapper',
+ 'block-dirty-bitmap-enable': 'BlockDirtyBitmapWrapper',
+ 'block-dirty-bitmap-disable': 'BlockDirtyBitmapWrapper',
+ 'block-dirty-bitmap-merge': 'BlockDirtyBitmapMergeWrapper',
+ 'blockdev-backup': 'BlockdevBackupWrapper',
+ 'blockdev-snapshot': 'BlockdevSnapshotWrapper',
+ 'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternalWrapper',
+ 'blockdev-snapshot-sync': 'BlockdevSnapshotSyncWrapper',
+ 'drive-backup': 'DriveBackupWrapper'
} }
##
--
2.31.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH 10/22] qapi: Convert simple union TransactionAction to flat one
2021-09-13 12:39 ` [PATCH 10/22] qapi: Convert simple union TransactionAction " Markus Armbruster
@ 2021-09-13 14:53 ` Eric Blake
2021-09-14 5:25 ` Markus Armbruster
2021-09-16 15:01 ` Hanna Reitz
1 sibling, 1 reply; 54+ messages in thread
From: Eric Blake @ 2021-09-13 14:53 UTC (permalink / raw)
To: Markus Armbruster
Cc: Kevin Wolf, marcandre.lureau, Hanna Reitz, jsnow, qemu-devel
On Mon, Sep 13, 2021 at 02:39:20PM +0200, Markus Armbruster wrote:
> Simple unions predate flat unions. Having both complicates the QAPI
> schema language and the QAPI generator. We haven't been using simple
> unions in new code for a long time, because they are less flexible and
> somewhat awkward on the wire.
>
> To prepare for their removal, convert simple union TransactionAction
> to an equivalent flat one. Adds some boilerplate to the schema, which
> is a bit ugly, but a lot easier to maintain than the simple union
> feature.
>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Hanna Reitz <hreitz@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> qapi/transaction.json | 111 +++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 99 insertions(+), 12 deletions(-)
Same comments for each of 5-10 as for 4; the conversion is sane, and
the only question is on documentation, whether you want...
>
> diff --git a/qapi/transaction.json b/qapi/transaction.json
> index 894258d9e2..d7fc73d7df 100644
> --- a/qapi/transaction.json
> +++ b/qapi/transaction.json
> @@ -38,6 +38,91 @@
> { 'enum': 'ActionCompletionMode',
> 'data': [ 'individual', 'grouped' ] }
>
> +##
> +# @TransactionActionKind:
> +#
> +# Since: 6.1
... 6.2 here, or to preserve the implicit...
> ##
> # @TransactionAction:
> #
> @@ -60,19 +145,21 @@
> # Since: 1.1
...1.1 matching when the simple union was first formed (actually, this
simple union has grown over time, which makes it trickier to decide
which historical Since: to use on each Wrapper, so I'd lean towards
6.2 on all of them as being less work).
For patches 5-10:
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [PATCH 10/22] qapi: Convert simple union TransactionAction to flat one
2021-09-13 14:53 ` Eric Blake
@ 2021-09-14 5:25 ` Markus Armbruster
0 siblings, 0 replies; 54+ messages in thread
From: Markus Armbruster @ 2021-09-14 5:25 UTC (permalink / raw)
To: Eric Blake; +Cc: Kevin Wolf, marcandre.lureau, Hanna Reitz, jsnow, qemu-devel
Eric Blake <eblake@redhat.com> writes:
> On Mon, Sep 13, 2021 at 02:39:20PM +0200, Markus Armbruster wrote:
>> Simple unions predate flat unions. Having both complicates the QAPI
>> schema language and the QAPI generator. We haven't been using simple
>> unions in new code for a long time, because they are less flexible and
>> somewhat awkward on the wire.
>>
>> To prepare for their removal, convert simple union TransactionAction
>> to an equivalent flat one. Adds some boilerplate to the schema, which
>> is a bit ugly, but a lot easier to maintain than the simple union
>> feature.
>>
>> Cc: Kevin Wolf <kwolf@redhat.com>
>> Cc: Hanna Reitz <hreitz@redhat.com>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> qapi/transaction.json | 111 +++++++++++++++++++++++++++++++++++++-----
>> 1 file changed, 99 insertions(+), 12 deletions(-)
>
> Same comments for each of 5-10 as for 4; the conversion is sane, and
> the only question is on documentation, whether you want...
>
>>
>> diff --git a/qapi/transaction.json b/qapi/transaction.json
>> index 894258d9e2..d7fc73d7df 100644
>> --- a/qapi/transaction.json
>> +++ b/qapi/transaction.json
>> @@ -38,6 +38,91 @@
>> { 'enum': 'ActionCompletionMode',
>> 'data': [ 'individual', 'grouped' ] }
>>
>> +##
>> +# @TransactionActionKind:
>> +#
>> +# Since: 6.1
>
> ... 6.2 here, or to preserve the implicit...
>
>> ##
>> # @TransactionAction:
>> #
>> @@ -60,19 +145,21 @@
>> # Since: 1.1
>
> ...1.1 matching when the simple union was first formed (actually, this
> simple union has grown over time, which makes it trickier to decide
> which historical Since: to use on each Wrapper, so I'd lean towards
> 6.2 on all of them as being less work).
The enum becomes explicit in the schema, but is the same as before. I
think copying "since" information from the no-longer-simple union and
its branches to the enum and its members makes sense.
The wrapper types become explicit, but with a new name. We can copy
"since" from their branch anyway. Doesn't really matter, unlike for the
enum.
> For patches 5-10:
> Reviewed-by: Eric Blake <eblake@redhat.com>
Thanks!
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 10/22] qapi: Convert simple union TransactionAction to flat one
2021-09-13 12:39 ` [PATCH 10/22] qapi: Convert simple union TransactionAction " Markus Armbruster
2021-09-13 14:53 ` Eric Blake
@ 2021-09-16 15:01 ` Hanna Reitz
1 sibling, 0 replies; 54+ messages in thread
From: Hanna Reitz @ 2021-09-16 15:01 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: Kevin Wolf, jsnow, eblake, marcandre.lureau
On 13.09.21 14:39, Markus Armbruster wrote:
> Simple unions predate flat unions. Having both complicates the QAPI
> schema language and the QAPI generator. We haven't been using simple
> unions in new code for a long time, because they are less flexible and
> somewhat awkward on the wire.
>
> To prepare for their removal, convert simple union TransactionAction
> to an equivalent flat one. Adds some boilerplate to the schema, which
> is a bit ugly, but a lot easier to maintain than the simple union
> feature.
>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Hanna Reitz <hreitz@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> qapi/transaction.json | 111 +++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 99 insertions(+), 12 deletions(-)
Acked-by: Hanna Reitz <hreitz@redhat.com>
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH 11/22] tests/qapi-schema: Prepare for simple union UserDefListUnion removal
2021-09-13 12:39 [PATCH 00/22] qapi: Remove simple unions from the schema language Markus Armbruster
` (9 preceding siblings ...)
2021-09-13 12:39 ` [PATCH 10/22] qapi: Convert simple union TransactionAction " Markus Armbruster
@ 2021-09-13 12:39 ` Markus Armbruster
2021-09-13 15:01 ` Eric Blake
2021-09-13 12:39 ` [PATCH 12/22] test-qobject-input-visitor: Wean off UserDefListUnion Markus Armbruster
` (11 subsequent siblings)
22 siblings, 1 reply; 54+ messages in thread
From: Markus Armbruster @ 2021-09-13 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, eblake, marcandre.lureau
Simple unions predate flat unions. Having both complicates the QAPI
schema language and the QAPI generator. We haven't been using simple
unions in new code for a long time, because they are less flexible and
somewhat awkward on the wire.
To prepare for their removal, simple union UserDefListUnion has to go.
It is used to cover arrays. The next few commits will eliminate its
uses, and then it gets deleted. As a first step, provide struct
ArrayStruct for the tests to be rewritten.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/qapi-schema/qapi-schema-test.json | 16 ++++++++++++++++
tests/qapi-schema/qapi-schema-test.out | 16 ++++++++++++++++
2 files changed, 32 insertions(+)
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 3c43e14e22..b2d795cb19 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -140,6 +140,22 @@
'sizes': ['size'],
'any': ['any'],
'user': ['Status'] } } # intentional forward ref. to sub-module
+{ 'struct': 'ArrayStruct',
+ 'data': { 'integer': ['int'],
+ 's8': ['int8'],
+ 's16': ['int16'],
+ 's32': ['int32'],
+ 's64': ['int64'],
+ 'u8': ['uint8'],
+ 'u16': ['uint16'],
+ 'u32': ['uint32'],
+ 'u64': ['uint64'],
+ 'number': ['number'],
+ 'boolean': ['bool'],
+ 'string': ['str'],
+ '*sz': ['size'],
+ '*any': ['any'],
+ '*user': ['Status'] } } # intentional forward ref. to sub-module
# for testing sub-modules
{ 'include': 'include/sub-module.json' }
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index d557fe2d89..7a488c1d06 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -189,6 +189,22 @@ object UserDefListUnion
case sizes: q_obj_sizeList-wrapper
case any: q_obj_anyList-wrapper
case user: q_obj_StatusList-wrapper
+object ArrayStruct
+ member integer: intList optional=False
+ member s8: int8List optional=False
+ member s16: int16List optional=False
+ member s32: int32List optional=False
+ member s64: int64List optional=False
+ member u8: uint8List optional=False
+ member u16: uint16List optional=False
+ member u32: uint32List optional=False
+ member u64: uint64List optional=False
+ member number: numberList optional=False
+ member boolean: boolList optional=False
+ member string: strList optional=False
+ member sz: sizeList optional=True
+ member any: anyList optional=True
+ member user: StatusList optional=True
include include/sub-module.json
command user-def-cmd None -> None
gen=True success_response=True boxed=False oob=False preconfig=False
--
2.31.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH 11/22] tests/qapi-schema: Prepare for simple union UserDefListUnion removal
2021-09-13 12:39 ` [PATCH 11/22] tests/qapi-schema: Prepare for simple union UserDefListUnion removal Markus Armbruster
@ 2021-09-13 15:01 ` Eric Blake
0 siblings, 0 replies; 54+ messages in thread
From: Eric Blake @ 2021-09-13 15:01 UTC (permalink / raw)
To: Markus Armbruster; +Cc: marcandre.lureau, jsnow, qemu-devel
On Mon, Sep 13, 2021 at 02:39:21PM +0200, Markus Armbruster wrote:
> Simple unions predate flat unions. Having both complicates the QAPI
> schema language and the QAPI generator. We haven't been using simple
> unions in new code for a long time, because they are less flexible and
> somewhat awkward on the wire.
>
> To prepare for their removal, simple union UserDefListUnion has to go.
> It is used to cover arrays. The next few commits will eliminate its
> uses, and then it gets deleted. As a first step, provide struct
> ArrayStruct for the tests to be rewritten.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> tests/qapi-schema/qapi-schema-test.json | 16 ++++++++++++++++
> tests/qapi-schema/qapi-schema-test.out | 16 ++++++++++++++++
> 2 files changed, 32 insertions(+)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH 12/22] test-qobject-input-visitor: Wean off UserDefListUnion
2021-09-13 12:39 [PATCH 00/22] qapi: Remove simple unions from the schema language Markus Armbruster
` (10 preceding siblings ...)
2021-09-13 12:39 ` [PATCH 11/22] tests/qapi-schema: Prepare for simple union UserDefListUnion removal Markus Armbruster
@ 2021-09-13 12:39 ` Markus Armbruster
2021-09-13 15:06 ` Eric Blake
2021-09-13 12:39 ` [PATCH 13/22] test-qobject-output-visitor: " Markus Armbruster
` (10 subsequent siblings)
22 siblings, 1 reply; 54+ messages in thread
From: Markus Armbruster @ 2021-09-13 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, eblake, marcandre.lureau
The test_visitor_in_list_union_FOO() use simple union UserDefListUnion
to cover lists of builtin types. Rewrite as
test_visitor_in_list_struct(), using struct ArrayStruct and a lot less
code.
test_visitor_in_fail_union_list() uses UserDefListUnion to cover
"variant members don't match the discriminator value". Cover that in
test_visitor_in_fail_union_flat() instead, and drop
test_visitor_in_fail_union_list(). Appropriating the former for this
purpose is okay, because it actually failed due to missing
discriminator, which is still covered by
test_visitor_in_fail_union_flat_no_discrim().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/unit/test-qobject-input-visitor.c | 460 ++++++++----------------
1 file changed, 148 insertions(+), 312 deletions(-)
diff --git a/tests/unit/test-qobject-input-visitor.c b/tests/unit/test-qobject-input-visitor.c
index e41b91a2a6..6f59a7f432 100644
--- a/tests/unit/test-qobject-input-visitor.c
+++ b/tests/unit/test-qobject-input-visitor.c
@@ -464,6 +464,151 @@ static void test_visitor_in_list(TestInputVisitorData *data,
g_assert(!head);
}
+static void test_visitor_in_list_struct(TestInputVisitorData *data,
+ const void *unused)
+{
+ const char *int_member[] = {
+ "integer", "s8", "s16", "s32", "s64", "u8", "u16", "u32", "u64" };
+ g_autoptr(GString) json = g_string_new("");
+ int i, j;
+ const char *sep;
+ g_autoptr(ArrayStruct) arrs = NULL;
+ Visitor *v;
+ intList *int_list;
+ int8List *s8_list;
+ int16List *s16_list;
+ int32List *s32_list;
+ int64List *s64_list;
+ uint8List *u8_list;
+ uint16List *u16_list;
+ uint32List *u32_list;
+ uint64List *u64_list;
+ numberList *num_list;
+ boolList *bool_list;
+ strList *str_list;
+
+ g_string_append_printf(json, "{");
+
+ for (i = 0; i < G_N_ELEMENTS(int_member); i++) {
+ g_string_append_printf(json, "'%s': [", int_member[i]);
+ sep = "";
+ for (j = 0; j < 32; j++) {
+ g_string_append_printf(json, "%s%d", sep, j);
+ sep = ", ";
+ }
+ g_string_append_printf(json, "], ");
+ }
+
+ g_string_append_printf(json, "'number': [");
+ sep = "";
+ for (i = 0; i < 32; i++) {
+ g_string_append_printf(json, "%s%f", sep, (double)i / 3);
+ sep = ", ";
+ }
+ g_string_append_printf(json, "], ");
+
+ g_string_append_printf(json, "'boolean': [");
+ sep = "";
+ for (i = 0; i < 32; i++) {
+ g_string_append_printf(json, "%s%s",
+ sep, i % 3 == 0 ? "true" : "false");
+ sep = ", ";
+ }
+ g_string_append_printf(json, "], ");
+
+ g_string_append_printf(json, "'string': [");
+ sep = "";
+ for (i = 0; i < 32; i++) {
+ g_string_append_printf(json, "%s'%d'", sep, i);
+ sep = ", ";
+ }
+ g_string_append_printf(json, "]");
+
+ g_string_append_printf(json, "}");
+
+ v = visitor_input_test_init_raw(data, json->str);
+ visit_type_ArrayStruct(v, NULL, &arrs, &error_abort);
+
+ i = 0;
+ for (int_list = arrs->integer; int_list; int_list = int_list->next) {
+ g_assert_cmpint(int_list->value, ==, i);
+ i++;
+ }
+
+ i = 0;
+ for (s8_list = arrs->s8; s8_list; s8_list = s8_list->next) {
+ g_assert_cmpint(s8_list->value, ==, i);
+ i++;
+ }
+
+ i = 0;
+ for (s16_list = arrs->s16; s16_list; s16_list = s16_list->next) {
+ g_assert_cmpint(s16_list->value, ==, i);
+ i++;
+ }
+
+ i = 0;
+ for (s32_list = arrs->s32; s32_list; s32_list = s32_list->next) {
+ g_assert_cmpint(s32_list->value, ==, i);
+ i++;
+ }
+
+ i = 0;
+ for (s64_list = arrs->s64; s64_list; s64_list = s64_list->next) {
+ g_assert_cmpint(s64_list->value, ==, i);
+ i++;
+ }
+
+ i = 0;
+ for (u8_list = arrs->u8; u8_list; u8_list = u8_list->next) {
+ g_assert_cmpint(u8_list->value, ==, i);
+ i++;
+ }
+
+ i = 0;
+ for (u16_list = arrs->u16; u16_list; u16_list = u16_list->next) {
+ g_assert_cmpint(u16_list->value, ==, i);
+ i++;
+ }
+
+ i = 0;
+ for (u32_list = arrs->u32; u32_list; u32_list = u32_list->next) {
+ g_assert_cmpint(u32_list->value, ==, i);
+ i++;
+ }
+
+ i = 0;
+ for (u64_list = arrs->u64; u64_list; u64_list = u64_list->next) {
+ g_assert_cmpint(u64_list->value, ==, i);
+ i++;
+ }
+
+ i = 0;
+ for (num_list = arrs->number; num_list; num_list = num_list->next) {
+ char expected[32], actual[32];
+
+ sprintf(expected, "%.6f", (double)i / 3);
+ sprintf(actual, "%.6f", num_list->value);
+ g_assert_cmpstr(expected, ==, actual);
+ i++;
+ }
+
+ i = 0;
+ for (bool_list = arrs->boolean; bool_list; bool_list = bool_list->next) {
+ g_assert_cmpint(bool_list->value, ==, i % 3 == 0);
+ i++;
+ }
+
+ i = 0;
+ for (str_list = arrs->string; str_list; str_list = str_list->next) {
+ char expected[32];
+
+ sprintf(expected, "%d", i);
+ g_assert_cmpstr(str_list->value, ==, expected);
+ i++;
+ }
+}
+
static void test_visitor_in_any(TestInputVisitorData *data,
const void *unused)
{
@@ -682,276 +827,6 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
qapi_free_AltEnumInt(asi);
}
-static void test_list_union_integer_helper(TestInputVisitorData *data,
- const void *unused,
- UserDefListUnionKind kind)
-{
- g_autoptr(UserDefListUnion) cvalue = NULL;
- Visitor *v;
- GString *gstr_list = g_string_new("");
- GString *gstr_union = g_string_new("");
- int i;
-
- for (i = 0; i < 32; i++) {
- g_string_append_printf(gstr_list, "%d", i);
- if (i != 31) {
- g_string_append(gstr_list, ", ");
- }
- }
- g_string_append_printf(gstr_union, "{ 'type': '%s', 'data': [ %s ] }",
- UserDefListUnionKind_str(kind),
- gstr_list->str);
- v = visitor_input_test_init_raw(data, gstr_union->str);
-
- visit_type_UserDefListUnion(v, NULL, &cvalue, &error_abort);
- g_assert(cvalue != NULL);
- g_assert_cmpint(cvalue->type, ==, kind);
-
- switch (kind) {
- case USER_DEF_LIST_UNION_KIND_INTEGER: {
- intList *elem = NULL;
- for (i = 0, elem = cvalue->u.integer.data;
- elem; elem = elem->next, i++) {
- g_assert_cmpint(elem->value, ==, i);
- }
- break;
- }
- case USER_DEF_LIST_UNION_KIND_S8: {
- int8List *elem = NULL;
- for (i = 0, elem = cvalue->u.s8.data; elem; elem = elem->next, i++) {
- g_assert_cmpint(elem->value, ==, i);
- }
- break;
- }
- case USER_DEF_LIST_UNION_KIND_S16: {
- int16List *elem = NULL;
- for (i = 0, elem = cvalue->u.s16.data; elem; elem = elem->next, i++) {
- g_assert_cmpint(elem->value, ==, i);
- }
- break;
- }
- case USER_DEF_LIST_UNION_KIND_S32: {
- int32List *elem = NULL;
- for (i = 0, elem = cvalue->u.s32.data; elem; elem = elem->next, i++) {
- g_assert_cmpint(elem->value, ==, i);
- }
- break;
- }
- case USER_DEF_LIST_UNION_KIND_S64: {
- int64List *elem = NULL;
- for (i = 0, elem = cvalue->u.s64.data; elem; elem = elem->next, i++) {
- g_assert_cmpint(elem->value, ==, i);
- }
- break;
- }
- case USER_DEF_LIST_UNION_KIND_U8: {
- uint8List *elem = NULL;
- for (i = 0, elem = cvalue->u.u8.data; elem; elem = elem->next, i++) {
- g_assert_cmpint(elem->value, ==, i);
- }
- break;
- }
- case USER_DEF_LIST_UNION_KIND_U16: {
- uint16List *elem = NULL;
- for (i = 0, elem = cvalue->u.u16.data; elem; elem = elem->next, i++) {
- g_assert_cmpint(elem->value, ==, i);
- }
- break;
- }
- case USER_DEF_LIST_UNION_KIND_U32: {
- uint32List *elem = NULL;
- for (i = 0, elem = cvalue->u.u32.data; elem; elem = elem->next, i++) {
- g_assert_cmpint(elem->value, ==, i);
- }
- break;
- }
- case USER_DEF_LIST_UNION_KIND_U64: {
- uint64List *elem = NULL;
- for (i = 0, elem = cvalue->u.u64.data; elem; elem = elem->next, i++) {
- g_assert_cmpint(elem->value, ==, i);
- }
- break;
- }
- default:
- g_assert_not_reached();
- }
-
- g_string_free(gstr_union, true);
- g_string_free(gstr_list, true);
-}
-
-static void test_visitor_in_list_union_int(TestInputVisitorData *data,
- const void *unused)
-{
- test_list_union_integer_helper(data, unused,
- USER_DEF_LIST_UNION_KIND_INTEGER);
-}
-
-static void test_visitor_in_list_union_int8(TestInputVisitorData *data,
- const void *unused)
-{
- test_list_union_integer_helper(data, unused,
- USER_DEF_LIST_UNION_KIND_S8);
-}
-
-static void test_visitor_in_list_union_int16(TestInputVisitorData *data,
- const void *unused)
-{
- test_list_union_integer_helper(data, unused,
- USER_DEF_LIST_UNION_KIND_S16);
-}
-
-static void test_visitor_in_list_union_int32(TestInputVisitorData *data,
- const void *unused)
-{
- test_list_union_integer_helper(data, unused,
- USER_DEF_LIST_UNION_KIND_S32);
-}
-
-static void test_visitor_in_list_union_int64(TestInputVisitorData *data,
- const void *unused)
-{
- test_list_union_integer_helper(data, unused,
- USER_DEF_LIST_UNION_KIND_S64);
-}
-
-static void test_visitor_in_list_union_uint8(TestInputVisitorData *data,
- const void *unused)
-{
- test_list_union_integer_helper(data, unused,
- USER_DEF_LIST_UNION_KIND_U8);
-}
-
-static void test_visitor_in_list_union_uint16(TestInputVisitorData *data,
- const void *unused)
-{
- test_list_union_integer_helper(data, unused,
- USER_DEF_LIST_UNION_KIND_U16);
-}
-
-static void test_visitor_in_list_union_uint32(TestInputVisitorData *data,
- const void *unused)
-{
- test_list_union_integer_helper(data, unused,
- USER_DEF_LIST_UNION_KIND_U32);
-}
-
-static void test_visitor_in_list_union_uint64(TestInputVisitorData *data,
- const void *unused)
-{
- test_list_union_integer_helper(data, unused,
- USER_DEF_LIST_UNION_KIND_U64);
-}
-
-static void test_visitor_in_list_union_bool(TestInputVisitorData *data,
- const void *unused)
-{
- g_autoptr(UserDefListUnion) cvalue = NULL;
- boolList *elem = NULL;
- Visitor *v;
- GString *gstr_list = g_string_new("");
- GString *gstr_union = g_string_new("");
- int i;
-
- for (i = 0; i < 32; i++) {
- g_string_append_printf(gstr_list, "%s",
- (i % 3 == 0) ? "true" : "false");
- if (i != 31) {
- g_string_append(gstr_list, ", ");
- }
- }
- g_string_append_printf(gstr_union, "{ 'type': 'boolean', 'data': [ %s ] }",
- gstr_list->str);
- v = visitor_input_test_init_raw(data, gstr_union->str);
-
- visit_type_UserDefListUnion(v, NULL, &cvalue, &error_abort);
- g_assert(cvalue != NULL);
- g_assert_cmpint(cvalue->type, ==, USER_DEF_LIST_UNION_KIND_BOOLEAN);
-
- for (i = 0, elem = cvalue->u.boolean.data; elem; elem = elem->next, i++) {
- g_assert_cmpint(elem->value, ==, (i % 3 == 0) ? 1 : 0);
- }
-
- g_string_free(gstr_union, true);
- g_string_free(gstr_list, true);
-}
-
-static void test_visitor_in_list_union_string(TestInputVisitorData *data,
- const void *unused)
-{
- g_autoptr(UserDefListUnion) cvalue = NULL;
- strList *elem = NULL;
- Visitor *v;
- GString *gstr_list = g_string_new("");
- GString *gstr_union = g_string_new("");
- int i;
-
- for (i = 0; i < 32; i++) {
- g_string_append_printf(gstr_list, "'%d'", i);
- if (i != 31) {
- g_string_append(gstr_list, ", ");
- }
- }
- g_string_append_printf(gstr_union, "{ 'type': 'string', 'data': [ %s ] }",
- gstr_list->str);
- v = visitor_input_test_init_raw(data, gstr_union->str);
-
- visit_type_UserDefListUnion(v, NULL, &cvalue, &error_abort);
- g_assert(cvalue != NULL);
- g_assert_cmpint(cvalue->type, ==, USER_DEF_LIST_UNION_KIND_STRING);
-
- for (i = 0, elem = cvalue->u.string.data; elem; elem = elem->next, i++) {
- gchar str[8];
- sprintf(str, "%d", i);
- g_assert_cmpstr(elem->value, ==, str);
- }
-
- g_string_free(gstr_union, true);
- g_string_free(gstr_list, true);
-}
-
-#define DOUBLE_STR_MAX 16
-
-static void test_visitor_in_list_union_number(TestInputVisitorData *data,
- const void *unused)
-{
- g_autoptr(UserDefListUnion) cvalue = NULL;
- numberList *elem = NULL;
- Visitor *v;
- GString *gstr_list = g_string_new("");
- GString *gstr_union = g_string_new("");
- int i;
-
- for (i = 0; i < 32; i++) {
- g_string_append_printf(gstr_list, "%f", (double)i / 3);
- if (i != 31) {
- g_string_append(gstr_list, ", ");
- }
- }
- g_string_append_printf(gstr_union, "{ 'type': 'number', 'data': [ %s ] }",
- gstr_list->str);
- v = visitor_input_test_init_raw(data, gstr_union->str);
-
- visit_type_UserDefListUnion(v, NULL, &cvalue, &error_abort);
- g_assert(cvalue != NULL);
- g_assert_cmpint(cvalue->type, ==, USER_DEF_LIST_UNION_KIND_NUMBER);
-
- for (i = 0, elem = cvalue->u.number.data; elem; elem = elem->next, i++) {
- GString *double_expected = g_string_new("");
- GString *double_actual = g_string_new("");
-
- g_string_printf(double_expected, "%.6f", (double)i / 3);
- g_string_printf(double_actual, "%.6f", elem->value);
- g_assert_cmpstr(double_expected->str, ==, double_actual->str);
-
- g_string_free(double_expected, true);
- g_string_free(double_actual, true);
- }
-
- g_string_free(gstr_union, true);
- g_string_free(gstr_list, true);
-}
-
static void input_visitor_test_add(const char *testpath,
const void *user_data,
void (*test_func)(TestInputVisitorData *data,
@@ -1184,21 +1059,6 @@ static void test_visitor_in_fail_list_nested(TestInputVisitorData *data,
visit_end_list(v, NULL);
}
-static void test_visitor_in_fail_union_list(TestInputVisitorData *data,
- const void *unused)
-{
- UserDefListUnion *tmp = NULL;
- Error *err = NULL;
- Visitor *v;
-
- v = visitor_input_test_init(data,
- "{ 'type': 'integer', 'data' : [ 'string' ] }");
-
- visit_type_UserDefListUnion(v, NULL, &tmp, &err);
- error_free_or_abort(&err);
- g_assert(!tmp);
-}
-
static void test_visitor_in_fail_union_flat(TestInputVisitorData *data,
const void *unused)
{
@@ -1206,7 +1066,7 @@ static void test_visitor_in_fail_union_flat(TestInputVisitorData *data,
Error *err = NULL;
Visitor *v;
- v = visitor_input_test_init(data, "{ 'string': 'c', 'integer': 41, 'boolean': true }");
+ v = visitor_input_test_init(data, "{ 'enum1': 'value2', 'string': 'c', 'integer': 41, 'boolean': true }");
visit_type_UserDefFlatUnion(v, NULL, &tmp, &err);
error_free_or_abort(&err);
@@ -1310,6 +1170,8 @@ int main(int argc, char **argv)
NULL, test_visitor_in_struct);
input_visitor_test_add("/visitor/input/struct-nested",
NULL, test_visitor_in_struct_nested);
+ input_visitor_test_add("/visitor/input/list2",
+ NULL, test_visitor_in_list_struct);
input_visitor_test_add("/visitor/input/list",
NULL, test_visitor_in_list);
input_visitor_test_add("/visitor/input/any",
@@ -1326,30 +1188,6 @@ int main(int argc, char **argv)
NULL, test_visitor_in_wrong_type);
input_visitor_test_add("/visitor/input/alternate-number",
NULL, test_visitor_in_alternate_number);
- input_visitor_test_add("/visitor/input/list_union/int",
- NULL, test_visitor_in_list_union_int);
- input_visitor_test_add("/visitor/input/list_union/int8",
- NULL, test_visitor_in_list_union_int8);
- input_visitor_test_add("/visitor/input/list_union/int16",
- NULL, test_visitor_in_list_union_int16);
- input_visitor_test_add("/visitor/input/list_union/int32",
- NULL, test_visitor_in_list_union_int32);
- input_visitor_test_add("/visitor/input/list_union/int64",
- NULL, test_visitor_in_list_union_int64);
- input_visitor_test_add("/visitor/input/list_union/uint8",
- NULL, test_visitor_in_list_union_uint8);
- input_visitor_test_add("/visitor/input/list_union/uint16",
- NULL, test_visitor_in_list_union_uint16);
- input_visitor_test_add("/visitor/input/list_union/uint32",
- NULL, test_visitor_in_list_union_uint32);
- input_visitor_test_add("/visitor/input/list_union/uint64",
- NULL, test_visitor_in_list_union_uint64);
- input_visitor_test_add("/visitor/input/list_union/bool",
- NULL, test_visitor_in_list_union_bool);
- input_visitor_test_add("/visitor/input/list_union/str",
- 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/fail/struct",
NULL, test_visitor_in_fail_struct);
input_visitor_test_add("/visitor/input/fail/struct-nested",
@@ -1368,8 +1206,6 @@ int main(int argc, char **argv)
NULL, test_visitor_in_fail_union_flat_no_discrim);
input_visitor_test_add("/visitor/input/fail/alternate",
NULL, test_visitor_in_fail_alternate);
- input_visitor_test_add("/visitor/input/fail/union-list",
- NULL, test_visitor_in_fail_union_list);
input_visitor_test_add("/visitor/input/qapi-introspect",
NULL, test_visitor_in_qmp_introspect);
--
2.31.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH 12/22] test-qobject-input-visitor: Wean off UserDefListUnion
2021-09-13 12:39 ` [PATCH 12/22] test-qobject-input-visitor: Wean off UserDefListUnion Markus Armbruster
@ 2021-09-13 15:06 ` Eric Blake
2021-09-14 5:50 ` Markus Armbruster
0 siblings, 1 reply; 54+ messages in thread
From: Eric Blake @ 2021-09-13 15:06 UTC (permalink / raw)
To: Markus Armbruster; +Cc: marcandre.lureau, jsnow, qemu-devel
On Mon, Sep 13, 2021 at 02:39:22PM +0200, Markus Armbruster wrote:
> The test_visitor_in_list_union_FOO() use simple union UserDefListUnion
> to cover lists of builtin types. Rewrite as
> test_visitor_in_list_struct(), using struct ArrayStruct and a lot less
> code.
>
> test_visitor_in_fail_union_list() uses UserDefListUnion to cover
> "variant members don't match the discriminator value". Cover that in
> test_visitor_in_fail_union_flat() instead, and drop
> test_visitor_in_fail_union_list(). Appropriating the former for this
> purpose is okay, because it actually failed due to missing
> discriminator, which is still covered by
> test_visitor_in_fail_union_flat_no_discrim().
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> tests/unit/test-qobject-input-visitor.c | 460 ++++++++----------------
> 1 file changed, 148 insertions(+), 312 deletions(-)
>
> @@ -1206,7 +1066,7 @@ static void test_visitor_in_fail_union_flat(TestInputVisitorData *data,
> Error *err = NULL;
> Visitor *v;
>
> - v = visitor_input_test_init(data, "{ 'string': 'c', 'integer': 41, 'boolean': true }");
> + v = visitor_input_test_init(data, "{ 'enum1': 'value2', 'string': 'c', 'integer': 41, 'boolean': true }");
Long line; do we care?
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [PATCH 12/22] test-qobject-input-visitor: Wean off UserDefListUnion
2021-09-13 15:06 ` Eric Blake
@ 2021-09-14 5:50 ` Markus Armbruster
0 siblings, 0 replies; 54+ messages in thread
From: Markus Armbruster @ 2021-09-14 5:50 UTC (permalink / raw)
To: Eric Blake; +Cc: marcandre.lureau, jsnow, qemu-devel
Eric Blake <eblake@redhat.com> writes:
> On Mon, Sep 13, 2021 at 02:39:22PM +0200, Markus Armbruster wrote:
>> The test_visitor_in_list_union_FOO() use simple union UserDefListUnion
>> to cover lists of builtin types. Rewrite as
>> test_visitor_in_list_struct(), using struct ArrayStruct and a lot less
>> code.
>>
>> test_visitor_in_fail_union_list() uses UserDefListUnion to cover
>> "variant members don't match the discriminator value". Cover that in
>> test_visitor_in_fail_union_flat() instead, and drop
>> test_visitor_in_fail_union_list(). Appropriating the former for this
>> purpose is okay, because it actually failed due to missing
>> discriminator, which is still covered by
>> test_visitor_in_fail_union_flat_no_discrim().
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> tests/unit/test-qobject-input-visitor.c | 460 ++++++++----------------
>> 1 file changed, 148 insertions(+), 312 deletions(-)
>>
>> @@ -1206,7 +1066,7 @@ static void test_visitor_in_fail_union_flat(TestInputVisitorData *data,
>> Error *err = NULL;
>> Visitor *v;
>>
>> - v = visitor_input_test_init(data, "{ 'string': 'c', 'integer': 41, 'boolean': true }");
>> + v = visitor_input_test_init(data, "{ 'enum1': 'value2', 'string': 'c', 'integer': 41, 'boolean': true }");
>
> Long line; do we care?
We have loads of similar lines in unit tests, and I've given up on
them.
> Reviewed-by: Eric Blake <eblake@redhat.com>
Thanks!
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH 13/22] test-qobject-output-visitor: Wean off UserDefListUnion
2021-09-13 12:39 [PATCH 00/22] qapi: Remove simple unions from the schema language Markus Armbruster
` (11 preceding siblings ...)
2021-09-13 12:39 ` [PATCH 12/22] test-qobject-input-visitor: Wean off UserDefListUnion Markus Armbruster
@ 2021-09-13 12:39 ` Markus Armbruster
2021-09-13 15:09 ` Eric Blake
2021-09-13 12:39 ` [PATCH 14/22] test-clone-visitor: " Markus Armbruster
` (9 subsequent siblings)
22 siblings, 1 reply; 54+ messages in thread
From: Markus Armbruster @ 2021-09-13 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, eblake, marcandre.lureau
The test_visitor_out_list_union_FOO() use simple union
UserDefListUnion to cover lists of builtin types. Rewrite as
test_visitor_out_list_struct(), using struct ArrayStruct and a lot
less code.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/unit/test-qobject-output-visitor.c | 391 ++++++-----------------
1 file changed, 93 insertions(+), 298 deletions(-)
diff --git a/tests/unit/test-qobject-output-visitor.c b/tests/unit/test-qobject-output-visitor.c
index 9dc1e075e7..34d67a439a 100644
--- a/tests/unit/test-qobject-output-visitor.c
+++ b/tests/unit/test-qobject-output-visitor.c
@@ -437,289 +437,118 @@ static void test_visitor_out_null(TestOutputVisitorData *data,
g_assert(qobject_type(nil) == QTYPE_QNULL);
}
-static void init_list_union(UserDefListUnion *cvalue)
-{
- int i;
- switch (cvalue->type) {
- case USER_DEF_LIST_UNION_KIND_INTEGER: {
- intList **tail = &cvalue->u.integer.data;
- for (i = 0; i < 32; i++) {
- QAPI_LIST_APPEND(tail, i);
- }
- break;
- }
- case USER_DEF_LIST_UNION_KIND_S8: {
- int8List **tail = &cvalue->u.s8.data;
- for (i = 0; i < 32; i++) {
- QAPI_LIST_APPEND(tail, i);
- }
- break;
- }
- case USER_DEF_LIST_UNION_KIND_S16: {
- int16List **tail = &cvalue->u.s16.data;
- for (i = 0; i < 32; i++) {
- QAPI_LIST_APPEND(tail, i);
- }
- break;
- }
- case USER_DEF_LIST_UNION_KIND_S32: {
- int32List **tail = &cvalue->u.s32.data;
- for (i = 0; i < 32; i++) {
- QAPI_LIST_APPEND(tail, i);
- }
- break;
- }
- case USER_DEF_LIST_UNION_KIND_S64: {
- int64List **tail = &cvalue->u.s64.data;
- for (i = 0; i < 32; i++) {
- QAPI_LIST_APPEND(tail, i);
- }
- break;
- }
- case USER_DEF_LIST_UNION_KIND_U8: {
- uint8List **tail = &cvalue->u.u8.data;
- for (i = 0; i < 32; i++) {
- QAPI_LIST_APPEND(tail, i);
- }
- break;
- }
- case USER_DEF_LIST_UNION_KIND_U16: {
- uint16List **tail = &cvalue->u.u16.data;
- for (i = 0; i < 32; i++) {
- QAPI_LIST_APPEND(tail, i);
- }
- break;
- }
- case USER_DEF_LIST_UNION_KIND_U32: {
- uint32List **tail = &cvalue->u.u32.data;
- for (i = 0; i < 32; i++) {
- QAPI_LIST_APPEND(tail, i);
- }
- break;
- }
- case USER_DEF_LIST_UNION_KIND_U64: {
- uint64List **tail = &cvalue->u.u64.data;
- for (i = 0; i < 32; i++) {
- QAPI_LIST_APPEND(tail, i);
- }
- break;
- }
- case USER_DEF_LIST_UNION_KIND_BOOLEAN: {
- boolList **tail = &cvalue->u.boolean.data;
- for (i = 0; i < 32; i++) {
- QAPI_LIST_APPEND(tail, QEMU_IS_ALIGNED(i, 3));
- }
- break;
- }
- case USER_DEF_LIST_UNION_KIND_STRING: {
- strList **tail = &cvalue->u.string.data;
- for (i = 0; i < 32; i++) {
- QAPI_LIST_APPEND(tail, g_strdup_printf("%d", i));
- }
- break;
- }
- case USER_DEF_LIST_UNION_KIND_NUMBER: {
- numberList **tail = &cvalue->u.number.data;
- for (i = 0; i < 32; i++) {
- QAPI_LIST_APPEND(tail, (double)i / 3);
- }
- break;
- }
- default:
- g_assert_not_reached();
- }
-}
-
-static void check_list_union(QObject *qobj,
- UserDefListUnionKind kind)
+static void test_visitor_out_list_struct(TestOutputVisitorData *data,
+ const void *unused)
{
+ const char *int_member[] = {
+ "integer", "s8", "s16", "s32", "s64", "u8", "u16", "u32", "u64" };
+ g_autoptr(ArrayStruct) arrs = g_new0(ArrayStruct, 1);
+ int i, j;
QDict *qdict;
QList *qlist;
- int i;
+ QListEntry *e;
- qdict = qobject_to(QDict, qobj);
- g_assert(qdict);
- g_assert(qdict_haskey(qdict, "data"));
- qlist = qlist_copy(qobject_to(QList, qdict_get(qdict, "data")));
-
- switch (kind) {
- case USER_DEF_LIST_UNION_KIND_U8:
- case USER_DEF_LIST_UNION_KIND_U16:
- case USER_DEF_LIST_UNION_KIND_U32:
- case USER_DEF_LIST_UNION_KIND_U64:
- for (i = 0; i < 32; i++) {
- QObject *tmp;
- QNum *qvalue;
- uint64_t val;
-
- tmp = qlist_peek(qlist);
- g_assert(tmp);
- qvalue = qobject_to(QNum, tmp);
- g_assert(qnum_get_try_uint(qvalue, &val));
- g_assert_cmpint(val, ==, i);
- qobject_unref(qlist_pop(qlist));
- }
- break;
-
- case USER_DEF_LIST_UNION_KIND_S8:
- case USER_DEF_LIST_UNION_KIND_S16:
- case USER_DEF_LIST_UNION_KIND_S32:
- case USER_DEF_LIST_UNION_KIND_S64:
- /*
- * All integer elements in JSON arrays get stored into QNums
- * when we convert to QObjects, so we can check them all in
- * the same fashion, so simply fall through here.
- */
- case USER_DEF_LIST_UNION_KIND_INTEGER:
- for (i = 0; i < 32; i++) {
- QObject *tmp;
- QNum *qvalue;
- int64_t val;
-
- tmp = qlist_peek(qlist);
- g_assert(tmp);
- qvalue = qobject_to(QNum, tmp);
- g_assert(qnum_get_try_int(qvalue, &val));
- g_assert_cmpint(val, ==, i);
- qobject_unref(qlist_pop(qlist));
- }
- break;
- case USER_DEF_LIST_UNION_KIND_BOOLEAN:
- for (i = 0; i < 32; i++) {
- QObject *tmp;
- QBool *qvalue;
- tmp = qlist_peek(qlist);
- g_assert(tmp);
- qvalue = qobject_to(QBool, tmp);
- g_assert_cmpint(qbool_get_bool(qvalue), ==, i % 3 == 0);
- qobject_unref(qlist_pop(qlist));
- }
- break;
- case USER_DEF_LIST_UNION_KIND_STRING:
- for (i = 0; i < 32; i++) {
- QObject *tmp;
- QString *qvalue;
- gchar str[8];
- tmp = qlist_peek(qlist);
- g_assert(tmp);
- qvalue = qobject_to(QString, tmp);
- sprintf(str, "%d", i);
- g_assert_cmpstr(qstring_get_str(qvalue), ==, str);
- qobject_unref(qlist_pop(qlist));
- }
- break;
- case USER_DEF_LIST_UNION_KIND_NUMBER:
- for (i = 0; i < 32; i++) {
- QObject *tmp;
- QNum *qvalue;
- GString *double_expected = g_string_new("");
- GString *double_actual = g_string_new("");
-
- tmp = qlist_peek(qlist);
- g_assert(tmp);
- qvalue = qobject_to(QNum, tmp);
- g_string_printf(double_expected, "%.6f", (double)i / 3);
- g_string_printf(double_actual, "%.6f", qnum_get_double(qvalue));
- g_assert_cmpstr(double_actual->str, ==, double_expected->str);
-
- qobject_unref(qlist_pop(qlist));
- g_string_free(double_expected, true);
- g_string_free(double_actual, true);
- }
- break;
- default:
- g_assert_not_reached();
+ for (i = 31; i >= 0; i--) {
+ QAPI_LIST_PREPEND(arrs->integer, i);
}
- qobject_unref(qlist);
-}
-static void test_list_union(TestOutputVisitorData *data,
- const void *unused,
- UserDefListUnionKind kind)
-{
- UserDefListUnion *cvalue = g_new0(UserDefListUnion, 1);
- QObject *obj;
+ for (i = 31; i >= 0; i--) {
+ QAPI_LIST_PREPEND(arrs->s8, i);
+ }
- cvalue->type = kind;
- init_list_union(cvalue);
+ for (i = 31; i >= 0; i--) {
+ QAPI_LIST_PREPEND(arrs->s16, i);
+ }
- visit_type_UserDefListUnion(data->ov, NULL, &cvalue, &error_abort);
+ for (i = 31; i >= 0; i--) {
+ QAPI_LIST_PREPEND(arrs->s32, i);
+ }
- obj = visitor_get(data);
- check_list_union(obj, cvalue->type);
- qapi_free_UserDefListUnion(cvalue);
-}
+ for (i = 31; i >= 0; i--) {
+ QAPI_LIST_PREPEND(arrs->s64, i);
+ }
-static void test_visitor_out_list_union_int(TestOutputVisitorData *data,
- const void *unused)
-{
- test_list_union(data, unused, USER_DEF_LIST_UNION_KIND_INTEGER);
-}
+ for (i = 31; i >= 0; i--) {
+ QAPI_LIST_PREPEND(arrs->u8, i);
+ }
-static void test_visitor_out_list_union_int8(TestOutputVisitorData *data,
- const void *unused)
-{
- test_list_union(data, unused, USER_DEF_LIST_UNION_KIND_S8);
-}
+ for (i = 31; i >= 0; i--) {
+ QAPI_LIST_PREPEND(arrs->u16, i);
+ }
-static void test_visitor_out_list_union_int16(TestOutputVisitorData *data,
- const void *unused)
-{
- test_list_union(data, unused, USER_DEF_LIST_UNION_KIND_S16);
-}
+ for (i = 31; i >= 0; i--) {
+ QAPI_LIST_PREPEND(arrs->u32, i);
+ }
-static void test_visitor_out_list_union_int32(TestOutputVisitorData *data,
- const void *unused)
-{
- test_list_union(data, unused, USER_DEF_LIST_UNION_KIND_S32);
-}
+ for (i = 31; i >= 0; i--) {
+ QAPI_LIST_PREPEND(arrs->u64, i);
+ }
-static void test_visitor_out_list_union_int64(TestOutputVisitorData *data,
- const void *unused)
-{
- test_list_union(data, unused, USER_DEF_LIST_UNION_KIND_S64);
-}
+ for (i = 31; i >= 0; i--) {
+ QAPI_LIST_PREPEND(arrs->number, (double)i / 3);
+ }
-static void test_visitor_out_list_union_uint8(TestOutputVisitorData *data,
- const void *unused)
-{
- test_list_union(data, unused, USER_DEF_LIST_UNION_KIND_U8);
-}
+ for (i = 31; i >= 0; i--) {
+ QAPI_LIST_PREPEND(arrs->boolean, QEMU_IS_ALIGNED(i, 3));
+ }
-static void test_visitor_out_list_union_uint16(TestOutputVisitorData *data,
- const void *unused)
-{
- test_list_union(data, unused, USER_DEF_LIST_UNION_KIND_U16);
-}
+ for (i = 31; i >= 0; i--) {
+ QAPI_LIST_PREPEND(arrs->string, g_strdup_printf("%d", i));
+ }
-static void test_visitor_out_list_union_uint32(TestOutputVisitorData *data,
- const void *unused)
-{
- test_list_union(data, unused, USER_DEF_LIST_UNION_KIND_U32);
-}
+ visit_type_ArrayStruct(data->ov, NULL, &arrs, &error_abort);
-static void test_visitor_out_list_union_uint64(TestOutputVisitorData *data,
- const void *unused)
-{
- test_list_union(data, unused, USER_DEF_LIST_UNION_KIND_U64);
-}
+ qdict = qobject_to(QDict, visitor_get(data));
+ g_assert(qdict);
-static void test_visitor_out_list_union_bool(TestOutputVisitorData *data,
- const void *unused)
-{
- test_list_union(data, unused, USER_DEF_LIST_UNION_KIND_BOOLEAN);
-}
+ for (i = 0; i < G_N_ELEMENTS(int_member); i++) {
+ qlist = qdict_get_qlist(qdict, int_member[i]);
+ g_assert(qlist);
+ j = 0;
+ QLIST_FOREACH_ENTRY(qlist, e) {
+ QNum *qvalue = qobject_to(QNum, qlist_entry_obj(e));
+ g_assert(qvalue);
+ g_assert_cmpint(qnum_get_int(qvalue), ==, j);
+ j++;
+ }
+ }
-static void test_visitor_out_list_union_str(TestOutputVisitorData *data,
- const void *unused)
-{
- test_list_union(data, unused, USER_DEF_LIST_UNION_KIND_STRING);
-}
+ qlist = qdict_get_qlist(qdict, "number");
+ g_assert(qlist);
+ i = 0;
+ QLIST_FOREACH_ENTRY(qlist, e) {
+ QNum *qvalue = qobject_to(QNum, qlist_entry_obj(e));
+ char expected[32], actual[32];
-static void test_visitor_out_list_union_number(TestOutputVisitorData *data,
- const void *unused)
-{
- test_list_union(data, unused, USER_DEF_LIST_UNION_KIND_NUMBER);
+ g_assert(qvalue);
+ sprintf(expected, "%.6f", (double)i / 3);
+ sprintf(actual, "%.6f", qnum_get_double(qvalue));
+ g_assert_cmpstr(actual, ==, expected);
+ i++;
+ }
+
+ qlist = qdict_get_qlist(qdict, "boolean");
+ g_assert(qlist);
+ i = 0;
+ QLIST_FOREACH_ENTRY(qlist, e) {
+ QBool *qvalue = qobject_to(QBool, qlist_entry_obj(e));
+ g_assert(qvalue);
+ g_assert_cmpint(qbool_get_bool(qvalue), ==, i % 3 == 0);
+ i++;
+ }
+
+ qlist = qdict_get_qlist(qdict, "string");
+ g_assert(qlist);
+ i = 0;
+ QLIST_FOREACH_ENTRY(qlist, e) {
+ QString *qvalue = qobject_to(QString, qlist_entry_obj(e));
+ char expected[32];
+
+ g_assert(qvalue);
+ sprintf(expected, "%d", i);
+ g_assert_cmpstr(qstring_get_str(qvalue), ==, expected);
+ i++;
+ }
}
static void output_visitor_test_add(const char *testpath,
@@ -764,42 +593,8 @@ int main(int argc, char **argv)
&out_visitor_data, test_visitor_out_alternate);
output_visitor_test_add("/visitor/output/null",
&out_visitor_data, test_visitor_out_null);
- output_visitor_test_add("/visitor/output/list_union/int",
- &out_visitor_data,
- test_visitor_out_list_union_int);
- output_visitor_test_add("/visitor/output/list_union/int8",
- &out_visitor_data,
- test_visitor_out_list_union_int8);
- output_visitor_test_add("/visitor/output/list_union/int16",
- &out_visitor_data,
- test_visitor_out_list_union_int16);
- output_visitor_test_add("/visitor/output/list_union/int32",
- &out_visitor_data,
- test_visitor_out_list_union_int32);
- output_visitor_test_add("/visitor/output/list_union/int64",
- &out_visitor_data,
- test_visitor_out_list_union_int64);
- output_visitor_test_add("/visitor/output/list_union/uint8",
- &out_visitor_data,
- test_visitor_out_list_union_uint8);
- output_visitor_test_add("/visitor/output/list_union/uint16",
- &out_visitor_data,
- test_visitor_out_list_union_uint16);
- output_visitor_test_add("/visitor/output/list_union/uint32",
- &out_visitor_data,
- test_visitor_out_list_union_uint32);
- output_visitor_test_add("/visitor/output/list_union/uint64",
- &out_visitor_data,
- test_visitor_out_list_union_uint64);
- output_visitor_test_add("/visitor/output/list_union/bool",
- &out_visitor_data,
- test_visitor_out_list_union_bool);
- output_visitor_test_add("/visitor/output/list_union/string",
- &out_visitor_data,
- test_visitor_out_list_union_str);
- output_visitor_test_add("/visitor/output/list_union/number",
- &out_visitor_data,
- test_visitor_out_list_union_number);
+ output_visitor_test_add("/visitor/output/list_struct",
+ &out_visitor_data, test_visitor_out_list_struct);
g_test_run();
--
2.31.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH 13/22] test-qobject-output-visitor: Wean off UserDefListUnion
2021-09-13 12:39 ` [PATCH 13/22] test-qobject-output-visitor: " Markus Armbruster
@ 2021-09-13 15:09 ` Eric Blake
0 siblings, 0 replies; 54+ messages in thread
From: Eric Blake @ 2021-09-13 15:09 UTC (permalink / raw)
To: Markus Armbruster; +Cc: marcandre.lureau, jsnow, qemu-devel
On Mon, Sep 13, 2021 at 02:39:23PM +0200, Markus Armbruster wrote:
> The test_visitor_out_list_union_FOO() use simple union
> UserDefListUnion to cover lists of builtin types. Rewrite as
> test_visitor_out_list_struct(), using struct ArrayStruct and a lot
> less code.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> tests/unit/test-qobject-output-visitor.c | 391 ++++++-----------------
> 1 file changed, 93 insertions(+), 298 deletions(-)
>
Nice simplification.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH 14/22] test-clone-visitor: Wean off UserDefListUnion
2021-09-13 12:39 [PATCH 00/22] qapi: Remove simple unions from the schema language Markus Armbruster
` (12 preceding siblings ...)
2021-09-13 12:39 ` [PATCH 13/22] test-qobject-output-visitor: " Markus Armbruster
@ 2021-09-13 12:39 ` Markus Armbruster
2021-09-13 15:10 ` Eric Blake
2021-09-13 12:39 ` [PATCH 15/22] tests/qapi-schema: " Markus Armbruster
` (8 subsequent siblings)
22 siblings, 1 reply; 54+ messages in thread
From: Markus Armbruster @ 2021-09-13 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, eblake, marcandre.lureau
test_clone_complex1() uses simple union UserDefListUnion to cover
unions. Use UserDefFlatUnion instead. Arrays are still covered by
test_clone_complex3().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/unit/test-clone-visitor.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/tests/unit/test-clone-visitor.c b/tests/unit/test-clone-visitor.c
index 4944b3d857..8357a90e60 100644
--- a/tests/unit/test-clone-visitor.c
+++ b/tests/unit/test-clone-visitor.c
@@ -99,18 +99,26 @@ static void test_clone_empty(void)
static void test_clone_complex1(void)
{
- UserDefListUnion *src, *dst;
+ UserDefFlatUnion *src, *dst;
- src = g_new0(UserDefListUnion, 1);
- src->type = USER_DEF_LIST_UNION_KIND_STRING;
+ src = g_new0(UserDefFlatUnion, 1);
+ src->integer = 123;
+ src->string = g_strdup("abc");
+ src->enum1 = ENUM_ONE_VALUE1;
+ src->u.value1.boolean = true;
- dst = QAPI_CLONE(UserDefListUnion, src);
+ dst = QAPI_CLONE(UserDefFlatUnion, src);
g_assert(dst);
- g_assert_cmpint(dst->type, ==, src->type);
- g_assert(!dst->u.string.data);
- qapi_free_UserDefListUnion(src);
- qapi_free_UserDefListUnion(dst);
+ g_assert_cmpint(dst->integer, ==, 123);
+ g_assert_cmpstr(dst->string, ==, "abc");
+ g_assert_cmpint(dst->enum1, ==, ENUM_ONE_VALUE1);
+ g_assert(dst->u.value1.boolean);
+ g_assert(!dst->u.value1.has_a_b);
+ g_assert_cmpint(dst->u.value1.a_b, ==, 0);
+
+ qapi_free_UserDefFlatUnion(src);
+ qapi_free_UserDefFlatUnion(dst);
}
static void test_clone_complex2(void)
--
2.31.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH 14/22] test-clone-visitor: Wean off UserDefListUnion
2021-09-13 12:39 ` [PATCH 14/22] test-clone-visitor: " Markus Armbruster
@ 2021-09-13 15:10 ` Eric Blake
0 siblings, 0 replies; 54+ messages in thread
From: Eric Blake @ 2021-09-13 15:10 UTC (permalink / raw)
To: Markus Armbruster; +Cc: marcandre.lureau, jsnow, qemu-devel
On Mon, Sep 13, 2021 at 02:39:24PM +0200, Markus Armbruster wrote:
> test_clone_complex1() uses simple union UserDefListUnion to cover
> unions. Use UserDefFlatUnion instead. Arrays are still covered by
> test_clone_complex3().
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> tests/unit/test-clone-visitor.c | 24 ++++++++++++++++--------
> 1 file changed, 16 insertions(+), 8 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH 15/22] tests/qapi-schema: Wean off UserDefListUnion
2021-09-13 12:39 [PATCH 00/22] qapi: Remove simple unions from the schema language Markus Armbruster
` (13 preceding siblings ...)
2021-09-13 12:39 ` [PATCH 14/22] test-clone-visitor: " Markus Armbruster
@ 2021-09-13 12:39 ` Markus Armbruster
2021-09-13 15:26 ` Eric Blake
2021-09-13 12:39 ` [PATCH 16/22] tests/qapi-schema: Simple union UserDefListUnion is now unused, drop Markus Armbruster
` (7 subsequent siblings)
22 siblings, 1 reply; 54+ messages in thread
From: Markus Armbruster @ 2021-09-13 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, eblake, marcandre.lureau
Command boxed-union uses simple union UserDefListUnion to cover
unions. Use UserDefFlatUnion instead.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/unit/test-qmp-cmds.c | 2 +-
tests/qapi-schema/qapi-schema-test.json | 2 +-
tests/qapi-schema/qapi-schema-test.out | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tests/unit/test-qmp-cmds.c b/tests/unit/test-qmp-cmds.c
index 83efa39720..83c9ef5b7c 100644
--- a/tests/unit/test-qmp-cmds.c
+++ b/tests/unit/test-qmp-cmds.c
@@ -119,7 +119,7 @@ void qmp_boxed_struct(UserDefZero *arg, Error **errp)
{
}
-void qmp_boxed_union(UserDefListUnion *arg, Error **errp)
+void qmp_boxed_union(UserDefFlatUnion *arg, Error **errp)
{
}
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index b2d795cb19..a4b4405f94 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -175,7 +175,7 @@
'returns': 'int' }
{ 'command': 'guest-sync', 'data': { 'arg': 'any' }, 'returns': 'any' }
{ 'command': 'boxed-struct', 'boxed': true, 'data': 'UserDefZero' }
-{ 'command': 'boxed-union', 'data': 'UserDefListUnion', 'boxed': true }
+{ 'command': 'boxed-union', 'data': 'UserDefFlatUnion', 'boxed': true }
{ 'command': 'boxed-empty', 'boxed': true, 'data': 'Empty1' }
# Smoke test on out-of-band and allow-preconfig-test
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 7a488c1d06..f120f10616 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -232,7 +232,7 @@ command guest-sync q_obj_guest-sync-arg -> any
gen=True success_response=True boxed=False oob=False preconfig=False
command boxed-struct UserDefZero -> None
gen=True success_response=True boxed=True oob=False preconfig=False
-command boxed-union UserDefListUnion -> None
+command boxed-union UserDefFlatUnion -> None
gen=True success_response=True boxed=True oob=False preconfig=False
command boxed-empty Empty1 -> None
gen=True success_response=True boxed=True oob=False preconfig=False
--
2.31.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH 15/22] tests/qapi-schema: Wean off UserDefListUnion
2021-09-13 12:39 ` [PATCH 15/22] tests/qapi-schema: " Markus Armbruster
@ 2021-09-13 15:26 ` Eric Blake
0 siblings, 0 replies; 54+ messages in thread
From: Eric Blake @ 2021-09-13 15:26 UTC (permalink / raw)
To: Markus Armbruster; +Cc: marcandre.lureau, jsnow, qemu-devel
On Mon, Sep 13, 2021 at 02:39:25PM +0200, Markus Armbruster wrote:
> Command boxed-union uses simple union UserDefListUnion to cover
> unions. Use UserDefFlatUnion instead.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> tests/unit/test-qmp-cmds.c | 2 +-
> tests/qapi-schema/qapi-schema-test.json | 2 +-
> tests/qapi-schema/qapi-schema-test.out | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH 16/22] tests/qapi-schema: Simple union UserDefListUnion is now unused, drop
2021-09-13 12:39 [PATCH 00/22] qapi: Remove simple unions from the schema language Markus Armbruster
` (14 preceding siblings ...)
2021-09-13 12:39 ` [PATCH 15/22] tests/qapi-schema: " Markus Armbruster
@ 2021-09-13 12:39 ` Markus Armbruster
2021-09-13 15:27 ` Eric Blake
2021-09-13 12:39 ` [PATCH 17/22] tests/qapi-schema: Rewrite simple union TestIfUnion to be flat Markus Armbruster
` (6 subsequent siblings)
22 siblings, 1 reply; 54+ messages in thread
From: Markus Armbruster @ 2021-09-13 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, eblake, marcandre.lureau
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/qapi-schema/qapi-schema-test.json | 17 -------
tests/qapi-schema/qapi-schema-test.out | 64 -------------------------
2 files changed, 81 deletions(-)
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index a4b4405f94..eae43f41c4 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -123,23 +123,6 @@
# for testing use of 'str' within alternates
{ 'alternate': 'AltStrObj', 'data': { 's': 'str', 'o': 'TestStruct' } }
-# for testing lists
-{ 'union': 'UserDefListUnion',
- 'data': { 'integer': ['int'],
- 's8': ['int8'],
- 's16': ['int16'],
- 's32': ['int32'],
- 's64': ['int64'],
- 'u8': ['uint8'],
- 'u16': ['uint16'],
- 'u32': ['uint32'],
- 'u64': ['uint64'],
- 'number': ['number'],
- 'boolean': ['bool'],
- 'string': ['str'],
- 'sizes': ['size'],
- 'any': ['any'],
- 'user': ['Status'] } } # intentional forward ref. to sub-module
{ 'struct': 'ArrayStruct',
'data': { 'integer': ['int'],
's8': ['int8'],
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index f120f10616..e43073d795 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -125,70 +125,6 @@ alternate AltStrObj
tag type
case s: str
case o: TestStruct
-object q_obj_intList-wrapper
- member data: intList optional=False
-object q_obj_int8List-wrapper
- member data: int8List optional=False
-object q_obj_int16List-wrapper
- member data: int16List optional=False
-object q_obj_int32List-wrapper
- member data: int32List optional=False
-object q_obj_int64List-wrapper
- member data: int64List optional=False
-object q_obj_uint8List-wrapper
- member data: uint8List optional=False
-object q_obj_uint16List-wrapper
- member data: uint16List optional=False
-object q_obj_uint32List-wrapper
- member data: uint32List optional=False
-object q_obj_uint64List-wrapper
- member data: uint64List optional=False
-object q_obj_numberList-wrapper
- member data: numberList optional=False
-object q_obj_boolList-wrapper
- member data: boolList optional=False
-object q_obj_strList-wrapper
- member data: strList optional=False
-object q_obj_sizeList-wrapper
- member data: sizeList optional=False
-object q_obj_anyList-wrapper
- member data: anyList optional=False
-object q_obj_StatusList-wrapper
- member data: StatusList optional=False
-enum UserDefListUnionKind
- member integer
- member s8
- member s16
- member s32
- member s64
- member u8
- member u16
- member u32
- member u64
- member number
- member boolean
- member string
- member sizes
- member any
- member user
-object UserDefListUnion
- member type: UserDefListUnionKind optional=False
- tag type
- case integer: q_obj_intList-wrapper
- case s8: q_obj_int8List-wrapper
- case s16: q_obj_int16List-wrapper
- case s32: q_obj_int32List-wrapper
- case s64: q_obj_int64List-wrapper
- case u8: q_obj_uint8List-wrapper
- case u16: q_obj_uint16List-wrapper
- case u32: q_obj_uint32List-wrapper
- case u64: q_obj_uint64List-wrapper
- case number: q_obj_numberList-wrapper
- case boolean: q_obj_boolList-wrapper
- case string: q_obj_strList-wrapper
- case sizes: q_obj_sizeList-wrapper
- case any: q_obj_anyList-wrapper
- case user: q_obj_StatusList-wrapper
object ArrayStruct
member integer: intList optional=False
member s8: int8List optional=False
--
2.31.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH 16/22] tests/qapi-schema: Simple union UserDefListUnion is now unused, drop
2021-09-13 12:39 ` [PATCH 16/22] tests/qapi-schema: Simple union UserDefListUnion is now unused, drop Markus Armbruster
@ 2021-09-13 15:27 ` Eric Blake
0 siblings, 0 replies; 54+ messages in thread
From: Eric Blake @ 2021-09-13 15:27 UTC (permalink / raw)
To: Markus Armbruster; +Cc: marcandre.lureau, jsnow, qemu-devel
On Mon, Sep 13, 2021 at 02:39:26PM +0200, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> tests/qapi-schema/qapi-schema-test.json | 17 -------
> tests/qapi-schema/qapi-schema-test.out | 64 -------------------------
> 2 files changed, 81 deletions(-)
Patches like these are fun to review ;)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH 17/22] tests/qapi-schema: Rewrite simple union TestIfUnion to be flat
2021-09-13 12:39 [PATCH 00/22] qapi: Remove simple unions from the schema language Markus Armbruster
` (15 preceding siblings ...)
2021-09-13 12:39 ` [PATCH 16/22] tests/qapi-schema: Simple union UserDefListUnion is now unused, drop Markus Armbruster
@ 2021-09-13 12:39 ` Markus Armbruster
2021-09-13 15:28 ` Eric Blake
2021-09-13 12:39 ` [PATCH 18/22] test-clone-visitor: Wean off __org.qemu_x-Union1 Markus Armbruster
` (5 subsequent siblings)
22 siblings, 1 reply; 54+ messages in thread
From: Markus Armbruster @ 2021-09-13 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, eblake, marcandre.lureau
Simple unions predate flat unions. Having both complicates the QAPI
schema language and the QAPI generator. We haven't been using simple
unions in new code for a long time, because they are less flexible and
somewhat awkward on the wire.
To prepare for their removal, rewrite TestIfUnion to be flat.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/qapi-schema/qapi-schema-test.json | 4 +++-
tests/qapi-schema/qapi-schema-test.out | 16 ++++++----------
2 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index eae43f41c4..ef17ab1aae 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -229,8 +229,10 @@
'if': 'TEST_IF_ENUM' }
{ 'union': 'TestIfUnion',
+ 'base': { 'type': 'TestIfEnum' },
+ 'discriminator': 'type',
'data': { 'foo': 'TestStruct',
- 'bar': { 'type': 'str', 'if': 'TEST_IF_UNION_BAR'} },
+ 'bar': { 'type': 'UserDefZero', 'if': 'TEST_IF_ENUM_BAR'} },
'if': { 'all': ['TEST_IF_UNION', 'TEST_IF_STRUCT'] } }
{ 'command': 'test-if-union-cmd',
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index e43073d795..07e4161331 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -257,19 +257,15 @@ enum TestIfEnum
member bar
if TEST_IF_ENUM_BAR
if TEST_IF_ENUM
-object q_obj_TestStruct-wrapper
- member data: TestStruct optional=False
-enum TestIfUnionKind
- member foo
- member bar
- if TEST_IF_UNION_BAR
+object q_obj_TestIfUnion-base
+ member type: TestIfEnum optional=False
if {'all': ['TEST_IF_UNION', 'TEST_IF_STRUCT']}
object TestIfUnion
- member type: TestIfUnionKind optional=False
+ base q_obj_TestIfUnion-base
tag type
- case foo: q_obj_TestStruct-wrapper
- case bar: q_obj_str-wrapper
- if TEST_IF_UNION_BAR
+ case foo: TestStruct
+ case bar: UserDefZero
+ if TEST_IF_ENUM_BAR
if {'all': ['TEST_IF_UNION', 'TEST_IF_STRUCT']}
object q_obj_test-if-union-cmd-arg
member union-cmd-arg: TestIfUnion optional=False
--
2.31.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH 17/22] tests/qapi-schema: Rewrite simple union TestIfUnion to be flat
2021-09-13 12:39 ` [PATCH 17/22] tests/qapi-schema: Rewrite simple union TestIfUnion to be flat Markus Armbruster
@ 2021-09-13 15:28 ` Eric Blake
0 siblings, 0 replies; 54+ messages in thread
From: Eric Blake @ 2021-09-13 15:28 UTC (permalink / raw)
To: Markus Armbruster; +Cc: marcandre.lureau, jsnow, qemu-devel
On Mon, Sep 13, 2021 at 02:39:27PM +0200, Markus Armbruster wrote:
> Simple unions predate flat unions. Having both complicates the QAPI
> schema language and the QAPI generator. We haven't been using simple
> unions in new code for a long time, because they are less flexible and
> somewhat awkward on the wire.
>
> To prepare for their removal, rewrite TestIfUnion to be flat.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> tests/qapi-schema/qapi-schema-test.json | 4 +++-
> tests/qapi-schema/qapi-schema-test.out | 16 ++++++----------
> 2 files changed, 9 insertions(+), 11 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH 18/22] test-clone-visitor: Wean off __org.qemu_x-Union1
2021-09-13 12:39 [PATCH 00/22] qapi: Remove simple unions from the schema language Markus Armbruster
` (16 preceding siblings ...)
2021-09-13 12:39 ` [PATCH 17/22] tests/qapi-schema: Rewrite simple union TestIfUnion to be flat Markus Armbruster
@ 2021-09-13 12:39 ` Markus Armbruster
2021-09-13 15:32 ` Eric Blake
2021-09-13 12:39 ` [PATCH 19/22] tests/qapi-schema: Drop simple union __org.qemu_x-Union1 Markus Armbruster
` (4 subsequent siblings)
22 siblings, 1 reply; 54+ messages in thread
From: Markus Armbruster @ 2021-09-13 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, eblake, marcandre.lureau
test_clone_complex3() uses simple union __org.qemu_x-Union1 to cover
arrays. Use UserDefOneList instead. Unions are still covered by
test_clone_complex1().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/unit/test-clone-visitor.c | 70 ++++++++++++++++++---------------
1 file changed, 38 insertions(+), 32 deletions(-)
diff --git a/tests/unit/test-clone-visitor.c b/tests/unit/test-clone-visitor.c
index 8357a90e60..4048018607 100644
--- a/tests/unit/test-clone-visitor.c
+++ b/tests/unit/test-clone-visitor.c
@@ -153,42 +153,48 @@ static void test_clone_complex2(void)
static void test_clone_complex3(void)
{
- __org_qemu_x_Struct2 *src, *dst;
- __org_qemu_x_Union1List *tmp;
+ UserDefOneList *src, *dst, *tail;
+ UserDefOne *elt;
- src = g_new0(__org_qemu_x_Struct2, 1);
- tmp = src->array = g_new0(__org_qemu_x_Union1List, 1);
- tmp->value = g_new0(__org_qemu_x_Union1, 1);
- tmp->value->type = ORG_QEMU_X_UNION1_KIND___ORG_QEMU_X_BRANCH;
- tmp->value->u.__org_qemu_x_branch.data = g_strdup("one");
- tmp = tmp->next = g_new0(__org_qemu_x_Union1List, 1);
- tmp->value = g_new0(__org_qemu_x_Union1, 1);
- tmp->value->type = ORG_QEMU_X_UNION1_KIND___ORG_QEMU_X_BRANCH;
- tmp->value->u.__org_qemu_x_branch.data = g_strdup("two");
- tmp = tmp->next = g_new0(__org_qemu_x_Union1List, 1);
- tmp->value = g_new0(__org_qemu_x_Union1, 1);
- tmp->value->type = ORG_QEMU_X_UNION1_KIND___ORG_QEMU_X_BRANCH;
- tmp->value->u.__org_qemu_x_branch.data = g_strdup("three");
+ src = NULL;
+ elt = g_new0(UserDefOne, 1);
+ elt->integer = 3;
+ elt->string = g_strdup("three");
+ elt->has_enum1 = true;
+ elt->enum1 = ENUM_ONE_VALUE3;
+ QAPI_LIST_PREPEND(src, elt);
+ elt = g_new0(UserDefOne, 1);
+ elt->integer = 2;
+ elt->string = g_strdup("two");
+ QAPI_LIST_PREPEND(src, elt);
+ elt = g_new0(UserDefOne, 1);
+ elt->integer = 1;
+ elt->string = g_strdup("one");
+ QAPI_LIST_PREPEND(src, elt);
+
+ dst = QAPI_CLONE(UserDefOneList, src);
- dst = QAPI_CLONE(__org_qemu_x_Struct2, src);
g_assert(dst);
- tmp = dst->array;
- g_assert(tmp);
- g_assert(tmp->value);
- g_assert_cmpstr(tmp->value->u.__org_qemu_x_branch.data, ==, "one");
- tmp = tmp->next;
- g_assert(tmp);
- g_assert(tmp->value);
- g_assert_cmpstr(tmp->value->u.__org_qemu_x_branch.data, ==, "two");
- tmp = tmp->next;
- g_assert(tmp);
- g_assert(tmp->value);
- g_assert_cmpstr(tmp->value->u.__org_qemu_x_branch.data, ==, "three");
- tmp = tmp->next;
- g_assert(!tmp);
+ tail = dst;
+ elt = tail->value;
+ g_assert_cmpint(elt->integer, ==, 1);
+ g_assert_cmpstr(elt->string, ==, "one");
+ g_assert(!elt->has_enum1);
+ tail = tail->next;
+ elt = tail->value;
+ g_assert_cmpint(elt->integer, ==, 2);
+ g_assert_cmpstr(elt->string, ==, "two");
+ g_assert(!elt->has_enum1);
+ tail = tail->next;
+ elt = tail->value;
+ g_assert_cmpint(elt->integer, ==, 3);
+ g_assert_cmpstr(elt->string, ==, "three");
+ g_assert(elt->has_enum1);
+ g_assert_cmpint(elt->enum1, ==, ENUM_ONE_VALUE3);
+ g_assert(!tail->next);
- qapi_free___org_qemu_x_Struct2(src);
- qapi_free___org_qemu_x_Struct2(dst);
+ qapi_free_UserDefOneList(src);
+ qapi_free_UserDefOneList(dst);
}
int main(int argc, char **argv)
--
2.31.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH 18/22] test-clone-visitor: Wean off __org.qemu_x-Union1
2021-09-13 12:39 ` [PATCH 18/22] test-clone-visitor: Wean off __org.qemu_x-Union1 Markus Armbruster
@ 2021-09-13 15:32 ` Eric Blake
0 siblings, 0 replies; 54+ messages in thread
From: Eric Blake @ 2021-09-13 15:32 UTC (permalink / raw)
To: Markus Armbruster; +Cc: marcandre.lureau, jsnow, qemu-devel
On Mon, Sep 13, 2021 at 02:39:28PM +0200, Markus Armbruster wrote:
> test_clone_complex3() uses simple union __org.qemu_x-Union1 to cover
> arrays. Use UserDefOneList instead. Unions are still covered by
> test_clone_complex1().
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> tests/unit/test-clone-visitor.c | 70 ++++++++++++++++++---------------
> 1 file changed, 38 insertions(+), 32 deletions(-)
>
> diff --git a/tests/unit/test-clone-visitor.c b/tests/unit/test-clone-visitor.c
> index 8357a90e60..4048018607 100644
> --- a/tests/unit/test-clone-visitor.c
> +++ b/tests/unit/test-clone-visitor.c
> @@ -153,42 +153,48 @@ static void test_clone_complex2(void)
>
> static void test_clone_complex3(void)
> {
> - __org_qemu_x_Struct2 *src, *dst;
> - __org_qemu_x_Union1List *tmp;
> + UserDefOneList *src, *dst, *tail;
> + UserDefOne *elt;
This unit test loses coverge of RFQDN downstream extensions, but I
think we still adequately cover that elsewhere in the testsuite, and
that it was not the primary focus of this test. Meanwhile, what this
test is really focused on (an accurate clone of a union) is still
preserved.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH 19/22] tests/qapi-schema: Drop simple union __org.qemu_x-Union1
2021-09-13 12:39 [PATCH 00/22] qapi: Remove simple unions from the schema language Markus Armbruster
` (17 preceding siblings ...)
2021-09-13 12:39 ` [PATCH 18/22] test-clone-visitor: Wean off __org.qemu_x-Union1 Markus Armbruster
@ 2021-09-13 12:39 ` Markus Armbruster
2021-09-13 15:35 ` Eric Blake
2021-09-13 12:39 ` [PATCH 20/22] tests/qapi-schema: Purge simple unions from tests Markus Armbruster
` (3 subsequent siblings)
22 siblings, 1 reply; 54+ messages in thread
From: Markus Armbruster @ 2021-09-13 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, eblake, marcandre.lureau
Replace simple union __org.qemu_x-Union1 flat union
__org.qemu_x-Union2, except drop it from __org.qemu_x-command, because
there it's only used to pull it into QMP. Now drop the unused simple
union.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/unit/test-qmp-cmds.c | 16 +++++-----------
tests/qapi-schema/qapi-schema-test.json | 6 ++----
tests/qapi-schema/qapi-schema-test.out | 14 +++-----------
3 files changed, 10 insertions(+), 26 deletions(-)
diff --git a/tests/unit/test-qmp-cmds.c b/tests/unit/test-qmp-cmds.c
index 83c9ef5b7c..a43b97d6c5 100644
--- a/tests/unit/test-qmp-cmds.c
+++ b/tests/unit/test-qmp-cmds.c
@@ -127,22 +127,16 @@ void qmp_boxed_empty(Empty1 *arg, Error **errp)
{
}
-__org_qemu_x_Union1 *qmp___org_qemu_x_command(__org_qemu_x_EnumList *a,
- __org_qemu_x_StructList *b,
- __org_qemu_x_Union2 *c,
- __org_qemu_x_Alt *d,
- Error **errp)
+void qmp___org_qemu_x_command(__org_qemu_x_EnumList *a,
+ __org_qemu_x_StructList *b,
+ __org_qemu_x_Union2 *c,
+ __org_qemu_x_Alt *d,
+ Error **errp)
{
- __org_qemu_x_Union1 *ret = g_new0(__org_qemu_x_Union1, 1);
-
- ret->type = ORG_QEMU_X_UNION1_KIND___ORG_QEMU_X_BRANCH;
- ret->u.__org_qemu_x_branch.data = strdup("blah1");
-
/* Also test that 'wchar-t' was munged to 'q_wchar_t' */
if (b && b->value && !b->value->has_q_wchar_t) {
b->value->q_wchar_t = 1;
}
- return ret;
}
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index ef17ab1aae..0c19d4820e 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -202,10 +202,9 @@
'data': { '__org.qemu_x-member1': '__org.qemu_x-Enum' } }
{ 'struct': '__org.qemu_x-Struct', 'base': '__org.qemu_x-Base',
'data': { '__org.qemu_x-member2': 'str', '*wchar-t': 'int' } }
-{ 'union': '__org.qemu_x-Union1', 'data': { '__org.qemu_x-branch': 'str' } }
{ 'alternate': '__org.qemu_x-Alt1', 'data': { '__org.qemu_x-branch': 'str' } }
{ 'struct': '__org.qemu_x-Struct2',
- 'data': { 'array': ['__org.qemu_x-Union1'] } }
+ 'data': { 'array': ['__org.qemu_x-Union2'] } }
{ 'union': '__org.qemu_x-Union2', 'base': '__org.qemu_x-Base',
'discriminator': '__org.qemu_x-member1',
'data': { '__org.qemu_x-value': '__org.qemu_x-Struct2' } }
@@ -214,8 +213,7 @@
{ 'event': '__ORG.QEMU_X-EVENT', 'data': '__org.qemu_x-Struct' }
{ 'command': '__org.qemu_x-command',
'data': { 'a': ['__org.qemu_x-Enum'], 'b': ['__org.qemu_x-Struct'],
- 'c': '__org.qemu_x-Union2', 'd': '__org.qemu_x-Alt' },
- 'returns': '__org.qemu_x-Union1' }
+ 'c': '__org.qemu_x-Union2', 'd': '__org.qemu_x-Alt' } }
# test 'if' condition handling
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 07e4161331..0b49dc3044 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -215,20 +215,12 @@ object __org.qemu_x-Struct
base __org.qemu_x-Base
member __org.qemu_x-member2: str optional=False
member wchar-t: int optional=True
-object q_obj_str-wrapper
- member data: str optional=False
-enum __org.qemu_x-Union1Kind
- member __org.qemu_x-branch
-object __org.qemu_x-Union1
- member type: __org.qemu_x-Union1Kind optional=False
- tag type
- case __org.qemu_x-branch: q_obj_str-wrapper
alternate __org.qemu_x-Alt1
tag type
case __org.qemu_x-branch: str
-array __org.qemu_x-Union1List __org.qemu_x-Union1
+array __org.qemu_x-Union2List __org.qemu_x-Union2
object __org.qemu_x-Struct2
- member array: __org.qemu_x-Union1List optional=False
+ member array: __org.qemu_x-Union2List optional=False
object __org.qemu_x-Union2
base __org.qemu_x-Base
tag __org.qemu_x-member1
@@ -245,7 +237,7 @@ object q_obj___org.qemu_x-command-arg
member b: __org.qemu_x-StructList optional=False
member c: __org.qemu_x-Union2 optional=False
member d: __org.qemu_x-Alt optional=False
-command __org.qemu_x-command q_obj___org.qemu_x-command-arg -> __org.qemu_x-Union1
+command __org.qemu_x-command q_obj___org.qemu_x-command-arg -> None
gen=True success_response=True boxed=False oob=False preconfig=False
object TestIfStruct
member foo: int optional=False
--
2.31.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH 19/22] tests/qapi-schema: Drop simple union __org.qemu_x-Union1
2021-09-13 12:39 ` [PATCH 19/22] tests/qapi-schema: Drop simple union __org.qemu_x-Union1 Markus Armbruster
@ 2021-09-13 15:35 ` Eric Blake
2021-09-14 5:55 ` Markus Armbruster
0 siblings, 1 reply; 54+ messages in thread
From: Eric Blake @ 2021-09-13 15:35 UTC (permalink / raw)
To: Markus Armbruster; +Cc: marcandre.lureau, jsnow, qemu-devel
On Mon, Sep 13, 2021 at 02:39:29PM +0200, Markus Armbruster wrote:
> Replace simple union __org.qemu_x-Union1 flat union
missing 'with'
> __org.qemu_x-Union2, except drop it from __org.qemu_x-command, because
> there it's only used to pull it into QMP. Now drop the unused simple
> union.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> tests/unit/test-qmp-cmds.c | 16 +++++-----------
> tests/qapi-schema/qapi-schema-test.json | 6 ++----
> tests/qapi-schema/qapi-schema-test.out | 14 +++-----------
> 3 files changed, 10 insertions(+), 26 deletions(-)
>
Looks a bit odd to leave things with Union2 but not Union1; up to you
if it is worth a further cleanup to rename what remains to get rid of
the odd gap. I don't think it's a show-stopper for your series to
keep the naming as-is, though.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 19/22] tests/qapi-schema: Drop simple union __org.qemu_x-Union1
2021-09-13 15:35 ` Eric Blake
@ 2021-09-14 5:55 ` Markus Armbruster
0 siblings, 0 replies; 54+ messages in thread
From: Markus Armbruster @ 2021-09-14 5:55 UTC (permalink / raw)
To: Eric Blake; +Cc: marcandre.lureau, jsnow, qemu-devel
Eric Blake <eblake@redhat.com> writes:
> On Mon, Sep 13, 2021 at 02:39:29PM +0200, Markus Armbruster wrote:
>> Replace simple union __org.qemu_x-Union1 flat union
>
> missing 'with'
Will fix.
>> __org.qemu_x-Union2, except drop it from __org.qemu_x-command, because
>> there it's only used to pull it into QMP. Now drop the unused simple
>> union.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> tests/unit/test-qmp-cmds.c | 16 +++++-----------
>> tests/qapi-schema/qapi-schema-test.json | 6 ++----
>> tests/qapi-schema/qapi-schema-test.out | 14 +++-----------
>> 3 files changed, 10 insertions(+), 26 deletions(-)
>>
>
> Looks a bit odd to leave things with Union2 but not Union1; up to you
> if it is worth a further cleanup to rename what remains to get rid of
> the odd gap. I don't think it's a show-stopper for your series to
> keep the naming as-is, though.
Can do.
Another cleanup I forgot: rename tests/qapi-schema/flat-union* after
PATCH 21.
> Reviewed-by: Eric Blake <eblake@redhat.com>
Thanks!
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH 20/22] tests/qapi-schema: Purge simple unions from tests
2021-09-13 12:39 [PATCH 00/22] qapi: Remove simple unions from the schema language Markus Armbruster
` (18 preceding siblings ...)
2021-09-13 12:39 ` [PATCH 19/22] tests/qapi-schema: Drop simple union __org.qemu_x-Union1 Markus Armbruster
@ 2021-09-13 12:39 ` Markus Armbruster
2021-09-13 15:37 ` Eric Blake
2021-09-13 12:39 ` [PATCH 21/22] qapi: Drop simple unions Markus Armbruster
` (2 subsequent siblings)
22 siblings, 1 reply; 54+ messages in thread
From: Markus Armbruster @ 2021-09-13 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, eblake, marcandre.lureau
Drop tests that are specifically about simple unions:
* SugaredUnion in doc-good: flat unions are covered by @Object.
* union-branch-case and union-clash-branches: branch naming for flat
unions is enforced for the tag enum instead, which is covered by
enum-member-case and enum-clash-member.
* union-empty: empty flat unions are covered by flat-union-empty.
Rewrite the remainder to use flat unions: args-union, bad-base,
flat-union-base-union, union-branch-invalid-dict, union-unknown.
Except drop union-optional-branch. because converting this one is not
worth the trouble; we don't explicitly check names beginning with '*'
in other places, either.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/qapi-schema/args-union.err | 2 +-
tests/qapi-schema/args-union.json | 8 ++++++-
tests/qapi-schema/bad-base.err | 2 +-
tests/qapi-schema/bad-base.json | 8 ++++++-
tests/qapi-schema/doc-good.json | 9 --------
tests/qapi-schema/doc-good.out | 22 -------------------
tests/qapi-schema/doc-good.txt | 20 -----------------
tests/qapi-schema/flat-union-base-union.err | 2 +-
tests/qapi-schema/flat-union-base-union.json | 3 +++
tests/qapi-schema/meson.build | 4 ----
tests/qapi-schema/union-branch-case.err | 2 --
tests/qapi-schema/union-branch-case.json | 2 --
tests/qapi-schema/union-branch-case.out | 0
.../qapi-schema/union-branch-invalid-dict.err | 2 +-
.../union-branch-invalid-dict.json | 4 ++++
tests/qapi-schema/union-clash-branches.err | 2 --
tests/qapi-schema/union-clash-branches.json | 7 ------
tests/qapi-schema/union-clash-branches.out | 0
tests/qapi-schema/union-empty.err | 2 --
tests/qapi-schema/union-empty.json | 2 --
tests/qapi-schema/union-empty.out | 0
tests/qapi-schema/union-optional-branch.err | 2 --
tests/qapi-schema/union-optional-branch.json | 2 --
tests/qapi-schema/union-optional-branch.out | 0
tests/qapi-schema/union-unknown.err | 2 +-
tests/qapi-schema/union-unknown.json | 5 ++++-
26 files changed, 30 insertions(+), 84 deletions(-)
delete mode 100644 tests/qapi-schema/union-branch-case.err
delete mode 100644 tests/qapi-schema/union-branch-case.json
delete mode 100644 tests/qapi-schema/union-branch-case.out
delete mode 100644 tests/qapi-schema/union-clash-branches.err
delete mode 100644 tests/qapi-schema/union-clash-branches.json
delete mode 100644 tests/qapi-schema/union-clash-branches.out
delete mode 100644 tests/qapi-schema/union-empty.err
delete mode 100644 tests/qapi-schema/union-empty.json
delete mode 100644 tests/qapi-schema/union-empty.out
delete mode 100644 tests/qapi-schema/union-optional-branch.err
delete mode 100644 tests/qapi-schema/union-optional-branch.json
delete mode 100644 tests/qapi-schema/union-optional-branch.out
diff --git a/tests/qapi-schema/args-union.err b/tests/qapi-schema/args-union.err
index 4bf4955027..4b80a99f74 100644
--- a/tests/qapi-schema/args-union.err
+++ b/tests/qapi-schema/args-union.err
@@ -1,2 +1,2 @@
args-union.json: In command 'oops':
-args-union.json:3: command's 'data' can take union type 'Uni' only with 'boxed': true
+args-union.json:9: command's 'data' can take union type 'Uni' only with 'boxed': true
diff --git a/tests/qapi-schema/args-union.json b/tests/qapi-schema/args-union.json
index 2fcaeaae16..aabb159063 100644
--- a/tests/qapi-schema/args-union.json
+++ b/tests/qapi-schema/args-union.json
@@ -1,3 +1,9 @@
# use of union arguments requires 'boxed':true
-{ 'union': 'Uni', 'data': { 'case1': 'int', 'case2': 'str' } }
+{ 'enum': 'Enum', 'data': [ 'case1', 'case2' ] }
+{ 'struct': 'Case1', 'data': { 'data': 'int' } }
+{ 'struct': 'Case2', 'data': { 'data': 'str' } }
+{ 'union': 'Uni',
+ 'base': { 'type': 'Enum' },
+ 'discriminator': 'type',
+ 'data': { 'case1': 'Case1', 'case2': 'Case2' } }
{ 'command': 'oops', 'data': 'Uni' }
diff --git a/tests/qapi-schema/bad-base.err b/tests/qapi-schema/bad-base.err
index 61a1efc2c0..1fad63e392 100644
--- a/tests/qapi-schema/bad-base.err
+++ b/tests/qapi-schema/bad-base.err
@@ -1,2 +1,2 @@
bad-base.json: In struct 'MyType':
-bad-base.json:3: 'base' requires a struct type, union type 'Union' isn't
+bad-base.json:9: 'base' requires a struct type, union type 'Union' isn't
diff --git a/tests/qapi-schema/bad-base.json b/tests/qapi-schema/bad-base.json
index a634331cdd..8c773ff544 100644
--- a/tests/qapi-schema/bad-base.json
+++ b/tests/qapi-schema/bad-base.json
@@ -1,3 +1,9 @@
# we reject a base that is not a struct
-{ 'union': 'Union', 'data': { 'a': 'int', 'b': 'str' } }
+{ 'enum': 'Enum', 'data': [ 'a', 'b' ] }
+{ 'struct': 'Int', 'data': { 'data': 'int' } }
+{ 'struct': 'Str', 'data': { 'data': 'str' } }
+{ 'union': 'Union',
+ 'base': { 'type': 'Enum' },
+ 'discriminator': 'type',
+ 'data': { 'a': 'Int', 'b': 'Str' } }
{ 'struct': 'MyType', 'base': 'Union', 'data': { 'c': 'int' } }
diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
index cbf5c56c4b..a20acffd8b 100644
--- a/tests/qapi-schema/doc-good.json
+++ b/tests/qapi-schema/doc-good.json
@@ -107,15 +107,6 @@
'two': { 'type': 'Variant2',
'if': { 'any': ['IFONE', 'IFTWO'] } } } }
-##
-# @SugaredUnion:
-# Features:
-# @union-feat2: a feature
-##
-{ 'union': 'SugaredUnion',
- 'features': [ 'union-feat2' ],
- 'data': { 'one': 'Variant1', 'two': { 'type': 'Variant2', 'if': 'IFTWO' } } }
-
##
# @Alternate:
# @i: an integer
diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index 478fe6f82e..5a324e2627 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -32,21 +32,6 @@ object Object
case two: Variant2
if {'any': ['IFONE', 'IFTWO']}
feature union-feat1
-object q_obj_Variant1-wrapper
- member data: Variant1 optional=False
-object q_obj_Variant2-wrapper
- member data: Variant2 optional=False
-enum SugaredUnionKind
- member one
- member two
- if IFTWO
-object SugaredUnion
- member type: SugaredUnionKind optional=False
- tag type
- case one: q_obj_Variant1-wrapper
- case two: q_obj_Variant2-wrapper
- if IFTWO
- feature union-feat2
alternate Alternate
tag type
case i: int
@@ -149,13 +134,6 @@ doc symbol=Object
feature=union-feat1
a feature
-doc symbol=SugaredUnion
- body=
-
- arg=type
-
- feature=union-feat2
-a feature
doc symbol=Alternate
body=
diff --git a/tests/qapi-schema/doc-good.txt b/tests/qapi-schema/doc-good.txt
index 0c59d75964..701402ee5e 100644
--- a/tests/qapi-schema/doc-good.txt
+++ b/tests/qapi-schema/doc-good.txt
@@ -130,26 +130,6 @@ Features
a feature
-"SugaredUnion" (Object)
------------------------
-
-
-Members
-~~~~~~~
-
-"type"
- One of "one", "two"
-
-"data": "Variant1" when "type" is ""one""
-"data": "Variant2" when "type" is ""two"" (**If: **"IFTWO")
-
-Features
-~~~~~~~~
-
-"union-feat2"
- a feature
-
-
"Alternate" (Alternate)
-----------------------
diff --git a/tests/qapi-schema/flat-union-base-union.err b/tests/qapi-schema/flat-union-base-union.err
index 3b0087220e..3563e8777e 100644
--- a/tests/qapi-schema/flat-union-base-union.err
+++ b/tests/qapi-schema/flat-union-base-union.err
@@ -1,2 +1,2 @@
flat-union-base-union.json: In union 'TestUnion':
-flat-union-base-union.json:14: 'base' requires a struct type, union type 'UnionBase' isn't
+flat-union-base-union.json:17: 'base' requires a struct type, union type 'UnionBase' isn't
diff --git a/tests/qapi-schema/flat-union-base-union.json b/tests/qapi-schema/flat-union-base-union.json
index 98b4eba181..82d4c96e57 100644
--- a/tests/qapi-schema/flat-union-base-union.json
+++ b/tests/qapi-schema/flat-union-base-union.json
@@ -8,7 +8,10 @@
'data': { 'string': 'str' } }
{ 'struct': 'TestTypeB',
'data': { 'integer': 'int' } }
+{ 'enum': 'Enum', 'data': [ 'kind1', 'kind2' ] }
{ 'union': 'UnionBase',
+ 'base': { 'type': 'Enum' },
+ 'discriminator': 'type',
'data': { 'kind1': 'TestTypeA',
'kind2': 'TestTypeB' } }
{ 'union': 'TestUnion',
diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
index 0798e94042..85d3de1481 100644
--- a/tests/qapi-schema/meson.build
+++ b/tests/qapi-schema/meson.build
@@ -192,14 +192,10 @@ schemas = [
'unclosed-string.json',
'union-base-empty.json',
'union-base-no-discriminator.json',
- 'union-branch-case.json',
'union-branch-if-invalid.json',
'union-branch-invalid-dict.json',
- 'union-clash-branches.json',
- 'union-empty.json',
'union-invalid-base.json',
'union-invalid-data.json',
- 'union-optional-branch.json',
'union-unknown.json',
'unknown-escape.json',
'unknown-expr-key.json',
diff --git a/tests/qapi-schema/union-branch-case.err b/tests/qapi-schema/union-branch-case.err
deleted file mode 100644
index d2d5cb8993..0000000000
--- a/tests/qapi-schema/union-branch-case.err
+++ /dev/null
@@ -1,2 +0,0 @@
-union-branch-case.json: In union 'Uni':
-union-branch-case.json:2: name of 'data' member 'Branch' must not use uppercase or '_'
diff --git a/tests/qapi-schema/union-branch-case.json b/tests/qapi-schema/union-branch-case.json
deleted file mode 100644
index b7894b75d6..0000000000
--- a/tests/qapi-schema/union-branch-case.json
+++ /dev/null
@@ -1,2 +0,0 @@
-# Branch names should be 'lower-case'
-{ 'union': 'Uni', 'data': { 'Branch': 'int' } }
diff --git a/tests/qapi-schema/union-branch-case.out b/tests/qapi-schema/union-branch-case.out
deleted file mode 100644
index e69de29bb2..0000000000
diff --git a/tests/qapi-schema/union-branch-invalid-dict.err b/tests/qapi-schema/union-branch-invalid-dict.err
index 8137c5a767..001cdec069 100644
--- a/tests/qapi-schema/union-branch-invalid-dict.err
+++ b/tests/qapi-schema/union-branch-invalid-dict.err
@@ -1,2 +1,2 @@
union-branch-invalid-dict.json: In union 'UnionInvalidBranch':
-union-branch-invalid-dict.json:2: 'data' member 'integer' misses key 'type'
+union-branch-invalid-dict.json:4: 'data' member 'integer' misses key 'type'
diff --git a/tests/qapi-schema/union-branch-invalid-dict.json b/tests/qapi-schema/union-branch-invalid-dict.json
index 9778598dbd..c7c81c0e00 100644
--- a/tests/qapi-schema/union-branch-invalid-dict.json
+++ b/tests/qapi-schema/union-branch-invalid-dict.json
@@ -1,4 +1,8 @@
# Long form of member must have a value member 'type'
+{ 'enum': 'TestEnum',
+ 'data': [ 'integer', 's8' ] }
{ 'union': 'UnionInvalidBranch',
+ 'base': { 'type': 'TestEnum' },
+ 'discriminator': 'type',
'data': { 'integer': { 'if': 'foo'},
's8': 'int8' } }
diff --git a/tests/qapi-schema/union-clash-branches.err b/tests/qapi-schema/union-clash-branches.err
deleted file mode 100644
index ef53645728..0000000000
--- a/tests/qapi-schema/union-clash-branches.err
+++ /dev/null
@@ -1,2 +0,0 @@
-union-clash-branches.json: In union 'TestUnion':
-union-clash-branches.json:6: name of 'data' member 'a_b' must not use uppercase or '_'
diff --git a/tests/qapi-schema/union-clash-branches.json b/tests/qapi-schema/union-clash-branches.json
deleted file mode 100644
index 7bdda0b0da..0000000000
--- a/tests/qapi-schema/union-clash-branches.json
+++ /dev/null
@@ -1,7 +0,0 @@
-# Union branch name collision
-# Naming rules make collision impossible (even with the pragma). If
-# that wasn't the case, then we'd get collisions in generated C: two
-# union members a_b, and two enum members TEST_UNION_A_B.
-{ 'pragma': { 'member-name-exceptions': [ 'TestUnion' ] } }
-{ 'union': 'TestUnion',
- 'data': { 'a-b': 'int', 'a_b': 'str' } }
diff --git a/tests/qapi-schema/union-clash-branches.out b/tests/qapi-schema/union-clash-branches.out
deleted file mode 100644
index e69de29bb2..0000000000
diff --git a/tests/qapi-schema/union-empty.err b/tests/qapi-schema/union-empty.err
deleted file mode 100644
index 59788c94ce..0000000000
--- a/tests/qapi-schema/union-empty.err
+++ /dev/null
@@ -1,2 +0,0 @@
-union-empty.json: In union 'Union':
-union-empty.json:2: union has no branches
diff --git a/tests/qapi-schema/union-empty.json b/tests/qapi-schema/union-empty.json
deleted file mode 100644
index df3e5e639a..0000000000
--- a/tests/qapi-schema/union-empty.json
+++ /dev/null
@@ -1,2 +0,0 @@
-# simple unions cannot be empty
-{ 'union': 'Union', 'data': { } }
diff --git a/tests/qapi-schema/union-empty.out b/tests/qapi-schema/union-empty.out
deleted file mode 100644
index e69de29bb2..0000000000
diff --git a/tests/qapi-schema/union-optional-branch.err b/tests/qapi-schema/union-optional-branch.err
deleted file mode 100644
index b33f111de4..0000000000
--- a/tests/qapi-schema/union-optional-branch.err
+++ /dev/null
@@ -1,2 +0,0 @@
-union-optional-branch.json: In union 'Union':
-union-optional-branch.json:2: 'data' member '*a' has an invalid name
diff --git a/tests/qapi-schema/union-optional-branch.json b/tests/qapi-schema/union-optional-branch.json
deleted file mode 100644
index 591615fc68..0000000000
--- a/tests/qapi-schema/union-optional-branch.json
+++ /dev/null
@@ -1,2 +0,0 @@
-# union branches cannot be optional
-{ 'union': 'Union', 'data': { '*a': 'int', 'b': 'str' } }
diff --git a/tests/qapi-schema/union-optional-branch.out b/tests/qapi-schema/union-optional-branch.out
deleted file mode 100644
index e69de29bb2..0000000000
diff --git a/tests/qapi-schema/union-unknown.err b/tests/qapi-schema/union-unknown.err
index 7aba9f94da..dad79beae0 100644
--- a/tests/qapi-schema/union-unknown.err
+++ b/tests/qapi-schema/union-unknown.err
@@ -1,2 +1,2 @@
union-unknown.json: In union 'Union':
-union-unknown.json:2: union uses unknown type 'MissingType'
+union-unknown.json:3: branch 'unknown' uses unknown type 'MissingType'
diff --git a/tests/qapi-schema/union-unknown.json b/tests/qapi-schema/union-unknown.json
index 64d3666176..4736f1ab08 100644
--- a/tests/qapi-schema/union-unknown.json
+++ b/tests/qapi-schema/union-unknown.json
@@ -1,3 +1,6 @@
# we reject a union with unknown type in branch
+{ 'enum': 'Enum', 'data': [ 'unknown' ] }
{ 'union': 'Union',
- 'data': { 'unknown': ['MissingType'] } }
+ 'base': { 'type': 'Enum' },
+ 'discriminator': 'type',
+ 'data': { 'unknown': 'MissingType' } }
--
2.31.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH 20/22] tests/qapi-schema: Purge simple unions from tests
2021-09-13 12:39 ` [PATCH 20/22] tests/qapi-schema: Purge simple unions from tests Markus Armbruster
@ 2021-09-13 15:37 ` Eric Blake
0 siblings, 0 replies; 54+ messages in thread
From: Eric Blake @ 2021-09-13 15:37 UTC (permalink / raw)
To: Markus Armbruster; +Cc: marcandre.lureau, jsnow, qemu-devel
On Mon, Sep 13, 2021 at 02:39:30PM +0200, Markus Armbruster wrote:
> Drop tests that are specifically about simple unions:
>
> * SugaredUnion in doc-good: flat unions are covered by @Object.
>
> * union-branch-case and union-clash-branches: branch naming for flat
> unions is enforced for the tag enum instead, which is covered by
> enum-member-case and enum-clash-member.
>
> * union-empty: empty flat unions are covered by flat-union-empty.
>
> Rewrite the remainder to use flat unions: args-union, bad-base,
> flat-union-base-union, union-branch-invalid-dict, union-unknown.
>
> Except drop union-optional-branch. because converting this one is not
> worth the trouble; we don't explicitly check names beginning with '*'
> in other places, either.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH 21/22] qapi: Drop simple unions
2021-09-13 12:39 [PATCH 00/22] qapi: Remove simple unions from the schema language Markus Armbruster
` (19 preceding siblings ...)
2021-09-13 12:39 ` [PATCH 20/22] tests/qapi-schema: Purge simple unions from tests Markus Armbruster
@ 2021-09-13 12:39 ` Markus Armbruster
2021-09-13 18:38 ` Eric Blake
2021-09-13 12:39 ` [PATCH 22/22] test-clone-visitor: Correct an accidental rename Markus Armbruster
2021-09-13 12:49 ` [PATCH 00/22] qapi: Remove simple unions from the schema language Markus Armbruster
22 siblings, 1 reply; 54+ messages in thread
From: Markus Armbruster @ 2021-09-13 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, eblake, marcandre.lureau
Simple unions predate flat unions. Having both complicates the QAPI
schema language and the QAPI generator. We haven't been using simple
unions in new code for a long time, because they are less flexible and
somewhat awkward on the wire.
The previous commits eliminated simple union from the tree. Now drop
them from the QAPI schema language entirely, and update mentions of
"flat union" to just "union".
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
docs/devel/qapi-code-gen.rst | 125 ++++--------------
scripts/qapi/expr.py | 21 +--
scripts/qapi/schema.py | 101 +++-----------
.../qapi-schema/flat-union-array-branch.json | 2 +-
tests/qapi-schema/flat-union-empty.json | 2 +-
tests/qapi-schema/flat-union-int-branch.json | 2 +-
tests/qapi-schema/flat-union-no-base.err | 2 +-
tests/qapi-schema/flat-union-no-base.json | 2 +-
tests/qapi-schema/qapi-schema-test.json | 2 +-
tests/qapi-schema/reserved-member-u.json | 2 +-
tests/qapi-schema/union-base-empty.json | 2 +-
.../union-base-no-discriminator.err | 2 +-
.../union-base-no-discriminator.json | 2 +-
13 files changed, 62 insertions(+), 205 deletions(-)
diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
index b154eae82e..b2569de486 100644
--- a/docs/devel/qapi-code-gen.rst
+++ b/docs/devel/qapi-code-gen.rst
@@ -319,13 +319,9 @@ Union types
Syntax::
UNION = { 'union': STRING,
- 'data': BRANCHES,
- '*if': COND,
- '*features': FEATURES }
- | { 'union': STRING,
- 'data': BRANCHES,
'base': ( MEMBERS | STRING ),
'discriminator': STRING,
+ 'data': BRANCHES,
'*if': COND,
'*features': FEATURES }
BRANCHES = { BRANCH, ... }
@@ -334,63 +330,30 @@ Syntax::
Member 'union' names the union type.
-There are two flavors of union types: simple (no discriminator or
-base), and flat (both discriminator and base).
-
-Each BRANCH of the 'data' object defines a branch of the union. A
-union must have at least one branch.
-
-The BRANCH's STRING name is the branch name.
-
-The BRANCH's value defines the branch's properties, in particular its
-type. The form TYPE-REF_ is shorthand for :code:`{ 'type': TYPE-REF }`.
-
-A simple union type defines a mapping from automatic discriminator
-values to data types like in this example::
-
- { 'struct': 'BlockdevOptionsFile', 'data': { 'filename': 'str' } }
- { 'struct': 'BlockdevOptionsQcow2',
- 'data': { 'backing': 'str', '*lazy-refcounts': 'bool' } }
-
- { 'union': 'BlockdevOptionsSimple',
- 'data': { 'file': 'BlockdevOptionsFile',
- 'qcow2': 'BlockdevOptionsQcow2' } }
-
-In the Client JSON Protocol, a simple union is represented by an
-object that contains the 'type' member as a discriminator, and a
-'data' member that is of the specified data type corresponding to the
-discriminator value, as in these examples::
-
- { "type": "file", "data": { "filename": "/some/place/my-image" } }
- { "type": "qcow2", "data": { "backing": "/some/place/my-image",
- "lazy-refcounts": true } }
-
-The generated C code uses a struct containing a union. Additionally,
-an implicit C enum 'NameKind' is created, corresponding to the union
-'Name', for accessing the various branches of the union. The value
-for each branch can be of any type.
-
-Flat unions permit arbitrary common members that occur in all variants
-of the union, not just a discriminator. Their discriminators need not
-be named 'type'. They also avoid nesting on the wire.
-
The 'base' member defines the common members. If it is a MEMBERS_
object, it defines common members just like a struct type's 'data'
member defines struct type members. If it is a STRING, it names a
struct type whose members are the common members.
-All flat union branches must be `Struct types`_.
+Member 'discriminator' must name a non-optional enum-typed member of
+the base struct. That member's value selects a branch by its name.
+If no such branch exists, an empty branch is assumed.
-In the Client JSON Protocol, a flat union is represented by an object
-with the common members (from the base type) and the selected branch's
-members. The two sets of member names must be disjoint. Member
-'discriminator' must name a non-optional enum-typed member of the base
-struct.
+Each BRANCH of the 'data' object defines a branch of the union. A
+union must have at least one branch.
-The following example enhances the above simple union example by
-adding an optional common member 'read-only', renaming the
-discriminator to something more applicable than the simple union's
-default of 'type', and reducing the number of ``{}`` required on the wire::
+The BRANCH's STRING name is the branch name. It must be a value of
+the discriminator enum type.
+
+The BRANCH's value defines the branch's properties, in particular its
+type. The type must a struct type. The form TYPE-REF_ is shorthand
+for :code:`{ 'type': TYPE-REF }`.
+
+In the Client JSON Protocol, a union is represented by an object with
+the common members (from the base type) and the selected branch's
+members. The two sets of member names must be disjoint.
+
+Example::
{ 'enum': 'BlockdevDriver', 'data': [ 'file', 'qcow2' ] }
{ 'union': 'BlockdevOptions',
@@ -406,30 +369,11 @@ Resulting in these JSON objects::
{ "driver": "qcow2", "read-only": false,
"backing": "/some/place/my-image", "lazy-refcounts": true }
-Notice that in a flat union, the discriminator name is controlled by
-the user, but because it must map to a base member with enum type, the
-code generator ensures that branches match the existing values of the
-enum. The order of branches need not match the order of the enum
-values. The branches need not cover all possible enum values.
-Omitted enum values are still valid branches that add no additional
-members to the data type. In the resulting generated C data types, a
-flat union is represented as a struct with the base members in QAPI
-schema order, and then a union of structures for each branch of the
-struct.
-
-A simple union can always be re-written as a flat union where the base
-class has a single member named 'type', and where each branch of the
-union has a struct with a single member named 'data'. That is, ::
-
- { 'union': 'Simple', 'data': { 'one': 'str', 'two': 'int' } }
-
-is identical on the wire to::
-
- { 'enum': 'Enum', 'data': ['one', 'two'] }
- { 'struct': 'Branch1', 'data': { 'data': 'str' } }
- { 'struct': 'Branch2', 'data': { 'data': 'int' } }
- { 'union': 'Flat', 'base': { 'type': 'Enum' }, 'discriminator': 'type',
- 'data': { 'one': 'Branch1', 'two': 'Branch2' } }
+The order of branches need not match the order of the enum values.
+The branches need not cover all possible enum values. In the
+resulting generated C data types, a union is represented as a struct
+with the base members in QAPI schema order, and then a union of
+structures for each branch of the struct.
The optional 'if' member specifies a conditional. See `Configuring
the schema`_ below for more on this.
@@ -1246,7 +1190,7 @@ that provides the variant members for this type tag value). The
"variants" array is in no particular order, and is not guaranteed to
list cases in the same order as the corresponding "tag" enum type.
-Example: the SchemaInfo for flat union BlockdevOptions from section
+Example: the SchemaInfo for union BlockdevOptions from section
`Union types`_ ::
{ "name": "BlockdevOptions", "meta-type": "object",
@@ -1261,27 +1205,6 @@ Example: the SchemaInfo for flat union BlockdevOptions from section
Note that base types are "flattened": its members are included in the
"members" array.
-A simple union implicitly defines an enumeration type for its implicit
-discriminator (called "type" on the wire, see section `Union types`_).
-
-A simple union implicitly defines an object type for each of its
-variants.
-
-Example: the SchemaInfo for simple union BlockdevOptionsSimple from section
-`Union types`_ ::
-
- { "name": "BlockdevOptionsSimple", "meta-type": "object",
- "members": [
- { "name": "type", "type": "BlockdevOptionsSimpleKind" } ],
- "tag": "type",
- "variants": [
- { "case": "file", "type": "q_obj-BlockdevOptionsFile-wrapper" },
- { "case": "qcow2", "type": "q_obj-BlockdevOptionsQcow2-wrapper" } ] }
-
- Enumeration type "BlockdevOptionsSimpleKind" and the object types
- "q_obj-BlockdevOptionsFile-wrapper", "q_obj-BlockdevOptionsQcow2-wrapper"
- are implicitly defined.
-
The SchemaInfo for an alternate type has meta-type "alternate", and
variant member "members". "members" is a JSON array. Each element is
a JSON object with member "type", which names a type. Values of the
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 91959ee79a..819ea6ad97 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -513,27 +513,18 @@ def check_union(expr: _JSONObject, info: QAPISourceInfo) -> None:
:return: None, ``expr`` is normalized in-place as needed.
"""
name = cast(str, expr['union']) # Checked in check_exprs
- base = expr.get('base')
- discriminator = expr.get('discriminator')
+ base = expr['base']
+ discriminator = expr['discriminator']
members = expr['data']
- if discriminator is None: # simple union
- if base is not None:
- raise QAPISemError(info, "'base' requires 'discriminator'")
- else: # flat union
- check_type(base, info, "'base'", allow_dict=name)
- if not base:
- raise QAPISemError(info, "'discriminator' requires 'base'")
- check_name_is_str(discriminator, info, "'discriminator'")
+ check_type(base, info, "'base'", allow_dict=name)
+ check_name_is_str(discriminator, info, "'discriminator'")
if not isinstance(members, dict):
raise QAPISemError(info, "'data' must be an object")
for (key, value) in members.items():
source = "'data' member '%s'" % key
- if discriminator is None:
- check_name_lower(key, info, source)
- # else: name is in discriminator enum, which gets checked
check_keys(value, info, source, ['type'], ['if'])
check_if(value, info, source)
check_type(value['type'], info, source, allow_array=not base)
@@ -664,8 +655,8 @@ def check_exprs(exprs: List[_JSONObject]) -> List[_JSONObject]:
check_enum(expr, info)
elif meta == 'union':
check_keys(expr, info, meta,
- ['union', 'data'],
- ['base', 'discriminator', 'if', 'features'])
+ ['union', 'base', 'discriminator', 'data'],
+ ['if', 'features'])
normalize_members(expr.get('base'))
normalize_members(expr['data'])
check_union(expr, info)
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 3d72c7dfc9..004d7095ff 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -321,8 +321,8 @@ def connect_doc(self, doc=None):
m.connect_doc(doc)
def is_implicit(self):
- # See QAPISchema._make_implicit_enum_type() and ._def_predefineds()
- return self.name.endswith('Kind') or self.name == 'QType'
+ # See QAPISchema._def_predefineds()
+ return self.name == 'QType'
def c_type(self):
return c_name(self.name)
@@ -393,8 +393,7 @@ class QAPISchemaObjectType(QAPISchemaType):
def __init__(self, name, info, doc, ifcond, features,
base, local_members, variants):
# 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
+ # union has base, variants, and no local_members
super().__init__(name, info, doc, ifcond, features)
self.meta = 'union' if variants else 'struct'
assert base is None or isinstance(base, str)
@@ -465,15 +464,6 @@ def connect_doc(self, doc=None):
for m in self.local_members:
m.connect_doc(doc)
- @property
- def ifcond(self):
- assert self._checked
- if isinstance(self._ifcond, QAPISchemaType):
- # Simple union wrapper type inherits from wrapped type;
- # see _make_implicit_object_type()
- return self._ifcond.ifcond
- return self._ifcond
-
def is_implicit(self):
# See QAPISchema._make_implicit_object_type(), as well as
# _def_predefineds()
@@ -576,10 +566,9 @@ def visit(self, visitor):
class QAPISchemaVariants:
def __init__(self, tag_name, info, tag_member, variants):
- # Flat unions pass tag_name but not tag_member.
- # Simple unions and alternates pass tag_member but not tag_name.
- # After check(), tag_member is always set, and tag_name remains
- # a reliable witness of being used by a flat union.
+ # Unions pass tag_name but not tag_member.
+ # Alternates pass tag_member but not tag_name.
+ # After check(), tag_member is always set.
assert bool(tag_member) != bool(tag_name)
assert (isinstance(tag_name, str) or
isinstance(tag_member, QAPISchemaObjectTypeMember))
@@ -595,7 +584,7 @@ def set_defined_in(self, name):
v.set_defined_in(name)
def check(self, schema, seen):
- if not self.tag_member: # flat union
+ if self._tag_name: # union
self.tag_member = seen.get(c_name(self._tag_name))
base = "'base'"
# Pointing to the base type when not implicit would be
@@ -625,11 +614,11 @@ def check(self, schema, seen):
self.info,
"discriminator member '%s' of %s must not be conditional"
% (self._tag_name, base))
- else: # simple union
+ else: # alternate
assert isinstance(self.tag_member.type, QAPISchemaEnumType)
assert not self.tag_member.optional
assert not self.tag_member.ifcond.is_present()
- if self._tag_name: # flat union
+ if self._tag_name: # union
# branches that are not explicitly covered get an empty type
cases = {v.name for v in self.variants}
for m in self.tag_member.type.members:
@@ -707,18 +696,10 @@ def describe(self, info):
assert role == 'member'
role = 'parameter'
elif defined_in.endswith('-base'):
- # Implicit type created for a flat union's dict 'base'
+ # Implicit type created for a union's dict 'base'
role = 'base ' + role
else:
- # Implicit type created for a simple union's branch
- assert defined_in.endswith('-wrapper')
- # Unreachable and not implemented
assert False
- elif defined_in.endswith('Kind'):
- # See QAPISchema._make_implicit_enum_type()
- # Implicit enum created for simple union's branches
- assert role == 'value'
- role = 'branch'
elif defined_in != info.defn_name:
return "%s '%s' of type '%s'" % (role, self.name, defined_in)
return "%s '%s'" % (role, self.name)
@@ -1004,15 +985,6 @@ def _make_enum_members(self, values, info):
QAPISchemaIfCond(v.get('if')))
for v in values]
- def _make_implicit_enum_type(self, name, info, ifcond, values):
- # See also QAPISchemaObjectTypeMember.describe()
- name = name + 'Kind' # reserved by check_defn_name_str()
- self._def_entity(QAPISchemaEnumType(
- name, info, None, ifcond, None,
- self._make_enum_members(values, info),
- None))
- return name
-
def _make_array_type(self, element_type, info):
name = element_type + 'List' # reserved by check_defn_name_str()
if not self.lookup_type(name):
@@ -1026,17 +998,9 @@ def _make_implicit_object_type(self, name, info, ifcond, role, members):
name = 'q_obj_%s-%s' % (name, role)
typ = self.lookup_entity(name, QAPISchemaObjectType)
if typ:
- # The implicit object type has multiple users. This is
- # either a duplicate definition (which will be flagged
- # later), or an implicit wrapper type used for multiple
- # simple unions. In the latter case, ifcond should be the
- # disjunction of its user's ifconds. Not implemented.
- # Instead, we always pass the wrapped type's ifcond, which
- # is trivially the same for all users. It's also
- # necessary for the wrapper to compile. But it's not
- # tight: the disjunction need not imply it. We may end up
- # compiling useless wrapper types.
- # TODO kill simple unions or implement the disjunction
+ # The implicit object type has multiple users. This can
+ # only be a duplicate definition, which will be flagged
+ # later.
pass
else:
self._def_entity(QAPISchemaObjectType(
@@ -1084,49 +1048,28 @@ def _def_struct_type(self, expr, info, doc):
def _make_variant(self, case, typ, ifcond, info):
return QAPISchemaVariant(case, info, typ, ifcond)
- def _make_simple_variant(self, case, typ, ifcond, info):
- if isinstance(typ, list):
- assert len(typ) == 1
- typ = self._make_array_type(typ[0], info)
- typ = self._make_implicit_object_type(
- typ, info, self.lookup_type(typ),
- 'wrapper', [self._make_member('data', typ, None, None, info)])
- return QAPISchemaVariant(case, info, typ, ifcond)
-
def _def_union_type(self, expr, info, doc):
name = expr['union']
+ base = expr['base']
+ tag_name = expr['discriminator']
data = expr['data']
- base = expr.get('base')
ifcond = QAPISchemaIfCond(expr.get('if'))
features = self._make_features(expr.get('features'), info)
- tag_name = expr.get('discriminator')
- tag_member = None
if isinstance(base, dict):
base = self._make_implicit_object_type(
name, info, ifcond,
'base', self._make_members(base, info))
- if tag_name:
- variants = [
- self._make_variant(key, value['type'],
- QAPISchemaIfCond(value.get('if')),
- info)
- for (key, value) in data.items()]
- members = []
- else:
- variants = [
- self._make_simple_variant(key, value['type'],
- QAPISchemaIfCond(value.get('if')),
- info)
- for (key, value) in data.items()]
- enum = [{'name': v.name, 'if': v.ifcond.ifcond} for v in variants]
- typ = self._make_implicit_enum_type(name, info, ifcond, enum)
- tag_member = QAPISchemaObjectTypeMember('type', info, typ, False)
- members = [tag_member]
+ variants = [
+ self._make_variant(key, value['type'],
+ QAPISchemaIfCond(value.get('if')),
+ info)
+ for (key, value) in data.items()]
+ members = []
self._def_entity(
QAPISchemaObjectType(name, info, doc, ifcond, features,
base, members,
QAPISchemaVariants(
- tag_name, info, tag_member, variants)))
+ tag_name, info, None, variants)))
def _def_alternate_type(self, expr, info, doc):
name = expr['alternate']
diff --git a/tests/qapi-schema/flat-union-array-branch.json b/tests/qapi-schema/flat-union-array-branch.json
index 0b98820a8f..6dda7ec379 100644
--- a/tests/qapi-schema/flat-union-array-branch.json
+++ b/tests/qapi-schema/flat-union-array-branch.json
@@ -1,4 +1,4 @@
-# we require flat union branches to be a struct
+# we require union branches to be a struct
{ 'enum': 'TestEnum',
'data': [ 'value1', 'value2' ] }
{ 'struct': 'Base',
diff --git a/tests/qapi-schema/flat-union-empty.json b/tests/qapi-schema/flat-union-empty.json
index 83e1cc7b96..584ed6098c 100644
--- a/tests/qapi-schema/flat-union-empty.json
+++ b/tests/qapi-schema/flat-union-empty.json
@@ -1,4 +1,4 @@
-# flat union discriminator cannot be empty
+# union discriminator enum cannot be empty
{ 'enum': 'Empty', 'data': [ ] }
{ 'struct': 'Base', 'data': { 'type': 'Empty' } }
{ 'union': 'Union', 'base': 'Base', 'discriminator': 'type', 'data': { } }
diff --git a/tests/qapi-schema/flat-union-int-branch.json b/tests/qapi-schema/flat-union-int-branch.json
index 9370c349e8..567043d9d2 100644
--- a/tests/qapi-schema/flat-union-int-branch.json
+++ b/tests/qapi-schema/flat-union-int-branch.json
@@ -1,4 +1,4 @@
-# we require flat union branches to be a struct
+# we require union branches to be a struct
{ 'enum': 'TestEnum',
'data': [ 'value1', 'value2' ] }
{ 'struct': 'Base',
diff --git a/tests/qapi-schema/flat-union-no-base.err b/tests/qapi-schema/flat-union-no-base.err
index 5167565b00..c60482f96b 100644
--- a/tests/qapi-schema/flat-union-no-base.err
+++ b/tests/qapi-schema/flat-union-no-base.err
@@ -1,2 +1,2 @@
flat-union-no-base.json: In union 'TestUnion':
-flat-union-no-base.json:8: 'discriminator' requires 'base'
+flat-union-no-base.json:8: union misses key 'base'
diff --git a/tests/qapi-schema/flat-union-no-base.json b/tests/qapi-schema/flat-union-no-base.json
index 327877b563..f6fe12da3b 100644
--- a/tests/qapi-schema/flat-union-no-base.json
+++ b/tests/qapi-schema/flat-union-no-base.json
@@ -1,4 +1,4 @@
-# flat unions require a base
+# unions require a base
{ 'struct': 'TestTypeA',
'data': { 'string': 'str' } }
{ 'struct': 'TestTypeB',
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 0c19d4820e..b10490ccc6 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -30,7 +30,7 @@
{ 'struct': 'Empty1', 'data': { } }
{ 'struct': 'Empty2', 'base': 'Empty1', 'data': { } }
-# Likewise for an empty flat union
+# Likewise for an empty union
{ 'union': 'Union',
'base': { 'type': 'EnumOne' }, 'discriminator': 'type',
'data': { } }
diff --git a/tests/qapi-schema/reserved-member-u.json b/tests/qapi-schema/reserved-member-u.json
index 2bfb8f59b6..d982ab5e0c 100644
--- a/tests/qapi-schema/reserved-member-u.json
+++ b/tests/qapi-schema/reserved-member-u.json
@@ -2,6 +2,6 @@
# We reject use of 'u' as a member name, to allow it for internal use in
# putting union branch members in a separate namespace from QMP members.
# This is true even for non-unions, because it is possible to convert a
-# struct to flat union while remaining backwards compatible in QMP.
+# struct to union while remaining backwards compatible in QMP.
# TODO - we could munge the member name to 'q_u' to avoid the collision
{ 'struct': 'Oops', 'data': { '*u': 'str' } }
diff --git a/tests/qapi-schema/union-base-empty.json b/tests/qapi-schema/union-base-empty.json
index d1843d33b4..6f8ef000db 100644
--- a/tests/qapi-schema/union-base-empty.json
+++ b/tests/qapi-schema/union-base-empty.json
@@ -1,4 +1,4 @@
-# Flat union with empty base and therefore without discriminator
+# Union with empty base and therefore without discriminator
{ 'struct': 'Empty', 'data': { } }
diff --git a/tests/qapi-schema/union-base-no-discriminator.err b/tests/qapi-schema/union-base-no-discriminator.err
index 9cd5d11b0b..a730b7fd3c 100644
--- a/tests/qapi-schema/union-base-no-discriminator.err
+++ b/tests/qapi-schema/union-base-no-discriminator.err
@@ -1,2 +1,2 @@
union-base-no-discriminator.json: In union 'TestUnion':
-union-base-no-discriminator.json:11: 'base' requires 'discriminator'
+union-base-no-discriminator.json:11: union misses key 'discriminator'
diff --git a/tests/qapi-schema/union-base-no-discriminator.json b/tests/qapi-schema/union-base-no-discriminator.json
index 1409cf5c9e..2e7cae9b22 100644
--- a/tests/qapi-schema/union-base-no-discriminator.json
+++ b/tests/qapi-schema/union-base-no-discriminator.json
@@ -1,4 +1,4 @@
-# we reject simple unions with a base (or flat unions without discriminator)
+# we reject unions without discriminator
{ 'struct': 'TestTypeA',
'data': { 'string': 'str' } }
--
2.31.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH 21/22] qapi: Drop simple unions
2021-09-13 12:39 ` [PATCH 21/22] qapi: Drop simple unions Markus Armbruster
@ 2021-09-13 18:38 ` Eric Blake
2021-09-14 5:57 ` Markus Armbruster
0 siblings, 1 reply; 54+ messages in thread
From: Eric Blake @ 2021-09-13 18:38 UTC (permalink / raw)
To: Markus Armbruster; +Cc: marcandre.lureau, jsnow, qemu-devel
On Mon, Sep 13, 2021 at 02:39:31PM +0200, Markus Armbruster wrote:
> Simple unions predate flat unions. Having both complicates the QAPI
> schema language and the QAPI generator. We haven't been using simple
> unions in new code for a long time, because they are less flexible and
> somewhat awkward on the wire.
>
> The previous commits eliminated simple union from the tree. Now drop
> them from the QAPI schema language entirely, and update mentions of
> "flat union" to just "union".
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> docs/devel/qapi-code-gen.rst | 125 ++++--------------
> scripts/qapi/expr.py | 21 +--
> scripts/qapi/schema.py | 101 +++-----------
> .../qapi-schema/flat-union-array-branch.json | 2 +-
> tests/qapi-schema/flat-union-empty.json | 2 +-
> tests/qapi-schema/flat-union-int-branch.json | 2 +-
> tests/qapi-schema/flat-union-no-base.err | 2 +-
> tests/qapi-schema/flat-union-no-base.json | 2 +-
> tests/qapi-schema/qapi-schema-test.json | 2 +-
> tests/qapi-schema/reserved-member-u.json | 2 +-
> tests/qapi-schema/union-base-empty.json | 2 +-
> .../union-base-no-discriminator.err | 2 +-
> .../union-base-no-discriminator.json | 2 +-
> 13 files changed, 62 insertions(+), 205 deletions(-)
Whee! What a fun ride!
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 21/22] qapi: Drop simple unions
2021-09-13 18:38 ` Eric Blake
@ 2021-09-14 5:57 ` Markus Armbruster
0 siblings, 0 replies; 54+ messages in thread
From: Markus Armbruster @ 2021-09-14 5:57 UTC (permalink / raw)
To: Eric Blake; +Cc: marcandre.lureau, jsnow, qemu-devel
Eric Blake <eblake@redhat.com> writes:
> On Mon, Sep 13, 2021 at 02:39:31PM +0200, Markus Armbruster wrote:
>> Simple unions predate flat unions. Having both complicates the QAPI
>> schema language and the QAPI generator. We haven't been using simple
>> unions in new code for a long time, because they are less flexible and
>> somewhat awkward on the wire.
>>
>> The previous commits eliminated simple union from the tree. Now drop
>> them from the QAPI schema language entirely, and update mentions of
>> "flat union" to just "union".
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> docs/devel/qapi-code-gen.rst | 125 ++++--------------
>> scripts/qapi/expr.py | 21 +--
>> scripts/qapi/schema.py | 101 +++-----------
>> .../qapi-schema/flat-union-array-branch.json | 2 +-
>> tests/qapi-schema/flat-union-empty.json | 2 +-
>> tests/qapi-schema/flat-union-int-branch.json | 2 +-
>> tests/qapi-schema/flat-union-no-base.err | 2 +-
>> tests/qapi-schema/flat-union-no-base.json | 2 +-
>> tests/qapi-schema/qapi-schema-test.json | 2 +-
>> tests/qapi-schema/reserved-member-u.json | 2 +-
>> tests/qapi-schema/union-base-empty.json | 2 +-
>> .../union-base-no-discriminator.err | 2 +-
>> .../union-base-no-discriminator.json | 2 +-
>> 13 files changed, 62 insertions(+), 205 deletions(-)
>
> Whee! What a fun ride!
3-2-1-gone! And good riddance :)
> Reviewed-by: Eric Blake <eblake@redhat.com>
Thank you for your quick review!
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH 22/22] test-clone-visitor: Correct an accidental rename
2021-09-13 12:39 [PATCH 00/22] qapi: Remove simple unions from the schema language Markus Armbruster
` (20 preceding siblings ...)
2021-09-13 12:39 ` [PATCH 21/22] qapi: Drop simple unions Markus Armbruster
@ 2021-09-13 12:39 ` Markus Armbruster
2021-09-13 18:40 ` Eric Blake
2021-09-13 12:49 ` [PATCH 00/22] qapi: Remove simple unions from the schema language Markus Armbruster
22 siblings, 1 reply; 54+ messages in thread
From: Markus Armbruster @ 2021-09-13 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, eblake, marcandre.lureau
Commit b359f4b203 "tests: Rename UserDefNativeListUnion to
UserDefListUnion" renamed test_clone_native_list() to
test_clone_list_union(). The function has nothing to do with unions.
Rename it to test_clone_list().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/unit/test-clone-visitor.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/unit/test-clone-visitor.c b/tests/unit/test-clone-visitor.c
index 4048018607..5d48e125b8 100644
--- a/tests/unit/test-clone-visitor.c
+++ b/tests/unit/test-clone-visitor.c
@@ -63,7 +63,7 @@ static void test_clone_alternate(void)
qapi_free_AltEnumBool(s_dst);
}
-static void test_clone_list_union(void)
+static void test_clone_list(void)
{
uint8List *src = NULL, *dst;
uint8List *tmp = NULL;
@@ -203,7 +203,7 @@ int main(int argc, char **argv)
g_test_add_func("/visitor/clone/struct", test_clone_struct);
g_test_add_func("/visitor/clone/alternate", test_clone_alternate);
- g_test_add_func("/visitor/clone/list_union", test_clone_list_union);
+ g_test_add_func("/visitor/clone/list", test_clone_list);
g_test_add_func("/visitor/clone/empty", test_clone_empty);
g_test_add_func("/visitor/clone/complex1", test_clone_complex1);
g_test_add_func("/visitor/clone/complex2", test_clone_complex2);
--
2.31.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH 22/22] test-clone-visitor: Correct an accidental rename
2021-09-13 12:39 ` [PATCH 22/22] test-clone-visitor: Correct an accidental rename Markus Armbruster
@ 2021-09-13 18:40 ` Eric Blake
0 siblings, 0 replies; 54+ messages in thread
From: Eric Blake @ 2021-09-13 18:40 UTC (permalink / raw)
To: Markus Armbruster; +Cc: marcandre.lureau, jsnow, qemu-devel
[IOn Mon, Sep 13, 2021 at 02:39:32PM +0200, Markus Armbruster wrote:
> Commit b359f4b203 "tests: Rename UserDefNativeListUnion to
> UserDefListUnion" renamed test_clone_native_list() to
> test_clone_list_union(). The function has nothing to do with unions.
> Rename it to test_clone_list().
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> tests/unit/test-clone-visitor.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 00/22] qapi: Remove simple unions from the schema language
2021-09-13 12:39 [PATCH 00/22] qapi: Remove simple unions from the schema language Markus Armbruster
` (21 preceding siblings ...)
2021-09-13 12:39 ` [PATCH 22/22] test-clone-visitor: Correct an accidental rename Markus Armbruster
@ 2021-09-13 12:49 ` Markus Armbruster
22 siblings, 0 replies; 54+ messages in thread
From: Markus Armbruster @ 2021-09-13 12:49 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
Eduardo Habkost, Stefan Berger, eblake, Hanna Reitz,
Gerd Hoffmann, Paolo Bonzini, marcandre.lureau, jsnow
Markus Armbruster <armbru@redhat.com> writes:
> Simple unions predate flat unions. Having both complicates the QAPI
> schema language and the QAPI generator. We haven't been using simple
> unions in new code for a long time, because they are less flexible and
> somewhat awkward on the wire.
>
> Get rid of them. We should've done this long ago.
>
> This adds some boilerplate to the schema:
>
> $ git-diff --shortstat master qapi
> 7 files changed, 461 insertions(+), 59 deletions(-)
>
> Well worth the language simplification, in my opinion:
>
> $ git-diff --stat master scripts/ docs/
> docs/devel/qapi-code-gen.rst | 137 ++++++++++---------------------------------
> scripts/qapi/common.py | 19 ++----
> scripts/qapi/expr.py | 48 +++++++--------
> scripts/qapi/schema.py | 101 +++++++------------------------
> 4 files changed, 80 insertions(+), 225 deletions(-)
>
> The complete diffstat looks even better, but is somewhat misleading,
> because it's dominated by two tests rewritten in a much more compact
> way.
>
> This series is based on my "[PULL 0/5] QAPI patches patches for
> 2021-09-13".
>
> Based-on: <20210913095038.3040776-1-armbru@redhat.com>
In master now.
^ permalink raw reply [flat|nested] 54+ messages in thread