From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40003) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XNVCs-0006vY-Jn for qemu-devel@nongnu.org; Fri, 29 Aug 2014 19:03:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XNVCn-0000XT-MV for qemu-devel@nongnu.org; Fri, 29 Aug 2014 19:03:46 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34799) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XNVCn-0000XJ-EX for qemu-devel@nongnu.org; Fri, 29 Aug 2014 19:03:41 -0400 Message-ID: <540106C8.20905@redhat.com> Date: Fri, 29 Aug 2014 17:03:36 -0600 From: Eric Blake MIME-Version: 1.0 References: <1409348463-16627-1-git-send-email-mreitz@redhat.com> <1409348463-16627-2-git-send-email-mreitz@redhat.com> In-Reply-To: <1409348463-16627-2-git-send-email-mreitz@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="oqA2ixtvKOS32UoFWsurd21lkkIfWKohh" Subject: Re: [Qemu-devel] [PATCH v5 01/11] qcow2: Calculate refcount block entry count List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz , qemu-devel@nongnu.org Cc: Kevin Wolf , Stefan Hajnoczi , =?UTF-8?B?QmVub8OudCBDYW5ldA==?= This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --oqA2ixtvKOS32UoFWsurd21lkkIfWKohh Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 08/29/2014 03:40 PM, Max Reitz wrote: > The size of a refblock entry is (in theory) variable; calculate > therefore the number of entries per refblock and the according bit shif= t > (1 << x =3D=3D entry count) when opening an image. >=20 > Signed-off-by: Max Reitz > --- > block/qcow2.c | 2 ++ > block/qcow2.h | 2 ++ > 2 files changed, 4 insertions(+) What is the maximum refcount_order? The specs don't mention; the file format is wide open to overflows. Even something as benign-sounding as refcount_order=3D6 (64 bits) means that each cluster can be referenced 2**64 times, which is far longer than our lifetimes to build it up that high incrementally, and represents far greater than the amount of storage in existence being deduplicated! Shockingly easy to start getting into undefined territory, so maybe we ought to explicitly cap refcount_order to 6. >=20 > diff --git a/block/qcow2.c b/block/qcow2.c > index f9e045f..172ad00 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -689,6 +689,8 @@ static int qcow2_open(BlockDriverState *bs, QDict *= options, int flags, > =20 > s->l2_bits =3D s->cluster_bits - 3; /* L2 is always one cluster */= > s->l2_size =3D 1 << s->l2_bits; > + s->refcount_block_bits =3D s->cluster_bits - (s->refcount_order - = 3); Hmm; we document that qemu requires cluster_bits to be between 9 and 21 inclusive. So, if cluster_bits is 9 (512-byte clusters), and refcount_order is 6, then we can pack in 9 - (6 - 3) or 2**6 (that is, 64) refcounts per cluster. On the other extreme, the minimum refcount_order of 0 (each cluster occupies refcount bits, and so is either allocated or not, but no sharing), starts having the math looks ugly, because you are mixing: (int) =3D (uint32_t) - ( (uint32_t) - (int) ) so at one point, you are doing s->cluster_bits - (4294967293U), but that wraps around (thankfully, wraparound is well-defined on unsigned types) for a net answer of cluster_bits + 3. But in the worst case, that means an image with 2M clusters will be packing 21 - (0 - 3) or 2**24 (that is, 16M) refcounts in one cluster. Still fits in an int, so it looks like you are safe... > + s->refcount_block_size =3D 1 << s->refcount_block_bits; =2E..that this particular shift will not cause undefined behavior, for reasonable refcount_order in the range [0,6]. Reviewed-by: Eric Blake [We really ought to tighten the qcow2 spec - but that's a separate patch]= --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --oqA2ixtvKOS32UoFWsurd21lkkIfWKohh Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg iQEcBAEBCAAGBQJUAQbIAAoJEKeha0olJ0NqlIsH/37oHkoaytaDZ4UPnB+AgugM lvfwEKBu9mZuDO3YDLfH3SZnbr5wG6qeUatY2RRGa7u++/NtJKYpRgje4MkiN0jh 93YfWVNpu94ppKuf03wkvlUM2bPwK9MEl22ksXE6YvvBGUjxdU6S/tjikPJKKZF3 0Fki97HXrg8yCukRFX9RVmxDVYWHWVMeoxb/UK1tkZfQgsKyyDsWFpDWlMyHrTXf /fQtBOzZ4EQbi+N/FO7cBqaM7dC4cW5oe/mezKwbwRNftWJ+Upzu8pnmacWrQwwU RpXM9CY/pZ0igfnNRD/G0LnYe5OVuQYog5DUT2H5dmm/6jEO/rSNyIt3DGY+5dQ= =FZrj -----END PGP SIGNATURE----- --oqA2ixtvKOS32UoFWsurd21lkkIfWKohh--