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

[Osier Yang]

On 23/01/14 08:04, Eric Blake wrote:
> 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?
>

Good idea, pushed with the fix, along with a bit change in the commit log.
Thanks.

Osier