[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH v4 16/23] qemu-img: Drop redundant
From: |
John Snow |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH v4 16/23] qemu-img: Drop redundant error message in compare |
Date: |
Wed, 27 Sep 2017 17:35:12 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
On 09/13/2017 12:03 PM, Eric Blake wrote:
> If a read error is encountered during 'qemu-img compare', we
> were printing the "Error while reading offset ..." message twice.
> Update the testsuite for the improved output.
>
> Further simplify the code by hoisting the error code conversion
> into the helper function, rather than repeating it at the callers.
>
> Signed-off-by: Eric Blake <address@hidden>
>
> ---
> v3: new patch
> ---
> qemu-img.c | 19 +++++--------------
> tests/qemu-iotests/074.out | 2 --
> 2 files changed, 5 insertions(+), 16 deletions(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index dfccebe6bc..3e1e373e8f 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -1196,8 +1196,10 @@ static int64_t sectors_to_bytes(int64_t sectors)
> /*
> * Check if passed sectors are empty (not allocated or contain only 0 bytes)
> *
> - * Returns 0 in case sectors are filled with 0, 1 if sectors contain non-zero
> - * data and negative value on error.
> + * Intended for use by 'qemu-img compare': Returns 0 in case sectors are
> + * filled with 0, 1 if sectors contain non-zero data (this is a comparison
> + * failure), and 4 on error (the exit status for read errors), after emitting
> + * an error message.
> *
> * @param blk: BlockBackend for the image
> * @param sect_num: Number of first sector to check
> @@ -1218,7 +1220,7 @@ static int check_empty_sectors(BlockBackend *blk,
> int64_t sect_num,
> if (ret < 0) {
> error_report("Error while reading offset %" PRId64 " of %s: %s",
> sectors_to_bytes(sect_num), filename, strerror(-ret));
> - return ret;
> + return 4;
> }
> idx = find_nonzero(buffer, sect_count * BDRV_SECTOR_SIZE);
> if (idx >= 0) {
> @@ -1473,11 +1475,6 @@ static int img_compare(int argc, char **argv)
> filename2, buf1, quiet);
> }
> if (ret) {
> - if (ret < 0) {
> - error_report("Error while reading offset %" PRId64 ":
> %s",
> - sectors_to_bytes(sector_num),
> strerror(-ret));
> - ret = 4;
> - }
> goto out;
> }
> }
> @@ -1522,12 +1519,6 @@ static int img_compare(int argc, char **argv)
> ret = check_empty_sectors(blk_over, sector_num, nb_sectors,
> filename_over, buf1, quiet);
> if (ret) {
> - if (ret < 0) {
> - error_report("Error while reading offset %" PRId64
> - " of %s: %s",
> sectors_to_bytes(sector_num),
> - filename_over, strerror(-ret));
> - ret = 4;
> - }
> goto out;
> }
> }
> diff --git a/tests/qemu-iotests/074.out b/tests/qemu-iotests/074.out
> index 8fba5aea9c..ede66c3f81 100644
> --- a/tests/qemu-iotests/074.out
> +++ b/tests/qemu-iotests/074.out
> @@ -4,7 +4,6 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
> wrote 512/512 bytes at offset 512
> 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> qemu-img: Error while reading offset 0 of
> blkdebug:TEST_DIR/blkdebug.conf:TEST_DIR/t.IMGFMT: Input/output error
> -qemu-img: Error while reading offset 0: Input/output error
> 4
> Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
> Formatting 'TEST_DIR/t.IMGFMT.2', fmt=IMGFMT size=0
> @@ -12,7 +11,6 @@ Formatting 'TEST_DIR/t.IMGFMT.2', fmt=IMGFMT size=0
> wrote 512/512 bytes at offset 512
> 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> qemu-img: Error while reading offset 0 of
> blkdebug:TEST_DIR/blkdebug.conf:TEST_DIR/t.IMGFMT: Input/output error
> -qemu-img: Error while reading offset 0 of
> blkdebug:TEST_DIR/blkdebug.conf:TEST_DIR/t.IMGFMT: Input/output error
> Warning: Image size mismatch!
> 4
> Cleanup
>
Hm, naively I might assume it's best for the caller to report the error
and to leave the function a nicely self-contained helper, but I won't
insist on it.
Reviewed-by: John Snow <address@hidden>
- Re: [Qemu-block] [Qemu-devel] [PATCH v4 13/23] qemu-img: Simplify logic in img_compare(), (continued)
- [Qemu-block] [PATCH v4 15/23] qemu-img: Add find_nonzero(), Eric Blake, 2017/09/13
- [Qemu-block] [PATCH v4 16/23] qemu-img: Drop redundant error message in compare, Eric Blake, 2017/09/13
- Re: [Qemu-block] [Qemu-devel] [PATCH v4 16/23] qemu-img: Drop redundant error message in compare,
John Snow <=
- [Qemu-block] [PATCH v4 19/23] qemu-img: Change img_rebase() to be byte-based, Eric Blake, 2017/09/13
- [Qemu-block] [PATCH v4 18/23] qemu-img: Change compare_sectors() to be byte-based, Eric Blake, 2017/09/13
- [Qemu-block] [PATCH v4 17/23] qemu-img: Change check_empty_sectors() to byte-based, Eric Blake, 2017/09/13
- [Qemu-block] [PATCH v4 20/23] qemu-img: Change img_compare() to be byte-based, Eric Blake, 2017/09/13
- [Qemu-block] [PATCH v4 21/23] block: Align block status requests, Eric Blake, 2017/09/13