On 01.09.2015 00:55, Jeff Cody wrote: > On Mon, Aug 31, 2015 at 09:05:12PM +0200, Max Reitz wrote: >> Currently, if a qemu/qemu-io/qemu-img/qemu-nbd invocation receives a >> segmentation fault, that message is invisible in most cases since the >> output is generally filtered and bash suppresses the segmentation fault >> notice for any but the last element of a pipe. >> >> Most of the time, the test will then fail anyway because of missing >> output, but not necessarily (as happened with test 82 recently). >> >> Fix this by making the corresponding environment variables point to >> wrapper functions which execute the respective command in a subshell. >> >> Signed-off-by: Max Reitz <mre...@redhat.com> >> --- >> tests/qemu-iotests/039 | 19 ++++++------------- >> tests/qemu-iotests/039.out | 6 +++--- >> tests/qemu-iotests/061 | 6 ++++-- >> tests/qemu-iotests/061.out | 2 ++ >> tests/qemu-iotests/check | 8 ++++---- >> tests/qemu-iotests/common.config | 34 ++++++++++++++++++++++++++++++---- >> tests/qemu-iotests/common.rc | 12 +++++++++++- >> tests/qemu-iotests/iotests.py | 6 +++--- >> 8 files changed, 63 insertions(+), 30 deletions(-) >> > > Test 082 fails now: > > Testing: amend -f qcow2 -o backing_file=TEST_DIR/t.qcow2, -o help > TEST_DIR/t.qcow2 > qemu-img: Invalid option list: backing_file=TEST_DIR/t.qcow2, > +./common.config: line 117: 737 Segmentation fault (core dumped) ( > exec $QEMU_IMG_CMD "$@" ) > > Testing: amend -f qcow2 -o backing_file=TEST_DIR/t.qcow2 -o ,help > TEST_DIR/t.qcow2 > qemu-img: Invalid option list: ,help > +./common.config: line 117: 746 Segmentation fault (core dumped) ( > exec $QEMU_IMG_CMD "$@" ) > > Testing: amend -f qcow2 -o backing_file=TEST_DIR/t.qcow2 -o ,, -o help > TEST_DIR/t.qcow2 > qemu-img: Invalid option list: ,, > +./common.config: line 117: 756 Segmentation fault (core dumped) ( > exec $QEMU_IMG_CMD "$@" ) > > Testing: amend -f qcow2 -o help > Supported options > > > That shows me your patches are working well :)
Yes, as intended. :-) fyi, the patch fixing this is http://lists.nongnu.org/archive/html/qemu-block/2015-08/msg00156.html. >> diff --git a/tests/qemu-iotests/039 b/tests/qemu-iotests/039 >> index 859705f..617f397 100755 >> --- a/tests/qemu-iotests/039 >> +++ b/tests/qemu-iotests/039 >> @@ -47,13 +47,6 @@ _supported_os Linux >> _default_cache_mode "writethrough" >> _supported_cache_modes "writethrough" >> >> -_subshell_exec() >> -{ >> - # Executing crashing commands in a subshell prevents information like >> the >> - # "Killed" line from being lost >> - (exec "$@") >> -} >> - >> size=128M >> >> echo >> @@ -74,8 +67,8 @@ echo "== Creating a dirty image file ==" >> IMGOPTS="compat=1.1,lazy_refcounts=on" >> _make_test_img $size >> >> -_subshell_exec $QEMU_IO -c "write -P 0x5a 0 512" \ >> - -c "sigraise $(kill -l KILL)" "$TEST_IMG" 2>&1 \ >> +$QEMU_IO -c "write -P 0x5a 0 512" \ >> + -c "sigraise $(kill -l KILL)" "$TEST_IMG" 2>&1 \ >> | _filter_qemu_io >> >> # The dirty bit must be set >> @@ -109,8 +102,8 @@ echo "== Opening a dirty image read/write should repair >> it ==" >> IMGOPTS="compat=1.1,lazy_refcounts=on" >> _make_test_img $size >> >> -_subshell_exec $QEMU_IO -c "write -P 0x5a 0 512" \ >> - -c "sigraise $(kill -l KILL)" "$TEST_IMG" 2>&1 \ >> +$QEMU_IO -c "write -P 0x5a 0 512" \ >> + -c "sigraise $(kill -l KILL)" "$TEST_IMG" 2>&1 \ >> | _filter_qemu_io >> >> # The dirty bit must be set >> @@ -127,8 +120,8 @@ echo "== Creating an image file with lazy_refcounts=off >> ==" >> IMGOPTS="compat=1.1,lazy_refcounts=off" >> _make_test_img $size >> >> -_subshell_exec $QEMU_IO -c "write -P 0x5a 0 512" \ >> - -c "sigraise $(kill -l KILL)" "$TEST_IMG" 2>&1 \ >> +$QEMU_IO -c "write -P 0x5a 0 512" \ >> + -c "sigraise $(kill -l KILL)" "$TEST_IMG" 2>&1 \ >> | _filter_qemu_io >> >> # The dirty bit must not be set since lazy_refcounts=off >> diff --git a/tests/qemu-iotests/039.out b/tests/qemu-iotests/039.out >> index d09751f..613ef1b 100644 >> --- a/tests/qemu-iotests/039.out >> +++ b/tests/qemu-iotests/039.out >> @@ -11,7 +11,7 @@ No errors were found on the image. >> Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 >> wrote 512/512 bytes at offset 0 >> 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) >> -./039: Killed ( exec "$@" ) >> +./common.config: Killed ( exec $QEMU_IO_CMD "$@" ) >> incompatible_features 0x1 >> ERROR cluster 5 refcount=0 reference=1 >> ERROR OFLAG_COPIED data cluster: l2_entry=8000000000050000 refcount=0 >> @@ -46,7 +46,7 @@ read 512/512 bytes at offset 0 >> Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 >> wrote 512/512 bytes at offset 0 >> 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) >> -./039: Killed ( exec "$@" ) >> +./common.config: Killed ( exec $QEMU_IO_CMD "$@" ) >> incompatible_features 0x1 >> ERROR cluster 5 refcount=0 reference=1 >> Rebuilding refcount structure >> @@ -60,7 +60,7 @@ incompatible_features 0x0 >> Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 >> wrote 512/512 bytes at offset 0 >> 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) >> -./039: Killed ( exec "$@" ) >> +./common.config: Killed ( exec $QEMU_IO_CMD "$@" ) >> incompatible_features 0x0 >> No errors were found on the image. >> >> diff --git a/tests/qemu-iotests/061 b/tests/qemu-iotests/061 >> index 8d37f8a..1df887a 100755 >> --- a/tests/qemu-iotests/061 >> +++ b/tests/qemu-iotests/061 >> @@ -58,7 +58,8 @@ echo >> echo "=== Testing dirty version downgrade ===" >> echo >> IMGOPTS="compat=1.1,lazy_refcounts=on" _make_test_img 64M >> -$QEMU_IO -c "write -P 0x2a 0 128k" -c flush -c abort "$TEST_IMG" | >> _filter_qemu_io >> +$QEMU_IO -c "write -P 0x2a 0 128k" -c flush -c abort "$TEST_IMG" 2>&1 \ >> + | _filter_qemu_io >> $PYTHON qcow2.py "$TEST_IMG" dump-header >> $QEMU_IMG amend -o "compat=0.10" "$TEST_IMG" >> $PYTHON qcow2.py "$TEST_IMG" dump-header >> @@ -91,7 +92,8 @@ echo >> echo "=== Testing dirty lazy_refcounts=off ===" >> echo >> IMGOPTS="compat=1.1,lazy_refcounts=on" _make_test_img 64M >> -$QEMU_IO -c "write -P 0x2a 0 128k" -c flush -c abort "$TEST_IMG" | >> _filter_qemu_io >> +$QEMU_IO -c "write -P 0x2a 0 128k" -c flush -c abort "$TEST_IMG" 2>&1 \ >> + | _filter_qemu_io >> $PYTHON qcow2.py "$TEST_IMG" dump-header >> $QEMU_IMG amend -o "lazy_refcounts=off" "$TEST_IMG" >> $PYTHON qcow2.py "$TEST_IMG" dump-header >> diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out >> index 5ec248f..c9e3917 100644 >> --- a/tests/qemu-iotests/061.out >> +++ b/tests/qemu-iotests/061.out >> @@ -57,6 +57,7 @@ No errors were found on the image. >> Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 >> wrote 131072/131072 bytes at offset 0 >> 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) >> +./common.config: Aborted (core dumped) ( exec $QEMU_IO_CMD >> "$@" ) >> magic 0x514649fb >> version 3 >> backing_file_offset 0x0 >> @@ -214,6 +215,7 @@ No errors were found on the image. >> Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 >> wrote 131072/131072 bytes at offset 0 >> 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) >> +./common.config: Aborted (core dumped) ( exec $QEMU_IO_CMD >> "$@" ) >> magic 0x514649fb >> version 3 >> backing_file_offset 0x0 >> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check >> index 6d58203..b5a535e 100755 >> --- a/tests/qemu-iotests/check >> +++ b/tests/qemu-iotests/check >> @@ -231,10 +231,10 @@ FULL_HOST_DETAILS=`_full_platform_details` >> #FULL_MOUNT_OPTIONS=`_scratch_mount_options` >> >> cat <<EOF >> -QEMU -- $QEMU >> -QEMU_IMG -- $QEMU_IMG >> -QEMU_IO -- $QEMU_IO >> -QEMU_NBD -- $QEMU_NBD >> +QEMU -- $QEMU_CMD >> +QEMU_IMG -- $QEMU_IMG_CMD >> +QEMU_IO -- $QEMU_IO_CMD >> +QEMU_NBD -- $QEMU_NBD_CMD >> IMGFMT -- $FULL_IMGFMT_DETAILS >> IMGPROTO -- $FULL_IMGPROTO_DETAILS >> PLATFORM -- $FULL_HOST_DETAILS >> diff --git a/tests/qemu-iotests/common.config >> b/tests/qemu-iotests/common.config >> index e0bf896..480efe6 100644 >> --- a/tests/qemu-iotests/common.config >> +++ b/tests/qemu-iotests/common.config >> @@ -103,10 +103,36 @@ if [ -z "$QEMU_NBD_PROG" ]; then >> export QEMU_NBD_PROG="`set_prog_path qemu-nbd`" >> fi >> >> -export QEMU="$QEMU_PROG $QEMU_OPTIONS" >> -export QEMU_IMG=$QEMU_IMG_PROG >> -export QEMU_IO="$QEMU_IO_PROG $QEMU_IO_OPTIONS" >> -export QEMU_NBD=$QEMU_NBD_PROG >> +export QEMU_CMD="$QEMU_PROG $QEMU_OPTIONS" >> +export QEMU_IMG_CMD=$QEMU_IMG_PROG >> +export QEMU_IO_CMD="$QEMU_IO_PROG $QEMU_IO_OPTIONS" >> +export QEMU_NBD_CMD=$QEMU_NBD_PROG >> + > > Unfortunately, these exports (old and new) make it so that pathnames > with spaces won't work (in case someone hasn't had it beaten into them > that spaced pathnames is begging for trouble...). But luckily, I > think your patch make it easier to fix this, since you have the > wrapper! > > I think we want to drop the _OPTIONS in each of the exports, e.g.: > > -export QEMU="$QEMU_PROG $QEMU_OPTIONS" > +export QEMU_CMD="$QEMU_PROG" > > And then instead of this: > >> +_qemu_wrapper() >> +{ >> + (exec $QEMU_CMD "$@") >> +} >> + > > Use this form, instead: > > +_qemu_wrapper() > +{ > + (exec "$QEMU_CMD" "$QEMU_OPTIONS" "$@") > +} > + Yes, I tried not to break anything in that regard that wasn't already broken, but if we have the chance to fix something, then we (I) should go for it. QEMU_CMD is used for the Python tests as the command line to be used for qemu invocation, so it cannot be modified like that. But what I can do is to drop QEMU.*_CMD and replace it by "$QEMU.*_PROG" "$QEMU.*_OPTIONS" everywhere, I guess. I'll have a look. Thanks for reviewing! Max >> +_qemu_img_wrapper() >> +{ >> + (exec $QEMU_IMG_CMD "$@") >> +} >> + >> +_qemu_io_wrapper() >> +{ >> + (exec $QEMU_IO_CMD "$@") >> +} >> + >> +_qemu_nbd_wrapper() >> +{ >> + (exec $QEMU_NBD_CMD "$@") >> +} >> + > > Repeat as appropriate, above. > >> +export QEMU=_qemu_wrapper >> +export QEMU_IMG=_qemu_img_wrapper >> +export QEMU_IO=_qemu_io_wrapper >> +export QEMU_NBD=_qemu_nbd_wrapper >> + >> default_machine=$($QEMU -machine \? | awk '/(default)/{print $1}') >> default_alias_machine=$($QEMU -machine \? |\ >> awk -v var_default_machine="$default_machine"\)\ >> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc >> index 22d3514..28e4bea 100644 >> --- a/tests/qemu-iotests/common.rc >> +++ b/tests/qemu-iotests/common.rc >> @@ -439,7 +439,17 @@ _unsupported_imgopts() >> # >> _require_command() >> { >> - eval c=\$$1 >> + if [ "$1" = "QEMU" ]; then >> + c=$QEMU_PROG >> + elif [ "$1" = "QEMU_IMG" ]; then >> + c=$QEMU_IMG_PROG >> + elif [ "$1" = "QEMU_IO" ]; then >> + c=$QEMU_IO_PROG >> + elif [ "$1" = "QEMU_NBD" ]; then >> + c=$QEMU_NBD_PROG >> + else >> + eval c=\$$1 >> + fi >> [ -x "$c" ] || _notrun "$1 utility required, skipped this test" >> } >> >> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py >> index 5579253..927c74a 100644 >> --- a/tests/qemu-iotests/iotests.py >> +++ b/tests/qemu-iotests/iotests.py >> @@ -33,9 +33,9 @@ __all__ = ['imgfmt', 'imgproto', 'test_dir' 'qemu_img', >> 'qemu_io', >> >> # This will not work if arguments or path contain spaces but is necessary >> if we >> # want to support the override options that ./check supports. >> -qemu_img_args = os.environ.get('QEMU_IMG', 'qemu-img').strip().split(' ') >> -qemu_io_args = os.environ.get('QEMU_IO', 'qemu-io').strip().split(' ') >> -qemu_args = os.environ.get('QEMU', 'qemu').strip().split(' ') >> +qemu_img_args = os.environ.get('QEMU_IMG_CMD', 'qemu-img').strip().split(' >> ') >> +qemu_io_args = os.environ.get('QEMU_IO_CMD', 'qemu-io').strip().split(' ') >> +qemu_args = os.environ.get('QEMU_CMD', 'qemu').strip().split(' ') >> >> imgfmt = os.environ.get('IMGFMT', 'raw') >> imgproto = os.environ.get('IMGPROTO', 'file') >> -- >> 2.5.0 >> >>
signature.asc
Description: OpenPGP digital signature
