All the setters are the same code except for parameter name and variable, so they can be converted to a macro to save a ton of duplicated code. --- src/qemu/qemu_driver.c | 78 +++++++++++--------------------------------------- 1 file changed, 16 insertions(+), 62 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ceceafa..e94275f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10575,7 +10575,6 @@ qemuDomainBlockStatsFlags(virDomainPtr dom, qemuDomainObjPrivatePtr priv; long long rd_req, rd_bytes, wr_req, wr_bytes, rd_total_times; long long wr_total_times, flush_req, flush_total_times, errs; - virTypedParameterPtr param; char *diskAlias = NULL; virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1); @@ -10656,73 +10655,28 @@ qemuDomainBlockStatsFlags(virDomainPtr dom, tmp = 0; ret = -1; - if (tmp < *nparams && wr_bytes != -1) { - param = &params[tmp]; - if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_WRITE_BYTES, - VIR_TYPED_PARAM_LLONG, wr_bytes) < 0) - goto endjob; - tmp++; +/* the following macro shall not be used with side effects statements */ +#define QEMU_BLOCK_STAT(VAR, NAME) \ + if (tmp < *nparams && (VAR) != -1) { \ + if (virTypedParameterAssign(params + tmp, NAME, VIR_TYPED_PARAM_LLONG,\ + (VAR)) < 0) \ + goto endjob; \ + tmp++; \ } - if (tmp < *nparams && wr_req != -1) { - param = &params[tmp]; - if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_WRITE_REQ, - VIR_TYPED_PARAM_LLONG, wr_req) < 0) - goto endjob; - tmp++; - } + QEMU_BLOCK_STAT(wr_bytes, VIR_DOMAIN_BLOCK_STATS_WRITE_BYTES); + QEMU_BLOCK_STAT(wr_req, VIR_DOMAIN_BLOCK_STATS_WRITE_REQ); - if (tmp < *nparams && rd_bytes != -1) { - param = &params[tmp]; - if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_READ_BYTES, - VIR_TYPED_PARAM_LLONG, rd_bytes) < 0) - goto endjob; - tmp++; - } + QEMU_BLOCK_STAT(rd_bytes, VIR_DOMAIN_BLOCK_STATS_READ_BYTES); + QEMU_BLOCK_STAT(rd_req, VIR_DOMAIN_BLOCK_STATS_READ_REQ); - if (tmp < *nparams && rd_req != -1) { - param = &params[tmp]; - if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_READ_REQ, - VIR_TYPED_PARAM_LLONG, rd_req) < 0) - goto endjob; - tmp++; - } + QEMU_BLOCK_STAT(flush_req, VIR_DOMAIN_BLOCK_STATS_FLUSH_REQ); - if (tmp < *nparams && flush_req != -1) { - param = &params[tmp]; - if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_FLUSH_REQ, - VIR_TYPED_PARAM_LLONG, flush_req) < 0) - goto endjob; - tmp++; - } + QEMU_BLOCK_STAT(wr_total_times, VIR_DOMAIN_BLOCK_STATS_WRITE_TOTAL_TIMES); + QEMU_BLOCK_STAT(rd_total_times, VIR_DOMAIN_BLOCK_STATS_READ_TOTAL_TIMES); + QEMU_BLOCK_STAT(flush_total_times, VIR_DOMAIN_BLOCK_STATS_FLUSH_TOTAL_TIMES); - if (tmp < *nparams && wr_total_times != -1) { - param = &params[tmp]; - if (virTypedParameterAssign(param, - VIR_DOMAIN_BLOCK_STATS_WRITE_TOTAL_TIMES, - VIR_TYPED_PARAM_LLONG, wr_total_times) < 0) - goto endjob; - tmp++; - } - - if (tmp < *nparams && rd_total_times != -1) { - param = &params[tmp]; - if (virTypedParameterAssign(param, - VIR_DOMAIN_BLOCK_STATS_READ_TOTAL_TIMES, - VIR_TYPED_PARAM_LLONG, rd_total_times) < 0) - goto endjob; - tmp++; - } - - if (tmp < *nparams && flush_total_times != -1) { - param = &params[tmp]; - if (virTypedParameterAssign(param, - VIR_DOMAIN_BLOCK_STATS_FLUSH_TOTAL_TIMES, - VIR_TYPED_PARAM_LLONG, - flush_total_times) < 0) - goto endjob; - tmp++; - } +#undef QEMU_BLOCK_STAT /* Field 'errs' is meaningless for QEMU, won't set it. */ -- 2.2.2

