On 11/26/18 12:56 PM, Wang Huaqiang wrote:
> Adding the interface in qemu to report CMT statistic information
> through command 'virsh domstats --cpu-total'.
>
> Below is a typical output:
>
> # virsh domstats 1 --cpu-total
> Domain: 'ubuntu16.04-base'
> ...
> cpu.cache.monitor.count=2
> cpu.cache.monitor.0.name=vcpus_1
> cpu.cache.monitor.0.vcpus=1
> cpu.cache.monitor.0.bank.count=2
> cpu.cache.monitor.0.bank.0.id=0
> cpu.cache.monitor.0.bank.0.bytes=4505600
> cpu.cache.monitor.0.bank.1.id=1
> cpu.cache.monitor.0.bank.1.bytes=5586944
> cpu.cache.monitor.1.name=vcpus_4-6
> cpu.cache.monitor.1.vcpus=4,5,6
> cpu.cache.monitor.1.bank.count=2
> cpu.cache.monitor.1.bank.0.id=0
> cpu.cache.monitor.1.bank.0.bytes=17571840
> cpu.cache.monitor.1.bank.1.id=1
> cpu.cache.monitor.1.bank.1.bytes=29106176
>
> Signed-off-by: Wang Huaqiang <huaqiang.wang@xxxxxxxxx>
> ---
> src/libvirt-domain.c | 12 ++++
> src/qemu/qemu_driver.c | 183 ++++++++++++++++++++++++++++++++++++++++++++++++-
> tools/virsh.pod | 14 ++++
> 3 files changed, 208 insertions(+), 1 deletion(-)
>
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 5b76458..73d602e 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -11415,6 +11415,18 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
> * "cpu.user" - user cpu time spent in nanoseconds as unsigned long long.
> * "cpu.system" - system cpu time spent in nanoseconds as unsigned long
> * long.
> + * "cpu.cache.monitor.count" - the number of cache monitors for this domain
> + * "cpu.cache.monitor.<num>.name" - the name of cache monitor <num>
> + * "cpu.cache.monitor.<num>.vcpus" - vcpu list of cache monitor <num>
> + * "cpu.cache.monitor.<num>.bank.count" - the number of cache banks in
> + * cache monitor <num>
> + * "cpu.cache.monitor.<num>.bank.<index>.id" - host allocated cache id for
> + * bank <index> in cache
> + * monitor <num>
> + * "cpu.cache.monitor.<num>.bank.<index>.bytes" - the number of bytes of
> + * last level cache that the
> + * domain is using on cache
> + * bank <index>
> *
> * VIR_DOMAIN_STATS_BALLOON:
> * Return memory balloon device information.
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 7fb9102..ac2be35 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -19929,6 +19929,181 @@ typedef enum {
> #define HAVE_JOB(flags) ((flags) & QEMU_DOMAIN_STATS_HAVE_JOB)
>
>
> +typedef struct _virQEMUResctrlMonData virQEMUResctrlMonData;
> +typedef virQEMUResctrlMonData *virQEMUResctrlMonDataPtr;
> +struct _virQEMUResctrlMonData {
> + char *name;
> + char *vcpus;
> + virResctrlMonitorStatsPtr *stats;
> + size_t nstats;
> +};
> +
> +
> +static void
> +qemuDomainFreeResctrlMonData(virQEMUResctrlMonDataPtr *resdata,
> + size_t nresdata)
> +{
> + size_t i = 0;
> +
> + for (i = 0; i < nresdata; i++) {
> + VIR_FREE(resdata[i]->name);
> + VIR_FREE(resdata[i]->vcpus);
> + virResctrlMonitorFreeStats(resdata[i]->stats, resdata[i]->nstats);
> + VIR_FREE(resdata[i]);
> + }
> +
> + VIR_FREE(resdata);
> +}
> +
> +
> +/**
> + * qemuDomainGetResctrlMonData:
> + * @dom: Pointer for the domain that the resctrl monitors reside in
> + * @resdata: Pointer of virQEMUResctrlMonDataPtr pointer for receiving the
> + * virQEMUResctrlMonDataPtr array. Caller is responsible for
> + * freeing the array.
> + * @nresdata: Pointer of size_t to report the size virQEMUResctrlMonDataPtr
> + * array to caller. If *@nresdata is not 0, even if function
> + * returns an error, the caller is also required to call
> + * qemuDomainFreeResctrlMonData to free the array in *@resdata
> + * @tag: Could be VIR_RESCTRL_MONITOR_TYPE_CACHE for getting cache statistics
> + * from @dom cache monitors. VIR_RESCTRL_MONITOR_TYPE_MEMBW for
> + * getting memory bandwidth statistics from memory bandwidth monitors.
> + *
> + * Get cache or memory bandwidth statistics from @dom monitors.
> + *
> + * Returns -1 on failure, or 0 on success.
> + */
> +static int
> +qemuDomainGetResctrlMonData(virDomainObjPtr dom,
> + virQEMUResctrlMonDataPtr **resdata,
> + size_t *nresdata,
> + virResctrlMonitorType tag)
> +{
> + virDomainResctrlDefPtr resctrl = NULL;
> + virQEMUResctrlMonDataPtr res = NULL;
> + size_t i = 0;
> + size_t j = 0;
> +
> + for (i = 0; i < dom->def->nresctrls; i++) {
> + resctrl = dom->def->resctrls[i];
> +
> + for (j = 0; j < resctrl->nmonitors; j++) {
> + virDomainResctrlMonDefPtr domresmon = NULL;
> + virResctrlMonitorPtr monitor = NULL;
> +
> + domresmon = resctrl->monitors[j];
> + monitor = domresmon->instance;
> +
> + if (domresmon->tag != tag)
> + continue;
> +
> + if (VIR_ALLOC(res) < 0)
> + return -1;
> +
> + /* If virBitmapFormat successfully returns an vcpu string, then
> + * res.vcpus is assigned with an memory space holding it,
> + * let this newly allocated memory buffer to be freed along with
> + * the free of 'res' */
> + if (!(res->vcpus = virBitmapFormat(domresmon->vcpus)))
> + goto error;
> +
> + if (VIR_STRDUP(res->name, virResctrlMonitorGetID(monitor)) < 0)
> + goto error;
> +
> + if (virResctrlMonitorGetCacheOccupancy(monitor,
> + &res->stats,
> + &res->nstats) < 0)
> + goto error;
> +
> + if (VIR_APPEND_ELEMENT(*resdata, *nresdata, res) < 0)
> + goto error;
> + }
> + }
> +
> + return 0;
> +
> + error:
> + if (res)
Cannot get here with res == NULL
> + qemuDomainFreeResctrlMonData(&res, 1);
@res is a single element not the array of elements.
What's going on here is closer to what virMediatedDeviceTypeFree does
I'll alter to just take an @resdata and have this called as:
qemuDomainFreeResctrlMonData(res);
and alter qemuDomainFreeResctrlMonData to just do what's inside the for
loop above.
then...
> +
> + return -1;
> +}
> +
> +
> +static int
> +qemuDomainGetStatsCpuCache(virDomainObjPtr dom,
> + virDomainStatsRecordPtr record,
> + int *maxparams)
> +{
> + char param_name[VIR_TYPED_PARAM_FIELD_LENGTH];
> + virQEMUResctrlMonDataPtr *resdata = NULL;
> + size_t nresdata = 0;
> + size_t i = 0;
> + size_t j = 0;
> + int ret = -1;
> +
> + if (!virDomainObjIsActive(dom))
> + return 0;
> +
> + if (qemuDomainGetResctrlMonData(dom, &resdata, &nresdata,
> + VIR_RESCTRL_MONITOR_TYPE_CACHE) < 0)
> + goto cleanup;
> +
> + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> + "cpu.cache.monitor.count");
> + if (virTypedParamsAddUInt(&record->params, &record->nparams,
> + maxparams, param_name, nresdata) < 0)
> + goto cleanup;
> +
> + for (i = 0; i < nresdata; i++) {
> + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> + "cpu.cache.monitor.%zu.name", i);
> + if (virTypedParamsAddString(&record->params,
> + &record->nparams,
> + maxparams,
> + param_name,
> + resdata[i]->name) < 0)
> + goto cleanup;
> +
> + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> + "cpu.cache.monitor.%zu.vcpus", i);
> + if (virTypedParamsAddString(&record->params, &record->nparams,
> + maxparams, param_name,
> + resdata[i]->vcpus) < 0)
> + goto cleanup;
> +
> + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> + "cpu.cache.monitor.%zu.bank.count", i);
> + if (virTypedParamsAddUInt(&record->params, &record->nparams,
> + maxparams, param_name,
> + resdata[i]->nstats) < 0)
> + goto cleanup;
> +
> + for (j = 0; j < resdata[i]->nstats; j++) {
> + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> + "cpu.cache.monitor.%zu.bank.%zu.id", i, j);
> + if (virTypedParamsAddUInt(&record->params, &record->nparams,
> + maxparams, param_name,
> + resdata[i]->stats[j]->id) < 0)
> + goto cleanup;
> +
> + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> + "cpu.cache.monitor.%zu.bank.%zu.bytes", i, j);
> + if (virTypedParamsAddUInt(&record->params, &record->nparams,
> + maxparams, param_name,
> + resdata[i]->stats[j]->val) < 0)
> + goto cleanup;
> + }
> + }
> +
> + ret = 0;
> + cleanup:
> + qemuDomainFreeResctrlMonData(resdata, nresdata);
Change this to:
for (i = 0; i < nresdata; i++)
qemuDomainFreeResctrlMonData(resdata[i]);
VIR_FREE(resdata);
Everything else seems fine
John
Most likely bad instructions on my part.
> + return ret;
> +}
> +
> +
> static int
> qemuDomainGetStatsCpuCgroup(virDomainObjPtr dom,
> virDomainStatsRecordPtr record,
> @@ -19976,7 +20151,13 @@ qemuDomainGetStatsCpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
> int *maxparams,
> unsigned int privflags ATTRIBUTE_UNUSED)
> {
> - return qemuDomainGetStatsCpuCgroup(dom, record, maxparams);
> + if (qemuDomainGetStatsCpuCgroup(dom, record, maxparams) < 0)
> + return -1;
> +
> + if (qemuDomainGetStatsCpuCache(dom, record, maxparams) < 0)
> + return -1;
> +
> + return 0;
> }
>
>
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index 4876656..86a4996 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -1012,6 +1012,20 @@ I<--cpu-total> returns:
> "cpu.time" - total cpu time spent for this domain in nanoseconds
> "cpu.user" - user cpu time spent in nanoseconds
> "cpu.system" - system cpu time spent in nanoseconds
> + "cpu.cache.monitor.count" - the number of cache monitors for this
> + domain
> + "cpu.cache.monitor.<num>.name" - the name of cache monitor <num>
> + "cpu.cache.monitor.<num>.vcpus" - vcpu list of cache monitor <num>
> + "cpu.cache.monitor.<num>.bank.count" - the number of cache banks
> + in cache monitor <num>
> + "cpu.cache.monitor.<num>.bank.<index>.id" - host allocated cache id
> + for bank <index> in
> + cache monitor <num>
> + "cpu.cache.monitor.<num>.bank.<index>.bytes" - the number of bytes
> + of last level cache
> + that the domain is
> + using on cache bank
> + <index>
>
> I<--balloon> returns:
>
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
