On 5/23/19 11:34 AM, Wang Huaqiang wrote:
Refactor 'virResctrlMonitorStats' to track multiple statistical records.
Signed-off-by: Wang Huaqiang <huaqiang.wang@intel.com> --- 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