qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v6 30/36] qapi: Use 'struct' instead of 'type' i


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH v6 30/36] qapi: Use 'struct' instead of 'type' in schema
Date: Tue, 28 Apr 2015 14:23:46 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux)

Eric Blake <address@hidden> writes:

> Referring to "type" as both a meta-type (built-in, enum, union,
> alternte, or struct) and a specific type (the name that the
> schema uses for declaring structs) is confusing.  Finish up the
> conversion to using "struct" in qapi schema by coverting all
> users, then remove the hack in the generator that allowed 'type'.
> Tweak a couple of tests whose output changes slightly due to
> longer lines.  The json files were changed with:
>
> for f in `find -name '*.json'; do sed -i "s/'type'/'struct'/"; done
>
> followed by manually filtering out the places where we have a
> 'type' embedded in 'data'.  I also verified that the generated
> files for QMP and QGA (such as qmp-commands.h) are the same
> before and after, as assurance that I didn't leave in any
> accidental member name changes.

Sounds safe enough.  Could miss comments, though.  See possible misses
inline.

> Signed-off-by: Eric Blake <address@hidden>
[...]
> diff --git a/tests/qapi-schema/flat-union-bad-base.json 
> b/tests/qapi-schema/flat-union-bad-base.json
> index bb0f02d..3f9c8fc 100644
> --- a/tests/qapi-schema/flat-union-bad-base.json
> +++ b/tests/qapi-schema/flat-union-bad-base.json
> @@ -2,9 +2,9 @@
   # we require the base to be an existing complex type

Should this be 'an existing struct type'?

>  # TODO: should we allow an anonymous inline base type?
>  { 'enum': 'TestEnum',
>    'data': [ 'value1', 'value2' ] }
> -{ 'type': 'TestTypeA',
> +{ 'struct': 'TestTypeA',
>    'data': { 'string': 'str' } }
> -{ 'type': 'TestTypeB',
> +{ 'struct': 'TestTypeB',
>    'data': { 'integer': 'int' } }
>  { 'union': 'TestUnion',
>    'base': { 'enum1': 'TestEnum', 'kind': 'str' },
> diff --git a/tests/qapi-schema/flat-union-bad-discriminator.json 
> b/tests/qapi-schema/flat-union-bad-discriminator.json
> index 982f072..cd10b9d 100644
> --- a/tests/qapi-schema/flat-union-bad-discriminator.json
> +++ b/tests/qapi-schema/flat-union-bad-discriminator.json
> @@ -2,11 +2,11 @@
>  # this tests the old syntax for anonymous unions before we added alternates
>  { 'enum': 'TestEnum',
>    'data': [ 'value1', 'value2' ] }
> -{ 'type': 'TestBase',
> +{ 'struct': 'TestBase',
>    'data': { 'enum1': 'TestEnum', 'kind': 'str' } }
> -{ 'type': 'TestTypeA',
> +{ 'struct': 'TestTypeA',
>    'data': { 'string': 'str' } }
> -{ 'type': 'TestTypeB',
> +{ 'struct': 'TestTypeB',
>    'data': { 'integer': 'int' } }
>  { 'union': 'TestUnion',
>    'base': 'TestBase',
> diff --git a/tests/qapi-schema/flat-union-base-star.json 
> b/tests/qapi-schema/flat-union-base-star.json
> index 994533a..ed9dbf1 100644
> --- a/tests/qapi-schema/flat-union-base-star.json
> +++ b/tests/qapi-schema/flat-union-base-star.json
> @@ -1,9 +1,9 @@
>  # we require the base to be an existing complex type

Likewise.

>  { 'enum': 'TestEnum',
>    'data': [ 'value1', 'value2' ] }
> -{ 'type': 'TestTypeA',
> +{ 'struct': 'TestTypeA',
>    'data': { 'string': 'str' } }
> -{ 'type': 'TestTypeB',
> +{ 'struct': 'TestTypeB',
>    'data': { 'integer': 'int' } }
>  { 'union': 'TestUnion',
>    'base': '**',
> diff --git a/tests/qapi-schema/flat-union-base-union.json 
> b/tests/qapi-schema/flat-union-base-union.json
> index 0ba6e28..6a8ea68 100644
> --- a/tests/qapi-schema/flat-union-base-union.json
> +++ b/tests/qapi-schema/flat-union-base-union.json
> @@ -1,9 +1,9 @@
>  # we require the base to be a struct
>  { 'enum': 'TestEnum',
>    'data': [ 'value1', 'value2' ] }
> -{ 'type': 'TestTypeA',
> +{ 'struct': 'TestTypeA',
>    'data': { 'string': 'str' } }
> -{ 'type': 'TestTypeB',
> +{ 'struct': 'TestTypeB',
>    'data': { 'integer': 'int' } }
>  { 'union': 'UnionBase',
>    'data': { 'kind1': 'TestTypeA',
> diff --git a/tests/qapi-schema/flat-union-branch-clash.json 
> b/tests/qapi-schema/flat-union-branch-clash.json
> index 4091477..8b0b807 100644
> --- a/tests/qapi-schema/flat-union-branch-clash.json
> +++ b/tests/qapi-schema/flat-union-branch-clash.json
> @@ -1,11 +1,11 @@
>  # FIXME: we should check for no duplicate keys between branches and base
>  { 'enum': 'TestEnum',
>    'data': [ 'value1', 'value2' ] }
> -{ 'type': 'Base',
> +{ 'struct': 'Base',
>    'data': { 'enum1': 'TestEnum', 'name': 'str' } }
> -{ 'type': 'Branch1',
> +{ 'struct': 'Branch1',
>    'data': { 'name': 'str' } }
> -{ 'type': 'Branch2',
> +{ 'struct': 'Branch2',
>    'data': { 'value': 'int' } }
>  { 'union': 'TestUnion',
>    'base': 'Base',
> diff --git a/tests/qapi-schema/flat-union-inline.json 
> b/tests/qapi-schema/flat-union-inline.json
> index f3da117..d95618b 100644
> --- a/tests/qapi-schema/flat-union-inline.json
> +++ b/tests/qapi-schema/flat-union-inline.json
> @@ -2,7 +2,7 @@
   # we require branches to be a complex type name

Should this be 'a struct type name'?

>  # TODO: should we allow anonymous inline types?
>  { 'enum': 'TestEnum',
>    'data': [ 'value1', 'value2' ] }
> -{ 'type': 'Base',
> +{ 'struct': 'Base',
>    'data': { 'enum1': 'TestEnum', 'kind': 'str' } }
>  { 'union': 'TestUnion',
>    'base': { 'enum1': 'TestEnum', 'kind': 'str' },
> diff --git a/tests/qapi-schema/flat-union-int-branch.json 
> b/tests/qapi-schema/flat-union-int-branch.json
> index d373131..108faef 100644
> --- a/tests/qapi-schema/flat-union-int-branch.json
> +++ b/tests/qapi-schema/flat-union-int-branch.json
> @@ -1,9 +1,9 @@
>  # we require flat union branches to be a complex type

Likewise.

>  { 'enum': 'TestEnum',
>    'data': [ 'value1', 'value2' ] }
> -{ 'type': 'Base',
> +{ 'struct': 'Base',
>    'data': { 'enum1': 'TestEnum' } }
> -{ 'type': 'TestTypeB',
> +{ 'struct': 'TestTypeB',
>    'data': { 'integer': 'int' } }
>  { 'union': 'TestUnion',
>    'base': 'Base',
> diff --git a/tests/qapi-schema/flat-union-invalid-branch-key.json 
> b/tests/qapi-schema/flat-union-invalid-branch-key.json
> index a624282..95ff774 100644
> --- a/tests/qapi-schema/flat-union-invalid-branch-key.json
> +++ b/tests/qapi-schema/flat-union-invalid-branch-key.json
> @@ -1,13 +1,13 @@
>  { 'enum': 'TestEnum',
>    'data': [ 'value1', 'value2' ] }
>
> -{ 'type': 'TestBase',
> +{ 'struct': 'TestBase',
>    'data': { 'enum1': 'TestEnum' } }
>
> -{ 'type': 'TestTypeA',
> +{ 'struct': 'TestTypeA',
>    'data': { 'string': 'str' } }
>
> -{ 'type': 'TestTypeB',
> +{ 'struct': 'TestTypeB',
>    'data': { 'integer': 'int' } }
>
>  { 'union': 'TestUnion',
> diff --git a/tests/qapi-schema/flat-union-invalid-discriminator.json 
> b/tests/qapi-schema/flat-union-invalid-discriminator.json
> index 887157e..48b94c3 100644
> --- a/tests/qapi-schema/flat-union-invalid-discriminator.json
> +++ b/tests/qapi-schema/flat-union-invalid-discriminator.json
> @@ -1,13 +1,13 @@
>  { 'enum': 'TestEnum',
>    'data': [ 'value1', 'value2' ] }
>
> -{ 'type': 'TestBase',
> +{ 'struct': 'TestBase',
>    'data': { 'enum1': 'TestEnum' } }
>
> -{ 'type': 'TestTypeA',
> +{ 'struct': 'TestTypeA',
>    'data': { 'string': 'str' } }
>
> -{ 'type': 'TestTypeB',
> +{ 'struct': 'TestTypeB',
>    'data': { 'integer': 'int' } }
>
>  { 'union': 'TestUnion',
> diff --git a/tests/qapi-schema/flat-union-no-base.json 
> b/tests/qapi-schema/flat-union-no-base.json
> index 9547bb8..ffc4c6f 100644
> --- a/tests/qapi-schema/flat-union-no-base.json
> +++ b/tests/qapi-schema/flat-union-no-base.json
> @@ -1,8 +1,8 @@
>  # flat unions require a base
>  # TODO: simple unions should be able to use an enum discriminator
> -{ 'type': 'TestTypeA',
> +{ 'struct': 'TestTypeA',
>    'data': { 'string': 'str' } }
> -{ 'type': 'TestTypeB',
> +{ 'struct': 'TestTypeB',
>    'data': { 'integer': 'int' } }
>  { 'enum': 'Enum',
>    'data': [ 'value1', 'value2' ] }
> diff --git a/tests/qapi-schema/flat-union-optional-discriminator.json 
> b/tests/qapi-schema/flat-union-optional-discriminator.json
> index 25ce0e6..08a8f7e 100644
> --- a/tests/qapi-schema/flat-union-optional-discriminator.json
> +++ b/tests/qapi-schema/flat-union-optional-discriminator.json
> @@ -1,8 +1,8 @@
>  # we require the discriminator to be non-optional
>  { 'enum': 'Enum', 'data': [ 'one', 'two' ] }
> -{ 'type': 'Base',
> +{ 'struct': 'Base',
>    'data': { '*switch': 'Enum' } }
> -{ 'type': 'Branch', 'data': { 'name': 'str' } }
> +{ 'struct': 'Branch', 'data': { 'name': 'str' } }
>  { 'union': 'MyUnion',
>    'base': 'Base',
>    'discriminator': '*switch',
> diff --git a/tests/qapi-schema/flat-union-reverse-define.json 
> b/tests/qapi-schema/flat-union-reverse-define.json
> index 9ea7e72..648bbfe 100644
> --- a/tests/qapi-schema/flat-union-reverse-define.json
> +++ b/tests/qapi-schema/flat-union-reverse-define.json
> @@ -4,14 +4,14 @@
>    'data': { 'value1': 'TestTypeA',
>              'value2': 'TestTypeB' } }
>
> -{ 'type': 'TestBase',
> +{ 'struct': 'TestBase',
>    'data': { 'enum1': 'TestEnum' } }
>
>  { 'enum': 'TestEnum',
>    'data': [ 'value1', 'value2' ] }
>
> -{ 'type': 'TestTypeA',
> +{ 'struct': 'TestTypeA',
>    'data': { 'string': 'str' } }
>
> -{ 'type': 'TestTypeB',
> +{ 'struct': 'TestTypeB',
>    'data': { 'integer': 'int' } }
> diff --git a/tests/qapi-schema/flat-union-string-discriminator.json 
> b/tests/qapi-schema/flat-union-string-discriminator.json
> index e966aeb..8af6033 100644
> --- a/tests/qapi-schema/flat-union-string-discriminator.json
> +++ b/tests/qapi-schema/flat-union-string-discriminator.json
> @@ -1,13 +1,13 @@
>  { 'enum': 'TestEnum',
>    'data': [ 'value1', 'value2' ] }
>
> -{ 'type': 'TestBase',
> +{ 'struct': 'TestBase',
>    'data': { 'enum1': 'TestEnum', 'kind': 'str' } }
>
> -{ 'type': 'TestTypeA',
> +{ 'struct': 'TestTypeA',
>    'data': { 'string': 'str' } }
>
> -{ 'type': 'TestTypeB',
> +{ 'struct': 'TestTypeB',
>    'data': { 'integer': 'int' } }
>
>  { 'union': 'TestUnion',
> diff --git a/tests/qapi-schema/qapi-schema-test.json 
> b/tests/qapi-schema/qapi-schema-test.json
> index dec8a7c..f10efe2 100644
> --- a/tests/qapi-schema/qapi-schema-test.json
> +++ b/tests/qapi-schema/qapi-schema-test.json
> @@ -3,40 +3,40 @@
>  # for testing enums
>  { 'enum': 'EnumOne',
>    'data': [ 'value1', 'value2', 'value3' ] }
> -{ 'type': 'NestedEnumsOne',
> +{ 'struct': 'NestedEnumsOne',
>    'data': { 'enum1': 'EnumOne', '*enum2': 'EnumOne', 'enum3': 'EnumOne', 
> '*enum4': 'EnumOne' } }
>
>  # for testing nested structs
> -{ 'type': 'UserDefZero',
> +{ 'struct': 'UserDefZero',
>    'data': { 'integer': 'int' } }
>
> -{ 'type': 'UserDefOne',
> +{ 'struct': 'UserDefOne',
>    'base': 'UserDefZero',
>    'data': { 'string': 'str', '*enum1': 'EnumOne' } }
>
> -{ 'type': 'UserDefTwo',
> +{ 'struct': 'UserDefTwo',
>    'data': { 'string': 'str',
>              'dict': { 'string': 'str',
>                        'dict': { 'userdef': 'UserDefOne', 'string': 'str' },
>                        '*dict2': { 'userdef': 'UserDefOne', 'string': 'str' } 
> } } }
>
> -{ 'type': 'UserDefNested',
> +{ 'struct': 'UserDefNested',
>    'data': { 'string0': 'str',
>              'dict1': { 'string1': 'str',
>                         'dict2': { 'userdef1': 'UserDefOne', 'string2': 'str' 
> },
>                         '*dict3': { 'userdef2': 'UserDefOne', 'string3': 
> 'str' } } } }
>
>  # for testing unions
> -{ 'type': 'UserDefA',
> +{ 'struct': 'UserDefA',
>    'data': { 'boolean': 'bool' } }
>
> -{ 'type': 'UserDefB',
> +{ 'struct': 'UserDefB',
>    'data': { 'integer': 'int' } }
>
> -{ 'type': 'UserDefC',
> +{ 'struct': 'UserDefC',
>    'data': { 'string1': 'str', 'string2': 'str' } }
>
> -{ 'type': 'UserDefUnionBase',
> +{ 'struct': 'UserDefUnionBase',
>    'data': { 'string': 'str', 'enum1': 'EnumOne' } }
>
>  { 'union': 'UserDefFlatUnion',
> @@ -88,7 +88,7 @@
>  #
>  # For simplicity, this example doesn't use [type=]discriminator nor optargs
>  # specific to discriminator values.
> -{ 'type': 'UserDefOptions',
> +{ 'struct': 'UserDefOptions',
>    'data': {
>      '*i64' : [ 'int'    ],
>      '*u64' : [ 'uint64' ],
> @@ -97,7 +97,7 @@
>      '*u64x':   'uint64'  } }
>
>  # testing event
> -{ 'type': 'EventStructOne',
> +{ 'struct': 'EventStructOne',
>    'data': { 'struct1': 'UserDefOne', 'string': 'str', '*enum2': 'EnumOne' } }
>
>  { 'event': 'EVENT_A' }
> diff --git a/tests/qapi-schema/redefined-builtin.json 
> b/tests/qapi-schema/redefined-builtin.json
> index df328cc..45b8a55 100644
> --- a/tests/qapi-schema/redefined-builtin.json
> +++ b/tests/qapi-schema/redefined-builtin.json
> @@ -1,2 +1,2 @@
>  # we reject types that duplicate builtin names
> -{ 'type': 'size', 'data': { 'myint': 'size' } }
> +{ 'struct': 'size', 'data': { 'myint': 'size' } }
> diff --git a/tests/qapi-schema/redefined-type.json 
> b/tests/qapi-schema/redefined-type.json
> index e6a5f24..a09e768 100644
> --- a/tests/qapi-schema/redefined-type.json
> +++ b/tests/qapi-schema/redefined-type.json
> @@ -1,3 +1,3 @@
>  # we reject types defined more than once
> -{ 'type': 'foo', 'data': { 'one': 'str' } }
> +{ 'struct': 'foo', 'data': { 'one': 'str' } }
>  { 'enum': 'foo', 'data': [ 'two' ] }
> diff --git a/tests/qapi-schema/union-bad-branch.json 
> b/tests/qapi-schema/union-bad-branch.json
> index 4303666..913aa38 100644
> --- a/tests/qapi-schema/union-bad-branch.json
> +++ b/tests/qapi-schema/union-bad-branch.json
> @@ -1,7 +1,7 @@
>  # we reject normal unions where branches would collide in C
> -{ 'type': 'One',
> +{ 'struct': 'One',
>    'data': { 'string': 'str' } }
> -{ 'type': 'Two',
> +{ 'struct': 'Two',
>    'data': { 'number': 'int' } }
>  { 'union': 'MyUnion',
>    'data': { 'one': 'One',
> diff --git a/tests/qapi-schema/union-base-no-discriminator.json 
> b/tests/qapi-schema/union-base-no-discriminator.json
> index 052596c..1409cf5 100644
> --- a/tests/qapi-schema/union-base-no-discriminator.json
> +++ b/tests/qapi-schema/union-base-no-discriminator.json
> @@ -1,11 +1,11 @@
>  # we reject simple unions with a base (or flat unions without discriminator)
> -{ 'type': 'TestTypeA',
> +{ 'struct': 'TestTypeA',
>    'data': { 'string': 'str' } }
>
> -{ 'type': 'TestTypeB',
> +{ 'struct': 'TestTypeB',
>    'data': { 'integer': 'int' } }
>
> -{ 'type': 'Base',
> +{ 'struct': 'Base',
>    'data': { 'string': 'str' } }
>
>  { 'union': 'TestUnion',
> diff --git a/tests/qapi-schema/union-invalid-base.json 
> b/tests/qapi-schema/union-invalid-base.json
> index bc5dc8d..92be39d 100644
> --- a/tests/qapi-schema/union-invalid-base.json
> +++ b/tests/qapi-schema/union-invalid-base.json
> @@ -1,8 +1,8 @@
>  # a union base type must be a struct
> -{ 'type': 'TestTypeA',
> +{ 'struct': 'TestTypeA',
>    'data': { 'string': 'str' } }
>
> -{ 'type': 'TestTypeB',
> +{ 'struct': 'TestTypeB',
>    'data': { 'integer': 'int' } }
>
>  { 'union': 'TestUnion',
> diff --git a/tests/qapi-schema/unknown-expr-key.json 
> b/tests/qapi-schema/unknown-expr-key.json
> index ba7bdf3..3b2be00 100644
> --- a/tests/qapi-schema/unknown-expr-key.json
> +++ b/tests/qapi-schema/unknown-expr-key.json
> @@ -1,2 +1,2 @@
>  # we reject an expression with unknown top-level keys
> -{ 'type': 'bar', 'data': { 'string': 'str'}, 'bogus': { } }
> +{ 'struct': 'bar', 'data': { 'string': 'str'}, 'bogus': { } }

Reviewed-by: Markus Armbruster <address@hidden>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]