[14/22] qcow2: delete bitmaps on truncate
diff mbox

Message ID 1475232808-4852-15-git-send-email-vsementsov@virtuozzo.com
State New
Headers show

Commit Message

Vladimir Sementsov-Ogievskiy Sept. 30, 2016, 10:53 a.m. UTC
This will not touch loaded bitmaps, as they are alredy removed from the
image.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/qcow2-bitmap.c | 38 ++++++++++++++++++++++++++++++++++++++
 block/qcow2.c        |  9 ++++++---
 block/qcow2.h        |  2 ++
 3 files changed, 46 insertions(+), 3 deletions(-)

Comments

Max Reitz Oct. 7, 2016, 7:58 p.m. UTC | #1
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 <vsementsov@virtuozzo.com>
> ---
>  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);
>

Patch
diff mbox

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);