Re: [Qemu-devel] [PATCH V5 11/12] NUMA: add qmp command query-numa

[Wanlong Gao]

On 07/17/2013 08:41 PM, Eric Blake wrote:
> On 07/17/2013 03:29 AM, Wanlong Gao wrote:
>> Add qmp command query-numa to show guest NUMA information.
>>
>> Signed-off-by: Wanlong Gao <address@hidden>
>> ---
>>  numa.c           | 78 
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  qapi-schema.json | 34 ++++++++++++++++++++++++
>>  qmp-commands.hx  | 49 +++++++++++++++++++++++++++++++++++
>>  3 files changed, 161 insertions(+)
> 
> Focusing on just the interface...
> 
>> +++ b/qapi-schema.json
>> @@ -3748,3 +3748,37 @@
>>  { 'command': 'set-mem-policy',
>>    'data': {'nodeid': 'int', '*policy': 'str',
>>             '*host-nodes': 'str'} }
>> +##
>> +# @NUMAInfo:
>> +#
>> +# Information about guest NUMA nodes
>> +#
>> +# @nodeid: NUMA node ID
>> +#
>> +# @cpus: VCPUs contained to this node
> 
> s/to/in/

OK.

> 
>> +#
>> +# @memory: memory size of this node
> 
> In what unit? Preferably bytes, please (although your example listed
> 512, which is awfully small).  HMP can round to nearest k or M, but QMP
> should be precise.
> 
>> +#
>> +# @policy: memory policy of this node
>> +#
>> +# @relative: if host nodes is relative for memory policy
> 
> s/is/are/

OK.

> 
>> +#
>> +# @host-nodes: host nodes for its memory policy
>> +#
>> +# Since: 1.6
>> +#
>> +##
>> +{ 'type': 'NUMAInfo',
>> +  'data': {'nodeid': 'int', 'cpus': ['int'], 'memory': 'int',
>> +           'policy': 'str', 'relative': 'bool', 'host-nodes': ['int'] }}
> 
> 'policy' needs to be an 'enum' type (the same one as I have requested
> you to use in your other patches).

Yeah, got it.

> 
>> +
>> +##
>> +# @query-numa:
>> +#
>> +# Returns a list of information about each guest node.
>> +#
>> +# Returns: a list of @NUMAInfo for each guest node
>> +#
>> +# Since: 1.6
>> +##
>> +{ 'command': 'query-numa', 'returns': ['NUMAInfo'] }
> 
> Seems like a reasonable command.  As in 1/12, you may end up using
> 'Since: 1.7' by the time this patch is actually taken.

Yeah, got it. Thank you very much for your review.

Wanlong Gao

>