qemu-img convert: Fix progress output
cover qemu-img convert: Fix progress output
Kevin Wolf (2):
qemu-img convert: Fix progress output
qemu-iotests: Test progress output for conversion
qemu-img.c | 20 +++++++------- tests/qemu-iotests/086 | 65 ++++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/086.out | 18 +++++++++++++ tests/qemu-iotests/group | 1 + 4 files changed, 95 insertions(+), 9 deletions(-) create mode 100755 tests/qemu-iotests/086 create mode 100644 tests/qemu-iotests/086.out
Signature · 1 line
1.8.1.4
PATCH 1/2 qemu-img convert: Fix progress output
Initialise progress output only when the -p and -q options have already been parsed, otherwise it's always disabled.
--- qemu-img.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
qemu-img.c +11 -9
@@ -1168,9 +1168,6 @@ static int img_convert(int argc, char **argv)Error *local_err =NULL ;QemuOpts *sn_opts =NULL ; -/* Initialize before goto out */ -qemu_progress_init (progress,1.0 ); - fmt =NULL ; out_fmt ="raw" ; cache ="unsafe" ;
@@ -1203,17 +1200,17 @@ static int img_convert(int argc, char **argv)error_report ("option -e is deprecated, please use \'-o " "encryption\' instead!" ); ret =-1 ; - goto out; + goto fail_getopt;case '6' :error_report ("option -6 is deprecated, please use \'-o " "compat6\' instead!" ); ret =-1 ; - goto out; + goto fail_getopt;case 'o' :if (!is_valid_option_list (optarg)) {error_report ("Invalid option list: %s" , optarg); ret =-1 ; - goto out; + goto fail_getopt; }if (!options) { options =g_strdup (optarg);
@@ -1233,7 +1230,7 @@ static int img_convert(int argc, char **argv)error_report ("Failed in parsing snapshot param '%s'" , optarg); ret =-1 ; - goto out; + goto fail_getopt; } }else { snapshot_name = optarg;
@@ -1247,7 +1244,7 @@ static int img_convert(int argc, char **argv)if (sval <0 || *end) {error_report ("Invalid minimum zero buffer size for sparse output specified" ); ret =-1 ; - goto out; + goto fail_getopt; } min_sparse = sval /BDRV_SECTOR_SIZE ;
@@ -1268,9 +1265,12 @@ static int img_convert(int argc, char **argv) } } +/* Initialize before goto out */ if (quiet) { progress =0 ; } +qemu_progress_init (progress,1.0 ); + bs_n = argc - optind -1 ; out_filename = bs_n >=1 ? argv[argc -1 ] :NULL ;
@@ -1673,7 +1673,6 @@ out:free_option_parameters (create_options);free_option_parameters (param);qemu_vfree (buf); -g_free (options);if (sn_opts) {qemu_opts_del (sn_opts); }
@@ -1688,6 +1687,9 @@ out: }g_free (bs); } +fail_getopt: +g_free (options); +if (ret) {return 1 ; }
Signature · 1 line
1.8.1.4
Quoted 8 lines · Kevin Wolf
Initialise progress output only when the -p and -q options have already been parsed, otherwise it's always disabled.
Reported-by Peter Lieven <[email protected]>Signed-off-by Kevin Wolf <[email protected]>--- qemu-img.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
and apologies for missing that the previous patch broke things.
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
PATCH 2/2 qemu-iotests: Test progress output for conversion
In reply to Kevin Wolf--- tests/qemu-iotests/086 | 65 ++++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/086.out | 18 +++++++++++++ tests/qemu-iotests/group | 1 + 3 files changed, 84 insertions(+) create mode 100755 tests/qemu-iotests/086 create mode 100644 tests/qemu-iotests/086.out
tests/qemu-iotests/086 new file +65 -0
@@ -0,0 +1,65 @@ +#!/bin/bash +# +# Test qemu-img progress output +# +# Copyright (C) 2014 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. +# + +# creator [email protected] + +seq=`basename $0` +echo "QA output created by $seq" + +here=`pwd` +tmp=/tmp/$$ +status=1 # failure is the default! + +_cleanup() +{ + _cleanup_test_img +} +trap "_cleanup; exit \$status" 0 1 2 3 15 + +# get standard environment, filters and checks +. ./common.rc +. ./common.filter + +_supported_fmt qcow2 +_supported_proto file +_supported_os Linux + +function run_qemu_img() +{ + echo + echo Testing: "$@" | _filter_testdir +} + +size=128M + +_make_test_img $size +$QEMU_IO -c 'write 0 1M' $TEST_IMG | _filter_qemu_io +$QEMU_IO -c 'write 2M 1M' $TEST_IMG | _filter_qemu_io +$QEMU_IO -c 'write 4M 1M' $TEST_IMG | _filter_qemu_io +$QEMU_IO -c 'write 32M 1M' $TEST_IMG | _filter_qemu_io + +$QEMU_IMG convert -p -O $IMGFMT -f $IMGFMT "$TEST_IMG" "$TEST_IMG".base 2>&1 |\ + _filter_testdir | sed -e 's/\r/\n/g' + +# success, all done +echo "*** done" +rm -f $seq.full +status=0
tests/qemu-iotests/086.out new file +18 -0
@@ -0,0 +1,18 @@ +QA output created by 086 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 +wrote 1048576/1048576 bytes at offset 0 +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +wrote 1048576/1048576 bytes at offset 2097152 +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +wrote 1048576/1048576 bytes at offset 4194304 +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +wrote 1048576/1048576 bytes at offset 33554432 +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + (0.00/100%) + (25.00/100%) + (50.00/100%) + (75.00/100%) + (100.00/100%) + (100.00/100%) + +*** done
tests/qemu-iotests/group +1 -0
@@ -85,3 +85,4 @@ 079 rw auto 081 rw auto 082 rw auto quick +086 rw auto quick
Signature · 1 line
1.8.1.4
Re: [Qemu-devel] [PATCH 2/2] qemu-iotests: Test progress output for conversion
tests/qemu-iotests/086 line 60
Quoted 13 lines · Kevin Wolf
Signed-off-by Kevin Wolf <[email protected]>--- tests/qemu-iotests/086 | 65 ++++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/086.out | 18 +++++++++++++ tests/qemu-iotests/group | 1 + 3 files changed, 84 insertions(+) create mode 100755 tests/qemu-iotests/086 create mode 100644 tests/qemu-iotests/086.outtests/qemu-iotests/086 new file +3
-0+ +$QEMU_IMG convert -p -O $IMGFMT -f $IMGFMT "$TEST_IMG" "$TEST_IMG".base 2>&1 |\ + _filter_testdir | sed -e 's/\r/\n/g'
\r is not portable sed. Alas, coming up with a portable way to represent carriage return in sed is a bear, so I can live with this reliance on GNU sed.
tests/qemu-iotests/086.out line 16
Quoted 8 lines
tests/qemu-iotests/086.out new file +8
-0+wrote 1048576/1048576 bytes at offset 33554432 +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + (0.00/100%) + (25.00/100%) + (50.00/100%) + (75.00/100%) + (100.00/100%) + (100.00/100%)
Feels fragile (will it ALWAYS be done in chunks of 25% completion?) but if it passes reliably, it's always better to have more test coverage. Can we test the other commands that support -p? And is anyone interested in adding -p support to more commands (there are several long-running commands that could use it but don't have it yet).
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [PATCH 2/2] qemu-iotests: Test progress output for conversion
Am 03.03.2014 um 19:20 hat Eric Blake geschrieben:
Quoted 11 lines · Kevin Wolf
Quoted 9 lines
Signed-off-by Kevin Wolf <[email protected]>--- tests/qemu-iotests/086 | 65 ++++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/086.out | 18 +++++++++++++ tests/qemu-iotests/group | 1 + 3 files changed, 84 insertions(+) create mode 100755 tests/qemu-iotests/086 create mode 100644 tests/qemu-iotests/086.out
Quoted 7 lines
tests/qemu-iotests/086 line 60
Quoted 3 lines
tests/qemu-iotests/086 new file +3
-0+ +$QEMU_IMG convert -p -O $IMGFMT -f $IMGFMT "$TEST_IMG" "$TEST_IMG".base 2>&1 |\ + _filter_testdir | sed -e 's/\r/\n/g'\r is not portable sed. Alas, coming up with a portable way to represent carriage return in sed is a bear, so I can live with this reliance on GNU sed.
We already have it in other places, so I guess this one doesn't make things worse.
Quoted 11 lines
tests/qemu-iotests/086.out line 16
Quoted 8 lines
tests/qemu-iotests/086.out new file +8
-0+wrote 1048576/1048576 bytes at offset 33554432 +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + (0.00/100%) + (25.00/100%) + (50.00/100%) + (75.00/100%) + (100.00/100%) + (100.00/100%)Feels fragile (will it ALWAYS be done in chunks of 25% completion?) but if it passes reliably, it's always better to have more test coverage.
Yes, this concern is what stopped me from writing such a test for a long time. However, I think we really need a test case for it because apparently we tend to break this, and after some pondering I came to the conclusion that results should be consistent as long as we restrict it to a single image format (drivers may behave differently), which is not raw (that would expose filesystem-specific behaviour).
With a qcow2-only case I think we should be fine as long as neither qcow2 nor qemu-img change their implementation.
The other case that I would love to test is not specifying -p but sending SIGUSR1. I'm not sure how to do that in a consistent way, though. Perhaps something with blkdebug.
Quoted 3 lines
Can we test the other commands that support -p? And is anyone interested in adding -p support to more commands (there are several long-running commands that could use it but don't have it yet).
Feel free to implement it. ;-)
(Perhaps it would be easier after converting some qemu-img subcommands to use block jobs internally.)
Kevin