On 10/02/2015 11:49 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 <lhu...@redhat.com>
---
src/util/virnuma.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
Perhaps the more entertaining part of this patch is/was it possible to
use "freepages 0 0"? Before patch 1, before patch 2, after patch 2.
Patch 1 + 2 can improve the error when use "freepages 0 0"
Based on the answer to the question below, I can adjust the commit
message (and code if necessary) a bit in order to remove the really long
line and make it flow better.
Thanks a lot for your help, i am not sure about if we need rework this
code or add a patch 3 to improve the error message, please help to check
the reply i will give in another mail.
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"));
+ goto cleanup;
+ }
+
Why is there no "if (node != -1 && !virNumaNodeIsAvailable(node)) {"
check prior to the virNumaGetHugePageInfoPath call?
<same for next change too>
Hmm... i think this check is enough since the output error is "page size
or NUMA node not available", the output message have already include the
case which pass a invalid NUMA node.
I thought that add a check like "if (node != -1 &&
!virNumaNodeIsAvailable(node)) {" just output a different error in this
place, however this (invalid nodeset) will be caught more early in
another place (in nodeGetFreePages function):
if (startCell > lastCell) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("start cell %d out of range (0-%d)"),
startCell, lastCell);
goto cleanup;
}
I noticed another reply from you to this mail, and think we can make the
error more clearly as your said in another reply. I will continue to
reply in another mail.
Thanks a lot for your review and help !
John
Luyao
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;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
