[libvirt] [PATCHv2 09/11] util: Extend virresctl API to retrieve multiple monitor statistics
Wang, Huaqiang
huaqiang.wang at intel.com
Tue Aug 6 11:18:17 UTC 2019
I'll fix it soon.
> -----Original Message-----
> From: Marc-André Lureau [mailto:marcandre.lureau at gmail.com]
> Sent: Tuesday, August 6, 2019 6:48 PM
> To: Wang, Huaqiang <huaqiang.wang at intel.com>; Michal Privoznik
> <mprivozn at redhat.com>
> Cc: libvir-list at redhat.com; Su, Tao <tao.su at intel.com>
> Subject: Re: [libvirt] [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 at intel.com> wrote:
> >
> > Export virResctrlMonitorGetStats and make
> > virResctrlMonitorGetCacheOccupancy obsoleted.
> >
> > Signed-off-by: Wang Huaqiang <huaqiang.wang at intel.com>
> > ---
> > 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 at redhat.com
> > https://www.redhat.com/mailman/listinfo/libvir-list
>
>
>
> --
> Marc-André Lureau
More information about the libvir-list
mailing list