Re: [PATCH] util: virhostcpu: Fail when fetching CPU Stats for invalid cpu

virHostCPUGetStatsLinux walks through every cpu in /proc/stat until it finds cpu%cpuNum that matches with the requested cpu. If none is found it logs the error but it should return -1, instead of 0. Otherwise virsh nodecpustats --cpu <invalid cpu number> and API bindings don't fail properly, printing a blank line instead of an error message. This patch also includes an additional test for virhostcputest to avoid this regression to happen again in the future. Reported-by: Satheesh Rajendran <satheera(a)in.ibm.com&gt; Signed-off-by: Mauro S. M. Rodrigues <maurosr(a)linux.vnet.ibm.com&gt; --- src/util/virhostcpu.c | 2 +- tests/virhostcputest.c | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 81293eea8c..20c8d0ce6c 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -847,7 +847,7 @@ virHostCPUGetStatsLinux(FILE *procstat, _("Invalid cpuNum in %s"), __FUNCTION__); - return 0; + return -1; }

diff --git a/tests/virhostcputest.c b/tests/virhostcputest.c index 7865b61578..2f569d8bd4 100644 --- a/tests/virhostcputest.c +++ b/tests/virhostcputest.c @@ -258,14 +258,17 @@ mymain(void) if (virTestRun(nodeData[i].testName, linuxTestHostCPU, &nodeData[i]) != 0) ret = -1; -# define DO_TEST_CPU_STATS(name, ncpus) \ +# define DO_TEST_CPU_STATS(name, ncpus, shouldFail) \ do { \ static struct nodeCPUStatsData data = { name, ncpus }; \ - if (virTestRun("CPU stats " name, linuxTestNodeCPUStats, &data) < 0) \ + if ((virTestRun("CPU stats " name, \ + linuxTestNodeCPUStats, \ + &data) < 0) != shouldFail) \

ret = -1; \ } while (0) - DO_TEST_CPU_STATS("24cpu", 24); + DO_TEST_CPU_STATS("24cpu", 24, false); + DO_TEST_CPU_STATS("24cpu", 25, true); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; }