Re: [Qemu-devel] [PATCH 16/16] add cpu-add qmp command and implement CPU hot-add for target-i386

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1461 bytes --]

On 04/15/2013 04:12 PM, Igor Mammedov wrote:
> ... via current_machine->cpu_hot_add() hook called by cpu-set QMP command,
> for x86 target.
> 
> cpu-add's "id" argument is a CPU thread number in a range [0..max-cpus - 1)

Off by one.  It's either [0..max-cpus) or [0..max-cpus - 1] (they mean
the same thing).

It might be worth including a sample QMP command in the commit message.

> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> v5:
>   * accept id=[0..max_cpus) range in cpu-add command

This notation is right, unlike the commit message.

Reviewing just the QMP portion:

> +++ b/qapi-schema.json
> @@ -1387,6 +1387,17 @@
>  { 'command': 'cpu', 'data': {'index': 'int'} }
>  
>  ##
> +# @cpu-add
> +#
> +# Adds CPU with specified id
> +#
> +# @id: cpu id of CPU to be created

Here it would be helpful to mention what forms a valid id (your
[0..max-cpus) notation from the commit message, for example).

> +#
> +# Returns: Nothing on success
> +##
> +{ 'command': 'cpu-add', 'data': {'id': 'int'} }
> +

Should be usable from libvirt's perspective, even if hot-plugging more
than one cpu requires more than one QMP call.  Do we have a counterpart
QMP call to easily determine which cpu ids can still be hotplugged?  If
so, should we mention that in the documentation of this command?

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]