The error count statistic is not supported by qemu, so there's no need to pass the variables around if the result is ignored anyways. --- src/qemu/qemu_driver.c | 11 ++++++----- src/qemu/qemu_monitor.c | 9 +++------ src/qemu/qemu_monitor.h | 3 +-- src/qemu/qemu_monitor_json.c | 6 ++---- src/qemu/qemu_monitor_json.h | 3 +-- src/qemu/qemu_monitor_text.c | 5 ++--- src/qemu/qemu_monitor_text.h | 3 +-- tests/qemumonitorjsontest.c | 19 +++++++++---------- 8 files changed, 25 insertions(+), 34 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e94275f..06168b1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10536,6 +10536,9 @@ qemuDomainBlockStats(virDomainPtr dom, priv = vm->privateData; + /* qemu doesn't report the error count */ + stats->errs = -1; + qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorGetBlockStatsInfo(priv->mon, diskAlias, @@ -10546,8 +10549,7 @@ qemuDomainBlockStats(virDomainPtr dom, &stats->wr_bytes, NULL, NULL, - NULL, - &stats->errs); + NULL); if (qemuDomainObjExitMonitor(driver, vm) < 0) ret = -1; @@ -10574,7 +10576,7 @@ qemuDomainBlockStatsFlags(virDomainPtr dom, virDomainObjPtr vm; qemuDomainObjPrivatePtr priv; long long rd_req, rd_bytes, wr_req, wr_bytes, rd_total_times; - long long wr_total_times, flush_req, flush_total_times, errs; + long long wr_total_times, flush_req, flush_total_times; char *diskAlias = NULL; virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1); @@ -10643,8 +10645,7 @@ qemuDomainBlockStatsFlags(virDomainPtr dom, &wr_bytes, &wr_total_times, &flush_req, - &flush_total_times, - &errs); + &flush_total_times); if (qemuDomainObjExitMonitor(driver, vm) < 0) ret = -1; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 94495cd..24e87b7 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1817,8 +1817,7 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, long long *wr_bytes, long long *wr_total_times, long long *flush_req, - long long *flush_total_times, - long long *errs) + long long *flush_total_times) { int ret; VIR_DEBUG("mon=%p dev=%s", mon, dev_name); @@ -1836,8 +1835,7 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, wr_req, wr_bytes, wr_total_times, flush_req, - flush_total_times, - errs); + flush_total_times); else ret = qemuMonitorTextGetBlockStatsInfo(mon, dev_name, rd_req, rd_bytes, @@ -1845,8 +1843,7 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, wr_req, wr_bytes, wr_total_times, flush_req, - flush_total_times, - errs); + flush_total_times); return ret; } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 133d42d..72498b3 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -366,8 +366,7 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, long long *wr_bytes, long long *wr_total_times, long long *flush_req, - long long *flush_total_times, - long long *errs); + long long *flush_total_times); typedef struct _qemuBlockStats qemuBlockStats; typedef qemuBlockStats *qemuBlockStatsPtr; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 832f589..612553b 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1677,15 +1677,14 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, long long *wr_bytes, long long *wr_total_times, long long *flush_req, - long long *flush_total_times, - long long *errs) + long long *flush_total_times) { qemuBlockStats *stats; virHashTablePtr blockstats = NULL; int ret = -1; *rd_req = *rd_bytes = -1; - *wr_req = *wr_bytes = *errs = -1; + *wr_req = *wr_bytes = -1; if (rd_total_times) *rd_total_times = -1; @@ -1709,7 +1708,6 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, *rd_bytes = stats->rd_bytes; *wr_req = stats->wr_req; *wr_bytes = stats->wr_bytes; - *errs = -1; /* QEMU does not have this */ if (rd_total_times) *rd_total_times = stats->rd_total_times; diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 1da1a00..23589cf 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -80,8 +80,7 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, long long *wr_bytes, long long *wr_total_times, long long *flush_req, - long long *flush_total_times, - long long *errs); + long long *flush_total_times); int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, virHashTablePtr *ret_stats, bool backingChain); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 70aeaca..2de281f 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -847,8 +847,7 @@ int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, long long *wr_bytes, long long *wr_total_times, long long *flush_req, - long long *flush_total_times, - long long *errs) + long long *flush_total_times) { char *info = NULL; int ret = -1; @@ -872,7 +871,7 @@ int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, } *rd_req = *rd_bytes = -1; - *wr_req = *wr_bytes = *errs = -1; + *wr_req = *wr_bytes = -1; if (rd_total_times) *rd_total_times = -1; diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index f118a30..695ac28 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -69,8 +69,7 @@ int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, long long *wr_bytes, long long *wr_total_times, long long *flush_req, - long long *flush_total_times, - long long *errs); + long long *flush_total_times); int qemuMonitorTextGetBlockStatsParamsNumber(qemuMonitorPtr mon, int *nparams); int qemuMonitorTextGetBlockExtent(qemuMonitorPtr mon, diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index bd92e63..da9cd6c 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1438,7 +1438,7 @@ testQemuMonitorJSONqemuMonitorJSONGetBlockStatsInfo(const void *data) int ret = -1; long long rd_req, rd_bytes, rd_total_times; long long wr_req, wr_bytes, wr_total_times; - long long flush_req, flush_total_times, errs; + long long flush_req, flush_total_times; int nparams; unsigned long long extent; @@ -1552,7 +1552,7 @@ testQemuMonitorJSONqemuMonitorJSONGetBlockStatsInfo(const void *data) } #define CHECK(RD_REQ, RD_BYTES, RD_TOTAL_TIMES, WR_REQ, WR_BYTES, WR_TOTAL_TIMES, \ - FLUSH_REQ, FLUSH_TOTAL_TIMES, ERRS) \ + FLUSH_REQ, FLUSH_TOTAL_TIMES) \ CHECK0(rd_req, RD_REQ) \ CHECK0(rd_bytes, RD_BYTES) \ CHECK0(rd_total_times, RD_TOTAL_TIMES) \ @@ -1560,32 +1560,31 @@ testQemuMonitorJSONqemuMonitorJSONGetBlockStatsInfo(const void *data) CHECK0(wr_bytes, WR_BYTES) \ CHECK0(wr_total_times, WR_TOTAL_TIMES) \ CHECK0(flush_req, FLUSH_REQ) \ - CHECK0(flush_total_times, FLUSH_TOTAL_TIMES) \ - CHECK0(errs, ERRS) + CHECK0(flush_total_times, FLUSH_TOTAL_TIMES) if (qemuMonitorJSONGetBlockStatsInfo(qemuMonitorTestGetMonitor(test), "virtio-disk0", &rd_req, &rd_bytes, &rd_total_times, &wr_req, &wr_bytes, &wr_total_times, - &flush_req, &flush_total_times, &errs) < 0) + &flush_req, &flush_total_times) < 0) goto cleanup; - CHECK(1279, 28505088, 640616474, 174, 2845696, 530699221, 0, 0, -1) + CHECK(1279, 28505088, 640616474, 174, 2845696, 530699221, 0, 0) if (qemuMonitorJSONGetBlockStatsInfo(qemuMonitorTestGetMonitor(test), "virtio-disk1", &rd_req, &rd_bytes, &rd_total_times, &wr_req, &wr_bytes, &wr_total_times, - &flush_req, &flush_total_times, &errs) < 0) + &flush_req, &flush_total_times) < 0) goto cleanup; - CHECK(85, 348160, 8232156, 0, 0, 0, 0, 0, -1) + CHECK(85, 348160, 8232156, 0, 0, 0, 0, 0) if (qemuMonitorJSONGetBlockStatsInfo(qemuMonitorTestGetMonitor(test), "ide0-1-0", &rd_req, &rd_bytes, &rd_total_times, &wr_req, &wr_bytes, &wr_total_times, - &flush_req, &flush_total_times, &errs) < 0) + &flush_req, &flush_total_times) < 0) goto cleanup; - CHECK(16, 49250, 1004952, 0, 0, 0, 0, 0, -1) + CHECK(16, 49250, 1004952, 0, 0, 0, 0, 0) if (qemuMonitorJSONGetBlockStatsParamsNumber(qemuMonitorTestGetMonitor(test), &nparams) < 0) -- 2.2.2

