11:12 a.m.
On 10/02/2015 11:54 PM, John Ferlan wrote:
On 09/30/2015 12:01 AM, Luyao Huang wrote:
> Commit 1c24cfe9 fix the problem in virNumaSetPagePoolSize,
> this patch just like it and fix the issue in another function.
>
> when user use freepages and specify a invalid node or page size,
> will get error like this:
>
> # virsh freepages 0 1
> error: Failed to open file
'/sys/devices/system/node/node0/hugepages/hugepages-1kB/free_hugepages': No such
file or directory
>
> Add two checks to catch this and therefore produce much more
> friendlier error messages.
>
> Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
> ---
> src/util/virnuma.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/src/util/virnuma.c b/src/util/virnuma.c
> index 8577bd8..c8beade 100644
> --- a/src/util/virnuma.c
> +++ b/src/util/virnuma.c
> @@ -560,6 +560,12 @@ virNumaGetHugePageInfo(int node,
> page_size, "nr_hugepages") <
0)
> goto cleanup;
>
> + if (!virFileExists(path)) {
> + virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> + _("page size or NUMA node not available"));
duh - meant to add:
as perhaps a "patch 2" of the series (making this patch 3) - would it be
perhaps better to indicate "page size of "%u" or NUMA node "%d"
not
available", using page_size, node as the arguments?
??
Good idea, and how about make the code like this:
diff --git a/src/util/virnuma.c b/src/util/virnuma.c
index cb80972..8d85dc3 100644
--- a/src/util/virnuma.c
+++ b/src/util/virnuma.c
@@ -553,6 +553,19 @@ virNumaGetHugePageInfo(int node,
page_size, "nr_hugepages") < 0)
goto cleanup;
+ if (!virFileExists(path)) {
+ if (node != -1 && !virNumaNodeIsAvailable(node)) {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("NUMA node %d is not available"),
+ node);
+ } else {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("page size %u is not available"),
+ page_size);
+ }
+ goto cleanup;
+ }
+
if (virFileReadAll(path, 1024, &buf) < 0)
goto cleanup;
@@ -572,6 +585,19 @@ virNumaGetHugePageInfo(int node,
page_size, "free_hugepages") < 0)
goto cleanup;
+ if (!virFileExists(path)) {
+ if (node != -1 && !virNumaNodeIsAvailable(node)) {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("NUMA node %d is not available"),
+ node);
+ } else {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("page size %u is not available"),
+ page_size);
+ }
+ goto cleanup;
+ }
+
if (virFileReadAll(path, 1024, &buf) < 0)
goto cleanup;
@@ -836,19 +862,19 @@ 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)) {
- virReportError(VIR_ERR_OPERATION_FAILED, "%s",
- _("page size or NUMA node not available"));
+ if (!virFileExists(path)) {
+ if (node != -1 && !virNumaNodeIsAvailable(node)) {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("NUMA node %d is not available"),
+ node);
+ } else {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("page size %u is not available"),
+ page_size);
+ }
goto cleanup;
}
We need check if node == -1 to avoid output error include "NUMA node
"-1" ", and instead of check the nodeset again and again, i think just
check them when we cannot find the file (!virFileExists(nr_path)),
check the return from virNumaNodeIsAvailable() to output different error.
Thanks in advance for your reply and help.
Luyao
John
> + goto cleanup;
> + }
> +
> if (virFileReadAll(path, 1024, &buf) < 0)
> goto cleanup;
>
> @@ -579,6 +585,12 @@ virNumaGetHugePageInfo(int node,
> page_size, "free_hugepages") <
0)
> goto cleanup;
>
> + if (!virFileExists(path)) {
> + virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> + _("page size or NUMA node not available"));
> + goto cleanup;
> + }
> +
> if (virFileReadAll(path, 1024, &buf) < 0)
> goto cleanup;
>
>