The Saturday 02 Aug 2014 à 01:49:20 (+0200), Max Reitz wrote :
> Currently, we have a bitmap for keeping track of which clusters have
> been created during the zero cluster expansion process. This was
> necessary because we need to properly increase the refcount for shared
> L2 tables.
>
> However, now we can simply take the L2 refcount and use it for the
> cluster allocated for expansion. This will be the correct refcount and
> therefore we don't have to remember that cluster having been allocated
> any more.
>
> Signed-off-by: Max Reitz <mre...@redhat.com>
> ---
> block/qcow2-cluster.c | 90
> ++++++++++++++++-----------------------------------
> 1 file changed, 28 insertions(+), 62 deletions(-)
>
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 2607715..7e65c13 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -1543,20 +1543,12 @@ fail:
> * Expands all zero clusters in a specific L1 table (or deallocates them, for
> * non-backed non-pre-allocated zero clusters).
> *
> - * expanded_clusters is a bitmap where every bit corresponds to one cluster
> in
> - * the image file; a bit gets set if the corresponding cluster has been used
> for
> - * zero expansion (i.e., has been filled with zeroes and is referenced from
> an
> - * L2 table). nb_clusters contains the total cluster count of the image file,
> - * i.e., the number of bits in expanded_clusters.
> - *
> * l1_entries and *visited_l1_entries are used to keep track of progress for
> * status_cb(). l1_entries contains the total number of L1 entries and
> * *visited_l1_entries counts all visited L1 entries.
> */
> static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t
> *l1_table,
> - int l1_size, uint8_t
> **expanded_clusters,
> - uint64_t *nb_clusters,
> - int64_t *visited_l1_entries,
> + int l1_size, int64_t
> *visited_l1_entries,
> int64_t l1_entries,
> BlockDriverAmendStatusCB *status_cb)
> {
> @@ -1575,6 +1567,7 @@ static int expand_zero_clusters_in_l1(BlockDriverState
> *bs, uint64_t *l1_table,
> for (i = 0; i < l1_size; i++) {
> uint64_t l2_offset = l1_table[i] & L1E_OFFSET_MASK;
> bool l2_dirty = false;
> + int l2_refcount;
>
> if (!l2_offset) {
> /* unallocated */
> @@ -1598,33 +1591,19 @@ static int
> expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,
> goto fail;
> }
>
> + l2_refcount = qcow2_get_refcount(bs, l2_offset >> s->cluster_bits);
> + if (l2_refcount < 0) {
> + ret = l2_refcount;
> + goto fail;
> + }
> +
> for (j = 0; j < s->l2_size; j++) {
> uint64_t l2_entry = be64_to_cpu(l2_table[j]);
> - int64_t offset = l2_entry & L2E_OFFSET_MASK, cluster_index;
> + int64_t offset = l2_entry & L2E_OFFSET_MASK;
> int cluster_type = qcow2_get_cluster_type(l2_entry);
> bool preallocated = offset != 0;
>
> - if (cluster_type == QCOW2_CLUSTER_NORMAL) {
> - cluster_index = offset >> s->cluster_bits;
> - assert((cluster_index >= 0) && (cluster_index <
> *nb_clusters));
> - if ((*expanded_clusters)[cluster_index / 8] &
> - (1 << (cluster_index % 8))) {
> - /* Probably a shared L2 table; this cluster was a zero
> - * cluster which has been expanded, its refcount
> - * therefore most likely requires an update. */
> - ret = qcow2_update_cluster_refcount(bs, cluster_index, 1,
> - QCOW2_DISCARD_NEVER);
> - if (ret < 0) {
> - goto fail;
> - }
> - /* Since we just increased the refcount, the COPIED flag
> may
> - * no longer be set. */
> - l2_table[j] = cpu_to_be64(l2_entry & ~QCOW_OFLAG_COPIED);
> - l2_dirty = true;
> - }
> - continue;
> - }
> - else if (qcow2_get_cluster_type(l2_entry) != QCOW2_CLUSTER_ZERO)
> {
> + if (cluster_type != QCOW2_CLUSTER_ZERO) {
> continue;
> }
>
> @@ -1642,6 +1621,19 @@ static int expand_zero_clusters_in_l1(BlockDriverState
> *bs, uint64_t *l1_table,
> ret = offset;
> goto fail;
> }
> +
> + if (l2_refcount > 1) {
> + /* For shared L2 tables, set the refcount accordingly
> (it is
> + * already 1 and needs to be l2_refcount) */
> + ret = qcow2_update_cluster_refcount(bs,
> + offset >> s->cluster_bits, l2_refcount - 1,
> + QCOW2_DISCARD_OTHER);
> + if (ret < 0) {
> + qcow2_free_clusters(bs, offset, s->cluster_size,
> + QCOW2_DISCARD_OTHER);
> + goto fail;
> + }
> + }
> }
>
> ret = qcow2_pre_write_overlap_check(bs, 0, offset,
> s->cluster_size);
> @@ -1663,29 +1655,12 @@ static int
> expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,
> goto fail;
> }
>
> - l2_table[j] = cpu_to_be64(offset | QCOW_OFLAG_COPIED);
> - l2_dirty = true;
> -
> - cluster_index = offset >> s->cluster_bits;
> -
> - if (cluster_index >= *nb_clusters) {
> - uint64_t old_bitmap_size = (*nb_clusters + 7) / 8;
> - uint64_t new_bitmap_size;
> - /* The offset may lie beyond the old end of the underlying
> image
> - * file for growable files only */
> - assert(bs->file->growable);
> - *nb_clusters = size_to_clusters(s, bs->file->total_sectors *
> - BDRV_SECTOR_SIZE);
> - new_bitmap_size = (*nb_clusters + 7) / 8;
> - *expanded_clusters = g_realloc(*expanded_clusters,
> - new_bitmap_size);
> - /* clear the newly allocated space */
> - memset(&(*expanded_clusters)[old_bitmap_size], 0,
> - new_bitmap_size - old_bitmap_size);
> + if (l2_refcount == 1) {
> + l2_table[j] = cpu_to_be64(offset | QCOW_OFLAG_COPIED);
> + } else {
> + l2_table[j] = cpu_to_be64(offset);
> }
> -
> - assert((cluster_index >= 0) && (cluster_index < *nb_clusters));
> - (*expanded_clusters)[cluster_index / 8] |= 1 << (cluster_index %
> 8);
> + l2_dirty = true;
> }
>
> if (is_active_l1) {
> @@ -1750,9 +1725,7 @@ int qcow2_expand_zero_clusters(BlockDriverState *bs,
> {
> BDRVQcowState *s = bs->opaque;
> uint64_t *l1_table = NULL;
> - uint64_t nb_clusters;
> int64_t l1_entries = 0, visited_l1_entries = 0;
> - uint8_t *expanded_clusters;
> int ret;
> int i, j;
>
> @@ -1763,12 +1736,7 @@ int qcow2_expand_zero_clusters(BlockDriverState *bs,
> }
> }
>
> - nb_clusters = size_to_clusters(s, bs->file->total_sectors *
> - BDRV_SECTOR_SIZE);
> - expanded_clusters = g_malloc0((nb_clusters + 7) / 8);
> -
> ret = expand_zero_clusters_in_l1(bs, s->l1_table, s->l1_size,
> - &expanded_clusters, &nb_clusters,
> &visited_l1_entries, l1_entries,
> status_cb);
> if (ret < 0) {
> @@ -1804,7 +1772,6 @@ int qcow2_expand_zero_clusters(BlockDriverState *bs,
> }
>
> ret = expand_zero_clusters_in_l1(bs, l1_table,
> s->snapshots[i].l1_size,
> - &expanded_clusters, &nb_clusters,
> &visited_l1_entries, l1_entries,
> status_cb);
> if (ret < 0) {
> @@ -1815,7 +1782,6 @@ int qcow2_expand_zero_clusters(BlockDriverState *bs,
> ret = 0;
>
> fail:
> - g_free(expanded_clusters);
> g_free(l1_table);
> return ret;
> }
> --
> 2.0.3
>
I don't understand this one very well so I will not Rev-By it.
