On Tue, Feb 3, 2015 at 7:10 AM, Eric Blake <ebl...@redhat.com> wrote:
> On 01/22/2015 01:03 AM, sfel...@gmail.com wrote:
>> From: Scott Feldman <sfel...@gmail.com>
>>
>> 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.

Reply via email to