[libvirt] [PATCH] util: Correct the NUMA node range checking

[Eric Blake]

On 01/22/2014 04:45 AM, Osier Yang wrote:
> There are 2 issues here: First we shouldn't add "1" to the return
> value of numa_max_node(), since the semanteme of the error message

s/semanteme/semantics/

> was changed, it's not saying about the number of total NUMA nodes
> anymore.  Second, the value of "bit" is the position of the first
> bit which exceeds either numa_max_node() or NUMA_NUM_NODES, it can
> be any number in the range, so saying "bigger than $bit" is quite
> confused now. For example, assuming there is a NUMA machine which
> has 10 NUMA nodes, and one specifies the "nodeset" as "0,5,88",
> the error message will be like:
> 
> Nodeset is out of range, host cannot support NUMA node bigger than 88
> 
> It sounds like all NUMA node number less than 88 is fine, but
> actually the maximum NUMA node number the machine supports is 9.
> 
> This patch fixes the issues by removing the addition with "1" and
> simplifies the error message as "NUMA node $bit is out of range".
> ---
>  src/util/virnuma.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)

> -    maxnode = numa_max_node() + 1;
> -
>      /* Convert nodemask to NUMA bitmask. */
>      nodemask_zero(&mask);
>      bit = -1;
>      while ((bit = virBitmapNextSetBit(tmp_nodemask, bit)) >= 0) {
> -        if (bit > maxnode || bit > NUMA_NUM_NODES) {
> +        if (bit > numa_max_node() || bit > NUMA_NUM_NODES) {

This calls numa_max_node() in a loop, where we used to call it just
once.  Since this is third-party code, do we know how efficient it is?
It may be smarter to cache the results once than to call every iteration
of the loop, to avoid O(n*2) behavior on large hosts.

For that matter, can't we just set the max node to the smaller of
numa_max_node() and NUMA_NUM_NODES up front, and avoid the || in the loop?

ACK if you can fix that up.

>              virReportError(VIR_ERR_INTERNAL_ERROR,
> -                           _("Nodeset is out of range, host cannot support "
> -                             "NUMA node bigger than %d"), bit);
> +                           _("NUMA node %d is out of range"), bit);
>              return -1;
>          }
>          nodemask_set(&mask, bit);
> 

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140122/1e466dff/attachment-0001.sig>