Re: [PATCH] virDomainGetCPUStats: fix boundary value problem of start_cpu

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 26.02.2014 10:18, Jincheng Miao wrote:
This API has boundary value problem, if start_cpu is equal to
the number of cpus, no error infomation will be reported.

This is because the confused meaning of variable max_id,
so change the comparision and rename the variable max_id to total_num.

Signed-off-by: Jincheng Miao <jmiao@xxxxxxxxxx>
---
  src/qemu/qemu_driver.c | 18 +++++++++---------
  src/util/vircgroup.c   | 18 +++++++++---------
  tools/virsh-domain.c   | 12 ++++++------
  3 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c9a865e..54b8e5b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15983,7 +15983,7 @@ qemuDomainGetPercpuStats(virDomainObjPtr vm,
  {
      int rv = -1;
      size_t i;
-    int id, max_id;
+    int id, total_num;
      char *pos;
      char *buf = NULL;
      unsigned long long *sum_cpu_time = NULL;
@@ -15999,19 +15999,19 @@ qemuDomainGetPercpuStats(virDomainObjPtr vm,
          return QEMU_NB_PER_CPU_STAT_PARAM;

      /* To parse account file, we need to know how many cpus are present.  */
-    max_id = nodeGetCPUCount();
-    if (max_id < 0)
+    total_num = nodeGetCPUCount();
+    if (total_num < 0)
          return rv;

-    if (ncpus == 0) { /* returns max cpu ID */
-        rv = max_id;
+    if (ncpus == 0) { /* returns total number of cpu */
+        rv = total_num;
          goto cleanup;
      }

-    if (start_cpu > max_id) {
+    if (start_cpu > total_num - 1) {

This actually is the fix. But ...

          virReportError(VIR_ERR_INVALID_ARG,
                         _("start_cpu %d larger than maximum of %d"),
-                       start_cpu, max_id);
+                       start_cpu, total_num - 1);
          goto cleanup;
      }

@@ -16024,8 +16024,8 @@ qemuDomainGetPercpuStats(virDomainObjPtr vm,
      param_idx = 0;

      /* number of cpus to compute */
-    if (start_cpu >= max_id - ncpus)
-        id = max_id - 1;
+    if (start_cpu + ncpus >= total_num)
+        id = total_num - 1;
      else
          id = start_cpu + ncpus - 1;

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 0f04b4d..2af06af 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -2836,7 +2836,7 @@ virCgroupGetPercpuStats(virCgroupPtr group,
  {
      int rv = -1;
      size_t i;
-    int id, max_id;
+    int id, total_num;
      char *pos;
      char *buf = NULL;
      virTypedParameterPtr ent;
@@ -2848,19 +2848,19 @@ virCgroupGetPercpuStats(virCgroupPtr group,
          return CGROUP_NB_PER_CPU_STAT_PARAM;

      /* To parse account file, we need to know how many cpus are present.  */
-    max_id = nodeGetCPUCount();
-    if (max_id < 0)
+    total_num = nodeGetCPUCount();
+    if (total_num < 0)
          return rv;

-    if (ncpus == 0) { /* returns max cpu ID */
-        rv = max_id;
+    if (ncpus == 0) { /* returns total number of cpu */
+        rv = total_num;
          goto cleanup;
      }

-    if (start_cpu > max_id) {
+    if (start_cpu > total_num - 1) {

because you had to duplicate the fix, it means, we are duplicating the code too. The fix is correct, but I think we should somehow make qemuDomainGetPercpuStats() call virCgroupGetPercpuStats() so we don't duplicate code.

Michal

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]