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",
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list