[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 07/10] qmp: add rocker device support
From: |
Scott Feldman |
Subject: |
Re: [Qemu-devel] [PATCH v5 07/10] qmp: add rocker device support |
Date: |
Wed, 4 Feb 2015 22:10:58 -0800 |
On Tue, Feb 3, 2015 at 7:10 AM, Eric Blake <address@hidden> wrote:
> On 01/22/2015 01:03 AM, address@hidden wrote:
>> From: Scott Feldman <address@hidden>
>>
>> Add QMP/HMP support for rocker devices. This is mostly for debugging
>> purposes
>> to see inside the device's tables and port configurations. Some examples:
>>
>
> QMP interface review:
>
>> +++ b/qapi-schema.json
>> @@ -3523,3 +3523,6 @@
>> # Since: 2.1
>> ##
>> { 'command': 'rtc-reset-reinjection' }
>> +
>> +# Rocker ethernet network switch
>> +{ 'include': 'qapi/rocker.json' }
>> diff --git a/qapi/rocker.json b/qapi/rocker.json
>> new file mode 100644
>> index 0000000..326c6c7
>> --- /dev/null
>> +++ b/qapi/rocker.json
>> @@ -0,0 +1,259 @@
>> +##
>> +# @Rocker:
>> +#
>> +# Rocker switch information.
>> +#
>> +# @name: switch name
>> +#
>> +# @id: switch ID
>> +#
>> +# @ports: number of front-panel ports
>> +##
>
> Missing a 'Since: 2.3' designation.
fixed all of these
>
>> +{ 'type': 'RockerSwitch',
>> + 'data': { 'name': 'str', 'id': 'uint64', 'ports': 'uint32' } }
>> +
>> +##
>> +# @rocker:
>> +#
>> +# Return rocker switch information.
>> +#
>> +# Returns: @Rocker information
>> +#
>> +# Since: 2.3
>> +##
>> +{ 'command': 'rocker',
>> + 'data': { 'name': 'str' },
>> + 'returns': 'RockerSwitch' }
>
> Should this command be named 'query-rocker', as it is used for queries?
added "query-" to all cmds.
> Should the 'name' argument be optional, and the output be an array (all
> rocker devices, rather than just a given rocker name lookup)?
we can add a query-rockers cmds later to get list of rocker names (or
list of RockerSwitch'es).
>
> Missing '#optional' tags on the various optional fields. Why are
> certain fields optional? Does it mean they have a default value, or
> that they don't make sense in some configurations? The docs could be
> more clear on that.
Added #optional for the optional fields and added comment for each
section that these are fields that may or may not be present depending
on context of object being returned.