From patchwork Wed Feb 10 16:54:41 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Wolf X-Patchwork-Id: 45047 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [199.232.76.165]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id AE497B7C48 for ; Thu, 11 Feb 2010 04:04:36 +1100 (EST) Received: from localhost ([127.0.0.1]:49696 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NfFzM-00066A-17 for incoming@patchwork.ozlabs.org; Wed, 10 Feb 2010 12:04:32 -0500 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NfFqy-0000j6-2t for qemu-devel@nongnu.org; Wed, 10 Feb 2010 11:55:52 -0500 Received: from [199.232.76.173] (port=49371 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NfFqx-0000ik-ME for qemu-devel@nongnu.org; Wed, 10 Feb 2010 11:55:51 -0500 Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim 4.60) (envelope-from ) id 1NfFqj-0004i1-Lv for qemu-devel@nongnu.org; Wed, 10 Feb 2010 11:55:49 -0500 Received: from mx20.gnu.org ([199.232.41.8]:36256) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1NfFqi-0004h4-DP for qemu-devel@nongnu.org; Wed, 10 Feb 2010 11:55:37 -0500 Received: from mx1.redhat.com ([209.132.183.28]) by mx20.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NfFqf-00026N-RZ for qemu-devel@nongnu.org; Wed, 10 Feb 2010 11:55:34 -0500 Received: from int-mx04.intmail.prod.int.phx2.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.17]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o1AGtV5V026057 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 10 Feb 2010 11:55:31 -0500 Received: from localhost.localdomain (dhcp-5-175.str.redhat.com [10.32.5.175]) by int-mx04.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o1AGtU70002254; Wed, 10 Feb 2010 11:55:30 -0500 From: Kevin Wolf To: qemu-devel@nongnu.org Date: Wed, 10 Feb 2010 17:54:41 +0100 Message-Id: <1265820881-22773-1-git-send-email-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.67 on 10.5.11.17 X-detected-operating-system: by mx20.gnu.org: Genre and OS details not recognized. X-detected-operating-system: by monty-python.gnu.org: GNU/Linux 2.6, seldom 2.4 (older, 4) Cc: kwolf@redhat.com Subject: [Qemu-devel] [RFC] qcow2: Rewrite alloc_refcount_block X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org The current implementation of alloc_refcount_block and grow_refcount_table has fundamental problems regarding error handling. There are some places where an I/O error means that the image is going to be corrupted. I have found that the only way to fix this is to completely rewrite the thing. This is a first version that passes qemu-iotests but hasn't undergone a lot of testing otherwise. For the real submission I think I'm going to split it into maybe three patches, but I want to get an early review as this is really critical code and tests alone won't be enough to verify it. Thorough reviews would really be appreciated. --- block/qcow2-refcount.c | 333 +++++++++++++++++++++++++++++++++++------------- 1 files changed, 243 insertions(+), 90 deletions(-) diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index 2fdc26b..66a7912 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -27,7 +27,7 @@ #include "block/qcow2.h" static int64_t alloc_clusters_noref(BlockDriverState *bs, int64_t size); -static int update_refcount(BlockDriverState *bs, +static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs, int64_t offset, int64_t length, int addend); @@ -123,124 +123,265 @@ static int get_refcount(BlockDriverState *bs, int64_t cluster_index) return be16_to_cpu(s->refcount_block_cache[block_index]); } -static int grow_refcount_table(BlockDriverState *bs, int min_size) +/* + * Rounds the refcount table size up to avoid growing the table for each single + * refcount block that is allocated. + */ +static unsigned int next_refcount_table_size(BDRVQcowState *s, + unsigned int min_size) { - BDRVQcowState *s = bs->opaque; - int new_table_size, new_table_size2, refcount_table_clusters, i, ret; - uint64_t *new_table; - int64_t table_offset; - uint8_t data[12]; - int old_table_size; - int64_t old_table_offset; + unsigned int refcount_table_clusters = 0; + unsigned int new_table_size = 1; - if (min_size <= s->refcount_table_size) - return 0; - /* compute new table size */ - refcount_table_clusters = s->refcount_table_size >> (s->cluster_bits - 3); - for(;;) { + while (min_size > new_table_size) { if (refcount_table_clusters == 0) { refcount_table_clusters = 1; } else { refcount_table_clusters = (refcount_table_clusters * 3 + 1) / 2; } new_table_size = refcount_table_clusters << (s->cluster_bits - 3); - if (min_size <= new_table_size) - break; } -#ifdef DEBUG_ALLOC2 - printf("grow_refcount_table from %d to %d\n", - s->refcount_table_size, - new_table_size); -#endif - new_table_size2 = new_table_size * sizeof(uint64_t); - new_table = qemu_mallocz(new_table_size2); - memcpy(new_table, s->refcount_table, - s->refcount_table_size * sizeof(uint64_t)); - for(i = 0; i < s->refcount_table_size; i++) - cpu_to_be64s(&new_table[i]); - /* Note: we cannot update the refcount now to avoid recursion */ - table_offset = alloc_clusters_noref(bs, new_table_size2); - ret = bdrv_pwrite(s->hd, table_offset, new_table, new_table_size2); - if (ret != new_table_size2) - goto fail; - for(i = 0; i < s->refcount_table_size; i++) - be64_to_cpus(&new_table[i]); - cpu_to_be64w((uint64_t*)data, table_offset); - cpu_to_be32w((uint32_t*)(data + 8), refcount_table_clusters); - ret = bdrv_pwrite(s->hd, offsetof(QCowHeader, refcount_table_offset), - data, sizeof(data)); - if (ret != sizeof(data)) { - goto fail; - } + return new_table_size; +} - qemu_free(s->refcount_table); - old_table_offset = s->refcount_table_offset; - old_table_size = s->refcount_table_size; - s->refcount_table = new_table; - s->refcount_table_size = new_table_size; - s->refcount_table_offset = table_offset; +/* Checks if two offsets are described by the same refcount block */ +static int in_same_refcount_block(BDRVQcowState *s, uint64_t offset_a, + uint64_t offset_b) +{ + uint64_t block_a = offset_a >> (2 * s->cluster_bits - REFCOUNT_SHIFT); + uint64_t block_b = offset_b >> (2 * s->cluster_bits - REFCOUNT_SHIFT); - update_refcount(bs, table_offset, new_table_size2, 1); - qcow2_free_clusters(bs, old_table_offset, old_table_size * sizeof(uint64_t)); - return 0; - fail: - qemu_free(new_table); - return ret < 0 ? ret : -EIO; + return (block_a == block_b); } - +/* + * Loads a refcount block. If it doesn't exist yet, it is allocated first + * (including growing the refcount table if needed). + * + * Returns the offset of the refcount block on success or -errno in error case + */ static int64_t alloc_refcount_block(BlockDriverState *bs, int64_t cluster_index) { BDRVQcowState *s = bs->opaque; - int64_t offset, refcount_block_offset; unsigned int refcount_table_index; + uint64_t refcount_block_offset; int ret; - uint64_t data64; - int cache = cache_refcount_updates; - /* Find L1 index and grow refcount table if needed */ + /* Find the refcount block for the given cluster */ refcount_table_index = cluster_index >> (s->cluster_bits - REFCOUNT_SHIFT); if (refcount_table_index >= s->refcount_table_size) { - ret = grow_refcount_table(bs, refcount_table_index + 1); - if (ret < 0) - return ret; + refcount_block_offset = 0; + } else { + refcount_block_offset = s->refcount_table[refcount_table_index]; } - /* Load or allocate the refcount block */ - refcount_block_offset = s->refcount_table[refcount_table_index]; - if (!refcount_block_offset) { - if (cache_refcount_updates) { - write_refcount_block(s); - cache_refcount_updates = 0; + /* If it's already there, we're done */ + if (refcount_block_offset) { + if (refcount_block_offset != s->refcount_block_cache_offset) { + ret = load_refcount_block(bs, refcount_block_offset); + if (ret < 0) { + return ret; + } } - /* create a new refcount block */ - /* Note: we cannot update the refcount now to avoid recursion */ - offset = alloc_clusters_noref(bs, s->cluster_size); - memset(s->refcount_block_cache, 0, s->cluster_size); - ret = bdrv_pwrite(s->hd, offset, s->refcount_block_cache, s->cluster_size); - if (ret != s->cluster_size) - return -EINVAL; - s->refcount_table[refcount_table_index] = offset; - data64 = cpu_to_be64(offset); - ret = bdrv_pwrite(s->hd, s->refcount_table_offset + - refcount_table_index * sizeof(uint64_t), - &data64, sizeof(data64)); - if (ret != sizeof(data64)) - return -EINVAL; - - refcount_block_offset = offset; - s->refcount_block_cache_offset = offset; - update_refcount(bs, offset, s->cluster_size, 1); - cache_refcount_updates = cache; + return refcount_block_offset; + } + + /* + * If we came here, we need to allocate something. Something is at least + * a cluster for the new refcount block. It may also include a new refcount + * table if the old refcount table is too small. + * + * Note that allocating clusters here needs some special care: + * + * - We can't use the normal qcow2_alloc_clusters(), it would try to + * increase the refcount and very likely we would end up with an endless + * recursion. Instead we must place the refcount blocks in a way that + * they can describe them themselves. + * + * - We need to consider that at this point we are inside update_refcounts + * and doing the initial refcount increase. This means that some clusters + * have already been allocated by the caller, but their refcount isn't + * accurate yet. free_cluster_index tells us where this allocation ends + * as long as we don't overwrite it by freeing clusters. + * + * - alloc_clusters_noref and qcow2_free_clusters may load a different + * refcount block into the cache + */ + + if (cache_refcount_updates) { + write_refcount_block(s); + } + + /* Allocate the refcount block itself and mark it as used */ + uint64_t new_block = alloc_clusters_noref(bs, s->cluster_size); + memset(s->refcount_block_cache, 0, s->cluster_size); + s->refcount_block_cache_offset = new_block; + +#ifdef DEBUG_ALLOC2 + fprintf(stderr, "qcow2: Allocate refcount block %d for %" PRIx64 + " at %" PRIx64 "\n", + refcount_table_index, cluster_index << s->cluster_bits, new_block); +#endif + + if (in_same_refcount_block(s, new_block, cluster_index << s->cluster_bits)) { + /* The block describes itself, need to update the cache */ + int block_index = (new_block >> s->cluster_bits) & + ((1 << (s->cluster_bits - REFCOUNT_SHIFT)) - 1); + s->refcount_block_cache[block_index] = cpu_to_be16(1); } else { - if (refcount_block_offset != s->refcount_block_cache_offset) { - if (load_refcount_block(bs, refcount_block_offset) < 0) - return -EIO; + /* Described somewhere else. This can recurse at most twice before we + * arrive at a block that describes itself. */ + ret = update_refcount(bs, new_block, s->cluster_size, 1); + if (ret < 0) { + goto fail_block; + } + } + + /* Now the new refcount block needs to be written to disk */ + ret = bdrv_pwrite(s->hd, new_block, s->refcount_block_cache, + s->cluster_size); + if (ret < 0) { + goto fail_block; + } + + /* If the refcount table is big enough, just hook the block up there */ + if (refcount_table_index < s->refcount_table_size) { + uint64_t data64 = cpu_to_be64(new_block); + ret = bdrv_pwrite(s->hd, + s->refcount_table_offset + refcount_table_index * sizeof(uint64_t), + &data64, sizeof(data64)); + if (ret < 0) { + goto fail_block; } + + s->refcount_table[refcount_table_index] = new_block; + return new_block; } - return refcount_block_offset; + /* + * If we come here, we need to grow the refcount table. Again, a new + * refcount table needs some space and we can't simply allocate to avoid + * endless recursion. + * + * Therefore let's grab new refcount blocks at the end of the image, which + * will describe themselves and the new refcount table. This way we can + * reference them only in the new table and do the switch to the new + * refcount table at once without producing an inconsistent state in + * between. + */ + /* Calculate the number of refcount blocks needed so far */ + uint64_t refcount_block_clusters = 1 << (s->cluster_bits - REFCOUNT_SHIFT); + uint64_t blocks_used = (s->free_cluster_index + + refcount_block_clusters - 1) / refcount_block_clusters; + + /* And now we need at least one block more for the new metadata */ + uint64_t table_size = next_refcount_table_size(s, blocks_used + 1); + uint64_t last_table_size = table_size; + uint64_t blocks_clusters; + do { + uint64_t table_clusters = size_to_clusters(s, table_size); + blocks_clusters = 1 + + ((table_clusters + refcount_block_clusters - 1) + / refcount_block_clusters); + uint64_t meta_clusters = table_clusters + blocks_clusters; + + table_size = next_refcount_table_size(s, blocks_used + + ((meta_clusters + refcount_block_clusters - 1) + / refcount_block_clusters)); + + } while (last_table_size != table_size); + +#ifdef DEBUG_ALLOC2 + fprintf(stderr, "qcow2: Grow refcount table %" PRId32 " => %" PRId64 "\n", + s->refcount_table_size, table_size); +#endif + + /* Create the new refcount table and blocks */ + uint64_t meta_offset = (blocks_used * refcount_block_clusters) * + s->cluster_size; + uint64_t table_offset = meta_offset + blocks_clusters * s->cluster_size; + uint16_t *new_blocks = qemu_mallocz(blocks_clusters * s->cluster_size); + uint64_t *new_table = qemu_mallocz(table_size * sizeof(uint64_t)); + + assert(meta_offset >= (s->free_cluster_index * s->cluster_size)); + + /* Fill the new refcount table */ + memcpy(new_table, s->refcount_table, + s->refcount_table_size * sizeof(uint64_t)); + new_table[refcount_table_index] = new_block; + + int i; + for (i = 0; i < blocks_clusters; i++) { + new_table[blocks_used + i] = meta_offset + (i * s->cluster_size); + } + + /* Fill the refcount blocks */ + uint64_t table_clusters = size_to_clusters(s, table_size * sizeof(uint64_t)); + int block = 0; + for (i = 0; i < table_clusters + blocks_clusters; i++) { + new_blocks[block++] = cpu_to_be16(1); + } + + /* Write refcount blocks to disk */ + ret = bdrv_pwrite(s->hd, meta_offset, new_blocks, + blocks_clusters * s->cluster_size); + qemu_free(new_blocks); + if (ret < 0) { + goto fail_table; + } + + /* Write refcount table to disk */ + for(i = 0; i < table_size; i++) { + cpu_to_be64s(&new_table[i]); + } + + ret = bdrv_pwrite(s->hd, table_offset, new_table, + table_size * sizeof(uint64_t)); + if (ret < 0) { + goto fail_table; + } + + for(i = 0; i < table_size; i++) { + cpu_to_be64s(&new_table[i]); + } + + /* Hook up the new refcount table in the qcow2 header */ + uint8_t data[12]; + cpu_to_be64w((uint64_t*)data, table_offset); + cpu_to_be32w((uint32_t*)(data + 8), table_clusters); + ret = bdrv_pwrite(s->hd, offsetof(QCowHeader, refcount_table_offset), + data, sizeof(data)); + if (ret < 0) { + goto fail_table; + } + + /* And switch it in memory */ + uint64_t old_table_offset = s->refcount_table_offset; + uint64_t old_table_size = s->refcount_table_size; + + qemu_free(s->refcount_table); + s->refcount_table = new_table; + s->refcount_table_size = table_size; + s->refcount_table_offset = table_offset; + + /* Free old table. Remember, we must not change free_cluster_index */ + uint64_t old_free_cluster_index = s->free_cluster_index; + qcow2_free_clusters(bs, old_table_offset, old_table_size * sizeof(uint64_t)); + s->free_cluster_index = old_free_cluster_index; + + ret = load_refcount_block(bs, new_block); + if (ret < 0) { + goto fail_block; + } + + return new_block; + +fail_table: + qemu_free(new_table); +fail_block: + s->refcount_block_cache_offset = 0; + return ret; } #define REFCOUNTS_PER_SECTOR (512 >> REFCOUNT_SHIFT) @@ -923,9 +1064,21 @@ int qcow2_check_refcounts(BlockDriverState *bs) for(i = 0; i < s->refcount_table_size; i++) { int64_t offset; offset = s->refcount_table[i]; + + /* Refcount blocks are cluster aligned */ + if (offset & (s->cluster_size - 1)) { + fprintf(stderr, "ERROR refcount block %d is not " + "cluster aligned; refcount table entry corrupted\n", i); + errors++; + } + if (offset != 0) { errors += inc_refcounts(bs, refcount_table, nb_clusters, offset, s->cluster_size); + if (refcount_table[offset / s->cluster_size] != 1) { + fprintf(stderr, "ERROR refcount block %d refcount=%d\n", + i, refcount_table[offset / s->cluster_size]); + } } }