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

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.