Re: [Qemu-devel] [PATCH v4 12/19] qapi: Add some type check tests

[Markus Armbruster]

Markus Armbruster <address@hidden> writes:

> Eric Blake <address@hidden> writes:
>
>> Demonstrate that the qapi generator silently parses confusing
>> types, which may cause other errors later on. Later patches
>> will update the expected results as the generator is made stricter.
[...]
>> diff --git a/tests/qapi-schema/returns-array-bad.json
>> b/tests/qapi-schema/returns-array-bad.json
>> new file mode 100644
>> index 0000000..14882c1
>> --- /dev/null
>> +++ b/tests/qapi-schema/returns-array-bad.json
>> @@ -0,0 +1,2 @@
>> +# FIXME: we should reject an array return that is not a single type
>> +{ 'command': 'oops', 'returns': [ 'str', 'str' ] }

Yes, we want this test, and your remaining tests of 'returns' are fine,
too.

> Do we want to permit anything but a complex type for 'returns'?
>
> For what it's worth, docs/qmp/qmp-spec.txt specifies:
>
>     2.4.1 success
>     -------------
>
>     The format of a success response is:
>
>     { "return": json-object, "id": json-value }
>
>      Where,
>
>     - The "return" member contains the command returned data, which is defined
>       in a per-command basis or an empty json-object if the command does not
>       return data
>     - The "id" member contains the transaction identification associated
>       with the command execution if issued by the Client
>
> The QAPI schema's 'returns' becomes "return" on the wire.  We suck.
>
> qmp-spec.txt is *wrong*!  We actually use json-array in addition to
> json-object.

Actually, we use json-int and json-str as well:
query-migrate-cache-size, ringbuf-read, human-monitor-command.

> Similar argument on types wanted as for 'data' / "arguments" above.  I
> think we should permit exactly the same QAPI types, plus lists.

The similarity to 'data' just isn't there.  Separate analysis needed.

Any QAPI types that don't make sense, other than list with length != 1?

[...]