qemu HMP commands sent by libvirt are terminated just by a '\r'. The fake monitor used in tests wasn't prepared to handle this and the communication would hang on an attempt to do a HMP conversation. Add a special case for handling commands separated by \r in case HMP is used. --- tests/qemumonitortestutils.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index 8155a69..3d34942 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -251,7 +251,8 @@ qemuMonitorTestIO(virNetSocketPtr sock, * if so, handle that command */ t1 = test->incoming; - while ((t2 = strstr(t1, "\n"))) { + while ((t2 = strstr(t1, "\n")) || + (!test->json && (t2 = strstr(t1, "\r")))) { *t2 = '\0'; if (qemuMonitorTestProcessCommand(test, t1) < 0) { -- 2.2.2

The function that is extracting block stats data from the QMP monitor reply contains a lot of repeated code. Since I'd be changing each of the copies in the next patch, lets convert it to a macro right away. --- src/qemu/qemu_monitor_json.c | 77 ++++++++++---------------------------------- 1 file changed, 17 insertions(+), 60 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index c88c7c3..52e1ad6 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1789,66 +1789,23 @@ qemuMonitorJSONGetOneBlockStatsInfo(virJSONValuePtr dev, goto cleanup; } - if (virJSONValueObjectGetNumberLong(stats, "rd_bytes", - &bstats->rd_bytes) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s statistic"), - "rd_bytes"); - goto cleanup; - } - if (virJSONValueObjectGetNumberLong(stats, "rd_operations", - &bstats->rd_req) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s statistic"), - "rd_operations"); - goto cleanup; - } - if (virJSONValueObjectHasKey(stats, "rd_total_time_ns") && - (virJSONValueObjectGetNumberLong(stats, "rd_total_time_ns", - &bstats->rd_total_times) < 0)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s statistic"), - "rd_total_time_ns"); - goto cleanup; - } - if (virJSONValueObjectGetNumberLong(stats, "wr_bytes", - &bstats->wr_bytes) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s statistic"), - "wr_bytes"); - goto cleanup; - } - if (virJSONValueObjectGetNumberLong(stats, "wr_operations", - &bstats->wr_req) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s statistic"), - "wr_operations"); - goto cleanup; - } - if (virJSONValueObjectHasKey(stats, "wr_total_time_ns") && - (virJSONValueObjectGetNumberLong(stats, "wr_total_time_ns", - &bstats->wr_total_times) < 0)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s statistic"), - "wr_total_time_ns"); - goto cleanup; - } - if (virJSONValueObjectHasKey(stats, "flush_operations") && - (virJSONValueObjectGetNumberLong(stats, "flush_operations", - &bstats->flush_req) < 0)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s statistic"), - "flush_operations"); - goto cleanup; - } - if (virJSONValueObjectHasKey(stats, "flush_total_time_ns") && - (virJSONValueObjectGetNumberLong(stats, "flush_total_time_ns", - &bstats->flush_total_times) < 0)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s statistic"), - "flush_total_time_ns"); - goto cleanup; - } +#define QEMU_MONITOR_JSON_BLOCK_STAT(NAME, VAR, MANDATORY) \ + if (MANDATORY || virJSONValueObjectHasKey(stats, NAME)) { \ + if (virJSONValueObjectGetNumberLong(stats, NAME, &VAR) < 0) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, \ + _("cannot read %s statistic"), NAME); \ + goto cleanup; \ + } \ + } + QEMU_MONITOR_JSON_BLOCK_STAT("rd_bytes", bstats->rd_bytes, true); + QEMU_MONITOR_JSON_BLOCK_STAT("wr_bytes", bstats->wr_bytes, true); + QEMU_MONITOR_JSON_BLOCK_STAT("rd_operations", bstats->rd_req, true); + QEMU_MONITOR_JSON_BLOCK_STAT("wr_operations", bstats->wr_req, true); + QEMU_MONITOR_JSON_BLOCK_STAT("rd_total_time_ns", bstats->rd_total_times, false); + QEMU_MONITOR_JSON_BLOCK_STAT("wr_total_time_ns", bstats->wr_total_times, false); + QEMU_MONITOR_JSON_BLOCK_STAT("flush_operations", bstats->flush_req, false); + QEMU_MONITOR_JSON_BLOCK_STAT("flush_total_time_ns", bstats->flush_total_times, false); +#undef QEMU_MONITOR_JSON_BLOCK_STAT /* it's ok to not have this information here. Just skip silently. */ qemuMonitorJSONDevGetBlockExtent(dev, &bstats->wr_highest_offset); -- 2.2.2

