On 07/21/2015 06:43 AM, Eric Blake wrote: > 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. >
>> +++ 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.
Or, to be clear, we document that arguments is always a dictionary, for:
{ "execute":"command", "arguments":{} }. Allowing an alternate would
break that, so it is a different level of change to allow an alternate
(change the QMP protocol) than what it would be to allow a union (the
QMP protocol is unchanged). Not that we can't do it if we ever have a
reason, it's just that I don't see it being worth a TODO statement.
>
>> +++ 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).
See also 32/47 - events have the same problem.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
