From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54550) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bA7zX-0003Rj-BT for qemu-devel@nongnu.org; Mon, 06 Jun 2016 23:47:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bA7zV-00009m-6d for qemu-devel@nongnu.org; Mon, 06 Jun 2016 23:47:46 -0400 References: <1465225155-18661-1-git-send-email-kwolf@redhat.com> <1465225155-18661-4-git-send-email-kwolf@redhat.com> From: Eric Blake Message-ID: <575643D7.3090807@redhat.com> Date: Mon, 6 Jun 2016 21:47:35 -0600 MIME-Version: 1.0 In-Reply-To: <1465225155-18661-4-git-send-email-kwolf@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="d4TQRoWD4akotRJfRfqRJkRQC1MoRFnkW" Subject: Re: [Qemu-devel] [PATCH v2 3/5] qcow2: Make copy_sectors() byte based List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf , qemu-block@nongnu.org Cc: mreitz@redhat.com, jsnow@redhat.com, qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --d4TQRoWD4akotRJfRfqRJkRQC1MoRFnkW Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 06/06/2016 08:59 AM, Kevin Wolf wrote: > This will allow copy on write operations where the overwritten part of > the cluster is not aligned to sector boundaries. >=20 > Also rename the function because it has nothing to do with sectors any > more. >=20 > Signed-off-by: Kevin Wolf > --- > block/qcow2-cluster.c | 54 +++++++++++++++++++++++++------------------= -------- > 1 file changed, 26 insertions(+), 28 deletions(-) >=20 > =20 > if (bs->encrypted) { > Error *err =3D NULL; > + int sector =3D (cluster_offset + offset_in_cluster) >> BDRV_SE= CTOR_BITS; Potentially the wrong type... > assert(s->cipher); > - if (qcow2_encrypt_sectors(s, start_sect + n_start, > - iov.iov_base, iov.iov_base, n, > - true, &err) < 0) { > + assert((offset_in_cluster & BDRV_SECTOR_MASK) =3D=3D 0); Why is this one true? If I have a cluster of 4 sectors, why must offset_in_cluster fall within only the first of those sectors? Are you missing a ~, to instead be asserting that offset_in_cluster is sector-aligned? > + assert((bytes & ~BDRV_SECTOR_MASK) =3D=3D 0); This one looks correct, stating that the number of bytes to copy is a sector multiple. > + if (qcow2_encrypt_sectors(s, sector, iov.iov_base, iov.iov_bas= e, > + bytes >> BDRV_SECTOR_BITS, true, &er= r) < 0) { =2E..since encryption allows a 64-bit sector number for the case where th= e image is larger than 2T. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --d4TQRoWD4akotRJfRfqRJkRQC1MoRFnkW Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJXVkPXAAoJEKeha0olJ0NqMkAH/1jh6DCKjb2cd1WtuW0r1BHx OZ2IKmrltKKJkWPsc0ROBsX+EprUy3mUkb8akztcZVo7Vxq51DdGZz0H7hGpZMeJ IQfg1dBcxEXCWIspuWTvAGVsVaS2vQum22HpARM2nDxGUSLCJNVIcI8xOIqxl+Ba E1LcwN3MUkuwst6+Sb8DzYcQYiTOWHZLd6baKH0l6CThTlpAU5U9U55r9pDrvKBD aWF83GQi1zsavcle6SfUVuvGQK5/V13MI9thyjre9xPOfo+h9ihFTsp3VuhcKIQB nXWpawNO4rhdJ93OqkJQuJvTxYIzqrt7+KGzAbHtu1Qqyjchku9Mzi6kEh7PCc4= =qIlK -----END PGP SIGNATURE----- --d4TQRoWD4akotRJfRfqRJkRQC1MoRFnkW--