* [PATCH v2 0/3] qapi: allow unions to contain further unions
@ 2023-02-23 13:40 Daniel P. Berrangé
2023-02-23 13:40 ` [PATCH v2 1/3] qapi: improve specificity of type/member descriptions Daniel P. Berrangé
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Daniel P. Berrangé @ 2023-02-23 13:40 UTC (permalink / raw)
To: qemu-devel
Cc: Eric Blake, Michael Roth, Markus Armbruster, Het Gala,
Daniel P. Berrangé
Currently it is not possible for a union type to contain a
further union as one (or more) of its branches. This relaxes
that restriction and adds the calls needed to validate field
name uniqueness as unions are flattened.
In v2:
* Improve specificity of type/members descriptions for
error reporting scenarios
* Make it easier to regenerate qapi test output
* Move expected "good data" into qapi-schema-test.json
* Add description to "bad data" test files
* Add unit tests to cover union-in-union serialization
/ deserialization to/from JSON
Daniel P. Berrangé (3):
qapi: improve specificity of type/member descriptions
qapi: use env var to trigger qapi test output updates
qapi: allow unions to contain further unions
scripts/qapi/schema.py | 11 ++--
tests/qapi-schema/alternate-any.err | 2 +-
.../alternate-conflict-bool-string.err | 2 +-
tests/qapi-schema/alternate-conflict-dict.err | 2 +-
.../alternate-conflict-enum-bool.err | 2 +-
.../alternate-conflict-enum-int.err | 2 +-
.../qapi-schema/alternate-conflict-lists.err | 2 +-
.../alternate-conflict-num-string.err | 2 +-
.../qapi-schema/alternate-conflict-string.err | 2 +-
tests/qapi-schema/alternate-nested.err | 2 +-
tests/qapi-schema/alternate-unknown.err | 2 +-
tests/qapi-schema/args-member-unknown.err | 2 +-
tests/qapi-schema/enum-clash-member.err | 2 +-
tests/qapi-schema/features-duplicate-name.err | 2 +-
tests/qapi-schema/meson.build | 2 +
tests/qapi-schema/qapi-schema-test.json | 32 ++++++++++
tests/qapi-schema/qapi-schema-test.out | 29 ++++++++++
tests/qapi-schema/struct-base-clash-deep.err | 2 +-
tests/qapi-schema/struct-base-clash.err | 2 +-
.../qapi-schema/struct-member-name-clash.err | 2 +-
tests/qapi-schema/test-qapi.py | 3 +-
tests/qapi-schema/union-bad-base.err | 2 +-
tests/qapi-schema/union-int-branch.err | 2 +-
.../union-invalid-union-subfield.err | 2 +
.../union-invalid-union-subfield.json | 30 ++++++++++
.../union-invalid-union-subfield.out | 0
.../union-invalid-union-subtype.err | 2 +
.../union-invalid-union-subtype.json | 29 ++++++++++
.../union-invalid-union-subtype.out | 0
tests/qapi-schema/union-unknown.err | 2 +-
tests/unit/test-qobject-input-visitor.c | 47 +++++++++++++++
tests/unit/test-qobject-output-visitor.c | 58 +++++++++++++++++++
32 files changed, 257 insertions(+), 26 deletions(-)
create mode 100644 tests/qapi-schema/union-invalid-union-subfield.err
create mode 100644 tests/qapi-schema/union-invalid-union-subfield.json
create mode 100644 tests/qapi-schema/union-invalid-union-subfield.out
create mode 100644 tests/qapi-schema/union-invalid-union-subtype.err
create mode 100644 tests/qapi-schema/union-invalid-union-subtype.json
create mode 100644 tests/qapi-schema/union-invalid-union-subtype.out
--
2.39.2
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v2 1/3] qapi: improve specificity of type/member descriptions 2023-02-23 13:40 [PATCH v2 0/3] qapi: allow unions to contain further unions Daniel P. Berrangé @ 2023-02-23 13:40 ` Daniel P. Berrangé 2023-03-17 12:08 ` Markus Armbruster 2023-02-23 13:40 ` [PATCH v2 2/3] qapi: use env var to trigger qapi test output updates Daniel P. Berrangé ` (2 subsequent siblings) 3 siblings, 1 reply; 13+ messages in thread From: Daniel P. Berrangé @ 2023-02-23 13:40 UTC (permalink / raw) To: qemu-devel Cc: Eric Blake, Michael Roth, Markus Armbruster, Het Gala, Daniel P. Berrangé When describing member types always include the context of the containing type. Although this is often redundant, in some cases it will help to reduce ambiguity. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- scripts/qapi/schema.py | 5 ++--- tests/qapi-schema/alternate-any.err | 2 +- tests/qapi-schema/alternate-conflict-bool-string.err | 2 +- tests/qapi-schema/alternate-conflict-dict.err | 2 +- tests/qapi-schema/alternate-conflict-enum-bool.err | 2 +- tests/qapi-schema/alternate-conflict-enum-int.err | 2 +- tests/qapi-schema/alternate-conflict-lists.err | 2 +- tests/qapi-schema/alternate-conflict-num-string.err | 2 +- tests/qapi-schema/alternate-conflict-string.err | 2 +- tests/qapi-schema/alternate-nested.err | 2 +- tests/qapi-schema/alternate-unknown.err | 2 +- tests/qapi-schema/args-member-unknown.err | 2 +- tests/qapi-schema/enum-clash-member.err | 2 +- tests/qapi-schema/features-duplicate-name.err | 2 +- tests/qapi-schema/struct-base-clash-deep.err | 2 +- tests/qapi-schema/struct-base-clash.err | 2 +- tests/qapi-schema/struct-member-name-clash.err | 2 +- tests/qapi-schema/union-bad-base.err | 2 +- tests/qapi-schema/union-int-branch.err | 2 +- tests/qapi-schema/union-unknown.err | 2 +- 20 files changed, 21 insertions(+), 22 deletions(-) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index cd8661125c..6c481ab0c0 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -713,9 +713,8 @@ def describe(self, info): role = 'base ' + role else: assert False - elif defined_in != info.defn_name: - return "%s '%s' of type '%s'" % (role, self.name, defined_in) - return "%s '%s'" % (role, self.name) + + return "%s '%s' of type '%s'" % (role, self.name, defined_in) class QAPISchemaEnumMember(QAPISchemaMember): diff --git a/tests/qapi-schema/alternate-any.err b/tests/qapi-schema/alternate-any.err index baeb3f66d1..6c12358a88 100644 --- a/tests/qapi-schema/alternate-any.err +++ b/tests/qapi-schema/alternate-any.err @@ -1,2 +1,2 @@ alternate-any.json: In alternate 'Alt': -alternate-any.json:2: branch 'one' cannot use built-in type 'any' +alternate-any.json:2: branch 'one' of type 'Alt' cannot use built-in type 'any' diff --git a/tests/qapi-schema/alternate-conflict-bool-string.err b/tests/qapi-schema/alternate-conflict-bool-string.err index 59ff5efa87..d7fd625632 100644 --- a/tests/qapi-schema/alternate-conflict-bool-string.err +++ b/tests/qapi-schema/alternate-conflict-bool-string.err @@ -1,2 +1,2 @@ alternate-conflict-bool-string.json: In alternate 'Alt': -alternate-conflict-bool-string.json:2: branch 'two' can't be distinguished from 'one' +alternate-conflict-bool-string.json:2: branch 'two' of type 'Alt' can't be distinguished from 'one' diff --git a/tests/qapi-schema/alternate-conflict-dict.err b/tests/qapi-schema/alternate-conflict-dict.err index d4970284ba..05174ab4f1 100644 --- a/tests/qapi-schema/alternate-conflict-dict.err +++ b/tests/qapi-schema/alternate-conflict-dict.err @@ -1,2 +1,2 @@ alternate-conflict-dict.json: In alternate 'Alt': -alternate-conflict-dict.json:6: branch 'two' can't be distinguished from 'one' +alternate-conflict-dict.json:6: branch 'two' of type 'Alt' can't be distinguished from 'one' diff --git a/tests/qapi-schema/alternate-conflict-enum-bool.err b/tests/qapi-schema/alternate-conflict-enum-bool.err index 5f35855274..02ed66b0bf 100644 --- a/tests/qapi-schema/alternate-conflict-enum-bool.err +++ b/tests/qapi-schema/alternate-conflict-enum-bool.err @@ -1,2 +1,2 @@ alternate-conflict-enum-bool.json: In alternate 'Alt': -alternate-conflict-enum-bool.json:4: branch 'two' can't be distinguished from 'one' +alternate-conflict-enum-bool.json:4: branch 'two' of type 'Alt' can't be distinguished from 'one' diff --git a/tests/qapi-schema/alternate-conflict-enum-int.err b/tests/qapi-schema/alternate-conflict-enum-int.err index 6a6d156664..1874558f49 100644 --- a/tests/qapi-schema/alternate-conflict-enum-int.err +++ b/tests/qapi-schema/alternate-conflict-enum-int.err @@ -1,2 +1,2 @@ alternate-conflict-enum-int.json: In alternate 'Alt': -alternate-conflict-enum-int.json:4: branch 'two' can't be distinguished from 'one' +alternate-conflict-enum-int.json:4: branch 'two' of type 'Alt' can't be distinguished from 'one' diff --git a/tests/qapi-schema/alternate-conflict-lists.err b/tests/qapi-schema/alternate-conflict-lists.err index f3374ec1e7..de480dc86b 100644 --- a/tests/qapi-schema/alternate-conflict-lists.err +++ b/tests/qapi-schema/alternate-conflict-lists.err @@ -1,2 +1,2 @@ alternate-conflict-lists.json: In alternate 'Alt': -alternate-conflict-lists.json:4: branch 'two' can't be distinguished from 'one' +alternate-conflict-lists.json:4: branch 'two' of type 'Alt' can't be distinguished from 'one' diff --git a/tests/qapi-schema/alternate-conflict-num-string.err b/tests/qapi-schema/alternate-conflict-num-string.err index 38c805ea1f..0d21ec590c 100644 --- a/tests/qapi-schema/alternate-conflict-num-string.err +++ b/tests/qapi-schema/alternate-conflict-num-string.err @@ -1,2 +1,2 @@ alternate-conflict-num-string.json: In alternate 'Alt': -alternate-conflict-num-string.json:2: branch 'two' can't be distinguished from 'one' +alternate-conflict-num-string.json:2: branch 'two' of type 'Alt' can't be distinguished from 'one' diff --git a/tests/qapi-schema/alternate-conflict-string.err b/tests/qapi-schema/alternate-conflict-string.err index 2fa08193db..97097cae97 100644 --- a/tests/qapi-schema/alternate-conflict-string.err +++ b/tests/qapi-schema/alternate-conflict-string.err @@ -1,2 +1,2 @@ alternate-conflict-string.json: In alternate 'Alt': -alternate-conflict-string.json:2: branch 'two' can't be distinguished from 'one' +alternate-conflict-string.json:2: branch 'two' of type 'Alt' can't be distinguished from 'one' diff --git a/tests/qapi-schema/alternate-nested.err b/tests/qapi-schema/alternate-nested.err index 3ae9cd2f11..5603aa95e3 100644 --- a/tests/qapi-schema/alternate-nested.err +++ b/tests/qapi-schema/alternate-nested.err @@ -1,2 +1,2 @@ alternate-nested.json: In alternate 'Alt2': -alternate-nested.json:4: branch 'nested' cannot use alternate type 'Alt1' +alternate-nested.json:4: branch 'nested' of type 'Alt2' cannot use alternate type 'Alt1' diff --git a/tests/qapi-schema/alternate-unknown.err b/tests/qapi-schema/alternate-unknown.err index 17fec1cd17..befd207a76 100644 --- a/tests/qapi-schema/alternate-unknown.err +++ b/tests/qapi-schema/alternate-unknown.err @@ -1,2 +1,2 @@ alternate-unknown.json: In alternate 'Alt': -alternate-unknown.json:2: branch 'unknown' uses unknown type 'MissingType' +alternate-unknown.json:2: branch 'unknown' of type 'Alt' uses unknown type 'MissingType' diff --git a/tests/qapi-schema/args-member-unknown.err b/tests/qapi-schema/args-member-unknown.err index 96b6e5d289..b24c2ae572 100644 --- a/tests/qapi-schema/args-member-unknown.err +++ b/tests/qapi-schema/args-member-unknown.err @@ -1,2 +1,2 @@ args-member-unknown.json: In command 'oops': -args-member-unknown.json:2: parameter 'member' uses unknown type 'NoSuchType' +args-member-unknown.json:2: parameter 'member' of type 'oops-arg' uses unknown type 'NoSuchType' diff --git a/tests/qapi-schema/enum-clash-member.err b/tests/qapi-schema/enum-clash-member.err index e4eb102ae2..0ede36d68e 100644 --- a/tests/qapi-schema/enum-clash-member.err +++ b/tests/qapi-schema/enum-clash-member.err @@ -1,2 +1,2 @@ enum-clash-member.json: In enum 'MyEnum': -enum-clash-member.json:3: value 'one_two' collides with value 'one-two' +enum-clash-member.json:3: value 'one_two' of type 'MyEnum' collides with value 'one-two' of type 'MyEnum' diff --git a/tests/qapi-schema/features-duplicate-name.err b/tests/qapi-schema/features-duplicate-name.err index 0adbee6b0a..ffd124f5b0 100644 --- a/tests/qapi-schema/features-duplicate-name.err +++ b/tests/qapi-schema/features-duplicate-name.err @@ -1,2 +1,2 @@ features-duplicate-name.json: In struct 'FeatureStruct0': -features-duplicate-name.json:1: feature 'foo' collides with feature 'foo' +features-duplicate-name.json:1: feature 'foo' of type 'FeatureStruct0' collides with feature 'foo' of type 'FeatureStruct0' diff --git a/tests/qapi-schema/struct-base-clash-deep.err b/tests/qapi-schema/struct-base-clash-deep.err index 79879681d9..632f78b6c0 100644 --- a/tests/qapi-schema/struct-base-clash-deep.err +++ b/tests/qapi-schema/struct-base-clash-deep.err @@ -1,2 +1,2 @@ struct-base-clash-deep.json: In struct 'Sub': -struct-base-clash-deep.json:10: member 'name' collides with member 'name' of type 'Base' +struct-base-clash-deep.json:10: member 'name' of type 'Sub' collides with member 'name' of type 'Base' diff --git a/tests/qapi-schema/struct-base-clash.err b/tests/qapi-schema/struct-base-clash.err index 46473947e6..1d54079c16 100644 --- a/tests/qapi-schema/struct-base-clash.err +++ b/tests/qapi-schema/struct-base-clash.err @@ -1,2 +1,2 @@ struct-base-clash.json: In struct 'Sub': -struct-base-clash.json:5: member 'name' collides with member 'name' of type 'Base' +struct-base-clash.json:5: member 'name' of type 'Sub' collides with member 'name' of type 'Base' diff --git a/tests/qapi-schema/struct-member-name-clash.err b/tests/qapi-schema/struct-member-name-clash.err index 7e53a605d2..ebf66cd5b8 100644 --- a/tests/qapi-schema/struct-member-name-clash.err +++ b/tests/qapi-schema/struct-member-name-clash.err @@ -1,2 +1,2 @@ struct-member-name-clash.json: In struct 'Oops': -struct-member-name-clash.json:5: member 'a_b' collides with member 'a-b' +struct-member-name-clash.json:5: member 'a_b' of type 'Oops' collides with member 'a-b' of type 'Oops' diff --git a/tests/qapi-schema/union-bad-base.err b/tests/qapi-schema/union-bad-base.err index 42b2ed1dda..9f92b35a07 100644 --- a/tests/qapi-schema/union-bad-base.err +++ b/tests/qapi-schema/union-bad-base.err @@ -1,2 +1,2 @@ union-bad-base.json: In union 'TestUnion': -union-bad-base.json:8: member 'string' of type 'TestTypeA' collides with base member 'string' +union-bad-base.json:8: member 'string' of type 'TestTypeA' collides with base member 'string' of type 'TestUnion-base' diff --git a/tests/qapi-schema/union-int-branch.err b/tests/qapi-schema/union-int-branch.err index 8fdc81edd1..302e3976e0 100644 --- a/tests/qapi-schema/union-int-branch.err +++ b/tests/qapi-schema/union-int-branch.err @@ -1,2 +1,2 @@ union-int-branch.json: In union 'TestUnion': -union-int-branch.json:8: branch 'value1' cannot use built-in type 'int' +union-int-branch.json:8: branch 'value1' of type 'TestUnion' cannot use built-in type 'int' diff --git a/tests/qapi-schema/union-unknown.err b/tests/qapi-schema/union-unknown.err index dad79beae0..e60ab9421a 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:3: branch 'unknown' uses unknown type 'MissingType' +union-unknown.json:3: branch 'unknown' of type 'Union' uses unknown type 'MissingType' -- 2.39.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/3] qapi: improve specificity of type/member descriptions 2023-02-23 13:40 ` [PATCH v2 1/3] qapi: improve specificity of type/member descriptions Daniel P. Berrangé @ 2023-03-17 12:08 ` Markus Armbruster 0 siblings, 0 replies; 13+ messages in thread From: Markus Armbruster @ 2023-03-17 12:08 UTC (permalink / raw) To: Daniel P. Berrangé; +Cc: qemu-devel, Eric Blake, Michael Roth, Het Gala Daniel P. Berrangé <berrange@redhat.com> writes: > When describing member types always include the context of the > containing type. Although this is often redundant, in some cases > it will help to reduce ambiguity. Unfortunately, it can also be confusing, as we shall see below. > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > scripts/qapi/schema.py | 5 ++--- > tests/qapi-schema/alternate-any.err | 2 +- > tests/qapi-schema/alternate-conflict-bool-string.err | 2 +- > tests/qapi-schema/alternate-conflict-dict.err | 2 +- > tests/qapi-schema/alternate-conflict-enum-bool.err | 2 +- > tests/qapi-schema/alternate-conflict-enum-int.err | 2 +- > tests/qapi-schema/alternate-conflict-lists.err | 2 +- > tests/qapi-schema/alternate-conflict-num-string.err | 2 +- > tests/qapi-schema/alternate-conflict-string.err | 2 +- > tests/qapi-schema/alternate-nested.err | 2 +- > tests/qapi-schema/alternate-unknown.err | 2 +- > tests/qapi-schema/args-member-unknown.err | 2 +- > tests/qapi-schema/enum-clash-member.err | 2 +- > tests/qapi-schema/features-duplicate-name.err | 2 +- > tests/qapi-schema/struct-base-clash-deep.err | 2 +- > tests/qapi-schema/struct-base-clash.err | 2 +- > tests/qapi-schema/struct-member-name-clash.err | 2 +- > tests/qapi-schema/union-bad-base.err | 2 +- > tests/qapi-schema/union-int-branch.err | 2 +- > tests/qapi-schema/union-unknown.err | 2 +- > 20 files changed, 21 insertions(+), 22 deletions(-) > > diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py > index cd8661125c..6c481ab0c0 100644 > --- a/scripts/qapi/schema.py > +++ b/scripts/qapi/schema.py > @@ -713,9 +713,8 @@ def describe(self, info): > role = 'base ' + role > else: > assert False > - elif defined_in != info.defn_name: > - return "%s '%s' of type '%s'" % (role, self.name, defined_in) > - return "%s '%s'" % (role, self.name) > + > + return "%s '%s' of type '%s'" % (role, self.name, defined_in) > > > class QAPISchemaEnumMember(QAPISchemaMember): > diff --git a/tests/qapi-schema/alternate-any.err b/tests/qapi-schema/alternate-any.err > index baeb3f66d1..6c12358a88 100644 > --- a/tests/qapi-schema/alternate-any.err > +++ b/tests/qapi-schema/alternate-any.err > @@ -1,2 +1,2 @@ > alternate-any.json: In alternate 'Alt': > -alternate-any.json:2: branch 'one' cannot use built-in type 'any' > +alternate-any.json:2: branch 'one' of type 'Alt' cannot use built-in type 'any' Redundant with the "In alternate" context line. I don't like it, but if that was all I don't like, I'd be willing to accept it to get PATCH 3 over the hump. > diff --git a/tests/qapi-schema/alternate-conflict-bool-string.err b/tests/qapi-schema/alternate-conflict-bool-string.err > index 59ff5efa87..d7fd625632 100644 > --- a/tests/qapi-schema/alternate-conflict-bool-string.err > +++ b/tests/qapi-schema/alternate-conflict-bool-string.err > @@ -1,2 +1,2 @@ > alternate-conflict-bool-string.json: In alternate 'Alt': > -alternate-conflict-bool-string.json:2: branch 'two' can't be distinguished from 'one' > +alternate-conflict-bool-string.json:2: branch 'two' of type 'Alt' can't be distinguished from 'one' > diff --git a/tests/qapi-schema/alternate-conflict-dict.err b/tests/qapi-schema/alternate-conflict-dict.err > index d4970284ba..05174ab4f1 100644 > --- a/tests/qapi-schema/alternate-conflict-dict.err > +++ b/tests/qapi-schema/alternate-conflict-dict.err > @@ -1,2 +1,2 @@ > alternate-conflict-dict.json: In alternate 'Alt': > -alternate-conflict-dict.json:6: branch 'two' can't be distinguished from 'one' > +alternate-conflict-dict.json:6: branch 'two' of type 'Alt' can't be distinguished from 'one' > diff --git a/tests/qapi-schema/alternate-conflict-enum-bool.err b/tests/qapi-schema/alternate-conflict-enum-bool.err > index 5f35855274..02ed66b0bf 100644 > --- a/tests/qapi-schema/alternate-conflict-enum-bool.err > +++ b/tests/qapi-schema/alternate-conflict-enum-bool.err > @@ -1,2 +1,2 @@ > alternate-conflict-enum-bool.json: In alternate 'Alt': > -alternate-conflict-enum-bool.json:4: branch 'two' can't be distinguished from 'one' > +alternate-conflict-enum-bool.json:4: branch 'two' of type 'Alt' can't be distinguished from 'one' > diff --git a/tests/qapi-schema/alternate-conflict-enum-int.err b/tests/qapi-schema/alternate-conflict-enum-int.err > index 6a6d156664..1874558f49 100644 > --- a/tests/qapi-schema/alternate-conflict-enum-int.err > +++ b/tests/qapi-schema/alternate-conflict-enum-int.err > @@ -1,2 +1,2 @@ > alternate-conflict-enum-int.json: In alternate 'Alt': > -alternate-conflict-enum-int.json:4: branch 'two' can't be distinguished from 'one' > +alternate-conflict-enum-int.json:4: branch 'two' of type 'Alt' can't be distinguished from 'one' > diff --git a/tests/qapi-schema/alternate-conflict-lists.err b/tests/qapi-schema/alternate-conflict-lists.err > index f3374ec1e7..de480dc86b 100644 > --- a/tests/qapi-schema/alternate-conflict-lists.err > +++ b/tests/qapi-schema/alternate-conflict-lists.err > @@ -1,2 +1,2 @@ > alternate-conflict-lists.json: In alternate 'Alt': > -alternate-conflict-lists.json:4: branch 'two' can't be distinguished from 'one' > +alternate-conflict-lists.json:4: branch 'two' of type 'Alt' can't be distinguished from 'one' > diff --git a/tests/qapi-schema/alternate-conflict-num-string.err b/tests/qapi-schema/alternate-conflict-num-string.err > index 38c805ea1f..0d21ec590c 100644 > --- a/tests/qapi-schema/alternate-conflict-num-string.err > +++ b/tests/qapi-schema/alternate-conflict-num-string.err > @@ -1,2 +1,2 @@ > alternate-conflict-num-string.json: In alternate 'Alt': > -alternate-conflict-num-string.json:2: branch 'two' can't be distinguished from 'one' > +alternate-conflict-num-string.json:2: branch 'two' of type 'Alt' can't be distinguished from 'one' > diff --git a/tests/qapi-schema/alternate-conflict-string.err b/tests/qapi-schema/alternate-conflict-string.err > index 2fa08193db..97097cae97 100644 > --- a/tests/qapi-schema/alternate-conflict-string.err > +++ b/tests/qapi-schema/alternate-conflict-string.err > @@ -1,2 +1,2 @@ > alternate-conflict-string.json: In alternate 'Alt': > -alternate-conflict-string.json:2: branch 'two' can't be distinguished from 'one' > +alternate-conflict-string.json:2: branch 'two' of type 'Alt' can't be distinguished from 'one' > diff --git a/tests/qapi-schema/alternate-nested.err b/tests/qapi-schema/alternate-nested.err > index 3ae9cd2f11..5603aa95e3 100644 > --- a/tests/qapi-schema/alternate-nested.err > +++ b/tests/qapi-schema/alternate-nested.err > @@ -1,2 +1,2 @@ > alternate-nested.json: In alternate 'Alt2': > -alternate-nested.json:4: branch 'nested' cannot use alternate type 'Alt1' > +alternate-nested.json:4: branch 'nested' of type 'Alt2' cannot use alternate type 'Alt1' > diff --git a/tests/qapi-schema/alternate-unknown.err b/tests/qapi-schema/alternate-unknown.err > index 17fec1cd17..befd207a76 100644 > --- a/tests/qapi-schema/alternate-unknown.err > +++ b/tests/qapi-schema/alternate-unknown.err > @@ -1,2 +1,2 @@ > alternate-unknown.json: In alternate 'Alt': > -alternate-unknown.json:2: branch 'unknown' uses unknown type 'MissingType' > +alternate-unknown.json:2: branch 'unknown' of type 'Alt' uses unknown type 'MissingType' Redundant the same way in all of the test cases so far. > diff --git a/tests/qapi-schema/args-member-unknown.err b/tests/qapi-schema/args-member-unknown.err > index 96b6e5d289..b24c2ae572 100644 > --- a/tests/qapi-schema/args-member-unknown.err > +++ b/tests/qapi-schema/args-member-unknown.err > @@ -1,2 +1,2 @@ > args-member-unknown.json: In command 'oops': > -args-member-unknown.json:2: parameter 'member' uses unknown type 'NoSuchType' > +args-member-unknown.json:2: parameter 'member' of type 'oops-arg' uses unknown type 'NoSuchType' This is args-member-unknown.json: # we reject data if it does not contain a known type { 'command': 'oops', 'data': { 'member': 'NoSuchType' } } There is no type 'oops-arg'. Confusing. 'oops-arg' is in fact the argument type implicitly defined by { 'member': 'NoSuchType' }. > diff --git a/tests/qapi-schema/enum-clash-member.err b/tests/qapi-schema/enum-clash-member.err > index e4eb102ae2..0ede36d68e 100644 > --- a/tests/qapi-schema/enum-clash-member.err > +++ b/tests/qapi-schema/enum-clash-member.err > @@ -1,2 +1,2 @@ > enum-clash-member.json: In enum 'MyEnum': > -enum-clash-member.json:3: value 'one_two' collides with value 'one-two' > +enum-clash-member.json:3: value 'one_two' of type 'MyEnum' collides with value 'one-two' of type 'MyEnum' > diff --git a/tests/qapi-schema/features-duplicate-name.err b/tests/qapi-schema/features-duplicate-name.err > index 0adbee6b0a..ffd124f5b0 100644 > --- a/tests/qapi-schema/features-duplicate-name.err > +++ b/tests/qapi-schema/features-duplicate-name.err > @@ -1,2 +1,2 @@ > features-duplicate-name.json: In struct 'FeatureStruct0': > -features-duplicate-name.json:1: feature 'foo' collides with feature 'foo' > +features-duplicate-name.json:1: feature 'foo' of type 'FeatureStruct0' collides with feature 'foo' of type 'FeatureStruct0' > diff --git a/tests/qapi-schema/struct-base-clash-deep.err b/tests/qapi-schema/struct-base-clash-deep.err > index 79879681d9..632f78b6c0 100644 > --- a/tests/qapi-schema/struct-base-clash-deep.err > +++ b/tests/qapi-schema/struct-base-clash-deep.err > @@ -1,2 +1,2 @@ > struct-base-clash-deep.json: In struct 'Sub': > -struct-base-clash-deep.json:10: member 'name' collides with member 'name' of type 'Base' > +struct-base-clash-deep.json:10: member 'name' of type 'Sub' collides with member 'name' of type 'Base' > diff --git a/tests/qapi-schema/struct-base-clash.err b/tests/qapi-schema/struct-base-clash.err > index 46473947e6..1d54079c16 100644 > --- a/tests/qapi-schema/struct-base-clash.err > +++ b/tests/qapi-schema/struct-base-clash.err > @@ -1,2 +1,2 @@ > struct-base-clash.json: In struct 'Sub': > -struct-base-clash.json:5: member 'name' collides with member 'name' of type 'Base' > +struct-base-clash.json:5: member 'name' of type 'Sub' collides with member 'name' of type 'Base' > diff --git a/tests/qapi-schema/struct-member-name-clash.err b/tests/qapi-schema/struct-member-name-clash.err > index 7e53a605d2..ebf66cd5b8 100644 > --- a/tests/qapi-schema/struct-member-name-clash.err > +++ b/tests/qapi-schema/struct-member-name-clash.err > @@ -1,2 +1,2 @@ > struct-member-name-clash.json: In struct 'Oops': > -struct-member-name-clash.json:5: member 'a_b' collides with member 'a-b' > +struct-member-name-clash.json:5: member 'a_b' of type 'Oops' collides with member 'a-b' of type 'Oops' More test cases where it's redundant. > diff --git a/tests/qapi-schema/union-bad-base.err b/tests/qapi-schema/union-bad-base.err > index 42b2ed1dda..9f92b35a07 100644 > --- a/tests/qapi-schema/union-bad-base.err > +++ b/tests/qapi-schema/union-bad-base.err > @@ -1,2 +1,2 @@ > union-bad-base.json: In union 'TestUnion': > -union-bad-base.json:8: member 'string' of type 'TestTypeA' collides with base member 'string' > +union-bad-base.json:8: member 'string' of type 'TestTypeA' collides with base member 'string' of type 'TestUnion-base' This is union-bad-base.json: # we allow anonymous base, but enforce no duplicate keys { 'enum': 'TestEnum', 'data': [ 'value1', 'value2' ] } { 'struct': 'TestTypeA', 'data': { 'string': 'str' } } { 'struct': 'TestTypeB', 'data': { 'integer': 'int' } } { 'union': 'TestUnion', 'base': { 'enum1': 'TestEnum', 'string': 'str' }, 'discriminator': 'enum1', 'data': { 'value1': 'TestTypeA', 'value2': 'TestTypeB' } } There is no type 'TestUnion-base'. Confusing. 'TestUnion-base' is in fact the base type implicitly defined by { 'enum1': 'TestEnum', 'string': 'str' }. > diff --git a/tests/qapi-schema/union-int-branch.err b/tests/qapi-schema/union-int-branch.err > index 8fdc81edd1..302e3976e0 100644 > --- a/tests/qapi-schema/union-int-branch.err > +++ b/tests/qapi-schema/union-int-branch.err > @@ -1,2 +1,2 @@ > union-int-branch.json: In union 'TestUnion': > -union-int-branch.json:8: branch 'value1' cannot use built-in type 'int' > +union-int-branch.json:8: branch 'value1' of type 'TestUnion' cannot use built-in type 'int' > diff --git a/tests/qapi-schema/union-unknown.err b/tests/qapi-schema/union-unknown.err > index dad79beae0..e60ab9421a 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:3: branch 'unknown' uses unknown type 'MissingType' > +union-unknown.json:3: branch 'unknown' of type 'Union' uses unknown type 'MissingType' Redundant again in these two. No test case demonstrates the commit message's claim "in some cases it will help to reduce ambiguity". PATCH 3 adds one: union-invalid-union-subfield. It also adds another confusing one: union-invalid-union-subtype. Let's have a closer look at QAPISchemaMember.describe() before the patch: def describe(self, info): role = self.role @role is 'member' when self is a QAPISchemaMember, 'value' when it's a QAPISchemaEnumMember, 'feature' when it's a QAPISchemaFeature, and 'branch' when it's a QAPISchemaVariant. defined_in = self.defined_in This is the name of the thing where this QAPISchemaMember is defined. When object type T inherits member M from B, it's B's name, not T's. assert defined_in if defined_in.startswith('q_obj_'): It's a member of an implicitly defined object type. Special treatment to avoid exposing the made-up names of implicitly defined object types to the user: # See QAPISchema._make_implicit_object_type() - reverse the # mapping there to create a nice human-readable description defined_in = defined_in[6:] if defined_in.endswith('-arg'): # Implicit type created for a command's dict 'data' Defined in a command's implicit argument type (you get that when the value of 'data' is { ... }). assert role == 'member' role = 'parameter' @defined_in already got adjusted to the command name. Adjust role. elif defined_in.endswith('-base'): # Implicit type created for a union's dict 'base' Defined in a union's implicit base type (you get that when the value of 'base' is { ... }). role = 'base ' + role @defined_in already got adjusted to the union name. Adjust role. else: assert False Explodes when we add another kind of implicitly defined object type and forget to cover it here. elif defined_in != info.defn_name: Not a member of an implicit object type, and @defined_in is not in the context line "FILE: In META 'NAME':" printed by QAPISourceInfo.__str__(), typically via. QAPISourceError.__str__() from main()'s exception handler for QAPIError. Use format "ROLE 'MEMBER-NAME' of type 'DEFINED-IN'": return "%s '%s' of type '%s'" % (role, self.name, defined_in) Either a member of an implicit object type, or @defined_in is in the context line. Description is "ROLE 'MEMBER-NAME': return "%s '%s'" % (role, self.name) The patch ditches the second format, exposing @defined_in to the user even when it's the name of an implicitly defined object type. Before we discuss how to avoid that, let's take a step back and examine the problem we're trying to solve. PATCH 3 permits union branches to be unions. It adds test case union-invalid-union-subtype.json: # Clash between common member and union variant's common member # Base's member 'type' clashes with TestTypeA's { 'enum': 'TestEnum', 'data': [ 'value-a', 'value-b' ] } { 'enum': 'TestEnumA', 'data': [ 'value-a1', 'value-a2' ] } { 'struct': 'TestTypeA1', 'data': { 'integer': 'int' } } { 'struct': 'TestTypeA2', 'data': { 'integer': 'int' } } { 'union': 'TestTypeA', 'base': { 'type': 'TestEnumA' }, 'discriminator': 'type', 'data': { 'value-a1': 'TestTypeA1', 'value-a2': 'TestTypeA2' } } { 'struct': 'TestTypeB', 'data': { 'integer': 'int' } } { 'union': 'TestUnion', 'base': { 'type': 'TestEnum' }, 'discriminator': 'type', 'data': { 'value-a': 'TestTypeA', 'value-b': 'TestTypeB' } } Without this patch, the clash is reported like this: union-invalid-union-subtype.json: In union 'TestUnion': union-invalid-union-subtype.json:25: base member 'type' collides with base member 'type' Crap. Comes from QAPISchemaMember.check_clash(): raise QAPISemError( info, "%s collides with %s" % (self.describe(info), seen[cname].describe(info))) @self is member 'type' of TestTypeA's base, and seen[cname] is member 'type' of TestUnion's base. What we'd like to see instead is something like "base member 'type' of type 'TestTypeA' collides with base member 'type'". Here's my attempt to get us this message: def describe(self, info): role = self.role meta = 'type' defined_in = self.defined_in assert defined_in if defined_in.startswith('q_obj_'): # See QAPISchema._make_implicit_object_type() - reverse the # mapping there to create a nice human-readable description defined_in = defined_in[6:] if defined_in.endswith('-arg'): # Implicit type created for a command's dict 'data' assert role == 'member' role = 'parameter' meta = 'command' defined_in = defined_in[:-4] elif defined_in.endswith('-base'): # Implicit type created for a union's dict 'base' role = 'base ' + role defined_in = defined_in[:-5] else: assert False if defined_in != info.defn_name: return "%s '%s' of %s '%s'" % (role, self.name, meta, defined_in) return "%s '%s'" % (role, self.name) No change to output of existing tests. ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 2/3] qapi: use env var to trigger qapi test output updates 2023-02-23 13:40 [PATCH v2 0/3] qapi: allow unions to contain further unions Daniel P. Berrangé 2023-02-23 13:40 ` [PATCH v2 1/3] qapi: improve specificity of type/member descriptions Daniel P. Berrangé @ 2023-02-23 13:40 ` Daniel P. Berrangé 2023-02-24 19:28 ` Eric Blake 2023-03-17 12:05 ` Markus Armbruster 2023-02-23 13:40 ` [PATCH v2 3/3] qapi: allow unions to contain further unions Daniel P. Berrangé 2023-03-17 15:55 ` [PATCH v2 0/3] " Markus Armbruster 3 siblings, 2 replies; 13+ messages in thread From: Daniel P. Berrangé @ 2023-02-23 13:40 UTC (permalink / raw) To: qemu-devel Cc: Eric Blake, Michael Roth, Markus Armbruster, Het Gala, Daniel P. Berrangé It is possible to pass --update to tests/qapi-schema/test-qapi.py to make it update the output files on error. This is inconvient to achieve though when test-qapi.py is run indirectly by make/meson. Instead simply allow for an env variable to be set: $ QAPI_TEST_UPDATE=1 make check-qapi-schema Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- tests/qapi-schema/test-qapi.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py index 2160cef082..75f2759fd6 100755 --- a/tests/qapi-schema/test-qapi.py +++ b/tests/qapi-schema/test-qapi.py @@ -215,7 +215,8 @@ def main(argv): (dir_name, base_name) = os.path.split(t) dir_name = dir_name or args.dir test_name = os.path.splitext(base_name)[0] - status |= test_and_diff(test_name, dir_name, args.update) + update = args.update or "QAPI_TEST_UPDATE" in os.environ + status |= test_and_diff(test_name, dir_name, update) exit(status) -- 2.39.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/3] qapi: use env var to trigger qapi test output updates 2023-02-23 13:40 ` [PATCH v2 2/3] qapi: use env var to trigger qapi test output updates Daniel P. Berrangé @ 2023-02-24 19:28 ` Eric Blake 2023-03-17 12:05 ` Markus Armbruster 1 sibling, 0 replies; 13+ messages in thread From: Eric Blake @ 2023-02-24 19:28 UTC (permalink / raw) To: Daniel P. Berrangé Cc: qemu-devel, Michael Roth, Markus Armbruster, Het Gala On Thu, Feb 23, 2023 at 01:40:26PM +0000, Daniel P. Berrangé wrote: > It is possible to pass --update to tests/qapi-schema/test-qapi.py > to make it update the output files on error. This is inconvient inconvenient > to achieve though when test-qapi.py is run indirectly by make/meson. > > Instead simply allow for an env variable to be set: > > $ QAPI_TEST_UPDATE=1 make check-qapi-schema > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > tests/qapi-schema/test-qapi.py | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py > index 2160cef082..75f2759fd6 100755 > --- a/tests/qapi-schema/test-qapi.py > +++ b/tests/qapi-schema/test-qapi.py > @@ -215,7 +215,8 @@ def main(argv): > (dir_name, base_name) = os.path.split(t) > dir_name = dir_name or args.dir > test_name = os.path.splitext(base_name)[0] > - status |= test_and_diff(test_name, dir_name, args.update) > + update = args.update or "QAPI_TEST_UPDATE" in os.environ > + status |= test_and_diff(test_name, dir_name, update) 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] 13+ messages in thread
* Re: [PATCH v2 2/3] qapi: use env var to trigger qapi test output updates 2023-02-23 13:40 ` [PATCH v2 2/3] qapi: use env var to trigger qapi test output updates Daniel P. Berrangé 2023-02-24 19:28 ` Eric Blake @ 2023-03-17 12:05 ` Markus Armbruster 1 sibling, 0 replies; 13+ messages in thread From: Markus Armbruster @ 2023-03-17 12:05 UTC (permalink / raw) To: Daniel P. Berrangé; +Cc: qemu-devel, Eric Blake, Michael Roth, Het Gala I have a mild dislike of abbreviations like "env var". Perhaps: qapi: Support updating expected test output via make Daniel P. Berrangé <berrange@redhat.com> writes: > It is possible to pass --update to tests/qapi-schema/test-qapi.py > to make it update the output files on error. This is inconvient > to achieve though when test-qapi.py is run indirectly by make/meson. > > Instead simply allow for an env variable to be set: > > $ QAPI_TEST_UPDATE=1 make check-qapi-schema Suggest to omit the value $ QAPI_TEST_UPDATE= make check-qapi-schema The value doesn't actually matter. A value of 1 looks as if it did, and as if a value of 0 would disable the thing. I'm no fan of environment variables duplicating options, but I understand the desire for a make invocation, and I don't have a better idea. > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > tests/qapi-schema/test-qapi.py | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py > index 2160cef082..75f2759fd6 100755 > --- a/tests/qapi-schema/test-qapi.py > +++ b/tests/qapi-schema/test-qapi.py > @@ -215,7 +215,8 @@ def main(argv): > (dir_name, base_name) = os.path.split(t) > dir_name = dir_name or args.dir > test_name = os.path.splitext(base_name)[0] > - status |= test_and_diff(test_name, dir_name, args.update) > + update = args.update or "QAPI_TEST_UPDATE" in os.environ > + status |= test_and_diff(test_name, dir_name, update) > > exit(status) Let's use argparse instead: parser.add_argument('-u', '--update', action='store_true', + default='QAPI_TEST_UPDATE' in os.environ, help="update expected test results") ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 3/3] qapi: allow unions to contain further unions 2023-02-23 13:40 [PATCH v2 0/3] qapi: allow unions to contain further unions Daniel P. Berrangé 2023-02-23 13:40 ` [PATCH v2 1/3] qapi: improve specificity of type/member descriptions Daniel P. Berrangé 2023-02-23 13:40 ` [PATCH v2 2/3] qapi: use env var to trigger qapi test output updates Daniel P. Berrangé @ 2023-02-23 13:40 ` Daniel P. Berrangé 2023-03-07 3:53 ` Het Gala 2023-03-17 15:48 ` Markus Armbruster 2023-03-17 15:55 ` [PATCH v2 0/3] " Markus Armbruster 3 siblings, 2 replies; 13+ messages in thread From: Daniel P. Berrangé @ 2023-02-23 13:40 UTC (permalink / raw) To: qemu-devel Cc: Eric Blake, Michael Roth, Markus Armbruster, Het Gala, Daniel P. Berrangé This extends the QAPI schema validation to permit unions inside unions, provided the checks for clashing fields pass. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- scripts/qapi/schema.py | 6 +- tests/qapi-schema/meson.build | 2 + tests/qapi-schema/qapi-schema-test.json | 32 ++++++++++ tests/qapi-schema/qapi-schema-test.out | 29 ++++++++++ .../union-invalid-union-subfield.err | 2 + .../union-invalid-union-subfield.json | 30 ++++++++++ .../union-invalid-union-subfield.out | 0 .../union-invalid-union-subtype.err | 2 + .../union-invalid-union-subtype.json | 29 ++++++++++ .../union-invalid-union-subtype.out | 0 tests/unit/test-qobject-input-visitor.c | 47 +++++++++++++++ tests/unit/test-qobject-output-visitor.c | 58 +++++++++++++++++++ 12 files changed, 234 insertions(+), 3 deletions(-) create mode 100644 tests/qapi-schema/union-invalid-union-subfield.err create mode 100644 tests/qapi-schema/union-invalid-union-subfield.json create mode 100644 tests/qapi-schema/union-invalid-union-subfield.out create mode 100644 tests/qapi-schema/union-invalid-union-subtype.err create mode 100644 tests/qapi-schema/union-invalid-union-subtype.json create mode 100644 tests/qapi-schema/union-invalid-union-subtype.out diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index 6c481ab0c0..5c4457f789 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -465,9 +465,10 @@ def check(self, schema): # on behalf of info, which is not necessarily self.info def check_clash(self, info, seen): assert self._checked - assert not self.variants # not implemented for m in self.members: m.check_clash(info, seen) + if self.variants: + self.variants.check_clash(info, seen) def connect_doc(self, doc=None): super().connect_doc(doc) @@ -652,8 +653,7 @@ def check(self, schema, seen): self.info, "branch '%s' is not a value of %s" % (v.name, self.tag_member.type.describe())) - if (not isinstance(v.type, QAPISchemaObjectType) - or v.type.variants): + if not isinstance(v.type, QAPISchemaObjectType): raise QAPISemError( self.info, "%s cannot use %s" diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build index d85b14f28c..1591eb322b 100644 --- a/tests/qapi-schema/meson.build +++ b/tests/qapi-schema/meson.build @@ -194,6 +194,8 @@ schemas = [ 'union-invalid-data.json', 'union-invalid-discriminator.json', 'union-invalid-if-discriminator.json', + 'union-invalid-union-subfield.json', + 'union-invalid-union-subtype.json', 'union-no-base.json', 'union-optional-discriminator.json', 'union-string-discriminator.json', diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json index ba7302f42b..40f1a3d88d 100644 --- a/tests/qapi-schema/qapi-schema-test.json +++ b/tests/qapi-schema/qapi-schema-test.json @@ -114,6 +114,38 @@ { 'struct': 'UserDefC', 'data': { 'string1': 'str', 'string2': 'str' } } +# this tests that unions can contain other unions in their branches +{ 'enum': 'TestUnionEnum', + 'data': [ 'value-a', 'value-b' ] } + +{ 'enum': 'TestUnionEnumA', + 'data': [ 'value-a1', 'value-a2' ] } + +{ 'struct': 'TestUnionTypeA1', + 'data': { 'integer': 'int', + 'name': 'str'} } + +{ 'struct': 'TestUnionTypeA2', + 'data': { 'integer': 'int', + 'size': 'int' } } + +{ 'union': 'TestUnionTypeA', + 'base': { 'type-a': 'TestUnionEnumA' }, + 'discriminator': 'type-a', + 'data': { 'value-a1': 'TestUnionTypeA1', + 'value-a2': 'TestUnionTypeA2' } } + +{ 'struct': 'TestUnionTypeB', + 'data': { 'integer': 'int', + 'onoff': 'bool' } } + +{ 'union': 'TestUnionInUnion', + 'base': { 'type': 'TestUnionEnum' }, + 'discriminator': 'type', + 'data': { 'value-a': 'TestUnionTypeA', + 'value-b': 'TestUnionTypeB' } } + + # for testing use of 'number' within alternates { 'alternate': 'AltEnumBool', 'data': { 'e': 'EnumOne', 'b': 'bool' } } { 'alternate': 'AltEnumNum', 'data': { 'e': 'EnumOne', 'n': 'number' } } diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out index 043d75c655..9fe1038944 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -105,6 +105,35 @@ alternate UserDefAlternate object UserDefC member string1: str optional=False member string2: str optional=False +enum TestUnionEnum + member value-a + member value-b +enum TestUnionEnumA + member value-a1 + member value-a2 +object TestUnionTypeA1 + member integer: int optional=False + member name: str optional=False +object TestUnionTypeA2 + member integer: int optional=False + member size: int optional=False +object q_obj_TestUnionTypeA-base + member type-a: TestUnionEnumA optional=False +object TestUnionTypeA + base q_obj_TestUnionTypeA-base + tag type-a + case value-a1: TestUnionTypeA1 + case value-a2: TestUnionTypeA2 +object TestUnionTypeB + member integer: int optional=False + member onoff: bool optional=False +object q_obj_TestUnionInUnion-base + member type: TestUnionEnum optional=False +object TestUnionInUnion + base q_obj_TestUnionInUnion-base + tag type + case value-a: TestUnionTypeA + case value-b: TestUnionTypeB alternate AltEnumBool tag type case e: EnumOne diff --git a/tests/qapi-schema/union-invalid-union-subfield.err b/tests/qapi-schema/union-invalid-union-subfield.err new file mode 100644 index 0000000000..43574dea79 --- /dev/null +++ b/tests/qapi-schema/union-invalid-union-subfield.err @@ -0,0 +1,2 @@ +union-invalid-union-subfield.json: In union 'TestUnion': +union-invalid-union-subfield.json:25: member 'teeth' of type 'TestTypeFish' collides with base member 'teeth' of type 'TestUnion-base' diff --git a/tests/qapi-schema/union-invalid-union-subfield.json b/tests/qapi-schema/union-invalid-union-subfield.json new file mode 100644 index 0000000000..e1639d3a96 --- /dev/null +++ b/tests/qapi-schema/union-invalid-union-subfield.json @@ -0,0 +1,30 @@ +# Clash between common member and union variant's variant member +# Base's member 'teeth' clashes with TestTypeFish's + +{ 'enum': 'TestEnum', + 'data': [ 'animals', 'plants' ] } + +{ 'enum': 'TestAnimals', + 'data': [ 'fish', 'birds'] } + +{ 'struct': 'TestTypeFish', + 'data': { 'scales': 'int', 'teeth': 'int' } } + +{ 'struct': 'TestTypeBirds', + 'data': { 'feathers': 'int' } } + +{ 'union': 'TestTypeAnimals', + 'base': { 'atype': 'TestAnimals' }, + 'discriminator': 'atype', + 'data': { 'fish': 'TestTypeFish', + 'birds': 'TestTypeBirds' } } + +{ 'struct': 'TestTypePlants', + 'data': { 'integer': 'int' } } + +{ 'union': 'TestUnion', + 'base': { 'type': 'TestEnum', + 'teeth': 'int' }, + 'discriminator': 'type', + 'data': { 'animals': 'TestTypeAnimals', + 'plants': 'TestTypePlants' } } diff --git a/tests/qapi-schema/union-invalid-union-subfield.out b/tests/qapi-schema/union-invalid-union-subfield.out new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/qapi-schema/union-invalid-union-subtype.err b/tests/qapi-schema/union-invalid-union-subtype.err new file mode 100644 index 0000000000..e45f330cec --- /dev/null +++ b/tests/qapi-schema/union-invalid-union-subtype.err @@ -0,0 +1,2 @@ +union-invalid-union-subtype.json: In union 'TestUnion': +union-invalid-union-subtype.json:25: base member 'type' of type 'TestTypeA-base' collides with base member 'type' of type 'TestUnion-base' diff --git a/tests/qapi-schema/union-invalid-union-subtype.json b/tests/qapi-schema/union-invalid-union-subtype.json new file mode 100644 index 0000000000..ce1de51d8d --- /dev/null +++ b/tests/qapi-schema/union-invalid-union-subtype.json @@ -0,0 +1,29 @@ +# Clash between common member and union variant's common member +# Base's member 'type' clashes with TestTypeA's + +{ 'enum': 'TestEnum', + 'data': [ 'value-a', 'value-b' ] } + +{ 'enum': 'TestEnumA', + 'data': [ 'value-a1', 'value-a2' ] } + +{ 'struct': 'TestTypeA1', + 'data': { 'integer': 'int' } } + +{ 'struct': 'TestTypeA2', + 'data': { 'integer': 'int' } } + +{ 'union': 'TestTypeA', + 'base': { 'type': 'TestEnumA' }, + 'discriminator': 'type', + 'data': { 'value-a1': 'TestTypeA1', + 'value-a2': 'TestTypeA2' } } + +{ 'struct': 'TestTypeB', + 'data': { 'integer': 'int' } } + +{ 'union': 'TestUnion', + 'base': { 'type': 'TestEnum' }, + 'discriminator': 'type', + 'data': { 'value-a': 'TestTypeA', + 'value-b': 'TestTypeB' } } diff --git a/tests/qapi-schema/union-invalid-union-subtype.out b/tests/qapi-schema/union-invalid-union-subtype.out new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/unit/test-qobject-input-visitor.c b/tests/unit/test-qobject-input-visitor.c index 77fbf985be..9b3e2dbe14 100644 --- a/tests/unit/test-qobject-input-visitor.c +++ b/tests/unit/test-qobject-input-visitor.c @@ -706,6 +706,51 @@ static void test_visitor_in_union_flat(TestInputVisitorData *data, g_assert(&base->enum1 == &tmp->enum1); } +static void test_visitor_in_union_in_union(TestInputVisitorData *data, + const void *unused) +{ + Visitor *v; + g_autoptr(TestUnionInUnion) tmp = NULL; + + v = visitor_input_test_init(data, + "{ 'type': 'value-a', " + " 'type-a': 'value-a1', " + " 'integer': 2, " + " 'name': 'fish' }"); + + visit_type_TestUnionInUnion(v, NULL, &tmp, &error_abort); + g_assert_cmpint(tmp->type, ==, TEST_UNION_ENUM_VALUE_A); + g_assert_cmpint(tmp->u.value_a.type_a, ==, TEST_UNION_ENUMA_VALUE_A1); + g_assert_cmpint(tmp->u.value_a.u.value_a1.integer, ==, 2); + g_assert_cmpint(strcmp(tmp->u.value_a.u.value_a1.name, "fish"), ==, 0); + + qapi_free_TestUnionInUnion(tmp); + + v = visitor_input_test_init(data, + "{ 'type': 'value-a', " + " 'type-a': 'value-a2', " + " 'integer': 1729, " + " 'size': 87539319 }"); + + visit_type_TestUnionInUnion(v, NULL, &tmp, &error_abort); + g_assert_cmpint(tmp->type, ==, TEST_UNION_ENUM_VALUE_A); + g_assert_cmpint(tmp->u.value_a.type_a, ==, TEST_UNION_ENUMA_VALUE_A2); + g_assert_cmpint(tmp->u.value_a.u.value_a2.integer, ==, 1729); + g_assert_cmpint(tmp->u.value_a.u.value_a2.size, ==, 87539319); + + qapi_free_TestUnionInUnion(tmp); + + v = visitor_input_test_init(data, + "{ 'type': 'value-b', " + " 'integer': 1729, " + " 'onoff': true }"); + + visit_type_TestUnionInUnion(v, NULL, &tmp, &error_abort); + g_assert_cmpint(tmp->type, ==, TEST_UNION_ENUM_VALUE_B); + g_assert_cmpint(tmp->u.value_b.integer, ==, 1729); + g_assert_cmpint(tmp->u.value_b.onoff, ==, true); +} + static void test_visitor_in_alternate(TestInputVisitorData *data, const void *unused) { @@ -1216,6 +1261,8 @@ int main(int argc, char **argv) NULL, test_visitor_in_null); input_visitor_test_add("/visitor/input/union-flat", NULL, test_visitor_in_union_flat); + input_visitor_test_add("/visitor/input/union-in-union", + NULL, test_visitor_in_union_in_union); input_visitor_test_add("/visitor/input/alternate", NULL, test_visitor_in_alternate); input_visitor_test_add("/visitor/input/errors", diff --git a/tests/unit/test-qobject-output-visitor.c b/tests/unit/test-qobject-output-visitor.c index 7f054289fe..1535b3ad17 100644 --- a/tests/unit/test-qobject-output-visitor.c +++ b/tests/unit/test-qobject-output-visitor.c @@ -352,6 +352,62 @@ static void test_visitor_out_union_flat(TestOutputVisitorData *data, qapi_free_UserDefFlatUnion(tmp); } +static void test_visitor_out_union_in_union(TestOutputVisitorData *data, + const void *unused) +{ + QDict *qdict; + + TestUnionInUnion *tmp = g_new0(TestUnionInUnion, 1); + tmp->type = TEST_UNION_ENUM_VALUE_A; + tmp->u.value_a.type_a = TEST_UNION_ENUMA_VALUE_A1; + tmp->u.value_a.u.value_a1.integer = 42; + tmp->u.value_a.u.value_a1.name = g_strdup("fish"); + + visit_type_TestUnionInUnion(data->ov, NULL, &tmp, &error_abort); + qdict = qobject_to(QDict, visitor_get(data)); + g_assert(qdict); + g_assert_cmpstr(qdict_get_str(qdict, "type"), ==, "value-a"); + g_assert_cmpstr(qdict_get_str(qdict, "type-a"), ==, "value-a1"); + g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 42); + g_assert_cmpstr(qdict_get_str(qdict, "name"), ==, "fish"); + + qapi_free_TestUnionInUnion(tmp); + + + visitor_reset(data); + tmp = g_new0(TestUnionInUnion, 1); + tmp->type = TEST_UNION_ENUM_VALUE_A; + tmp->u.value_a.type_a = TEST_UNION_ENUMA_VALUE_A2; + tmp->u.value_a.u.value_a2.integer = 1729; + tmp->u.value_a.u.value_a2.size = 87539319; + + visit_type_TestUnionInUnion(data->ov, NULL, &tmp, &error_abort); + qdict = qobject_to(QDict, visitor_get(data)); + g_assert(qdict); + g_assert_cmpstr(qdict_get_str(qdict, "type"), ==, "value-a"); + g_assert_cmpstr(qdict_get_str(qdict, "type-a"), ==, "value-a2"); + g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 1729); + g_assert_cmpint(qdict_get_int(qdict, "size"), ==, 87539319); + + qapi_free_TestUnionInUnion(tmp); + + + visitor_reset(data); + tmp = g_new0(TestUnionInUnion, 1); + tmp->type = TEST_UNION_ENUM_VALUE_B; + tmp->u.value_b.integer = 1729; + tmp->u.value_b.onoff = true; + + visit_type_TestUnionInUnion(data->ov, NULL, &tmp, &error_abort); + qdict = qobject_to(QDict, visitor_get(data)); + g_assert(qdict); + g_assert_cmpstr(qdict_get_str(qdict, "type"), ==, "value-b"); + g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 1729); + g_assert_cmpint(qdict_get_bool(qdict, "onoff"), ==, true); + + qapi_free_TestUnionInUnion(tmp); +} + static void test_visitor_out_alternate(TestOutputVisitorData *data, const void *unused) { @@ -586,6 +642,8 @@ int main(int argc, char **argv) &out_visitor_data, test_visitor_out_list_qapi_free); output_visitor_test_add("/visitor/output/union-flat", &out_visitor_data, test_visitor_out_union_flat); + output_visitor_test_add("/visitor/output/union-in-union", + &out_visitor_data, test_visitor_out_union_in_union); output_visitor_test_add("/visitor/output/alternate", &out_visitor_data, test_visitor_out_alternate); output_visitor_test_add("/visitor/output/null", -- 2.39.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/3] qapi: allow unions to contain further unions 2023-02-23 13:40 ` [PATCH v2 3/3] qapi: allow unions to contain further unions Daniel P. Berrangé @ 2023-03-07 3:53 ` Het Gala 2023-03-17 15:48 ` Markus Armbruster 1 sibling, 0 replies; 13+ messages in thread From: Het Gala @ 2023-03-07 3:53 UTC (permalink / raw) To: Daniel P. Berrangé, qemu-devel Cc: Eric Blake, Michael Roth, Markus Armbruster Hi, On 23/02/23 7:10 pm, Daniel P. Berrangé wrote: > This extends the QAPI schema validation to permit unions inside unions, > provided the checks for clashing fields pass. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > scripts/qapi/schema.py | 6 +- > tests/qapi-schema/meson.build | 2 + > tests/qapi-schema/qapi-schema-test.json | 32 ++++++++++ > tests/qapi-schema/qapi-schema-test.out | 29 ++++++++++ > .../union-invalid-union-subfield.err | 2 + > .../union-invalid-union-subfield.json | 30 ++++++++++ > .../union-invalid-union-subfield.out | 0 > .../union-invalid-union-subtype.err | 2 + > .../union-invalid-union-subtype.json | 29 ++++++++++ > .../union-invalid-union-subtype.out | 0 > tests/unit/test-qobject-input-visitor.c | 47 +++++++++++++++ > tests/unit/test-qobject-output-visitor.c | 58 +++++++++++++++++++ > 12 files changed, 234 insertions(+), 3 deletions(-) > create mode 100644 tests/qapi-schema/union-invalid-union-subfield.err > create mode 100644 tests/qapi-schema/union-invalid-union-subfield.json > create mode 100644 tests/qapi-schema/union-invalid-union-subfield.out > create mode 100644 tests/qapi-schema/union-invalid-union-subtype.err > create mode 100644 tests/qapi-schema/union-invalid-union-subtype.json > create mode 100644 tests/qapi-schema/union-invalid-union-subtype.out > > diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py > index 6c481ab0c0..5c4457f789 100644 > --- a/scripts/qapi/schema.py > +++ b/scripts/qapi/schema.py > @@ -465,9 +465,10 @@ def check(self, schema): > # on behalf of info, which is not necessarily self.info > def check_clash(self, info, seen): > assert self._checked > - assert not self.variants # not implemented > for m in self.members: > m.check_clash(info, seen) > + if self.variants: > + self.variants.check_clash(info, seen) > > def connect_doc(self, doc=None): > super().connect_doc(doc) > @@ -652,8 +653,7 @@ def check(self, schema, seen): > self.info, > "branch '%s' is not a value of %s" > % (v.name, self.tag_member.type.describe())) > - if (not isinstance(v.type, QAPISchemaObjectType) > - or v.type.variants): > + if not isinstance(v.type, QAPISchemaObjectType): > raise QAPISemError( > self.info, > "%s cannot use %s" > diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build > index d85b14f28c..1591eb322b 100644 > --- a/tests/qapi-schema/meson.build > +++ b/tests/qapi-schema/meson.build > @@ -194,6 +194,8 @@ schemas = [ > 'union-invalid-data.json', > 'union-invalid-discriminator.json', > 'union-invalid-if-discriminator.json', > + 'union-invalid-union-subfield.json', > + 'union-invalid-union-subtype.json', > 'union-no-base.json', > 'union-optional-discriminator.json', > 'union-string-discriminator.json', > diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json > index ba7302f42b..40f1a3d88d 100644 > --- a/tests/qapi-schema/qapi-schema-test.json > +++ b/tests/qapi-schema/qapi-schema-test.json > @@ -114,6 +114,38 @@ > { 'struct': 'UserDefC', > 'data': { 'string1': 'str', 'string2': 'str' } } > > +# this tests that unions can contain other unions in their branches > +{ 'enum': 'TestUnionEnum', > + 'data': [ 'value-a', 'value-b' ] } > + > +{ 'enum': 'TestUnionEnumA', > + 'data': [ 'value-a1', 'value-a2' ] } > + > +{ 'struct': 'TestUnionTypeA1', > + 'data': { 'integer': 'int', > + 'name': 'str'} } > + > +{ 'struct': 'TestUnionTypeA2', > + 'data': { 'integer': 'int', > + 'size': 'int' } } > + > +{ 'union': 'TestUnionTypeA', > + 'base': { 'type-a': 'TestUnionEnumA' }, > + 'discriminator': 'type-a', > + 'data': { 'value-a1': 'TestUnionTypeA1', > + 'value-a2': 'TestUnionTypeA2' } } > + > +{ 'struct': 'TestUnionTypeB', > + 'data': { 'integer': 'int', > + 'onoff': 'bool' } } > + > +{ 'union': 'TestUnionInUnion', > + 'base': { 'type': 'TestUnionEnum' }, > + 'discriminator': 'type', > + 'data': { 'value-a': 'TestUnionTypeA', > + 'value-b': 'TestUnionTypeB' } } > + > + > # for testing use of 'number' within alternates > { 'alternate': 'AltEnumBool', 'data': { 'e': 'EnumOne', 'b': 'bool' } } > { 'alternate': 'AltEnumNum', 'data': { 'e': 'EnumOne', 'n': 'number' } } > diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out > index 043d75c655..9fe1038944 100644 > --- a/tests/qapi-schema/qapi-schema-test.out > +++ b/tests/qapi-schema/qapi-schema-test.out > @@ -105,6 +105,35 @@ alternate UserDefAlternate > object UserDefC > member string1: str optional=False > member string2: str optional=False > +enum TestUnionEnum > + member value-a > + member value-b > +enum TestUnionEnumA > + member value-a1 > + member value-a2 > +object TestUnionTypeA1 > + member integer: int optional=False > + member name: str optional=False > +object TestUnionTypeA2 > + member integer: int optional=False > + member size: int optional=False > +object q_obj_TestUnionTypeA-base > + member type-a: TestUnionEnumA optional=False > +object TestUnionTypeA > + base q_obj_TestUnionTypeA-base > + tag type-a > + case value-a1: TestUnionTypeA1 > + case value-a2: TestUnionTypeA2 > +object TestUnionTypeB > + member integer: int optional=False > + member onoff: bool optional=False > +object q_obj_TestUnionInUnion-base > + member type: TestUnionEnum optional=False > +object TestUnionInUnion > + base q_obj_TestUnionInUnion-base > + tag type > + case value-a: TestUnionTypeA > + case value-b: TestUnionTypeB > alternate AltEnumBool > tag type > case e: EnumOne > diff --git a/tests/qapi-schema/union-invalid-union-subfield.err b/tests/qapi-schema/union-invalid-union-subfield.err > new file mode 100644 > index 0000000000..43574dea79 > --- /dev/null > +++ b/tests/qapi-schema/union-invalid-union-subfield.err > @@ -0,0 +1,2 @@ > +union-invalid-union-subfield.json: In union 'TestUnion': > +union-invalid-union-subfield.json:25: member 'teeth' of type 'TestTypeFish' collides with base member 'teeth' of type 'TestUnion-base' > diff --git a/tests/qapi-schema/union-invalid-union-subfield.json b/tests/qapi-schema/union-invalid-union-subfield.json > new file mode 100644 > index 0000000000..e1639d3a96 > --- /dev/null > +++ b/tests/qapi-schema/union-invalid-union-subfield.json > @@ -0,0 +1,30 @@ > +# Clash between common member and union variant's variant member > +# Base's member 'teeth' clashes with TestTypeFish's > + > +{ 'enum': 'TestEnum', > + 'data': [ 'animals', 'plants' ] } > + > +{ 'enum': 'TestAnimals', > + 'data': [ 'fish', 'birds'] } > + > +{ 'struct': 'TestTypeFish', > + 'data': { 'scales': 'int', 'teeth': 'int' } } > + > +{ 'struct': 'TestTypeBirds', > + 'data': { 'feathers': 'int' } } > + > +{ 'union': 'TestTypeAnimals', > + 'base': { 'atype': 'TestAnimals' }, > + 'discriminator': 'atype', > + 'data': { 'fish': 'TestTypeFish', > + 'birds': 'TestTypeBirds' } } > + > +{ 'struct': 'TestTypePlants', > + 'data': { 'integer': 'int' } } > + > +{ 'union': 'TestUnion', > + 'base': { 'type': 'TestEnum', > + 'teeth': 'int' }, > + 'discriminator': 'type', > + 'data': { 'animals': 'TestTypeAnimals', > + 'plants': 'TestTypePlants' } } > diff --git a/tests/qapi-schema/union-invalid-union-subfield.out b/tests/qapi-schema/union-invalid-union-subfield.out > new file mode 100644 > index 0000000000..e69de29bb2 > diff --git a/tests/qapi-schema/union-invalid-union-subtype.err b/tests/qapi-schema/union-invalid-union-subtype.err > new file mode 100644 > index 0000000000..e45f330cec > --- /dev/null > +++ b/tests/qapi-schema/union-invalid-union-subtype.err > @@ -0,0 +1,2 @@ > +union-invalid-union-subtype.json: In union 'TestUnion': > +union-invalid-union-subtype.json:25: base member 'type' of type 'TestTypeA-base' collides with base member 'type' of type 'TestUnion-base' > diff --git a/tests/qapi-schema/union-invalid-union-subtype.json b/tests/qapi-schema/union-invalid-union-subtype.json > new file mode 100644 > index 0000000000..ce1de51d8d > --- /dev/null > +++ b/tests/qapi-schema/union-invalid-union-subtype.json > @@ -0,0 +1,29 @@ > +# Clash between common member and union variant's common member > +# Base's member 'type' clashes with TestTypeA's > + > +{ 'enum': 'TestEnum', > + 'data': [ 'value-a', 'value-b' ] } > + > +{ 'enum': 'TestEnumA', > + 'data': [ 'value-a1', 'value-a2' ] } > + > +{ 'struct': 'TestTypeA1', > + 'data': { 'integer': 'int' } } > + > +{ 'struct': 'TestTypeA2', > + 'data': { 'integer': 'int' } } > + > +{ 'union': 'TestTypeA', > + 'base': { 'type': 'TestEnumA' }, > + 'discriminator': 'type', > + 'data': { 'value-a1': 'TestTypeA1', > + 'value-a2': 'TestTypeA2' } } > + > +{ 'struct': 'TestTypeB', > + 'data': { 'integer': 'int' } } > + > +{ 'union': 'TestUnion', > + 'base': { 'type': 'TestEnum' }, > + 'discriminator': 'type', > + 'data': { 'value-a': 'TestTypeA', > + 'value-b': 'TestTypeB' } } > diff --git a/tests/qapi-schema/union-invalid-union-subtype.out b/tests/qapi-schema/union-invalid-union-subtype.out > new file mode 100644 > index 0000000000..e69de29bb2 > diff --git a/tests/unit/test-qobject-input-visitor.c b/tests/unit/test-qobject-input-visitor.c > index 77fbf985be..9b3e2dbe14 100644 > --- a/tests/unit/test-qobject-input-visitor.c > +++ b/tests/unit/test-qobject-input-visitor.c > @@ -706,6 +706,51 @@ static void test_visitor_in_union_flat(TestInputVisitorData *data, > g_assert(&base->enum1 == &tmp->enum1); > } > > +static void test_visitor_in_union_in_union(TestInputVisitorData *data, > + const void *unused) > +{ > + Visitor *v; > + g_autoptr(TestUnionInUnion) tmp = NULL; > + > + v = visitor_input_test_init(data, > + "{ 'type': 'value-a', " > + " 'type-a': 'value-a1', " > + " 'integer': 2, " > + " 'name': 'fish' }"); > + > + visit_type_TestUnionInUnion(v, NULL, &tmp, &error_abort); > + g_assert_cmpint(tmp->type, ==, TEST_UNION_ENUM_VALUE_A); > + g_assert_cmpint(tmp->u.value_a.type_a, ==, TEST_UNION_ENUMA_VALUE_A1); > + g_assert_cmpint(tmp->u.value_a.u.value_a1.integer, ==, 2); > + g_assert_cmpint(strcmp(tmp->u.value_a.u.value_a1.name, "fish"), ==, 0); > + > + qapi_free_TestUnionInUnion(tmp); > + > + v = visitor_input_test_init(data, > + "{ 'type': 'value-a', " > + " 'type-a': 'value-a2', " > + " 'integer': 1729, " > + " 'size': 87539319 }"); > + > + visit_type_TestUnionInUnion(v, NULL, &tmp, &error_abort); > + g_assert_cmpint(tmp->type, ==, TEST_UNION_ENUM_VALUE_A); > + g_assert_cmpint(tmp->u.value_a.type_a, ==, TEST_UNION_ENUMA_VALUE_A2); > + g_assert_cmpint(tmp->u.value_a.u.value_a2.integer, ==, 1729); > + g_assert_cmpint(tmp->u.value_a.u.value_a2.size, ==, 87539319); > + > + qapi_free_TestUnionInUnion(tmp); > + > + v = visitor_input_test_init(data, > + "{ 'type': 'value-b', " > + " 'integer': 1729, " > + " 'onoff': true }"); > + > + visit_type_TestUnionInUnion(v, NULL, &tmp, &error_abort); > + g_assert_cmpint(tmp->type, ==, TEST_UNION_ENUM_VALUE_B); > + g_assert_cmpint(tmp->u.value_b.integer, ==, 1729); > + g_assert_cmpint(tmp->u.value_b.onoff, ==, true); > +} > + > static void test_visitor_in_alternate(TestInputVisitorData *data, > const void *unused) > { > @@ -1216,6 +1261,8 @@ int main(int argc, char **argv) > NULL, test_visitor_in_null); > input_visitor_test_add("/visitor/input/union-flat", > NULL, test_visitor_in_union_flat); > + input_visitor_test_add("/visitor/input/union-in-union", > + NULL, test_visitor_in_union_in_union); > input_visitor_test_add("/visitor/input/alternate", > NULL, test_visitor_in_alternate); > input_visitor_test_add("/visitor/input/errors", > diff --git a/tests/unit/test-qobject-output-visitor.c b/tests/unit/test-qobject-output-visitor.c > index 7f054289fe..1535b3ad17 100644 > --- a/tests/unit/test-qobject-output-visitor.c > +++ b/tests/unit/test-qobject-output-visitor.c > @@ -352,6 +352,62 @@ static void test_visitor_out_union_flat(TestOutputVisitorData *data, > qapi_free_UserDefFlatUnion(tmp); > } > > +static void test_visitor_out_union_in_union(TestOutputVisitorData *data, > + const void *unused) > +{ > + QDict *qdict; > + > + TestUnionInUnion *tmp = g_new0(TestUnionInUnion, 1); > + tmp->type = TEST_UNION_ENUM_VALUE_A; > + tmp->u.value_a.type_a = TEST_UNION_ENUMA_VALUE_A1; > + tmp->u.value_a.u.value_a1.integer = 42; > + tmp->u.value_a.u.value_a1.name = g_strdup("fish"); > + > + visit_type_TestUnionInUnion(data->ov, NULL, &tmp, &error_abort); > + qdict = qobject_to(QDict, visitor_get(data)); > + g_assert(qdict); > + g_assert_cmpstr(qdict_get_str(qdict, "type"), ==, "value-a"); > + g_assert_cmpstr(qdict_get_str(qdict, "type-a"), ==, "value-a1"); > + g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 42); > + g_assert_cmpstr(qdict_get_str(qdict, "name"), ==, "fish"); > + > + qapi_free_TestUnionInUnion(tmp); > + > + > + visitor_reset(data); > + tmp = g_new0(TestUnionInUnion, 1); > + tmp->type = TEST_UNION_ENUM_VALUE_A; > + tmp->u.value_a.type_a = TEST_UNION_ENUMA_VALUE_A2; > + tmp->u.value_a.u.value_a2.integer = 1729; > + tmp->u.value_a.u.value_a2.size = 87539319; > + > + visit_type_TestUnionInUnion(data->ov, NULL, &tmp, &error_abort); > + qdict = qobject_to(QDict, visitor_get(data)); > + g_assert(qdict); > + g_assert_cmpstr(qdict_get_str(qdict, "type"), ==, "value-a"); > + g_assert_cmpstr(qdict_get_str(qdict, "type-a"), ==, "value-a2"); > + g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 1729); > + g_assert_cmpint(qdict_get_int(qdict, "size"), ==, 87539319); > + > + qapi_free_TestUnionInUnion(tmp); > + > + > + visitor_reset(data); > + tmp = g_new0(TestUnionInUnion, 1); > + tmp->type = TEST_UNION_ENUM_VALUE_B; > + tmp->u.value_b.integer = 1729; > + tmp->u.value_b.onoff = true; > + > + visit_type_TestUnionInUnion(data->ov, NULL, &tmp, &error_abort); > + qdict = qobject_to(QDict, visitor_get(data)); > + g_assert(qdict); > + g_assert_cmpstr(qdict_get_str(qdict, "type"), ==, "value-b"); > + g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 1729); > + g_assert_cmpint(qdict_get_bool(qdict, "onoff"), ==, true); > + > + qapi_free_TestUnionInUnion(tmp); > +} > + > static void test_visitor_out_alternate(TestOutputVisitorData *data, > const void *unused) > { > @@ -586,6 +642,8 @@ int main(int argc, char **argv) > &out_visitor_data, test_visitor_out_list_qapi_free); > output_visitor_test_add("/visitor/output/union-flat", > &out_visitor_data, test_visitor_out_union_flat); > + output_visitor_test_add("/visitor/output/union-in-union", > + &out_visitor_data, test_visitor_out_union_in_union); > output_visitor_test_add("/visitor/output/alternate", > &out_visitor_data, test_visitor_out_alternate); > output_visitor_test_add("/visitor/output/null", The changes look good to me. I have tried to built migrate QAPI changes on top of Daniel's v1 changes, and it was successful. Once Markus and other maintainers approves this patchset, I can post v4 version of 'migrate' qapi changes on top of this change. Looking forward for replies on this patchset :) Regards, Het Gala ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/3] qapi: allow unions to contain further unions 2023-02-23 13:40 ` [PATCH v2 3/3] qapi: allow unions to contain further unions Daniel P. Berrangé 2023-03-07 3:53 ` Het Gala @ 2023-03-17 15:48 ` Markus Armbruster 1 sibling, 0 replies; 13+ messages in thread From: Markus Armbruster @ 2023-03-17 15:48 UTC (permalink / raw) To: Daniel P. Berrangé; +Cc: qemu-devel, Eric Blake, Michael Roth, Het Gala Daniel P. Berrangé <berrange@redhat.com> writes: > This extends the QAPI schema validation to permit unions inside unions, > provided the checks for clashing fields pass. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > scripts/qapi/schema.py | 6 +- > tests/qapi-schema/meson.build | 2 + > tests/qapi-schema/qapi-schema-test.json | 32 ++++++++++ > tests/qapi-schema/qapi-schema-test.out | 29 ++++++++++ > .../union-invalid-union-subfield.err | 2 + > .../union-invalid-union-subfield.json | 30 ++++++++++ > .../union-invalid-union-subfield.out | 0 > .../union-invalid-union-subtype.err | 2 + > .../union-invalid-union-subtype.json | 29 ++++++++++ > .../union-invalid-union-subtype.out | 0 > tests/unit/test-qobject-input-visitor.c | 47 +++++++++++++++ > tests/unit/test-qobject-output-visitor.c | 58 +++++++++++++++++++ > 12 files changed, 234 insertions(+), 3 deletions(-) > create mode 100644 tests/qapi-schema/union-invalid-union-subfield.err > create mode 100644 tests/qapi-schema/union-invalid-union-subfield.json > create mode 100644 tests/qapi-schema/union-invalid-union-subfield.out > create mode 100644 tests/qapi-schema/union-invalid-union-subtype.err > create mode 100644 tests/qapi-schema/union-invalid-union-subtype.json > create mode 100644 tests/qapi-schema/union-invalid-union-subtype.out > > diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py > index 6c481ab0c0..5c4457f789 100644 > --- a/scripts/qapi/schema.py > +++ b/scripts/qapi/schema.py > @@ -465,9 +465,10 @@ def check(self, schema): > # on behalf of info, which is not necessarily self.info > def check_clash(self, info, seen): > assert self._checked > - assert not self.variants # not implemented > for m in self.members: > m.check_clash(info, seen) > + if self.variants: > + self.variants.check_clash(info, seen) > > def connect_doc(self, doc=None): > super().connect_doc(doc) > @@ -652,8 +653,7 @@ def check(self, schema, seen): > self.info, > "branch '%s' is not a value of %s" > % (v.name, self.tag_member.type.describe())) > - if (not isinstance(v.type, QAPISchemaObjectType) > - or v.type.variants): > + if not isinstance(v.type, QAPISchemaObjectType): > raise QAPISemError( > self.info, > "%s cannot use %s" I stared at the code some to convince myself this is complete. > diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build > index d85b14f28c..1591eb322b 100644 > --- a/tests/qapi-schema/meson.build > +++ b/tests/qapi-schema/meson.build > @@ -194,6 +194,8 @@ schemas = [ > 'union-invalid-data.json', > 'union-invalid-discriminator.json', > 'union-invalid-if-discriminator.json', > + 'union-invalid-union-subfield.json', > + 'union-invalid-union-subtype.json', > 'union-no-base.json', > 'union-optional-discriminator.json', > 'union-string-discriminator.json', > diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json > index ba7302f42b..40f1a3d88d 100644 > --- a/tests/qapi-schema/qapi-schema-test.json > +++ b/tests/qapi-schema/qapi-schema-test.json > @@ -114,6 +114,38 @@ > { 'struct': 'UserDefC', > 'data': { 'string1': 'str', 'string2': 'str' } } > > +# this tests that unions can contain other unions in their branches > +{ 'enum': 'TestUnionEnum', > + 'data': [ 'value-a', 'value-b' ] } > + > +{ 'enum': 'TestUnionEnumA', > + 'data': [ 'value-a1', 'value-a2' ] } > + > +{ 'struct': 'TestUnionTypeA1', > + 'data': { 'integer': 'int', > + 'name': 'str'} } > + > +{ 'struct': 'TestUnionTypeA2', > + 'data': { 'integer': 'int', > + 'size': 'int' } } > + > +{ 'union': 'TestUnionTypeA', > + 'base': { 'type-a': 'TestUnionEnumA' }, > + 'discriminator': 'type-a', > + 'data': { 'value-a1': 'TestUnionTypeA1', > + 'value-a2': 'TestUnionTypeA2' } } > + > +{ 'struct': 'TestUnionTypeB', > + 'data': { 'integer': 'int', > + 'onoff': 'bool' } } > + > +{ 'union': 'TestUnionInUnion', > + 'base': { 'type': 'TestUnionEnum' }, > + 'discriminator': 'type', > + 'data': { 'value-a': 'TestUnionTypeA', > + 'value-b': 'TestUnionTypeB' } } > + > + > # for testing use of 'number' within alternates > { 'alternate': 'AltEnumBool', 'data': { 'e': 'EnumOne', 'b': 'bool' } } > { 'alternate': 'AltEnumNum', 'data': { 'e': 'EnumOne', 'n': 'number' } } > diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out > index 043d75c655..9fe1038944 100644 > --- a/tests/qapi-schema/qapi-schema-test.out > +++ b/tests/qapi-schema/qapi-schema-test.out > @@ -105,6 +105,35 @@ alternate UserDefAlternate > object UserDefC > member string1: str optional=False > member string2: str optional=False > +enum TestUnionEnum > + member value-a > + member value-b > +enum TestUnionEnumA > + member value-a1 > + member value-a2 > +object TestUnionTypeA1 > + member integer: int optional=False > + member name: str optional=False > +object TestUnionTypeA2 > + member integer: int optional=False > + member size: int optional=False > +object q_obj_TestUnionTypeA-base > + member type-a: TestUnionEnumA optional=False > +object TestUnionTypeA > + base q_obj_TestUnionTypeA-base > + tag type-a > + case value-a1: TestUnionTypeA1 > + case value-a2: TestUnionTypeA2 > +object TestUnionTypeB > + member integer: int optional=False > + member onoff: bool optional=False > +object q_obj_TestUnionInUnion-base > + member type: TestUnionEnum optional=False > +object TestUnionInUnion > + base q_obj_TestUnionInUnion-base > + tag type > + case value-a: TestUnionTypeA > + case value-b: TestUnionTypeB > alternate AltEnumBool > tag type > case e: EnumOne Looks good to me. I also inspected the generated code; no complaints. > diff --git a/tests/qapi-schema/union-invalid-union-subfield.err b/tests/qapi-schema/union-invalid-union-subfield.err > new file mode 100644 > index 0000000000..43574dea79 > --- /dev/null > +++ b/tests/qapi-schema/union-invalid-union-subfield.err > @@ -0,0 +1,2 @@ > +union-invalid-union-subfield.json: In union 'TestUnion': > +union-invalid-union-subfield.json:25: member 'teeth' of type 'TestTypeFish' collides with base member 'teeth' of type 'TestUnion-base' Bad error message, see my review of PATCH 1. > diff --git a/tests/qapi-schema/union-invalid-union-subfield.json b/tests/qapi-schema/union-invalid-union-subfield.json > new file mode 100644 > index 0000000000..e1639d3a96 > --- /dev/null > +++ b/tests/qapi-schema/union-invalid-union-subfield.json > @@ -0,0 +1,30 @@ > +# Clash between common member and union variant's variant member > +# Base's member 'teeth' clashes with TestTypeFish's > + > +{ 'enum': 'TestEnum', > + 'data': [ 'animals', 'plants' ] } > + > +{ 'enum': 'TestAnimals', > + 'data': [ 'fish', 'birds'] } > + > +{ 'struct': 'TestTypeFish', > + 'data': { 'scales': 'int', 'teeth': 'int' } } > + > +{ 'struct': 'TestTypeBirds', > + 'data': { 'feathers': 'int' } } > + > +{ 'union': 'TestTypeAnimals', > + 'base': { 'atype': 'TestAnimals' }, > + 'discriminator': 'atype', > + 'data': { 'fish': 'TestTypeFish', > + 'birds': 'TestTypeBirds' } } > + > +{ 'struct': 'TestTypePlants', > + 'data': { 'integer': 'int' } } > + > +{ 'union': 'TestUnion', > + 'base': { 'type': 'TestEnum', > + 'teeth': 'int' }, > + 'discriminator': 'type', > + 'data': { 'animals': 'TestTypeAnimals', > + 'plants': 'TestTypePlants' } } > diff --git a/tests/qapi-schema/union-invalid-union-subfield.out b/tests/qapi-schema/union-invalid-union-subfield.out > new file mode 100644 > index 0000000000..e69de29bb2 > diff --git a/tests/qapi-schema/union-invalid-union-subtype.err b/tests/qapi-schema/union-invalid-union-subtype.err > new file mode 100644 > index 0000000000..e45f330cec > --- /dev/null > +++ b/tests/qapi-schema/union-invalid-union-subtype.err > @@ -0,0 +1,2 @@ > +union-invalid-union-subtype.json: In union 'TestUnion': > +union-invalid-union-subtype.json:25: base member 'type' of type 'TestTypeA-base' collides with base member 'type' of type 'TestUnion-base' Likewise. > diff --git a/tests/qapi-schema/union-invalid-union-subtype.json b/tests/qapi-schema/union-invalid-union-subtype.json > new file mode 100644 > index 0000000000..ce1de51d8d > --- /dev/null > +++ b/tests/qapi-schema/union-invalid-union-subtype.json > @@ -0,0 +1,29 @@ > +# Clash between common member and union variant's common member > +# Base's member 'type' clashes with TestTypeA's > + > +{ 'enum': 'TestEnum', > + 'data': [ 'value-a', 'value-b' ] } > + > +{ 'enum': 'TestEnumA', > + 'data': [ 'value-a1', 'value-a2' ] } > + > +{ 'struct': 'TestTypeA1', > + 'data': { 'integer': 'int' } } > + > +{ 'struct': 'TestTypeA2', > + 'data': { 'integer': 'int' } } > + > +{ 'union': 'TestTypeA', > + 'base': { 'type': 'TestEnumA' }, > + 'discriminator': 'type', > + 'data': { 'value-a1': 'TestTypeA1', > + 'value-a2': 'TestTypeA2' } } > + > +{ 'struct': 'TestTypeB', > + 'data': { 'integer': 'int' } } > + > +{ 'union': 'TestUnion', > + 'base': { 'type': 'TestEnum' }, > + 'discriminator': 'type', > + 'data': { 'value-a': 'TestTypeA', > + 'value-b': 'TestTypeB' } } > diff --git a/tests/qapi-schema/union-invalid-union-subtype.out b/tests/qapi-schema/union-invalid-union-subtype.out > new file mode 100644 > index 0000000000..e69de29bb2 > diff --git a/tests/unit/test-qobject-input-visitor.c b/tests/unit/test-qobject-input-visitor.c > index 77fbf985be..9b3e2dbe14 100644 > --- a/tests/unit/test-qobject-input-visitor.c > +++ b/tests/unit/test-qobject-input-visitor.c > @@ -706,6 +706,51 @@ static void test_visitor_in_union_flat(TestInputVisitorData *data, > g_assert(&base->enum1 == &tmp->enum1); > } > > +static void test_visitor_in_union_in_union(TestInputVisitorData *data, > + const void *unused) > +{ > + Visitor *v; > + g_autoptr(TestUnionInUnion) tmp = NULL; > + > + v = visitor_input_test_init(data, > + "{ 'type': 'value-a', " > + " 'type-a': 'value-a1', " > + " 'integer': 2, " > + " 'name': 'fish' }"); > + > + visit_type_TestUnionInUnion(v, NULL, &tmp, &error_abort); > + g_assert_cmpint(tmp->type, ==, TEST_UNION_ENUM_VALUE_A); > + g_assert_cmpint(tmp->u.value_a.type_a, ==, TEST_UNION_ENUMA_VALUE_A1); > + g_assert_cmpint(tmp->u.value_a.u.value_a1.integer, ==, 2); > + g_assert_cmpint(strcmp(tmp->u.value_a.u.value_a1.name, "fish"), ==, 0); > + > + qapi_free_TestUnionInUnion(tmp); > + > + v = visitor_input_test_init(data, > + "{ 'type': 'value-a', " > + " 'type-a': 'value-a2', " > + " 'integer': 1729, " > + " 'size': 87539319 }"); > + > + visit_type_TestUnionInUnion(v, NULL, &tmp, &error_abort); > + g_assert_cmpint(tmp->type, ==, TEST_UNION_ENUM_VALUE_A); > + g_assert_cmpint(tmp->u.value_a.type_a, ==, TEST_UNION_ENUMA_VALUE_A2); > + g_assert_cmpint(tmp->u.value_a.u.value_a2.integer, ==, 1729); > + g_assert_cmpint(tmp->u.value_a.u.value_a2.size, ==, 87539319); > + > + qapi_free_TestUnionInUnion(tmp); > + > + v = visitor_input_test_init(data, > + "{ 'type': 'value-b', " > + " 'integer': 1729, " > + " 'onoff': true }"); > + > + visit_type_TestUnionInUnion(v, NULL, &tmp, &error_abort); > + g_assert_cmpint(tmp->type, ==, TEST_UNION_ENUM_VALUE_B); > + g_assert_cmpint(tmp->u.value_b.integer, ==, 1729); > + g_assert_cmpint(tmp->u.value_b.onoff, ==, true); > +} > + > static void test_visitor_in_alternate(TestInputVisitorData *data, > const void *unused) > { > @@ -1216,6 +1261,8 @@ int main(int argc, char **argv) > NULL, test_visitor_in_null); > input_visitor_test_add("/visitor/input/union-flat", > NULL, test_visitor_in_union_flat); > + input_visitor_test_add("/visitor/input/union-in-union", > + NULL, test_visitor_in_union_in_union); > input_visitor_test_add("/visitor/input/alternate", > NULL, test_visitor_in_alternate); > input_visitor_test_add("/visitor/input/errors", > diff --git a/tests/unit/test-qobject-output-visitor.c b/tests/unit/test-qobject-output-visitor.c > index 7f054289fe..1535b3ad17 100644 > --- a/tests/unit/test-qobject-output-visitor.c > +++ b/tests/unit/test-qobject-output-visitor.c > @@ -352,6 +352,62 @@ static void test_visitor_out_union_flat(TestOutputVisitorData *data, > qapi_free_UserDefFlatUnion(tmp); > } > > +static void test_visitor_out_union_in_union(TestOutputVisitorData *data, > + const void *unused) > +{ > + QDict *qdict; > + > + TestUnionInUnion *tmp = g_new0(TestUnionInUnion, 1); > + tmp->type = TEST_UNION_ENUM_VALUE_A; > + tmp->u.value_a.type_a = TEST_UNION_ENUMA_VALUE_A1; > + tmp->u.value_a.u.value_a1.integer = 42; > + tmp->u.value_a.u.value_a1.name = g_strdup("fish"); > + > + visit_type_TestUnionInUnion(data->ov, NULL, &tmp, &error_abort); > + qdict = qobject_to(QDict, visitor_get(data)); > + g_assert(qdict); > + g_assert_cmpstr(qdict_get_str(qdict, "type"), ==, "value-a"); > + g_assert_cmpstr(qdict_get_str(qdict, "type-a"), ==, "value-a1"); > + g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 42); > + g_assert_cmpstr(qdict_get_str(qdict, "name"), ==, "fish"); > + > + qapi_free_TestUnionInUnion(tmp); > + > + > + visitor_reset(data); > + tmp = g_new0(TestUnionInUnion, 1); > + tmp->type = TEST_UNION_ENUM_VALUE_A; > + tmp->u.value_a.type_a = TEST_UNION_ENUMA_VALUE_A2; > + tmp->u.value_a.u.value_a2.integer = 1729; > + tmp->u.value_a.u.value_a2.size = 87539319; > + > + visit_type_TestUnionInUnion(data->ov, NULL, &tmp, &error_abort); > + qdict = qobject_to(QDict, visitor_get(data)); > + g_assert(qdict); > + g_assert_cmpstr(qdict_get_str(qdict, "type"), ==, "value-a"); > + g_assert_cmpstr(qdict_get_str(qdict, "type-a"), ==, "value-a2"); > + g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 1729); > + g_assert_cmpint(qdict_get_int(qdict, "size"), ==, 87539319); > + > + qapi_free_TestUnionInUnion(tmp); > + > + > + visitor_reset(data); > + tmp = g_new0(TestUnionInUnion, 1); > + tmp->type = TEST_UNION_ENUM_VALUE_B; > + tmp->u.value_b.integer = 1729; > + tmp->u.value_b.onoff = true; > + > + visit_type_TestUnionInUnion(data->ov, NULL, &tmp, &error_abort); > + qdict = qobject_to(QDict, visitor_get(data)); > + g_assert(qdict); > + g_assert_cmpstr(qdict_get_str(qdict, "type"), ==, "value-b"); > + g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 1729); > + g_assert_cmpint(qdict_get_bool(qdict, "onoff"), ==, true); > + > + qapi_free_TestUnionInUnion(tmp); > +} > + > static void test_visitor_out_alternate(TestOutputVisitorData *data, > const void *unused) > { > @@ -586,6 +642,8 @@ int main(int argc, char **argv) > &out_visitor_data, test_visitor_out_list_qapi_free); > output_visitor_test_add("/visitor/output/union-flat", > &out_visitor_data, test_visitor_out_union_flat); > + output_visitor_test_add("/visitor/output/union-in-union", > + &out_visitor_data, test_visitor_out_union_in_union); > output_visitor_test_add("/visitor/output/alternate", > &out_visitor_data, test_visitor_out_alternate); > output_visitor_test_add("/visitor/output/null", Reviewed-by: Markus Armbruster <armbru@redhat.com> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/3] qapi: allow unions to contain further unions 2023-02-23 13:40 [PATCH v2 0/3] qapi: allow unions to contain further unions Daniel P. Berrangé ` (2 preceding siblings ...) 2023-02-23 13:40 ` [PATCH v2 3/3] qapi: allow unions to contain further unions Daniel P. Berrangé @ 2023-03-17 15:55 ` Markus Armbruster 2023-03-31 11:49 ` Het Gala 3 siblings, 1 reply; 13+ messages in thread From: Markus Armbruster @ 2023-03-17 15:55 UTC (permalink / raw) To: Daniel P. Berrangé; +Cc: qemu-devel, Eric Blake, Michael Roth, Het Gala Daniel P. Berrangé <berrange@redhat.com> writes: > Currently it is not possible for a union type to contain a > further union as one (or more) of its branches. This relaxes > that restriction and adds the calls needed to validate field > name uniqueness as unions are flattened. I apologize for the long delay. Sick child, sick me, much snot, little sleep. PATCH 1 is wrong, but I was able to figure out what's going on there, and suggested a patch that hopefully works. PATCH 2 is okay. I suggested a few tweaks. I'd put it first, but that's up to you. PATCH 3 looks good. Looking forward to v3. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/3] qapi: allow unions to contain further unions 2023-03-17 15:55 ` [PATCH v2 0/3] " Markus Armbruster @ 2023-03-31 11:49 ` Het Gala 2023-04-14 6:32 ` Het Gala 0 siblings, 1 reply; 13+ messages in thread From: Het Gala @ 2023-03-31 11:49 UTC (permalink / raw) To: Markus Armbruster, Daniel P. Berrangé Cc: qemu-devel, Eric Blake, Michael Roth Hi all, On 17/03/23 9:25 pm, Markus Armbruster wrote: > Daniel P. Berrangé <berrange@redhat.com> writes: > >> Currently it is not possible for a union type to contain a >> further union as one (or more) of its branches. This relaxes >> that restriction and adds the calls needed to validate field >> name uniqueness as unions are flattened. > I apologize for the long delay. Sick child, sick me, much snot, little > sleep. > > PATCH 1 is wrong, but I was able to figure out what's going on there, > and suggested a patch that hopefully works. > > PATCH 2 is okay. I suggested a few tweaks. I'd put it first, but > that's up to you. > > PATCH 3 looks good. > > Looking forward to v3. Thankyou Markus for your suggestions and I hope everyone is in good health now. This is just a friendly reminder if Daniel is ready with v3 patches for the same :) Regards, Het Gala ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/3] qapi: allow unions to contain further unions 2023-03-31 11:49 ` Het Gala @ 2023-04-14 6:32 ` Het Gala 2023-04-25 13:29 ` Markus Armbruster 0 siblings, 1 reply; 13+ messages in thread From: Het Gala @ 2023-04-14 6:32 UTC (permalink / raw) To: Markus Armbruster, Daniel P. Berrangé Cc: qemu-devel, Eric Blake, Michael Roth On 31/03/23 5:19 pm, Het Gala wrote: > Hi all, > > On 17/03/23 9:25 pm, Markus Armbruster wrote: >> Daniel P. Berrangé <berrange@redhat.com> writes: >> >>> Currently it is not possible for a union type to contain a >>> further union as one (or more) of its branches. This relaxes >>> that restriction and adds the calls needed to validate field >>> name uniqueness as unions are flattened. >> I apologize for the long delay. Sick child, sick me, much snot, little >> sleep. >> >> PATCH 1 is wrong, but I was able to figure out what's going on there, >> and suggested a patch that hopefully works. >> >> PATCH 2 is okay. I suggested a few tweaks. I'd put it first, but >> that's up to you. >> >> PATCH 3 looks good. >> >> Looking forward to v3. > > Thankyou Markus for your suggestions and I hope everyone is in good > health now. This is just a friendly reminder if Daniel is ready with > v3 patches for the same :) > > Regards, > Het Gala Hi, this is just a reminder mail to check if Daniel has plan to post v3 patches in the coming days. Would like these patches to get merged in qemu as soon as possible, so that we all can focus on restructuring of 'migrate' QAPI :) Regards, Het Gala ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/3] qapi: allow unions to contain further unions 2023-04-14 6:32 ` Het Gala @ 2023-04-25 13:29 ` Markus Armbruster 0 siblings, 0 replies; 13+ messages in thread From: Markus Armbruster @ 2023-04-25 13:29 UTC (permalink / raw) To: Het Gala; +Cc: Daniel P. Berrangé, qemu-devel, Eric Blake, Michael Roth Het Gala <het.gala@nutanix.com> writes: > Hi, this is just a reminder mail to check if Daniel has plan to post v3 patches in the coming days. Would like these patches to get merged in qemu as soon as possible, so that we all can focus on restructuring of 'migrate' QAPI :) I just queued v3. Expect a pull request today or tomorrow. ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2023-04-25 13:29 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-02-23 13:40 [PATCH v2 0/3] qapi: allow unions to contain further unions Daniel P. Berrangé 2023-02-23 13:40 ` [PATCH v2 1/3] qapi: improve specificity of type/member descriptions Daniel P. Berrangé 2023-03-17 12:08 ` Markus Armbruster 2023-02-23 13:40 ` [PATCH v2 2/3] qapi: use env var to trigger qapi test output updates Daniel P. Berrangé 2023-02-24 19:28 ` Eric Blake 2023-03-17 12:05 ` Markus Armbruster 2023-02-23 13:40 ` [PATCH v2 3/3] qapi: allow unions to contain further unions Daniel P. Berrangé 2023-03-07 3:53 ` Het Gala 2023-03-17 15:48 ` Markus Armbruster 2023-03-17 15:55 ` [PATCH v2 0/3] " Markus Armbruster 2023-03-31 11:49 ` Het Gala 2023-04-14 6:32 ` Het Gala 2023-04-25 13:29 ` Markus Armbruster
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).