From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34129) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XgsA6-00030z-O8 for qemu-devel@nongnu.org; Wed, 22 Oct 2014 05:25:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xgs0J-0000ai-Ds for qemu-devel@nongnu.org; Wed, 22 Oct 2014 05:16:17 -0400 Received: from mx1.redhat.com ([209.132.183.28]:20085) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xgs0J-0000Zl-10 for qemu-devel@nongnu.org; Wed, 22 Oct 2014 05:14:51 -0400 Date: Wed, 22 Oct 2014 11:14:44 +0200 From: Kevin Wolf Message-ID: <20141022091444.GC3188@noname.str.redhat.com> References: <1413965324-14541-1-git-send-email-mreitz@redhat.com> <1413965324-14541-11-git-send-email-mreitz@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1413965324-14541-11-git-send-email-mreitz@redhat.com> Subject: Re: [Qemu-devel] [PATCH v7 10/13] qcow2: Rebuild refcount structure during check List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: qemu-devel@nongnu.org, Stefan Hajnoczi , =?iso-8859-1?Q?Beno=EEt?= Canet Am 22.10.2014 um 10:08 hat Max Reitz geschrieben: > The previous commit introduced the "rebuild" variable to qcow2's > implementation of the image consistency check. Now make use of this by > adding a function which creates a completely new refcount structure > based solely on the in-memory information gathered before. > > The old refcount structure will be leaked, however. This leak will be > dealt with in a follow-up commit. > > Signed-off-by: Max Reitz > --- > block/qcow2-refcount.c | 310 ++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 304 insertions(+), 6 deletions(-) > > diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c > index 101d7bb..8ca2324 100644 > --- a/block/qcow2-refcount.c > +++ b/block/qcow2-refcount.c > @@ -1640,6 +1640,284 @@ static void compare_refcounts(BlockDriverState *bs, BdrvCheckResult *res, > } > > /* > + * Allocates clusters using an in-memory refcount table (IMRT) in contrast to > + * the on-disk refcount structures. > + * > + * On input, *first_free_cluster tells where to start looking, and need not > + * actually be a free cluster; the returned offset will not be before that > + * cluster. On output, *first_free_cluster points to the first gap found, even > + * if that gap was too small to be used as the returned offset. > + * > + * Note that *first_free_cluster is a cluster index whereas the return value is > + * an offset. > + */ > +static int64_t alloc_clusters_imrt(BlockDriverState *bs, > + int cluster_count, > + uint16_t **refcount_table, > + int64_t *imrt_nb_clusters, > + int64_t *first_free_cluster) > +{ > + BDRVQcowState *s = bs->opaque; > + int64_t cluster = *first_free_cluster, i; > + bool first_gap = true; > + int contiguous_free_clusters; > + > + /* Starting at *first_free_cluster, find a range of at least cluster_count > + * continuously free clusters */ > + for (contiguous_free_clusters = 0; > + cluster < *imrt_nb_clusters && > + contiguous_free_clusters < cluster_count; > + cluster++) > + { > + if (!(*refcount_table)[cluster]) { > + contiguous_free_clusters++; > + if (first_gap) { > + /* If this is the first free cluster found, update > + * *first_free_cluster accordingly */ > + *first_free_cluster = cluster; > + first_gap = false; > + } > + } else if (contiguous_free_clusters) { > + contiguous_free_clusters = 0; > + } > + } > + > + /* If contiguous_free_clusters is greater than zero, it contains the number > + * of continuously free clusters until the current cluster; the first free > + * cluster in the current "gap" is therefore > + * cluster - contiguous_free_clusters */ > + > + /* If no such range could be found, grow the in-memory refcount table > + * accordingly to append free clusters at the end of the image */ > + if (contiguous_free_clusters < cluster_count) { > + int64_t old_imrt_nb_clusters = *imrt_nb_clusters; > + uint16_t *new_refcount_table; > + > + /* contiguous_free_clusters clusters are already empty at the image end; > + * we need cluster_count clusters; therefore, we have to allocate > + * cluster_count - contiguous_free_clusters new clusters at the end of > + * the image (which is the current value of cluster; note that cluster > + * may exceed old_imrt_nb_clusters if *first_free_cluster pointed beyond > + * the image end) */ > + *imrt_nb_clusters = cluster + cluster_count - contiguous_free_clusters; > + new_refcount_table = g_try_realloc(*refcount_table, > + *imrt_nb_clusters * > + sizeof(**refcount_table)); > + if (!new_refcount_table) { > + return -ENOMEM; Now the old refcount table is preserved, but nb_clusters has already been updated. Smells like the next thing will be a buffer overflow, doesn't it? > + } > + *refcount_table = new_refcount_table; > + > + memset(*refcount_table + old_imrt_nb_clusters, 0, > + (*imrt_nb_clusters - old_imrt_nb_clusters) * > + sizeof(**refcount_table)); > + } Kevin