On 09/07/2016 05:24 AM, Nikolay Shirokovskiy wrote:
---
src/qemu/qemu_domain.h | 1 +
src/qemu/qemu_monitor_json.c | 17 +++++++++++++++++
tests/qemumonitorjsontest.c | 36 ++++++++++++++++++++++++++++++++++++
3 files changed, 54 insertions(+)
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 13c0372..ea57111 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -337,6 +337,7 @@ struct qemuDomainDiskInfo {
bool tray_open;
bool empty;
int io_status;
+ unsigned long long guest_size;
a/k/a: 'virtual-size' in qemu and 'capacity' in libvirt
I probably would stick with capacity or something that says "disk"
rather than "guest".
};
Trying to determine/decide whether this patch should be "separated" and
perhaps made usable with/for the existing callers or whether patch 3
should use the block stats (qemuDomainGetStatsBlock) which already
handles some details that could be missing here (at least w/r/t backing
chains).
Another option is adjusting the qemuMonitorJSONDiskNameLookup code to be
more multi-purpose since it's using the "query-block" for
*BlockPullCommon (from Rebase and Pull) as well as from *BlockCommit and
it already checks/expects both "inserted" and "image" to be present
in
order to return that name.
typedef struct _qemuDomainHostdevPrivate qemuDomainHostdevPrivate;
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 3d0a120..7903b47 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1800,6 +1800,8 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
for (i = 0; i < virJSONValueArraySize(devices); i++) {
virJSONValuePtr dev = virJSONValueArrayGet(devices, i);
+ virJSONValuePtr inserted;
+ virJSONValuePtr image;
struct qemuDomainDiskInfo *info;
const char *thisdev;
const char *status;
@@ -1854,6 +1856,21 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
if (info->io_status < 0)
goto cleanup;
}
+
+ if ((inserted = virJSONValueObjectGetObject(dev, "inserted"))) {
+ if (!(image = virJSONValueObjectGetObject(inserted, "image"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("cannot read 'inserted/image'
value"));
+ goto cleanup;
+ }
Other code that checks "inserted" and "image" doesn't fail - it
just
ignores. If there's only going to be one consumer, then I don't believe
we want a failure such as this to affect other callers that may not
care. That could mean having some sort of "marker" in the returned
buffer that the fetch of "virtual-size" did occur (or just using not
zero). It would be a shame to have some unexpected failures for a field
that nothing else uses especially since the *GetBlockInfo is being used
during process launch and reconnect (via qemuProcessRefreshDisks).
Futhermore, what if there's a "backing-image" for "this"
particular
path? Will that be important for the pull backups support? Without
looking at patch 3 I would think so...
John
+
+ if (virJSONValueObjectGetNumberUlong(image, "virtual-size",
+ &info->guest_size) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("cannot read 'inserted/image/virtual-size'
value"));
+ goto cleanup;
+ }
+ }
}
ret = 0;
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index d3ec3b1..d1922fd 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -50,6 +50,23 @@ const char *queryBlockReply =
" \"locked\": false,"
" \"removable\": false,"
" \"inserted\": {"
+" \"image\": {"
+" \"virtual-size\": 68719476736,"
+" \"filename\":
\"/home/zippy/work/tmp/gentoo.qcow2\","
+" \"cluster-size\": 65536,"
+" \"format\": \"qcow2\","
+" \"actual-size\": 156901376,"
+" \"format-specific\": {"
+" \"type\": \"qcow2\","
+" \"data\": {"
+" \"compat\": \"1.1\","
+" \"lazy-refcounts\": true,"
+" \"refcount-bits\": 16,"
+" \"corrupt\": false"
+" }"
+" },"
+" \"dirty-flag\": false"
+" },"
" \"iops_rd\": 5,"
" \"iops_wr\": 6,"
" \"ro\": false,"
@@ -78,6 +95,13 @@ const char *queryBlockReply =
" \"locked\": false,"
" \"removable\": false,"
" \"inserted\": {"
+" \"image\": {"
+" \"virtual-size\": 34359738368,"
+" \"filename\":
\"/home/zippy/test.bin\","
+" \"format\": \"raw\","
+" \"actual-size\": 34359738368,"
+" \"dirty-flag\": false"
+" },"
" \"iops_rd\": 0,"
" \"iops_wr\": 0,"
" \"ro\": false,"
@@ -99,6 +123,13 @@ const char *queryBlockReply =
" \"locked\": true,"
" \"removable\": true,"
" \"inserted\": {"
+" \"image\": {"
+" \"virtual-size\": 17179869184,"
+" \"filename\":
\"/home/zippy/test.bin\","
+" \"format\": \"raw\","
+" \"actual-size\": 17179869184,"
+" \"dirty-flag\": false"
+" },"
" \"iops_rd\": 0,"
" \"iops_wr\": 0,"
" \"ro\": true,"
@@ -1413,6 +1444,8 @@ testQemuMonitorJSONqemuMonitorJSONGetBlockInfo(const void *data)
if (VIR_ALLOC(info) < 0)
goto cleanup;
+ info->guest_size = 68719476736ULL;
+
if (virHashAddEntry(expectedBlockDevices, "virtio-disk0", info) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
"Unable to create expectedBlockDevices hash table");
@@ -1422,6 +1455,8 @@ testQemuMonitorJSONqemuMonitorJSONGetBlockInfo(const void *data)
if (VIR_ALLOC(info) < 0)
goto cleanup;
+ info->guest_size = 34359738368ULL;
+
if (virHashAddEntry(expectedBlockDevices, "virtio-disk1", info) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
"Unable to create expectedBlockDevices hash table");
@@ -1434,6 +1469,7 @@ testQemuMonitorJSONqemuMonitorJSONGetBlockInfo(const void *data)
info->locked = true;
info->removable = true;
info->tray = true;
+ info->guest_size = 17179869184ULL;
if (virHashAddEntry(expectedBlockDevices, "ide0-1-0", info) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",