On 10/21/2015 12:13 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@xxxxxxxxxx>
> ---
> src/util/virnuma.c | 38 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
>
Now that I see things in their final form, one more suggestion which I
can take care of... You've repeated the same "if (!virFileExists)"
check and error messages for all 3 callers to virNumaGetHugePageInfo.
So I think moving that check into virNumaGetHugePageInfo would now be
appropriate (see the attached patch which I'll squash)
I'll also fix up the commit messages and push some time later today
John
> diff --git a/src/util/virnuma.c b/src/util/virnuma.c
> index 815cbc1..cd000f9 100644
> --- a/src/util/virnuma.c
> +++ b/src/util/virnuma.c
> @@ -553,6 +553,25 @@ virNumaGetHugePageInfo(int node,
> page_size, "nr_hugepages") < 0)
> goto cleanup;
>
> + if (!virFileExists(path)) {
> + if (node != -1) {
> + if (!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 on node %d"),
> + page_size, 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 +591,25 @@ virNumaGetHugePageInfo(int node,
> page_size, "free_hugepages") < 0)
> goto cleanup;
>
> + if (!virFileExists(path)) {
> + if (node != -1) {
> + if (!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 on node %d"),
> + page_size, 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;
>
>
>From 84d6454e68d24e50e4d208ef3e5b0ae201faeb41 Mon Sep 17 00:00:00 2001
From: John Ferlan <jferlan@xxxxxxxxxx>
Date: Wed, 21 Oct 2015 09:53:24 -0400
Subject: [PATCH] squash into patch3
Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
---
src/util/virnuma.c | 94 +++++++++++++++++-------------------------------------
1 file changed, 30 insertions(+), 64 deletions(-)
diff --git a/src/util/virnuma.c b/src/util/virnuma.c
index cd000f9..df0f93a 100644
--- a/src/util/virnuma.c
+++ b/src/util/virnuma.c
@@ -493,18 +493,41 @@ virNumaGetHugePageInfoPath(char **path,
unsigned int page_size,
const char *suffix)
{
+ int ret;
+
if (node == -1) {
/* We are aiming at overall system info */
- return virAsprintf(path,
- HUGEPAGES_SYSTEM_PREFIX HUGEPAGES_PREFIX "%ukB/%s",
- page_size, suffix ? suffix : "");
+ ret = virAsprintf(path,
+ HUGEPAGES_SYSTEM_PREFIX HUGEPAGES_PREFIX "%ukB/%s",
+ page_size, suffix ? suffix : "");
} else {
/* We are aiming on specific NUMA node */
- return virAsprintf(path,
- HUGEPAGES_NUMA_PREFIX "node%d/hugepages/"
- HUGEPAGES_PREFIX "%ukB/%s",
- node, page_size, suffix ? suffix : "");
+ ret = virAsprintf(path,
+ HUGEPAGES_NUMA_PREFIX "node%d/hugepages/"
+ HUGEPAGES_PREFIX "%ukB/%s",
+ node, page_size, suffix ? suffix : "");
}
+
+ if (ret >= 0 && !virFileExists(*path)) {
+ ret = -1;
+ if (node != -1) {
+ if (!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 on node %d"),
+ page_size, node);
+ }
+ } else {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("page size %u is not available"),
+ page_size);
+ }
+ }
+
+ return ret;
}
static int
@@ -553,25 +576,6 @@ virNumaGetHugePageInfo(int node,
page_size, "nr_hugepages") < 0)
goto cleanup;
- if (!virFileExists(path)) {
- if (node != -1) {
- if (!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 on node %d"),
- page_size, 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;
@@ -591,25 +595,6 @@ virNumaGetHugePageInfo(int node,
page_size, "free_hugepages") < 0)
goto cleanup;
- if (!virFileExists(path)) {
- if (node != -1) {
- if (!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 on node %d"),
- page_size, 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;
@@ -877,25 +862,6 @@ virNumaSetPagePoolSize(int node,
if (virNumaGetHugePageInfoPath(&nr_path, node, page_size, "nr_hugepages") < 0)
goto cleanup;
- if (!virFileExists(nr_path)) {
- if (node != -1) {
- if (!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 on node %d"),
- page_size, node);
- }
- } else {
- virReportError(VIR_ERR_OPERATION_FAILED,
- _("page size %u is 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.1.0
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list