Re: [Qemu-devel] [PATCH V4 04/13] block: add snapshot info query function bdrv_query_snapshot_infolist()

[Eric Blake <eblake@redhat.com>]

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

On 01/17/2013 12:40 AM, Wenchao Xia wrote:
>   This patch add function bdrv_query_snapshot_infolist(), which will
> return snapshot info of an image in qmp object format. The implementation
> code are mostly copied from qemu-img.c with modification to fit more
> for qmp based block layer API.
>   To help filter out snapshot info not needed, a call back function is
> added in bdrv_query_snapshot_infolist().
>   bdrv_can_read_snapshot() should be called before call this function,
> to avoid got *errp set unexpectly.

s/unexpectly/unexpectedly/

> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
>  block.c               |   46 ++++++++++++++++++++++++++++++++++++++++++++++
>  include/block/block.h |    7 +++++++
>  2 files changed, 53 insertions(+), 0 deletions(-)
> 

> +SnapshotInfoList *bdrv_query_snapshot_infolist(BlockDriverState *bs,
> +                                               SnapshotFilterFunc filter,
> +                                               void *opaque,
> +                                               Error **errp)
> +{
> +    int i, sn_count;
> +    QEMUSnapshotInfo *sn_tab = NULL;
> +    SnapshotInfoList *info_list, *cur_item = NULL, *head = NULL;
> +
> +    sn_count = bdrv_snapshot_list(bs, &sn_tab);
> +    if (sn_count < 0) {
> +        /* Fix me:  set errp in bdrv_snapshot_list() instead of here */

You didn't remove this fixme anywhere in the series; it would be better
to respin things to avoid a net gain of fixmes.

> +        error_setg(errp, "bdrv_snapshot_list: error %d\n", sn_count);
> +        return NULL;
> +    }

-- 
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 --]