* [Qemu-devel] [PATCH 0/2] iotests: Emit signal-kill messages @ 2015-08-31 19:05 Max Reitz 2015-08-31 19:05 ` [Qemu-devel] [PATCH 1/2] iotests: Do not suppress segfaults in bash tests Max Reitz 2015-08-31 19:05 ` [Qemu-devel] [PATCH 2/2] iotests: Warn if python subprocess is killed Max Reitz 0 siblings, 2 replies; 13+ messages in thread From: Max Reitz @ 2015-08-31 19:05 UTC (permalink / raw) To: qemu-block; +Cc: Kevin Wolf, qemu-devel, Max Reitz Currently, if a qemu-related command (qemu, qemu-io, qemu-img, qemu-nbd) is invoked in an iotest, receives a signal and is subsequently killed (e.g. a segmentation fault), this is not logged in the test output. The first patch in this series makes the bash tests no longer suppress that line, and the second patch adds a similar notification for the python tests. Max Reitz (2): iotests: Do not suppress segfaults in bash tests iotests: Warn if python subprocess is killed 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 | 32 ++++++++++++++++++++++++-------- 8 files changed, 84 insertions(+), 35 deletions(-) -- 2.5.0 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 1/2] iotests: Do not suppress segfaults in bash tests 2015-08-31 19:05 [Qemu-devel] [PATCH 0/2] iotests: Emit signal-kill messages Max Reitz @ 2015-08-31 19:05 ` Max Reitz 2015-08-31 22:55 ` [Qemu-devel] [Qemu-block] " Jeff Cody 2015-10-29 14:57 ` [Qemu-devel] " Kevin Wolf 2015-08-31 19:05 ` [Qemu-devel] [PATCH 2/2] iotests: Warn if python subprocess is killed Max Reitz 1 sibling, 2 replies; 13+ messages in thread From: Max Reitz @ 2015-08-31 19:05 UTC (permalink / raw) To: qemu-block; +Cc: Kevin Wolf, qemu-devel, Max Reitz 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 <mreitz@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(-) 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 + +_qemu_wrapper() +{ + (exec $QEMU_CMD "$@") +} + +_qemu_img_wrapper() +{ + (exec $QEMU_IMG_CMD "$@") +} + +_qemu_io_wrapper() +{ + (exec $QEMU_IO_CMD "$@") +} + +_qemu_nbd_wrapper() +{ + (exec $QEMU_NBD_CMD "$@") +} + +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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] iotests: Do not suppress segfaults in bash tests 2015-08-31 19:05 ` [Qemu-devel] [PATCH 1/2] iotests: Do not suppress segfaults in bash tests Max Reitz @ 2015-08-31 22:55 ` Jeff Cody 2015-09-02 15:09 ` Max Reitz 2015-10-29 14:57 ` [Qemu-devel] " Kevin Wolf 1 sibling, 1 reply; 13+ messages in thread From: Jeff Cody @ 2015-08-31 22:55 UTC (permalink / raw) To: Max Reitz; +Cc: Kevin Wolf, qemu-devel, qemu-block 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 <mreitz@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 :) > 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" "$@") +} + > +_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 > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] iotests: Do not suppress segfaults in bash tests 2015-08-31 22:55 ` [Qemu-devel] [Qemu-block] " Jeff Cody @ 2015-09-02 15:09 ` Max Reitz 2015-09-02 15:30 ` Jeff Cody 0 siblings, 1 reply; 13+ messages in thread From: Max Reitz @ 2015-09-02 15:09 UTC (permalink / raw) To: Jeff Cody; +Cc: Kevin Wolf, qemu-devel, qemu-block [-- Attachment #1: Type: text/plain, Size: 12788 bytes --] 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 <mreitz@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 >> >> [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] iotests: Do not suppress segfaults in bash tests 2015-09-02 15:09 ` Max Reitz @ 2015-09-02 15:30 ` Jeff Cody 0 siblings, 0 replies; 13+ messages in thread From: Jeff Cody @ 2015-09-02 15:30 UTC (permalink / raw) To: Max Reitz; +Cc: Kevin Wolf, qemu-devel, qemu-block On Wed, Sep 02, 2015 at 05:09:41PM +0200, Max Reitz wrote: > 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 <mreitz@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(-) > >> [snip] > >> > >> -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. > Good point. I don't think it needs to be done in this patch series, as your patches don't change this behavior. It could be done in a follow-up series, by you or someone else. I'm happy to give my r-b as-is, and we can change it later.: Reviewed-by: Jeff Cody <jcody@redhat.com> > Thanks for reviewing! > > Max You're welcome :) > > >> +_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 > >> > >> > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] iotests: Do not suppress segfaults in bash tests 2015-08-31 19:05 ` [Qemu-devel] [PATCH 1/2] iotests: Do not suppress segfaults in bash tests Max Reitz 2015-08-31 22:55 ` [Qemu-devel] [Qemu-block] " Jeff Cody @ 2015-10-29 14:57 ` Kevin Wolf 2015-10-29 18:05 ` [Qemu-devel] [Qemu-block] " Jeff Cody 1 sibling, 1 reply; 13+ messages in thread From: Kevin Wolf @ 2015-10-29 14:57 UTC (permalink / raw) To: Max Reitz; +Cc: qemu-devel, qemu-block Am 31.08.2015 um 21:05 hat Max Reitz geschrieben: > 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 <mreitz@redhat.com> This patch seems to have broken -valgrind. Kevin ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] iotests: Do not suppress segfaults in bash tests 2015-10-29 14:57 ` [Qemu-devel] " Kevin Wolf @ 2015-10-29 18:05 ` Jeff Cody 0 siblings, 0 replies; 13+ messages in thread From: Jeff Cody @ 2015-10-29 18:05 UTC (permalink / raw) To: Kevin Wolf; +Cc: qemu-devel, qemu-block, Max Reitz On Thu, Oct 29, 2015 at 03:57:17PM +0100, Kevin Wolf wrote: > Am 31.08.2015 um 21:05 hat Max Reitz geschrieben: > > 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 <mreitz@redhat.com> > > This patch seems to have broken -valgrind. > Hi Kevin, I just submitted another patch to fix this issue ("qemu-iotests: fix -valgrind option for check") Thanks, Jeff ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 2/2] iotests: Warn if python subprocess is killed 2015-08-31 19:05 [Qemu-devel] [PATCH 0/2] iotests: Emit signal-kill messages Max Reitz 2015-08-31 19:05 ` [Qemu-devel] [PATCH 1/2] iotests: Do not suppress segfaults in bash tests Max Reitz @ 2015-08-31 19:05 ` Max Reitz 2015-09-08 21:25 ` [Qemu-devel] [Qemu-block] " John Snow 1 sibling, 1 reply; 13+ messages in thread From: Max Reitz @ 2015-08-31 19:05 UTC (permalink / raw) To: qemu-block; +Cc: Kevin Wolf, qemu-devel, Max Reitz Currently, if a subprocess of a python test (i.e. qemu-io, qemu-img, or qemu) receives a signal and is subsequently aborted, this is not logged. This patch makes python tests always check the exit code of these subprocesses, and emit a message if they have been killed. Signed-off-by: Max Reitz <mreitz@redhat.com> --- tests/qemu-iotests/iotests.py | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index 927c74a..d082597 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -49,20 +49,34 @@ socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', 'socket_scm_helper') def qemu_img(*args): '''Run qemu-img and return the exit code''' devnull = open('/dev/null', 'r+') - return subprocess.call(qemu_img_args + list(args), stdin=devnull, stdout=devnull) + exitcode = subprocess.call(qemu_img_args + list(args), stdin=devnull, stdout=devnull) + if exitcode < 0: + sys.stderr.write('qemu-img received signal %i: %s\n' % (-exitcode, ' '.join(qemu_img_args + list(args)))) + return exitcode def qemu_img_verbose(*args): '''Run qemu-img without suppressing its output and return the exit code''' - return subprocess.call(qemu_img_args + list(args)) + exitcode = subprocess.call(qemu_img_args + list(args)) + if exitcode < 0: + sys.stderr.write('qemu-img received signal %i: %s\n' % (-exitcode, ' '.join(qemu_img_args + list(args)))) + return exitcode def qemu_img_pipe(*args): '''Run qemu-img and return its output''' - return subprocess.Popen(qemu_img_args + list(args), stdout=subprocess.PIPE).communicate()[0] + subp = subprocess.Popen(qemu_img_args + list(args), stdout=subprocess.PIPE) + exitcode = subp.wait() + if exitcode < 0: + sys.stderr.write('qemu-img received signal %i: %s\n' % (-exitcode, ' '.join(qemu_img_args + list(args)))) + return subp.communicate()[0] def qemu_io(*args): '''Run qemu-io and return the stdout data''' args = qemu_io_args + list(args) - return subprocess.Popen(args, stdout=subprocess.PIPE).communicate()[0] + subp = subprocess.Popen(args, stdout=subprocess.PIPE) + exitcode = subp.wait() + if exitcode < 0: + sys.stderr.write('qemu-io received signal %i: %s\n' % (-exitcode, ' '.join(args))) + return subp.communicate()[0] def compare_images(img1, img2): '''Return True if two image files are identical''' @@ -197,7 +211,9 @@ class VM(object): '''Terminate the VM and clean up''' if not self._popen is None: self._qmp.cmd('quit') - self._popen.wait() + exitcode = self._popen.wait() + if exitcode < 0: + sys.stderr.write('qemu received signal %i: %s\n' % (-exitcode, ' '.join(self._args))) os.remove(self._monitor_path) os.remove(self._qtest_path) os.remove(self._qemu_log_path) -- 2.5.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH 2/2] iotests: Warn if python subprocess is killed 2015-08-31 19:05 ` [Qemu-devel] [PATCH 2/2] iotests: Warn if python subprocess is killed Max Reitz @ 2015-09-08 21:25 ` John Snow 2015-09-08 21:29 ` Max Reitz 0 siblings, 1 reply; 13+ messages in thread From: John Snow @ 2015-09-08 21:25 UTC (permalink / raw) To: Max Reitz, qemu-block; +Cc: Kevin Wolf, qemu-devel On 08/31/2015 03:05 PM, Max Reitz wrote: > Currently, if a subprocess of a python test (i.e. qemu-io, qemu-img, or > qemu) receives a signal and is subsequently aborted, this is not logged. > > This patch makes python tests always check the exit code of these > subprocesses, and emit a message if they have been killed. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > tests/qemu-iotests/iotests.py | 26 +++++++++++++++++++++----- > 1 file changed, 21 insertions(+), 5 deletions(-) > > diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py > index 927c74a..d082597 100644 > --- a/tests/qemu-iotests/iotests.py > +++ b/tests/qemu-iotests/iotests.py > @@ -49,20 +49,34 @@ socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', 'socket_scm_helper') > def qemu_img(*args): > '''Run qemu-img and return the exit code''' > devnull = open('/dev/null', 'r+') > - return subprocess.call(qemu_img_args + list(args), stdin=devnull, stdout=devnull) > + exitcode = subprocess.call(qemu_img_args + list(args), stdin=devnull, stdout=devnull) > + if exitcode < 0: > + sys.stderr.write('qemu-img received signal %i: %s\n' % (-exitcode, ' '.join(qemu_img_args + list(args)))) > + return exitcode > Why tack on the arguments after the retcode for the print? The format makes it look like it should be a description for the signal received. > def qemu_img_verbose(*args): > '''Run qemu-img without suppressing its output and return the exit code''' > - return subprocess.call(qemu_img_args + list(args)) > + exitcode = subprocess.call(qemu_img_args + list(args)) > + if exitcode < 0: > + sys.stderr.write('qemu-img received signal %i: %s\n' % (-exitcode, ' '.join(qemu_img_args + list(args)))) > + return exitcode > > def qemu_img_pipe(*args): > '''Run qemu-img and return its output''' > - return subprocess.Popen(qemu_img_args + list(args), stdout=subprocess.PIPE).communicate()[0] > + subp = subprocess.Popen(qemu_img_args + list(args), stdout=subprocess.PIPE) > + exitcode = subp.wait() > + if exitcode < 0: > + sys.stderr.write('qemu-img received signal %i: %s\n' % (-exitcode, ' '.join(qemu_img_args + list(args)))) > + return subp.communicate()[0] > > def qemu_io(*args): > '''Run qemu-io and return the stdout data''' > args = qemu_io_args + list(args) > - return subprocess.Popen(args, stdout=subprocess.PIPE).communicate()[0] > + subp = subprocess.Popen(args, stdout=subprocess.PIPE) > + exitcode = subp.wait() > + if exitcode < 0: > + sys.stderr.write('qemu-io received signal %i: %s\n' % (-exitcode, ' '.join(args))) > + return subp.communicate()[0] > > def compare_images(img1, img2): > '''Return True if two image files are identical''' > @@ -197,7 +211,9 @@ class VM(object): > '''Terminate the VM and clean up''' > if not self._popen is None: > self._qmp.cmd('quit') > - self._popen.wait() > + exitcode = self._popen.wait() > + if exitcode < 0: > + sys.stderr.write('qemu received signal %i: %s\n' % (-exitcode, ' '.join(self._args))) > os.remove(self._monitor_path) > os.remove(self._qtest_path) > os.remove(self._qemu_log_path) > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH 2/2] iotests: Warn if python subprocess is killed 2015-09-08 21:25 ` [Qemu-devel] [Qemu-block] " John Snow @ 2015-09-08 21:29 ` Max Reitz 2015-09-08 21:37 ` John Snow 0 siblings, 1 reply; 13+ messages in thread From: Max Reitz @ 2015-09-08 21:29 UTC (permalink / raw) To: John Snow, qemu-block; +Cc: Kevin Wolf, qemu-devel [-- Attachment #1: Type: text/plain, Size: 3777 bytes --] On 08.09.2015 23:25, John Snow wrote: > > > On 08/31/2015 03:05 PM, Max Reitz wrote: >> Currently, if a subprocess of a python test (i.e. qemu-io, qemu-img, or >> qemu) receives a signal and is subsequently aborted, this is not logged. >> >> This patch makes python tests always check the exit code of these >> subprocesses, and emit a message if they have been killed. >> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- >> tests/qemu-iotests/iotests.py | 26 +++++++++++++++++++++----- >> 1 file changed, 21 insertions(+), 5 deletions(-) >> >> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py >> index 927c74a..d082597 100644 >> --- a/tests/qemu-iotests/iotests.py >> +++ b/tests/qemu-iotests/iotests.py >> @@ -49,20 +49,34 @@ socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', 'socket_scm_helper') >> def qemu_img(*args): >> '''Run qemu-img and return the exit code''' >> devnull = open('/dev/null', 'r+') >> - return subprocess.call(qemu_img_args + list(args), stdin=devnull, stdout=devnull) >> + exitcode = subprocess.call(qemu_img_args + list(args), stdin=devnull, stdout=devnull) >> + if exitcode < 0: >> + sys.stderr.write('qemu-img received signal %i: %s\n' % (-exitcode, ' '.join(qemu_img_args + list(args)))) >> + return exitcode >> > > Why tack on the arguments after the retcode for the print? The format > makes it look like it should be a description for the signal received. qemu_img_args contains the qemu-img filename as well, so it should be obvious that that is the command line. For the "why": I hope it will make debugging easier by providing the exact command line so you can reproduce the problem outside of the test. Max > >> def qemu_img_verbose(*args): >> '''Run qemu-img without suppressing its output and return the exit code''' >> - return subprocess.call(qemu_img_args + list(args)) >> + exitcode = subprocess.call(qemu_img_args + list(args)) >> + if exitcode < 0: >> + sys.stderr.write('qemu-img received signal %i: %s\n' % (-exitcode, ' '.join(qemu_img_args + list(args)))) >> + return exitcode >> >> def qemu_img_pipe(*args): >> '''Run qemu-img and return its output''' >> - return subprocess.Popen(qemu_img_args + list(args), stdout=subprocess.PIPE).communicate()[0] >> + subp = subprocess.Popen(qemu_img_args + list(args), stdout=subprocess.PIPE) >> + exitcode = subp.wait() >> + if exitcode < 0: >> + sys.stderr.write('qemu-img received signal %i: %s\n' % (-exitcode, ' '.join(qemu_img_args + list(args)))) >> + return subp.communicate()[0] >> >> def qemu_io(*args): >> '''Run qemu-io and return the stdout data''' >> args = qemu_io_args + list(args) >> - return subprocess.Popen(args, stdout=subprocess.PIPE).communicate()[0] >> + subp = subprocess.Popen(args, stdout=subprocess.PIPE) >> + exitcode = subp.wait() >> + if exitcode < 0: >> + sys.stderr.write('qemu-io received signal %i: %s\n' % (-exitcode, ' '.join(args))) >> + return subp.communicate()[0] >> >> def compare_images(img1, img2): >> '''Return True if two image files are identical''' >> @@ -197,7 +211,9 @@ class VM(object): >> '''Terminate the VM and clean up''' >> if not self._popen is None: >> self._qmp.cmd('quit') >> - self._popen.wait() >> + exitcode = self._popen.wait() >> + if exitcode < 0: >> + sys.stderr.write('qemu received signal %i: %s\n' % (-exitcode, ' '.join(self._args))) >> os.remove(self._monitor_path) >> os.remove(self._qtest_path) >> os.remove(self._qemu_log_path) >> [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH 2/2] iotests: Warn if python subprocess is killed 2015-09-08 21:29 ` Max Reitz @ 2015-09-08 21:37 ` John Snow 2015-09-08 21:38 ` Max Reitz 0 siblings, 1 reply; 13+ messages in thread From: John Snow @ 2015-09-08 21:37 UTC (permalink / raw) To: Max Reitz, qemu-block; +Cc: Kevin Wolf, qemu-devel On 09/08/2015 05:29 PM, Max Reitz wrote: > On 08.09.2015 23:25, John Snow wrote: >> >> >> On 08/31/2015 03:05 PM, Max Reitz wrote: >>> Currently, if a subprocess of a python test (i.e. qemu-io, >>> qemu-img, or qemu) receives a signal and is subsequently >>> aborted, this is not logged. >>> >>> This patch makes python tests always check the exit code of >>> these subprocesses, and emit a message if they have been >>> killed. >>> >>> Signed-off-by: Max Reitz <mreitz@redhat.com> --- >>> tests/qemu-iotests/iotests.py | 26 +++++++++++++++++++++----- 1 >>> file changed, 21 insertions(+), 5 deletions(-) >>> >>> diff --git a/tests/qemu-iotests/iotests.py >>> b/tests/qemu-iotests/iotests.py index 927c74a..d082597 100644 >>> --- a/tests/qemu-iotests/iotests.py +++ >>> b/tests/qemu-iotests/iotests.py @@ -49,20 +49,34 @@ >>> socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', >>> 'socket_scm_helper') def qemu_img(*args): '''Run qemu-img and >>> return the exit code''' devnull = open('/dev/null', 'r+') - >>> return subprocess.call(qemu_img_args + list(args), >>> stdin=devnull, stdout=devnull) + exitcode = >>> subprocess.call(qemu_img_args + list(args), stdin=devnull, >>> stdout=devnull) + if exitcode < 0: + >>> sys.stderr.write('qemu-img received signal %i: %s\n' % >>> (-exitcode, ' '.join(qemu_img_args + list(args)))) + return >>> exitcode >>> >> >> Why tack on the arguments after the retcode for the print? The >> format makes it look like it should be a description for the >> signal received. > > qemu_img_args contains the qemu-img filename as well, so it should > be obvious that that is the command line. > > For the "why": I hope it will make debugging easier by providing > the exact command line so you can reproduce the problem outside of > the test. > > Max > Sorry for being captain bikeshed: can this be something like: "%s received signal %i; args: %s" to avoid the string looking like it was meant to answer the question of what the signal was? I won't insist on it, though: Reviewed-by: John Snow <jsnow@redhat.com> >> >>> def qemu_img_verbose(*args): '''Run qemu-img without >>> suppressing its output and return the exit code''' - return >>> subprocess.call(qemu_img_args + list(args)) + exitcode = >>> subprocess.call(qemu_img_args + list(args)) + if exitcode < >>> 0: + sys.stderr.write('qemu-img received signal %i: >>> %s\n' % (-exitcode, ' '.join(qemu_img_args + list(args)))) + >>> return exitcode >>> >>> def qemu_img_pipe(*args): '''Run qemu-img and return its >>> output''' - return subprocess.Popen(qemu_img_args + >>> list(args), stdout=subprocess.PIPE).communicate()[0] + subp >>> = subprocess.Popen(qemu_img_args + list(args), >>> stdout=subprocess.PIPE) + exitcode = subp.wait() + if >>> exitcode < 0: + sys.stderr.write('qemu-img received >>> signal %i: %s\n' % (-exitcode, ' '.join(qemu_img_args + >>> list(args)))) + return subp.communicate()[0] >>> >>> def qemu_io(*args): '''Run qemu-io and return the stdout >>> data''' args = qemu_io_args + list(args) - return >>> subprocess.Popen(args, >>> stdout=subprocess.PIPE).communicate()[0] + subp = >>> subprocess.Popen(args, stdout=subprocess.PIPE) + exitcode = >>> subp.wait() + if exitcode < 0: + >>> sys.stderr.write('qemu-io received signal %i: %s\n' % >>> (-exitcode, ' '.join(args))) + return subp.communicate()[0] >>> >>> def compare_images(img1, img2): '''Return True if two image >>> files are identical''' @@ -197,7 +211,9 @@ class VM(object): >>> '''Terminate the VM and clean up''' if not self._popen is >>> None: self._qmp.cmd('quit') - self._popen.wait() + >>> exitcode = self._popen.wait() + if exitcode < 0: + >>> sys.stderr.write('qemu received signal %i: %s\n' % (-exitcode, >>> ' '.join(self._args))) os.remove(self._monitor_path) >>> os.remove(self._qtest_path) os.remove(self._qemu_log_path) >>> > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH 2/2] iotests: Warn if python subprocess is killed 2015-09-08 21:37 ` John Snow @ 2015-09-08 21:38 ` Max Reitz 2015-09-08 21:42 ` John Snow 0 siblings, 1 reply; 13+ messages in thread From: Max Reitz @ 2015-09-08 21:38 UTC (permalink / raw) To: John Snow, qemu-block; +Cc: Kevin Wolf, qemu-devel [-- Attachment #1: Type: text/plain, Size: 2406 bytes --] On 08.09.2015 23:37, John Snow wrote: > > > On 09/08/2015 05:29 PM, Max Reitz wrote: >> On 08.09.2015 23:25, John Snow wrote: >>> >>> >>> On 08/31/2015 03:05 PM, Max Reitz wrote: >>>> Currently, if a subprocess of a python test (i.e. qemu-io, >>>> qemu-img, or qemu) receives a signal and is subsequently >>>> aborted, this is not logged. >>>> >>>> This patch makes python tests always check the exit code of >>>> these subprocesses, and emit a message if they have been >>>> killed. >>>> >>>> Signed-off-by: Max Reitz <mreitz@redhat.com> --- >>>> tests/qemu-iotests/iotests.py | 26 +++++++++++++++++++++----- 1 >>>> file changed, 21 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/tests/qemu-iotests/iotests.py >>>> b/tests/qemu-iotests/iotests.py index 927c74a..d082597 100644 >>>> --- a/tests/qemu-iotests/iotests.py +++ >>>> b/tests/qemu-iotests/iotests.py @@ -49,20 +49,34 @@ >>>> socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', >>>> 'socket_scm_helper') def qemu_img(*args): '''Run qemu-img and >>>> return the exit code''' devnull = open('/dev/null', 'r+') - >>>> return subprocess.call(qemu_img_args + list(args), >>>> stdin=devnull, stdout=devnull) + exitcode = >>>> subprocess.call(qemu_img_args + list(args), stdin=devnull, >>>> stdout=devnull) + if exitcode < 0: + >>>> sys.stderr.write('qemu-img received signal %i: %s\n' % >>>> (-exitcode, ' '.join(qemu_img_args + list(args)))) + return >>>> exitcode >>>> >>> >>> Why tack on the arguments after the retcode for the print? The >>> format makes it look like it should be a description for the >>> signal received. >> >> qemu_img_args contains the qemu-img filename as well, so it should >> be obvious that that is the command line. >> >> For the "why": I hope it will make debugging easier by providing >> the exact command line so you can reproduce the problem outside of >> the test. >> >> Max >> > > Sorry for being captain bikeshed: can this be something like: > > "%s received signal %i; args: %s" > > to avoid the string looking like it was meant to answer the question > of what the signal was? > I won't insist on it, though: > > Reviewed-by: John Snow <jsnow@redhat.com> Well, feel free to fix it since it's already in master. :-) (also, compare how bash does it: > $FILE: line $LINE: $PID $SIGNAL_NAME (core dumped) ($ARGS) ) Max [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH 2/2] iotests: Warn if python subprocess is killed 2015-09-08 21:38 ` Max Reitz @ 2015-09-08 21:42 ` John Snow 0 siblings, 0 replies; 13+ messages in thread From: John Snow @ 2015-09-08 21:42 UTC (permalink / raw) To: Max Reitz, qemu-block; +Cc: Kevin Wolf, qemu-devel On 09/08/2015 05:38 PM, Max Reitz wrote: > On 08.09.2015 23:37, John Snow wrote: >> >> >> On 09/08/2015 05:29 PM, Max Reitz wrote: >>> On 08.09.2015 23:25, John Snow wrote: >>>> >>>> >>>> On 08/31/2015 03:05 PM, Max Reitz wrote: >>>>> Currently, if a subprocess of a python test (i.e. qemu-io, >>>>> qemu-img, or qemu) receives a signal and is subsequently >>>>> aborted, this is not logged. >>>>> >>>>> This patch makes python tests always check the exit code >>>>> of these subprocesses, and emit a message if they have >>>>> been killed. >>>>> >>>>> Signed-off-by: Max Reitz <mreitz@redhat.com> --- >>>>> tests/qemu-iotests/iotests.py | 26 >>>>> +++++++++++++++++++++----- 1 file changed, 21 >>>>> insertions(+), 5 deletions(-) >>>>> >>>>> diff --git a/tests/qemu-iotests/iotests.py >>>>> b/tests/qemu-iotests/iotests.py index 927c74a..d082597 >>>>> 100644 --- a/tests/qemu-iotests/iotests.py +++ >>>>> b/tests/qemu-iotests/iotests.py @@ -49,20 +49,34 @@ >>>>> socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', >>>>> 'socket_scm_helper') def qemu_img(*args): '''Run qemu-img >>>>> and return the exit code''' devnull = open('/dev/null', >>>>> 'r+') - return subprocess.call(qemu_img_args + list(args), >>>>> stdin=devnull, stdout=devnull) + exitcode = >>>>> subprocess.call(qemu_img_args + list(args), stdin=devnull, >>>>> stdout=devnull) + if exitcode < 0: + >>>>> sys.stderr.write('qemu-img received signal %i: %s\n' % >>>>> (-exitcode, ' '.join(qemu_img_args + list(args)))) + >>>>> return exitcode >>>>> >>>> >>>> Why tack on the arguments after the retcode for the print? >>>> The format makes it look like it should be a description for >>>> the signal received. >>> >>> qemu_img_args contains the qemu-img filename as well, so it >>> should be obvious that that is the command line. >>> >>> For the "why": I hope it will make debugging easier by >>> providing the exact command line so you can reproduce the >>> problem outside of the test. >>> >>> Max >>> >> >> Sorry for being captain bikeshed: can this be something like: >> >> "%s received signal %i; args: %s" >> >> to avoid the string looking like it was meant to answer the >> question of what the signal was? I won't insist on it, though: >> >> Reviewed-by: John Snow <jsnow@redhat.com> > > Well, feel free to fix it since it's already in master. :-) > > (also, compare how bash does it: >> $FILE: line $LINE: $PID $SIGNAL_NAME (core dumped) ($ARGS) > ) > > Max > ugh! pfeh! didn't see the v2, sorry. ignore this garbage. --js ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2015-10-29 18:05 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-08-31 19:05 [Qemu-devel] [PATCH 0/2] iotests: Emit signal-kill messages Max Reitz 2015-08-31 19:05 ` [Qemu-devel] [PATCH 1/2] iotests: Do not suppress segfaults in bash tests Max Reitz 2015-08-31 22:55 ` [Qemu-devel] [Qemu-block] " Jeff Cody 2015-09-02 15:09 ` Max Reitz 2015-09-02 15:30 ` Jeff Cody 2015-10-29 14:57 ` [Qemu-devel] " Kevin Wolf 2015-10-29 18:05 ` [Qemu-devel] [Qemu-block] " Jeff Cody 2015-08-31 19:05 ` [Qemu-devel] [PATCH 2/2] iotests: Warn if python subprocess is killed Max Reitz 2015-09-08 21:25 ` [Qemu-devel] [Qemu-block] " John Snow 2015-09-08 21:29 ` Max Reitz 2015-09-08 21:37 ` John Snow 2015-09-08 21:38 ` Max Reitz 2015-09-08 21:42 ` John Snow
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).