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

[zhangzhiming]

hi, thanks for the review and the format of code changed.

have a good day!

zhangzhiming

address@hidden

Signed-off-by: zhangzhiming <zhangzhiming02@meituan.com>

---

 block.c                |    4 ++--

 block/qcow2-cluster.c  |    4 ++--

 block/qcow2-snapshot.c |    5 ++---

 block/qcow2.c          |    4 ++--

 4 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/block.c b/block.c

index 729f820..b6f2004 100644

--- a/block.c

+++ b/block.c

@@ -2643,9 +2643,9 @@ int bdrv_apply_snapshot(BlockDriverState *bs, const char *snapshot_id,

     if (ret < 0) {

         return ret;

     }

-    bdrv_dirty_bitmap_truncate(bs); /* void return */

+    bdrv_dirty_bitmap_truncate(bs);

     if (bs->blk) {

-        blk_dev_resize_cb(bs->blk); /* void return too */

+        blk_dev_resize_cb(bs->blk);

     }

     return ret;

 }

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c

index e4c5c05..f921fd8 100644

--- a/block/qcow2-cluster.c

+++ b/block/qcow2-cluster.c

@@ -37,14 +37,14 @@ int shrink_l1_table(BlockDriverState *bs, int64_t new_l1_size)

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

+                                             s->l1_size, 1);

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

+                                         s->l1_size, -1);

     if (ret < 0) {

         return ret;

     }

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c

index 9c77096..1ed0e18 100644

--- a/block/qcow2-snapshot.c

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

@@ -562,12 +562,11 @@ 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.

      */

-    memset(s->l1_table, 0, s->l1_size*sizeof(uint64_t));

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

+    memset(s->l1_table, 0, s->l1_size * sizeof(uint64_t));

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

         s->l1_table[i] = be64_to_cpu(sn_l1_table[i]);

     }

 

-

     if (ret < 0) {

         goto fail;

     }

diff --git a/block/qcow2.c b/block/qcow2.c

index 70ec890..58b53e1 100644

--- a/block/qcow2.c

+++ b/block/qcow2.c

@@ -2503,14 +2503,14 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset)

 

     bool v3_truncate = (s->qcow_version == 3);

 

-    /* cannot proceed if image has snapshots and qcow_version is not 3*/

+    /* cannot proceed if image has snapshots and qcow_version is not 3 */

     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 supported from version 3*/

+    /* shrinking is supported from version 3 */

     if (!v3_truncate && offset < bs->total_sectors * 512) {

         error_report("qcow2 doesn't support shrinking images yet while"

                      " qcow_version is not 3");

-- 

1.7.1

On Jun 1, 2016, at 12:50 AM, Eric Blake <address@hidden> wrote:

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
address@hidden <mailto:address@hidden>

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