Re: [Qemu-devel] [PATCH V7 05/14] block: add snapshot info query function bdrv_query_snapshot_infolist()

[Stefan Hajnoczi]

On Tue, Feb 26, 2013 at 06:40:19PM +0800, Wenchao Xia wrote:
> diff --git a/include/block/block.h b/include/block/block.h
> index e6d915c..51a14f2 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -319,6 +319,13 @@ void bdrv_get_backing_filename(BlockDriverState *bs,
>                                 char *filename, int filename_size);
>  void bdrv_get_full_backing_filename(BlockDriverState *bs,
>                                      char *dest, size_t sz);
> +
> +typedef int (*SnapshotFilterFunc)(const QEMUSnapshotInfo *sn, void *opaque);
> +/* assume bs is already opened, use qapi_free_* to free returned value. */
> +SnapshotInfoList *bdrv_query_snapshot_infolist(BlockDriverState *bs,
> +                                               SnapshotFilterFunc filter,
> +                                               void *opaque,
> +                                               Error **errp);

bdrv_query_snapshot_infolist() uses SnapshotInfoList, therefore it makes
sense to pass a SnapshotInfo argument to SnapshotFilterFunc() instead of
a QEMUSnapshotInfo.  The QEMUSnapshotInfo type is an implementation
detail that should not be part of the API.

> diff --git a/qemu-img.c b/qemu-img.c
> index b650d17..1034cc5 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -1280,7 +1280,13 @@ static ImageInfoList *collect_image_info_list(const 
> char *filename,
>  
>          info = g_new0(ImageInfo, 1);
>          collect_image_info(bs, info, filename);
> -        collect_snapshots(bs, info);
> +        if (bdrv_can_read_snapshot(bs)) {
> +            info->snapshots = bdrv_query_snapshot_infolist(bs, NULL,
> +                                                           NULL, NULL);
> +            if (info->snapshots) {
> +                info->has_snapshots = true;
> +            }
> +        }

bdrv_can_read_snapshot() is not needed.  The code works fine without the
if check.