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?
> Signed-off-by: Max Reitz <mre...@redhat.com>
> ---
> qemu-img.c | 21 ++++++++++++++-------
> 1 file changed, 14 insertions(+), 7 deletions(-)
>
> 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 = corruptions_fixed;
> }
>
> - 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;
> + }
> }
>
> 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?
> + if (ret) {
> + error_report("Check failed: %s", strerror(-ret));
> + } else {
> + error_report("Check failed");
> + }
> ret = 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)'.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
