Re: [Qemu-devel] [PATCH 06/11] qmp: add interface query-image

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1656 bytes --]

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', ...} }]

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]