[libvirt] [PATCH] qemu: fix an off-by-one error in qemuDomainGetPercpuStats
The max value of number of cpus to compute(id) should not be equal or greater than max cpu number. The bug ocurrs when id value is equal to max cpu number which leads to the off-by-one error in the following for loop. # virsh cpu-stats guest --start 1 error: Failed to virDomainGetCPUStats() error: internal error cpuacct parse error --- src/qemu/qemu_driver.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index dc35b91..54a6d35 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14259,9 +14259,9 @@ qemuDomainGetPercpuStats(virDomainObjPtr vm, param_idx = 0; /* number of cpus to compute */ - id = max_id; - - if (max_id - start_cpu > ncpus - 1) + if ((start_cpu + ncpus) >= max_id) + id = max_id - 1; + else id = start_cpu + ncpus - 1; for (i = 0; i <= id; i++) { -- 1.7.11.2
On 02/20/2013 04:51 AM, Guannan Ren wrote:
The max value of number of cpus to compute(id) should not be equal or greater than max cpu number. The bug ocurrs when id value is equal to max cpu number which
s/ocurrs/occurs/
leads to the off-by-one error in the following for loop.
# virsh cpu-stats guest --start 1 error: Failed to virDomainGetCPUStats()
error: internal error cpuacct parse error --- src/qemu/qemu_driver.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
/* number of cpus to compute */ - id = max_id; - - if (max_id - start_cpu > ncpus - 1)
The old code was trying to avoid the possibility of integer overflow, by only using subtraction (src/libvirt.c already ensured we have non-negative values, but not necessarily that the sum of two positive values won't wrap around to negative).
+ if ((start_cpu + ncpus) >= max_id)
The new code is not as careful, and has redundant (). ACK if you change this to the equivalent: if (start_cpu >= max_id - ncpus)
+ id = max_id - 1; + else id = start_cpu + ncpus - 1;
for (i = 0; i <= id; i++) {
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 02/20/2013 11:25 PM, Eric Blake wrote:
On 02/20/2013 04:51 AM, Guannan Ren wrote:
The max value of number of cpus to compute(id) should not be equal or greater than max cpu number. The bug ocurrs when id value is equal to max cpu number which s/ocurrs/occurs/
leads to the off-by-one error in the following for loop.
# virsh cpu-stats guest --start 1 error: Failed to virDomainGetCPUStats()
error: internal error cpuacct parse error --- src/qemu/qemu_driver.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
/* number of cpus to compute */ - id = max_id; - - if (max_id - start_cpu > ncpus - 1)
The old code was trying to avoid the possibility of integer overflow, by only using subtraction (src/libvirt.c already ensured we have non-negative values, but not necessarily that the sum of two positive values won't wrap around to negative).
Get it now, I was curious about the subtraction before.
+ if ((start_cpu + ncpus) >= max_id) The new code is not as careful, and has redundant ().
ACK if you change this to the equivalent:
if (start_cpu >= max_id - ncpus)
Thanks. I pushed with your changes. Guannan
participants (2)
-
Eric Blake -
Guannan Ren