hi, thanks for the review and the format of code changed.
have a good day!
zhangzhiming
zhangzhimin...@meituan.com
Signed-off-by: zhangzhiming <zhangzhimin...@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 <ebl...@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
>> zhangzhimin...@meituan.com <mailto:zhangzhimin...@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
>
