Re: [Qemu-devel] [RFC] QMP: add query-hotpluggable-cpus

[Eric Blake]

On 02/15/2016 10:10 AM, Igor Mammedov wrote:
> it will allow mgmt to query present and possible to hotplug CPUs
> it is required from a target platform that wish to support
> command to set board specific MachineClass.possible_cpus() hook,
> which will return a list of possible CPUs with options
> that would be needed for hotplugging possible CPUs.
> 
> For RFC there are:
>    'arch_id': 'int' - mandatory unique CPU number,
>                       for x86 it's APIC ID for ARM it's MPIDR
>    'type': 'str' - CPU object type for usage with device_add
> 
> and a set of optional fields that would allows mgmt tools
> to know at what granularity and where a new CPU could be
> hotplugged;
> [node],[socket],[core],[thread]
> Hopefully that should cover needs for CPU hotplug porposes for
> magor targets and we can extend structure in future adding
> more fields if it will be needed.
> 
> also for present CPUs there is a 'cpu_link' field which
> would allow mgmt inspect whatever object/abstraction
> the target platform considers as CPU object.
> 
> For RFC purposes implements only for x86 target so far.
> 
> Signed-off-by: Igor Mammedov <address@hidden>
> ---

Just an interface review for now:

> +++ b/qapi-schema.json
> @@ -4083,3 +4083,33 @@
>  ##
>  { 'enum': 'ReplayMode',
>    'data': [ 'none', 'record', 'play' ] }
> +
> +##
> +# @HotpluggableCPU
> +#
> +# @type: CPU object tyep for usage with device_add command

s/tyep/type/

> +# @arch_id: unique number designating the CPU within board

Please use '-' rather than '_' in new interfaces (this should be 'arch-id')

> +# @node: NUMA node ID the CPU belongs to, optional

Most optional fields are marked with a prefix of '#optional', not an
unmarked suffix. This will matter once we get to Marc-Andre's patches
for automated documentation.

> +# @socket: socket number within node/board the CPU belongs to, optional
> +# @core: core number within socket the CPU belongs to, optional
> +# @thread: thread number within core the CPU belongs to, optional
> +# @cpu_link: link to existing CPU object is CPU is present or

Again, 'cpu-link'.

> +#            omitted if CPU is not present.
> +#
> +# Since: 2.6

Missing '##' marker line.

> +{ 'struct': 'HotpluggableCPU',
> +  'data': { 'type': 'str',
> +            'arch_id': 'int',
> +            '*node': 'int',
> +            '*socket': 'int',
> +            '*core': 'int',
> +            '*thread': 'int',
> +            '*cpu_link': 'str'
> +          }
> +}
> +
> +##
> +# @query-hotpluggable-cpus
> +#
> +# Since: 2.6

Missing '##' terminator, and also lacking on details.

> +{ 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] }

Why do we need a new command?  Why can't the existing 'CpuInfo' be
expanded to provide the new information as part of the existing
'query-cpus'?

> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 020e5ee..cbe0ba4 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -4818,3 +4818,29 @@ Example:
>                   {"type": 0, "out-pport": 0, "pport": 0, "vlan-id": 3840,
>                    "pop-vlan": 1, "id": 251658240}
>     ]}
> +
> +EQMP
> +
> +    {
> +        .name       = "query-hotpluggable-cpus",
> +        .args_type  = "",
> +        .mhandler.cmd_new = qmp_marshal_query_hotpluggable_cpus,
> +    },
> +
> +SQMP
> +Show  existing/possible CPUs
> +-------------------------------

Why two spaces? --- separator line should be same length as the line above.

> +
> +Arguments: None.
> +
> +Example for x86 target started with -smp 
> 2,sockets=2,cores=1,threads=3,maxcpus=6:
> +
> +-> { "execute": "query-hotpluggable-cpus" }
> +<- {"return": [
> +     {"core": 0, "socket": 1, "thread": 2, "arch_id": 6, "type": 
> "qemu64-x86_64-cpu"},
> +     {"core": 0, "socket": 1, "thread": 1, "arch_id": 5, "type": 
> "qemu64-x86_64-cpu"},
> +     {"core": 0, "socket": 1, "thread": 0, "arch_id": 4, "type": 
> "qemu64-x86_64-cpu"},
> +     {"core": 0, "socket": 0, "thread": 2, "arch_id": 2, "type": 
> "qemu64-x86_64-cpu"},
> +     {"core": 0, "arch_id": 1, "socket": 0, "thread": 1, "type": 
> "qemu64-x86_64-cpu", "cpu_link": "/machine/unattached/device[3]"},
> +     {"core": 0, "arch_id": 0, "socket": 0, "thread": 0, "type": 
> "qemu64-x86_64-cpu", "cpu_link": "/machine/unattached/device[0]"}

Long line. Please wrap the example to fit in 80 columns (we've already
added stylistic whitespace beyond the single-line JSON output that we
really get from QMP).


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature