[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] qemu-img: Print error if check failed
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH] qemu-img: Print error if check failed |
Date: |
Thu, 23 Oct 2014 07:51:53 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 |
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 <address@hidden>
> ---
> 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