From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59006) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XhIoD-0007mW-0l for qemu-devel@nongnu.org; Thu, 23 Oct 2014 09:52:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XhIo8-0006Z1-3K for qemu-devel@nongnu.org; Thu, 23 Oct 2014 09:52:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:23467) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XhIo7-0006X1-Sh for qemu-devel@nongnu.org; Thu, 23 Oct 2014 09:52:04 -0400 Message-ID: <544907F9.8010302@redhat.com> Date: Thu, 23 Oct 2014 07:51:53 -0600 From: Eric Blake MIME-Version: 1.0 References: <1414070952-25865-1-git-send-email-mreitz@redhat.com> In-Reply-To: <1414070952-25865-1-git-send-email-mreitz@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="mom62bhmPtHwWNNLtwl4apRKQV5AsGXiH" Subject: Re: [Qemu-devel] [PATCH] qemu-img: Print error if check failed List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz , qemu-devel@nongnu.org Cc: Kevin Wolf , Peter Lieven , =?UTF-8?B?QmVub8OudCBDYW5ldA==?= , Stefan Hajnoczi This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --mom62bhmPtHwWNNLtwl4apRKQV5AsGXiH Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 10/23/2014 07:29 AM, Max Reitz wrote: > Currently, if bdrv_check() fails either by returning -errno or having > check_errors set, qemu-img check just exits with 1 after having told th= e > user that there were no errors on the image. This is bad. >=20 > Instead of printing the check result if there were internal errors whic= h > were so bad that bdrv_check() could not even complete with 0 as a retur= n > value, qemu-img check should inform the user about the error. >=20 Is there a way to exercise this in the testsuite? > Signed-off-by: Max Reitz > --- > qemu-img.c | 21 ++++++++++++++------- > 1 file changed, 14 insertions(+), 7 deletions(-) >=20 > diff --git a/qemu-img.c b/qemu-img.c > index 09e7e72..731502c 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -687,16 +687,23 @@ static int img_check(int argc, char **argv) > check->corruptions_fixed =3D corruptions_fixed; > } > =20 > - switch (output_format) { > - case OFORMAT_HUMAN: > - dump_human_image_check(check, quiet); > - break; > - case OFORMAT_JSON: > - dump_json_image_check(check, quiet); > - break; > + if (!ret) { > + switch (output_format) { > + case OFORMAT_HUMAN: > + dump_human_image_check(check, quiet); > + break; > + case OFORMAT_JSON: > + dump_json_image_check(check, quiet); > + break; > + } > } > =20 > if (ret || check->check_errors) { Can we ever have ret =3D=3D 0 (so we attempted dump_*_image_check) AND check->check_errors? Will that be confusing output, to have both a (probably incorrect) dump on stdout and an error message on stderr? > + if (ret) { > + error_report("Check failed: %s", strerror(-ret)); > + } else { > + error_report("Check failed"); > + } > ret =3D 1; > goto fail; > } Or rephrasing the question, would it be better to hoist this chunk to occur before the switch (output_format)? And if so, then you don't need to reindent that code inside 'if (!ret)'. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --mom62bhmPtHwWNNLtwl4apRKQV5AsGXiH 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 iQEcBAEBCAAGBQJUSQf5AAoJEKeha0olJ0Nqe18H/1zd54FsuOnvcA4og/+WMfeY gv7X6CFSlOELDYL8Tfuw8IHT6A81tm5PguZT5WrUkX23PlZX7uLABRf7y5wemzD9 +GgaVkmY+ZfeGJOX3hX4n/wembVO1a4eVmKMMUFCyazzoDhKFDLteq7l3nVQ+p6Z 5ouadqBi91EQsOMgs1h0VntcOqbYfDwnlR4QVHzUS4sVDcT0abAAX/X4L/ZzrcLH 8BWjDF+jf57gFokqsdFDmJXZkvzCnQ2p78tsUlTtxfqEXGdSz4us2xugi9CUX37a /+Y+WHHtgSEahabBqY4aIUhfzngMPS2/i6Z9ltsQmh00Htu04UgLSFxSqoznOyM= =Ica3 -----END PGP SIGNATURE----- --mom62bhmPtHwWNNLtwl4apRKQV5AsGXiH--