From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LF7Fj-0006D0-47 for qemu-devel@nongnu.org; Tue, 23 Dec 2008 08:24:51 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LF7Fi-0006CX-2l for qemu-devel@nongnu.org; Tue, 23 Dec 2008 08:24:50 -0500 Received: from [199.232.76.173] (port=57050 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LF7Fh-0006CR-Tt for qemu-devel@nongnu.org; Tue, 23 Dec 2008 08:24:49 -0500 Received: from mx2.redhat.com ([66.187.237.31]:57570) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1LF7Fh-00032i-EN for qemu-devel@nongnu.org; Tue, 23 Dec 2008 08:24:49 -0500 Date: Tue, 23 Dec 2008 15:25:40 +0200 From: Gleb Natapov Message-ID: <20081223132540.GB3435@redhat.com> References: <494A8690.1080309@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <494A8690.1080309@suse.de> Subject: [Qemu-devel] Re: [PATCH] qcow2: Fix cluster allocation Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-devel@nongnu.org On Thu, Dec 18, 2008 at 06:21:20PM +0100, Kevin Wolf wrote: > When allocating multiple clusters at once, the qcow2 implementation > tries to find as many physically contiguous clusters as possible to > allow larger writes. This search includes allocated clusters which are > in the right place and still free clusters. If the range to allocate > spans clusters in patterns like "10 allocated, then 10 free, then again > 10 allocated" it is only checked that the chunks of allocated clusters > are contiguous for themselves. > > However, what is actually needed is to have _all_ allocated clusters > contiguous, starting at the first cluster of the allocation and spanning > multiple such chunks. This patch changes the check so that each offset > is not compared to the offset of the first cluster in its own chunk but > to the first cluster in the whole allocation. > > I haven't seen it happen, but without this fix data corruption on qcow2 > images is possible. > > This patch is meant to be applied on top of Gleb's compressed image fix. > > Signed-off-by: Kevin Wolf Acked-by: Gleb Natapov > Index: qemu-svn/block-qcow2.c > =================================================================== > --- qemu-svn.orig/block-qcow2.c > +++ qemu-svn/block-qcow2.c > @@ -615,7 +615,7 @@ static int size_to_clusters(BDRVQcowStat > } > > static int count_contiguous_clusters(uint64_t nb_clusters, int cluster_size, > - uint64_t *l2_table, uint64_t mask) > + uint64_t *l2_table, uint64_t start, uint64_t mask) > { > int i; > uint64_t offset = be64_to_cpu(l2_table[0]) & ~mask; > @@ -623,11 +623,11 @@ static int count_contiguous_clusters(uin > if (!offset) > return 0; > > - for (i = 0; i < nb_clusters; i++) > + for (i = start; i < start + nb_clusters; i++) > if (offset + i * cluster_size != (be64_to_cpu(l2_table[i]) & ~mask)) > break; > > - return i; > + return (i - start); > } > > static int count_contiguous_free_clusters(uint64_t nb_clusters, uint64_t *l2_table) > @@ -714,7 +714,7 @@ static uint64_t get_cluster_offset(Block > } else { > /* how many allocated clusters ? */ > c = count_contiguous_clusters(nb_clusters, s->cluster_size, > - &l2_table[l2_index], QCOW_OFLAG_COPIED); > + &l2_table[l2_index], 0, QCOW_OFLAG_COPIED); > } > > nb_available = (c * s->cluster_sectors); > @@ -968,7 +968,7 @@ static uint64_t alloc_cluster_offset(Blo > > if (cluster_offset & QCOW_OFLAG_COPIED) { > nb_clusters = count_contiguous_clusters(nb_clusters, s->cluster_size, > - &l2_table[l2_index], 0); > + &l2_table[l2_index], 0, 0); > > cluster_offset &= ~QCOW_OFLAG_COPIED; > m->nb_clusters = 0; > @@ -985,7 +985,7 @@ static uint64_t alloc_cluster_offset(Blo > > while (i < nb_clusters) { > i += count_contiguous_clusters(nb_clusters - i, s->cluster_size, > - &l2_table[l2_index + i], 0); > + &l2_table[l2_index], i, 0); > > if(be64_to_cpu(l2_table[l2_index + i])) > break; -- Gleb.