From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57259) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZX9fe-0004BB-1q for qemu-devel@nongnu.org; Wed, 02 Sep 2015 11:09:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZX9fc-0003of-0H for qemu-devel@nongnu.org; Wed, 02 Sep 2015 11:09:53 -0400 References: <1441047913-30596-1-git-send-email-mreitz@redhat.com> <1441047913-30596-2-git-send-email-mreitz@redhat.com> <20150831225519.GG31272@localhost.localdomain> From: Max Reitz Message-ID: <55E71135.1060403@redhat.com> Date: Wed, 2 Sep 2015 17:09:41 +0200 MIME-Version: 1.0 In-Reply-To: <20150831225519.GG31272@localhost.localdomain> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="PgWDf6D4eD9H5LFCrMTqEKiuF7r3PhFem" Subject: Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] iotests: Do not suppress segfaults in bash tests List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jeff Cody Cc: Kevin Wolf , qemu-devel@nongnu.org, qemu-block@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --PgWDf6D4eD9H5LFCrMTqEKiuF7r3PhFem Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable 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 faul= t >> 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 >> --- >> 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(-) >> >=20 > Test 082 fails now: >=20 > Testing: amend -f qcow2 -o backing_file=3DTEST_DIR/t.qcow2, -o help T= EST_DIR/t.qcow2 > qemu-img: Invalid option list: backing_file=3DTEST_DIR/t.qcow2, > +./common.config: line 117: 737 Segmentation fault (core dumped= ) ( exec $QEMU_IMG_CMD "$@" ) > =20 > Testing: amend -f qcow2 -o backing_file=3DTEST_DIR/t.qcow2 -o ,help T= EST_DIR/t.qcow2 > qemu-img: Invalid option list: ,help > +./common.config: line 117: 746 Segmentation fault (core dumped= ) ( exec $QEMU_IMG_CMD "$@" ) > =20 > Testing: amend -f qcow2 -o backing_file=3DTEST_DIR/t.qcow2 -o ,, -o h= elp TEST_DIR/t.qcow2 > qemu-img: Invalid option list: ,, > +./common.config: line 117: 756 Segmentation fault (core dumped= ) ( exec $QEMU_IMG_CMD "$@" ) > =20 > Testing: amend -f qcow2 -o help > Supported options >=20 >=20 > 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" >> =20 >> -_subshell_exec() >> -{ >> - # Executing crashing commands in a subshell prevents information = like the >> - # "Killed" line from being lost >> - (exec "$@") >> -} >> - >> size=3D128M >> =20 >> echo >> @@ -74,8 +67,8 @@ echo "=3D=3D Creating a dirty image file =3D=3D" >> IMGOPTS=3D"compat=3D1.1,lazy_refcounts=3Don" >> _make_test_img $size >> =20 >> -_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 >> =20 >> # The dirty bit must be set >> @@ -109,8 +102,8 @@ echo "=3D=3D Opening a dirty image read/write shou= ld repair it =3D=3D" >> IMGOPTS=3D"compat=3D1.1,lazy_refcounts=3Don" >> _make_test_img $size >> =20 >> -_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 >> =20 >> # The dirty bit must be set >> @@ -127,8 +120,8 @@ echo "=3D=3D Creating an image file with lazy_refc= ounts=3Doff =3D=3D" >> IMGOPTS=3D"compat=3D1.1,lazy_refcounts=3Doff" >> _make_test_img $size >> =20 >> -_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 >> =20 >> # The dirty bit must not be set since lazy_refcounts=3Doff >> 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=3DIMGFMT size=3D134217728 >> 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=3D0 reference=3D1 >> ERROR OFLAG_COPIED data cluster: l2_entry=3D8000000000050000 refcount= =3D0 >> @@ -46,7 +46,7 @@ read 512/512 bytes at offset 0 >> Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D134217728 >> 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=3D0 reference=3D1 >> Rebuilding refcount structure >> @@ -60,7 +60,7 @@ incompatible_features 0x0 >> Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D134217728 >> 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. >> =20 >> 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 "=3D=3D=3D Testing dirty version downgrade =3D=3D=3D" >> echo >> IMGOPTS=3D"compat=3D1.1,lazy_refcounts=3Don" _make_test_img 64M >> -$QEMU_IO -c "write -P 0x2a 0 128k" -c flush -c abort "$TEST_IMG" | _f= ilter_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=3D0.10" "$TEST_IMG" >> $PYTHON qcow2.py "$TEST_IMG" dump-header >> @@ -91,7 +92,8 @@ echo >> echo "=3D=3D=3D Testing dirty lazy_refcounts=3Doff =3D=3D=3D" >> echo >> IMGOPTS=3D"compat=3D1.1,lazy_refcounts=3Don" _make_test_img 64M >> -$QEMU_IO -c "write -P 0x2a 0 128k" -c flush -c abort "$TEST_IMG" | _f= ilter_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=3Doff" "$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=3DIMGFMT size=3D67108864 >> 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_I= O_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=3DIMGFMT size=3D67108864 >> 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_I= O_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=3D`_full_platform_details` >> #FULL_MOUNT_OPTIONS=3D`_scratch_mount_options` >> =20 >> cat <> -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/com= mon.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=3D"`set_prog_path qemu-nbd`" >> fi >> =20 >> -export QEMU=3D"$QEMU_PROG $QEMU_OPTIONS" >> -export QEMU_IMG=3D$QEMU_IMG_PROG >> -export QEMU_IO=3D"$QEMU_IO_PROG $QEMU_IO_OPTIONS" >> -export QEMU_NBD=3D$QEMU_NBD_PROG >> +export QEMU_CMD=3D"$QEMU_PROG $QEMU_OPTIONS" >> +export QEMU_IMG_CMD=3D$QEMU_IMG_PROG >> +export QEMU_IO_CMD=3D"$QEMU_IO_PROG $QEMU_IO_OPTIONS" >> +export QEMU_NBD_CMD=3D$QEMU_NBD_PROG >> + >=20 > 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! >=20 > I think we want to drop the _OPTIONS in each of the exports, e.g.: >=20 > -export QEMU=3D"$QEMU_PROG $QEMU_OPTIONS" > +export QEMU_CMD=3D"$QEMU_PROG" >=20 > And then instead of this: >=20 >> +_qemu_wrapper() >> +{ >> + (exec $QEMU_CMD "$@") >> +} >> + >=20 > Use this form, instead: >=20 > +_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 "$@") >> +} >> + >=20 > Repeat as appropriate, above. >=20 >> +export QEMU=3D_qemu_wrapper >> +export QEMU_IMG=3D_qemu_img_wrapper >> +export QEMU_IO=3D_qemu_io_wrapper >> +export QEMU_NBD=3D_qemu_nbd_wrapper >> + >> default_machine=3D$($QEMU -machine \? | awk '/(default)/{print $1}') >> default_alias_machine=3D$($QEMU -machine \? |\ >> awk -v var_default_machine=3D"$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=3D\$$1 >> + if [ "$1" =3D "QEMU" ]; then >> + c=3D$QEMU_PROG >> + elif [ "$1" =3D "QEMU_IMG" ]; then >> + c=3D$QEMU_IMG_PROG >> + elif [ "$1" =3D "QEMU_IO" ]; then >> + c=3D$QEMU_IO_PROG >> + elif [ "$1" =3D "QEMU_NBD" ]; then >> + c=3D$QEMU_NBD_PROG >> + else >> + eval c=3D\$$1 >> + fi >> [ -x "$c" ] || _notrun "$1 utility required, skipped this test" >> } >> =20 >> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotest= s.py >> index 5579253..927c74a 100644 >> --- a/tests/qemu-iotests/iotests.py >> +++ b/tests/qemu-iotests/iotests.py >> @@ -33,9 +33,9 @@ __all__ =3D ['imgfmt', 'imgproto', 'test_dir' 'qemu_= img', 'qemu_io', >> =20 >> # This will not work if arguments or path contain spaces but is neces= sary if we >> # want to support the override options that ./check supports. >> -qemu_img_args =3D os.environ.get('QEMU_IMG', 'qemu-img').strip().spli= t(' ') >> -qemu_io_args =3D os.environ.get('QEMU_IO', 'qemu-io').strip().split('= ') >> -qemu_args =3D os.environ.get('QEMU', 'qemu').strip().split(' ') >> +qemu_img_args =3D os.environ.get('QEMU_IMG_CMD', 'qemu-img').strip().= split(' ') >> +qemu_io_args =3D os.environ.get('QEMU_IO_CMD', 'qemu-io').strip().spl= it(' ') >> +qemu_args =3D os.environ.get('QEMU_CMD', 'qemu').strip().split(' ') >> =20 >> imgfmt =3D os.environ.get('IMGFMT', 'raw') >> imgproto =3D os.environ.get('IMGPROTO', 'file') >> --=20 >> 2.5.0 >> >> --PgWDf6D4eD9H5LFCrMTqEKiuF7r3PhFem Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJV5xE1AAoJEDuxQgLoOKytQfkIAKkx3e3oS5KuXHQMWZLvELNY J/eCfiKPH1oi758/oMYq4Jz/EAT2H/OL+ibhFMR/4ua+OISVpci8H4IMiE6+Vdtq oCkkzGV8EKQc79UVVxdzKupTTLUW6imau6rSV3Wa67hqVzJMoft1SsdZq8g2EkWn xCUobxnYQyJC/mzk89gKLWCyKt/Xj68VQ67iUh8TnTYVIPSgQLxhbwCD9xYO9wTI 6pSdWLLhxMJarBwWzKN1Hv7v57/x1TyJbHdDjf64daT9dVcPjmdPJ0u8k2/4KA9p IFp12gP4IARN0NSL4tiq9cNMciDHX7SRba6//I6X7qINuCB9ILsPSG7RBvwdn1w= =Rh1Y -----END PGP SIGNATURE----- --PgWDf6D4eD9H5LFCrMTqEKiuF7r3PhFem--