Re: [PATCH v6 23/23] xl: vNUMA support

[Ian Campbell <ian.campbell@citrix.com>]

On Thu, 2015-02-26 at 15:56 +0000, Wei Liu wrote:
> This patch includes configuration options parser and documentation.
> 
> Please find the hunk to xl.cfg.pod.5 for more information.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> ---
> Changes in v6:
> 1. Disable NUMA auto-placement.
> ---
>  docs/man/xl.cfg.pod.5    |  54 ++++++++++++++++++
>  tools/libxl/xl_cmdimpl.c | 140 ++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 193 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index 408653f..2a27b1c 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -266,6 +266,60 @@ it will crash.
>  
>  =back
>  
> +=head3 Guest Virtual NUMA Configuration
> +
> +=over 4
> +
> +=item B<vnuma=[ VNODE_SPEC, VNODE_SPEC, ... ]
> +
> +Specify virtual NUMA configuration with positional arguments. The
> +nth B<VNODE_SPECE> in the list specifies the configuration of nth

                    ^stray E?

> +virtual node.
> +
> +Each B<VNODE_SPEC> is a list, which has a form of
> +"[VNODE_CONFIG_OPTION,VNODE_CONFIG_OPTION, ... ]"  (without quotes).

BTW, near the top of this file is:
        A list of C<VALUES> of the above types. Lists are homogeneous and are
        not nested.

"are not nested" is no longer true.

> +
> +For example vnuma = [ ["pnode=0","size=512","vcpus=0-4","vdistances=10,20"] ]
> +means vnode 0 is mapped to pnode 0, has 512MB ram, has vcpus 0 to 4, the
> +distance to itself is 10 and the distance to vnode 1 is 20.
> +
> +Each B<VNODE_CONFIG_OPTION> is a quoted string.

s/string/key=value pair/?

>  Supported
> +B<VNODE_CONFIG_OPTION>s are:
> +
> +=over 4
> +
> +=item B<pnode=NUMBER>
> +
> +Specify which physical node this virtual node maps to.
> +
> +=item B<size=MBYTES>
> +
> +Specify the size of this virtual node. The sum of memory size of all
> +vnodes must match B<maxmem=> (or B<memory=> if B<maxmem=> is not
> +specified).

Didn't you suggest in a reply v4 that this need for the user to do
tedious book keeping could go away after some other change: "This point
becomes moot if we use other syntax to specify vcpu list."?

Should it be allowable to no specify maxmem and have xl do the sums over
the vnodes  to come up with the correct value? (memory= could still be
given in that case).

The code looks fine.