From: zhangzhiming <zhangzhiming02@meituan.com>
To: Eric Blake <eblake@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>, lihuiba <lihuiba@meituan.com>,
qemu-devel@nongnu.org, qemu-block@nongnu.org,
Max Reitz <mreitz@redhat.com>
Subject: Re: [Qemu-devel] [Qemu-block] [PATCH] qcow2 resize with snapshot
Date: Wed, 1 Jun 2016 18:24:22 +0800 [thread overview]
Message-ID: <6DA33516-D7FB-44C2-A4B8-01974A3E3B77@meituan.com> (raw)
In-Reply-To: <467DE7EB-A927-4100-96FB-736444263B06@meituan.com>
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/>
>>
>
next prev parent reply other threads:[~2016-06-01 10:24 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
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 ` zhangzhiming [this message]
2016-06-07 8:25 ` [Qemu-devel] [Qemu-block] " zhangzhiming
2016-06-23 14:34 ` zhangzhiming
2016-06-24 9:21 ` Stefan Hajnoczi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6DA33516-D7FB-44C2-A4B8-01974A3E3B77@meituan.com \
--to=zhangzhiming02@meituan.com \
--cc=eblake@redhat.com \
--cc=kwolf@redhat.com \
--cc=lihuiba@meituan.com \
--cc=mreitz@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.