Re: [Qemu-devel] [PATCH V13 3/6] qmp: add recursive member in ImageInfo

[Eric Blake <eblake@redhat.com>]

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

On 05/24/2013 10:24 PM, Wenchao Xia wrote:
> New member *backing-image is added to reflect the backing chain
> status.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  block/qapi.c     |   16 +++++++++++++++-
>  qapi-schema.json |    5 ++++-
>  2 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/block/qapi.c b/block/qapi.c
> index 680ec23..cbef584 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -88,7 +88,21 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
>      return 0;
>  }
>  
> -/* @p_info will be set only on success. */
> +/**
> + * bdrv_query_image_info:
> + * @bs: block device to examine
> + * @p_info: location to store image information
> + * @errp: location to store error information
> + *
> + * Store "flat" image inforation in @p_info.

s/inforation/information/

> + *
> + * "Flat" means it does *not* query backing image information,
> + * i.e. (*pinfo)->has_backing_image will be set to false and
> + * (*pinfo)->backing_image to NULL even when the image does in fact have
> + * a backing image.
> + *
> + * @p_info will be set only on success. On error, store error in @errp.
> + */

Does this comment hunk belong in the previous patch?

>  void bdrv_query_image_info(BlockDriverState *bs,
>                             ImageInfo **p_info,
>                             Error **errp)
> diff --git a/qapi-schema.json b/qapi-schema.json
> index ef1f657..a02999d 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -236,6 +236,8 @@
>  #
>  # @snapshots: #optional list of VM snapshots
>  #
> +# @backing-image: #optional info of the backing image (since 1.6)
> +#
>  # Since: 1.3
>  #
>  ##
> @@ -245,7 +247,8 @@
>             '*actual-size': 'int', 'virtual-size': 'int',
>             '*cluster-size': 'int', '*encrypted': 'bool',
>             '*backing-filename': 'str', '*full-backing-filename': 'str',
> -           '*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'] } }
> +           '*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'],
> +           '*backing-image': 'ImageInfo' } }

The API change looks fine, except there is no code change to actually
populate the new field.  This hunk should probably be squashed with the
patch that implements the field.  Also, are you missing any changes to
qmp-commands.hx?

-- 
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: 621 bytes --]