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 which >>> were so bad that bdrv_check() could not even complete with 0 as a return >>> value, qemu-img check should inform the user about the error. >>> >> Is there a way to exercise this in the testsuite? > > It would involve some blkdebug things which try to break the qcow2 check > function. I wouldn't rely on it, because this rather exercises the qcow2 > 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 == 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?
>
> 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.
>
> 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 dump
> 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 <ebl...@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
