[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 2/5] arm: qmp: add query-gic-capabilities int
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v5 2/5] arm: qmp: add query-gic-capabilities interface |
Date: |
Tue, 22 Mar 2016 20:09:03 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> On 03/17/2016 09:27 PM, Peter Xu wrote:
>> This patch adds the command "query-gic-capabilities" but not implemnet
>
> s/not implemnet/does not implement/
>
>> it. The command is ARM-only. Return of the command is a list of
>> GICCapability struct that describes all GIC versions that current QEMU
>> and system support.
>>
>> Signed-off-by: Peter Xu <address@hidden>
>> ---
>
>> +++ b/qapi-schema.json
>> @@ -4156,3 +4156,14 @@
>> 'data': { 'version': 'int',
>> 'emulated': 'bool',
>> 'kernel': 'bool' } }
>> +
>> +##
>> +# @query-gic-capabilities:
>> +#
>> +# Return a list of supported GIC version capabilities.
>> +#
>> +# Returns: a list of GICCapability.
>> +#
>> +# Since: 2.6
>> +##
>> +{ 'command': 'query-gic-capabilities', 'returns': ['GICCapability'] }
>
> On the surface, this seems okay. As mentioned before, I would have
> squashed 1 and 2 into a single patch. The GICCapability type is
> extensible, and introspection is sufficient at seeing what the type is
> currently capable of exposing.
>
> On the other hand...
>
>> +++ b/scripts/qapi.py
>> @@ -46,6 +46,7 @@ returns_whitelist = [
>> 'query-tpm-models',
>> 'query-tpm-types',
>> 'ringbuf-read',
>> + 'query-gic-capability',
>
> ...it required a whitelist, because you are violating the usual
> convention of returning a dict. If you DO need the whitelist, your
> addition should have been kept sorted. But you don't need it, if you
> would modify your QAPI to return a dict:
>
> { 'struct': 'GICCapabilitiesReturn',
> 'data': { 'capabilities': ['GICCapability'] } }
> { 'command': 'query-gic-capabilities',
> 'returns': 'GICCapabilitiesReturn' }
>
> Yes, the dict has only a single key, and that key points to the same
> list; but now you have future extensibility: in the future, we could
> return any future global data as a sibling to the array, without having
> to modify every element of the array to repeat redundant information.
I just tested it, the whitelist entry is superfluous, check_command()
accepts a list of dicts just fine.
- Re: [Qemu-devel] [PATCH v5 1/5] arm: qmp: add GICCapability struct, (continued)
Re: [Qemu-devel] [PATCH v5 1/5] arm: qmp: add GICCapability struct, Eric Blake, 2016/03/22
[Qemu-devel] [PATCH v5 2/5] arm: qmp: add query-gic-capabilities interface, Peter Xu, 2016/03/17
Re: [Qemu-devel] [PATCH v5 2/5] arm: qmp: add query-gic-capabilities interface, Eric Blake, 2016/03/22
[Qemu-devel] [PATCH v5 3/5] arm: enhance kvm_arm_create_scratch_host_vcpu, Peter Xu, 2016/03/17
[Qemu-devel] [PATCH v5 4/5] kvm: add kvm_support_device() helper function, Peter Xu, 2016/03/17
[Qemu-devel] [PATCH v5 5/5] arm: implement query-gic-capabilities, Peter Xu, 2016/03/17
Re: [Qemu-devel] [PATCH v5 0/5] ARM: add query-gic-capabilities SMP command, Andrea Bolognani, 2016/03/21
Re: [Qemu-devel] [PATCH v5 0/5] ARM: add query-gic-capabilities SMP command, Markus Armbruster, 2016/03/22
Re: [Qemu-devel] [PATCH v5 0/5] ARM: add query-gic-capabilities SMP command, Peter Maydell, 2016/03/22