Re: [libvirt] [PATCHv2] cpustat: fix regression when cpus are offline

It turns out that the cpuacct results properly account for offline cpus, and always returns results for every possible cpu, not just the online ones. So there is no need to check the map of online cpus in the first place, merely only a need to know the maximum possible cpu. Meanwhile, virNodeGetCPUBitmap had a subtle change from returning the maximum id to instead returning the width of the bitmap (one larger than the maximum id) in commit 2f4c5338, which made this code encounter some off-by-one logic leading to bad error messages when a cpu was offline: $ virsh cpu-stats dom error: Failed to virDomainGetCPUStats() error: An error occurred, but the cause is unknown Cleaning this up unraveled a chain of other unused variables. * src/qemu/qemu_driver.c (qemuDomainGetPercpuStats): Drop pointless check for cpumap changes, and use correct number of cpus. Simplify signature. (qemuDomainGetCPUStats): Adjust caller. * src/nodeinfo.h (nodeGetCPUCount): New prototype. (nodeGetCPUBitmap): Drop unused parameter. * src/nodeinfo.c (nodeGetCPUBitmap): Likewise. (nodeGetCPUMap): Adjust caller. (nodeGetCPUCount): New function. * src/libvirt_private.syms (nodeinfo.h): Export it. --- v2: add new function for determining just host cpu count, and do even more cleanups based on unused variables src/libvirt_private.syms | 1 + src/nodeinfo.c | 30 +++++++++++++++++++++++------- src/nodeinfo.h | 4 ++-- src/qemu/qemu_driver.c | 39 ++++++++------------------------------- 4 files changed, 34 insertions(+), 40 deletions(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 8b494df..b0be3ed 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -949,9 +949,24 @@ int nodeGetMemoryStats(virConnectPtr conn ATTRIBUTE_UNUSED, #endif } +int +nodeGetCPUCount(void) +{ +#ifdef __linux__ + /* XXX should we also work on older kernels, like RHEL5, that lack + * cpu/present and cpu/online files? Those kernels also lack cpu + * hotplugging, so it would be a matter of finding the largest + * cpu/cpuNN directory, and returning NN + 1 */ + return linuxParseCPUmax(SYSFS_SYSTEM_PATH "/cpu/present"); +#else + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("host cpu counting not implemented on this platform")); + return NULL;

+#endif +} + virBitmapPtr -nodeGetCPUBitmap(virConnectPtr conn ATTRIBUTE_UNUSED, - int *max_id ATTRIBUTE_UNUSED) +nodeGetCPUBitmap(int *max_id ATTRIBUTE_UNUSED) { #ifdef __linux__ virBitmapPtr cpumap;