diff mbox

qemu-iotests: fix -valgrind option for check

Message ID 3e718db40af2d521420a3431020bd0395541aed6.1446139820.git.jcody@redhat.com
State New
Headers show

Commit Message

Jeff Cody Oct. 29, 2015, 6:04 p.m. UTC
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 <kwolf@redhat.com>
Signed-off-by: Jeff Cody <jcody@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(-)

Comments

Max Reitz Oct. 30, 2015, 5:16 p.m. UTC | #1
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 <kwolf@redhat.com>
> Signed-off-by: Jeff Cody <jcody@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).

> +    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.

Max

> +        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
>
Jeff Cody Oct. 30, 2015, 6:04 p.m. UTC | #2
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 <kwolf@redhat.com>
> > Signed-off-by: Jeff Cody <jcody@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
> > 
> 
>
Max Reitz Oct. 30, 2015, 6:09 p.m. UTC | #3
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 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 <kwolf@redhat.com>
>>> Signed-off-by: Jeff Cody <jcody@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/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. :(

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}" == "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

Yes, that's what I had in mind.

Max

>>> +        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
>>>
>>
>>
> 
>
diff mbox

Patch

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=$?
+    if [ "${VALGRIND_QEMU}" == "y" ]; then
+        if [ $RETVAL != 0 ]; then
+            cat "${VALGRIND_LOGFILE}"
+        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