From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44104) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YOaJm-0005Yq-FK for qemu-devel@nongnu.org; Thu, 19 Feb 2015 18:15:39 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YOaJh-0000lW-Ej for qemu-devel@nongnu.org; Thu, 19 Feb 2015 18:15:38 -0500 Received: from mx1.redhat.com ([209.132.183.28]:48451) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YOaJh-0000lE-78 for qemu-devel@nongnu.org; Thu, 19 Feb 2015 18:15:33 -0500 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t1JNFUBo029279 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Thu, 19 Feb 2015 18:15:31 -0500 Message-ID: <54E66E91.2070705@redhat.com> Date: Thu, 19 Feb 2015 16:15:29 -0700 From: Eric Blake MIME-Version: 1.0 References: <1424299260-1763-1-git-send-email-mreitz@redhat.com> <1424299260-1763-2-git-send-email-mreitz@redhat.com> <54E54F99.4030301@redhat.com> <54E5ECCA.7080807@redhat.com> In-Reply-To: <54E5ECCA.7080807@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="rigwAKxa1RoiNRl6PIhNNTXrt3nbfGNOa" Subject: Re: [Qemu-devel] [PATCH v7 11/25] qcow2: refcount_order parameter for qcow2_create2 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz , qemu-devel@nongnu.org Cc: Kevin Wolf , Stefan Hajnoczi This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --rigwAKxa1RoiNRl6PIhNNTXrt3nbfGNOa Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 02/19/2015 07:01 AM, Max Reitz wrote: >>> @@ -2010,6 +2022,8 @@ static int qcow2_create(const char *filename, >>> QemuOpts *opts, Error **errp) >>> size_t cluster_size =3D DEFAULT_CLUSTER_SIZE; >>> PreallocMode prealloc; >>> int version =3D 3; >>> + uint64_t refcount_bits =3D 16; >>> + int refcount_order; >>> Error *local_err =3D NULL; >>> + refcount_order =3D ffs(refcount_bits) - 1; >> ffs() doesn't work on uint64_t (it gives the wrong answer for >> 0x100000000, for example); you want to use ffsll(). But ffsll() isn't= >> portable. But we have include/qemu/host-utils.h that gives us ctz64()= >> which is what we want (where ctz=3D=3Dffs-1 other than for the special= case >> of 0). So this should be: >> >> refcount_order =3D ctz64(refcount_bits); >> >> With that change, >> Reviewed-by: Eric Blake >=20 > I intentionally left the ffs() here, because after this patch, > refcount_bits is guaranteed to be 16, and after patch 14, it's > guaranteed to be 64 or less (it's more obvious after patch 14 than > here). So I would be fine with ctz64(), but in my opinion, ffs() is jus= t > fine, too. Ah, you are right - here, we are calling ffs(constant), and in patch 14, the ffs() is quite obviously bounded by a range check, so what you have works, even if it is not as short as possible, and even though it caused me to think about integer wrapping. I'll leave it up to Kevin whether to switch to ctz64() or stick with ffs(= ). >=20 > Max >=20 >> (Hmm, that means that at least qcow2.c:qcow2_create2() has a bug for >> calling ffs(size_t), and we probably ought to eradicate other uses of >> ffs from the tree - but as a separate followup). This still remains true - we should scrub all uses of ffs() on size_t or uint64_t and make sure they can't suffer from wraparound issues, but doing that shouldn't hold up this series. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --rigwAKxa1RoiNRl6PIhNNTXrt3nbfGNOa 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 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJU5m6RAAoJEKeha0olJ0NqKOAIAKE2r8ncLO6Sey/gWWQcQxPr 5G1E8tgKWBfrZ+2yIzrSZ470KI/B2QS6eCaq6/fCm3fxpCGraateHYzBssnwm+4z QaxIka4khlIZrerF568DuvtWBsdTlx0VJ5rG3Rgq+0cuKnpSGRoqzXD+MtOQSfDc xwtUsEQIwjCF7yWM7RctYYnoClEg+WgduUvty/t+QR2RVHz0tWhOfF+pLRps+yR9 CXqZnlwLFm+fpWrnC3PPiBiusn7ELFyugJkKHpU+dFzR/KsG3I4AOxE8NPjjs8Lq UQSA2ivrIfQ9Aa7Wve0mlXv+y2tFiE+Z2Zj5Hj544Te6xnZcx9VnusWYcMcYas4= =1YFf -----END PGP SIGNATURE----- --rigwAKxa1RoiNRl6PIhNNTXrt3nbfGNOa--