Re: [Qemu-devel] [PATCH v1] docs: add cpu-hotplug.txt

[Fam Zheng <famz@redhat.com>]

Hello Liyang, Thanks for the contribution!

On Tue, 08/09 14:48, Dou Liyang wrote:
> This document describes how to use cpu hotplug in QEMU.
> 
> Change log v1:
>   From Igor's advice:
>     1. Remove any mentioning of apic-id from the document.
>     2. Remove the "device_del qom_path" from the CPU hot-unplug.
>     3. Fix some comment.

In the future, please put the revision log after the "---" line below your
signed-off-by line, so that it doesn't get committed to the git history (this
information is not useful there) once get merged.

I'm not a native English speaker either, but I'll try to give some suggestions
on the documentation below. Please don't mind, and take with a grain of salt.  :)

> 
> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
> ---
>  docs/cpu-hotplug.txt | 124 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 124 insertions(+)
>  create mode 100644 docs/cpu-hotplug.txt
> 
> diff --git a/docs/cpu-hotplug.txt b/docs/cpu-hotplug.txt
> new file mode 100644
> index 0000000..2c51e27
> --- /dev/null
> +++ b/docs/cpu-hotplug.txt
> @@ -0,0 +1,124 @@
> +QEMU CPU hotplug
> +===================

I think a prettier way to write markdown style docs is to let the underlines be
as long as the text, like:

QEMU CPU hotplug
================

The same applies to following section titles as well.

> +
> +This document explains how to use the CPU hotplug feature in QEMU,
> +which is present since v2.7.

This isn't quite accurate, cpu-add has been available since 1.5 according to
http://wiki.qemu.org/Features/CPUHotplug, I guess you are talking about
something different. So you probably want to revise the sentence to clarify.

> +
> +Guest support is required for CPU hotplug to work.
> +
> +CPU hot-plug
> +-------------------
> +
> +In order to be able to hotplug CPUs, QEMU has to be told what is the

s/what is//, or put "is" after "grow".

> +maximum amount of CPUs the guest can grow. This is done at startup
> +time by means of the -smp command-line option, which has the following
> +format:
> +
> + -smp [cpus=]n[,maxcpus=cpus][,cores=cores][,threads=threads]
> +	[,sockets=sockets]
> +
> +Where,
> +
> + - "cpus" set the number of CPUs to 'n' [default=1]

Not very consistent in the syntax used in this list. There is a verb in "cpus"
but none in the others. I suggest adding a verb to each item.

> + - "maxcpus" maximum number of CPUs, including offline VCPUs for
> +	hotplug, etc
> + - "cores" number of CPU cores on one socket
> + - "threads= number of threads on one CPU core
> + - "sockets= number of discrete sockets in the system(on sPAPR,

s/system(on/system (on)/

> +	sockets have no real meaning, it has no real effect on the
> +	guest.)
> +
> +
> +For example, the following command-line:
> +
> + qemu [...] -smp 3,maxcpus=10,sockets=2,cores=2,threads=2
> +
> +Creates a guest with 3 VCPUs and it support up to 10 VCPUs. The CPU

s/support/supports/

> +topology is sockets (2) * cores (2) * threads (2) and can't greater

s/greater/be greater/

> +than maxcpus. When the guest is just booted, the guest will see 3

s/just//

> +VCPUs.
> +
> +There are seven VCPUs can be hotplugged by socket/core/thread-ids

s/There are seven/Seven more/

> +which should be obtained from the properties advertised by QEMU via

s/should be obtained/can be observed/

> +the QMP command query-hotpluggable-cpus or the corresponding HMP
> +command "info hotpluggable-cpus".

This paragraph seems to be repeating what is following. Maybe be more brief and
say "more on this below"?

> +
> +Query possible available CPU objects
> +--------------------------------------
> +
> +Before add the VCPUs, we should know the topology properties of the

s/add/adding/

> +possible available CPUs objects, so that we can find out the available
> +place (socket,core,thread) for a new VCPU.
> +
> +A monitor commands are used to list the possible CPU objects:

s/commands are used/command can be used/

> +
> +  (qemu) info hotpluggable-cpus
> +
> +Select the hotpluggable CPUs including "CPUInstance Properties" for
> +hotpluging. Such as this:
> +
> +  ...
> +  type: "qemu64-x86_64-cpu"
> +  vcpus_count: "1"
> +  CPUInstance Properties:
> +    socket-id: "0"
> +    core-id: "1"
> +    thread-id: "0"
> +  ...
> +
> +Hotplug CPUs
> +----------------------
> +
> +A monitor commands are used to hotplug CPUs:

s/commands are used/command can be used/

> +
> + - "device_add": creates a VCPU device and inserts it into the
> +	specific place as a device
> +
> +For example, the following commands add a VCPU which id is cpu1 in

s/commands add/command adds/
s/which id is/which has id/

> +the last position of the guest's CPU sockets which was discussed
> +earlier by using the socket/core/thread-ids:
> +
> +  (qemu) device_add qemu64-x86_64-cpu,id=cpu9,socket-id=2,core-id=0,
> +	thread-id=1
> +
> +Where,
> +
> + - "qemu64-x86_64-cpu" is the CPU modle.

s/modle/model/

> + - "id" is the unique identifier in the device sets. there must have
> +	it.

s/there must have it/It is required./

> + - "socket-id/core-id/thread-id" is represented the designated position
> + which are gained form the above possible list of CPUs.

s/is represented/represent/
s/are gained/is obtained/


> +
> +It's also possible to start a guest with cpu cold-plugged into the
> +specific place (socket,core,thread).
> +
> +In the following command-lines example, a guest which has 3 VCPUs is
> +created:
> +
> + qemu  [...] -smp 1,maxcpus=10,sockets=2,cores=2,threads=2 \
> +	-device qemu64-x86_64-cpu,id=cpu1,socket-id=1, \
> +	core-id=1,thread-id=0 \
> +	-device qemu64-x86_64-cpu,id=cpu2,socket-id=1, \
> +	core-id=1,thread-id=1 \
> +
> +Two VCPUs are cold-plugged by "-device" parameter. They are in the
> +same socket and core with different thread-id. After that, the guest

s/thread-id/thread-ids/

> +has additional seven VCPUs to be hot-plugged when needed.
> +
> +CPU hot-unplug
> +------------------------
> +
> +In order to be able to hot unplug cpu device, QEMU has the way
> +to remove cpu device.
> +
> +  Using the ids which were assigned when you hot plugged cpus.

Why the indentation for this line?

> +
> +A monitor commands are used to hot unplug cpus:

s/commands are used/command can be used/

> +
> + - "device_del": deletes a cpu device
> +
> +For example, assuming that the cpu device with id "cpu1" exists,
> +the following commands tries to remove it.

s/commands/command/

> +
> +  (qemu) device_del cpu1
> +

Can you also document what should be done in guest to make this operation
successful?

Fam