From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43012) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XC6sq-00014V-6M for qemu-devel@nongnu.org; Tue, 29 Jul 2014 08:52:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XC6sg-0004yC-8G for qemu-devel@nongnu.org; Tue, 29 Jul 2014 08:52:00 -0400 Received: from mx1.redhat.com ([209.132.183.28]:22070) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XC6sg-0004y7-0F for qemu-devel@nongnu.org; Tue, 29 Jul 2014 08:51:50 -0400 Date: Tue, 29 Jul 2014 13:51:44 +0100 From: Stefan Hajnoczi Message-ID: <20140729125144.GA25860@stefanha-thinkpad.redhat.com> References: <1401180562-29680-1-git-send-email-famz@redhat.com> <1401180562-29680-3-git-send-email-famz@redhat.com> <20140728151110.GG13872@stefanha-thinkpad.redhat.com> <20140729010043.GA6544@T430.nay.redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="0OAP2g/MAC+5xKAE" Content-Disposition: inline In-Reply-To: <20140729010043.GA6544@T430.nay.redhat.com> Subject: Re: [Qemu-devel] [PATCH v5 2/2] vmdk: Optimize cluster allocation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: Kevin Wolf , Stefan Hajnoczi , qemu-devel@nongnu.org --0OAP2g/MAC+5xKAE Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Jul 29, 2014 at 09:00:43AM +0800, Fam Zheng wrote: > On Mon, 07/28 16:11, Stefan Hajnoczi wrote: > > On Tue, May 27, 2014 at 04:49:22PM +0800, Fam Zheng wrote: > > > + if (!bs->backing_hd) { > > > + memset(whole_grain, 0, skip_start_sector << BDRV_SECTOR_BIT= S); > > > + memset(whole_grain + (skip_end_sector << BDRV_SECTOR_BITS), = 0, > > > + cluster_bytes - (skip_end_sector << BDRV_SECTOR_BITS)= ); > > > + } > > > + > > > + assert(skip_end_sector <=3D sector_num + extent->cluster_sectors= ); > >=20 > > Does this assertion make sense? skip_end_sector is a small number of > > sectors (relative to start of cluster), while sector_num + > > extent->cluster_sectors is a large absolute sector offset. >=20 > skip_end_sector is absolute sector number too. The caller hunk in this pa= tch > is: I disagree. If it was an absolute sector number then the memset() a few lines above would be incorrect: memset(whole_grain, 0, skip_start_sector << BDRV_SECTOR_BITS); memset(whole_grain + (skip_end_sector << BDRV_SECTOR_BITS), 0, cluster_bytes - (skip_end_sector << BDRV_SECTOR_BITS)); Look at the code you pasted again: > @@ -1406,12 +1468,17 @@ static int vmdk_write(BlockDriverState *bs, int64= _t sector_num, > if (!extent) { > return -EIO; > } > - ret =3D get_cluster_offset( > - bs, > - extent, > - &m_data, > - sector_num << 9, !extent->compressed, > - &cluster_offset); > + extent_begin_sector =3D extent->end_sector - extent->sectors; > + extent_relative_sector_num =3D sector_num - extent_begin_sector; > + index_in_cluster =3D extent_relative_sector_num % extent->cluste= r_sectors; > + n =3D extent->cluster_sectors - index_in_cluster; > + if (n > nb_sectors) { > + n =3D nb_sectors; > + } > + ret =3D get_cluster_offset(bs, extent, &m_data, sector_num << 9, > + !(extent->compressed || zeroed), > + &cluster_offset, > + index_in_cluster, index_in_cluster + n); > if (extent->compressed) { > if (ret =3D=3D VMDK_OK) { > /* Refuse write to allocated cluster for streamOptimized= */ >=20 > See the last parameter of get_cluster_offset. The last parameter is (extent_relative_sector_num % extent->cluster_sectors) + (extent->cluster_sectors - index_in_cluster). Those are definitely sector counts (like nb_sectors) and not absolute sector numbers (like sector_num). --0OAP2g/MAC+5xKAE Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJT15jgAAoJEJykq7OBq3PIkk8IAKGgNY9JHdIifES+9QxF4UJn bDJtfiXiy2QtbjI4llxLpPhdsjx7MSpmJpxDxed7nM9C0iY8UyzcDVuQQ6WTZJjC Jz924+FtX6BG6ZZ4WTMpzRTmePEz1RelC3pQjMX/mhFg7Iof9gu6LNOE9XpqDdhm IodI0TDbIQPR8mc3q/5bwPbOiPvO+GphX5s9QloLfv+6qsYQY4lGxpbZGHybP7NQ Qjag3B7qyZ5SD5SnI0pk2VdHNNR17BU7JsC9TJ1Sz7+D6PWh6kDxfng9n6ksmzlm H+kjQ7FFTaNBlDQg4H9LcARolfaDj1ydwkShEJaO209hlEV+LgN4ivkG9EOsB7Y= =kGS9 -----END PGP SIGNATURE----- --0OAP2g/MAC+5xKAE--