[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH 1/2] iotests: Do not suppress segfaults in bash
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [PATCH 1/2] iotests: Do not suppress segfaults in bash tests |
Date: |
Wed, 2 Sep 2015 17:09:41 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 |
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 <address@hidden>
>> ---
>> 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
- Re: [Qemu-block] [PATCH 1/2] iotests: Do not suppress segfaults in bash tests,
Max Reitz <=