From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50621) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XwCWq-0004wI-2F for qemu-devel@nongnu.org; Wed, 03 Dec 2014 11:11:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XwCWl-0005w2-4M for qemu-devel@nongnu.org; Wed, 03 Dec 2014 11:11:47 -0500 Received: from mx1.redhat.com ([209.132.183.28]:39379) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XwCWk-0005vy-SE for qemu-devel@nongnu.org; Wed, 03 Dec 2014 11:11:43 -0500 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sB3GBeuZ031548 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Wed, 3 Dec 2014 11:11:41 -0500 Message-ID: <547F363B.6040600@redhat.com> Date: Wed, 03 Dec 2014 09:11:39 -0700 From: Eric Blake MIME-Version: 1.0 References: <1417613866-25890-1-git-send-email-mreitz@redhat.com> <1417613866-25890-7-git-send-email-mreitz@redhat.com> In-Reply-To: <1417613866-25890-7-git-send-email-mreitz@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="GaPcVPek1D0SX1HX03ixmORi3J6b62uV5" Subject: Re: [Qemu-devel] [PATCH v4 06/26] qcow2: Use 64 bits for refcount values 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) --GaPcVPek1D0SX1HX03ixmORi3J6b62uV5 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 12/03/2014 06:37 AM, Max Reitz wrote: > Refcounts may have a width of up to 64 bits, so qemu should use the sam= e > width to represent refcount values internally. >=20 > Signed-off-by: Max Reitz > --- > block/qcow2-cluster.c | 2 +- > block/qcow2-refcount.c | 42 ++++++++++++++++++++----------------------= > block/qcow2.h | 4 ++-- > 3 files changed, 23 insertions(+), 25 deletions(-) >=20 > @@ -1698,7 +1696,7 @@ static void compare_refcounts(BlockDriverState *b= s, BdrvCheckResult *res, > refcount2 - refcount1, > refcount1 > refcount2, > QCOW2_DISCARD_ALWAYS); > - if (ret >=3D 0) { > + if (ret =3D=3D 0) { > (*num_fixed)++; > continue; > } This hunk looks a bit misplaced (it is unrelated to a sizing change). Patch 5 altered update_refcount, but does not document its return value. But a careful step through update_refcount() shows that all error paths return negative, and the only places where we do things like 'ret =3D alloc_refcount_block(...)" are later followed by an unconditional 'ret =3D= 0' on success paths (so I didn't check whether alloc_refcount_block returns positive values, but didn't need to). If you have to respin, it might be better to add documentation of the return value and update callers to assume a non-positive return as a separate patch, rather than sliding it in with this one. But as I don't see any code faults, and am not sure it is worth a respin just for this issue, Reviewed-by: Eric Blake --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --GaPcVPek1D0SX1HX03ixmORi3J6b62uV5 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 iQEcBAEBCAAGBQJUfzY7AAoJEKeha0olJ0Nqh50IAJO83g/D4QOS4Rv+AFVNi+7j s4PsJEGMP9/mifHE93CEujj1+qoqaBO+2rcm9jQ/dx1Bw7FuayNP99kqpvzjwGCk ZHJCmUlGmrvf0TP64Af4zKsos5AbTKQtmdO453eNd6FZADiIEqKNNiIMe/SS2ksd sW+MHywR1FVfavk178EmjthJbVsu1TVLmC5wh6zARVNlxg8RX40OHkxFX/8cChw1 9gzi/q//EMWw3Zf/iIdEDtTVICSV8fu+uPprlqy27/IBM2IFptbclkmHsHZmrUbv Op1DPUQIyF00CTb/0zTY498IVBg4924g8TRJAbjuFy11dkvjwEtu0wimoxl+OTQ= =AfBG -----END PGP SIGNATURE----- --GaPcVPek1D0SX1HX03ixmORi3J6b62uV5--