| Message ID | C9673717-4844-4AB9-B2F6-391FB117C288@meituan.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
ping zhangzhiming zhangzhiming02@meituan.com > On Aug 15, 2016, at 7:56 PM, zhangzhiming <zhangzhiming02@meituan.com> wrote: > > hi, i modified my code by Kevin Wolf’s review. and thanks for review again. > > > > Signed-off-by: zhangzhiming <zhangzhiming02@meituan.com> > --- > block.c | 19 ---------------- > block/qcow2-cluster.c | 55 ++++++++++++++++++++++++++++++++++-------------- > block/qcow2-snapshot.c | 34 ++++++++++++++-------------- > block/qcow2.c | 2 +- > block/qcow2.h | 2 +- > 5 files changed, 58 insertions(+), 54 deletions(-) > > diff --git a/block.c b/block.c > index 0de7b2d..f54bc25 100644 > --- a/block.c > +++ b/block.c > @@ -2586,25 +2586,6 @@ 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 ebadddf..2190528 100644 > --- a/block/qcow2-cluster.c > +++ b/block/qcow2-cluster.c > @@ -32,32 +32,55 @@ > #include "qemu/bswap.h" > #include "trace.h" > > -int shrink_l1_table(BlockDriverState *bs, int64_t new_l1_size) > +static int free_some_l2_table(BlockDriverState *bs, int64_t old_size, > + int64_t new_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; > + int old_l1_size = size_to_l1(s, old_size); > + int new_l1_size = size_to_l1(s, new_size); > + if (old_l1_size == new_l1_size) { > + return 0; > } > + int i; > + for (i = new_l1_size; i < old_l1_size; i++) { > + uint64_t l2_offset = s->l1_table[i] & L1E_OFFSET_MASK; > + qcow2_free_clusters(bs, l2_offset, s->cluster_size, > + QCOW2_DISCARD_OTHER); > + s->l1_table[i] = 0; > + /* write l1 entry will be called for several times and > + * because l1 is short, i think is tolerated! */ > + qcow2_write_l1_entry(bs, i); > + } > + return 0; > +} > > - s->l1_size = old_l1_size; > - ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset, > - s->l1_size, -1); > +int shrink_disk(BlockDriverState *bs, int64_t new_size) > +{ > + BDRVQcow2State *s = bs->opaque; > + int64_t old_size = bs->total_sectors * 512; > + int64_t new_sector_nb = DIV_ROUND_UP(new_size, 512); > + int64_t discard_sectors_nb = bs->total_sectors - new_sector_nb; > + int64_t old_cluster_nb = DIV_ROUND_UP(old_size, s->cluster_size); > + int64_t new_cluster_nb = DIV_ROUND_UP(new_size, s->cluster_size); > + if (old_cluster_nb == new_cluster_nb) { > + return 0; > + } > + s->l1_size = size_to_l1(s, new_size); > + bs->total_sectors = new_sector_nb; > + int ret = qcow2_update_header(bs); > if (ret < 0) { > + s->l1_size = size_to_l1(s, old_size); > + bs->total_sectors = old_size / 512; > 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)); > + /* can't be undone here, if failed nothing will be done */ > + ret = qcow2_discard_clusters(bs, new_size, discard_sectors_nb, > + QCOW2_DISCARD_OTHER, true); > if (ret < 0) { > - return ret; > + return 0; > } > - > + free_some_l2_table(bs, old_size, new_size); > return 0; > } > > diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c > index 6dd6769..40a0d26 100644 > --- a/block/qcow2-snapshot.c > +++ b/block/qcow2-snapshot.c > @@ -490,6 +490,7 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) > > cur_l1_bytes = s->l1_size * sizeof(uint64_t); > sn_l1_bytes = sn->l1_size * sizeof(uint64_t); > + int max_l1_size_bytes = MAX(sn_l1_bytes, cur_l1_bytes); > > /* > * Copy the snapshot L1 table to the current L1 table. > @@ -499,7 +500,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(sn_l1_bytes); > + sn_l1_table = g_try_malloc0(max_l1_size_bytes); > if (sn_l1_bytes && sn_l1_table == NULL) { > ret = -ENOMEM; > goto fail; > @@ -517,34 +518,32 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) > goto fail; > } > > - ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_ACTIVE_L1, > - 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, > - sn_l1_bytes); > + /* update meta data first, for failed */ > + int old_l1_size = s->l1_size; > + uint64_t old_total_sectors = bs->total_sectors; > + bs->total_sectors = sn->disk_size / BDRV_SECTOR_SIZE; > + s->l1_size = sn->l1_size; > + ret = qcow2_update_header(bs); > if (ret < 0) { > + s->l1_size = old_l1_size; > + bs->total_sectors = old_total_sectors; > 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)); > + /* and then update l1 table */ > + ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_ACTIVE_L1, > + s->l1_table_offset, max_l1_size_bytes); > 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)); > + ret = bdrv_pwrite_sync(bs->file->bs, s->l1_table_offset, sn_l1_table, > + max_l1_size_bytes); > if (ret < 0) { > goto fail; > } > > - s->l1_vm_state_index = sn->l1_size; > + s->l1_vm_state_index = size_to_l1(s, sn->disk_size);; > > /* > * Decrease refcount of clusters of current L1 table. > @@ -556,6 +555,7 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) > * the in-memory data instead of really using the offset to load a new one, > * which is why this works. > */ > + s->l1_size = old_l1_size; > ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset, > s->l1_size, -1); > > diff --git a/block/qcow2.c b/block/qcow2.c > index 612a534..01a66f2 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -2513,7 +2513,7 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset) > > new_l1_size = size_to_l1(s, offset); > if (offset < bs->total_sectors * 512) { > - ret = shrink_l1_table(bs, new_l1_size); > + ret = shrink_disk(bs, offset); > if (ret < 0) { > return ret; > } > diff --git a/block/qcow2.h b/block/qcow2.h > index 8efa735..ba622b6 100644 > --- a/block/qcow2.h > +++ b/block/qcow2.h > @@ -534,7 +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 shrink_disk(BlockDriverState *bs, int64_t new_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); > -- > 1.7.1 > > > > > zhangzhiming > zhangzhiming02@meituan.com > > >
Hi, You should change your patch's commit message to show what your patch solved. Please refer to the guide: http://wiki.qemu.org/Contribute/SubmitAPatch Regards, -Gonglei On 2016/9/2 14:41, zhangzhiming wrote: > ping > > zhangzhiming > zhangzhiming02@meituan.com > > > >> On Aug 15, 2016, at 7:56 PM, zhangzhiming <zhangzhiming02@meituan.com> wrote: >> >> hi, i modified my code by Kevin Wolf’s review. and thanks for review again. >> >> >> >> Signed-off-by: zhangzhiming <zhangzhiming02@meituan.com> >> --- >> block.c | 19 ---------------- >> block/qcow2-cluster.c | 55 ++++++++++++++++++++++++++++++++++-------------- >> block/qcow2-snapshot.c | 34 ++++++++++++++-------------- >> block/qcow2.c | 2 +- >> block/qcow2.h | 2 +- >> 5 files changed, 58 insertions(+), 54 deletions(-) >> >> diff --git a/block.c b/block.c >> index 0de7b2d..f54bc25 100644 >> --- a/block.c >> +++ b/block.c >> @@ -2586,25 +2586,6 @@ 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 ebadddf..2190528 100644 >> --- a/block/qcow2-cluster.c >> +++ b/block/qcow2-cluster.c >> @@ -32,32 +32,55 @@ >> #include "qemu/bswap.h" >> #include "trace.h" >> >> -int shrink_l1_table(BlockDriverState *bs, int64_t new_l1_size) >> +static int free_some_l2_table(BlockDriverState *bs, int64_t old_size, >> + int64_t new_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; >> + int old_l1_size = size_to_l1(s, old_size); >> + int new_l1_size = size_to_l1(s, new_size); >> + if (old_l1_size == new_l1_size) { >> + return 0; >> } >> + int i; >> + for (i = new_l1_size; i < old_l1_size; i++) { >> + uint64_t l2_offset = s->l1_table[i] & L1E_OFFSET_MASK; >> + qcow2_free_clusters(bs, l2_offset, s->cluster_size, >> + QCOW2_DISCARD_OTHER); >> + s->l1_table[i] = 0; >> + /* write l1 entry will be called for several times and >> + * because l1 is short, i think is tolerated! */ >> + qcow2_write_l1_entry(bs, i); >> + } >> + return 0; >> +} >> >> - s->l1_size = old_l1_size; >> - ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset, >> - s->l1_size, -1); >> +int shrink_disk(BlockDriverState *bs, int64_t new_size) >> +{ >> + BDRVQcow2State *s = bs->opaque; >> + int64_t old_size = bs->total_sectors * 512; >> + int64_t new_sector_nb = DIV_ROUND_UP(new_size, 512); >> + int64_t discard_sectors_nb = bs->total_sectors - new_sector_nb; >> + int64_t old_cluster_nb = DIV_ROUND_UP(old_size, s->cluster_size); >> + int64_t new_cluster_nb = DIV_ROUND_UP(new_size, s->cluster_size); >> + if (old_cluster_nb == new_cluster_nb) { >> + return 0; >> + } >> + s->l1_size = size_to_l1(s, new_size); >> + bs->total_sectors = new_sector_nb; >> + int ret = qcow2_update_header(bs); >> if (ret < 0) { >> + s->l1_size = size_to_l1(s, old_size); >> + bs->total_sectors = old_size / 512; >> 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)); >> + /* can't be undone here, if failed nothing will be done */ >> + ret = qcow2_discard_clusters(bs, new_size, discard_sectors_nb, >> + QCOW2_DISCARD_OTHER, true); >> if (ret < 0) { >> - return ret; >> + return 0; >> } >> - >> + free_some_l2_table(bs, old_size, new_size); >> return 0; >> } >> >> diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c >> index 6dd6769..40a0d26 100644 >> --- a/block/qcow2-snapshot.c >> +++ b/block/qcow2-snapshot.c >> @@ -490,6 +490,7 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) >> >> cur_l1_bytes = s->l1_size * sizeof(uint64_t); >> sn_l1_bytes = sn->l1_size * sizeof(uint64_t); >> + int max_l1_size_bytes = MAX(sn_l1_bytes, cur_l1_bytes); >> >> /* >> * Copy the snapshot L1 table to the current L1 table. >> @@ -499,7 +500,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(sn_l1_bytes); >> + sn_l1_table = g_try_malloc0(max_l1_size_bytes); >> if (sn_l1_bytes && sn_l1_table == NULL) { >> ret = -ENOMEM; >> goto fail; >> @@ -517,34 +518,32 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) >> goto fail; >> } >> >> - ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_ACTIVE_L1, >> - 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, >> - sn_l1_bytes); >> + /* update meta data first, for failed */ >> + int old_l1_size = s->l1_size; >> + uint64_t old_total_sectors = bs->total_sectors; >> + bs->total_sectors = sn->disk_size / BDRV_SECTOR_SIZE; >> + s->l1_size = sn->l1_size; >> + ret = qcow2_update_header(bs); >> if (ret < 0) { >> + s->l1_size = old_l1_size; >> + bs->total_sectors = old_total_sectors; >> 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)); >> + /* and then update l1 table */ >> + ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_ACTIVE_L1, >> + s->l1_table_offset, max_l1_size_bytes); >> 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)); >> + ret = bdrv_pwrite_sync(bs->file->bs, s->l1_table_offset, sn_l1_table, >> + max_l1_size_bytes); >> if (ret < 0) { >> goto fail; >> } >> >> - s->l1_vm_state_index = sn->l1_size; >> + s->l1_vm_state_index = size_to_l1(s, sn->disk_size);; >> >> /* >> * Decrease refcount of clusters of current L1 table. >> @@ -556,6 +555,7 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) >> * the in-memory data instead of really using the offset to load a new one, >> * which is why this works. >> */ >> + s->l1_size = old_l1_size; >> ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset, >> s->l1_size, -1); >> >> diff --git a/block/qcow2.c b/block/qcow2.c >> index 612a534..01a66f2 100644 >> --- a/block/qcow2.c >> +++ b/block/qcow2.c >> @@ -2513,7 +2513,7 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset) >> >> new_l1_size = size_to_l1(s, offset); >> if (offset < bs->total_sectors * 512) { >> - ret = shrink_l1_table(bs, new_l1_size); >> + ret = shrink_disk(bs, offset); >> if (ret < 0) { >> return ret; >> } >> diff --git a/block/qcow2.h b/block/qcow2.h >> index 8efa735..ba622b6 100644 >> --- a/block/qcow2.h >> +++ b/block/qcow2.h >> @@ -534,7 +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 shrink_disk(BlockDriverState *bs, int64_t new_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); >> -- >> 1.7.1 >> >> >> >> >> zhangzhiming >> zhangzhiming02@meituan.com >> >> >> > > > > . >
diff --git a/block.c b/block.c index 0de7b2d..f54bc25 100644 --- a/block.c +++ b/block.c @@ -2586,25 +2586,6 @@ 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 ebadddf..2190528 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -32,32 +32,55 @@ #include "qemu/bswap.h" #include "trace.h" -int shrink_l1_table(BlockDriverState *bs, int64_t new_l1_size) +static int free_some_l2_table(BlockDriverState *bs, int64_t old_size, + int64_t new_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; + int old_l1_size = size_to_l1(s, old_size); + int new_l1_size = size_to_l1(s, new_size); + if (old_l1_size == new_l1_size) { + return 0; } + int i; + for (i = new_l1_size; i < old_l1_size; i++) { + uint64_t l2_offset = s->l1_table[i] & L1E_OFFSET_MASK; + qcow2_free_clusters(bs, l2_offset, s->cluster_size, + QCOW2_DISCARD_OTHER); + s->l1_table[i] = 0; + /* write l1 entry will be called for several times and + * because l1 is short, i think is tolerated! */ + qcow2_write_l1_entry(bs, i); + } + return 0; +} - s->l1_size = old_l1_size; - ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset, - s->l1_size, -1); +int shrink_disk(BlockDriverState *bs, int64_t new_size) +{ + BDRVQcow2State *s = bs->opaque; + int64_t old_size = bs->total_sectors * 512; + int64_t new_sector_nb = DIV_ROUND_UP(new_size, 512); + int64_t discard_sectors_nb = bs->total_sectors - new_sector_nb; + int64_t old_cluster_nb = DIV_ROUND_UP(old_size, s->cluster_size); + int64_t new_cluster_nb = DIV_ROUND_UP(new_size, s->cluster_size); + if (old_cluster_nb == new_cluster_nb) { + return 0; + } + s->l1_size = size_to_l1(s, new_size); + bs->total_sectors = new_sector_nb; + int ret = qcow2_update_header(bs); if (ret < 0) { + s->l1_size = size_to_l1(s, old_size); + bs->total_sectors = old_size / 512; 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)); + /* can't be undone here, if failed nothing will be done */ + ret = qcow2_discard_clusters(bs, new_size, discard_sectors_nb, + QCOW2_DISCARD_OTHER, true); if (ret < 0) { - return ret; + return 0; } - + free_some_l2_table(bs, old_size, new_size); return 0; } diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c index 6dd6769..40a0d26 100644 --- a/block/qcow2-snapshot.c +++ b/block/qcow2-snapshot.c @@ -490,6 +490,7 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) cur_l1_bytes = s->l1_size * sizeof(uint64_t); sn_l1_bytes = sn->l1_size * sizeof(uint64_t); + int max_l1_size_bytes = MAX(sn_l1_bytes, cur_l1_bytes); /* * Copy the snapshot L1 table to the current L1 table. @@ -499,7 +500,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(sn_l1_bytes); + sn_l1_table = g_try_malloc0(max_l1_size_bytes); if (sn_l1_bytes && sn_l1_table == NULL) { ret = -ENOMEM; goto fail; @@ -517,34 +518,32 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) goto fail; } - ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_ACTIVE_L1, - 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, - sn_l1_bytes); + /* update meta data first, for failed */ + int old_l1_size = s->l1_size; + uint64_t old_total_sectors = bs->total_sectors; + bs->total_sectors = sn->disk_size / BDRV_SECTOR_SIZE; + s->l1_size = sn->l1_size; + ret = qcow2_update_header(bs); if (ret < 0) { + s->l1_size = old_l1_size; + bs->total_sectors = old_total_sectors; 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)); + /* and then update l1 table */ + ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_ACTIVE_L1, + s->l1_table_offset, max_l1_size_bytes); 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)); + ret = bdrv_pwrite_sync(bs->file->bs, s->l1_table_offset, sn_l1_table, + max_l1_size_bytes); if (ret < 0) { goto fail; } - s->l1_vm_state_index = sn->l1_size; + s->l1_vm_state_index = size_to_l1(s, sn->disk_size);; /* * Decrease refcount of clusters of current L1 table. @@ -556,6 +555,7 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) * the in-memory data instead of really using the offset to load a new one, * which is why this works. */ + s->l1_size = old_l1_size; ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset, s->l1_size, -1); diff --git a/block/qcow2.c b/block/qcow2.c index 612a534..01a66f2 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -2513,7 +2513,7 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset) new_l1_size = size_to_l1(s, offset); if (offset < bs->total_sectors * 512) { - ret = shrink_l1_table(bs, new_l1_size); + ret = shrink_disk(bs, offset); if (ret < 0) { return ret; } diff --git a/block/qcow2.h b/block/qcow2.h index 8efa735..ba622b6 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -534,7 +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 shrink_disk(BlockDriverState *bs, int64_t new_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);
hi, i modified my code by Kevin Wolf’s review. and thanks for review again. Signed-off-by: zhangzhiming <zhangzhiming02@meituan.com> --- block.c | 19 ---------------- block/qcow2-cluster.c | 55 ++++++++++++++++++++++++++++++++++-------------- block/qcow2-snapshot.c | 34 ++++++++++++++-------------- block/qcow2.c | 2 +- block/qcow2.h | 2 +- 5 files changed, 58 insertions(+), 54 deletions(-)