From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60016) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b7xZm-0000Oe-TJ for qemu-devel@nongnu.org; Wed, 01 Jun 2016 00:16:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b7xZk-0005LW-Ef for qemu-devel@nongnu.org; Wed, 01 Jun 2016 00:16:14 -0400 Mime-Version: 1.0 (Mac OS X Mail 8.2 \(2070.6\)) From: zhangzhiming In-Reply-To: <574DC0CB.7020707@redhat.com> Date: Wed, 1 Jun 2016 12:15:44 +0800 Message-Id: <467DE7EB-A927-4100-96FB-736444263B06@meituan.com> References: <8E87729A-E996-4D12-B940-87B77C1D22EC@meituan.com> <574DC0CB.7020707@redhat.com> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] qcow2 resize with snapshot List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: qemu-block@nongnu.org, qemu-devel@nongnu.org, 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 --- 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 =3D s->l1_size; s->l1_size =3D new_l1_size; int ret =3D qcow2_update_snapshot_refcount(bs, s->l1_table_offset, - s->l1_size, 1); + s->l1_size, 1); if (ret < 0) { return ret; } =20 s->l1_size =3D old_l1_size; ret =3D 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 =3D 0;i < sn->l1_size; i++) { + memset(s->l1_table, 0, s->l1_size * sizeof(uint64_t)); + for (i =3D 0; i < sn->l1_size; i++) { s->l1_table[i] =3D be64_to_cpu(sn_l1_table[i]); } =20 - 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) =20 bool v3_truncate =3D (s->qcow_version =3D=3D 3); =20 - /* 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; } =20 - /* 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"); --=20 1.7.1 > On Jun 1, 2016, at 12:50 AM, Eric Blake wrote: >=20 > On 05/27/2016 02:14 AM, zhangzhiming wrote: >> Hi, i modified my code for qcow2 resize, and delete some code related = to=20 >> qemu monitor. >>=20 >> and thanks for the review. >>=20 >> zhangzhiming >> zhangzhiming02@meituan.com >=20 > Still missing a Signed-off-by designation, so it can't be applied = as-is. >=20 >>=20 >> --- >> 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(-) >>=20 >> 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; >> } >>=20 >> +int bdrv_apply_snapshot(BlockDriverState *bs, const char = *snapshot_id, >> + uint64_t snapshot_size) >> +{ >> + int ret =3D bdrv_snapshot_goto(bs, snapshot_id); >> + if (ret < 0) { >> + return ret; >> + } >> + >> + ret =3D 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 */ >=20 > The comments don't add anything here. >=20 >> + } >> + 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" >>=20 >> +int shrink_l1_table(BlockDriverState *bs, int64_t new_l1_size) >> +{ >> + BDRVQcow2State *s =3D bs->opaque; >> + int64_t old_l1_size =3D s->l1_size; >> + s->l1_size =3D new_l1_size; >> + int ret =3D qcow2_update_snapshot_refcount(bs, = s->l1_table_offset, >> + s->l1_size, 1); >=20 > Indentation is off. >=20 >> + if (ret < 0) { >> + return ret; >> + } >> + >> + s->l1_size =3D old_l1_size; >> + ret =3D qcow2_update_snapshot_refcount(bs, s->l1_table_offset, >> + s->l1_size, = -1); >=20 > and again. >=20 >> + if (ret < 0) { >> + return ret; >> + } >> + s->l1_size =3D new_l1_size; >> + >=20 >> +++ b/block/qcow2-snapshot.c >=20 >> @@ -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 =3D 0;i < s->l1_size; i++) { >> + memset(s->l1_table, 0, s->l1_size*sizeof(uint64_t)); >> + for(i =3D 0;i < sn->l1_size; i++) { >=20 > As long as you are touching this, use the preferred spacing: >=20 > for (i =3D 0; i < sn->l1_size; i++) { >=20 >> s->l1_table[i] =3D be64_to_cpu(sn_l1_table[i]); >> } >>=20 >> + >> if (ret < 0) { >=20 > Why the added blank line? >=20 >=20 >> +++ b/block/qcow2.c >> @@ -2501,22 +2501,33 @@ static int qcow2_truncate(BlockDriverState = *bs, int64_t offset) >> return -EINVAL; >> } >>=20 >> - /* cannot proceed if image has snapshots */ >> - if (s->nb_snapshots) { >> - error_report("Can't resize an image which has snapshots"); >> + bool v3_truncate =3D (s->qcow_version =3D=3D 3); >> + >> + /* cannot proceed if image has snapshots and qcow_version is not = 3*/ >=20 > Space before */ >=20 >> + if (!v3_truncate && s->nb_snapshots) { >> + error_report("Can't resize an image which has snapshots and = " >> + "qcow_version is not 3"); >> return -ENOTSUP; >> } >>=20 >> - /* 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*/ >=20 > and again >=20 >> + 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; >> } >=20 >=20 > --=20 > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org >=20