Re: [Qemu-devel] [PATCH RFC v2 14/47] qapi-tests: New tests for union, alternate command arguments

[Eric Blake]

On 07/01/2015 02:22 PM, Markus Armbruster wrote:
> A command's 'data' must be a struct type, given either as a
> dictionary, or as struct type name.
> 
> Existing test case data-int.json covers simple type 'int'.  Add test
> cases for type names referring to union and alternate types.

We could probably relax things to allow a union (which is always a
dictionary on the wire), but I agree that allowing an alternate type is
not appropriate (the goal here is that we require a dictionary).  But
it's also easier to be conservative now and relax later.

> 
> The latter is caught (good), but the former is not (bug).
> 
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
>  tests/Makefile                        | 3 ++-
>  tests/qapi-schema/args-alternate.err  | 1 +
>  tests/qapi-schema/args-alternate.exit | 1 +
>  tests/qapi-schema/args-alternate.json | 4 ++++
>  tests/qapi-schema/args-alternate.out  | 0
>  tests/qapi-schema/args-union.err      | 0
>  tests/qapi-schema/args-union.exit     | 1 +
>  tests/qapi-schema/args-union.json     | 4 ++++
>  tests/qapi-schema/args-union.out      | 4 ++++
>  9 files changed, 17 insertions(+), 1 deletion(-)

> +++ b/tests/qapi-schema/args-alternate.json
> @@ -0,0 +1,4 @@
> +# we do not allow alternate arguments
> +# TODO should we support this?

I see no reason to allow a non-dictionary in QMP, so this TODO could be
dropped.

> +++ b/tests/qapi-schema/args-union.json
> @@ -0,0 +1,4 @@
> +# FIXME we should reject union arguments
> +# TODO should we support this?
> +{ 'union': 'Uni', 'data': { 'case1': 'int', 'case2': 'str' } }
> +{ 'command': 'oops', 'data': 'Uni' }

This, on the other hand, seems valid from the wire format (it will
always be a dictionary).  I guess the problem is that we generate a C
function signature based by calling out each member of the dictionary -
but how do you do that for a union?  So I see what you are doing:
marking that this test currently passes the parser, but then causes
problems for generating C code, so we should either reject it up front,
or fix the generator.  The FIXME documents what you will do later in the
series (reject it up front) and the TODO documents what we can do down
the road (fix the generator to allow it).

Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature