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@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