* [Qemu-devel] [PATCH] qcow2 resize with snapshot @ 2016-05-27 8:14 zhangzhiming 2016-05-31 16:50 ` Eric Blake 0 siblings, 1 reply; 7+ messages in thread From: zhangzhiming @ 2016-05-27 8:14 UTC (permalink / raw) Cc: qemu-block, qemu-devel, Kevin Wolf, eblake, Max Reitz, lihuiba 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> --- 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 */ + } + 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); + 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); + if (ret < 0) { + return ret; + } + s->l1_size = new_l1_size; + + uint32_t be_l1_size = cpu_to_be32(s->l1_size); + ret = bdrv_pwrite_sync(bs->file->bs, offsetof(QCowHeader, l1_size), + &be_l1_size, sizeof(be_l1_size)); + if (ret < 0) { + return ret; + } + + return 0; +} + int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, bool exact_size) { diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c index 5f4a17e..9c77096 100644 --- a/block/qcow2-snapshot.c +++ b/block/qcow2-snapshot.c @@ -477,13 +477,6 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) } sn = &s->snapshots[snapshot_index]; - if (sn->disk_size != bs->total_sectors * BDRV_SECTOR_SIZE) { - error_report("qcow2: Loading snapshots with different disk " - "size is not implemented"); - ret = -ENOTSUP; - goto fail; - } - /* * Make sure that the current L1 table is big enough to contain the whole * L1 table of the snapshot. If the snapshot L1 table is smaller, the @@ -505,7 +498,7 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) * Decrease the refcount referenced by the old one only when the L1 * table is overwritten. */ - sn_l1_table = g_try_malloc0(cur_l1_bytes); + sn_l1_table = g_try_malloc0(sn_l1_bytes); if (cur_l1_bytes && sn_l1_table == NULL) { ret = -ENOMEM; goto fail; @@ -530,11 +523,28 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) } ret = bdrv_pwrite_sync(bs->file->bs, s->l1_table_offset, sn_l1_table, - cur_l1_bytes); + sn_l1_bytes); + if (ret < 0) { + goto fail; + } + + /* write updated header.size */ + uint64_t be_disk_size = cpu_to_be64(sn->disk_size); + ret = bdrv_pwrite_sync(bs->file->bs, offsetof(QCowHeader, size), + &be_disk_size, sizeof(uint64_t)); if (ret < 0) { goto fail; } + uint32_t be_l1_size = cpu_to_be32(sn->l1_size); + ret = bdrv_pwrite_sync(bs->file->bs, offsetof(QCowHeader, l1_size), + &be_l1_size, sizeof(be_l1_size)); + if (ret < 0) { + goto fail; + } + + s->l1_vm_state_index = sn->l1_size; + /* * Decrease refcount of clusters of current L1 table. * @@ -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++) { s->l1_table[i] = be64_to_cpu(sn_l1_table[i]); } + if (ret < 0) { goto fail; } @@ -563,6 +575,7 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) g_free(sn_l1_table); sn_l1_table = NULL; + s->l1_size = sn->l1_size; /* * Update QCOW_OFLAG_COPIED in the active L1 table (it may have changed * when we decreased the refcount of the old snapshot. @@ -675,6 +688,7 @@ int qcow2_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab) sn_info->date_sec = sn->date_sec; sn_info->date_nsec = sn->date_nsec; sn_info->vm_clock_nsec = sn->vm_clock_nsec; + sn_info->disk_size = sn->disk_size; } *psn_tab = sn_tab; return s->nb_snapshots; diff --git a/block/qcow2.c b/block/qcow2.c index 62febfc..70ec890 100644 --- a/block/qcow2.c +++ 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*/ + 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*/ + 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; } new_l1_size = size_to_l1(s, offset); - ret = qcow2_grow_l1_table(bs, new_l1_size, true); - if (ret < 0) { - return ret; + if (offset < bs->total_sectors * 512) { + ret = shrink_l1_table(bs, new_l1_size); + if (ret < 0) { + return ret; + } + } else { + ret = qcow2_grow_l1_table(bs, new_l1_size, true); + if (ret < 0) { + return ret; + } } /* write updated header.size */ diff --git a/block/qcow2.h b/block/qcow2.h index a063a3c..dab9e48 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -534,6 +534,7 @@ int qcow2_change_refcount_order(BlockDriverState *bs, int refcount_order, void *cb_opaque, Error **errp); /* qcow2-cluster.c functions */ +int shrink_l1_table(BlockDriverState *bs, int64_t new_l1_size); int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, bool exact_size); int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index); diff --git a/include/block/snapshot.h b/include/block/snapshot.h index e5c0553..7279c12 100644 --- a/include/block/snapshot.h +++ b/include/block/snapshot.h @@ -44,6 +44,7 @@ typedef struct QEMUSnapshotInfo { uint32_t date_sec; /* UTC date of the snapshot */ uint32_t date_nsec; uint64_t vm_clock_nsec; /* VM clock relative to boot */ + uint64_t disk_size; } QEMUSnapshotInfo; int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info, -- 1.7.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] qcow2 resize with snapshot 2016-05-27 8:14 [Qemu-devel] [PATCH] qcow2 resize with snapshot zhangzhiming @ 2016-05-31 16:50 ` Eric Blake 2016-06-01 4:15 ` zhangzhiming 0 siblings, 1 reply; 7+ messages in thread From: Eric Blake @ 2016-05-31 16:50 UTC (permalink / raw) To: zhangzhiming, qemu-block; +Cc: qemu-devel, Kevin Wolf, Max Reitz, lihuiba [-- 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 --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] qcow2 resize with snapshot 2016-05-31 16:50 ` Eric Blake @ 2016-06-01 4:15 ` zhangzhiming 2016-06-01 10:24 ` [Qemu-devel] [Qemu-block] " zhangzhiming 0 siblings, 1 reply; 7+ messages in thread From: zhangzhiming @ 2016-06-01 4:15 UTC (permalink / raw) To: Eric Blake; +Cc: qemu-block, qemu-devel, Kevin Wolf, Max Reitz, lihuiba hi, thanks for the review and the format of code changed. have a good day! zhangzhiming zhangzhiming02@meituan.com 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 <eblake@redhat.com> 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 >> 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 > ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH] qcow2 resize with snapshot 2016-06-01 4:15 ` zhangzhiming @ 2016-06-01 10:24 ` zhangzhiming 2016-06-07 8:25 ` zhangzhiming 0 siblings, 1 reply; 7+ messages in thread From: zhangzhiming @ 2016-06-01 10:24 UTC (permalink / raw) To: Eric Blake; +Cc: Kevin Wolf, lihuiba, qemu-devel, qemu-block, Max Reitz hi, here are all changes of my code. thanks for the review! zhangzhiming zhangzhiming02@meituan.com Signed-off-by: zhangzhiming <zhangzhiming02@meituan.com> --- block.c | 19 +++++++++++++++++++ block/qcow2-cluster.c | 29 +++++++++++++++++++++++++++++ block/qcow2-snapshot.c | 33 +++++++++++++++++++++++---------- block/qcow2.c | 29 ++++++++++++++++++++--------- block/qcow2.h | 1 + include/block/snapshot.h | 1 + 6 files changed, 93 insertions(+), 19 deletions(-) diff --git a/block.c b/block.c index 18a497f..b6f2004 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); + if (bs->blk) { + blk_dev_resize_cb(bs->blk); + } + 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..f921fd8 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); + 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); + if (ret < 0) { + return ret; + } + s->l1_size = new_l1_size; + + uint32_t be_l1_size = cpu_to_be32(s->l1_size); + ret = bdrv_pwrite_sync(bs->file->bs, offsetof(QCowHeader, l1_size), + &be_l1_size, sizeof(be_l1_size)); + if (ret < 0) { + return ret; + } + + return 0; +} + int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, bool exact_size) { diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c index 5f4a17e..1ed0e18 100644 --- a/block/qcow2-snapshot.c +++ b/block/qcow2-snapshot.c @@ -477,13 +477,6 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) } sn = &s->snapshots[snapshot_index]; - if (sn->disk_size != bs->total_sectors * BDRV_SECTOR_SIZE) { - error_report("qcow2: Loading snapshots with different disk " - "size is not implemented"); - ret = -ENOTSUP; - goto fail; - } - /* * Make sure that the current L1 table is big enough to contain the whole * L1 table of the snapshot. If the snapshot L1 table is smaller, the @@ -505,7 +498,7 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) * Decrease the refcount referenced by the old one only when the L1 * table is overwritten. */ - sn_l1_table = g_try_malloc0(cur_l1_bytes); + sn_l1_table = g_try_malloc0(sn_l1_bytes); if (cur_l1_bytes && sn_l1_table == NULL) { ret = -ENOMEM; goto fail; @@ -530,11 +523,28 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) } ret = bdrv_pwrite_sync(bs->file->bs, s->l1_table_offset, sn_l1_table, - cur_l1_bytes); + sn_l1_bytes); + if (ret < 0) { + goto fail; + } + + /* write updated header.size */ + uint64_t be_disk_size = cpu_to_be64(sn->disk_size); + ret = bdrv_pwrite_sync(bs->file->bs, offsetof(QCowHeader, size), + &be_disk_size, sizeof(uint64_t)); if (ret < 0) { goto fail; } + uint32_t be_l1_size = cpu_to_be32(sn->l1_size); + ret = bdrv_pwrite_sync(bs->file->bs, offsetof(QCowHeader, l1_size), + &be_l1_size, sizeof(be_l1_size)); + if (ret < 0) { + goto fail; + } + + s->l1_vm_state_index = sn->l1_size; + /* * Decrease refcount of clusters of current L1 table. * @@ -552,7 +562,8 @@ 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++) { s->l1_table[i] = be64_to_cpu(sn_l1_table[i]); } @@ -563,6 +574,7 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) g_free(sn_l1_table); sn_l1_table = NULL; + s->l1_size = sn->l1_size; /* * Update QCOW_OFLAG_COPIED in the active L1 table (it may have changed * when we decreased the refcount of the old snapshot. @@ -675,6 +687,7 @@ int qcow2_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab) sn_info->date_sec = sn->date_sec; sn_info->date_nsec = sn->date_nsec; sn_info->vm_clock_nsec = sn->vm_clock_nsec; + sn_info->disk_size = sn->disk_size; } *psn_tab = sn_tab; return s->nb_snapshots; diff --git a/block/qcow2.c b/block/qcow2.c index 62febfc..58b53e1 100644 --- a/block/qcow2.c +++ 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 */ + 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 */ + 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; } new_l1_size = size_to_l1(s, offset); - ret = qcow2_grow_l1_table(bs, new_l1_size, true); - if (ret < 0) { - return ret; + if (offset < bs->total_sectors * 512) { + ret = shrink_l1_table(bs, new_l1_size); + if (ret < 0) { + return ret; + } + } else { + ret = qcow2_grow_l1_table(bs, new_l1_size, true); + if (ret < 0) { + return ret; + } } /* write updated header.size */ diff --git a/block/qcow2.h b/block/qcow2.h index a063a3c..dab9e48 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -534,6 +534,7 @@ int qcow2_change_refcount_order(BlockDriverState *bs, int refcount_order, void *cb_opaque, Error **errp); /* qcow2-cluster.c functions */ +int shrink_l1_table(BlockDriverState *bs, int64_t new_l1_size); int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, bool exact_size); int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index); diff --git a/include/block/snapshot.h b/include/block/snapshot.h index e5c0553..7279c12 100644 --- a/include/block/snapshot.h +++ b/include/block/snapshot.h @@ -44,6 +44,7 @@ typedef struct QEMUSnapshotInfo { uint32_t date_sec; /* UTC date of the snapshot */ uint32_t date_nsec; uint64_t vm_clock_nsec; /* VM clock relative to boot */ + uint64_t disk_size; } QEMUSnapshotInfo; int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info, -- 1.7.1 > On Jun 1, 2016, at 12:15 PM, zhangzhiming <zhangzhiming02@meituan.com> wrote: > > hi, thanks for the review and the format of code changed. > have a good day! > > zhangzhiming > zhangzhiming02@meituan.com <mailto:zhangzhiming02@meituan.com> > > Signed-off-by: zhangzhiming <zhangzhiming02@meituan.com <http://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 <eblake@redhat.com <mailto:eblake@redhat.com>> 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 >>> zhangzhiming02@meituan.com <mailto:zhangzhiming02@meituan.com> <mailto: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 <http://libvirt.org/> >> > ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH] qcow2 resize with snapshot 2016-06-01 10:24 ` [Qemu-devel] [Qemu-block] " zhangzhiming @ 2016-06-07 8:25 ` zhangzhiming 2016-06-23 14:34 ` zhangzhiming 0 siblings, 1 reply; 7+ messages in thread From: zhangzhiming @ 2016-06-07 8:25 UTC (permalink / raw) Cc: Kevin Wolf, lihuiba, qemu-devel, qemu-block, Max Reitz, eblake hi, need for someone's review of my code. no one does it for nearly one week. thanks! zhangzhiming zhangzhiming02@meituan.com > On Jun 1, 2016, at 6:24 PM, zhangzhiming <zhangzhiming02@meituan.com> wrote: > > hi, here are all changes of my code. thanks for the review! > > zhangzhiming > zhangzhiming02@meituan.com <mailto:zhangzhiming02@meituan.com> > > Signed-off-by: zhangzhiming <zhangzhiming02@meituan.com <mailto:zhangzhiming02@meituan.com>> > --- > block.c | 19 +++++++++++++++++++ > block/qcow2-cluster.c | 29 +++++++++++++++++++++++++++++ > block/qcow2-snapshot.c | 33 +++++++++++++++++++++++---------- > block/qcow2.c | 29 ++++++++++++++++++++--------- > block/qcow2.h | 1 + > include/block/snapshot.h | 1 + > 6 files changed, 93 insertions(+), 19 deletions(-) > > diff --git a/block.c b/block.c > index 18a497f..b6f2004 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); > + if (bs->blk) { > + blk_dev_resize_cb(bs->blk); > + } > + 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..f921fd8 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); > + 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); > + if (ret < 0) { > + return ret; > + } > + s->l1_size = new_l1_size; > + > + uint32_t be_l1_size = cpu_to_be32(s->l1_size); > + ret = bdrv_pwrite_sync(bs->file->bs, offsetof(QCowHeader, l1_size), > + &be_l1_size, sizeof(be_l1_size)); > + if (ret < 0) { > + return ret; > + } > + > + return 0; > +} > + > int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, > bool exact_size) > { > diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c > index 5f4a17e..1ed0e18 100644 > --- a/block/qcow2-snapshot.c > +++ b/block/qcow2-snapshot.c > @@ -477,13 +477,6 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) > } > sn = &s->snapshots[snapshot_index]; > > - if (sn->disk_size != bs->total_sectors * BDRV_SECTOR_SIZE) { > - error_report("qcow2: Loading snapshots with different disk " > - "size is not implemented"); > - ret = -ENOTSUP; > - goto fail; > - } > - > /* > * Make sure that the current L1 table is big enough to contain the whole > * L1 table of the snapshot. If the snapshot L1 table is smaller, the > @@ -505,7 +498,7 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) > * Decrease the refcount referenced by the old one only when the L1 > * table is overwritten. > */ > - sn_l1_table = g_try_malloc0(cur_l1_bytes); > + sn_l1_table = g_try_malloc0(sn_l1_bytes); > if (cur_l1_bytes && sn_l1_table == NULL) { > ret = -ENOMEM; > goto fail; > @@ -530,11 +523,28 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) > } > > ret = bdrv_pwrite_sync(bs->file->bs, s->l1_table_offset, sn_l1_table, > - cur_l1_bytes); > + sn_l1_bytes); > + if (ret < 0) { > + goto fail; > + } > + > + /* write updated header.size */ > + uint64_t be_disk_size = cpu_to_be64(sn->disk_size); > + ret = bdrv_pwrite_sync(bs->file->bs, offsetof(QCowHeader, size), > + &be_disk_size, sizeof(uint64_t)); > if (ret < 0) { > goto fail; > } > > + uint32_t be_l1_size = cpu_to_be32(sn->l1_size); > + ret = bdrv_pwrite_sync(bs->file->bs, offsetof(QCowHeader, l1_size), > + &be_l1_size, sizeof(be_l1_size)); > + if (ret < 0) { > + goto fail; > + } > + > + s->l1_vm_state_index = sn->l1_size; > + > /* > * Decrease refcount of clusters of current L1 table. > * > @@ -552,7 +562,8 @@ 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++) { > s->l1_table[i] = be64_to_cpu(sn_l1_table[i]); > } > > @@ -563,6 +574,7 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) > g_free(sn_l1_table); > sn_l1_table = NULL; > > + s->l1_size = sn->l1_size; > /* > * Update QCOW_OFLAG_COPIED in the active L1 table (it may have changed > * when we decreased the refcount of the old snapshot. > @@ -675,6 +687,7 @@ int qcow2_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab) > sn_info->date_sec = sn->date_sec; > sn_info->date_nsec = sn->date_nsec; > sn_info->vm_clock_nsec = sn->vm_clock_nsec; > + sn_info->disk_size = sn->disk_size; > } > *psn_tab = sn_tab; > return s->nb_snapshots; > diff --git a/block/qcow2.c b/block/qcow2.c > index 62febfc..58b53e1 100644 > --- a/block/qcow2.c > +++ 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 */ > + 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 */ > + 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; > } > > new_l1_size = size_to_l1(s, offset); > - ret = qcow2_grow_l1_table(bs, new_l1_size, true); > - if (ret < 0) { > - return ret; > + if (offset < bs->total_sectors * 512) { > + ret = shrink_l1_table(bs, new_l1_size); > + if (ret < 0) { > + return ret; > + } > + } else { > + ret = qcow2_grow_l1_table(bs, new_l1_size, true); > + if (ret < 0) { > + return ret; > + } > } > > /* write updated header.size */ > diff --git a/block/qcow2.h b/block/qcow2.h > index a063a3c..dab9e48 100644 > --- a/block/qcow2.h > +++ b/block/qcow2.h > @@ -534,6 +534,7 @@ int qcow2_change_refcount_order(BlockDriverState *bs, int refcount_order, > void *cb_opaque, Error **errp); > > /* qcow2-cluster.c functions */ > +int shrink_l1_table(BlockDriverState *bs, int64_t new_l1_size); > int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, > bool exact_size); > int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index); > diff --git a/include/block/snapshot.h b/include/block/snapshot.h > index e5c0553..7279c12 100644 > --- a/include/block/snapshot.h > +++ b/include/block/snapshot.h > @@ -44,6 +44,7 @@ typedef struct QEMUSnapshotInfo { > uint32_t date_sec; /* UTC date of the snapshot */ > uint32_t date_nsec; > uint64_t vm_clock_nsec; /* VM clock relative to boot */ > + uint64_t disk_size; > } QEMUSnapshotInfo; > > int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info, > -- > 1.7.1 > > >> On Jun 1, 2016, at 12:15 PM, zhangzhiming <zhangzhiming02@meituan.com <mailto:zhangzhiming02@meituan.com>> wrote: >> >> hi, thanks for the review and the format of code changed. >> have a good day! >> >> zhangzhiming >> zhangzhiming02@meituan.com <mailto:zhangzhiming02@meituan.com> >> >> Signed-off-by: zhangzhiming <zhangzhiming02@meituan.com <http://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 <eblake@redhat.com <mailto:eblake@redhat.com>> 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 >>>> zhangzhiming02@meituan.com <mailto:zhangzhiming02@meituan.com> <mailto: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 <http://libvirt.org/> >>> >> > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH] qcow2 resize with snapshot 2016-06-07 8:25 ` zhangzhiming @ 2016-06-23 14:34 ` zhangzhiming 2016-06-24 9:21 ` Stefan Hajnoczi 0 siblings, 1 reply; 7+ messages in thread From: zhangzhiming @ 2016-06-23 14:34 UTC (permalink / raw) To: qemu-block; +Cc: Kevin Wolf, lihuiba, qemu-devel, Max Reitz, eblake hi, please help to review my code. i have checked it for serval times. it is short and very easy too read. and i will be very grateful to you for taking a very little time to read it. thanks very much! zhangzhiming zhangzhiming02@meituan.com <mailto:zhangzhiming02@meituan.com> Signed-off-by: zhangzhiming <zhangzhiming02@meituan.com <mailto:zhangzhiming02@meituan.com>> --- block.c | 19 +++++++++++++++++++ block/qcow2-cluster.c | 29 +++++++++++++++++++++++++++++ block/qcow2-snapshot.c | 37 +++++++++++++++++++++++++------------ block/qcow2.c | 29 ++++++++++++++++++++--------- block/qcow2.h | 1 + include/block/snapshot.h | 1 + 6 files changed, 95 insertions(+), 21 deletions(-) diff --git a/block.c b/block.c index f54bc25..0de7b2d 100644 --- a/block.c +++ b/block.c @@ -2586,6 +2586,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); + if (bs->blk) { + blk_dev_resize_cb(bs->blk); + } + 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 b04bfaf..ebadddf 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -32,6 +32,35 @@ #include "qemu/bswap.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); + 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); + if (ret < 0) { + return ret; + } + s->l1_size = new_l1_size; + + uint32_t be_l1_size = cpu_to_be32(s->l1_size); + ret = bdrv_pwrite_sync(bs->file->bs, offsetof(QCowHeader, l1_size), + &be_l1_size, sizeof(be_l1_size)); + if (ret < 0) { + return ret; + } + + return 0; +} + int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, bool exact_size) { diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c index 242fb21..6dd6769 100644 --- a/block/qcow2-snapshot.c +++ b/block/qcow2-snapshot.c @@ -478,13 +478,6 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) } sn = &s->snapshots[snapshot_index]; - if (sn->disk_size != bs->total_sectors * BDRV_SECTOR_SIZE) { - error_report("qcow2: Loading snapshots with different disk " - "size is not implemented"); - ret = -ENOTSUP; - goto fail; - } - /* * Make sure that the current L1 table is big enough to contain the whole * L1 table of the snapshot. If the snapshot L1 table is smaller, the @@ -506,8 +499,8 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) * Decrease the refcount referenced by the old one only when the L1 * table is overwritten. */ - sn_l1_table = g_try_malloc0(cur_l1_bytes); - if (cur_l1_bytes && sn_l1_table == NULL) { + sn_l1_table = g_try_malloc0(sn_l1_bytes); + if (sn_l1_bytes && sn_l1_table == NULL) { ret = -ENOMEM; goto fail; } @@ -525,17 +518,34 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) } ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_ACTIVE_L1, - s->l1_table_offset, cur_l1_bytes); + s->l1_table_offset, sn_l1_bytes); if (ret < 0) { goto fail; } ret = bdrv_pwrite_sync(bs->file->bs, s->l1_table_offset, sn_l1_table, - cur_l1_bytes); + sn_l1_bytes); + if (ret < 0) { + goto fail; + } + + /* write updated header.size */ + uint64_t be_disk_size = cpu_to_be64(sn->disk_size); + ret = bdrv_pwrite_sync(bs->file->bs, offsetof(QCowHeader, size), + &be_disk_size, sizeof(uint64_t)); if (ret < 0) { goto fail; } + uint32_t be_l1_size = cpu_to_be32(sn->l1_size); + ret = bdrv_pwrite_sync(bs->file->bs, offsetof(QCowHeader, l1_size), + &be_l1_size, sizeof(be_l1_size)); + if (ret < 0) { + goto fail; + } + + s->l1_vm_state_index = sn->l1_size; + /* * Decrease refcount of clusters of current L1 table. * @@ -553,7 +563,8 @@ 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++) { s->l1_table[i] = be64_to_cpu(sn_l1_table[i]); } @@ -564,6 +575,7 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) g_free(sn_l1_table); sn_l1_table = NULL; + s->l1_size = sn->l1_size; /* * Update QCOW_OFLAG_COPIED in the active L1 table (it may have changed * when we decreased the refcount of the old snapshot. @@ -676,6 +688,7 @@ int qcow2_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab) sn_info->date_sec = sn->date_sec; sn_info->date_nsec = sn->date_nsec; sn_info->vm_clock_nsec = sn->vm_clock_nsec; + sn_info->disk_size = sn->disk_size; } *psn_tab = sn_tab; return s->nb_snapshots; diff --git a/block/qcow2.c b/block/qcow2.c index 6f5fb81..612a534 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -2495,22 +2495,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 */ + 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 */ + 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; } new_l1_size = size_to_l1(s, offset); - ret = qcow2_grow_l1_table(bs, new_l1_size, true); - if (ret < 0) { - return ret; + if (offset < bs->total_sectors * 512) { + ret = shrink_l1_table(bs, new_l1_size); + if (ret < 0) { + return ret; + } + } else { + ret = qcow2_grow_l1_table(bs, new_l1_size, true); + if (ret < 0) { + return ret; + } } /* write updated header.size */ diff --git a/block/qcow2.h b/block/qcow2.h index 7db9795..8efa735 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -534,6 +534,7 @@ int qcow2_change_refcount_order(BlockDriverState *bs, int refcount_order, void *cb_opaque, Error **errp); /* qcow2-cluster.c functions */ +int shrink_l1_table(BlockDriverState *bs, int64_t new_l1_size); int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, bool exact_size); int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index); diff --git a/include/block/snapshot.h b/include/block/snapshot.h index e5c0553..7279c12 100644 --- a/include/block/snapshot.h +++ b/include/block/snapshot.h @@ -44,6 +44,7 @@ typedef struct QEMUSnapshotInfo { uint32_t date_sec; /* UTC date of the snapshot */ uint32_t date_nsec; uint64_t vm_clock_nsec; /* VM clock relative to boot */ + uint64_t disk_size; } QEMUSnapshotInfo; int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info, -- 1.7.1 > On Jun 7, 2016, at 4:25 PM, zhangzhiming <zhangzhiming02@meituan.com> wrote: > > hi, need for someone's review of my code. no one does it for nearly one week. > thanks! > > zhangzhiming > zhangzhiming02@meituan.com <mailto:zhangzhiming02@meituan.com> > > > >> On Jun 1, 2016, at 6:24 PM, zhangzhiming <zhangzhiming02@meituan.com <mailto:zhangzhiming02@meituan.com>> wrote: >> >> hi, here are all changes of my code. thanks for the review! >> >> zhangzhiming >> zhangzhiming02@meituan.com <mailto:zhangzhiming02@meituan.com> >> >> Signed-off-by: zhangzhiming <zhangzhiming02@meituan.com <mailto:zhangzhiming02@meituan.com>> >> --- >> block.c | 19 +++++++++++++++++++ >> block/qcow2-cluster.c | 29 +++++++++++++++++++++++++++++ >> block/qcow2-snapshot.c | 33 +++++++++++++++++++++++---------- >> block/qcow2.c | 29 ++++++++++++++++++++--------- >> block/qcow2.h | 1 + >> include/block/snapshot.h | 1 + >> 6 files changed, 93 insertions(+), 19 deletions(-) >> >> diff --git a/block.c b/block.c >> index 18a497f..b6f2004 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); >> + if (bs->blk) { >> + blk_dev_resize_cb(bs->blk); >> + } >> + 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..f921fd8 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); >> + 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); >> + if (ret < 0) { >> + return ret; >> + } >> + s->l1_size = new_l1_size; >> + >> + uint32_t be_l1_size = cpu_to_be32(s->l1_size); >> + ret = bdrv_pwrite_sync(bs->file->bs, offsetof(QCowHeader, l1_size), >> + &be_l1_size, sizeof(be_l1_size)); >> + if (ret < 0) { >> + return ret; >> + } >> + >> + return 0; >> +} >> + >> int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, >> bool exact_size) >> { >> diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c >> index 5f4a17e..1ed0e18 100644 >> --- a/block/qcow2-snapshot.c >> +++ b/block/qcow2-snapshot.c >> @@ -477,13 +477,6 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) >> } >> sn = &s->snapshots[snapshot_index]; >> >> - if (sn->disk_size != bs->total_sectors * BDRV_SECTOR_SIZE) { >> - error_report("qcow2: Loading snapshots with different disk " >> - "size is not implemented"); >> - ret = -ENOTSUP; >> - goto fail; >> - } >> - >> /* >> * Make sure that the current L1 table is big enough to contain the whole >> * L1 table of the snapshot. If the snapshot L1 table is smaller, the >> @@ -505,7 +498,7 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) >> * Decrease the refcount referenced by the old one only when the L1 >> * table is overwritten. >> */ >> - sn_l1_table = g_try_malloc0(cur_l1_bytes); >> + sn_l1_table = g_try_malloc0(sn_l1_bytes); >> if (cur_l1_bytes && sn_l1_table == NULL) { >> ret = -ENOMEM; >> goto fail; >> @@ -530,11 +523,28 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) >> } >> >> ret = bdrv_pwrite_sync(bs->file->bs, s->l1_table_offset, sn_l1_table, >> - cur_l1_bytes); >> + sn_l1_bytes); >> + if (ret < 0) { >> + goto fail; >> + } >> + >> + /* write updated header.size */ >> + uint64_t be_disk_size = cpu_to_be64(sn->disk_size); >> + ret = bdrv_pwrite_sync(bs->file->bs, offsetof(QCowHeader, size), >> + &be_disk_size, sizeof(uint64_t)); >> if (ret < 0) { >> goto fail; >> } >> >> + uint32_t be_l1_size = cpu_to_be32(sn->l1_size); >> + ret = bdrv_pwrite_sync(bs->file->bs, offsetof(QCowHeader, l1_size), >> + &be_l1_size, sizeof(be_l1_size)); >> + if (ret < 0) { >> + goto fail; >> + } >> + >> + s->l1_vm_state_index = sn->l1_size; >> + >> /* >> * Decrease refcount of clusters of current L1 table. >> * >> @@ -552,7 +562,8 @@ 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++) { >> s->l1_table[i] = be64_to_cpu(sn_l1_table[i]); >> } >> >> @@ -563,6 +574,7 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) >> g_free(sn_l1_table); >> sn_l1_table = NULL; >> >> + s->l1_size = sn->l1_size; >> /* >> * Update QCOW_OFLAG_COPIED in the active L1 table (it may have changed >> * when we decreased the refcount of the old snapshot. >> @@ -675,6 +687,7 @@ int qcow2_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab) >> sn_info->date_sec = sn->date_sec; >> sn_info->date_nsec = sn->date_nsec; >> sn_info->vm_clock_nsec = sn->vm_clock_nsec; >> + sn_info->disk_size = sn->disk_size; >> } >> *psn_tab = sn_tab; >> return s->nb_snapshots; >> diff --git a/block/qcow2.c b/block/qcow2.c >> index 62febfc..58b53e1 100644 >> --- a/block/qcow2.c >> +++ 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 */ >> + 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 */ >> + 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; >> } >> >> new_l1_size = size_to_l1(s, offset); >> - ret = qcow2_grow_l1_table(bs, new_l1_size, true); >> - if (ret < 0) { >> - return ret; >> + if (offset < bs->total_sectors * 512) { >> + ret = shrink_l1_table(bs, new_l1_size); >> + if (ret < 0) { >> + return ret; >> + } >> + } else { >> + ret = qcow2_grow_l1_table(bs, new_l1_size, true); >> + if (ret < 0) { >> + return ret; >> + } >> } >> >> /* write updated header.size */ >> diff --git a/block/qcow2.h b/block/qcow2.h >> index a063a3c..dab9e48 100644 >> --- a/block/qcow2.h >> +++ b/block/qcow2.h >> @@ -534,6 +534,7 @@ int qcow2_change_refcount_order(BlockDriverState *bs, int refcount_order, >> void *cb_opaque, Error **errp); >> >> /* qcow2-cluster.c functions */ >> +int shrink_l1_table(BlockDriverState *bs, int64_t new_l1_size); >> int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, >> bool exact_size); >> int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index); >> diff --git a/include/block/snapshot.h b/include/block/snapshot.h >> index e5c0553..7279c12 100644 >> --- a/include/block/snapshot.h >> +++ b/include/block/snapshot.h >> @@ -44,6 +44,7 @@ typedef struct QEMUSnapshotInfo { >> uint32_t date_sec; /* UTC date of the snapshot */ >> uint32_t date_nsec; >> uint64_t vm_clock_nsec; /* VM clock relative to boot */ >> + uint64_t disk_size; >> } QEMUSnapshotInfo; >> >> int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info, >> -- >> 1.7.1 >> >> >>> On Jun 1, 2016, at 12:15 PM, zhangzhiming <zhangzhiming02@meituan.com <mailto:zhangzhiming02@meituan.com>> wrote: >>> >>> hi, thanks for the review and the format of code changed. >>> have a good day! >>> >>> zhangzhiming >>> zhangzhiming02@meituan.com <mailto:zhangzhiming02@meituan.com> >>> >>> Signed-off-by: zhangzhiming <zhangzhiming02@meituan.com <http://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 <eblake@redhat.com <mailto:eblake@redhat.com>> 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 >>>>> zhangzhiming02@meituan.com <mailto:zhangzhiming02@meituan.com> <mailto: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 <http://libvirt.org/> >>>> >>> >> > ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH] qcow2 resize with snapshot 2016-06-23 14:34 ` zhangzhiming @ 2016-06-24 9:21 ` Stefan Hajnoczi 0 siblings, 0 replies; 7+ messages in thread From: Stefan Hajnoczi @ 2016-06-24 9:21 UTC (permalink / raw) To: zhangzhiming; +Cc: qemu-block, Kevin Wolf, lihuiba, qemu-devel, Max Reitz [-- Attachment #1: Type: text/plain, Size: 814 bytes --] On Thu, Jun 23, 2016 at 10:34:48PM +0800, zhangzhiming wrote: > hi, please help to review my code. i have checked it for serval times. > it is short and very easy too read. and i will be very grateful to you for taking a very little time > to read it. thanks very much! > > zhangzhiming > zhangzhiming02@meituan.com <mailto:zhangzhiming02@meituan.com> > > Signed-off-by: zhangzhiming <zhangzhiming02@meituan.com <mailto:zhangzhiming02@meituan.com>> Please follow the guidelines for submitting patches: http://qemu-project.org/Contribute/SubmitAPatch Send patches as new email threads, not replies to old emails. Include a meaningful commit description that explains why this patch is necessary. Do not put comments into the commit description that aren't useful in the git log. Stefan [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-06-24 9:21 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-05-27 8:14 [Qemu-devel] [PATCH] qcow2 resize with snapshot zhangzhiming 2016-05-31 16:50 ` Eric Blake 2016-06-01 4:15 ` zhangzhiming 2016-06-01 10:24 ` [Qemu-devel] [Qemu-block] " zhangzhiming 2016-06-07 8:25 ` zhangzhiming 2016-06-23 14:34 ` zhangzhiming 2016-06-24 9:21 ` Stefan Hajnoczi
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.