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(a)redhat.com&gt; --- 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. */

+ virReportError(VIR_ERR_OPERATION_FAILED, + _("page size %u not available"), + page_size); + goto cleanup; + } + /* Firstly check, if there's anything for us to do */ if (virFileReadAll(nr_path, 1024, &nr_buf) < 0) goto cleanup; -- 2.3.6 -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

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(a)redhat.com&gt; >> --- >> 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?