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",