Patchwork [06/11] qmp: add interface query-image

login
register
mail settings
Submitter Wayne Xia
Date Dec. 29, 2012, 8:45 a.m.
Message ID <1356770725-4804-7-git-send-email-xiawenc@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/208638/
State New
Headers show

Comments

Wayne Xia - Dec. 29, 2012, 8:45 a.m.
This mirror function will return all image info including
snapshots. Now Qemu have both query-image and query-block
interfaces, and qemu-img share the code for image info
retrieving with qemu emulator.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 block.c          |   16 ++++++++++++++++
 qapi-schema.json |   11 +++++++++++
 2 files changed, 27 insertions(+), 0 deletions(-)
Eric Blake - Jan. 4, 2013, 11:48 p.m.
On 12/29/2012 01:45 AM, Wenchao Xia wrote:
>   This mirror function will return all image info including
> snapshots. Now Qemu have both query-image and query-block
> interfaces, and qemu-img share the code for image info
> retrieving with qemu emulator.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  block.c          |   16 ++++++++++++++++
>  qapi-schema.json |   11 +++++++++++
>  2 files changed, 27 insertions(+), 0 deletions(-)

Should there be a counterpart change to qmp-commands.hx, demonstrating
the type of output expected?

>  
>  ##
> +# @query-image:
> +#
> +# Get a list of BlockImage for all virtual block devices.
> +#
> +# Returns: a list of @BlockImage describing each virtual block device

Here, you use BlockImage twice,

> +#
> +# Since: 1.4
> +##
> +{ 'command': 'query-image', 'returns': ['ImageInfo'] }

but here, the type is named ImageInfo.

This interface is weak - it tells me a list of filenames that are in
use, but doesn't tell me which element of the array is tied to which device.

Also, this command is singular, but returns a plural listing; elsewhere,
we have used plurals (think query-commands).

I'd rather see something like:

{ 'type': 'DeviceImageInfo',
  'data': {'device': 'str', 'info': 'ImageInfo' } }
{ 'command': 'query-images', 'returns': ['DeviceImageInfo'] }

so that I can get the pairings between
 [{ 'device':'virtio0', 'info':{ 'filename':'/path1', ...} },
  { 'device':'virtio1', 'info':{ 'filename':'/path2', ...} }]

Patch

diff --git a/block.c b/block.c
index 9dcecec..0e9f414 100644
--- a/block.c
+++ b/block.c
@@ -2964,6 +2964,22 @@  ImageInfo *bdrv_query_image_info(BlockDriverState *bs, Error **errp)
     return info;
 }
 
+ImageInfoList *qmp_query_image(Error **errp)
+{
+    ImageInfoList *head = NULL, **p_next = &head;
+    BlockDriverState *bs;
+
+    QTAILQ_FOREACH(bs, &bdrv_states, list) {
+        ImageInfoList *info = g_malloc0(sizeof(*info));
+        info->value = bdrv_query_image_info(bs, NULL);
+
+        *p_next = info;
+        p_next = &info->next;
+    }
+
+    return head;
+}
+
 BlockInfo *bdrv_query_block_info(BlockDriverState *bs)
 {
     BlockInfo *info = g_malloc0(sizeof(*info));
diff --git a/qapi-schema.json b/qapi-schema.json
index 5dfa052..40f96f3 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -720,6 +720,17 @@ 
 { 'command': 'query-block', 'returns': ['BlockInfo'] }
 
 ##
+# @query-image:
+#
+# Get a list of BlockImage for all virtual block devices.
+#
+# Returns: a list of @BlockImage describing each virtual block device
+#
+# Since: 1.4
+##
+{ 'command': 'query-image', 'returns': ['ImageInfo'] }
+
+##
 # @BlockDeviceStats:
 #
 # Statistics of a virtual block device or a block backing device.