Re: [Qemu-arm] [Qemu-devel] [PATCH v5 2/5] arm: qmp: add query-gic-capabilities interface

[Markus Armbruster <armbru@redhat.com>]

Peter Xu <peterx@redhat.com> writes:

> On Tue, Mar 22, 2016 at 07:28:13PM +0100, Markus Armbruster wrote:
>> Peter Xu <peterx@redhat.com> writes:
>> > diff --git a/scripts/qapi.py b/scripts/qapi.py
>> > index 6b2aa6e..716474e 100644
>> > --- a/scripts/qapi.py
>> > +++ b/scripts/qapi.py
>> > @@ -46,6 +46,7 @@ returns_whitelist = [
>>    # Whitelist of commands allowed to return a non-dictionary
>>    returns_whitelist = [
>>        'human-monitor-command',
>>        'qom-get',
>>        'query-migrate-cache-size',
>> >      'query-tpm-models',
>> >      'query-tpm-types',
>> >      'ringbuf-read',
>> > +    'query-gic-capability',
>> >  
>> >      # From QGA:
>> >      'guest-file-open',
>> 
>> The whitelist exists to except existing commands from design rules on
>> return types.  New commands don't get to violate the rules without a
>> really, really compelling reason.
>> 
>> Do you actually need this?

I've since tested it and double-checked the code: you don't need this.

>> If yes, why should your command be permitted to violate the design
>> rules?
>
> This might not be required. Agree with you and Eric. Will use a hash
> instead with single key.

The rule against returning non-dictionaries exists to avoid interfaces
that cannot evolve.  With a dictionary, you can evolve by adding
members.

The rule does *not* forbid returning lists of dictionaries.  When a
command fundamentally returns a list of things, being able to evolve the
things suffices.

query-gic-capabilities looks like it fundamentally returns a list of
capabilities.  Returning ['GICCapability'] is just fine then.

Only if query-gic-capabilities doesn't have this list nature should you
complicate its return value by wrapping it in another object type.

In either case, drop the change to returns_whitelist.

[...]