From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49903) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZZPPg-000222-0N for qemu-devel@nongnu.org; Tue, 08 Sep 2015 16:22:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZZPPe-0007rG-Hu for qemu-devel@nongnu.org; Tue, 08 Sep 2015 16:22:43 -0400 References: <1441742995-11794-1-git-send-email-mreitz@redhat.com> <1441742995-11794-2-git-send-email-mreitz@redhat.com> From: Eric Blake Message-ID: <55EF4387.9060503@redhat.com> Date: Tue, 8 Sep 2015 14:22:31 -0600 MIME-Version: 1.0 In-Reply-To: <1441742995-11794-2-git-send-email-mreitz@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="O9Aa5Ta3VGiE0exnGvhMU29pQ7oVArpx0" Subject: Re: [Qemu-devel] [PATCH 1/2] qcow2: Make size_to_clusters() return int64_t List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz , qemu-block@nongnu.org Cc: Kevin Wolf , qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --O9Aa5Ta3VGiE0exnGvhMU29pQ7oVArpx0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 09/08/2015 02:09 PM, Max Reitz wrote: > Sadly, some images may have more clusters than what can be represented > using a plain int. We should be prepared for that case (in > qcow2_check_refcounts() we actually were trying to catch that case, but= > since size_to_clusters() truncated the returned value, that check never= > did anything useful). >=20 > Signed-off-by: Max Reitz > --- > block/qcow2-cluster.c | 20 +++++++++++--------- > block/qcow2.h | 2 +- > 2 files changed, 12 insertions(+), 10 deletions(-) >=20 > diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c > index 2975b83..a34f0b1 100644 > --- a/block/qcow2-cluster.c > +++ b/block/qcow2-cluster.c > @@ -473,8 +473,8 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, = uint64_t offset, > unsigned int l2_index; > uint64_t l1_index, l2_offset, *l2_table; > int l1_bits, c; > - unsigned int index_in_cluster, nb_clusters; > - uint64_t nb_available, nb_needed; > + unsigned int index_in_cluster; > + uint64_t nb_available, nb_needed, nb_clusters; Most uses are storing the results unsigned... > =20 > -static inline int size_to_clusters(BDRVQcow2State *s, int64_t size) > +static inline int64_t size_to_clusters(BDRVQcow2State *s, int64_t size= ) > { > return (size + (s->cluster_size - 1)) >> s->cluster_bits; > } =2E..and the function itself doesn't appear to intentionally return negative (unless size was passed in as negative, but then that may be accidental). Should it just return uint64_t instead? At any rate, I agree that 'int' is too small, so: Reviewed-by: Eric Blake --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --O9Aa5Ta3VGiE0exnGvhMU29pQ7oVArpx0 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/ iQEcBAEBCAAGBQJV70OHAAoJEKeha0olJ0NqIAwH/jiEB/qdZy/euZWGmP9YZoJY 7Rb4k9wM6oBpGRPa1H3Uxe/osObwKa9yENkHDaEItKBfDb7aQuHnvVxnY9LJ+UUH +PlG3iTbNgTplGk0LXjVX2HaK32oZz3uh3y1aNrFW+MX9E/Mfx1cL2MrEe8p9Lu7 3GgABz0kx9Sld4ee9hHZdEUeyFBl2J7gJfOWKlCZ6vfMVmQIIUhBHt9aZ0uwkJPO /G99R6kEfxS0iAq+QrPsZZDq+2kKzACruePHFBOwXNupeQGK7SyV6hM9wbdPpPxS HfYHn9VTeoVrntIzfpI9cgz+62X0zucc17gmXxxsu0iT29GYzzVOupnPRa/jh/k= =oK/h -----END PGP SIGNATURE----- --O9Aa5Ta3VGiE0exnGvhMU29pQ7oVArpx0--