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

Daniel P. Berrangé posted 3 patches 1 year, 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230223134027.2294640-1-berrange@redhat.com
Maintainers: Markus Armbruster <armbru@redhat.com>, Michael Roth <michael.roth@amd.com>
There is a newer version of this series
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
[PATCH v2 0/3] qapi: allow unions to contain further unions
Posted by Daniel P. Berrangé 1 year, 2 months ago
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


Re: [PATCH v2 0/3] qapi: allow unions to contain further unions
Posted by Markus Armbruster 1 year, 2 months ago
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.
Re: [PATCH v2 0/3] qapi: allow unions to contain further unions
Posted by Het Gala 1 year, 1 month ago
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

Re: [PATCH v2 0/3] qapi: allow unions to contain further unions
Posted by Het Gala 1 year, 1 month ago
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

Re: [PATCH v2 0/3] qapi: allow unions to contain further unions
Posted by Markus Armbruster 1 year ago
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.