Re: [Qemu-devel] [PATCH v4] Allow setting NUMA distance for different NUMA nodes

[He Chen <he.chen@linux.intel.com>]

On Mon, Apr 03, 2017 at 01:58:34PM -0500, Eric Blake wrote:
> On 04/03/2017 03:38 AM, Andrew Jones wrote:
> > On Sat, Apr 01, 2017 at 06:25:26PM +0800, He Chen wrote:
> >> Current, QEMU does not provide a clear command to set vNUMA distance for
> >> guest although we already have `-numa` command to set vNUMA nodes.
> >>
> 
> >> +++ b/qapi-schema.json
> >> @@ -5644,10 +5644,14 @@
> >>  ##
> >>  # @NumaOptionsType:
> >>  #
> >> +# @node: NUMA nodes configuration
> >> +#
> >> +# @dist: NUMA distance configuration
> >> +#
> 
> Missing a '(since 2.10)' tag on @dist.
> 
> 
> >>  ##
> >> +# @NumaDistOptions:
> >> +#
> >> +# Set the distance between 2 NUMA nodes.
> >> +#
> >> +# @src: source NUMA node.
> >> +#
> >> +# @dst: destination NUMA node.
> >> +#
> >> +# @val: NUMA distance from source node to destination node.
> 
> Missing mention of the magic of 255.
> 
> >> +#
> >> +# Since: 2.10
> >> +##
> >> +{ 'struct': 'NumaDistOptions',
> >> +  'data': {
> >> +   'src': 'uint16',
> >> +   'dst': 'uint16',
> >> +   'val': 'uint8' }}
> >> +
> 
> Is a user allowed to pass { "src":0, "dst":0, "val":11 }, or do you
> hard-validate that the diagonal of the matrix is always 10 (the user can
> omit it, but if specified it must be 10).  Do you enforce that no
> distance is less than 10?
> 

I see your concern, I think it's reasonable that a user is allowed to
set distance 11 to local node but qemu refuses and complains about it.

If the distance of local is omitted, qemu can fill it, but if specified,
I think it must be 10.

In current version patch, the distance is enforced to be >= 10, this
check is done in numa_distance_parser.
Anyway, I will imporve it in next patch, thanks for your review.

> >> +@var{source} and @var{destination} are NUMA node IDs.
> >> +@var{distance} is the NUMA distance from @var{source} to @var{destination}.
> >> +The distance from node A to node B may be different from the distance from
> >> +node B to node A as the distance can to be asymmetrical. If a node is
> >> +unreachable, set 255 as distance.
> > 
> > The distance from a node to itself is always 10.  If no distance values
> > are given for node pairs, then the default distance of 20 is used for each
> > pair.  If any pair of nodes is given a distance, then all pairs must be
> > given distances.  Although, when distances are only given in one direction
> > for each pair of nodes, then the distances in the opposite directions are
> > assumed to be the same.  If, however, an asymmetrical pair of distances is
> > given for even one node pair, then all node pairs must be provided
> > distance values for both directions, even when they are symmetrical.  When
> > a node is unreachable from another node, set the pair's distance to 255.
> 
> I don't like duplication where it is not necessary, but don't know if
> there's an easy way to make the .json file and qemu-options.hx refer to
> one another, since they both feed separate user-visible documentation.
> So you may have to repeat some of this in the .json file (such as my
> mention above that at least documenting that 255 is special).
> 

I am afraid that I am not an expert in qapi, I do see some parts in
these two files are duplicate and I am not sure I can deal with it well,
if you agree, I may probably just document magic 255 in next patch.

Thanks,
-He