Our virDomainBlockStatsFlags API uses the old approach where, when it's called without the typed parameter array, returs the count of parameters supported by qemu. The supported parameter count is obtained via separate monitor calls which is a waste since we can calculate it when gathering the data. This patch adds code to the qemuMonitorGetAllBlockStatsInfo workers that allows to track the count of supported fields reported by qemu and will allow to remove the old duplicate code. --- src/qemu/qemu_monitor.c | 26 +++++++++++++++++++------- src/qemu/qemu_monitor_json.c | 15 +++++++++++---- src/qemu/qemu_monitor_text.c | 10 +++++++++- 3 files changed, 39 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 149e743..e4ff06e 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1848,14 +1848,24 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, } -/* Creates a hash table in 'ret_stats' with all block stats. - * Returns <0 on error, 0 on success. +/** + * qemuMonitorGetAllBlockStatsInfo: + * @mon: monitor object + * @ret_stats: pointer that is filled with a hash table containing the stats + * @backingChain: recurse into the bakcing chain of devices + * + * Creates a hash table in @ret_stats with block stats of all devices. In case + * @backingChain is true @ret_stats will additionally contain stats for + * backing chain members of block devices. + * + * Returns < 0 on error, count of supported block stats fields on success. */ int qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon, virHashTablePtr *ret_stats, bool backingChain) { + int ret = -1; VIR_DEBUG("mon=%p ret_stats=%p, backing=%d", mon, ret_stats, backingChain); if (!mon->json) { @@ -1868,8 +1878,8 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon, goto error; if (mon->json) { - if (qemuMonitorJSONGetAllBlockStatsInfo(mon, *ret_stats, backingChain) < 0) - goto error; + ret = qemuMonitorJSONGetAllBlockStatsInfo(mon, *ret_stats, + backingChain); } else { if (backingChain) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", @@ -1878,11 +1888,13 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon, goto error; } - if (qemuMonitorTextGetAllBlockStatsInfo(mon, *ret_stats) < 0) - goto error; + ret = qemuMonitorTextGetAllBlockStatsInfo(mon, *ret_stats); } - return 0; + if (ret < 0) + goto error; + + return ret; error: virHashFree(*ret_stats); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 52e1ad6..76baaf6 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1773,6 +1773,7 @@ qemuMonitorJSONGetOneBlockStatsInfo(virJSONValuePtr dev, qemuBlockStatsPtr bstats = NULL; virJSONValuePtr stats; int ret = -1; + int nstats = 0; char *entry_name = qemuDomainStorageAlias(dev_name, depth); virJSONValuePtr backing; @@ -1791,6 +1792,7 @@ qemuMonitorJSONGetOneBlockStatsInfo(virJSONValuePtr dev, #define QEMU_MONITOR_JSON_BLOCK_STAT(NAME, VAR, MANDATORY) \ if (MANDATORY || virJSONValueObjectHasKey(stats, NAME)) { \ + nstats++; \ if (virJSONValueObjectGetNumberLong(stats, NAME, &VAR) < 0) { \ virReportError(VIR_ERR_INTERNAL_ERROR, \ _("cannot read %s statistic"), NAME); \ @@ -1820,7 +1822,7 @@ qemuMonitorJSONGetOneBlockStatsInfo(virJSONValuePtr dev, hash, true) < 0) goto cleanup; - ret = 0; + ret = nstats; cleanup: VIR_FREE(bstats); VIR_FREE(entry_name); @@ -1834,6 +1836,7 @@ qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, bool backingChain) { int ret = -1; + int nstats = 0; int rc; size_t i; virJSONValuePtr cmd; @@ -1874,13 +1877,17 @@ qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, goto cleanup; } - if (qemuMonitorJSONGetOneBlockStatsInfo(dev, dev_name, 0, hash, - backingChain) < 0) + rc = qemuMonitorJSONGetOneBlockStatsInfo(dev, dev_name, 0, hash, + backingChain); + + if (rc < 0) goto cleanup; + if (rc > nstats) + nstats = rc; } - ret = 0; + ret = nstats; cleanup: virJSONValueFree(cmd); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 8b2ef90..203859c 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -854,6 +854,8 @@ qemuMonitorTextGetAllBlockStatsInfo(qemuMonitorPtr mon, size_t i; size_t j; int ret = -1; + int nstats; + int maxstats = 0; if (qemuMonitorHMPCommand(mon, "info blockstats", &info) < 0) goto cleanup; @@ -911,6 +913,8 @@ qemuMonitorTextGetAllBlockStatsInfo(qemuMonitorPtr mon, if (!(values = virStringSplit(line, " ", 0))) goto cleanup; + nstats = 0; + for (j = 0; values[j] && *values[j]; j++) { key = values[j]; @@ -925,6 +929,7 @@ qemuMonitorTextGetAllBlockStatsInfo(qemuMonitorPtr mon, #define QEMU_MONITOR_TEXT_READ_BLOCK_STAT(NAME, VAR) \ if (STREQ(key, NAME)) { \ + nstats++; \ if (virStrToLong_ll(value, NULL, 10, &VAR) < 0) { \ virReportError(VIR_ERR_INTERNAL_ERROR, \ _("'info blockstats' contains malformed " \ @@ -948,6 +953,9 @@ qemuMonitorTextGetAllBlockStatsInfo(qemuMonitorPtr mon, VIR_DEBUG("unknown block stat field '%s'", key); } + if (nstats > maxstats) + maxstats = nstats; + if (virHashAddEntry(hash, dev_name, stats) < 0) goto cleanup; stats = NULL; @@ -956,7 +964,7 @@ qemuMonitorTextGetAllBlockStatsInfo(qemuMonitorPtr mon, values = NULL; } - ret = 0; + ret = maxstats; cleanup: virStringFreeList(lines); -- 2.2.2

