[libvirt] [PATCH] Fix virCgroupGetPercpuStats with non-continuous present CPUs

[Eric Blake]

On 01/22/2015 04:26 AM, Ján Tomko wrote:
> Per-cpu stats are only shown for present CPUs in the cgroups,
> but we were only parsing the largest CPU number from
> /sys/devices/system/cpu/present and looking for stats even for
> non-present CPUs.
> This resulted in:
> internal error: cpuacct parse error
> ---
>  cfg.mk                |  2 +-
>  src/nodeinfo.c        | 17 +++++++++++++++++
>  src/nodeinfo.h        |  1 +
>  src/util/vircgroup.c  | 24 ++++++++++++++++++------
>  tests/vircgroupmock.c | 44 ++++++++++++++++++++++++++++++++++++++------
>  tests/vircgrouptest.c | 47 +++++++++++++++++++++++++++++++++++------------
>  6 files changed, 110 insertions(+), 25 deletions(-)
> 

> +++ b/tests/vircgroupmock.c
> @@ -51,6 +51,8 @@ const char *fakedevicedir1 = FAKEDEVDIR1;
>  
>  
>  # define SYSFS_PREFIX "/not/really/sys/fs/cgroup/"
> +# define SYSFS_CPU_PRESENT "/sys/devices/system/cpu/present"
> +# define SYSFS_CPU_PRESENT_MOCKED "devices_system_cpu_present"

This mock file is not working on RHEL 5:

> +++ b/tests/vircgrouptest.c
> @@ -538,12 +538,35 @@ static int testCgroupGetPercpuStats(const void *args ATTRIBUTE_UNUSED)
>      virCgroupPtr cgroup = NULL;
>      size_t i;
>      int rv, ret = -1;
> -    virTypedParameter params[2];
> +    virTypedParameterPtr params = NULL;
> +# define EXPECTED_NCPUS 160

> @@ -553,37 +576,37 @@ static int testCgroupGetPercpuStats(const void *args ATTRIBUTE_UNUSED)
>          goto cleanup;
>      }
>  
> -    if (nodeGetCPUCount() < 1) {
> +    if (nodeGetCPUCount() != EXPECTED_NCPUS) {

On Fedora 21, I see the mocking kicking in during linuxParseCPUmax():

nodeGetCPUCount () at nodeinfo.c:1209
1209	{
(gdb) n
1216	    char *cpupath = NULL;
(gdb)
1219	    if (virFileExists(SYSFS_SYSTEM_PATH "/cpu/present")) {
(gdb)
1220	        ncpu = linuxParseCPUmax(SYSFS_SYSTEM_PATH "/cpu/present");
(gdb)
1237	    VIR_FREE(cpupath);
(gdb) p ncpu
$1 = 160

but in RHEL 5, we aren't mocking the virFileExists() gate, and I see:

nodeGetCPUCount () at nodeinfo.c:1209
1209    {
(gdb) n
1216        char *cpupath = NULL;
(gdb)
1219        if (virFileExists(SYSFS_SYSTEM_PATH "/cpu/present")) {
(gdb)
1221        } else if (virFileExists(SYSFS_SYSTEM_PATH "/cpu/cpu0")) {
(gdb)
1225                VIR_FREE(cpupath);
(gdb)
1224                ncpu++;
(gdb)
1225                VIR_FREE(cpupath);
(gdb)
1226                if (virAsprintf(&cpupath, "%s/cpu/cpu%d",
(gdb) fin
Run till exit from #0  nodeGetCPUCount () at nodeinfo.c:1226
0x0804ae19 in testCgroupGetPercpuStats (args=0x0) at vircgrouptest.c:659
659         if (nodeGetCPUCount() != EXPECTED_NCPUS) {
Value returned is $1 = 3

I didn't investigate further, but we need to improve the mock code so
that virFileExists(SYSFS_SYSTEM_PATH "/cpu/present") works even when run
on an older kernel where it is not natively present.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150415/ceca2095/attachment-0001.sig>