From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35016) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XhJ3k-0002HT-Tt for qemu-devel@nongnu.org; Thu, 23 Oct 2014 10:08:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XhJ3f-0004Qm-Sh for qemu-devel@nongnu.org; Thu, 23 Oct 2014 10:08:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:1900) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XhJ3f-0004Qc-LN for qemu-devel@nongnu.org; Thu, 23 Oct 2014 10:08:07 -0400 Message-ID: <54490BBD.8010006@redhat.com> Date: Thu, 23 Oct 2014 08:07:57 -0600 From: Eric Blake MIME-Version: 1.0 References: <1414070952-25865-1-git-send-email-mreitz@redhat.com> <544907F9.8010302@redhat.com> <544909A6.2020204@redhat.com> In-Reply-To: <544909A6.2020204@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="3pxWRPwtftFN2IREW6H3xD9GKVhufWwt2" 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) --3pxWRPwtftFN2IREW6H3xD9GKVhufWwt2 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 10/23/2014 07:59 AM, Max Reitz wrote: > On 2014-10-23 at 15:51, Eric Blake wrote: >> 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 = the >>> user that there were no errors on the image. This is bad. >>> >>> Instead of printing the check result if there were internal errors wh= ich >>> were so bad that bdrv_check() could not even complete with 0 as a ret= urn >>> value, qemu-img check should inform the user about the error. >>> >> Is there a way to exercise this in the testsuite? >=20 > It would involve some blkdebug things which try to break the qcow2 chec= k > function. I wouldn't rely on it, because this rather exercises the qcow= 2 > check function than this patch. Fair enough. >>> + case OFORMAT_JSON: >>> + dump_json_image_check(check, quiet); >>> + break; >>> + } >>> } >>> 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? >=20 > Yes, I think we can. I interpreted that as "Test completed, but there > were errors". The dump should not be incorrect, because if it was, the > check function should not have returned 0. >=20 > Therefore, I think we should dump the test result because by returning = 0 > the check function says it's valid. If there were check_errors, the dum= p > function will show their number, too. Furthermore, if 'quiet' is true, then the dump_* call probably output nothing, but the fact that we want to return non-zero to flag that check detected problems is worth being noisy about. If I understand correctly, 'quiet' is only about suppressing stdout, not stderr, and always being noisy on stderr for non-zero exit is a good idea. Okay, I think your answers convinced me, and I'm comfortable enough with the patch as-is to give: Reviewed-by: Eric Blake --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --3pxWRPwtftFN2IREW6H3xD9GKVhufWwt2 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 iQEcBAEBCAAGBQJUSQu9AAoJEKeha0olJ0NqQyIIAI0pDQ3dZnDOwPvnyWzCMCYe eQ6Co2kETDJORarFzf0ry2JeJtPwRFBMVW5eLrbNJIgoOVfBZ0JK5PIYSSAL6Xd1 I7X1md95HKRjbI7G9M4xQI9Scec50E83S+j1Ze6WWvHc1nVBQ1V9iGaetcaKVckf 4uc6wUNdy8tEAMrrApfonAnhKGodF1/haDlyXXkPbJS5SyjKu7RimsrFTJc0VVJh XB21Jlj7FsVeT2TDrASB3jLzNN6n/lmmeU09tn63snSGQt1+8jql59vkAXtjz/Xb TBne2PDYEMFsysg5+FKVoYEnsGHOsIKD4PxPeSNlD/0wqD35ZdFbW7c+29c+y34= =NOwn -----END PGP SIGNATURE----- --3pxWRPwtftFN2IREW6H3xD9GKVhufWwt2--