Re: [Qemu-devel] [PATCH] Re: [PATCH] qcow2: Patch for shrinking qcow2 disk image

[lijun]

Hi list,
   Ping. Could someone help to review this patch, thank you very much. 
The following is the patch url:
http://lists.nongnu.org/archive/html/qemu-devel/2014-04/msg00805.html

On 04/05/2014 12:52 AM, Jun Li wrote:
> As the realization of raw shrinking, so when do qcow2 shrinking, do not
> check l1 entries. When resize to size1(size1 < "disk size"), the custemer
> knows this will destory the data. So no need to check the l1 entries which
> is used or not.
> BTW, there will possible be "disk size" > "virtual size". As this, when do
> qcow2 shrinking, we should to discard the unused clusters(reduce the refcount
> of the unused clusters, release these clusters).
> This is the update patch. thx.
>
> Signed-off-by: Jun Li <address@hidden>
> ---
>   block/qcow2-cluster.c  | 16 +++++++++++-----
>   block/qcow2-snapshot.c |  2 +-
>   block/qcow2.c          | 10 ++--------
>   block/qcow2.h          |  4 ++--
>   4 files changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 60a6910..5ccc2a4 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -29,8 +29,8 @@
>   #include "block/qcow2.h"
>   #include "trace.h"
>   
> -int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
> -                        bool exact_size)
> +int qcow2_truncate_l1_table(BlockDriverState *bs, uint64_t min_size,
> +                            bool exact_size)
>   {
>       BDRVQcowState *s = bs->opaque;
>       int new_l1_size2, ret, i;
> @@ -39,7 +39,7 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t 
> min_size,
>       int64_t new_l1_table_offset, new_l1_size;
>       uint8_t data[12];
>   
> -    if (min_size <= s->l1_size)
> +    if (min_size == s->l1_size)
>           return 0;
>   
>       if (exact_size) {
> @@ -66,7 +66,13 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t 
> min_size,
>   
>       new_l1_size2 = sizeof(uint64_t) * new_l1_size;
>       new_l1_table = g_malloc0(align_offset(new_l1_size2, 512));
> -    memcpy(new_l1_table, s->l1_table, s->l1_size * sizeof(uint64_t));
> +
> +    /* shrinking or growing l1 table */
> +    if (min_size < s->l1_size) {
> +        memcpy(new_l1_table, s->l1_table, new_l1_size2);
> +    } else {
> +        memcpy(new_l1_table, s->l1_table, s->l1_size * sizeof(uint64_t));
> +    }
>   
>       /* write new table (align to cluster) */
>       BLKDBG_EVENT(bs->file, BLKDBG_L1_GROW_ALLOC_TABLE);
> @@ -550,7 +556,7 @@ static int get_cluster_table(BlockDriverState *bs, uint64_t 
> offset,
>   
>       l1_index = offset >> (s->l2_bits + s->cluster_bits);
>       if (l1_index >= s->l1_size) {
> -        ret = qcow2_grow_l1_table(bs, l1_index + 1, false);
> +        ret = qcow2_truncate_l1_table(bs, l1_index + 1, false);
>           if (ret < 0) {
>               return ret;
>           }
> diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
> index 0aa9def..6ba460e 100644
> --- a/block/qcow2-snapshot.c
> +++ b/block/qcow2-snapshot.c
> @@ -483,7 +483,7 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char 
> *snapshot_id)
>        * L1 table of the snapshot. If the snapshot L1 table is smaller, the
>        * current one must be padded with zeros.
>        */
> -    ret = qcow2_grow_l1_table(bs, sn->l1_size, true);
> +    ret = qcow2_truncate_l1_table(bs, sn->l1_size, true);
>       if (ret < 0) {
>           goto fail;
>       }
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 333e26d..4fe580e 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1858,7 +1858,7 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t 
> offset)
>       int64_t new_l1_size;
>       int ret;
>   
> -    if (offset & 511) {
> +    if (offset <= 0 || offset & 511) {
>           error_report("The new size must be a multiple of 512");
>           return -EINVAL;
>       }
> @@ -1869,14 +1869,8 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t 
> offset)
>           return -ENOTSUP;
>       }
>   
> -    /* shrinking is currently not supported */
> -    if (offset < bs->total_sectors * 512) {
> -        error_report("qcow2 doesn't support shrinking images yet");
> -        return -ENOTSUP;
> -    }
> -
>       new_l1_size = size_to_l1(s, offset);
> -    ret = qcow2_grow_l1_table(bs, new_l1_size, true);
> +    ret = qcow2_truncate_l1_table(bs, new_l1_size, true);
>       if (ret < 0) {
>           return ret;
>       }
> diff --git a/block/qcow2.h b/block/qcow2.h
> index b49424b..fa36930 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -499,8 +499,8 @@ int qcow2_pre_write_overlap_check(BlockDriverState *bs, int 
> ign, int64_t offset,
>                                     int64_t size);
>   
>   /* qcow2-cluster.c functions */
> -int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
> -                        bool exact_size);
> +int qcow2_truncate_l1_table(BlockDriverState *bs, uint64_t min_size,
> +                            bool exact_size);
>   int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index);
>   void qcow2_l2_cache_reset(BlockDriverState *bs);
>   int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset);