Re: [Qemu-devel] [RFC PATCH v0 7/8] qmp: Implement query cpu-packages

[Igor Mammedov]

On Mon, 22 Feb 2016 09:49:40 -0700
Eric Blake <address@hidden> wrote:

> On 02/21/2016 10:01 PM, Bharata B Rao wrote:
> > Signed-off-by: Bharata B Rao <address@hidden>
> > ---
> >  hw/cpu/package.c    | 19 +++++++++++++
> >  hw/ppc/spapr.c      | 79 
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  include/hw/boards.h |  1 +
> >  qapi-schema.json    | 48 ++++++++++++++++++++++++++++++++
> >  4 files changed, 147 insertions(+)
> >   
> 
> > +++ b/qapi-schema.json
> > @@ -4083,3 +4083,51 @@
> >  ##
> >  { 'enum': 'ReplayMode',
> >    'data': [ 'none', 'record', 'play' ] }
> > +
> > +##
> > +# @CPUThreadInfo:
> > +#
> > +# Information about CPU Threads
> > +#  
> 
> Missing documentation for the fields.
> 
> > +# Since: 2.6
> > +##
> > +
> > +{ 'struct': 'CPUInfo',
> > +  'data': { 'arch_id': 'int',  
> 
> New QMP code should favor '-' over '_'; this should be 'arch-id'.
> 
> > +            'type': 'str',  
> 
> Is this string free-form, or is it a finite set of values?  If the
> latter, then it should be an enum type.
I'd say it's a limited set of values. The same applies to other
targets. It's basically cpu_model translated into corresponding
QOM type name.
Is there a way to auto-generate this QAPI enum dynamically?

> 
> > +            '*thread': 'int',
> > +            '*core': 'int',
> > +            '*socket' : 'int',
> > +            '*node' : 'int',
> > +            '*qom_path': 'str'  
> 
> 'qom-path'. But this one is definitely free-form, so 'str' is right.
> 
> > +          }
> > +}
> > +
> > +##
> > +# @CPUPackageInfo:
> > +#
> > +# Information about CPU Packages
> > +#  
> 
> Missing field documentation.
> 
> > +# Since: 2.6
> > +##
> > +
> > +{ 'struct': 'CPUPackageInfo',
> > +  'data': { '*id': 'str',
> > +            'type': 'str',
> > +            'qom_path': 'str',
> > +            'realized': 'bool',
> > +            'nr_cpus': 'int',  
> 
> 'nr-cpus'. Is this field redundant, given that the caller can just count
> the length of the 'cpus' array?
> 
> > +            'cpus' : ['CPUInfo']
> > +          }
> > +}
> > +
> > +##
> > +# @query-cpu-packages:
> > +#
> > +# Returns information for all CPU packages
> > +#
> > +# Returns: a list of @CPUPackageInfo
> > +#
> > +# Since: 2.6
> > +##
> > +{ 'command': 'query-cpu-packages', 'returns': ['CPUPackageInfo'] }
> >   
>