[PATCH] util: virhostcpu: Fail when fetching CPU Stats for invalid cpu
Christian Ehrhardt
christian.ehrhardt at canonical.com
Mon Mar 23 14:20:02 UTC 2020
On Fri, Feb 21, 2020 at 7:11 PM Mauro S. M. Rodrigues <
maurosr at linux.vnet.ibm.com> wrote:
> 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.
>
I reviewed and tested this in the scope of [1]
Before:
root at f:~# virsh nodecpustats --cpu 47
root at f:~# echo $?
0
After
root at f:~# virsh nodecpustats --cpu 47
error: Unable to get node cpu stats
error: Invalid cpuNum in virHostCPUGetStatsLinux
root at f-dcap-after-cap:~# echo $?
1
You also see the added test passing
PASS: virhostcputest
in the build log at [2]
Thanks Mauro, feel free to add my review and test tags:
Reviewed-by: Christian Ehrhardt <christian.ehrhardt at canonical.com>
Tested-by: Christian Ehrhardt <christian.ehrhardt at canonical.com>
[1]: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1868528
[2]:
https://launchpadlibrarian.net/470309458/buildlog_ubuntu-focal-amd64.libvirt_6.0.0-0ubuntu6~ppa2_BUILDING.txt.gz
> Reported-by: Satheesh Rajendran <satheera at in.ibm.com>
> Signed-off-by: Mauro S. M. Rodrigues <maurosr at linux.vnet.ibm.com>
> ---
> 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;
> }
> --
> 2.24.1
>
>
>
--
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200323/b64929d2/attachment-0001.htm>
More information about the libvir-list
mailing list