From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39156) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZsE7i-00025D-Tn for qemu-devel@nongnu.org; Fri, 30 Oct 2015 14:09:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZsE7h-0005nQ-M8 for qemu-devel@nongnu.org; Fri, 30 Oct 2015 14:09:58 -0400 References: <3e718db40af2d521420a3431020bd0395541aed6.1446139820.git.jcody@redhat.com> <5633A5ED.70304@redhat.com> <20151030180411.GC2628@localhost.localdomain> From: Max Reitz Message-ID: <5633B266.6000207@redhat.com> Date: Fri, 30 Oct 2015 19:09:42 +0100 MIME-Version: 1.0 In-Reply-To: <20151030180411.GC2628@localhost.localdomain> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="upgNgO8aIDwLgQ1DKSHI7gO7KterlwCR0" Subject: Re: [Qemu-devel] [PATCH] qemu-iotests: fix -valgrind option for check List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jeff Cody Cc: kwolf@redhat.com, jsnow@redhat.com, qemu-devel@nongnu.org, qemu-block@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --upgNgO8aIDwLgQ1DKSHI7gO7KterlwCR0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable On 30.10.2015 19:04, Jeff Cody wrote: > On Fri, Oct 30, 2015 at 06:16:29PM +0100, Max Reitz wrote: >> On 29.10.2015 19:04, Jeff Cody wrote: >>> Commit 934659c switched the iotests to run qemu-io from a bash subshe= ll, >>> in order to catch segfaults. This method is incompatible with the >>> current valgrind_qemu_io() bash function. >>> >>> Move the valgrind usage into the exec subshell in _qemu_io_wrapper(),= >>> while making sure the original return value is passed back to the >>> caller. >>> >>> Update test output for tests 039, 061, and 137 as it looks for the >>> specific subshell command when the process is terminated. >>> >>> Reported-by: Kevin Wolf >>> Signed-off-by: Jeff Cody >>> --- >>> tests/qemu-iotests/039.out | 30 +++++++++++++++++++++++++-----= >>> tests/qemu-iotests/061.out | 12 ++++++++++-- >>> tests/qemu-iotests/137.out | 6 +++++- >>> tests/qemu-iotests/common | 9 ++------- >>> tests/qemu-iotests/common.config | 18 +++++++++++++++++- >>> tests/qemu-iotests/common.rc | 10 ---------- >>> 6 files changed, 59 insertions(+), 26 deletions(-) >>> [...] >>> diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/co= mmon.config >>> index 4d8665f..db9702b 100644 >>> --- a/tests/qemu-iotests/common.config >>> +++ b/tests/qemu-iotests/common.config >>> @@ -122,7 +122,23 @@ _qemu_img_wrapper() >>> =20 >>> _qemu_io_wrapper() >>> { >>> - (exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@") >>> + local VALGRIND_LOGFILE=3D/tmp/$$.valgrind >>> + local RETVAL >>> + ( >>> + if [ "${VALGRIND_QEMU}" =3D=3D "y" ]; then >>> + exec valgrind --log-file=3D"${VALGRIND_LOGFILE}" --error= -exitcode=3D99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" >>> + else >>> + exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" >>> + fi >>> + ) >>> + RETVAL=3D$? >> >> Er, well, this is nice... When just invoking $QEMU_IO -c 'sigraise 9',= I >> get the appropriate error message. ("$PID Killed [...]"). But the >> instant an image is opened, it just disappears. Yes, using -c open mak= es >> it disappear, too. >> >> Since all of our qemu-io invocations do use image files (that's its >> purpose after all), that means that all of them seem to exit just fine= >> when running under valgrind. That is... strange. >> >> Is it just me? Maybe I have a broken valgrind, I don't know (3.11.0 he= re). >> >=20 > I have valgrind-3.9.0 here, and I get the same behavior... it is not > just you. There are also some tests where valgrind itself segfaults. >=20 > I was going to suggest using kill -l TERM instead of kill -l KILL. > However, I just tried that with test 137, and it causes valgrind to > segfault. :( Hm, well, then I guess we can just ignore this and declare it broken. If someone complains, it'll be his/her job to fix it. ;-) >>> + if [ "${VALGRIND_QEMU}" =3D=3D "y" ]; then >>> + if [ $RETVAL !=3D 0 ]; then >>> + cat "${VALGRIND_LOGFILE}" >> >> If I got the error message and RETVAL would be correctly set to 137, >> this would print the log file. I'm not sure whether that's what we >> want...? If valgrind exits with any error code but 99, the log file wi= ll >> probably not contain anything interesting. >> >> But if the qemu-io process was killed on purpose, this breaks the test= , >> which I don't think is necessary. >> >=20 > Good point... How about just: >=20 > if [ $RETVAL =3D=3D 99 ]; then > cat "${VALGRIND_LOGFILE}" > fi Yes, that's what I had in mind. Max >>> + fi >>> + rm -f "${VALGRIND_LOGFILE}" >>> + fi >>> + (exit $RETVAL) >>> } >>> =20 >>> _qemu_nbd_wrapper() >>> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common= =2Erc >>> index 4878e99..d9913f8 100644 >>> --- a/tests/qemu-iotests/common.rc >>> +++ b/tests/qemu-iotests/common.rc >>> @@ -70,16 +70,6 @@ else >>> TEST_IMG=3D$IMGPROTO:$TEST_DIR/t.$IMGFMT >>> fi >>> =20 >>> -function valgrind_qemu_io() >>> -{ >>> - valgrind --log-file=3D/tmp/$$.valgrind --error-exitcode=3D99 $RE= AL_QEMU_IO "$@" >>> - if [ $? !=3D 0 ]; then >>> - cat /tmp/$$.valgrind >>> - fi >>> - rm -f /tmp/$$.valgrind >>> -} >>> - >>> - >>> _optstr_add() >>> { >>> if [ -n "$1" ]; then >>> >> >> >=20 >=20 --upgNgO8aIDwLgQ1DKSHI7gO7KterlwCR0 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 iQEcBAEBCAAGBQJWM7JmAAoJEDuxQgLoOKytjYMH/2nJgz1kR/PSYT3h2z35Gk6k DB5vEdNKCM5uQ2NNCwtVX6choba6Km7plPoJ4H83rYvq0rSzdySAMFNxccTMTe3y p7hYZmtAJOuIKMTKJgj27lLLy49VphYWS5a/7LDMP3a2G3pBOSEFKhGpBPKFYzkW OQuiaFiX7XweYDbSGMLrDjFWKxvdD4y8gmI+HLsYyD/AllMUe2VmQ9+QxYoBqi6x m2s3KGQieDDv9gVXEjaizzIY5vB92UzK0rjrdlHzB3pe0nUdMUyNmrv+1JMJqBHN moq3i0o80vvS8CltuuFguAXyXoy1ocFDwidqjWargZW1ZEyQhyLdIyOg2Iwvr3k= =OdOY -----END PGP SIGNATURE----- --upgNgO8aIDwLgQ1DKSHI7gO7KterlwCR0--