qemu-img.c | 76 +++++++++++++++++--------------------- tests/qemu-iotests/049.out | 8 ++-- 2 files changed, 38 insertions(+), 46 deletions(-)
Hi, The following series adds two parameters to qemu-img map: 1. start-offset: mapping starting offset. 2. max-length: the length of the mapping. These parameters proved useful when mapping large disk spread across long store file chains. It allows us to bound the execution time of each qemu-img map execution as well as recover from failed mapping operations. In addition the map operation can divided to multiple independent tasks. V3 changes: 1. Add cvtnum_full and made cvtnum a wrapper function. 2. Keep the original boundaries checks. 3. Tone down error messages. V2 changes: 1. Add error reporting to cvtnum. 2. Add image length validation in img_map. 3. Rebase over QEMU 5.0. Eyal Moscovici (1): qemu_img: add cvtnum_full to print error reports qemu-img.c | 76 +++++++++++++++++--------------------- tests/qemu-iotests/049.out | 8 ++-- 2 files changed, 38 insertions(+), 46 deletions(-) -- 2.17.2 (Apple Git-113)
On 5/13/20 8:36 AM, Eyal Moscovici wrote: > Hi, > > The following series adds two parameters to qemu-img map: > 1. start-offset: mapping starting offset. > 2. max-length: the length of the mapping. > > These parameters proved useful when mapping large disk spread across > long store file chains. It allows us to bound the execution time of each > qemu-img map execution as well as recover from failed mapping > operations. In addition the map operation can divided to > multiple independent tasks. > > V3 changes: > 1. Add cvtnum_full and made cvtnum a wrapper function. > 2. Keep the original boundaries checks. > 3. Tone down error messages. While this does not directly touch NBD code, I find it quite handy for my tests of incremental backups over NBD (since I frequently use x-dirty-bitmap coupled with qemu-img map to read bitmaps, and subsetting the output is indeed nice), so I'll queue this through my NBD tree. It may be another week or so before I send a pull request including this and other collected patches. Congratulations on your first qemu contribution! > qemu-img.c | 76 +++++++++++++++++--------------------- > tests/qemu-iotests/049.out | 8 ++-- > 2 files changed, 38 insertions(+), 46 deletions(-) This series diffstat is off; later in the series, in 4/4, I see: > docs/tools/qemu-img.rst | 2 +- > qemu-img-cmds.hx | 4 ++-- > qemu-img.c | 22 +++++++++++++++++++++- > 3 files changed, 24 insertions(+), 4 deletions(-) What I don't see is any iotest coverage of the new options, to ensure they don't regress. Either a new iotest, or an enhancement to an existing iotest. If you feel up to the task, post a 5/4 patch; if not, I'll probably enhance 223 (my x-dirty-bitmap reading code mentioned above). -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
On 13/05/2020 20:49, Eric Blake wrote: > On 5/13/20 8:36 AM, Eyal Moscovici wrote: >> Hi, >> >> The following series adds two parameters to qemu-img map: >> 1. start-offset: mapping starting offset. >> 2. max-length: the length of the mapping. >> >> These parameters proved useful when mapping large disk spread across >> long store file chains. It allows us to bound the execution time of each >> qemu-img map execution as well as recover from failed mapping >> operations. In addition the map operation can divided to >> multiple independent tasks. >> >> V3 changes: >> 1. Add cvtnum_full and made cvtnum a wrapper function. >> 2. Keep the original boundaries checks. >> 3. Tone down error messages. > > While this does not directly touch NBD code, I find it quite handy for > my tests of incremental backups over NBD (since I frequently use > x-dirty-bitmap coupled with qemu-img map to read bitmaps, and > subsetting the output is indeed nice), so I'll queue this through my > NBD tree. It may be another week or so before I send a pull request > including this and other collected patches. > > Congratulations on your first qemu contribution! Thanks :) > >> qemu-img.c | 76 +++++++++++++++++--------------------- >> tests/qemu-iotests/049.out | 8 ++-- >> 2 files changed, 38 insertions(+), 46 deletions(-) > > This series diffstat is off; later in the series, in 4/4, I see: I had some copy & paste issues with my cover letter, sorry. > >> docs/tools/qemu-img.rst | 2 +- >> qemu-img-cmds.hx | 4 ++-- >> qemu-img.c | 22 +++++++++++++++++++++- >> 3 files changed, 24 insertions(+), 4 deletions(-) > > What I don't see is any iotest coverage of the new options, to ensure > they don't regress. Either a new iotest, or an enhancement to an > existing iotest. If you feel up to the task, post a 5/4 patch; if > not, I'll probably enhance 223 (my x-dirty-bitmap reading code > mentioned above). > I will take a look at test 223, and see if I can enhance it.
Patchew URL: https://patchew.org/QEMU/20200513133629.18508-1-eyal.moscovici@oracle.com/
Hi,
This series failed the docker-mingw@fedora 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-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===
CC x86_64-softmmu/ioport.o
CC x86_64-softmmu/qtest.o
/tmp/qemu-test/src/qemu-img.c: In function 'cvtnum_full':
/tmp/qemu-test/src/qemu-img.c:488:63: error: format '%ld' expects argument of type 'long int', but argument 3 has type 'int64_t' {aka 'long long int'} [-Werror=format=]
error_report("Invalid %s specified. Must be between %ld bytes "
~~^
%lld
"to %ld bytes.", name, min, max);
~~~
/tmp/qemu-test/src/qemu-img.c:488:22: error: format '%ld' expects argument of type 'long int', but argument 4 has type 'int64_t' {aka 'long long int'} [-Werror=format=]
error_report("Invalid %s specified. Must be between %ld bytes "
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
"to %ld bytes.", name, min, max);
---
~~^
%lld
cc1: all warnings being treated as errors
make: *** [/tmp/qemu-test/src/rules.mak:69: qemu-img.o] Error 1
make: *** Waiting for unfinished jobs....
CC aarch64-softmmu/dump/dump.o
CC x86_64-softmmu/memory.o
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=33f83b7d61224ad79355fee02312ee9c', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-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-z4fvcbiq/src/docker-src.2020-05-13-18.10.43.324:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=33f83b7d61224ad79355fee02312ee9c
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-z4fvcbiq/src'
make: *** [docker-run-test-mingw@fedora] Error 2
real 2m36.638s
user 0m7.720s
The full log is available at
http://patchew.org/logs/20200513133629.18508-1-eyal.moscovici@oracle.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
On 5/13/20 5:13 PM, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20200513133629.18508-1-eyal.moscovici@oracle.com/
>
>
>
> Hi,
>
> This series failed the docker-mingw@fedora 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-mingw@fedora J=14 NETWORK=1
> === TEST SCRIPT END ===
>
> CC x86_64-softmmu/ioport.o
> CC x86_64-softmmu/qtest.o
> /tmp/qemu-test/src/qemu-img.c: In function 'cvtnum_full':
> /tmp/qemu-test/src/qemu-img.c:488:63: error: format '%ld' expects argument of type 'long int', but argument 3 has type 'int64_t' {aka 'long long int'} [-Werror=format=]
> error_report("Invalid %s specified. Must be between %ld bytes "
> ~~^
> %lld
> "to %ld bytes.", name, min, max);
patchew is correct; printing int64_t values requires "%"PRId64 rather
than "%ld". I'm fine with touching that up in my queue, unless you'd
like to submit v4.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
© 2016 - 2024 Red Hat, Inc.