7:06 a.m.
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
7:06 a.m.
New subject: [libvirt] [PATCH 1/3] qemu: monitor: return block stats data as a hash to avoid disk mixup
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
1:17 p.m.
New subject: [libvirt] [PATCH 1/3] qemu: monitor: return block stats data as a hash to avoid disk mixup
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
4:04 a.m.
New subject: [libvirt] [PATCH 1/3] qemu: monitor: return block stats data as a hash to avoid disk mixup
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
7:06 a.m.
New subject: [libvirt] [PATCH 2/3] qemu: monitor: Add helper function to fill physical/virtual image size
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
1:22 p.m.
New subject: [libvirt] [PATCH 2/3] qemu: monitor: Add helper function to fill physical/virtual image size
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
7:06 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: bulk stats: add block allocation information
From: Francesco Romani <fromani(a)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(a)redhat.com>
Signed-off-by: Peter Krempa <pkrempa(a)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
1:32 p.m.
New subject: [libvirt] [PATCH 3/3] qemu: bulk stats: add block allocation information
On 09/25/2014 06:06 AM, Peter Krempa wrote:
From: Francesco Romani <fromani(a)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(a)redhat.com>
Signed-off-by: Peter Krempa <pkrempa(a)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
4:21 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: bulk stats: add block allocation information
On 09/29/14 20:32, Eric Blake wrote:
On 09/25/2014 06:06 AM, Peter Krempa wrote:
> From: Francesco Romani <fromani(a)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(a)redhat.com>
> Signed-off-by: Peter Krempa <pkrempa(a)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
5:21 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: bulk stats: add block allocation information
On 09/25/2014 08:06 AM, Peter Krempa wrote:
From: Francesco Romani <fromani(a)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(a)redhat.com>
Signed-off-by: Peter Krempa <pkrempa(a)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;
}
}
11:12 a.m.
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
3704
days inactive
3710
days old
10 comments
3 participants
participants (3)
-
Eric Blake -
John Ferlan -
Peter Krempa