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 <ebl...@redhat.com>
>
> ---
> 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 <js...@redhat.com>
