Re: [Qemu-devel] [PATCH V6 08/14] qmp: add interface query-images.

[Eric Blake]

On 02/18/2013 07:09 AM, Wenchao Xia wrote:
>   This mirror function will return all image info including
> snapshots, if specified backing image's info will also be returned.
> Now Qemu have both query-images and query-block interfaces.
> 
> Signed-off-by: Wenchao Xia <address@hidden>
> ---
>  block.c          |   83 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  qapi-schema.json |   35 ++++++++++++++++++++++
>  qmp-commands.hx  |   76 +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 194 insertions(+), 0 deletions(-)
> 

> +++ b/qapi-schema.json
> @@ -245,6 +245,22 @@
>             '*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'] 
> } }
>  
>  ##
> +# @DeviceImageInfo:
> +#
> +# Information about an image used by QEMU block device

s/by/by a/

> +#
> +# @device: name of the block device
> +#
> +# @image: #optional info of the image used.

Not consistent on whether there is a trailing '.' (looks like most
one-line docstrings don't have one, but it is not a hard rule)

>  
>  ##
> +# @query-images:
> +#
> +# Get a list of DeviceImageInfo for all virtual block devices.
> +#
> +# @device: #optional the name of the device to get image info from. If not
> +#          specified, all block devices's image info would be returned.

s/devices's image info would be returned/devices will be queried/

> +# @backing: #optional whether show info of backing images if they exist. If 
> not
> +#          specified, only the top image info of the block devices would be
> +#          returned.

A bit verbose; I'd suggest:

@backing: #optional true to show information on backing images, false or
omitted to show just the top image of a block device

> +#
> +# Returns: a list of @DeviceImageInfo describing each virtual block device
> +#
> +# Since: 1.5
> +##
> +{ 'command': 'query-images',
> +  'data': { '*device': 'str', '*backing': 'bool' },
> +  'returns': ['DeviceImageInfo'] }
> +
> +##

At any rate, I like how the interface has turned out, when my only
comments are regarding grammar of the documentation.

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

signature.asc
Description: OpenPGP digital signature