From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KyPgd-00077Y-55 for qemu-devel@nongnu.org; Fri, 07 Nov 2008 06:39:35 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KyPga-00075Y-Ib for qemu-devel@nongnu.org; Fri, 07 Nov 2008 06:39:32 -0500 Received: from [199.232.76.173] (port=34114 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KyPga-000757-E7 for qemu-devel@nongnu.org; Fri, 07 Nov 2008 06:39:32 -0500 Received: from ecfrec.frec.bull.fr ([129.183.4.8]:41568) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1KyPgZ-0004o0-5J for qemu-devel@nongnu.org; Fri, 07 Nov 2008 06:39:31 -0500 Subject: Re: [Qemu-devel] [PATCH 2/4] qcow2: Allow update_cluster_refcount() to update several clusters refcount. From: Laurent Vivier In-Reply-To: <491416BF.8070007@suse.de> References: <20081106165212.380421945@bull.net> <1225990558.6576.12.camel@frecb07144> <4913336E.3060309@suse.de> <1226052194.4046.21.camel@frecb07144> <491416BF.8070007@suse.de> Content-Type: text/plain; charset=utf-8 Date: Fri, 07 Nov 2008 12:39:26 +0100 Message-Id: <1226057966.4046.40.camel@frecb07144> Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable 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 Le vendredi 07 novembre 2008 =C3=A0 11:21 +0100, Kevin Wolf a =C3=A9crit = : > Laurent Vivier schrieb: [...] > >>> =20 > >>> @@ -2437,7 +2469,7 @@ static void update_refcount(BlockDriverS > >>> int addend) > >>> { > >>> BDRVQcowState *s =3D bs->opaque; > >>> - int64_t start, last, cluster_offset; > >>> + int64_t start, last; > >>> =20 > >>> #ifdef DEBUG_ALLOC2 > >>> printf("update_refcount: offset=3D%lld size=3D%lld addend=3D%d= \n", > >>> @@ -2445,12 +2477,9 @@ static void update_refcount(BlockDriverS > >>> #endif > >>> if (length <=3D 0) > >>> return; > >>> - start =3D offset & ~(s->cluster_size - 1); > >>> - last =3D (offset + length - 1) & ~(s->cluster_size - 1); > >>> - for(cluster_offset =3D start; cluster_offset <=3D last; > >>> - cluster_offset +=3D s->cluster_size) { > >>> - update_cluster_refcount(bs, cluster_offset >> s->cluster_b= its, addend); > >>> - } > >>> + start =3D offset >> s->cluster_bits; > >>> + last =3D (offset + length) >> s->cluster_bits; > >> Off by one for length % cluster_size =3D=3D 0? > >=20 > > Explain, please (but notice the "<=3D" in the "for (...)"). >=20 > I didn't even look at the old code yesterday, but the difference is tha= t > you dropped the - 1 for last. >=20 > Let's do it by example: Assume cluster_size =3D 0x1000 for simplicity a= nd > we get offset =3D 0x1000 and length =3D 0x2000 (i.e. the last affected = byte > would be 0x2fff). The old code correctly produces start =3D 0x1000 and > last =3D 0x2000 whereas you get start =3D 1 and last =3D 3. Yes, "last" must be "(offset + length - 1) >> s->cluster_bits" Thank you, Laurent --=20 ------------------ Laurent.Vivier@bull.net ------------------ "Tout ce qui est impossible reste =C3=A0 accomplir" Jules Verne "Things are only impossible until they're not" Jean-Luc Picard