[libvirt] [PATCH] virNumaSetPagePoolSize: Produce friendlier error message

[Michal Privoznik]

On 09.06.2015 08:42, Martin Kletzander wrote:
> On Mon, Jun 08, 2015 at 02:41:05PM +0200, Michal Privoznik wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1224587
>>
>> The function takes two important arguments (among many others): @node
>> and @page_size. From these two a path under /sys is constructed. The
>> path is then used to read and write the desired size of huge pages
>> pool. However, if the path does not exists due to either @node or
>> @page_size having nonexistent value (e.g. there's no such NUMA node or
>> no page size like -2), an cryptic error message is produced:
>>
>>  virsh # allocpages --pagesize 2049 --pagecount 8 --cellno -2
>>  error: Failed to open file
>> '/sys/devices/system/node/node-2/hugepages/hugepages-2049kB/nr_hugepages':
>> No such file or directory
>>
>> Add two more checks to catch this and therefore produce much more
>> friendlier error messages.
>>
>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>> ---
>> src/util/virnuma.c | 18 ++++++++++++++++++
>> 1 file changed, 18 insertions(+)
>>
>> diff --git a/src/util/virnuma.c b/src/util/virnuma.c
>> index 669192a..5807d8f 100644
>> --- a/src/util/virnuma.c
>> +++ b/src/util/virnuma.c
>> @@ -849,9 +849,27 @@ virNumaSetPagePoolSize(int node,
>>         goto cleanup;
>>     }
>>
>> +    if (node != -1 && !virNumaNodeIsAvailable(node)) {
>> +        virReportError(VIR_ERR_OPERATION_FAILED,
>> +                       _("NUMA node %d is not available"),
>> +                       node);
>> +        goto cleanup;
>> +    }
>> +
>>     if (virNumaGetHugePageInfoPath(&nr_path, node, page_size,
>> "nr_hugepages") < 0)
>>         goto cleanup;
>>
>> +    if (!virFileExists(nr_path)) {
>> +        /* Strictly speaking, @nr_path contains both NUMA node and
>> page size.
>> +         * So if it doesn't exist it can be due to any of those two
>> is wrong.
>> +         * However, the existence of the node was checked a few lines
>> above, so
>> +         * it can be only page size here. */
> 
> Über-strictly speaking, unless you compile with both WITH_NUMACTL &&
> HAVE_NUMA_BITMASK_ISBITSET then virNumaNodeIsAvailable() can pass for
> invalid node in case of non-contiguous NUMA node numbers.

So what are you saying is that I should update the comment or the error
message or leave everything as-is and push it?

Michal