On 10/21/2015 09:58 PM, John Ferlan wrote:
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
Oh, right, i should move the check in virNumaGetHugePageInfo to avoid code duplicated. Your patch looks good to me. And thanks a lot for your quick review and lots of help!
John
Luyao
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;
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list