On 09/12/14 13:48, Francesco Romani wrote:
This patch implements the VIR_DOMAIN_STATS_BLOCK
group of statistics.
To do so, an helper function to get the block stats
of all the disks of a domain is added.
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
include/libvirt/libvirt.h.in | 1 +
src/libvirt.c | 20 +++++++
src/qemu/qemu_driver.c | 96 ++++++++++++++++++++++++++++++
src/qemu/qemu_monitor.c | 26 +++++++++
src/qemu/qemu_monitor.h | 20 +++++++
src/qemu/qemu_monitor_json.c | 136 +++++++++++++++++++++++++++++--------------
src/qemu/qemu_monitor_json.h | 4 ++
7 files changed, 260 insertions(+), 43 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7e5d707..4644f4a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9687,6 +9687,31 @@ qemuDomainBlockStats(virDomainPtr dom,
return ret;
}
+
+/*
+ * Returns at most the first `nstats' stats, then stops.
+ * Returns the number of stats filled.
+ */
+static int
+qemuDomainHelperGetBlockStats(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ qemuBlockStatsPtr stats,
+ int nstats)
+{
+ int ret;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+
+ qemuDomainObjEnterMonitor(driver, vm);
+
+ ret = qemuMonitorGetAllBlockStatsInfo(priv->mon, NULL,
+ stats, nstats);
Humm, is it worth doing this helper? This pretty much can be inlined as
it has only one caller.
+
+ qemuDomainObjExitMonitor(driver, vm);
+
+ return ret;
+}
+
+
static int
qemuDomainBlockStatsFlags(virDomainPtr dom,
const char *path,
@@ -17541,6 +17566,76 @@ qemuDomainGetStatsInterface(virQEMUDriverPtr driver
ATTRIBUTE_UNUSED,
#undef QEMU_ADD_NET_PARAM
+/* expects a LL, but typed parameter must be ULL */
+#define QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, num, name, value) \
+do { \
+ char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \
+ snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \
+ "block.%zu.%s", num, name); \
+ if (value >= 0 && virTypedParamsAddULLong(&(record)->params, \
+ &(record)->nparams, \
+ maxparams, \
+ param_name, \
+ value) < 0) \
+ goto cleanup; \
+} while (0)
+
+static int
+qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
+ virDomainObjPtr dom,
+ virDomainStatsRecordPtr record,
+ int *maxparams,
+ unsigned int privflags)
+{
+ size_t i;
+ int ret = -1;
+ int nstats = 0;
+ qemuBlockStatsPtr stats = NULL;
+
+ if (!HAVE_MONITOR(privflags) || !virDomainObjIsActive(dom))
+ return 0; /* it's ok, just go ahead silently */
+
+ if (VIR_ALLOC_N(stats, dom->def->ndisks) < 0)
+ return -1;
+
+ nstats = qemuDomainHelperGetBlockStats(driver, dom, stats,
+ dom->def->ndisks);
+ if (nstats < 0)
Are we erroring out on block stats failure? Other statistics gatherers
just skip it if it's not available.
+ goto cleanup;
+
+ QEMU_ADD_COUNT_PARAM(record, maxparams, "block", dom->def->ndisks);
+
+ for (i = 0; i < nstats; i++) {
+ QEMU_ADD_NAME_PARAM(record, maxparams,
+ "block", i, dom->def->disks[i]->dst);
+
+ QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i,
+ "rd.reqs", stats[i].rd_req);
+ QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i,
+ "rd.bytes", stats[i].rd_bytes);
+ QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i,
+ "rd.times", stats[i].rd_total_times);
+ QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i,
+ "wr.reqs", stats[i].wr_req);
+ QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i,
+ "wr.bytes", stats[i].wr_bytes);
+ QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i,
+ "wr.times", stats[i].wr_total_times);
+ QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i,
+ "fl.reqs", stats[i].flush_req);
+ QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i,
+ "fl.times", stats[i].flush_total_times);
+ }
+
+ ret = 0;
+
+ cleanup:
+ VIR_FREE(stats);
+ return ret;
+}
+
+#undef QEMU_ADD_BLOCK_PARAM_LL
+
#undef QEMU_ADD_NAME_PARAM
#undef QEMU_ADD_COUNT_PARAM
Otherwise looks good.
Peter