[libvirt] [PATCHv10 1/4] util: Return a list of pointer in virResctrlMonitorGetStats

Discussion:

Wang Huaqiang

2018-11-26 17:56:14 UTC

Return a list of virResctrlMonitorStatsPtr instead of
a virResctrlMonitorStats array in virResctrlMonitorGetStats.

Signed-off-by: Wang Huaqiang <***@intel.com>
---
src/util/virresctrl.c | 10 +++++-----
src/util/virresctrl.h | 4 ++--
2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index b32eedc..3268310 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -2647,8 +2647,8 @@ virResctrlMonitorStatsSorter(const void *a,
* @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.
- * @stats: Array of virResctrlMonitorStatsPtr for holding cache or memory
- * bandwidth usage data.
+ * @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
*
* Get cache or memory bandwidth utilization information.
@@ -2658,7 +2658,7 @@ virResctrlMonitorStatsSorter(const void *a,
static int
virResctrlMonitorGetStats(virResctrlMonitorPtr monitor,
const char *resource,
- virResctrlMonitorStatsPtr *stats,
+ virResctrlMonitorStatsPtr **stats,
size_t *nstats)
{
int rv = -1;
@@ -2729,7 +2729,7 @@ virResctrlMonitorGetStats(virResctrlMonitorPtr monitor,
if (rv < 0)
goto cleanup;

- if (VIR_APPEND_ELEMENT(*stats, *nstats, *stat) < 0)
+ if (VIR_APPEND_ELEMENT(*stats, *nstats, stat) < 0)
goto cleanup;
}

@@ -2762,7 +2762,7 @@ virResctrlMonitorGetStats(virResctrlMonitorPtr monitor,

int
virResctrlMonitorGetCacheOccupancy(virResctrlMonitorPtr monitor,
- virResctrlMonitorStatsPtr *stats,
+ virResctrlMonitorStatsPtr **stats,
size_t *nstats)
{
return virResctrlMonitorGetStats(monitor, "llc_occupancy",
diff --git a/src/util/virresctrl.h b/src/util/virresctrl.h
index 45ec967..e2ed4ee 100644
--- a/src/util/virresctrl.h
+++ b/src/util/virresctrl.h
@@ -229,6 +229,6 @@ virResctrlMonitorRemove(virResctrlMonitorPtr monitor);

int
virResctrlMonitorGetCacheOccupancy(virResctrlMonitorPtr monitor,
- virResctrlMonitorStatsPtr *caches,
- size_t *ncaches);
+ virResctrlMonitorStatsPtr **stats,
+ size_t *nstats);
#endif /* __VIR_RESCTRL_H__ */

--
2.7.4

Wang Huaqiang

2018-11-26 17:56:15 UTC

Permalink

The call of virResctrlMonitorGetStats will allocate the memory for
holding cache occupancy or memory bandwidth statistics.

This patch added an function, virResctrlMonitorFreeStats, as the
opposing action of virResctrlMonitorGetStats to free these memory.

Signed-off-by: Wang Huaqiang <***@intel.com>
---
src/libvirt_private.syms | 1 +
src/util/virresctrl.c | 16 ++++++++++++++++
src/util/virresctrl.h | 4 ++++
3 files changed, 21 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8889aaa..5018a13 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2692,6 +2692,7 @@ virResctrlInfoNew;
virResctrlMonitorAddPID;
virResctrlMonitorCreate;
virResctrlMonitorDeterminePath;
+virResctrlMonitorFreeStats;
virResctrlMonitorGetCacheOccupancy;
virResctrlMonitorGetID;
virResctrlMonitorNew;
diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index 3268310..6ffd71f 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -2747,6 +2747,22 @@ virResctrlMonitorGetStats(virResctrlMonitorPtr monitor,
}

+void
+virResctrlMonitorFreeStats(virResctrlMonitorStatsPtr *stats,
+ size_t nstats)
+{
+ size_t i = 0;
+
+ if (!stats)
+ return;
+
+ for (i = 0; i < nstats; i++)
+ VIR_FREE(stats[i]);
+
+ VIR_FREE(stats);
+}
+
+
/*
* virResctrlMonitorGetCacheOccupancy
*
diff --git a/src/util/virresctrl.h b/src/util/virresctrl.h
index e2ed4ee..8ea9758 100644
--- a/src/util/virresctrl.h
+++ b/src/util/virresctrl.h
@@ -231,4 +231,8 @@ int
virResctrlMonitorGetCacheOccupancy(virResctrlMonitorPtr monitor,
virResctrlMonitorStatsPtr **stats,
size_t *nstats);
+
+void
+virResctrlMonitorFreeStats(virResctrlMonitorStatsPtr *stats,
+ size_t nstats);
#endif /* __VIR_RESCTRL_H__ */

--
2.7.4

Wang Huaqiang

2018-11-26 17:56:17 UTC

Permalink

Signed-off-by: Wang Huaqiang <***@intel.com>
---
docs/news.xml | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index 4406aeb..deadb85 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -68,6 +68,18 @@
be viewed via the domain statistics.
</description>
</change>
+ <change>
+ <summary>
+ qemu: Added support for CMT (Cache Monitoring Technology)
+ </summary>
+ <description>
+ Introduced cache monitoring using the <code>monitor</code>
+ element in <code>cachetune</code> for vCPU threads. Added
+ interfaces to get and display the cache utilization statistics
+ through the command 'virsh domstats' via the
+ virConnectGetAllDomainStats API.
+ </description>
+ </change>
</section>
<section title="Improvements">
</section>

--
2.7.4

Wang Huaqiang

2018-11-26 17:56:16 UTC

Permalink

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 <***@intel.com>
---
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)
+ qemuDomainFreeResctrlMonData(&res, 1);
+
+ 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);
+ 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:

--
2.7.4

John Ferlan

2018-11-26 21:05:21 UTC

Permalink

Post by Wang Huaqiang
Adding the interface in qemu to report CMT statistic information
through command 'virsh domstats --cpu-total'.
# 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
---
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>
*
* 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);
+}
+
+
+/**
+ * virQEMUResctrlMonDataPtr array. Caller is responsible for
+ * freeing the array.
+ * returns an error, the caller is also required to call
+ * getting memory bandwidth statistics from memory bandwidth 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;
+
+ if (res)

Cannot get here with res == NULL

Post by Wang Huaqiang
+ 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...

Post by Wang Huaqiang
+
+ 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;
+ 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.

Post by Wang Huaqiang
+ 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
"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>

John Ferlan

2018-11-27 01:48:52 UTC

Permalink

These patches are the remaining part for the CMT enabling series,
and most of the series have been merged.
This series is addressing John's review comments and suggestions,
which are
https://www.redhat.com/archives/libvir-list/2018-November/msg00907.html
https://www.redhat.com/archives/libvir-list/2018-November/msg00510.html
https://www.redhat.com/archives/libvir-list/2018-November/msg00561.html
-. Add tag (virResctrlMonitorType) in qemuDomainGetResctrlMonData, thus
qemuDomainGetResctrlMonData could be reused for MBM.
-. Using VIR_APPEND_ELEMENT to append virQEMUResctrlMonDataPtr list.
-. Add qemuDomainFreeResctrlMonData.
-. Add virResctrlMonitorFreeStats.
-. Return a list of virResctrlMonitorStatsPtr instead of
a virResctrlMonitorStats array in virResctrlMonitorGetStats.
-. Addressing code review comments form John.
-. Refined the names for new data structure and new functions.
-. Merged qemuDomainGetStatsCpuResMonitorPerTag and qemuDomainGetStatsCpuResMonitor,
and refined new function name based on the fact that we only support cache monitor now.
util: Return a list of pointer in virResctrlMonitorGetStats
util: Add function to free monitor statistical data
qemu: Report cache occupancy (CMT) with domstats
docs: Updated news.xml for CMT
docs/news.xml | 12 ++++
src/libvirt-domain.c | 12 ++++
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 183 ++++++++++++++++++++++++++++++++++++++++++++++-
src/util/virresctrl.c | 26 +++++--
src/util/virresctrl.h | 8 ++-
tools/virsh.pod | 14 ++++
7 files changed, 248 insertions(+), 8 deletions(-)

Reviewed-by: John Ferlan <***@redhat.com>
(series)

and pushed,

John

Wang, Huaqiang

2018-11-27 02:39:01 UTC

Permalink

Hi John,

Really appreciate your hard work for the CMT series. Next I'll working on the MBM.

In testing the newly pushed code, I find a problem:

<error message>
[***@dl-c200 ~]$ sudo virsh domstats
error: An error occurred, but the cause is unknown
</error message>

seems it is caused by qemuDomainGetStatsIOThread not by the new series.
What I test is return 0 immediately at top of qemuDomainGetStatsIOThread, the
command 'virsh domstats' reports the cache statistics normally.

BR
Huaqiang

-----Original Message-----
Sent: Tuesday, November 27, 2018 9:49 AM
Subject: Re: [PATCHv10 0/4] Introduce x86 Cache Monitoring Technology
(CMT)

November/msg00907.htm

l
https://www.redhat.com/archives/libvir-list/2018-

November/msg00510.htm

l
https://www.redhat.com/archives/libvir-list/2018-

November/msg00561.htm

l
-. Add tag (virResctrlMonitorType) in qemuDomainGetResctrlMonData,

thus

qemuDomainGetResctrlMonData could be reused for MBM.
-. Using VIR_APPEND_ELEMENT to append virQEMUResctrlMonDataPtr

list.

-. Add qemuDomainFreeResctrlMonData.
-. Add virResctrlMonitorFreeStats.
-. Return a list of virResctrlMonitorStatsPtr instead of
a virResctrlMonitorStats array in virResctrlMonitorGetStats.
-. Addressing code review comments form John.
-. Refined the names for new data structure and new functions.
-. Merged qemuDomainGetStatsCpuResMonitorPerTag and

qemuDomainGetStatsCpuResMonitor,

and refined new function name based on the fact that we only support

cache monitor now.

util: Return a list of pointer in virResctrlMonitorGetStats
util: Add function to free monitor statistical data
qemu: Report cache occupancy (CMT) with domstats
docs: Updated news.xml for CMT
docs/news.xml | 12 ++++
src/libvirt-domain.c | 12 ++++
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 183

++++++++++++++++++++++++++++++++++++++++++++++-

src/util/virresctrl.c | 26 +++++--
src/util/virresctrl.h | 8 ++-
tools/virsh.pod | 14 ++++
7 files changed, 248 insertions(+), 8 deletions(-)

(series)
and pushed,
John

John Ferlan

2018-11-27 02:58:58 UTC

Permalink

Post by Wang, Huaqiang
Hi John,
Really appreciate your hard work for the CMT series. Next I'll working on the MBM.
<error message>
error: An error occurred, but the cause is unknown
</error message>

I couldn't reproduced in a quick test here. Can you get a thread trace
of the failure?

What I usually do, build libvirt, then in a terminal session at the top
of the git tree "./run gdb src/libvirtd" (dbg> r)... THen in another
terminal session run the virsh command and when the libvirtd session
stops do a "t a a bt" (thread apply all backtrace)...

John

(done for the night)

Post by Wang, Huaqiang
seems it is caused by qemuDomainGetStatsIOThread not by the new series.
What I test is return 0 immediately at top of qemuDomainGetStatsIOThread, the
command 'virsh domstats' reports the cache statistics normally.
BR
Huaqiang

-----Original Message-----
Sent: Tuesday, November 27, 2018 9:49 AM
Subject: Re: [PATCHv10 0/4] Introduce x86 Cache Monitoring Technology
(CMT)

November/msg00907.htm

l
https://www.redhat.com/archives/libvir-list/2018-

November/msg00510.htm

l
https://www.redhat.com/archives/libvir-list/2018-

November/msg00561.htm

l
-. Add tag (virResctrlMonitorType) in qemuDomainGetResctrlMonData,

thus

qemuDomainGetResctrlMonData could be reused for MBM.
-. Using VIR_APPEND_ELEMENT to append virQEMUResctrlMonDataPtr

list.

qemuDomainGetStatsCpuResMonitor,

and refined new function name based on the fact that we only support

cache monitor now.

++++++++++++++++++++++++++++++++++++++++++++++-

src/util/virresctrl.c | 26 +++++--
src/util/virresctrl.h | 8 ++-
tools/virsh.pod | 14 ++++
7 files changed, 248 insertions(+), 8 deletions(-)

(series)
and pushed,
John

Wang, Huaqiang

2018-11-27 03:20:39 UTC

Permalink

-----Original Message-----
Sent: Tuesday, November 27, 2018 10:59 AM
Subject: Re: [PATCHv10 0/4] Introduce x86 Cache Monitoring Technology
(CMT)

Post by Wang, Huaqiang
Hi John,
Really appreciate your hard work for the CMT series. Next I'll working on

the MBM.

Post by Wang, Huaqiang
<error message>
error: An error occurred, but the cause is unknown </error message>

I couldn't reproduced in a quick test here. Can you get a thread trace of the
failure?
What I usually do, build libvirt, then in a terminal session at the top of the
git tree "./run gdb src/libvirtd" (dbg> r)... THen in another terminal session
run the virsh command and when the libvirtd session stops do a "t a a bt"
(thread apply all backtrace)...
John

I'll trace the error.

Thanks.
Huaqiang

(done for the night)

Post by Wang, Huaqiang
seems it is caused by qemuDomainGetStatsIOThread not by the new

series.

Post by Wang, Huaqiang
What I test is return 0 immediately at top of
qemuDomainGetStatsIOThread, the command 'virsh domstats' reports

the cache statistics normally.

Post by Wang, Huaqiang
BR
Huaqiang

-----Original Message-----
Sent: Tuesday, November 27, 2018 9:49 AM
Subject: Re: [PATCHv10 0/4] Introduce x86 Cache Monitoring

Technology

Post by Wang, Huaqiang

(CMT)

November/msg00907.htm

l
https://www.redhat.com/archives/libvir-list/2018-

November/msg00510.htm

l
https://www.redhat.com/archives/libvir-list/2018-

November/msg00561.htm

l
-. Add tag (virResctrlMonitorType) in qemuDomainGetResctrlMonData,

thus

qemuDomainGetResctrlMonData could be reused for MBM.
-. Using VIR_APPEND_ELEMENT to append virQEMUResctrlMonDataPtr

list.

qemuDomainGetStatsCpuResMonitor,

and refined new function name based on the fact that we only support

cache monitor now.

++++++++++++++++++++++++++++++++++++++++++++++-

src/util/virresctrl.c | 26 +++++--
src/util/virresctrl.h | 8 ++-
tools/virsh.pod | 14 ++++
7 files changed, 248 insertions(+), 8 deletions(-)

(series)
and pushed,
John

Wang, Huaqiang

2018-11-27 13:28:09 UTC

Permalink

Hi John,

The issue mentioned is generated by a -1 returned by qemuDomainGetIOThreadsMon.

Further, it is caused by a -1 returned by virQEMUCapsGet:
6174 if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) {
6175 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
6176 _("IOThreads not supported with this binary"));
6177 goto endjob;
6178 }

By checking qemuCaps->flags, it seems the bit of QEMU_CAPS_OBJECT_IOTHREAD(176) is not set.
(gdb) call virBitmapFormat(qemuCaps->flags)
$4 = 0x7f08a4004cb0 "13,33,46,50,54-55,58,61-64,66-70,72-73,77-78,80,87-88,92-93,95-97,99,102-110,112,114,119-121,128-131,141-142,146,148-149,151-156,158-161,165,167,174-175,180,182,188,193-198,210,214,221-222,225,250,255"...

Breakpoint 1 at 0x7f088ef00880: file qemu/qemu_driver.c, line 5492.
(gdb) c
Continuing.
[Switching to Thread 0x7f08bcdcb700 (LWP 24711)]

Breakpoint 1, qemuDomainGetIOThreadsMon (driver=0x7f0874049b80, vm=0x7f08740cd840, iothreads=0x7f08bcdca8b8) at qemu/qemu_driver.c:5492
5492 {
Unprocessed breakpoint [1]
(gdb) finish
Run till exit from #0 qemuDomainGetIOThreadsMon (driver=0x7f0874049b80, vm=0x7f08740cd840, iothreads=0x7f08bcdca8b8) at qemu/qemu_driver.c:5492
0x00007f088ef0098f in qemuDomainGetStatsIOThread (driver=<optimized out>, dom=<optimized out>, record=0x7f08ac000b60, maxparams=0x7f08bcdca9e4, privflags=<optimized out>) at qemu/qemu_driver.c:20885
20885 if ((niothreads = qemuDomainGetIOThreadsMon(driver, dom, &iothreads)) < 0)
Value returned is $1 = -1
(gdb) bt
#0 0x00007f088ef0098f in qemuDomainGetStatsIOThread (driver=<optimized out>, dom=<optimized out>, record=0x7f08ac000b60, maxparams=0x7f08bcdca9e4, privflags=<optimized out>) at qemu/qemu_driver.c:20885
#1 0x00007f088eee70a9 in qemuDomainGetStats (flags=1, record=<synthetic pointer>, stats=255, dom=0x7f08740cd840, conn=0x7f08ac000c50) at qemu/qemu_driver.c:21062
#2 qemuConnectGetAllDomainStats (conn=0x7f08ac000c50, doms=<optimized out>, ndoms=<optimized out>, stats=255, retStats=0x7f08bcdcab10, flags=<optimized out>) at qemu/qemu_driver.c:21162
#3 0x00007f08cd147646 in virConnectGetAllDomainStats (conn=0x7f08ac000c50, stats=0, retStats=***@entry=0x7f08bcdcab10, flags=0) at libvirt-domain.c:11685
#4 0x0000562e2404d2c0 in remoteDispatchConnectGetAllDomainStats (server=0x562e24ee74a0, msg=<optimized out>, ret=0x7f08ac001ec0, args=0x7f08ac001f30, rerr=0x7f08bcdcac50, client=0x562e24ef5200) at remote/remote_daemon_dispatch.c:6665
#5 remoteDispatchConnectGetAllDomainStatsHelper (server=0x562e24ee74a0, client=0x562e24ef5200, msg=<optimized out>, rerr=0x7f08bcdcac50, args=0x7f08ac001f30, ret=0x7f08ac001ec0) at remote/remote_daemon_dispatch_stubs.h:743
#6 0x00007f08cd05dad5 in virNetServerProgramDispatchCall (msg=0x562e24ef5910, client=0x562e24ef5200, server=0x562e24ee74a0, prog=0x562e24ee6650) at rpc/virnetserverprogram.c:437
#7 virNetServerProgramDispatch (prog=0x562e24ee6650, server=***@entry=0x562e24ee74a0, client=0x562e24ef5200, msg=0x562e24ef5910) at rpc/virnetserverprogram.c:304
#8 0x00007f08cd063f7d in virNetServerProcessMsg (msg=<optimized out>, prog=<optimized out>, client=<optimized out>, srv=0x562e24ee74a0) at rpc/virnetserver.c:144
#9 virNetServerHandleJob (jobOpaque=<optimized out>, opaque=0x562e24ee74a0) at rpc/virnetserver.c:165
#10 0x00007f08ccf95171 in virThreadPoolWorker (opaque=***@entry=0x562e24ee69a0) at util/virthreadpool.c:167
#11 0x00007f08ccf944f8 in virThreadHelper (data=<optimized out>) at util/virthread.c:206
#12 0x00007f08cb213dc5 in start_thread (arg=0x7f08bcdcb700) at pthread_create.c:308
#13 0x00007f08cab3b76d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:113
(gdb)

My domain configuration is：
[***@dl-c200 libvirt]# virsh list
v^H Id Name State
----------------------------------
1 ubuntu16.04-base running

[***@dl-c200 libvirt]# virsh dumpxml 1
<domain type='kvm' id='1'>
<name>ubuntu16.04-base</name>
<uuid>19d01bd1-ad54-4176-84bd-e510de08fe00</uuid>
<memory unit='KiB'>10485760</memory>
<currentMemory unit='KiB'>10485760</currentMemory>
<vcpu placement='static' current='4'>8</vcpu>
<cputune>
<cachetune vcpus='0' id='vcpus_0'>
<monitor level='3' vcpus='0'/>
</cachetune>
</cputune>
<resource>
<partition>/machine</partition>
</resource>
<os>
<type arch='x86_64' machine='pc-i440fx-rhel7.0.0'>hvm</type>
<boot dev='hd'/>
</os>
<features>
<acpi/>
<apic/>
</features>
<cpu mode='custom' match='exact' check='full'>
<model fallback='forbid'>Broadwell</model>
<feature policy='require' name='hypervisor'/>
<feature policy='require' name='xsaveopt'/>
</cpu>
<clock offset='utc'>
<timer name='rtc' tickpolicy='catchup'/>
<timer name='pit' tickpolicy='delay'/>
<timer name='hpet' present='no'/>
</clock>
<on_poweroff>destroy</on_poweroff>
<on_reboot>restart</on_reboot>
<on_crash>destroy</on_crash>
<pm>
<suspend-to-mem enabled='no'/>
<suspend-to-disk enabled='no'/>
</pm>
<devices>
<emulator>/usr/libexec/qemu-kvm</emulator>
<disk type='file' device='disk'>
<driver name='qemu' type='qcow2'/>
<source file='/tmp/kvm-donot-delete/vm-ubuntu16.04-base'/>
<backingStore/>
<target dev='vda' bus='virtio'/>
<alias name='virtio-disk0'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/>
</disk>
<disk type='file' device='cdrom'>
<driver name='qemu'/>
<target dev='hda' bus='ide'/>
<readonly/>
<alias name='ide0-0-0'/>
<address type='drive' controller='0' bus='0' target='0' unit='0'/>
</disk>
<controller type='usb' index='0' model='ich9-ehci1'>
<alias name='usb'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x7'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci1'>
<alias name='usb'/>
<master startport='0'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0' multifunction='on'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci2'>
<alias name='usb'/>
<master startport='2'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x1'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci3'>
<alias name='usb'/>
<master startport='4'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x2'/>
</controller>
<controller type='pci' index='0' model='pci-root'>
<alias name='pci.0'/>
</controller>
<controller type='ide' index='0'>
<alias name='ide'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
</controller>
<interface type='bridge'>
<mac address='52:54:00:eb:24:d3'/>
<source bridge='virbr0'/>
<target dev='vnet0'/>
<model type='virtio'/>
<alias name='net0'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
</interface>
<serial type='pty'>
<source path='/dev/pts/1'/>
<target type='isa-serial' port='0'>
<model name='isa-serial'/>
</target>
<alias name='serial0'/>
</serial>
<console type='pty' tty='/dev/pts/1'>
<source path='/dev/pts/1'/>
<target type='serial' port='0'/>
<alias name='serial0'/>
</console>
<input type='mouse' bus='ps2'>
<alias name='input0'/>
</input>
<input type='keyboard' bus='ps2'>
<alias name='input1'/>
</input>
<graphics type='vnc' port='5901' autoport='yes' listen='127.0.0.1'>
<listen type='address' address='127.0.0.1'/>
</graphics>
<video>
<model type='cirrus' vram='16384' heads='1' primary='yes'/>
<alias name='video0'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
</video>
<memballoon model='virtio'>
<alias name='balloon0'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/>
</memballoon>
</devices>
<seclabel type='dynamic' model='selinux' relabel='yes'>
<label>system_u:system_r:svirt_t:s0:c157,c912</label>
<imagelabel>system_u:object_r:svirt_image_t:s0:c157,c912</imagelabel>
</seclabel>
<seclabel type='dynamic' model='dac' relabel='yes'>
<label>+0:+0</label>
<imagelabel>+0:+0</imagelabel>
</seclabel>
</domain>

I wonder if these message is enough for you to locate the root cause, I don’t have too much time in investigating this today, I'll try to create a fix tomorrow if you need.

Huaqiang

-----Original Message-----
From: Wang, Huaqiang
Sent: Tuesday, November 27, 2018 11:21 AM
Subject: RE: [PATCHv10 0/4] Introduce x86 Cache Monitoring Technology
(CMT)

-----Original Message-----
Sent: Tuesday, November 27, 2018 10:59 AM
Subject: Re: [PATCHv10 0/4] Introduce x86 Cache Monitoring Technology
(CMT)

Post by Wang, Huaqiang
Hi John,
Really appreciate your hard work for the CMT series. Next I'll
working on

the MBM.

Post by Wang, Huaqiang
<error message>
error: An error occurred, but the cause is unknown </error message>

I couldn't reproduced in a quick test here. Can you get a thread trace
of the failure?
What I usually do, build libvirt, then in a terminal session at the
top of the git tree "./run gdb src/libvirtd" (dbg> r)... THen in
another terminal session run the virsh command and when the libvirtd

session stops do a "t a a bt"

(thread apply all backtrace)...
John

I'll trace the error.
Thanks.
Huaqiang

(done for the night)

Post by Wang, Huaqiang
seems it is caused by qemuDomainGetStatsIOThread not by the new

series.

Post by Wang, Huaqiang
What I test is return 0 immediately at top of
qemuDomainGetStatsIOThread, the command 'virsh domstats' reports

the cache statistics normally.

Post by Wang, Huaqiang
BR
Huaqiang

-----Original Message-----
Sent: Tuesday, November 27, 2018 9:49 AM
Subject: Re: [PATCHv10 0/4] Introduce x86 Cache Monitoring

Technology

Post by Wang, Huaqiang

(CMT)

November/msg00907.htm

l
https://www.redhat.com/archives/libvir-list/2018-

November/msg00510.htm

l
https://www.redhat.com/archives/libvir-list/2018-

November/msg00561.htm

l
-. Add tag (virResctrlMonitorType) in

qemuDomainGetResctrlMonData,

Post by Wang, Huaqiang

thus

qemuDomainGetResctrlMonData could be reused for MBM.
-. Using VIR_APPEND_ELEMENT to append

virQEMUResctrlMonDataPtr

Post by Wang, Huaqiang

list.

qemuDomainGetStatsCpuResMonitor,

and refined new function name based on the fact that we only support

cache monitor now.

++++++++++++++++++++++++++++++++++++++++++++++-

src/util/virresctrl.c | 26 +++++--
src/util/virresctrl.h | 8 ++-
tools/virsh.pod | 14 ++++
7 files changed, 248 insertions(+), 8 deletions(-)

(series)
and pushed,
John

John Ferlan

2018-11-27 16:05:53 UTC

Permalink

Post by Wang, Huaqiang
Hi John,
The issue mentioned is generated by a -1 returned by qemuDomainGetIOThreadsMon.
6174 if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) {
6175 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
6176 _("IOThreads not supported with this binary"));
6177 goto endjob;
6178 }
By checking qemuCaps->flags, it seems the bit of QEMU_CAPS_OBJECT_IOTHREAD(176) is not set.
(gdb) call virBitmapFormat(qemuCaps->flags)
$4 = 0x7f08a4004cb0 "13,33,46,50,54-55,58,61-64,66-70,72-73,77-78,80,87-88,92-93,95-97,99,102-110,112,114,119-121,128-131,141-142,146,148-149,151-156,158-161,165,167,174-175,180,182,188,193-198,210,214,221-222,225,250,255"...

Hmm... Thanks for the details...

This is an "interesting" failure. There's a couple of bugs I suppose.
One simple to fix - the qemuConnectGetAllDomainStats should have saved
the error, but the call to virDomainStatsRecordListFree wipes out the
error when calling virDomainFree (via virResetLastError) when there's
more than one domain's worth of stats being collect and "some" domains
had already succeeded. So saving that error across the failure is
possible via a couple of means.

The other part of this would be a change to qemuDomainGetStatsIOThread
to not fail when the cap isn't available, but yet still allow a failure
if the cap was there, but the mon call failed.

As an aside, your QEMU must be fairly "old" to fail on the lack of the
capability QEMU_CAPS_OBJECT_IOTHREAD... Perhaps older than 2.0 - which
doesn't quite make sense given everything else.

I'll post a couple of patches shortly...

Tks,

John

Post by Wang, Huaqiang
Breakpoint 1 at 0x7f088ef00880: file qemu/qemu_driver.c, line 5492.
(gdb) c
Continuing.
[Switching to Thread 0x7f08bcdcb700 (LWP 24711)]
Breakpoint 1, qemuDomainGetIOThreadsMon (driver=0x7f0874049b80, vm=0x7f08740cd840, iothreads=0x7f08bcdca8b8) at qemu/qemu_driver.c:5492
5492 {
Unprocessed breakpoint [1]
(gdb) finish
Run till exit from #0 qemuDomainGetIOThreadsMon (driver=0x7f0874049b80, vm=0x7f08740cd840, iothreads=0x7f08bcdca8b8) at qemu/qemu_driver.c:5492
0x00007f088ef0098f in qemuDomainGetStatsIOThread (driver=<optimized out>, dom=<optimized out>, record=0x7f08ac000b60, maxparams=0x7f08bcdca9e4, privflags=<optimized out>) at qemu/qemu_driver.c:20885
20885 if ((niothreads = qemuDomainGetIOThreadsMon(driver, dom, &iothreads)) < 0)
Value returned is $1 = -1
(gdb) bt
#0 0x00007f088ef0098f in qemuDomainGetStatsIOThread (driver=<optimized out>, dom=<optimized out>, record=0x7f08ac000b60, maxparams=0x7f08bcdca9e4, privflags=<optimized out>) at qemu/qemu_driver.c:20885
#1 0x00007f088eee70a9 in qemuDomainGetStats (flags=1, record=<synthetic pointer>, stats=255, dom=0x7f08740cd840, conn=0x7f08ac000c50) at qemu/qemu_driver.c:21062
#2 qemuConnectGetAllDomainStats (conn=0x7f08ac000c50, doms=<optimized out>, ndoms=<optimized out>, stats=255, retStats=0x7f08bcdcab10, flags=<optimized out>) at qemu/qemu_driver.c:21162
#4 0x0000562e2404d2c0 in remoteDispatchConnectGetAllDomainStats (server=0x562e24ee74a0, msg=<optimized out>, ret=0x7f08ac001ec0, args=0x7f08ac001f30, rerr=0x7f08bcdcac50, client=0x562e24ef5200) at remote/remote_daemon_dispatch.c:6665
#5 remoteDispatchConnectGetAllDomainStatsHelper (server=0x562e24ee74a0, client=0x562e24ef5200, msg=<optimized out>, rerr=0x7f08bcdcac50, args=0x7f08ac001f30, ret=0x7f08ac001ec0) at remote/remote_daemon_dispatch_stubs.h:743
#6 0x00007f08cd05dad5 in virNetServerProgramDispatchCall (msg=0x562e24ef5910, client=0x562e24ef5200, server=0x562e24ee74a0, prog=0x562e24ee6650) at rpc/virnetserverprogram.c:437
#8 0x00007f08cd063f7d in virNetServerProcessMsg (msg=<optimized out>, prog=<optimized out>, client=<optimized out>, srv=0x562e24ee74a0) at rpc/virnetserver.c:144
#9 virNetServerHandleJob (jobOpaque=<optimized out>, opaque=0x562e24ee74a0) at rpc/virnetserver.c:165
#11 0x00007f08ccf944f8 in virThreadHelper (data=<optimized out>) at util/virthread.c:206
#12 0x00007f08cb213dc5 in start_thread (arg=0x7f08bcdcb700) at pthread_create.c:308
#13 0x00007f08cab3b76d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:113
(gdb)
My domain configuration is：
v^H Id Name State
----------------------------------
1 ubuntu16.04-base running
<domain type='kvm' id='1'>
<name>ubuntu16.04-base</name>
<uuid>19d01bd1-ad54-4176-84bd-e510de08fe00</uuid>
<memory unit='KiB'>10485760</memory>
<currentMemory unit='KiB'>10485760</currentMemory>
<vcpu placement='static' current='4'>8</vcpu>
<cputune>
<cachetune vcpus='0' id='vcpus_0'>
<monitor level='3' vcpus='0'/>
</cachetune>
</cputune>
<resource>
<partition>/machine</partition>
</resource>
<os>
<type arch='x86_64' machine='pc-i440fx-rhel7.0.0'>hvm</type>
<boot dev='hd'/>
</os>
<features>
<acpi/>
<apic/>
</features>
<cpu mode='custom' match='exact' check='full'>
<model fallback='forbid'>Broadwell</model>
<feature policy='require' name='hypervisor'/>
<feature policy='require' name='xsaveopt'/>
</cpu>
<clock offset='utc'>
<timer name='rtc' tickpolicy='catchup'/>
<timer name='pit' tickpolicy='delay'/>
<timer name='hpet' present='no'/>
</clock>
<on_poweroff>destroy</on_poweroff>
<on_reboot>restart</on_reboot>
<on_crash>destroy</on_crash>
<pm>
<suspend-to-mem enabled='no'/>
<suspend-to-disk enabled='no'/>
</pm>
<devices>
<emulator>/usr/libexec/qemu-kvm</emulator>
<disk type='file' device='disk'>
<driver name='qemu' type='qcow2'/>
<source file='/tmp/kvm-donot-delete/vm-ubuntu16.04-base'/>
<backingStore/>
<target dev='vda' bus='virtio'/>
<alias name='virtio-disk0'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/>
</disk>
<disk type='file' device='cdrom'>
<driver name='qemu'/>
<target dev='hda' bus='ide'/>
<readonly/>
<alias name='ide0-0-0'/>
<address type='drive' controller='0' bus='0' target='0' unit='0'/>
</disk>
<controller type='usb' index='0' model='ich9-ehci1'>
<alias name='usb'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x7'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci1'>
<alias name='usb'/>
<master startport='0'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0' multifunction='on'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci2'>
<alias name='usb'/>
<master startport='2'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x1'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci3'>
<alias name='usb'/>
<master startport='4'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x2'/>
</controller>
<controller type='pci' index='0' model='pci-root'>
<alias name='pci.0'/>
</controller>
<controller type='ide' index='0'>
<alias name='ide'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
</controller>
<interface type='bridge'>
<mac address='52:54:00:eb:24:d3'/>
<source bridge='virbr0'/>
<target dev='vnet0'/>
<model type='virtio'/>
<alias name='net0'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
</interface>
<serial type='pty'>
<source path='/dev/pts/1'/>
<target type='isa-serial' port='0'>
<model name='isa-serial'/>
</target>
<alias name='serial0'/>
</serial>
<console type='pty' tty='/dev/pts/1'>
<source path='/dev/pts/1'/>
<target type='serial' port='0'/>
<alias name='serial0'/>
</console>
<input type='mouse' bus='ps2'>
<alias name='input0'/>
</input>
<input type='keyboard' bus='ps2'>
<alias name='input1'/>
</input>
<graphics type='vnc' port='5901' autoport='yes' listen='127.0.0.1'>
<listen type='address' address='127.0.0.1'/>
</graphics>
<video>
<model type='cirrus' vram='16384' heads='1' primary='yes'/>
<alias name='video0'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
</video>
<memballoon model='virtio'>
<alias name='balloon0'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/>
</memballoon>
</devices>
<seclabel type='dynamic' model='selinux' relabel='yes'>
<label>system_u:system_r:svirt_t:s0:c157,c912</label>
<imagelabel>system_u:object_r:svirt_image_t:s0:c157,c912</imagelabel>
</seclabel>
<seclabel type='dynamic' model='dac' relabel='yes'>
<label>+0:+0</label>
<imagelabel>+0:+0</imagelabel>
</seclabel>
</domain>
I wonder if these message is enough for you to locate the root cause, I don’t have too much time in investigating this today, I'll try to create a fix tomorrow if you need.
Huaqiang

-----Original Message-----
From: Wang, Huaqiang
Sent: Tuesday, November 27, 2018 11:21 AM
Subject: RE: [PATCHv10 0/4] Introduce x86 Cache Monitoring Technology
(CMT)

-----Original Message-----
Sent: Tuesday, November 27, 2018 10:59 AM
Subject: Re: [PATCHv10 0/4] Introduce x86 Cache Monitoring Technology
(CMT)

Post by Wang, Huaqiang
Hi John,
Really appreciate your hard work for the CMT series. Next I'll
working on

the MBM.

Post by Wang, Huaqiang
<error message>
error: An error occurred, but the cause is unknown </error message>

I couldn't reproduced in a quick test here. Can you get a thread trace
of the failure?
What I usually do, build libvirt, then in a terminal session at the
top of the git tree "./run gdb src/libvirtd" (dbg> r)... THen in
another terminal session run the virsh command and when the libvirtd

session stops do a "t a a bt"

(thread apply all backtrace)...
John

I'll trace the error.
Thanks.
Huaqiang

(done for the night)

Post by Wang, Huaqiang
seems it is caused by qemuDomainGetStatsIOThread not by the new

series.

Post by Wang, Huaqiang
What I test is return 0 immediately at top of
qemuDomainGetStatsIOThread, the command 'virsh domstats' reports

the cache statistics normally.

Post by Wang, Huaqiang
BR
Huaqiang

-----Original Message-----
Sent: Tuesday, November 27, 2018 9:49 AM
Subject: Re: [PATCHv10 0/4] Introduce x86 Cache Monitoring

Technology

Post by Wang, Huaqiang

(CMT)

November/msg00907.htm

l
https://www.redhat.com/archives/libvir-list/2018-

November/msg00510.htm

l
https://www.redhat.com/archives/libvir-list/2018-

November/msg00561.htm

l
-. Add tag (virResctrlMonitorType) in

qemuDomainGetResctrlMonData,

Post by Wang, Huaqiang

thus

qemuDomainGetResctrlMonData could be reused for MBM.
-. Using VIR_APPEND_ELEMENT to append

virQEMUResctrlMonDataPtr

Post by Wang, Huaqiang

list.

qemuDomainGetStatsCpuResMonitor,

and refined new function name based on the fact that we only support

cache monitor now.

++++++++++++++++++++++++++++++++++++++++++++++-

src/util/virresctrl.c | 26 +++++--
src/util/virresctrl.h | 8 ++-
tools/virsh.pod | 14 ++++
7 files changed, 248 insertions(+), 8 deletions(-)

(series)
and pushed,
John