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

[Peter Krempa]

On 10/26/12 19:52, Eric Blake wrote:
> 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;

s/NULL/-1/

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

ACK with the nit fixed.

Peter