Re: [Qemu-devel] [PATCH] qcow2 resize with snapshot

[Eric Blake <eblake@redhat.com>]

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

On 05/27/2016 02:14 AM, zhangzhiming wrote:
> Hi, i modified my code for qcow2 resize, and delete some code related to 
> qemu monitor.
> 
> and thanks for the review.
> 
> zhangzhiming
> zhangzhiming02@meituan.com <mailto:zhangzhiming02@meituan.com>

Still missing a Signed-off-by designation, so it can't be applied as-is.

> 
> ---
>  block.c                  |   19 +++++++++++++++++++
>  block/qcow2-cluster.c    |   29 +++++++++++++++++++++++++++++
>  block/qcow2-snapshot.c   |   34 ++++++++++++++++++++++++----------
>  block/qcow2.c            |   29 ++++++++++++++++++++---------
>  block/qcow2.h            |    1 +
>  include/block/snapshot.h |    1 +
>  6 files changed, 94 insertions(+), 19 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 18a497f..729f820 100644
> --- a/block.c
> +++ b/block.c
> @@ -2631,6 +2631,25 @@ int bdrv_truncate(BlockDriverState *bs, int64_t offset)
>      return ret;
>  }
>  
> +int bdrv_apply_snapshot(BlockDriverState *bs, const char *snapshot_id,
> +                        uint64_t snapshot_size)
> +{
> +    int ret = bdrv_snapshot_goto(bs, snapshot_id);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +
> +    ret = refresh_total_sectors(bs, snapshot_size >> BDRV_SECTOR_BITS);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +    bdrv_dirty_bitmap_truncate(bs); /* void return */
> +    if (bs->blk) {
> +        blk_dev_resize_cb(bs->blk); /* void return too */

The comments don't add anything here.

> +    }
> +    return ret;
> +}
> +
>  /**
>   * Length of a allocated file in bytes. Sparse files are counted by actual
>   * allocated space. Return < 0 if error or unknown.
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 31ecc10..e4c5c05 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -31,6 +31,35 @@
>  #include "block/qcow2.h"
>  #include "trace.h"
>  
> +int shrink_l1_table(BlockDriverState *bs, int64_t new_l1_size)
> +{
> +    BDRVQcow2State *s = bs->opaque;
> +    int64_t old_l1_size = s->l1_size;
> +    s->l1_size = new_l1_size;
> +    int ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset,
> +                                                     s->l1_size, 1);

Indentation is off.

> +    if (ret < 0) {
> +        return ret;
> +    }
> +
> +    s->l1_size = old_l1_size;
> +    ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset,
> +                                                     s->l1_size, -1);

and again.

> +    if (ret < 0) {
> +        return ret;
> +    }
> +    s->l1_size = new_l1_size;
> +

> +++ b/block/qcow2-snapshot.c

> @@ -552,10 +562,12 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id)
>       * Now update the in-memory L1 table to be in sync with the on-disk one. We
>       * need to do this even if updating refcounts failed.
>       */
> -    for(i = 0;i < s->l1_size; i++) {
> +    memset(s->l1_table, 0, s->l1_size*sizeof(uint64_t));
> +    for(i = 0;i < sn->l1_size; i++) {

As long as you are touching this, use the preferred spacing:

for (i = 0; i < sn->l1_size; i++) {

>          s->l1_table[i] = be64_to_cpu(sn_l1_table[i]);
>      }
>  
> +
>      if (ret < 0) {

Why the added blank line?


> +++ b/block/qcow2.c
> @@ -2501,22 +2501,33 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset)
>          return -EINVAL;
>      }
>  
> -    /* cannot proceed if image has snapshots */
> -    if (s->nb_snapshots) {
> -        error_report("Can't resize an image which has snapshots");
> +    bool v3_truncate = (s->qcow_version == 3);
> +
> +    /* cannot proceed if image has snapshots and qcow_version is not 3*/

Space before */

> +    if (!v3_truncate && s->nb_snapshots) {
> +        error_report("Can't resize an image which has snapshots and "
> +                     "qcow_version is not 3");
>          return -ENOTSUP;
>      }
>  
> -    /* shrinking is currently not supported */
> -    if (offset < bs->total_sectors * 512) {
> -        error_report("qcow2 doesn't support shrinking images yet");
> +    /* shrinking is supported from version 3*/

and again

> +    if (!v3_truncate && offset < bs->total_sectors * 512) {
> +        error_report("qcow2 doesn't support shrinking images yet while"
> +                     " qcow_version is not 3");
>          return -ENOTSUP;
>      }


-- 
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: 604 bytes --]