[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 18/28] qapi: Unify type bypass and add tests
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v5 18/28] qapi: Unify type bypass and add tests |
Date: |
Fri, 27 Mar 2015 13:40:42 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
Eric Blake <address@hidden> writes:
> On 03/26/2015 11:38 AM, Markus Armbruster wrote:
>> Eric Blake <address@hidden> writes:
>>
>>> For a few QMP commands, we are forced to pass an arbitrary type
>>> without tracking it properly in QAPI. Among the existing clients,
>>> this unnamed type was spelled 'dict', 'visitor', and '**'; this
>>> patch standardizes on '**'.
>>>
>>> Meanwhile, for both 'gen' and 'success-response' keys, we have been
>>> ignoring the value, although the schema consistently used "'no'".
>>> But now that we can support a literal "false" in the schema, we
>>> might as well use that rather than ignoring the value or
>>> special-casing a random string.
>>>
>>> There is no difference to the generated code. As these features
>>> were previously undocumented before this series, add some tests
>>> and documentation on what we'd like to guarantee, although it will
>>> take later patches to clean up test results and actually enforce
>>> the use of a bool parameter.
>>
>> You don't actually add documentation in this patch.
>
> Hmm, more evidence that I waffled about per-commit doc fixes, vs.
> lumping it all in patch 1, and I obviously failed to scrub the commit
> messages after changing my mind.
Fortunately, commit messages don't need testing, so this is easy enough
to fix in a respin :)
>> Aside: 'gen': false is required when '**' is used anywhere in the
>> command. If it was permitted only then, it would be redundant. I think
>> we happily accept 'gen': false without '**' so far, although we don't
>> use it. That's okay.
>
> Also, even though the code accepts 'gen':false, it rejects 'gen':true
> ('gen' is only a one-way switch away from the default). Also something
> I didn't think worth worrying about.
Agree.
>>> Signed-off-by: Eric Blake <address@hidden>
>>
>> Reviewed-by: Markus Armbruster <address@hidden>
>>
>>
- Re: [Qemu-devel] [PATCH v5 12/28] qapi: Introduce 'alternate' to replace anonymous union, (continued)
[Qemu-devel] [PATCH v5 18/28] qapi: Unify type bypass and add tests, Eric Blake, 2015/03/24
[Qemu-devel] [PATCH v5 11/28] qapi: Rename anonymous union type in test, Eric Blake, 2015/03/24
[Qemu-devel] [PATCH v5 17/28] qapi: Allow true, false and null in schema json, Eric Blake, 2015/03/24
[Qemu-devel] [PATCH v5 05/28] qapi: Better error messages for bad enums, Eric Blake, 2015/03/24
[Qemu-devel] [PATCH v5 15/28] qapi: Add tests of redefined expressions, Eric Blake, 2015/03/24