mbox series

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

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

Message

Daniel P. Berrangé Feb. 23, 2023, 1:40 p.m. UTC
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

Comments

Markus Armbruster March 17, 2023, 3:55 p.m. UTC | #1
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.
Het Gala March 31, 2023, 11:49 a.m. UTC | #2
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
Het Gala April 14, 2023, 6:32 a.m. UTC | #3
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
Markus Armbruster April 25, 2023, 1:29 p.m. UTC | #4
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.