Extract the code to look up the disk alias and return the block stats struct so that it can be reused later in qemuDomainBlockStatsFlags. --- src/qemu/qemu_driver.c | 122 +++++++++++++++++++++++++++++++++---------------- 1 file changed, 82 insertions(+), 40 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 06168b1..e280cdf 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10480,6 +10480,80 @@ qemuDomainBlockResize(virDomainPtr dom, return ret; } + +/** + * qemuDomainBlocksStatsGather: + * @driver: driver object + * @vm: domain object + * @path: to gather the statistics for + * @retstats: returns pointer to structure holding the stats + * + * Gathers the block statistics for use in qemuDomainBlockStats* APIs. + * + * Returns -1 on error; number of filled block statistics on success. + */ +static int +qemuDomainBlocksStatsGather(virQEMUDriverPtr driver, + virDomainObjPtr vm, + const char *path, + qemuBlockStatsPtr *retstats) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDiskDefPtr disk; + virHashTablePtr blockstats = NULL; + qemuBlockStatsPtr stats; + int nstats; + char *diskAlias = NULL; + int ret = -1; + + if (*path) { + int idx; + + if ((idx = virDomainDiskIndexByName(vm->def, path, false)) < 0) { + virReportError(VIR_ERR_INVALID_ARG, _("invalid path: %s"), path); + goto cleanup; + } + disk = vm->def->disks[idx]; + + if (!disk->info.alias) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("missing disk device alias name for %s"), disk->dst); + goto cleanup; + } + + if (VIR_STRDUP(diskAlias, disk->info.alias) < 0) + goto cleanup; + } else { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("summary statistics are not supported yet")); + goto cleanup; + } + + qemuDomainObjEnterMonitor(driver, vm); + nstats = qemuMonitorGetAllBlockStatsInfo(priv->mon, &blockstats, false); + if (qemuDomainObjExitMonitor(driver, vm) < 0 || nstats < 0) + goto cleanup; + + if (VIR_ALLOC(*retstats) < 0) + goto cleanup; + + if (!(stats = virHashLookup(blockstats, diskAlias))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find statistics for device '%s'"), diskAlias); + goto cleanup; + } + + **retstats = *stats; + + ret = nstats; + + cleanup: + VIR_FREE(diskAlias); + virHashFree(blockstats); + return ret; +} + + /* This uses the 'info blockstats' monitor command which was * integrated into both qemu & kvm in late 2007. If the command is * not supported we detect this and return the appropriate error. @@ -10490,18 +10564,9 @@ qemuDomainBlockStats(virDomainPtr dom, virDomainBlockStatsPtr stats) { virQEMUDriverPtr driver = dom->conn->privateData; - int idx; + qemuBlockStatsPtr blockstats = NULL; int ret = -1; virDomainObjPtr vm; - virDomainDiskDefPtr disk = NULL; - qemuDomainObjPrivatePtr priv; - char *diskAlias = NULL; - - if (!*path) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("summary statistics are not supported yet")); - return ret; - } if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; @@ -10518,47 +10583,24 @@ qemuDomainBlockStats(virDomainPtr dom, goto endjob; } - if ((idx = virDomainDiskIndexByName(vm->def, path, false)) < 0) { - virReportError(VIR_ERR_INVALID_ARG, - _("invalid path: %s"), path); + if (qemuDomainBlocksStatsGather(driver, vm, path, &blockstats) < 0) goto endjob; - } - disk = vm->def->disks[idx]; - - if (!disk->info.alias) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("missing disk device alias name for %s"), disk->dst); - goto endjob; - } - - if (VIR_STRDUP(diskAlias, disk->info.alias) < 0) - goto endjob; - - priv = vm->privateData; + stats->rd_req = blockstats->rd_req; + stats->rd_bytes = blockstats->rd_bytes; + stats->wr_req = blockstats->wr_req; + stats->wr_bytes = blockstats->wr_bytes; /* qemu doesn't report the error count */ stats->errs = -1; - qemuDomainObjEnterMonitor(driver, vm); - ret = qemuMonitorGetBlockStatsInfo(priv->mon, - diskAlias, - &stats->rd_req, - &stats->rd_bytes, - NULL, - &stats->wr_req, - &stats->wr_bytes, - NULL, - NULL, - NULL); - if (qemuDomainObjExitMonitor(driver, vm) < 0) - ret = -1; + ret = 0; endjob: qemuDomainObjEndJob(driver, vm); cleanup: qemuDomObjEndAPI(&vm); - VIR_FREE(diskAlias); + VIR_FREE(blockstats); return ret; } -- 2.2.2

In the LXC driver, if the disk path is not provided the API returns total statistics for all disks of the domain. With the new text monitor implementation this can be now done in the qemu driver too. Add code that wil total the stats for all disks if the path is not provided. --- src/qemu/qemu_driver.c | 44 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e280cdf..4458c52 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10481,6 +10481,30 @@ qemuDomainBlockResize(virDomainPtr dom, } +static void +qemuDomainBlockStatsGatherTotals(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *opaque) +{ + qemuBlockStatsPtr data = payload; + qemuBlockStatsPtr total = opaque; + +#define QEMU_BLOCK_STAT_TOTAL(NAME) \ + if (data->NAME > 0) \ + total->NAME += data->NAME + + QEMU_BLOCK_STAT_TOTAL(wr_bytes); + QEMU_BLOCK_STAT_TOTAL(wr_req); + QEMU_BLOCK_STAT_TOTAL(rd_bytes); + QEMU_BLOCK_STAT_TOTAL(rd_req); + QEMU_BLOCK_STAT_TOTAL(flush_req); + QEMU_BLOCK_STAT_TOTAL(wr_total_times); + QEMU_BLOCK_STAT_TOTAL(rd_total_times); + QEMU_BLOCK_STAT_TOTAL(flush_total_times); +#undef QEMU_BLOCK_STAT_TOTAL +} + + /** * qemuDomainBlocksStatsGather: * @driver: driver object @@ -10523,10 +10547,6 @@ qemuDomainBlocksStatsGather(virQEMUDriverPtr driver, if (VIR_STRDUP(diskAlias, disk->info.alias) < 0) goto cleanup; - } else { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("summary statistics are not supported yet")); - goto cleanup; } qemuDomainObjEnterMonitor(driver, vm); @@ -10537,13 +10557,17 @@ qemuDomainBlocksStatsGather(virQEMUDriverPtr driver, if (VIR_ALLOC(*retstats) < 0) goto cleanup; - if (!(stats = virHashLookup(blockstats, diskAlias))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find statistics for device '%s'"), diskAlias); - goto cleanup; - } + if (diskAlias) { + if (!(stats = virHashLookup(blockstats, diskAlias))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find statistics for device '%s'"), diskAlias); + goto cleanup; + } - **retstats = *stats; + **retstats = *stats; + } else { + virHashForEach(blockstats, qemuDomainBlockStatsGatherTotals, *retstats); + } ret = nstats; -- 2.2.2

On Tue, Mar 10, 2015 at 05:26:39PM +0100, Peter Krempa wrote: What manual page do you want? Do we need this comment? VIR_FREE(blockstats); ACK with the leak fixed. Jan

The functions and their QMP and HMP implementations are no longer needed since everything is now done via the *AllStats functions. --- src/qemu/qemu_monitor.c | 62 -------------- src/qemu/qemu_monitor.h | 14 --- src/qemu/qemu_monitor_json.c | 135 ----------------------------- src/qemu/qemu_monitor_json.h | 12 --- src/qemu/qemu_monitor_text.c | 198 ------------------------------------------- src/qemu/qemu_monitor_text.h | 12 --- 6 files changed, 433 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index e4ff06e..4bdd8d8 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1808,45 +1808,6 @@ qemuMonitorBlockInfoLookup(virHashTablePtr blockInfo, return info; } -int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, - const char *dev_name, - long long *rd_req, - long long *rd_bytes, - long long *rd_total_times, - long long *wr_req, - long long *wr_bytes, - long long *wr_total_times, - long long *flush_req, - long long *flush_total_times) -{ - int ret; - VIR_DEBUG("mon=%p dev=%s", mon, dev_name); - - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } - - if (mon->json) - ret = qemuMonitorJSONGetBlockStatsInfo(mon, dev_name, - rd_req, rd_bytes, - rd_total_times, - wr_req, wr_bytes, - wr_total_times, - flush_req, - flush_total_times); - else - ret = qemuMonitorTextGetBlockStatsInfo(mon, dev_name, - rd_req, rd_bytes, - rd_total_times, - wr_req, wr_bytes, - wr_total_times, - flush_req, - flush_total_times); - return ret; -} - /** * qemuMonitorGetAllBlockStatsInfo: @@ -1921,29 +1882,6 @@ qemuMonitorBlockStatsUpdateCapacity(qemuMonitorPtr mon, } -/* Return 0 and update @nparams with the number of block stats - * QEMU supports if success. Return -1 if failure. - */ -int qemuMonitorGetBlockStatsParamsNumber(qemuMonitorPtr mon, - int *nparams) -{ - int ret; - VIR_DEBUG("mon=%p nparams=%p", mon, nparams); - - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } - - if (mon->json) - ret = qemuMonitorJSONGetBlockStatsParamsNumber(mon, nparams); - else - ret = qemuMonitorTextGetBlockStatsParamsNumber(mon, nparams); - - return ret; -} - int qemuMonitorGetBlockExtent(qemuMonitorPtr mon, const char *dev_name, unsigned long long *extent) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 72498b3..b30da34 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -357,17 +357,6 @@ struct qemuDomainDiskInfo * qemuMonitorBlockInfoLookup(virHashTablePtr blockInfo, const char *dev_name); -int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, - const char *dev_name, - long long *rd_req, - long long *rd_bytes, - long long *rd_total_times, - long long *wr_req, - long long *wr_bytes, - long long *wr_total_times, - long long *flush_req, - long long *flush_total_times); - typedef struct _qemuBlockStats qemuBlockStats; typedef qemuBlockStats *qemuBlockStatsPtr; struct _qemuBlockStats { @@ -394,9 +383,6 @@ int qemuMonitorBlockStatsUpdateCapacity(qemuMonitorPtr mon, bool backingChain) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); -int qemuMonitorGetBlockStatsParamsNumber(qemuMonitorPtr mon, - int *nparams); - int qemuMonitorGetBlockExtent(qemuMonitorPtr mon, const char *dev_name, unsigned long long *extent); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 76baaf6..0f32a0a 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1668,67 +1668,6 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon, } -int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, - const char *dev_name, - long long *rd_req, - long long *rd_bytes, - long long *rd_total_times, - long long *wr_req, - long long *wr_bytes, - long long *wr_total_times, - long long *flush_req, - long long *flush_total_times) -{ - qemuBlockStats *stats; - virHashTablePtr blockstats = NULL; - int ret = -1; - - *rd_req = *rd_bytes = -1; - *wr_req = *wr_bytes = -1; - - if (rd_total_times) - *rd_total_times = -1; - if (wr_total_times) - *wr_total_times = -1; - if (flush_req) - *flush_req = -1; - if (flush_total_times) - *flush_total_times = -1; - - if (!(blockstats = virHashCreate(10, virHashValueFree))) - goto cleanup; - - if (qemuMonitorJSONGetAllBlockStatsInfo(mon, blockstats, false) < 0) - goto cleanup; - - if (!(stats = virHashLookup(blockstats, dev_name))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find statistics for device '%s'"), dev_name); - goto cleanup; - } - - *rd_req = stats->rd_req; - *rd_bytes = stats->rd_bytes; - *wr_req = stats->wr_req; - *wr_bytes = stats->wr_bytes; - - if (rd_total_times) - *rd_total_times = stats->rd_total_times; - if (wr_total_times) - *wr_total_times = stats->wr_total_times; - if (flush_req) - *flush_req = stats->flush_req; - if (flush_total_times) - *flush_total_times = stats->flush_total_times; - - ret = 0; - - cleanup: - virHashFree(blockstats); - return ret; -} - - typedef enum { QEMU_MONITOR_BLOCK_EXTENT_ERROR_OK, QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOPARENT, @@ -2013,80 +1952,6 @@ qemuMonitorJSONBlockStatsUpdateCapacity(qemuMonitorPtr mon, } -int qemuMonitorJSONGetBlockStatsParamsNumber(qemuMonitorPtr mon, - int *nparams) -{ - int ret, num = 0; - size_t i; - virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-blockstats", - NULL); - virJSONValuePtr reply = NULL; - virJSONValuePtr devices = NULL; - virJSONValuePtr dev = NULL; - virJSONValuePtr stats = NULL; - - if (!cmd) - return -1; - - ret = qemuMonitorJSONCommand(mon, cmd, &reply); - - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); - if (ret < 0) - goto cleanup; - ret = -1; - - devices = virJSONValueObjectGet(reply, "return"); - if (!devices || devices->type != VIR_JSON_TYPE_ARRAY) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("blockstats reply was missing device list")); - goto cleanup; - } - - dev = virJSONValueArrayGet(devices, 0); - - if (!dev || dev->type != VIR_JSON_TYPE_OBJECT) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("blockstats device entry was not in expected format")); - goto cleanup; - } - - if ((stats = virJSONValueObjectGet(dev, "stats")) == NULL || - stats->type != VIR_JSON_TYPE_OBJECT) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("blockstats stats entry was not in expected format")); - goto cleanup; - } - - for (i = 0; i < stats->data.object.npairs; i++) { - const char *key = stats->data.object.pairs[i].key; - - if (STREQ(key, "rd_bytes") || - STREQ(key, "rd_operations") || - STREQ(key, "rd_total_time_ns") || - STREQ(key, "wr_bytes") || - STREQ(key, "wr_operations") || - STREQ(key, "wr_total_time_ns") || - STREQ(key, "flush_operations") || - STREQ(key, "flush_total_time_ns")) { - num++; - } else { - /* wr_highest_offset is parsed by qemuMonitorJSONGetBlockExtent. */ - if (STRNEQ(key, "wr_highest_offset")) - VIR_DEBUG("Missed block stat: %s", key); - } - } - - *nparams = num; - ret = 0; - - cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; -} - - static int qemuMonitorJSONReportBlockExtentError(qemuMonitorBlockExtentError error) { diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 0fcb0c0..8ceea8a 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -71,24 +71,12 @@ int qemuMonitorJSONSetMemoryStatsPeriod(qemuMonitorPtr mon, int period); int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon, virHashTablePtr table); -int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, - const char *dev_name, - long long *rd_req, - long long *rd_bytes, - long long *rd_total_times, - long long *wr_req, - long long *wr_bytes, - long long *wr_total_times, - long long *flush_req, - long long *flush_total_times); int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, virHashTablePtr hash, bool backingChain); int qemuMonitorJSONBlockStatsUpdateCapacity(qemuMonitorPtr mon, virHashTablePtr stats, bool backingChain); -int qemuMonitorJSONGetBlockStatsParamsNumber(qemuMonitorPtr mon, - int *nparams); int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon, const char *dev_name, unsigned long long *extent); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 203859c..9973a17 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -975,204 +975,6 @@ qemuMonitorTextGetAllBlockStatsInfo(qemuMonitorPtr mon, } -int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, - const char *dev_name, - long long *rd_req, - long long *rd_bytes, - long long *rd_total_times, - long long *wr_req, - long long *wr_bytes, - long long *wr_total_times, - long long *flush_req, - long long *flush_total_times) -{ - char *info = NULL; - int ret = -1; - char *dummy; - const char *p, *eol; - int devnamelen = strlen(dev_name); - - if (qemuMonitorHMPCommand(mon, "info blockstats", &info) < 0) - goto cleanup; - - /* If the command isn't supported then qemu prints the supported - * info commands, so the output starts "info ". Since this is - * unlikely to be the name of a block device, we can use this - * to detect if qemu supports the command. - */ - if (strstr(info, "\ninfo ")) { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", - _("'info blockstats' not supported by this qemu")); - goto cleanup; - } - - *rd_req = *rd_bytes = -1; - *wr_req = *wr_bytes = -1; - - if (rd_total_times) - *rd_total_times = -1; - if (wr_total_times) - *wr_total_times = -1; - if (flush_req) - *flush_req = -1; - if (flush_total_times) - *flush_total_times = -1; - - /* The output format for both qemu & KVM is: - * blockdevice: rd_bytes=% wr_bytes=% rd_operations=% wr_operations=% - * (repeated for each block device) - * where '%' is a 64 bit number. - */ - p = info; - - while (*p) { - /* New QEMU has separate names for host & guest side of the disk - * and libvirt gives the host side a 'drive-' prefix. The passed - * in dev_name is the guest side though - */ - if (STRPREFIX(p, QEMU_DRIVE_HOST_PREFIX)) - p += strlen(QEMU_DRIVE_HOST_PREFIX); - - if (STREQLEN(p, dev_name, devnamelen) - && p[devnamelen] == ':' && p[devnamelen+1] == ' ') { - - eol = strchr(p, '\n'); - if (!eol) - eol = p + strlen(p); - - p += devnamelen+2; /* Skip to first label. */ - - while (*p) { - if (STRPREFIX(p, "rd_bytes=")) { - p += strlen("rd_bytes="); - if (virStrToLong_ll(p, &dummy, 10, rd_bytes) == -1) - VIR_DEBUG("error reading rd_bytes: %s", p); - } else if (STRPREFIX(p, "wr_bytes=")) { - p += strlen("wr_bytes="); - if (virStrToLong_ll(p, &dummy, 10, wr_bytes) == -1) - VIR_DEBUG("error reading wr_bytes: %s", p); - } else if (STRPREFIX(p, "rd_operations=")) { - p += strlen("rd_operations="); - if (virStrToLong_ll(p, &dummy, 10, rd_req) == -1) - VIR_DEBUG("error reading rd_req: %s", p); - } else if (STRPREFIX(p, "wr_operations=")) { - p += strlen("wr_operations="); - if (virStrToLong_ll(p, &dummy, 10, wr_req) == -1) - VIR_DEBUG("error reading wr_req: %s", p); - } else if (rd_total_times && - STRPREFIX(p, "rd_total_time_ns=")) { - p += strlen("rd_total_time_ns="); - if (virStrToLong_ll(p, &dummy, 10, rd_total_times) == -1) - VIR_DEBUG("error reading rd_total_times: %s", p); - } else if (wr_total_times && - STRPREFIX(p, "wr_total_time_ns=")) { - p += strlen("wr_total_time_ns="); - if (virStrToLong_ll(p, &dummy, 10, wr_total_times) == -1) - VIR_DEBUG("error reading wr_total_times: %s", p); - } else if (flush_req && - STRPREFIX(p, "flush_operations=")) { - p += strlen("flush_operations="); - if (virStrToLong_ll(p, &dummy, 10, flush_req) == -1) - VIR_DEBUG("error reading flush_req: %s", p); - } else if (flush_total_times && - STRPREFIX(p, "flush_total_time_ns=")) { - p += strlen("flush_total_time_ns="); - if (virStrToLong_ll(p, &dummy, 10, flush_total_times) == -1) - VIR_DEBUG("error reading flush_total_times: %s", p); - } else { - VIR_DEBUG("unknown block stat near %s", p); - } - - /* Skip to next label. */ - p = strchr(p, ' '); - if (!p || p >= eol) break; - p++; - } - ret = 0; - goto cleanup; - } - - /* Skip to next line. */ - p = strchr(p, '\n'); - if (!p) break; - p++; - } - - /* If we reach here then the device was not found. */ - virReportError(VIR_ERR_INVALID_ARG, - _("no stats found for device %s"), dev_name); - - cleanup: - VIR_FREE(info); - return ret; -} - -int qemuMonitorTextGetBlockStatsParamsNumber(qemuMonitorPtr mon, - int *nparams) -{ - char *info = NULL; - int ret = -1; - int num = 0; - const char *p, *eol; - - if (qemuMonitorHMPCommand(mon, "info blockstats", &info) < 0) - goto cleanup; - - /* If the command isn't supported then qemu prints the supported - * info commands, so the output starts "info ". Since this is - * unlikely to be the name of a block device, we can use this - * to detect if qemu supports the command. - */ - if (strstr(info, "\ninfo ")) { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", - _("'info blockstats' not supported by this qemu")); - goto cleanup; - } - - /* The output format for both qemu & KVM is: - * blockdevice: rd_bytes=% wr_bytes=% rd_operations=% wr_operations=% - * (repeated for each block device) - * where '%' is a 64 bit number. - */ - p = info; - - eol = strchr(p, '\n'); - if (!eol) - eol = p + strlen(p); - - /* Skip the device name and following ":", and spaces (e.g. - * "floppy0: ") - */ - p = strchr(p, ' '); - - while (p && p < eol) { - if (STRPREFIX(p, " rd_bytes=") || - STRPREFIX(p, " wr_bytes=") || - STRPREFIX(p, " rd_operations=") || - STRPREFIX(p, " wr_operations=") || - STRPREFIX(p, " rd_total_time_ns=") || - STRPREFIX(p, " wr_total_time_ns=") || - STRPREFIX(p, " flush_operations=") || - STRPREFIX(p, " flush_total_time_ns=")) { - num++; - } else { - VIR_DEBUG("unknown block stat near %s", p); - } - - /* Skip to next label. */ - p = strchr(p + 1, ' '); - } - - *nparams = num; - ret = 0; - - cleanup: - VIR_FREE(info); - return ret; -} - int qemuMonitorTextGetBlockExtent(qemuMonitorPtr mon ATTRIBUTE_UNUSED, const char *dev_name ATTRIBUTE_UNUSED, unsigned long long *extent ATTRIBUTE_UNUSED) diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index a1bc2b2..40edc9a 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -63,18 +63,6 @@ int qemuMonitorTextGetBlockInfo(qemuMonitorPtr mon, int qemuMonitorTextGetAllBlockStatsInfo(qemuMonitorPtr mon, virHashTablePtr hash); -int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, - const char *dev_name, - long long *rd_req, - long long *rd_bytes, - long long *rd_total_times, - long long *wr_req, - long long *wr_bytes, - long long *wr_total_times, - long long *flush_req, - long long *flush_total_times); -int qemuMonitorTextGetBlockStatsParamsNumber(qemuMonitorPtr mon, - int *nparams); int qemuMonitorTextGetBlockExtent(qemuMonitorPtr mon, const char *dev_name, unsigned long long *extent); -- 2.2.2