Re: [Qemu-devel] [PATCH 7/7] monitor: QMP/HMP support for retrieving VNVRAM details

[Corey Bryant <coreyb@linux.vnet.ibm.com>]

On 05/23/2013 01:59 PM, Eric Blake wrote:
> On 05/23/2013 11:44 AM, Corey Bryant wrote:
>> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
>> ---
>
> Might help to list a sample HMP or QMP usage in the commit message.
>
>> +++ b/qapi-schema.json
>> @@ -3619,3 +3619,50 @@
>>               '*cpuid-input-ecx': 'int',
>>               'cpuid-register': 'X86CPURegister32',
>>               'features': 'int' } }
>> +
>> +# @VNVRAMEntryInfo:
>> +#
>> +# Information about an entry in the VNVRAM.
>> +#
>> +# @name: name of the entry
>> +#
>> +# @cur-size: current size of the entry's blob in bytes
>> +#
>> +# @max-size: max size of the entry's blob in bytes
>> +#
>> +# Since: 1.6
>> +#
>> +##
>> +{ 'type': 'VNVRAMEntryInfo',
>> +  'data': {'name': 'str', 'cur-size': 'int', 'max-size': 'int', } }
>
> No trailing commas in JSON.  :(
>

I'll fix that.

>> +
>> +##
>> +# @VNVRAMInfo:
>> +#
>> +# Information about the VNVRAM device.
>> +#
>> +# @drive-id: ID of the VNVRAM (and associated drive)
>> +#
>> +# @virtual-disk-size: Virtual size of the associated disk drive in bytes
>> +#
>> +# @vnvram-size: Size of the VNVRAM in bytes
>> +#
>> +# @entries: Array of @VNVRAMEntryInfo
>> +#
>> +# Since: 1.6
>> +#
>> +##
>> +{ 'type': 'VNVRAMInfo',
>> +  'data': {'drive-id': 'str', 'virtual-disk-size': 'int',
>> +           'vnvram-size': 'int', 'entries' : ['VNVRAMEntryInfo']} }
>> +
>> +##
>> +# @query-vnvram:
>> +#
>> +# Return information about the VNVRAM devices.
>> +#
>> +# Returns: @VNVRAMInfo on success
>> +#
>> +# Since: 1.6
>> +##
>> +{ 'command': 'query-vnvram', 'returns': ['VNVRAMInfo'] }
>
> Other than that, this looks fine from an interface point of view.  I
> haven't closely reviewed code, though.
>
>> +
>> +Example:
>> +
>> +-> { "execute": "query-vnvram" }
>> +<- {"return": [
>> +      { "vnvram-size": 2050, "virtual-disk-size": 2000896,
>> +        "drive-id": "drive-ide0-0-0",
>> +        "entries": [
>> +         { "name": "this-entry", "cur-size": 2048, "max-size": 21504 },
>> +         { "name": "that-entry", "cur-size": 1024, "max-size": 21504 },
>> +         { "name": "other-entry", "cur-size": 4096, "max-size": 41472 } ]
>> +      } ]
>> +   }
>
> Looks reasonable.
>

Thanks for the review!

-- 
Regards,
Corey Bryant