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 > zhangzhimin...@meituan.com > > > >> On Aug 15, 2016, at 7:56 PM, zhangzhiming <zhangzhimin...@meituan.com> wrote: >> >> hi, i modified my code by Kevin Wolf’s review. and thanks for review again. >> >> >> >> Signed-off-by: zhangzhiming <zhangzhimin...@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 >> zhangzhimin...@meituan.com >> >> >> > > > > . >
