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 subshell,
> > 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 <kw...@redhat.com>
> > Signed-off-by: Jeff Cody <jc...@redhat.com>
> > ---
> > 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/039.out b/tests/qemu-iotests/039.out
> > index 03a31c5..32c8846 100644
> > --- a/tests/qemu-iotests/039.out
> > +++ b/tests/qemu-iotests/039.out
> > @@ -11,7 +11,11 @@ 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)
> > -./common.config: Killed ( exec "$QEMU_IO_PROG"
> > $QEMU_IO_OPTIONS "$@" )
> > +./common.config: Killed ( if [ "${VALGRIND_QEMU}" == "y"
> > ]; then
> > + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99
> > "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
> > +else
> > + exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
> > +fi )
> > incompatible_features 0x1
> > ERROR cluster 5 refcount=0 reference=1
> > ERROR OFLAG_COPIED data cluster: l2_entry=8000000000050000 refcount=0
> > @@ -46,7 +50,11 @@ 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)
> > -./common.config: Killed ( exec "$QEMU_IO_PROG"
> > $QEMU_IO_OPTIONS "$@" )
> > +./common.config: Killed ( if [ "${VALGRIND_QEMU}" == "y"
> > ]; then
> > + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99
> > "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
> > +else
> > + exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
> > +fi )
> > incompatible_features 0x1
> > ERROR cluster 5 refcount=0 reference=1
> > Rebuilding refcount structure
> > @@ -60,7 +68,11 @@ 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)
> > -./common.config: Killed ( exec "$QEMU_IO_PROG"
> > $QEMU_IO_OPTIONS "$@" )
> > +./common.config: Killed ( if [ "${VALGRIND_QEMU}" == "y"
> > ]; then
> > + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99
> > "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
> > +else
> > + exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
> > +fi )
> > incompatible_features 0x0
> > No errors were found on the image.
> >
> > @@ -79,7 +91,11 @@ 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)
> > -./common.config: Killed ( exec "$QEMU_IO_PROG"
> > $QEMU_IO_OPTIONS "$@" )
> > +./common.config: Killed ( if [ "${VALGRIND_QEMU}" == "y"
> > ]; then
> > + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99
> > "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
> > +else
> > + exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
> > +fi )
> > incompatible_features 0x1
> > ERROR cluster 5 refcount=0 reference=1
> > ERROR OFLAG_COPIED data cluster: l2_entry=8000000000050000 refcount=0
> > @@ -89,7 +105,11 @@ Data may be corrupted, or further writes to the image
> > may corrupt it.
> > 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)
> > -./common.config: Killed ( exec "$QEMU_IO_PROG"
> > $QEMU_IO_OPTIONS "$@" )
> > +./common.config: Killed ( if [ "${VALGRIND_QEMU}" == "y"
> > ]; then
> > + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99
> > "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
> > +else
> > + exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
> > +fi )
> > incompatible_features 0x0
> > No errors were found on the image.
> > *** done
> > diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
> > index b16bea9..f2598a8 100644
> > --- a/tests/qemu-iotests/061.out
> > +++ b/tests/qemu-iotests/061.out
> > @@ -57,7 +57,11 @@ 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: Killed ( exec "$QEMU_IO_PROG"
> > $QEMU_IO_OPTIONS "$@" )
> > +./common.config: Killed ( if [ "${VALGRIND_QEMU}" == "y"
> > ]; then
> > + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99
> > "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
> > +else
> > + exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
> > +fi )
> > magic 0x514649fb
> > version 3
> > backing_file_offset 0x0
> > @@ -215,7 +219,11 @@ 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: Killed ( exec "$QEMU_IO_PROG"
> > $QEMU_IO_OPTIONS "$@" )
> > +./common.config: Killed ( if [ "${VALGRIND_QEMU}" == "y"
> > ]; then
> > + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99
> > "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
> > +else
> > + exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
> > +fi )
> > magic 0x514649fb
> > version 3
> > backing_file_offset 0x0
> > diff --git a/tests/qemu-iotests/137.out b/tests/qemu-iotests/137.out
> > index cf55a41..88c702c 100644
> > --- a/tests/qemu-iotests/137.out
> > +++ b/tests/qemu-iotests/137.out
> > @@ -31,7 +31,11 @@ Cache clean interval too big
> > Unsupported value 'blubb' for qcow2 option 'overlap-check'. Allowed are
> > any of the following: none, constant, cached, all
> > wrote 512/512 bytes at offset 0
> > 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> > -./common.config: Killed ( exec "$QEMU_IO_PROG"
> > $QEMU_IO_OPTIONS "$@" )
> > +./common.config: Killed ( if [ "${VALGRIND_QEMU}" == "y"
> > ]; then
> > + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99
> > "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
> > +else
> > + exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
> > +fi )
> > incompatible_features 0x0
> > Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
> > wrote 65536/65536 bytes at offset 0
> > diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common
> > index 25c351b..ff84f4b 100644
> > --- a/tests/qemu-iotests/common
> > +++ b/tests/qemu-iotests/common
> > @@ -41,7 +41,6 @@ sortme=false
> > expunge=true
> > have_test_arg=false
> > randomize=false
> > -valgrind=false
> > cachemode=false
> > rm -f $tmp.list $tmp.tmp $tmp.sed
> >
> > @@ -53,6 +52,7 @@ export CACHEMODE="writeback"
> > export QEMU_IO_OPTIONS=""
> > export CACHEMODE_IS_DEFAULT=true
> > export QEMU_OPTIONS="-nodefaults"
> > +export VALGRIND_QEMU=
> >
> > for r
> > do
> > @@ -278,7 +278,7 @@ testlist options
> > ;;
> >
> > -valgrind)
> > - valgrind=true
> > + VALGRIND_QEMU='y'
> > xpand=false
> > ;;
> >
> > @@ -436,8 +436,3 @@ fi
> > if [ "$IMGPROTO" = "nbd" ] ; then
> > [ "$QEMU_NBD" = "" ] && _fatal "qemu-nbd not found"
> > fi
> > -
> > -if $valgrind; then
> > - export REAL_QEMU_IO="$QEMU_IO_PROG"
> > - export QEMU_IO_PROG=valgrind_qemu_io
> > -fi
> > diff --git a/tests/qemu-iotests/common.config
> > b/tests/qemu-iotests/common.config
> > index 4d8665f..db9702b 100644
> > --- a/tests/qemu-iotests/common.config
> > +++ b/tests/qemu-iotests/common.config
> > @@ -122,7 +122,23 @@ _qemu_img_wrapper()
> >
> > _qemu_io_wrapper()
> > {
> > - (exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@")
> > + local VALGRIND_LOGFILE=/tmp/$$.valgrind
> > + local RETVAL
> > + (
> > + if [ "${VALGRIND_QEMU}" == "y" ]; then
> > + exec valgrind --log-file="${VALGRIND_LOGFILE}"
> > --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"
> > + else
> > + exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"
> > + fi
> > + )
> > + RETVAL=$?
>
> 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 makes
> 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 here).
>
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.
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. :(
> > + if [ "${VALGRIND_QEMU}" == "y" ]; then
> > + if [ $RETVAL != 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 will
> 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.
>
Good point... How about just:
if [ $RETVAL == 99 ]; then
cat "${VALGRIND_LOGFILE}"
fi
> > + fi
> > + rm -f "${VALGRIND_LOGFILE}"
> > + fi
> > + (exit $RETVAL)
> > }
> >
> > _qemu_nbd_wrapper()
> > diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
> > index 4878e99..d9913f8 100644
> > --- a/tests/qemu-iotests/common.rc
> > +++ b/tests/qemu-iotests/common.rc
> > @@ -70,16 +70,6 @@ else
> > TEST_IMG=$IMGPROTO:$TEST_DIR/t.$IMGFMT
> > fi
> >
> > -function valgrind_qemu_io()
> > -{
> > - valgrind --log-file=/tmp/$$.valgrind --error-exitcode=99 $REAL_QEMU_IO
> > "$@"
> > - if [ $? != 0 ]; then
> > - cat /tmp/$$.valgrind
> > - fi
> > - rm -f /tmp/$$.valgrind
> > -}
> > -
> > -
> > _optstr_add()
> > {
> > if [ -n "$1" ]; then
> >
>
>
