Re: [PATCH 7/9] util: Refactor 'virResctrlMonitorStats'

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

 



On 5/23/19 11:34 AM, Wang Huaqiang wrote:
Refactor 'virResctrlMonitorStats' to track multiple statistical
records.

Signed-off-by: Wang Huaqiang <huaqiang.wang@xxxxxxxxx>
---
  src/qemu/qemu_driver.c |  2 +-
  src/util/virresctrl.c  | 17 ++++++++++++++---
  src/util/virresctrl.h  | 12 ++++++++++--
  3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2abed86..4ea346c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20120,7 +20120,7 @@ qemuDomainGetStatsCpuCache(virDomainObjPtr dom,
                       "cpu.cache.monitor.%zu.bank.%zu.bytes", i, j);
              if (virTypedParamsAddUInt(&record->params, &record->nparams,
                                        maxparams, param_name,
-                                      resdata[i]->stats[j]->val) < 0)
+                                      resdata[i]->stats[j]->vals[0]) < 0)

So why undergo the whole torture of 8/9 and 9/9 if we will report one value only?

Also, I'm not sure @vals is going to be allocated at all times, will it?

                  goto cleanup;
          }
      }
diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index 0f18d2b..c2fe2ed 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -2686,6 +2686,7 @@ virResctrlMonitorGetStats(virResctrlMonitorPtr monitor,
  {
      int rv = -1;
      int ret = -1;
+    unsigned int val = 0;
      DIR *dirp = NULL;
      char *datapath = NULL;
      char *filepath = NULL;
@@ -2742,7 +2743,7 @@ virResctrlMonitorGetStats(virResctrlMonitorPtr monitor,
          if (virStrToLong_uip(node_id, NULL, 0, &stat->id) < 0)
              goto cleanup;
- rv = virFileReadValueUint(&stat->val, "%s/%s/%s", datapath,
+        rv = virFileReadValueUint(&val, "%s/%s/%s", datapath,
                                    ent->d_name, resource);
          if (rv == -2) {
              virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -2752,6 +2753,12 @@ virResctrlMonitorGetStats(virResctrlMonitorPtr monitor,
          if (rv < 0)
              goto cleanup;
+ if (VIR_APPEND_ELEMENT(stat->vals, stat->nvals, val) < 0)
+            goto cleanup;
+
+        if (virStringListAdd(&stat->features, resource) < 0)
+            goto cleanup;
+
          if (VIR_APPEND_ELEMENT(*stats, *nstats, stat) < 0)
              goto cleanup;
      }
@@ -2779,8 +2786,12 @@ virResctrlMonitorFreeStats(virResctrlMonitorStatsPtr *stats,
      if (!stats)
          return;
- for (i = 0; i < nstats; i++)
-        VIR_FREE(stats[i]);
+    for (i = 0; i < nstats; i++) {
+        if (stats[i]) {
+            VIR_FREE(stats[i]->vals);
+            virStringListFree(stats[i]->features);
+        }
+    }
  }
diff --git a/src/util/virresctrl.h b/src/util/virresctrl.h
index abdeb59..97205bc 100644
--- a/src/util/virresctrl.h
+++ b/src/util/virresctrl.h
@@ -194,8 +194,16 @@ typedef virResctrlMonitor *virResctrlMonitorPtr;
  typedef struct _virResctrlMonitorStats virResctrlMonitorStats;
  typedef virResctrlMonitorStats *virResctrlMonitorStatsPtr;
  struct _virResctrlMonitorStats {
-    unsigned int id;
-    unsigned int val;
+    /* The system assigned cache ID associated with statistical record */
+     unsigned int id;
+    /* @features is a NULL terminal string list tracking the statistical record
+     * name.*/
+    char **features;
+    /* @vals store the statistical record values and @val[0] is the value for
+     * @features[0], @val[1] for@features[1] ... respectively */
+    unsigned int *vals;
+    /* The length of @vals array */
+    size_t nvals;

We like the following style more:

struct X {
  int memberA;  /* some description of this member
                   split into two lines */
  bool memberB; /* one line description */
};

But on the other hand, this is consistent with the rest of resctrl code. Your call which style to use.

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]

  Powered by Linux