[libvirt] [PATCH 0/3] Fix and improve block bulk stats
Francesco Romani (1): qemu: bulk stats: add block allocation information Peter Krempa (2): qemu: monitor: return block stats data as a hash to avoid disk mixup qemu: monitor: Add helper function to fill physical/virtual image size src/libvirt.c | 6 + src/qemu/qemu_driver.c | 71 +++++++---- src/qemu/qemu_monitor.c | 31 +++-- src/qemu/qemu_monitor.h | 13 ++- src/qemu/qemu_monitor_json.c | 273 +++++++++++++++++++++++++++++++------------ src/qemu/qemu_monitor_json.h | 6 +- 6 files changed, 286 insertions(+), 114 deletions(-) -- 2.1.0
The current block stats code matched up the disk name with the actual stats by the order in the data returned from qemu. This unfortunately isn't right as qemu may return the disks in any order. Fix this by returning a hash of stats and index them by the disk alias. --- src/qemu/qemu_driver.c | 44 ++++++++--------- src/qemu/qemu_monitor.c | 14 ++---- src/qemu/qemu_monitor.h | 6 +-- src/qemu/qemu_monitor_json.c | 112 ++++++++++++++++++++----------------------- src/qemu/qemu_monitor_json.h | 4 +- 5 files changed, 82 insertions(+), 98 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 543de79..b324830 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17868,24 +17868,18 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, { size_t i; int ret = -1; - int nstats = dom->def->ndisks; - qemuBlockStatsPtr stats = NULL; + int rc; + virHashTablePtr stats = NULL; qemuDomainObjPrivatePtr priv = dom->privateData; if (!HAVE_JOB(privflags) || !virDomainObjIsActive(dom)) return 0; /* it's ok, just go ahead silently */ - if (VIR_ALLOC_N(stats, nstats) < 0) - return -1; - qemuDomainObjEnterMonitor(driver, dom); - - nstats = qemuMonitorGetAllBlockStatsInfo(priv->mon, NULL, - stats, nstats); - + rc = qemuMonitorGetAllBlockStatsInfo(priv->mon, &stats); qemuDomainObjExitMonitor(driver, dom); - if (nstats < 0) { + if (rc < 0) { virResetLastError(); ret = 0; /* still ok, again go ahead silently */ goto cleanup; @@ -17893,32 +17887,38 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, 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); + for (i = 0; i < dom->def->ndisks; i++) { + qemuBlockStats *entry; + virDomainDiskDefPtr disk = dom->def->disks[i]; + + QEMU_ADD_NAME_PARAM(record, maxparams, "block", i, disk->dst); + + if (!disk->info.alias || + !(entry = virHashLookup(stats, disk->info.alias))) + continue; QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i, - "rd.reqs", stats[i].rd_req); + "rd.reqs", entry->rd_req); QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i, - "rd.bytes", stats[i].rd_bytes); + "rd.bytes", entry->rd_bytes); QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i, - "rd.times", stats[i].rd_total_times); + "rd.times", entry->rd_total_times); QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i, - "wr.reqs", stats[i].wr_req); + "wr.reqs", entry->wr_req); QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i, - "wr.bytes", stats[i].wr_bytes); + "wr.bytes", entry->wr_bytes); QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i, - "wr.times", stats[i].wr_total_times); + "wr.times", entry->wr_total_times); QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i, - "fl.reqs", stats[i].flush_req); + "fl.reqs", entry->flush_req); QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i, - "fl.times", stats[i].flush_total_times); + "fl.times", entry->flush_total_times); } ret = 0; cleanup: - VIR_FREE(stats); + virHashFree(stats); return ret; } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index c25f002..543384d 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1763,23 +1763,17 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, */ int qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon, - const char *dev_name, - qemuBlockStatsPtr stats, - int nstats) + virHashTablePtr *ret_stats) { - int ret; - VIR_DEBUG("mon=%p dev=%s", mon, dev_name); + VIR_DEBUG("mon=%p ret_stats=%p", mon, ret_stats); - if (mon->json) { - ret = qemuMonitorJSONGetAllBlockStatsInfo(mon, dev_name, - stats, nstats); - } else { + if (!mon->json) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("unable to query all block stats with this QEMU")); return -1; } - return ret; + return qemuMonitorJSONGetAllBlockStatsInfo(mon, ret_stats); } /* Return 0 and update @nparams with the number of block stats diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 6b91e29..adf18ab 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -361,10 +361,8 @@ struct _qemuBlockStats { }; int qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon, - const char *dev_name, - qemuBlockStatsPtr stats, - int nstats) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3); + virHashTablePtr *ret_stats) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int qemuMonitorGetBlockStatsParamsNumber(qemuMonitorPtr mon, int *nparams); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index a3d7c2c..ba3d8f3 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1734,7 +1734,8 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, long long *flush_total_times, long long *errs) { - qemuBlockStats stats; + qemuBlockStats *stats; + virHashTablePtr blockstats = NULL; int ret = -1; *rd_req = *rd_bytes = -1; @@ -1749,56 +1750,62 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, if (flush_total_times) *flush_total_times = -1; - if (qemuMonitorJSONGetAllBlockStatsInfo(mon, dev_name, &stats, 1) != 1) + if (qemuMonitorJSONGetAllBlockStatsInfo(mon, &blockstats) < 0) goto cleanup; - *rd_req = stats.rd_req; - *rd_bytes = stats.rd_bytes; - *wr_req = stats.wr_req; - *wr_bytes = stats.wr_bytes; + 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; *errs = -1; /* QEMU does not have this */ if (rd_total_times) - *rd_total_times = stats.rd_total_times; + *rd_total_times = stats->rd_total_times; if (wr_total_times) - *wr_total_times = stats.wr_total_times; + *wr_total_times = stats->wr_total_times; if (flush_req) - *flush_req = stats.flush_req; + *flush_req = stats->flush_req; if (flush_total_times) - *flush_total_times = stats.flush_total_times; + *flush_total_times = stats->flush_total_times; ret = 0; cleanup: + virHashFree(blockstats); return ret; } int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, - const char *dev_name, - qemuBlockStatsPtr bstats, - int nstats) + virHashTablePtr *ret_stats) { - int ret, count; + int ret = -1; + int rc; size_t i; - virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-blockstats", - NULL); + virJSONValuePtr cmd; virJSONValuePtr reply = NULL; virJSONValuePtr devices; + qemuBlockStatsPtr bstats = NULL; - if (!cmd) - return -1; + virHashTablePtr hash = NULL; - if (!bstats || nstats <= 0) + if (!(cmd = qemuMonitorJSONMakeCommand("query-blockstats", NULL))) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (!(hash = virHashCreate(10, virHashValueFree))) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); - if (ret < 0) + if ((rc = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) + goto cleanup; + + if (qemuMonitorJSONCheckError(cmd, reply) < 0) goto cleanup; - ret = -1; devices = virJSONValueObjectGet(reply, "return"); if (!devices || devices->type != VIR_JSON_TYPE_ARRAY) { @@ -1807,10 +1814,14 @@ int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, goto cleanup; } - count = 0; - for (i = 0; i < virJSONValueArraySize(devices) && count < nstats; i++) { + for (i = 0; i < virJSONValueArraySize(devices); i++) { virJSONValuePtr dev = virJSONValueArrayGet(devices, i); virJSONValuePtr stats; + const char *devname; + + if (VIR_ALLOC(bstats) < 0) + goto cleanup; + if (!dev || dev->type != VIR_JSON_TYPE_OBJECT) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("blockstats device entry was not " @@ -1818,29 +1829,16 @@ int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, goto cleanup; } - /* If dev_name is specified, we are looking for a specific device, - * so we must be stricter. - */ - if (dev_name) { - const char *thisdev = virJSONValueObjectGetString(dev, "device"); - if (!thisdev) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("blockstats device entry was not " - "in expected format")); - goto cleanup; - } - - /* 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(thisdev, QEMU_DRIVE_HOST_PREFIX)) - thisdev += strlen(QEMU_DRIVE_HOST_PREFIX); - - if (STRNEQ(thisdev, dev_name)) - continue; + if (!(devname = virJSONValueObjectGetString(dev, "device"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("blockstats device entry was not " + "in expected format")); + goto cleanup; } + if (STRPREFIX(devname, QEMU_DRIVE_HOST_PREFIX)) + devname += strlen(QEMU_DRIVE_HOST_PREFIX); + if ((stats = virJSONValueObjectGet(dev, "stats")) == NULL || stats->type != VIR_JSON_TYPE_OBJECT) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -1910,22 +1908,18 @@ int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, goto cleanup; } - count++; - bstats++; - - if (dev_name && count) - break; - } - - if (dev_name && !count) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find statistics for device '%s'"), dev_name); - goto cleanup; + if (virHashAddEntry(hash, devname, bstats) < 0) + goto cleanup; + bstats = NULL; } - ret = count; + *ret_stats = hash; + hash = NULL; + ret = 0; cleanup: + VIR_FREE(bstats); + virHashFree(hash); virJSONValueFree(cmd); virJSONValueFree(reply); return ret; diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index d366179..ef9b552 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -80,9 +80,7 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, long long *flush_total_times, long long *errs); int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, - const char *dev_name, - qemuBlockStatsPtr stats, - int nstats); + virHashTablePtr *ret_stats); int qemuMonitorJSONGetBlockStatsParamsNumber(qemuMonitorPtr mon, int *nparams); int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon, -- 2.1.0
On 09/25/2014 06:06 AM, Peter Krempa wrote:
The current block stats code matched up the disk name with the actual stats by the order in the data returned from qemu. This unfortunately isn't right as qemu may return the disks in any order. Fix this by returning a hash of stats and index them by the disk alias. --- src/qemu/qemu_driver.c | 44 ++++++++--------- src/qemu/qemu_monitor.c | 14 ++---- src/qemu/qemu_monitor.h | 6 +-- src/qemu/qemu_monitor_json.c | 112 ++++++++++++++++++++----------------------- src/qemu/qemu_monitor_json.h | 4 +- 5 files changed, 82 insertions(+), 98 deletions(-)
@@ -1749,56 +1750,62 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, if (flush_total_times) *flush_total_times = -1;
- if (qemuMonitorJSONGetAllBlockStatsInfo(mon, dev_name, &stats, 1) != 1) + if (qemuMonitorJSONGetAllBlockStatsInfo(mon, &blockstats) < 0) goto cleanup;
- *rd_req = stats.rd_req; - *rd_bytes = stats.rd_bytes; - *wr_req = stats.wr_req; - *wr_bytes = stats.wr_bytes; + if (!(stats = virHashLookup(blockstats, dev_name))) {
Maybe slightly less efficient (we now malloc stats for all members, only to throw away everything but the one we want), but I can live with it. ACK; worth having in 1.2.9 -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 09/29/14 20:17, Eric Blake wrote:
On 09/25/2014 06:06 AM, Peter Krempa wrote:
The current block stats code matched up the disk name with the actual stats by the order in the data returned from qemu. This unfortunately isn't right as qemu may return the disks in any order. Fix this by returning a hash of stats and index them by the disk alias. --- src/qemu/qemu_driver.c | 44 ++++++++--------- src/qemu/qemu_monitor.c | 14 ++---- src/qemu/qemu_monitor.h | 6 +-- src/qemu/qemu_monitor_json.c | 112 ++++++++++++++++++++----------------------- src/qemu/qemu_monitor_json.h | 4 +- 5 files changed, 82 insertions(+), 98 deletions(-)
@@ -1749,56 +1750,62 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, if (flush_total_times) *flush_total_times = -1;
- if (qemuMonitorJSONGetAllBlockStatsInfo(mon, dev_name, &stats, 1) != 1) + if (qemuMonitorJSONGetAllBlockStatsInfo(mon, &blockstats) < 0) goto cleanup;
- *rd_req = stats.rd_req; - *rd_bytes = stats.rd_bytes; - *wr_req = stats.wr_req; - *wr_bytes = stats.wr_bytes; + if (!(stats = virHashLookup(blockstats, dev_name))) {
Maybe slightly less efficient (we now malloc stats for all members, only to throw away everything but the one we want), but I can live with it.
ACK; worth having in 1.2.9
I've pushed this one and I'll hold off the rest until the release. Peter
While our code gathers block stats via "query-blockstats" some information need to be gathered via "query-block". Add a helper function that will update the blockstats structure if requested. --- src/qemu/qemu_monitor.c | 17 ++++++++++ src/qemu/qemu_monitor.h | 6 ++++ src/qemu/qemu_monitor_json.c | 78 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 2 ++ 4 files changed, 103 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 543384d..c9929c3 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1776,6 +1776,23 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon, return qemuMonitorJSONGetAllBlockStatsInfo(mon, ret_stats); } + +/* Updates "stats" to fill virtual and physical size of the image */ +int qemuMonitorBlockStatsUpdateCapacity(qemuMonitorPtr mon, + virHashTablePtr stats) +{ + VIR_DEBUG("mon=%p, stats=%p", mon, stats); + + if (!mon->json) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("block capacity/size info requires JSON monitor")); + return -1; + } + + return qemuMonitorJSONBlockStatsUpdateCapacity(mon, stats); +} + + /* Return 0 and update @nparams with the number of block stats * QEMU supports if success. Return -1 if failure. */ diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index adf18ab..616d960 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -358,12 +358,18 @@ struct _qemuBlockStats { long long wr_total_times; long long flush_req; long long flush_total_times; + unsigned long long capacity; + unsigned long long physical; }; int qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon, virHashTablePtr *ret_stats) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +int qemuMonitorBlockStatsUpdateCapacity(qemuMonitorPtr mon, + virHashTablePtr stats) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + int qemuMonitorGetBlockStatsParamsNumber(qemuMonitorPtr mon, int *nparams); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index ba3d8f3..b634121 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1926,6 +1926,84 @@ int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, } +int qemuMonitorJSONBlockStatsUpdateCapacity(qemuMonitorPtr mon, + virHashTablePtr stats) +{ + int ret = -1; + int rc; + size_t i; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr devices; + + if (!(cmd = qemuMonitorJSONMakeCommand("query-block", NULL))) + return -1; + + if ((rc = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) + goto cleanup; + + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + + devices = virJSONValueObjectGet(reply, "return"); + if (!devices || devices->type != VIR_JSON_TYPE_ARRAY) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-block reply was missing device list")); + goto cleanup; + } + + for (i = 0; i < virJSONValueArraySize(devices); i++) { + virJSONValuePtr dev = virJSONValueArrayGet(devices, i); + virJSONValuePtr inserted; + virJSONValuePtr image; + qemuBlockStatsPtr bstats; + const char *devname; + + if (!dev || dev->type != VIR_JSON_TYPE_OBJECT) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-block device entry was not " + "in expected format")); + goto cleanup; + } + + if (!(devname = virJSONValueObjectGetString(dev, "device"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-block device entry was not " + "in expected format")); + goto cleanup; + } + + if (STRPREFIX(devname, QEMU_DRIVE_HOST_PREFIX)) + devname += strlen(QEMU_DRIVE_HOST_PREFIX); + + /* ignore missing info */ + if (!(bstats = virHashLookup(stats, devname))) + continue; + + /* drive may be empty */ + if (!(inserted = virJSONValueObjectGet(dev, "inserted")) || + !(image = virJSONValueObjectGet(inserted, "image"))) + continue; + + if (virJSONValueObjectGetNumberUlong(image, "virtual-size", + &bstats->capacity) < 0) + continue; + + /* if actual-size is missing, image is not thin provisioned */ + if (virJSONValueObjectGetNumberUlong(image, "actual-size", + &bstats->physical) < 0) + bstats->physical = bstats->capacity; + } + + ret = 0; + + cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + + int qemuMonitorJSONGetBlockStatsParamsNumber(qemuMonitorPtr mon, int *nparams) { diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index ef9b552..c7dd416 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -81,6 +81,8 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, long long *errs); int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, virHashTablePtr *ret_stats); +int qemuMonitorJSONBlockStatsUpdateCapacity(qemuMonitorPtr mon, + virHashTablePtr stats); int qemuMonitorJSONGetBlockStatsParamsNumber(qemuMonitorPtr mon, int *nparams); int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon, -- 2.1.0
On 09/25/2014 06:06 AM, Peter Krempa wrote:
While our code gathers block stats via "query-blockstats" some information need to be gathered via "query-block". Add a helper function that will update the blockstats structure if requested. --- src/qemu/qemu_monitor.c | 17 ++++++++++ src/qemu/qemu_monitor.h | 6 ++++ src/qemu/qemu_monitor_json.c | 78 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 2 ++ 4 files changed, 103 insertions(+)
ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: Francesco Romani <fromani@redhat.com> Management software wants to be able to allocate disk space on demand. To support this they need keep track of the space occupation of the block device. This information is reported by qemu as part of block stats. This patch extend the block information in the bulk stats with the allocation information. To keep the same behaviour a helper is extracted from qemuMonitorJSONGetBlockExtent in order to get per-device allocation information. Signed-off-by: Francesco Romani <fromani@redhat.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt.c | 6 +++ src/qemu/qemu_driver.c | 27 +++++++++++++ src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 91 ++++++++++++++++++++++++++++++++++---------- 4 files changed, 105 insertions(+), 20 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index aa83365..b078eed 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -21656,6 +21656,12 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * unsigned long long. * "block.<num>.errors" - Xen only: the 'oo_req' value as * unsigned long long. + * "block.<num>.allocation" - offset of the highest written sector + * as unsigned long long. + * "block.<num>.capacity" - logical size in bytes of the block device backing + * image as unsinged long long. + * "block.<num>.physical" - physical size in bytes of the container of the + * backing image as unsigned long long. * * Note that entire stats groups or individual stat fields may be missing from * the output in case they are not supported by the given hypervisor, are not diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b324830..b93d7d0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17859,6 +17859,19 @@ do { \ goto cleanup; \ } while (0) +#define QEMU_ADD_BLOCK_PARAM_ULL(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 (virTypedParamsAddULLong(&(record)->params, \ + &(record)->nparams, \ + maxparams, \ + param_name, \ + value) < 0) \ + goto cleanup; \ +} while (0) + static int qemuDomainGetStatsBlock(virQEMUDriverPtr driver, virDomainObjPtr dom, @@ -17877,6 +17890,7 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, dom); rc = qemuMonitorGetAllBlockStatsInfo(priv->mon, &stats); + ignore_value(qemuMonitorBlockStatsUpdateCapacity(priv->mon, stats)); qemuDomainObjExitMonitor(driver, dom); if (rc < 0) { @@ -17913,6 +17927,17 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, "fl.reqs", entry->flush_req); QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i, "fl.times", entry->flush_total_times); + + QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, i, + "allocation", entry->wr_highest_offset); + + if (entry->capacity) + QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, i, + "capacity", entry->capacity); + if (entry->physical) + QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, i, + "physical", entry->physical); + } ret = 0; @@ -17924,6 +17949,8 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, #undef QEMU_ADD_BLOCK_PARAM_LL +#undef QEMU_ADD_BLOCK_PARAM_ULL + #undef QEMU_ADD_NAME_PARAM #undef QEMU_ADD_COUNT_PARAM diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 616d960..f0c0b0a 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -360,6 +360,7 @@ struct _qemuBlockStats { long long flush_total_times; unsigned long long capacity; unsigned long long physical; + unsigned long long wr_highest_offset; }; int qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index b634121..c41a7fb 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1782,6 +1782,40 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, } +typedef enum { + QEMU_MONITOR_BLOCK_EXTENT_ERROR_OK, + QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOPARENT, + QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOSTATS, + QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOOFFSET +} qemuMonitorBlockExtentError; + + +static int +qemuMonitorJSONDevGetBlockExtent(virJSONValuePtr dev, + unsigned long long *extent) +{ + virJSONValuePtr stats; + virJSONValuePtr parent; + + if ((parent = virJSONValueObjectGet(dev, "parent")) == NULL || + parent->type != VIR_JSON_TYPE_OBJECT) { + return QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOPARENT; + } + + if ((stats = virJSONValueObjectGet(parent, "stats")) == NULL || + stats->type != VIR_JSON_TYPE_OBJECT) { + return QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOSTATS; + } + + if (virJSONValueObjectGetNumberUlong(stats, "wr_highest_offset", + extent) < 0) { + return QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOOFFSET; + } + + return QEMU_MONITOR_BLOCK_EXTENT_ERROR_OK; +} + + int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, virHashTablePtr *ret_stats) { @@ -1908,6 +1942,9 @@ int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, goto cleanup; } + /* it's ok to not have this information here. Just skip silently. */ + qemuMonitorJSONDevGetBlockExtent(dev, &bstats->wr_highest_offset); + if (virHashAddEntry(hash, devname, bstats) < 0) goto cleanup; bstats = NULL; @@ -2077,6 +2114,36 @@ int qemuMonitorJSONGetBlockStatsParamsNumber(qemuMonitorPtr mon, return ret; } + +static int +qemuMonitorJSONReportBlockExtentError(qemuMonitorBlockExtentError error) +{ + switch (error) { + case QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOPARENT: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("blockstats parent entry was not in " + "expected format")); + break; + + case QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOSTATS: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("blockstats stats entry was not in " + "expected format")); + + case QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOOFFSET: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot read %s statistic"), + "wr_highest_offset"); + break; + + case QEMU_MONITOR_BLOCK_EXTENT_ERROR_OK: + return 0; + } + + return -1; +} + + int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon, const char *dev_name, unsigned long long *extent) @@ -2111,9 +2178,8 @@ int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon, for (i = 0; i < virJSONValueArraySize(devices); i++) { virJSONValuePtr dev = virJSONValueArrayGet(devices, i); - virJSONValuePtr stats; - virJSONValuePtr parent; const char *thisdev; + int err; if (!dev || dev->type != VIR_JSON_TYPE_OBJECT) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("blockstats device entry was not in expected format")); @@ -2137,24 +2203,9 @@ int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon, continue; found = true; - if ((parent = virJSONValueObjectGet(dev, "parent")) == NULL || - parent->type != VIR_JSON_TYPE_OBJECT) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("blockstats parent entry was not in expected format")); - goto cleanup; - } - - if ((stats = virJSONValueObjectGet(parent, "stats")) == NULL || - stats->type != VIR_JSON_TYPE_OBJECT) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("blockstats stats entry was not in expected format")); - goto cleanup; - } - - if (virJSONValueObjectGetNumberUlong(stats, "wr_highest_offset", extent) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s statistic"), - "wr_highest_offset"); + if ((err = qemuMonitorJSONDevGetBlockExtent(dev, extent)) != + QEMU_MONITOR_BLOCK_EXTENT_ERROR_OK) { + qemuMonitorJSONReportBlockExtentError(err); goto cleanup; } } -- 2.1.0
On 09/25/2014 06:06 AM, Peter Krempa wrote:
From: Francesco Romani <fromani@redhat.com>
Management software wants to be able to allocate disk space on demand. To support this they need keep track of the space occupation of the block device. This information is reported by qemu as part of block stats.
This patch extend the block information in the bulk stats with the allocation information.
To keep the same behaviour a helper is extracted from qemuMonitorJSONGetBlockExtent in order to get per-device allocation information.
And this also sounds useful for my work on trying to add a flag for showing capacity information during dumpxml.
Signed-off-by: Francesco Romani <fromani@redhat.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt.c | 6 +++ src/qemu/qemu_driver.c | 27 +++++++++++++ src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 91 ++++++++++++++++++++++++++++++++++---------- 4 files changed, 105 insertions(+), 20 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c index aa83365..b078eed 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -21656,6 +21656,12 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * unsigned long long. * "block.<num>.errors" - Xen only: the 'oo_req' value as * unsigned long long. + * "block.<num>.allocation" - offset of the highest written sector + * as unsigned long long. + * "block.<num>.capacity" - logical size in bytes of the block device backing + * image as unsinged long long.
s/unsinged/unsigned/
+ * "block.<num>.physical" - physical size in bytes of the container of the + * backing image as unsigned long long. *
+typedef enum { + QEMU_MONITOR_BLOCK_EXTENT_ERROR_OK, + QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOPARENT, + QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOSTATS, + QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOOFFSET +} qemuMonitorBlockExtentError;
Prefer trailing commas in the enum. ACK. Border-line on whether 2 and 3 are a feature addition, or rounding out the completed implementation (and therefore a bug fix) of a feature already being introduced in 1.2.9. So I'll leave it up to you on whether to push or delay, if no one else chimes in. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 09/29/14 20:32, Eric Blake wrote:
On 09/25/2014 06:06 AM, Peter Krempa wrote:
From: Francesco Romani <fromani@redhat.com>
Management software wants to be able to allocate disk space on demand. To support this they need keep track of the space occupation of the block device. This information is reported by qemu as part of block stats.
This patch extend the block information in the bulk stats with the allocation information.
To keep the same behaviour a helper is extracted from qemuMonitorJSONGetBlockExtent in order to get per-device allocation information.
And this also sounds useful for my work on trying to add a flag for showing capacity information during dumpxml.
Signed-off-by: Francesco Romani <fromani@redhat.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt.c | 6 +++ src/qemu/qemu_driver.c | 27 +++++++++++++ src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 91 ++++++++++++++++++++++++++++++++++---------- 4 files changed, 105 insertions(+), 20 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c index aa83365..b078eed 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -21656,6 +21656,12 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * unsigned long long. * "block.<num>.errors" - Xen only: the 'oo_req' value as * unsigned long long. + * "block.<num>.allocation" - offset of the highest written sector + * as unsigned long long. + * "block.<num>.capacity" - logical size in bytes of the block device backing + * image as unsinged long long.
s/unsinged/unsigned/
+ * "block.<num>.physical" - physical size in bytes of the container of the + * backing image as unsigned long long. *
+typedef enum { + QEMU_MONITOR_BLOCK_EXTENT_ERROR_OK, + QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOPARENT, + QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOSTATS, + QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOOFFSET +} qemuMonitorBlockExtentError;
Prefer trailing commas in the enum.
ACK.
Border-line on whether 2 and 3 are a feature addition, or rounding out the completed implementation (and therefore a bug fix) of a feature already being introduced in 1.2.9. So I'll leave it up to you on whether to push or delay, if no one else chimes in.
They are now pushed as the release is out. Peter
On 09/25/2014 08:06 AM, Peter Krempa wrote:
From: Francesco Romani <fromani@redhat.com>
Management software wants to be able to allocate disk space on demand. To support this they need keep track of the space occupation of the block device. This information is reported by qemu as part of block stats.
This patch extend the block information in the bulk stats with the allocation information.
To keep the same behaviour a helper is extracted from qemuMonitorJSONGetBlockExtent in order to get per-device allocation information.
Signed-off-by: Francesco Romani <fromani@redhat.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt.c | 6 +++ src/qemu/qemu_driver.c | 27 +++++++++++++ src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 91 ++++++++++++++++++++++++++++++++++---------- 4 files changed, 105 insertions(+), 20 deletions(-)
,,,
int qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index b634121..c41a7fb 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1782,6 +1782,40 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, }
+typedef enum { + QEMU_MONITOR_BLOCK_EXTENT_ERROR_OK, + QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOPARENT, + QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOSTATS, + QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOOFFSET +} qemuMonitorBlockExtentError; + + +static int +qemuMonitorJSONDevGetBlockExtent(virJSONValuePtr dev, + unsigned long long *extent) +{ + virJSONValuePtr stats; + virJSONValuePtr parent; + + if ((parent = virJSONValueObjectGet(dev, "parent")) == NULL || + parent->type != VIR_JSON_TYPE_OBJECT) { + return QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOPARENT; + } + + if ((stats = virJSONValueObjectGet(parent, "stats")) == NULL || + stats->type != VIR_JSON_TYPE_OBJECT) { + return QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOSTATS; + } + + if (virJSONValueObjectGetNumberUlong(stats, "wr_highest_offset", + extent) < 0) { + return QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOOFFSET; + } + + return QEMU_MONITOR_BLOCK_EXTENT_ERROR_OK; +} + + int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, virHashTablePtr *ret_stats) { @@ -1908,6 +1942,9 @@ int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, goto cleanup; }
+ /* it's ok to not have this information here. Just skip silently. */ + qemuMonitorJSONDevGetBlockExtent(dev, &bstats->wr_highest_offset); + if (virHashAddEntry(hash, devname, bstats) < 0) goto cleanup; bstats = NULL; @@ -2077,6 +2114,36 @@ int qemuMonitorJSONGetBlockStatsParamsNumber(qemuMonitorPtr mon, return ret; }
+ +static int +qemuMonitorJSONReportBlockExtentError(qemuMonitorBlockExtentError error) +{ + switch (error) { + case QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOPARENT: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("blockstats parent entry was not in " + "expected format")); + break; + + case QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOSTATS: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("blockstats stats entry was not in " + "expected format")); +
Missing a break; (found by my monrning Coverity run) John
+ case QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOOFFSET: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot read %s statistic"), + "wr_highest_offset"); + break; + + case QEMU_MONITOR_BLOCK_EXTENT_ERROR_OK: + return 0; + } + + return -1; +} + + int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon, const char *dev_name, unsigned long long *extent) @@ -2111,9 +2178,8 @@ int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon,
for (i = 0; i < virJSONValueArraySize(devices); i++) { virJSONValuePtr dev = virJSONValueArrayGet(devices, i); - virJSONValuePtr stats; - virJSONValuePtr parent; const char *thisdev; + int err; if (!dev || dev->type != VIR_JSON_TYPE_OBJECT) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("blockstats device entry was not in expected format")); @@ -2137,24 +2203,9 @@ int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon, continue;
found = true; - if ((parent = virJSONValueObjectGet(dev, "parent")) == NULL || - parent->type != VIR_JSON_TYPE_OBJECT) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("blockstats parent entry was not in expected format")); - goto cleanup; - } - - if ((stats = virJSONValueObjectGet(parent, "stats")) == NULL || - stats->type != VIR_JSON_TYPE_OBJECT) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("blockstats stats entry was not in expected format")); - goto cleanup; - } - - if (virJSONValueObjectGetNumberUlong(stats, "wr_highest_offset", extent) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s statistic"), - "wr_highest_offset"); + if ((err = qemuMonitorJSONDevGetBlockExtent(dev, extent)) != + QEMU_MONITOR_BLOCK_EXTENT_ERROR_OK) { + qemuMonitorJSONReportBlockExtentError(err); goto cleanup; } }
On 09/25/14 14:06, Peter Krempa wrote:
Francesco Romani (1): qemu: bulk stats: add block allocation information
Peter Krempa (2): qemu: monitor: return block stats data as a hash to avoid disk mixup qemu: monitor: Add helper function to fill physical/virtual image size
src/libvirt.c | 6 + src/qemu/qemu_driver.c | 71 +++++++---- src/qemu/qemu_monitor.c | 31 +++-- src/qemu/qemu_monitor.h | 13 ++- src/qemu/qemu_monitor_json.c | 273 +++++++++++++++++++++++++++++++------------ src/qemu/qemu_monitor_json.h | 6 +- 6 files changed, 286 insertions(+), 114 deletions(-)
Ping? At least 1/1 is kind of a bugfix. Peter
participants (3)
-
Eric Blake -
John Ferlan -
Peter Krempa