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 > > >