From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59872) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XAju2-0002Ad-HB for qemu-devel@nongnu.org; Fri, 25 Jul 2014 14:07:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XAjtv-0005IN-0x for qemu-devel@nongnu.org; Fri, 25 Jul 2014 14:07:34 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35016) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XAjtu-0005HO-Pf for qemu-devel@nongnu.org; Fri, 25 Jul 2014 14:07:26 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s6PI7PTF002803 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Fri, 25 Jul 2014 14:07:26 -0400 From: Max Reitz Date: Fri, 25 Jul 2014 20:07:43 +0200 Message-Id: <1406311665-2814-7-git-send-email-mreitz@redhat.com> In-Reply-To: <1406311665-2814-1-git-send-email-mreitz@redhat.com> References: <1406311665-2814-1-git-send-email-mreitz@redhat.com> Subject: [Qemu-devel] [PATCH 6/8] block/qcow2: Simplify shared L2 handling in amend List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Kevin Wolf , Stefan Hajnoczi , Max Reitz 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 --- block/qcow2-cluster.c | 83 +++++++++++++-------------------------------------- 1 file changed, 21 insertions(+), 62 deletions(-) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 0f52ef6..905beb6 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -1543,12 +1543,6 @@ 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. - * * zero_clusters_count and *expanded_count are used to keep track of progress * for status_cb(). zero_clusters_count contains the total number of L2 zero * cluster entries. Accordingly, *expanded_count counts all visited L2 zero @@ -1557,9 +1551,7 @@ fail: * cluster. */ 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 *expanded_count, + int l1_size, int64_t *expanded_count, int64_t zero_clusters_count, BlockDriverAmendStatusCB *status_cb) { @@ -1606,31 +1598,11 @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table, 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; } @@ -1650,6 +1622,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); @@ -1671,29 +1656,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) { @@ -1829,9 +1797,7 @@ int qcow2_expand_zero_clusters(BlockDriverState *bs, { BDRVQcowState *s = bs->opaque; uint64_t *l1_table = NULL; - uint64_t nb_clusters; int64_t zero_cluster_count = 0, expanded_count = 0; - uint8_t *expanded_clusters; int ret; int i, j; @@ -1871,12 +1837,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, &expanded_count, zero_cluster_count, status_cb); if (ret < 0) { @@ -1912,7 +1873,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, &expanded_count, zero_cluster_count, status_cb); if (ret < 0) { @@ -1923,7 +1883,6 @@ int qcow2_expand_zero_clusters(BlockDriverState *bs, ret = 0; fail: - g_free(expanded_clusters); g_free(l1_table); return ret; } -- 2.0.1