On 09/15/14 10: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@redhat.com> --- include/libvirt/libvirt.h.in | 1 + src/libvirt.c | 20 +++++++ src/qemu/qemu_driver.c | 81 ++++++++++++++++++++++++++ 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, 245 insertions(+), 43 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 016499d..446b04b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9664,6 +9664,7 @@ qemuDomainBlockStats(virDomainPtr dom, return ret; }
+
I've just noticed this spurious newline.
static int qemuDomainBlockStatsFlags(virDomainPtr dom, const char *path, @@ -17621,6 +17622,85 @@ 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;
nstats is here initialized to 0.
+ qemuBlockStatsPtr stats = NULL; + qemuDomainObjPrivatePtr priv = dom->privateData; + + 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; + + qemuDomainObjEnterMonitor(driver, dom); + + nstats = qemuMonitorGetAllBlockStatsInfo(priv->mon, NULL, + stats, nstats);
and not touched, thus it's still 0 when called here, which ...
+ + qemuDomainObjExitMonitor(driver, dom); + + if (nstats < 0) { + virResetLastError(); + ret = 0; /* still ok, again go ahead silently */ + 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
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 0c4832a..d45c41f 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c
+ +int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, + const char *dev_name, + qemuBlockStatsPtr bstats, + int nstats) +{ + int ret, count; + size_t i; + virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-blockstats", + NULL); + virJSONValuePtr reply = NULL; + virJSONValuePtr devices; + if (!cmd) return -1;
+ if (!bstats || nstats <= 0) + return -1;
.. returns failure here.
+ ret = qemuMonitorJSONCommand(mon, cmd, &reply);
if (ret == 0)
nstats needs to be set to the disk count, otherwise this stats group won't work. Peter