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@intel.com>
---
src/libvirt-domain.c | 12 ++++
src/qemu/qemu_driver.c | 160 ++++++++++++++++++++++++++++++++++++++++++++++++-
tools/virsh.pod | 14 +++++
3 files changed, 185 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..d9e216c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19929,6 +19929,158 @@ typedef enum {
#define HAVE_JOB(flags) ((flags) & QEMU_DOMAIN_STATS_HAVE_JOB)
+typedef struct _virQEMUResctrlMonData virQEMUResctrlMonData;
+typedef virQEMUResctrlMonData *virQEMUResctrlMonDataPtr;
+struct _virQEMUResctrlMonData {
+ const char *name;
+ char *vcpus;
+ virResctrlMonitorStatsPtr stats;
+ size_t nstats;
+};
+
+
+static int
+qemuDomainGetResctrlMonData(virDomainObjPtr dom,
+ virQEMUResctrlMonDataPtr resdata)
+{
+ virDomainResctrlDefPtr resctrl = NULL;
+ size_t i = 0;
+ size_t j = 0;
+ size_t k = 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 != VIR_RESCTRL_MONITOR_TYPE_CACHE)
+ continue;
+
+ /* If virBitmapFormat successfully returns an vcpu string, then
+ * resdata[k].vcpus is assigned with an memory space holding it,
+ * let this newly allocated memory buffer to be freed along with
+ * the free of 'resdata' */
+ if (!(resdata[k].vcpus = virBitmapFormat(domresmon->vcpus)))
+ return -1;
+
+ if (!(resdata[k].name = virResctrlMonitorGetID(monitor))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Could not get monitor ID"));
+ return -1;
+ }
+
+ if (virResctrlMonitorGetCacheOccupancy(monitor,
+ &resdata[k].stats,
+ &resdata[k].nstats) < 0)
+ return -1;
+
+ k++;
+ }
+ }
+
+ return 0;
+}
+
+
+static int
+qemuDomainGetStatsCpuCache(virDomainObjPtr dom,
+ virDomainStatsRecordPtr record,
+ int *maxparams)
+{
+ char param_name[VIR_TYPED_PARAM_FIELD_LENGTH];
+ virQEMUResctrlMonDataPtr resdata = NULL;
+ virDomainResctrlDefPtr resctrl = NULL;
+ virDomainResctrlMonDefPtr domresmon = NULL;
+ unsigned int nresdata = 0;
+ size_t i = 0;
+ size_t j = 0;
+ int ret = -1;
+
+ if (!virDomainObjIsActive(dom))
+ return 0;
+
+ for (i = 0; i < dom->def->nresctrls; i++) {
+ resctrl = dom->def->resctrls[i];
+ for (j = 0; j < resctrl->nmonitors; j++) {
+ domresmon = resctrl->monitors[j];
+ if (domresmon->tag == VIR_RESCTRL_MONITOR_TYPE_CACHE)
+ nresdata++;
+ }
+ }
+
+ if (nresdata == 0)
+ return 0;
+
+ if (VIR_ALLOC_N(resdata, nresdata) < 0)
+ return -1;
+
+ if (qemuDomainGetResctrlMonData(dom, resdata) < 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:
+ for (i = 0; i < nresdata; i++) {
+ VIR_FREE(resdata[i].vcpus);
+ VIR_FREE(resdata[i].stats);
+ }
+ VIR_FREE(resdata);
+
+ return ret;
+}
+
+
static int
qemuDomainGetStatsCpuCgroup(virDomainObjPtr dom,
virDomainStatsRecordPtr record,
@@ -19976,7 +20128,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:
--
2.7.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 11/20/18 8:56 AM, 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@intel.com>
> ---
> src/libvirt-domain.c | 12 ++++
> src/qemu/qemu_driver.c | 160 ++++++++++++++++++++++++++++++++++++++++++++++++-
> tools/virsh.pod | 14 +++++
> 3 files changed, 185 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..d9e216c 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -19929,6 +19929,158 @@ typedef enum {
> #define HAVE_JOB(flags) ((flags) & QEMU_DOMAIN_STATS_HAVE_JOB)
>
>
> +typedef struct _virQEMUResctrlMonData virQEMUResctrlMonData;
> +typedef virQEMUResctrlMonData *virQEMUResctrlMonDataPtr;
> +struct _virQEMUResctrlMonData {
> + const char *name;
> + char *vcpus;
> + virResctrlMonitorStatsPtr stats;
> + size_t nstats;
> +};
> +
> +
> +static int
> +qemuDomainGetResctrlMonData(virDomainObjPtr dom,
> + virQEMUResctrlMonDataPtr resdata)
> +{
> + virDomainResctrlDefPtr resctrl = NULL;
> + size_t i = 0;
> + size_t j = 0;
> + size_t k = 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 != VIR_RESCTRL_MONITOR_TYPE_CACHE)
> + continue;
If you want to make this generic, then you could pass this tag from
qemuDomainGetStatsCpuCache as the rest would seemingly be useful for
VIR_RESCTRL_MONITOR_TYPE_MEMBW eventually, just different results.
> +
> + /* If virBitmapFormat successfully returns an vcpu string, then
> + * resdata[k].vcpus is assigned with an memory space holding it,
> + * let this newly allocated memory buffer to be freed along with
> + * the free of 'resdata' */
> + if (!(resdata[k].vcpus = virBitmapFormat(domresmon->vcpus)))
> + return -1;
> +
> + if (!(resdata[k].name = virResctrlMonitorGetID(monitor))) {
Could this ever be NULL? Perhaps we just assign directly and assume
we're good. Alternatively it's a VIR_STRDUP() w/ the corresponding
VIR_FREE(*->name).
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Could not get monitor ID"));
> + return -1;
> + }
> +
> + if (virResctrlMonitorGetCacheOccupancy(monitor,
> + &resdata[k].stats,
> + &resdata[k].nstats) < 0)
> + return -1;
> +
> + k++;
> + }
> + }
> +
> + return 0;
> +}
> +
> +
> +static int
> +qemuDomainGetStatsCpuCache(virDomainObjPtr dom,
> + virDomainStatsRecordPtr record,
> + int *maxparams)
> +{
> + char param_name[VIR_TYPED_PARAM_FIELD_LENGTH];
> + virQEMUResctrlMonDataPtr resdata = NULL;
> + virDomainResctrlDefPtr resctrl = NULL;
> + virDomainResctrlMonDefPtr domresmon = NULL;
> + unsigned int nresdata = 0;
> + size_t i = 0;
> + size_t j = 0;
> + int ret = -1;
> +
> + if (!virDomainObjIsActive(dom))
> + return 0;
> +
> + for (i = 0; i < dom->def->nresctrls; i++) {
> + resctrl = dom->def->resctrls[i];
> + for (j = 0; j < resctrl->nmonitors; j++) {
> + domresmon = resctrl->monitors[j];
> + if (domresmon->tag == VIR_RESCTRL_MONITOR_TYPE_CACHE)
> + nresdata++;
> + }
> + }
> +
> + if (nresdata == 0)
> + return 0;
> +
> + if (VIR_ALLOC_N(resdata, nresdata) < 0)
> + return -1;
Given below - perhaps none of the above really matters if you follow how
virResctrlMonitorGetStats was coded using VIR_APPEND_ELEMENT to append
on each @resdata.
> +
> + if (qemuDomainGetResctrlMonData(dom, resdata) < 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:
> + for (i = 0; i < nresdata; i++) {
> + VIR_FREE(resdata[i].vcpus);
> + VIR_FREE(resdata[i].stats);
> + }
> + VIR_FREE(resdata);
All of this should be replaced by a call to qemuDomainFreeResctrlMonData
which would do the above, but replace the VIR_FREE(resdata[i].stats)
with a call to virResctrlMonitorFreeStats which would essentially:
if (!stats)
return;
for (i = 0; i < nstats; i++)
VIR_FREE(stats[i]);
VIR_FREE(stats);
This being the opposing action of virResctrlMonitorGetStats.
See and test if the attached patch works for you.
John
> +
> + return ret;
> +}
> +
> +
> static int
> qemuDomainGetStatsCpuCgroup(virDomainObjPtr dom,
> virDomainStatsRecordPtr record,
> @@ -19976,7 +20128,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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
在 11/24/2018 1:33 AM, John Ferlan 写道:
>
> On 11/20/18 8:56 AM, 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@intel.com>
>> ---
>> src/libvirt-domain.c | 12 ++++
>> src/qemu/qemu_driver.c | 160 ++++++++++++++++++++++++++++++++++++++++++++++++-
>> tools/virsh.pod | 14 +++++
>> 3 files changed, 185 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..d9e216c 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -19929,6 +19929,158 @@ typedef enum {
>> #define HAVE_JOB(flags) ((flags) & QEMU_DOMAIN_STATS_HAVE_JOB)
>>
>>
>> +typedef struct _virQEMUResctrlMonData virQEMUResctrlMonData;
>> +typedef virQEMUResctrlMonData *virQEMUResctrlMonDataPtr;
>> +struct _virQEMUResctrlMonData {
>> + const char *name;
>> + char *vcpus;
>> + virResctrlMonitorStatsPtr stats;
>> + size_t nstats;
>> +};
>> +
>> +
>> +static int
>> +qemuDomainGetResctrlMonData(virDomainObjPtr dom,
>> + virQEMUResctrlMonDataPtr resdata)
>> +{
>> + virDomainResctrlDefPtr resctrl = NULL;
>> + size_t i = 0;
>> + size_t j = 0;
>> + size_t k = 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 != VIR_RESCTRL_MONITOR_TYPE_CACHE)
>> + continue;
> If you want to make this generic, then you could pass this tag from
> qemuDomainGetStatsCpuCache as the rest would seemingly be useful for
> VIR_RESCTRL_MONITOR_TYPE_MEMBW eventually, just different results.
Good suggestion. Thanks.
>> +
>> + /* If virBitmapFormat successfully returns an vcpu string, then
>> + * resdata[k].vcpus is assigned with an memory space holding it,
>> + * let this newly allocated memory buffer to be freed along with
>> + * the free of 'resdata' */
>> + if (!(resdata[k].vcpus = virBitmapFormat(domresmon->vcpus)))
>> + return -1;
>> +
>> + if (!(resdata[k].name = virResctrlMonitorGetID(monitor))) {
> Could this ever be NULL? Perhaps we just assign directly and assume
> we're good. Alternatively it's a VIR_STRDUP() w/ the corresponding
> VIR_FREE(*->name).
Resctrl monitor ID will not be NULL at this place.
I am using VIR_STRDUP to make a copy of name and ensure it is freed at
the end
of function in next version.
>
>> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> + _("Could not get monitor ID"));
>> + return -1;
>> + }
>> +
>> + if (virResctrlMonitorGetCacheOccupancy(monitor,
>> + &resdata[k].stats,
>> + &resdata[k].nstats) < 0)
>> + return -1;
>> +
>> + k++;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +
>> +static int
>> +qemuDomainGetStatsCpuCache(virDomainObjPtr dom,
>> + virDomainStatsRecordPtr record,
>> + int *maxparams)
>> +{
>> + char param_name[VIR_TYPED_PARAM_FIELD_LENGTH];
>> + virQEMUResctrlMonDataPtr resdata = NULL;
>> + virDomainResctrlDefPtr resctrl = NULL;
>> + virDomainResctrlMonDefPtr domresmon = NULL;
>> + unsigned int nresdata = 0;
>> + size_t i = 0;
>> + size_t j = 0;
>> + int ret = -1;
>> +
>> + if (!virDomainObjIsActive(dom))
>> + return 0;
>> +
>> + for (i = 0; i < dom->def->nresctrls; i++) {
>> + resctrl = dom->def->resctrls[i];
>> + for (j = 0; j < resctrl->nmonitors; j++) {
>> + domresmon = resctrl->monitors[j];
>> + if (domresmon->tag == VIR_RESCTRL_MONITOR_TYPE_CACHE)
>> + nresdata++;
>> + }
>> + }
>> +
>> + if (nresdata == 0)
>> + return 0;
>> +
>> + if (VIR_ALLOC_N(resdata, nresdata) < 0)
>> + return -1;
> Given below - perhaps none of the above really matters if you follow how
> virResctrlMonitorGetStats was coded using VIR_APPEND_ELEMENT to append
> on each @resdata.
OK. I'll use VIR_APPEND_ELEMENT to allocate and append new element
for @resdata. That means the lines of 'if (VIR_ALLOC_N(resdata,
nresdata) < 0)'
will be removed and the @nresdata will not be accumulated before calling
qemuDomainGetResctrlMonData.
>
>> +
>> + if (qemuDomainGetResctrlMonData(dom, resdata) < 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:
>> + for (i = 0; i < nresdata; i++) {
>> + VIR_FREE(resdata[i].vcpus);
>> + VIR_FREE(resdata[i].stats);
>> + }
>> + VIR_FREE(resdata);
> All of this should be replaced by a call to qemuDomainFreeResctrlMonData
> which would do the above, but replace the VIR_FREE(resdata[i].stats)
> with a call to virResctrlMonitorFreeStats which would essentially:
>
> if (!stats)
> return;
>
> for (i = 0; i < nstats; i++)
> VIR_FREE(stats[i]);
>
> VIR_FREE(stats);
>
> This being the opposing action of virResctrlMonitorGetStats.
>
>
> See and test if the attached patch works for you.
The patch wouldn't work. In virResctrlMonitorGetStats the @stats
is patched as a array of virResctrlMonitorStats not
virResctrlMonitorStatsPtr,
so the line VIR_FREE(stats[i]) does not work. Element of *stats is not
pointer
but a virResctrlMonitorStats structure.
The VIR_APPEND_ELEMENTis:
if (VIR_APPEND_ELEMENT(*stats, *nstats, *stat) < 0)
and @stat is
virResctrlMonitorStatsPtr stat = NULL;
So it appends the whole element to the end of *@stats.
If you think it is more common to append a pointer not the data
structure, I can change it.
Anyway I'll send a patch to append the pointer (virResctrlMonitorStatsPtr).
You can make the decision to use it or not.
> John
Thanks for review.
Huaqiang
>> +
>> + return ret;
>> +}
>> +
>> +
>> static int
>> qemuDomainGetStatsCpuCgroup(virDomainObjPtr dom,
>> virDomainStatsRecordPtr record,
>> @@ -19976,7 +20128,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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.