Re: [Qemu-block] [PATCH 14/22] qcow2: delete bitmaps on truncate

[Max Reitz]

On 30.09.2016 12:53, Vladimir Sementsov-Ogievskiy wrote:
> This will not touch loaded bitmaps, as they are alredy removed from the
> image.

Well, yes, and I don't think that's ideal. :-)

I think marking them in_use instead of deleting them would be better.

Also, I'm not so sure whether this patch is an improvement at all.
Wouldn't it be better to have a qemu-img tool (and/or QMP interface) for
bitmap management and then making the user delete all the bitmaps before
the image can be resized?

Max

> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
>  block/qcow2-bitmap.c | 38 ++++++++++++++++++++++++++++++++++++++
>  block/qcow2.c        |  9 ++++++---
>  block/qcow2.h        |  2 ++
>  3 files changed, 46 insertions(+), 3 deletions(-)
> 
> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
> index 1c3abea..2642afe 100644
> --- a/block/qcow2-bitmap.c
> +++ b/block/qcow2-bitmap.c
> @@ -624,6 +624,44 @@ out:
>      return ret;
>  }
>  
> +
> +int qcow2_delete_bitmaps(BlockDriverState *bs)
> +{
> +    int ret;
> +    BDRVQcow2State *s = bs->opaque;
> +    uint8_t *dir;
> +    uint64_t dir_size;
> +    Qcow2BitmapDirEntry *e;
> +
> +    if (s->nb_bitmaps == 0) {
> +        /* No bitmaps - nothing to do */
> +        return 0;
> +    }
> +
> +    dir_size = s->bitmap_directory_size;
> +    dir = directory_read(bs, s->bitmap_directory_offset,
> +                         s->bitmap_directory_size, NULL);
> +    if (dir == NULL) {
> +        ret = -EINVAL;
> +        goto out;
> +    }
> +
> +    ret = directory_update(bs, NULL, 0, 0);
> +    if (ret < 0) {
> +        goto out;
> +    }
> +
> +    /* to be consistent, free bitmap only after successfull directory update 
> */
> +    for_each_bitmap_dir_entry(e, dir, dir_size) {
> +        free_bitmap_clusters(bs, e);
> +    }
> +
> +out:
> +    g_free(dir);
> +
> +    return ret;
> +}
> +
>  /* store_bitmap_data()
>   * Store bitmap to image, filling bitamp table accordingly.
>   */
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 8238205..aa967ed 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -2592,9 +2592,12 @@ static int qcow2_truncate(BlockDriverState *bs, 
> int64_t offset)
>  
>      /* cannot proceed if image has bitmaps */
>      if (s->nb_bitmaps) {
> -        /* FIXME */
> -        error_report("Can't resize an image which has bitmaps");
> -        return -ENOTSUP;
> +        /* FIXME: not loaded bitmaps will be lost */
> +        ret = qcow2_delete_bitmaps(bs);
> +        if (ret < 0) {
> +            error_report("Can't remove bitmaps from qcow2 on truncate");
> +            return ret;
> +        }
>      }
>  
>      /* shrinking is currently not supported */
> diff --git a/block/qcow2.h b/block/qcow2.h
> index dfcf4c6..af18efc 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -606,6 +606,8 @@ int qcow2_read_snapshots(BlockDriverState *bs);
>  /* qcow2-bitmap.c functions */
>  int qcow2_read_bitmaps(BlockDriverState *bs, Error **errp);
>  
> +int qcow2_delete_bitmaps(BlockDriverState *bs);
> +
>  /* qcow2-cache.c functions */
>  Qcow2Cache *qcow2_cache_create(BlockDriverState *bs, int num_tables);
>  int qcow2_cache_destroy(BlockDriverState* bs, Qcow2Cache *c);
>

signature.asc
Description: OpenPGP digital signature