I'll fix it soon. > -----Original Message----- > From: Marc-André Lureau [mailto:marcandre.lureau@xxxxxxxxx] > Sent: Tuesday, August 6, 2019 6:48 PM > To: Wang, Huaqiang <huaqiang.wang@xxxxxxxxx>; Michal Privoznik > <mprivozn@xxxxxxxxxx> > Cc: libvir-list@xxxxxxxxxx; Su, Tao <tao.su@xxxxxxxxx> > Subject: Re: [PATCHv2 09/11] util: Extend virresctl API to retrieve > multiple monitor statistics > > Hi > > On Tue, Jun 11, 2019 at 7:34 AM Wang Huaqiang > <huaqiang.wang@xxxxxxxxx> wrote: > > > > Export virResctrlMonitorGetStats and make > > virResctrlMonitorGetCacheOccupancy obsoleted. > > > > Signed-off-by: Wang Huaqiang <huaqiang.wang@xxxxxxxxx> > > --- > > src/libvirt_private.syms | 1 + > > src/qemu/qemu_driver.c | 33 +++++++++++++++++++++++++-------- > > src/util/virresctrl.c | 44 +++++++++++++++++++++++++------------------- > > src/util/virresctrl.h | 6 ++++++ > > 4 files changed, 57 insertions(+), 27 deletions(-) > > > > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index > > 1d949b3..3a4f526 100644 > > --- a/src/libvirt_private.syms > > +++ b/src/libvirt_private.syms > > @@ -2771,6 +2771,7 @@ virResctrlMonitorCreate; > > virResctrlMonitorDeterminePath; virResctrlMonitorGetCacheOccupancy; > > virResctrlMonitorGetID; > > +virResctrlMonitorGetStats; > > virResctrlMonitorNew; > > virResctrlMonitorRemove; > > virResctrlMonitorSetAlloc; > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index > > 6b7d3ea..300a5de 100644 > > --- a/src/qemu/qemu_driver.c > > +++ b/src/qemu/qemu_driver.c > > @@ -19991,6 +19991,7 @@ > > qemuDomainFreeResctrlMonData(virQEMUResctrlMonDataPtr resdata) > > /** > > * qemuDomainGetResctrlMonData: > > * @dom: Pointer for the domain that the resctrl monitors reside in > > + * @driver: Pointer to qemu driver > > * @resdata: Pointer of virQEMUResctrlMonDataPtr pointer for receiving > the > > * virQEMUResctrlMonDataPtr array. Caller is responsible for > > * freeing the array. > > @@ -20008,16 +20009,32 @@ > qemuDomainFreeResctrlMonData(virQEMUResctrlMonDataPtr resdata) > > * Returns -1 on failure, or 0 on success. > > */ > > static int > > -qemuDomainGetResctrlMonData(virDomainObjPtr dom, > > +qemuDomainGetResctrlMonData(virQEMUDriverPtr driver, > > + virDomainObjPtr dom, > > virQEMUResctrlMonDataPtr **resdata, > > size_t *nresdata, > > virResctrlMonitorType tag) { > > virDomainResctrlDefPtr resctrl = NULL; > > virQEMUResctrlMonDataPtr res = NULL; > > + char **features = NULL; > > + virCapsPtr caps = NULL; > > size_t i = 0; > > size_t j = 0; > > > > + caps = virQEMUDriverGetCapabilities(driver, false); > > + > > + if (tag == VIR_RESCTRL_MONITOR_TYPE_CACHE) { > > + features = caps->host.cache.monitor->features; > > This makes libvirt crash for me, because caps->host.cache.monitor is NULL. > Any idea? > > thanks > > > + } else { > > + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", > > + _("Unsupported resctrl monitor type")); > > + return -1; > > + } > > + > > + if (virStringListLength((const char * const *)features) == 0) > > + return 0; > > + > > for (i = 0; i < dom->def->nresctrls; i++) { > > resctrl = dom->def->resctrls[i]; > > > > @@ -20044,9 +20061,8 @@ > qemuDomainGetResctrlMonData(virDomainObjPtr dom, > > if (VIR_STRDUP(res->name, virResctrlMonitorGetID(monitor)) < 0) > > goto error; > > > > - if (virResctrlMonitorGetCacheOccupancy(monitor, > > - &res->stats, > > - &res->nstats) < 0) > > + if (virResctrlMonitorGetStats(monitor, (const char **)features, > > + &res->stats, &res->nstats) > > + < 0) > > goto error; > > > > if (VIR_APPEND_ELEMENT(*resdata, *nresdata, res) < 0) @@ > > -20063,7 +20079,8 @@ > qemuDomainGetResctrlMonData(virDomainObjPtr dom, > > > > > > static int > > -qemuDomainGetStatsCpuCache(virDomainObjPtr dom, > > +qemuDomainGetStatsCpuCache(virQEMUDriverPtr driver, > > + virDomainObjPtr dom, > > virDomainStatsRecordPtr record, > > int *maxparams) { @@ -20077,7 +20094,7 @@ > > qemuDomainGetStatsCpuCache(virDomainObjPtr dom, > > if (!virDomainObjIsActive(dom)) > > return 0; > > > > - if (qemuDomainGetResctrlMonData(dom, &resdata, &nresdata, > > + if (qemuDomainGetResctrlMonData(driver, dom, &resdata, &nresdata, > > VIR_RESCTRL_MONITOR_TYPE_CACHE) < 0) > > goto cleanup; > > > > @@ -20178,7 +20195,7 @@ > qemuDomainGetStatsCpuCgroup(virDomainObjPtr > > dom, > > > > > > static int > > -qemuDomainGetStatsCpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, > > +qemuDomainGetStatsCpu(virQEMUDriverPtr driver, > > virDomainObjPtr dom, > > virDomainStatsRecordPtr record, > > int *maxparams, @@ -20187,7 +20204,7 @@ > > qemuDomainGetStatsCpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, > > if (qemuDomainGetStatsCpuCgroup(dom, record, maxparams) < 0) > > return -1; > > > > - if (qemuDomainGetStatsCpuCache(dom, record, maxparams) < 0) > > + if (qemuDomainGetStatsCpuCache(driver, dom, record, maxparams) < > > + 0) > > return -1; > > > > return 0; > > diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index > > 0117b8f..80acb70 100644 > > --- a/src/util/virresctrl.c > > +++ b/src/util/virresctrl.c > > @@ -2669,8 +2669,7 @@ virResctrlMonitorStatsSorter(const void *a, > > * virResctrlMonitorGetStats > > * > > * @monitor: The monitor that the statistic data will be retrieved from. > > - * @resource: The name for resource name. 'llc_occupancy' for cache > resource. > > - * "mbm_total_bytes" and "mbm_local_bytes" for memory bandwidth > resource. > > + * @resources: A string list for the monitor feature names. > > * @stats: Pointer of of virResctrlMonitorStatsPtr array for holding cache > or > > * memory bandwidth usage data. > > * @nstats: A size_t pointer to hold the returned array length of > > @stats @@ -2679,14 +2678,15 @@ virResctrlMonitorStatsSorter(const > void *a, > > * > > * Returns 0 on success, -1 on error. > > */ > > -static int > > +int > > virResctrlMonitorGetStats(virResctrlMonitorPtr monitor, > > - const char *resource, > > + const char **resources, > > virResctrlMonitorStatsPtr **stats, > > size_t *nstats) { > > int rv = -1; > > int ret = -1; > > + size_t i = 0; > > unsigned int val = 0; > > DIR *dirp = NULL; > > char *datapath = NULL; > > @@ -2744,21 +2744,23 @@ > virResctrlMonitorGetStats(virResctrlMonitorPtr monitor, > > if (virStrToLong_uip(node_id, NULL, 0, &stat->id) < 0) > > goto cleanup; > > > > - rv = virFileReadValueUint(&val, "%s/%s/%s", datapath, > > - ent->d_name, resource); > > - if (rv == -2) { > > - virReportError(VIR_ERR_INTERNAL_ERROR, > > - _("File '%s/%s/%s' does not exist."), > > - datapath, ent->d_name, resource); > > - } > > - if (rv < 0) > > - goto cleanup; > > + for (i = 0; resources[i]; i++) { > > + rv = virFileReadValueUint(&val, "%s/%s/%s", datapath, > > + ent->d_name, resources[i]); > > + if (rv == -2) { > > + virReportError(VIR_ERR_INTERNAL_ERROR, > > + _("File '%s/%s/%s' does not exist."), > > + datapath, ent->d_name, resources[i]); > > + } > > + if (rv < 0) > > + goto cleanup; > > > > - if (VIR_APPEND_ELEMENT(stat->vals, stat->nvals, val) < 0) > > - goto cleanup; > > + if (VIR_APPEND_ELEMENT(stat->vals, stat->nvals, val) < 0) > > + goto cleanup; > > > > - if (virStringListAdd(&stat->features, resource) < 0) > > - goto cleanup; > > + if (virStringListAdd(&stat->features, resources[i]) < 0) > > + goto cleanup; > > + } > > > > if (VIR_APPEND_ELEMENT(*stats, *nstats, stat) < 0) > > goto cleanup; > > @@ -2808,6 +2810,10 @@ > virResctrlMonitorGetCacheOccupancy(virResctrlMonitorPtr monitor, > > virResctrlMonitorStatsPtr **stats, > > size_t *nstats) { > > - return virResctrlMonitorGetStats(monitor, "llc_occupancy", > > - stats, nstats); > > + int ret = -1; > > + const char *features[2] = {"llc_occupancy", NULL}; > > + > > + ret = virResctrlMonitorGetStats(monitor, features, stats, > > + nstats); > > + > > + return ret; > > } > > diff --git a/src/util/virresctrl.h b/src/util/virresctrl.h index > > 4dabb9c..9ef51b7 100644 > > --- a/src/util/virresctrl.h > > +++ b/src/util/virresctrl.h > > @@ -236,6 +236,12 @@ int > > virResctrlMonitorRemove(virResctrlMonitorPtr monitor); > > > > int > > +virResctrlMonitorGetStats(virResctrlMonitorPtr monitor, > > + const char **resources, > > + virResctrlMonitorStatsPtr **stats, > > + size_t *nstats); > > + > > +int > > virResctrlMonitorGetCacheOccupancy(virResctrlMonitorPtr monitor, > > virResctrlMonitorStatsPtr **stats, > > size_t *nstats); > > -- > > 2.7.4 > > > > -- > > libvir-list mailing list > > libvir-list@xxxxxxxxxx > > https://www.redhat.com/mailman/listinfo/libvir-list > > > > -- > Marc-André Lureau -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
