Re: [Qemu-devel] [PATCH V3 03/11] block: add snapshot and image info query function

[Luiz Capitulino <lcapitulino@redhat.com>]

On Mon, 14 Jan 2013 15:09:39 +0800
Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:

>   This patch added function bdrv_query_image_info() and
> bdrv_query_snapshot_infolist(), which will return info in qmp object
> format. The implementation code are mostly copied from collect_image_info()
> and collect_snapshot() in qemu-img.c.
>   To help filter out snapshot info not needed, a call back function is
> added in bdrv_query_snapshot_infolist().
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
>  block.c               |  117 +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/block/block.h |    9 ++++
>  2 files changed, 126 insertions(+), 0 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 5f95da5..81765a3 100644
> --- a/block.c
> +++ b/block.c
> @@ -2846,6 +2846,123 @@ int coroutine_fn bdrv_co_is_allocated_above(BlockDriverState *top,
>      return 0;
>  }
>  
> +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) {
> +        if (errp != NULL) {

This check is already done by error_setg(), you don't have to do it.

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

This doesn't tell much about the error cause. The best thing to do here
is to guarantee that drv->bdrv_snapshot_list() callbacks always set errno
and then use error_setg_errno().

> +        }
> +        return NULL;
> +    }
> +
> +    for (i = 0; i < sn_count; i++) {
> +        if (filter && filter(&sn_tab[i], opaque) != 0) {
> +            continue;
> +        }
> +
> +        info_list = g_new0(SnapshotInfoList, 1);
> +
> +        info_list->value                = g_new0(SnapshotInfo, 1);
> +        info_list->value->id            = g_strdup(sn_tab[i].id_str);
> +        info_list->value->name          = g_strdup(sn_tab[i].name);
> +        info_list->value->vm_state_size = sn_tab[i].vm_state_size;
> +        info_list->value->date_sec      = sn_tab[i].date_sec;
> +        info_list->value->date_nsec     = sn_tab[i].date_nsec;
> +        info_list->value->vm_clock_sec  = sn_tab[i].vm_clock_nsec / 1000000000;
> +        info_list->value->vm_clock_nsec = sn_tab[i].vm_clock_nsec % 1000000000;
> +
> +        /* XXX: waiting for the qapi to support qemu-queue.h types */
> +        if (!cur_item) {
> +            head = cur_item = info_list;
> +        } else {
> +            cur_item->next = info_list;
> +            cur_item = info_list;
> +        }
> +
> +    }
> +
> +    g_free(sn_tab);
> +    return head;
> +}
> +
> +/* collect all internal snapshot info in a image for ImageInfo */
> +static void collect_snapshots_info(BlockDriverState *bs,
> +                                   ImageInfo *info,
> +                                   Error **errp)
> +{
> +    SnapshotInfoList *info_list;
> +
> +    info_list = bdrv_query_snapshot_infolist(bs, NULL, NULL, errp);
> +    if (info_list != NULL) {
> +        info->has_snapshots = true;
> +        info->snapshots = info_list;
> +    }
> +    return;
> +}
> +
> +static void collect_image_info(BlockDriverState *bs,
> +                               ImageInfo *info)

IMO, this and the next function have to split from this patch.

> +{
> +    uint64_t total_sectors;
> +    char backing_filename[1024];
> +    char backing_filename2[1024];
> +    BlockDriverInfo bdi;
> +    const char *filename;
> +
> +    filename = bdrv_get_filename(bs);
> +    bdrv_get_geometry(bs, &total_sectors);
> +
> +    info->filename        = g_strdup(filename);
> +    info->format          = g_strdup(bdrv_get_format_name(bs));
> +    info->virtual_size    = total_sectors * 512;
> +    info->actual_size     = bdrv_get_allocated_file_size(bs);
> +    info->has_actual_size = info->actual_size >= 0;
> +    if (bdrv_is_encrypted(bs)) {
> +        info->encrypted = true;
> +        info->has_encrypted = true;
> +    }
> +    if (bdrv_get_info(bs, &bdi) >= 0) {
> +        if (bdi.cluster_size != 0) {
> +            info->cluster_size = bdi.cluster_size;
> +            info->has_cluster_size = true;
> +        }
> +        info->dirty_flag = bdi.is_dirty;
> +        info->has_dirty_flag = true;
> +    }
> +    bdrv_get_backing_filename(bs, backing_filename, sizeof(backing_filename));
> +    if (backing_filename[0] != '\0') {
> +        info->backing_filename = g_strdup(backing_filename);
> +        info->has_backing_filename = true;
> +        bdrv_get_full_backing_filename(bs, backing_filename2,
> +                                       sizeof(backing_filename2));
> +
> +        if (strcmp(backing_filename, backing_filename2) != 0) {
> +            info->full_backing_filename = g_strdup(backing_filename2);
> +            info->has_full_backing_filename = true;
> +        }
> +
> +        if (bs->backing_format[0]) {
> +            info->backing_filename_format = g_strdup(bs->backing_format);
> +            info->has_backing_filename_format = true;
> +        }
> +    }
> +}
> +
> +ImageInfo *bdrv_query_image_info(BlockDriverState *bs, Error **errp)
> +{
> +    ImageInfo *info = g_new0(ImageInfo, 1);
> +    collect_image_info(bs, info);
> +    collect_snapshots_info(bs, info, errp);
> +    return info;
> +}
> +
>  BlockInfo *bdrv_query_info(BlockDriverState *bs)
>  {
>      BlockInfo *info = g_malloc0(sizeof(*info));
> diff --git a/include/block/block.h b/include/block/block.h
> index a0fc2a6..67f0d13 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -316,8 +316,17 @@ 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);
> +ImageInfo *bdrv_query_image_info(BlockDriverState *bs, Error **errp);
>  BlockInfo *bdrv_query_info(BlockDriverState *s);
>  BlockStats *bdrv_query_stats(const BlockDriverState *bs);
> +
>  int bdrv_can_snapshot(BlockDriverState *bs);
>  int bdrv_is_snapshot(BlockDriverState *bs);
>  BlockDriverState *bdrv_snapshots(void);