[PATCH for-5.0?] qemu-img: Report convert errors by bytes, not sectors

[PATCH for-5.0?] qemu-img: Report convert errors by bytes, not sectors

[Eric Blake]

Various qemu-img commands are inconsistent on whether they report
status/errors in terms of bytes or sector offsets.  The latter is
confusing (especially as more places move to 4k block sizes), so let's
switch everything to just use bytes everywhere.

Signed-off-by: Eric Blake <eblake@redhat.com>
---

Noticed while investigating https://bugzilla.redhat.com/1819240

By the way, I found it odd that even without --salvage, qemu-img
convert will process up to 8 EIO failures (based on its default
coroutine depth of 8) before finally exiting, rather than quitting
immediately on the first EIO failure.

 qemu-img.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index b167376bd72e..77219e25b33b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1924,8 +1924,9 @@ retry:
         if (status == BLK_DATA && !copy_range) {
             ret = convert_co_read(s, sector_num, n, buf);
             if (ret < 0) {
-                error_report("error while reading sector %" PRId64
-                             ": %s", sector_num, strerror(-ret));
+                error_report("error while reading at byte %" PRId64
+                             ": %s", sector_num * BDRV_SECTOR_SIZE,
+                             strerror(-ret));
                 s->ret = ret;
             }
         } else if (!s->min_sparse && status == BLK_ZERO) {
@@ -1953,8 +1954,9 @@ retry:
                 ret = convert_co_write(s, sector_num, n, buf, status);
             }
             if (ret < 0) {
-                error_report("error while writing sector %" PRId64
-                             ": %s", sector_num, strerror(-ret));
+                error_report("error while writing at byte %" PRId64
+                             ": %s", sector_num * BDRV_SECTOR_SIZE,
+                             strerror(-ret));
                 s->ret = ret;
             }
         }
-- 
2.26.0.rc2

Re: [PATCH for-5.0?] qemu-img: Report convert errors by bytes, not sectors

[Vladimir Sementsov-Ogievskiy]

01.04.2020 1:24, Eric Blake wrote:
> Various qemu-img commands are inconsistent on whether they report
> status/errors in terms of bytes or sector offsets.  The latter is
> confusing (especially as more places move to 4k block sizes), so let's
> switch everything to just use bytes everywhere.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

> ---
> 
> Noticed while investigating https://bugzilla.redhat.com/1819240
> 
> By the way, I found it odd that even without --salvage, qemu-img
> convert will process up to 8 EIO failures (based on its default
> coroutine depth of 8) before finally exiting, rather than quitting
> immediately on the first EIO failure.
> 
>   qemu-img.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index b167376bd72e..77219e25b33b 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -1924,8 +1924,9 @@ retry:
>           if (status == BLK_DATA && !copy_range) {
>               ret = convert_co_read(s, sector_num, n, buf);
>               if (ret < 0) {
> -                error_report("error while reading sector %" PRId64
> -                             ": %s", sector_num, strerror(-ret));
> +                error_report("error while reading at byte %" PRId64
> +                             ": %s", sector_num * BDRV_SECTOR_SIZE,
> +                             strerror(-ret));
>                   s->ret = ret;
>               }
>           } else if (!s->min_sparse && status == BLK_ZERO) {
> @@ -1953,8 +1954,9 @@ retry:
>                   ret = convert_co_write(s, sector_num, n, buf, status);
>               }
>               if (ret < 0) {
> -                error_report("error while writing sector %" PRId64
> -                             ": %s", sector_num, strerror(-ret));
> +                error_report("error while writing at byte %" PRId64
> +                             ": %s", sector_num * BDRV_SECTOR_SIZE,
> +                             strerror(-ret));
>                   s->ret = ret;
>               }
>           }
> 


-- 
Best regards,
Vladimir

Re: [PATCH for-5.0?] qemu-img: Report convert errors by bytes, not sectors

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200331222442.273158-1-eblake@redhat.com/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

  BUILD   pc-bios/optionrom/pvh.img
  BUILD   pc-bios/optionrom/pvh.raw
  SIGN    pc-bios/optionrom/pvh.bin
/tmp/qemu-test/src/qemu-img.c:1928:38: error: format specifies type 'long' but the argument has type 'unsigned long long' [-Werror,-Wformat]
                             ": %s", sector_num * BDRV_SECTOR_SIZE,
                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/qemu-test/src/qemu-img.c:1958:38: error: format specifies type 'long' but the argument has type 'unsigned long long' [-Werror,-Wformat]
                             ": %s", sector_num * BDRV_SECTOR_SIZE,
                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2 errors generated.
make: *** [/tmp/qemu-test/src/rules.mak:69: qemu-img.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 664, in <module>
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=f02081d59b2a43498576aa8b629d2330', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=x86_64-softmmu', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-j_hd3wr5/src/docker-src.2020-04-01-04.29.19.31174:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-debug']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=f02081d59b2a43498576aa8b629d2330
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-j_hd3wr5/src'
make: *** [docker-run-test-debug@fedora] Error 2

real    4m12.821s
user    0m8.194s


The full log is available at
http://patchew.org/logs/20200331222442.273158-1-eblake@redhat.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH for-5.0?] qemu-img: Report convert errors by bytes, not sectors

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200331222442.273158-1-eblake@redhat.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  BUILD   pc-bios/optionrom/pvh.raw
  SIGN    pc-bios/optionrom/pvh.bin
/tmp/qemu-test/src/qemu-img.c: In function 'convert_co_do_copy':
/tmp/qemu-test/src/qemu-img.c:1929:30: error: format '%ld' expects argument of type 'long int', but argument 2 has type 'long long unsigned int' [-Werror=format=]
                              strerror(-ret));
                              ^
/tmp/qemu-test/src/qemu-img.c:1959:30: error: format '%ld' expects argument of type 'long int', but argument 2 has type 'long long unsigned int' [-Werror=format=]
                              strerror(-ret));
                              ^
cc1: all warnings being treated as errors
make: *** [qemu-img.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 664, in <module>
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=011cc2d621b8449f8f588a13e3801be0', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-4l3gh6qb/src/docker-src.2020-04-01-04.35.04.4990:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=011cc2d621b8449f8f588a13e3801be0
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-4l3gh6qb/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    1m59.298s
user    0m8.025s


The full log is available at
http://patchew.org/logs/20200331222442.273158-1-eblake@redhat.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH for-5.0?] qemu-img: Report convert errors by bytes, not sectors

[Eric Blake]

On 4/1/20 3:33 AM, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20200331222442.273158-1-eblake@redhat.com/
> 
> 
> 
> Hi,
> 
> This series failed the asan build test. Please find the testing commands and
> their output below. If you have Docker installed, you can probably reproduce it
> locally.
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> export ARCH=x86_64
> make docker-image-fedora V=1 NETWORK=1
> time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
> === TEST SCRIPT END ===
> 
>    BUILD   pc-bios/optionrom/pvh.img
>    BUILD   pc-bios/optionrom/pvh.raw
>    SIGN    pc-bios/optionrom/pvh.bin
> /tmp/qemu-test/src/qemu-img.c:1928:38: error: format specifies type 'long' but the argument has type 'unsigned long long' [-Werror,-Wformat]
>                               ": %s", sector_num * BDRV_SECTOR_SIZE,
>                                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Yep.  I'll have to post v2.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org