qemu-block
[Top][All Lists]
Advanced

[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
>>
>>


Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]