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

Peter Krempa pkrempa at redhat.com
Fri Oct 26 13:59:24 UTC 2012


On 10/25/12 22:57, 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), 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
>
> * src/qemu/qemu_driver.c (qemuDomainGetPercpuStats): Drop
> pointless check for cpumap changes, and use correct number of
> cpus.
> ---
>
> Fixes the regression noticed here:
> https://www.redhat.com/archives/libvir-list/2012-October/msg01508.html
>
>   src/qemu/qemu_driver.c | 26 +++-----------------------
>   1 file changed, 3 insertions(+), 23 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 18be7d9..f817319 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -13597,7 +13597,6 @@ qemuDomainGetPercpuStats(virDomainPtr domain,
>                            unsigned int ncpus)
>   {
>       virBitmapPtr map = NULL;
> -    virBitmapPtr map2 = NULL;

When you apply this patch "map" is also not needed (never actualy used).

Is there a way to replace
"map = nodeGetCPUBitmap(domain->conn, &max_id);" to get max_id? That's 
the only thing that code is used to.

>       int rv = -1;
>       int i, id, max_id;
>       char *pos;

Otherwise looks good. (ACK if there isn't any other option to get max_id 
than to call nodeGetCPUBitmap)

Peter




More information about the libvir-list mailing list