With the '-valgrind' option, let all the QEMU processes be run under
the Valgrind tool. The Valgrind own parameters may be set with its
environment variable VALGRIND_OPTS, e.g.
VALGRIND_OPTS="--leak-check=yes" ./check -qcow2 -valgrind <test#>
or they may be listed in the Valgrind checked file ./.valgrindrc or
~/.valgrindrc like
--memcheck:leak-check=no
--memcheck:track-origins=yes
When QEMU-IO process is being killed, the shell report refers to the
text of the command in _qemu_io_wrapper(), which was modified with this
patch. So, the benchmark output for the tests 039, 061 and 137 is to be
changed also.
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
tests/qemu-iotests/039.out | 30 ++++---------------
tests/qemu-iotests/061.out | 12 ++------
tests/qemu-iotests/137.out | 6 +---
tests/qemu-iotests/common.rc | 69 ++++++++++++++++++++++++++++++++------------
4 files changed, 59 insertions(+), 58 deletions(-)
diff --git a/tests/qemu-iotests/039.out b/tests/qemu-iotests/039.out
index 724d7b2..972c6c0 100644
--- a/tests/qemu-iotests/039.out
+++ b/tests/qemu-iotests/039.out
@@ -11,11 +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)
-./common.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
- exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
-else
- exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
-fi )
+./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
incompatible_features 0x1
ERROR cluster 5 refcount=0 reference=1
ERROR OFLAG_COPIED data cluster: l2_entry=8000000000050000 refcount=0
@@ -50,11 +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)
-./common.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
- exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
-else
- exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
-fi )
+./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
incompatible_features 0x1
ERROR cluster 5 refcount=0 reference=1
Rebuilding refcount structure
@@ -68,11 +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)
-./common.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
- exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
-else
- exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
-fi )
+./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
incompatible_features 0x0
No errors were found on the image.
@@ -91,11 +79,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)
-./common.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
- exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
-else
- exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
-fi )
+./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
incompatible_features 0x1
ERROR cluster 5 refcount=0 reference=1
ERROR OFLAG_COPIED data cluster: l2_entry=8000000000050000 refcount=0
@@ -105,11 +89,7 @@ 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.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
- exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
-else
- exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
-fi )
+./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
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 1aa7d37..8cb57eb 100644
--- a/tests/qemu-iotests/061.out
+++ b/tests/qemu-iotests/061.out
@@ -118,11 +118,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.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
- exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
-else
- exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
-fi )
+./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
magic 0x514649fb
version 3
backing_file_offset 0x0
@@ -280,11 +276,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.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
- exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
-else
- exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
-fi )
+./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
magic 0x514649fb
version 3
backing_file_offset 0x0
diff --git a/tests/qemu-iotests/137.out b/tests/qemu-iotests/137.out
index 22d59df..7fed5e6 100644
--- a/tests/qemu-iotests/137.out
+++ b/tests/qemu-iotests/137.out
@@ -35,11 +35,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
qemu-io: 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.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
- exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
-else
- exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
-fi )
+./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
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.rc b/tests/qemu-iotests/common.rc
index 5502c3d..6e461a1 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -60,19 +60,52 @@ if ! . ./common.config
exit 1
fi
+_qemu_proc_wrapper()
+{
+ local VALGRIND_LOGFILE="$1"
+ shift
+ if [ "${VALGRIND_QEMU}" == "y" ]; then
+ exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$@"
+ else
+ exec "$@"
+ fi
+}
+
+_qemu_proc_valgrind_log()
+{
+ local VALGRIND_LOGFILE="$1"
+ local RETVAL="$2"
+ if [ "${VALGRIND_QEMU}" == "y" ]; then
+ if [ $RETVAL == 99 ]; then
+ cat "${VALGRIND_LOGFILE}"
+ fi
+ rm -f "${VALGRIND_LOGFILE}"
+ fi
+}
+
_qemu_wrapper()
{
+ local VALGRIND_LOGFILE="${TEST_DIR}"/$$.valgrind
(
if [ -n "${QEMU_NEED_PID}" ]; then
echo $BASHPID > "${QEMU_TEST_DIR}/qemu-${_QEMU_HANDLE}.pid"
fi
- exec "$QEMU_PROG" $QEMU_OPTIONS "$@"
+ _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_PROG" $QEMU_OPTIONS "$@"
)
+ RETVAL=$?
+ _qemu_proc_valgrind_log "${VALGRIND_LOGFILE}" $RETVAL
+ return $RETVAL
}
_qemu_img_wrapper()
{
- (exec "$QEMU_IMG_PROG" $QEMU_IMG_OPTIONS "$@")
+ local VALGRIND_LOGFILE="${TEST_DIR}"/$$.valgrind
+ (
+ _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IMG_PROG" $QEMU_IMG_OPTIONS "$@"
+ )
+ RETVAL=$?
+ _qemu_proc_valgrind_log "${VALGRIND_LOGFILE}" $RETVAL
+ return $RETVAL
}
_qemu_io_wrapper()
@@ -85,36 +118,36 @@ _qemu_io_wrapper()
QEMU_IO_ARGS="--object secret,id=keysec0,data=$IMGKEYSECRET $QEMU_IO_ARGS"
fi
fi
- local RETVAL
(
- if [ "${VALGRIND_QEMU}" == "y" ]; then
- exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"
- else
- exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"
- fi
+ _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"
)
RETVAL=$?
- if [ "${VALGRIND_QEMU}" == "y" ]; then
- if [ $RETVAL == 99 ]; then
- cat "${VALGRIND_LOGFILE}"
- fi
- rm -f "${VALGRIND_LOGFILE}"
- fi
- (exit $RETVAL)
+ _qemu_proc_valgrind_log "${VALGRIND_LOGFILE}" $RETVAL
+ return $RETVAL
}
_qemu_nbd_wrapper()
{
- "$QEMU_NBD_PROG" --pid-file="${QEMU_TEST_DIR}/qemu-nbd.pid" \
- $QEMU_NBD_OPTIONS "$@"
+ local VALGRIND_LOGFILE="${TEST_DIR}"/$$.valgrind
+ (
+ _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_NBD_PROG" \
+ --pid-file="${QEMU_TEST_DIR}/qemu-nbd.pid" $QEMU_NBD_OPTIONS "$@"
+ )
+ RETVAL=$?
+ _qemu_proc_valgrind_log "${VALGRIND_LOGFILE}" $RETVAL
+ return $RETVAL
}
_qemu_vxhs_wrapper()
{
+ local VALGRIND_LOGFILE="${TEST_DIR}"/$$.valgrind
(
echo $BASHPID > "${TEST_DIR}/qemu-vxhs.pid"
- exec "$QEMU_VXHS_PROG" $QEMU_VXHS_OPTIONS "$@"
+ _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_VXHS_PROG" $QEMU_VXHS_OPTIONS "$@"
)
+ RETVAL=$?
+ _qemu_proc_valgrind_log "${VALGRIND_LOGFILE}" $RETVAL
+ return $RETVAL
}
export QEMU=_qemu_wrapper
--
1.8.3.1
On 7/19/19 12:30 PM, Andrey Shinkevich wrote:
> With the '-valgrind' option, let all the QEMU processes be run under
> the Valgrind tool. The Valgrind own parameters may be set with its
> environment variable VALGRIND_OPTS, e.g.
> VALGRIND_OPTS="--leak-check=yes" ./check -qcow2 -valgrind <test#>
> or they may be listed in the Valgrind checked file ./.valgrindrc or
> ~/.valgrindrc like
> --memcheck:leak-check=no
> --memcheck:track-origins=yes
> When QEMU-IO process is being killed, the shell report refers to the
> text of the command in _qemu_io_wrapper(), which was modified with this
> patch. So, the benchmark output for the tests 039, 061 and 137 is to be
> changed also.
>
Oh, weird. "VALGRIND_QEMU=y" actually has just meant ... valgrind
qemu-io. OK.
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
> tests/qemu-iotests/039.out | 30 ++++---------------
> tests/qemu-iotests/061.out | 12 ++------
> tests/qemu-iotests/137.out | 6 +---
> tests/qemu-iotests/common.rc | 69 ++++++++++++++++++++++++++++++++------------
> 4 files changed, 59 insertions(+), 58 deletions(-)
>
> diff --git a/tests/qemu-iotests/039.out b/tests/qemu-iotests/039.out
> index 724d7b2..972c6c0 100644
> --- a/tests/qemu-iotests/039.out
> +++ b/tests/qemu-iotests/039.out
> @@ -11,11 +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)
> -./common.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
> -else
> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
> -fi )
> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
> incompatible_features 0x1
> ERROR cluster 5 refcount=0 reference=1
> ERROR OFLAG_COPIED data cluster: l2_entry=8000000000050000 refcount=0
> @@ -50,11 +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)
> -./common.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
> -else
> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
> -fi )
> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
> incompatible_features 0x1
> ERROR cluster 5 refcount=0 reference=1
> Rebuilding refcount structure
> @@ -68,11 +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)
> -./common.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
> -else
> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
> -fi )
> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
> incompatible_features 0x0
> No errors were found on the image.
>
> @@ -91,11 +79,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)
> -./common.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
> -else
> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
> -fi )
> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
> incompatible_features 0x1
> ERROR cluster 5 refcount=0 reference=1
> ERROR OFLAG_COPIED data cluster: l2_entry=8000000000050000 refcount=0
> @@ -105,11 +89,7 @@ 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.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
> -else
> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
> -fi )
> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
> 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 1aa7d37..8cb57eb 100644
> --- a/tests/qemu-iotests/061.out
> +++ b/tests/qemu-iotests/061.out
> @@ -118,11 +118,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.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
> -else
> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
> -fi )
> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
> magic 0x514649fb
> version 3
> backing_file_offset 0x0
> @@ -280,11 +276,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.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
> -else
> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
> -fi )
> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
> magic 0x514649fb
> version 3
> backing_file_offset 0x0
> diff --git a/tests/qemu-iotests/137.out b/tests/qemu-iotests/137.out
> index 22d59df..7fed5e6 100644
> --- a/tests/qemu-iotests/137.out
> +++ b/tests/qemu-iotests/137.out
> @@ -35,11 +35,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
> qemu-io: 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.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
> -else
> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
> -fi )
> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
> 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.rc b/tests/qemu-iotests/common.rc
> index 5502c3d..6e461a1 100644
> --- a/tests/qemu-iotests/common.rc
> +++ b/tests/qemu-iotests/common.rc
> @@ -60,19 +60,52 @@ if ! . ./common.config
> exit 1
> fi
>
> +_qemu_proc_wrapper()
> +{
> + local VALGRIND_LOGFILE="$1"
> + shift
> + if [ "${VALGRIND_QEMU}" == "y" ]; then
> + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$@"
> + else
> + exec "$@"
> + fi
> +}
> +
Why do we need a second wrapper? I get nervous with each new wrapper we
make, because I feel like it has unintended consequences with pipe
handling and so on in the test dispatcher scripts.
> +_qemu_proc_valgrind_log()
> +{
> + local VALGRIND_LOGFILE="$1"
> + local RETVAL="$2"
> + if [ "${VALGRIND_QEMU}" == "y" ]; then
> + if [ $RETVAL == 99 ]; then
> + cat "${VALGRIND_LOGFILE}"
> + fi
> + rm -f "${VALGRIND_LOGFILE}"
> + fi
> +}
> +
> _qemu_wrapper()
> {
> + local VALGRIND_LOGFILE="${TEST_DIR}"/$$.valgrind
> (
> if [ -n "${QEMU_NEED_PID}" ]; then
> echo $BASHPID > "${QEMU_TEST_DIR}/qemu-${_QEMU_HANDLE}.pid"
> fi
> - exec "$QEMU_PROG" $QEMU_OPTIONS "$@"
> + _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_PROG" $QEMU_OPTIONS "$@"
Can we not inline that logic here? especially because:
> )
> + RETVAL=$?
> + _qemu_proc_valgrind_log "${VALGRIND_LOGFILE}" $RETVAL
We're already making valgrind calls here anyway.
> + return $RETVAL
> }
>
> _qemu_img_wrapper()
> {
> - (exec "$QEMU_IMG_PROG" $QEMU_IMG_OPTIONS "$@")
> + local VALGRIND_LOGFILE="${TEST_DIR}"/$$.valgrind
> + (
> + _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IMG_PROG" $QEMU_IMG_OPTIONS "$@"
> + )
> + RETVAL=$?
> + _qemu_proc_valgrind_log "${VALGRIND_LOGFILE}" $RETVAL
> + return $RETVAL
> }
>
> _qemu_io_wrapper()
> @@ -85,36 +118,36 @@ _qemu_io_wrapper()
> QEMU_IO_ARGS="--object secret,id=keysec0,data=$IMGKEYSECRET $QEMU_IO_ARGS"
> fi
> fi
> - local RETVAL
> (
> - if [ "${VALGRIND_QEMU}" == "y" ]; then
> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"
> - else
> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"
> - fi
> + _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"
> )
> RETVAL=$?
> - if [ "${VALGRIND_QEMU}" == "y" ]; then
> - if [ $RETVAL == 99 ]; then
> - cat "${VALGRIND_LOGFILE}"
> - fi
> - rm -f "${VALGRIND_LOGFILE}"
> - fi
> - (exit $RETVAL)
> + _qemu_proc_valgrind_log "${VALGRIND_LOGFILE}" $RETVAL
> + return $RETVAL
> }
>
> _qemu_nbd_wrapper()
> {
> - "$QEMU_NBD_PROG" --pid-file="${QEMU_TEST_DIR}/qemu-nbd.pid" \
> - $QEMU_NBD_OPTIONS "$@"
> + local VALGRIND_LOGFILE="${TEST_DIR}"/$$.valgrind
> + (
> + _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_NBD_PROG" \
> + --pid-file="${QEMU_TEST_DIR}/qemu-nbd.pid" $QEMU_NBD_OPTIONS "$@"
> + )
> + RETVAL=$?
> + _qemu_proc_valgrind_log "${VALGRIND_LOGFILE}" $RETVAL
> + return $RETVAL
> }
>
> _qemu_vxhs_wrapper()
> {
> + local VALGRIND_LOGFILE="${TEST_DIR}"/$$.valgrind
> (
> echo $BASHPID > "${TEST_DIR}/qemu-vxhs.pid"
> - exec "$QEMU_VXHS_PROG" $QEMU_VXHS_OPTIONS "$@"
> + _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_VXHS_PROG" $QEMU_VXHS_OPTIONS "$@"
> )
> + RETVAL=$?
> + _qemu_proc_valgrind_log "${VALGRIND_LOGFILE}" $RETVAL
> + return $RETVAL
> }
>
> export QEMU=_qemu_wrapper
>
Only other thought: at the moment, valgrind turns on valgrind for
qemu-io; this wraps many more tools and potentially slows down iotests a
lot. If there are problems, we might want easy access to turn /some/ but
not all of these options off again.
do we want to be able to specify which subprocesses we use valgrind on,
Perhaps with environment variables for fine-tuning?
QEMU_VALGRIND_QEMU_IO = "off"
QEMU_VALGRIND_QEMU = "off"
QEMU_VALGRIND_NBD = "off"
(Only an idle question, not necessary for this series IMO.)
On 16/08/2019 01:49, John Snow wrote:
>
>
> On 7/19/19 12:30 PM, Andrey Shinkevich wrote:
>> With the '-valgrind' option, let all the QEMU processes be run under
>> the Valgrind tool. The Valgrind own parameters may be set with its
>> environment variable VALGRIND_OPTS, e.g.
>> VALGRIND_OPTS="--leak-check=yes" ./check -qcow2 -valgrind <test#>
>> or they may be listed in the Valgrind checked file ./.valgrindrc or
>> ~/.valgrindrc like
>> --memcheck:leak-check=no
>> --memcheck:track-origins=yes
>> When QEMU-IO process is being killed, the shell report refers to the
>> text of the command in _qemu_io_wrapper(), which was modified with this
>> patch. So, the benchmark output for the tests 039, 061 and 137 is to be
>> changed also.
>>
>
> Oh, weird. "VALGRIND_QEMU=y" actually has just meant ... valgrind
> qemu-io. OK.
>
>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>> ---
>> tests/qemu-iotests/039.out | 30 ++++---------------
>> tests/qemu-iotests/061.out | 12 ++------
>> tests/qemu-iotests/137.out | 6 +---
>> tests/qemu-iotests/common.rc | 69 ++++++++++++++++++++++++++++++++------------
>> 4 files changed, 59 insertions(+), 58 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/039.out b/tests/qemu-iotests/039.out
>> index 724d7b2..972c6c0 100644
>> --- a/tests/qemu-iotests/039.out
>> +++ b/tests/qemu-iotests/039.out
>> @@ -11,11 +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)
>> -./common.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
>> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>> -else
>> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>> -fi )
>> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>> incompatible_features 0x1
>> ERROR cluster 5 refcount=0 reference=1
>> ERROR OFLAG_COPIED data cluster: l2_entry=8000000000050000 refcount=0
>> @@ -50,11 +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)
>> -./common.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
>> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>> -else
>> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>> -fi )
>> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>> incompatible_features 0x1
>> ERROR cluster 5 refcount=0 reference=1
>> Rebuilding refcount structure
>> @@ -68,11 +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)
>> -./common.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
>> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>> -else
>> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>> -fi )
>> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>> incompatible_features 0x0
>> No errors were found on the image.
>>
>> @@ -91,11 +79,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)
>> -./common.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
>> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>> -else
>> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>> -fi )
>> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>> incompatible_features 0x1
>> ERROR cluster 5 refcount=0 reference=1
>> ERROR OFLAG_COPIED data cluster: l2_entry=8000000000050000 refcount=0
>> @@ -105,11 +89,7 @@ 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.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
>> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>> -else
>> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>> -fi )
>> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>> 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 1aa7d37..8cb57eb 100644
>> --- a/tests/qemu-iotests/061.out
>> +++ b/tests/qemu-iotests/061.out
>> @@ -118,11 +118,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.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
>> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>> -else
>> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>> -fi )
>> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>> magic 0x514649fb
>> version 3
>> backing_file_offset 0x0
>> @@ -280,11 +276,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.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
>> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>> -else
>> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>> -fi )
>> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>> magic 0x514649fb
>> version 3
>> backing_file_offset 0x0
>> diff --git a/tests/qemu-iotests/137.out b/tests/qemu-iotests/137.out
>> index 22d59df..7fed5e6 100644
>> --- a/tests/qemu-iotests/137.out
>> +++ b/tests/qemu-iotests/137.out
>> @@ -35,11 +35,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
>> qemu-io: 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.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
>> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>> -else
>> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>> -fi )
>> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>> 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.rc b/tests/qemu-iotests/common.rc
>> index 5502c3d..6e461a1 100644
>> --- a/tests/qemu-iotests/common.rc
>> +++ b/tests/qemu-iotests/common.rc
>> @@ -60,19 +60,52 @@ if ! . ./common.config
>> exit 1
>> fi
>>
>> +_qemu_proc_wrapper()
>> +{
>> + local VALGRIND_LOGFILE="$1"
>> + shift
>> + if [ "${VALGRIND_QEMU}" == "y" ]; then
>> + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$@"
>> + else
>> + exec "$@"
>> + fi
>> +}
>> +
>
> Why do we need a second wrapper? I get nervous with each new wrapper we
> make, because I feel like it has unintended consequences with pipe
> handling and so on in the test dispatcher scripts.
>
The _qemu_proc_wrapper() is acctually the function rather than a new
wrapper. The suffix _wrapper can be changed to mislead a reader not. The
routine code originates from the _qemu_io_wrapper() to repeat the same
functionality for the QEMU processes other than qemu-io and to keep the
uniformity. It is also true for the function _qemu_proc_valgrind_log()
that dumps Valgrind reports in case of errors. The behavior is the same
to one in the _qemu_io_wrapper(). Actually, nothing new except that
_qemu_nbd_wrapper() gets the subshell exec to keep the uniformity.
Andrey
>> +_qemu_proc_valgrind_log()
>> +{
>> + local VALGRIND_LOGFILE="$1"
>> + local RETVAL="$2"
>> + if [ "${VALGRIND_QEMU}" == "y" ]; then
>> + if [ $RETVAL == 99 ]; then
>> + cat "${VALGRIND_LOGFILE}"
>> + fi
>> + rm -f "${VALGRIND_LOGFILE}"
>> + fi
>> +}
>> +
>> _qemu_wrapper()
>> {
>> + local VALGRIND_LOGFILE="${TEST_DIR}"/$$.valgrind
>> (
>> if [ -n "${QEMU_NEED_PID}" ]; then
>> echo $BASHPID > "${QEMU_TEST_DIR}/qemu-${_QEMU_HANDLE}.pid"
>> fi
>> - exec "$QEMU_PROG" $QEMU_OPTIONS "$@"
>> + _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_PROG" $QEMU_OPTIONS "$@"
>
> Can we not inline that logic here? especially because:
>
>> )
>> + RETVAL=$?
>> + _qemu_proc_valgrind_log "${VALGRIND_LOGFILE}" $RETVAL
>
> We're already making valgrind calls here anyway.
>
We can of cause, but we would repeat that for all other
_qemu_*_wrapper() functions as well. The _qemu_proc_wrapper() runs a
process under the Valgrind if VALGRIND_QEMU=y and the
_qemu_proc_valgrind_log() dumps Valgrind error reports, if any and
removes the Valgrind temporary log file. Again, the same pattern is used
as it is in the current implementation of _qemu_io_wrapper(). I have
just copied the functionality to all other QEMU wrapper functions.
Andrey
>> + return $RETVAL
>> }
>>
>> _qemu_img_wrapper()
>> {
>> - (exec "$QEMU_IMG_PROG" $QEMU_IMG_OPTIONS "$@")
>> + local VALGRIND_LOGFILE="${TEST_DIR}"/$$.valgrind
>> + (
>> + _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IMG_PROG" $QEMU_IMG_OPTIONS "$@"
>> + )
>> + RETVAL=$?
>> + _qemu_proc_valgrind_log "${VALGRIND_LOGFILE}" $RETVAL
>> + return $RETVAL
>> }
>>
>> _qemu_io_wrapper()
>> @@ -85,36 +118,36 @@ _qemu_io_wrapper()
>> QEMU_IO_ARGS="--object secret,id=keysec0,data=$IMGKEYSECRET $QEMU_IO_ARGS"
>> fi
>> fi
>> - local RETVAL
>> (
>> - if [ "${VALGRIND_QEMU}" == "y" ]; then
>> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"
>> - else
>> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"
>> - fi
>> + _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@"
>> )
>> RETVAL=$?
>> - if [ "${VALGRIND_QEMU}" == "y" ]; then
>> - if [ $RETVAL == 99 ]; then
>> - cat "${VALGRIND_LOGFILE}"
>> - fi
>> - rm -f "${VALGRIND_LOGFILE}"
>> - fi
>> - (exit $RETVAL)
>> + _qemu_proc_valgrind_log "${VALGRIND_LOGFILE}" $RETVAL
>> + return $RETVAL
>> }
>>
>> _qemu_nbd_wrapper()
>> {
>> - "$QEMU_NBD_PROG" --pid-file="${QEMU_TEST_DIR}/qemu-nbd.pid" \
>> - $QEMU_NBD_OPTIONS "$@"
>> + local VALGRIND_LOGFILE="${TEST_DIR}"/$$.valgrind
>> + (
>> + _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_NBD_PROG" \
>> + --pid-file="${QEMU_TEST_DIR}/qemu-nbd.pid" $QEMU_NBD_OPTIONS "$@"
>> + )
>> + RETVAL=$?
>> + _qemu_proc_valgrind_log "${VALGRIND_LOGFILE}" $RETVAL
>> + return $RETVAL
>> }
>>
>> _qemu_vxhs_wrapper()
>> {
>> + local VALGRIND_LOGFILE="${TEST_DIR}"/$$.valgrind
>> (
>> echo $BASHPID > "${TEST_DIR}/qemu-vxhs.pid"
>> - exec "$QEMU_VXHS_PROG" $QEMU_VXHS_OPTIONS "$@"
>> + _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_VXHS_PROG" $QEMU_VXHS_OPTIONS "$@"
>> )
>> + RETVAL=$?
>> + _qemu_proc_valgrind_log "${VALGRIND_LOGFILE}" $RETVAL
>> + return $RETVAL
>> }
>>
>> export QEMU=_qemu_wrapper
>>
>
> Only other thought: at the moment, valgrind turns on valgrind for
> qemu-io; this wraps many more tools and potentially slows down iotests a
> lot. If there are problems, we might want easy access to turn /some/ but
> not all of these options off again.
>
>
> do we want to be able to specify which subprocesses we use valgrind on,
> Perhaps with environment variables for fine-tuning?
>
> QEMU_VALGRIND_QEMU_IO = "off"
> QEMU_VALGRIND_QEMU = "off"
> QEMU_VALGRIND_NBD = "off"
>
> (Only an idle question, not necessary for this series IMO.)
>
--
With the best regards,
Andrey Shinkevich
On 8/25/19 11:26 AM, Andrey Shinkevich wrote:
>
>
> On 16/08/2019 01:49, John Snow wrote:
>>
>>
>> On 7/19/19 12:30 PM, Andrey Shinkevich wrote:
>>> With the '-valgrind' option, let all the QEMU processes be run under
>>> the Valgrind tool. The Valgrind own parameters may be set with its
>>> environment variable VALGRIND_OPTS, e.g.
>>> VALGRIND_OPTS="--leak-check=yes" ./check -qcow2 -valgrind <test#>
>>> or they may be listed in the Valgrind checked file ./.valgrindrc or
>>> ~/.valgrindrc like
>>> --memcheck:leak-check=no
>>> --memcheck:track-origins=yes
>>> When QEMU-IO process is being killed, the shell report refers to the
>>> text of the command in _qemu_io_wrapper(), which was modified with this
>>> patch. So, the benchmark output for the tests 039, 061 and 137 is to be
>>> changed also.
>>>
>>
>> Oh, weird. "VALGRIND_QEMU=y" actually has just meant ... valgrind
>> qemu-io. OK.
>>
>>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>>> ---
>>> tests/qemu-iotests/039.out | 30 ++++---------------
>>> tests/qemu-iotests/061.out | 12 ++------
>>> tests/qemu-iotests/137.out | 6 +---
>>> tests/qemu-iotests/common.rc | 69 ++++++++++++++++++++++++++++++++------------
>>> 4 files changed, 59 insertions(+), 58 deletions(-)
>>>
>>> diff --git a/tests/qemu-iotests/039.out b/tests/qemu-iotests/039.out
>>> index 724d7b2..972c6c0 100644
>>> --- a/tests/qemu-iotests/039.out
>>> +++ b/tests/qemu-iotests/039.out
>>> @@ -11,11 +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)
>>> -./common.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
>>> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>> -else
>>> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>> -fi )
>>> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>>> incompatible_features 0x1
>>> ERROR cluster 5 refcount=0 reference=1
>>> ERROR OFLAG_COPIED data cluster: l2_entry=8000000000050000 refcount=0
>>> @@ -50,11 +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)
>>> -./common.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
>>> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>> -else
>>> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>> -fi )
>>> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>>> incompatible_features 0x1
>>> ERROR cluster 5 refcount=0 reference=1
>>> Rebuilding refcount structure
>>> @@ -68,11 +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)
>>> -./common.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
>>> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>> -else
>>> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>> -fi )
>>> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>>> incompatible_features 0x0
>>> No errors were found on the image.
>>>
>>> @@ -91,11 +79,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)
>>> -./common.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
>>> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>> -else
>>> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>> -fi )
>>> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>>> incompatible_features 0x1
>>> ERROR cluster 5 refcount=0 reference=1
>>> ERROR OFLAG_COPIED data cluster: l2_entry=8000000000050000 refcount=0
>>> @@ -105,11 +89,7 @@ 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.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
>>> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>> -else
>>> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>> -fi )
>>> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>>> 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 1aa7d37..8cb57eb 100644
>>> --- a/tests/qemu-iotests/061.out
>>> +++ b/tests/qemu-iotests/061.out
>>> @@ -118,11 +118,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.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
>>> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>> -else
>>> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>> -fi )
>>> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>>> magic 0x514649fb
>>> version 3
>>> backing_file_offset 0x0
>>> @@ -280,11 +276,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.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
>>> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>> -else
>>> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>> -fi )
>>> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>>> magic 0x514649fb
>>> version 3
>>> backing_file_offset 0x0
>>> diff --git a/tests/qemu-iotests/137.out b/tests/qemu-iotests/137.out
>>> index 22d59df..7fed5e6 100644
>>> --- a/tests/qemu-iotests/137.out
>>> +++ b/tests/qemu-iotests/137.out
>>> @@ -35,11 +35,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
>>> qemu-io: 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.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
>>> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>> -else
>>> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>> -fi )
>>> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>>> 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.rc b/tests/qemu-iotests/common.rc
>>> index 5502c3d..6e461a1 100644
>>> --- a/tests/qemu-iotests/common.rc
>>> +++ b/tests/qemu-iotests/common.rc
>>> @@ -60,19 +60,52 @@ if ! . ./common.config
>>> exit 1
>>> fi
>>>
>>> +_qemu_proc_wrapper()
>>> +{
>>> + local VALGRIND_LOGFILE="$1"
>>> + shift
>>> + if [ "${VALGRIND_QEMU}" == "y" ]; then
>>> + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$@"
>>> + else
>>> + exec "$@"
>>> + fi
>>> +}
>>> +
>>
>> Why do we need a second wrapper? I get nervous with each new wrapper we
>> make, because I feel like it has unintended consequences with pipe
>> handling and so on in the test dispatcher scripts.
>>
>
> The _qemu_proc_wrapper() is acctually the function rather than a new
> wrapper. The suffix _wrapper can be changed to mislead a reader not. The
> routine code originates from the _qemu_io_wrapper() to repeat the same
> functionality for the QEMU processes other than qemu-io and to keep the
> uniformity. It is also true for the function _qemu_proc_valgrind_log()
> that dumps Valgrind reports in case of errors. The behavior is the same
> to one in the _qemu_io_wrapper(). Actually, nothing new except that
> _qemu_nbd_wrapper() gets the subshell exec to keep the uniformity.
>
> Andrey
>
>>> +_qemu_proc_valgrind_log()
>>> +{
>>> + local VALGRIND_LOGFILE="$1"
>>> + local RETVAL="$2"
>>> + if [ "${VALGRIND_QEMU}" == "y" ]; then
>>> + if [ $RETVAL == 99 ]; then
>>> + cat "${VALGRIND_LOGFILE}"
>>> + fi
>>> + rm -f "${VALGRIND_LOGFILE}"
>>> + fi
>>> +}
>>> +
>>> _qemu_wrapper()
>>> {
>>> + local VALGRIND_LOGFILE="${TEST_DIR}"/$$.valgrind
>>> (
>>> if [ -n "${QEMU_NEED_PID}" ]; then
>>> echo $BASHPID > "${QEMU_TEST_DIR}/qemu-${_QEMU_HANDLE}.pid"
>>> fi
>>> - exec "$QEMU_PROG" $QEMU_OPTIONS "$@"
>>> + _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_PROG" $QEMU_OPTIONS "$@"
>>
>> Can we not inline that logic here? especially because:
>>
>>> )
>>> + RETVAL=$?
>>> + _qemu_proc_valgrind_log "${VALGRIND_LOGFILE}" $RETVAL
>>
>> We're already making valgrind calls here anyway.
>>
>
> We can of cause, but we would repeat that for all other
> _qemu_*_wrapper() functions as well. The _qemu_proc_wrapper() runs a
> process under the Valgrind if VALGRIND_QEMU=y and the
> _qemu_proc_valgrind_log() dumps Valgrind error reports, if any and
> removes the Valgrind temporary log file. Again, the same pattern is used
> as it is in the current implementation of _qemu_io_wrapper(). I have
> just copied the functionality to all other QEMU wrapper functions.
>
> Andrey
>
Oh, I see now. I was just asleep at the wheel. Sorry for the hassle.
This is fine then, but maybe name it like _valgrind_exec_wrapper or some
such?
--js
On 27/08/2019 22:56, John Snow wrote:
>
>
> On 8/25/19 11:26 AM, Andrey Shinkevich wrote:
>>
>>
>> On 16/08/2019 01:49, John Snow wrote:
>>>
>>>
>>> On 7/19/19 12:30 PM, Andrey Shinkevich wrote:
>>>> With the '-valgrind' option, let all the QEMU processes be run under
>>>> the Valgrind tool. The Valgrind own parameters may be set with its
>>>> environment variable VALGRIND_OPTS, e.g.
>>>> VALGRIND_OPTS="--leak-check=yes" ./check -qcow2 -valgrind <test#>
>>>> or they may be listed in the Valgrind checked file ./.valgrindrc or
>>>> ~/.valgrindrc like
>>>> --memcheck:leak-check=no
>>>> --memcheck:track-origins=yes
>>>> When QEMU-IO process is being killed, the shell report refers to the
>>>> text of the command in _qemu_io_wrapper(), which was modified with this
>>>> patch. So, the benchmark output for the tests 039, 061 and 137 is to be
>>>> changed also.
>>>>
>>>
>>> Oh, weird. "VALGRIND_QEMU=y" actually has just meant ... valgrind
>>> qemu-io. OK.
>>>
>>>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>>>> ---
>>>> tests/qemu-iotests/039.out | 30 ++++---------------
>>>> tests/qemu-iotests/061.out | 12 ++------
>>>> tests/qemu-iotests/137.out | 6 +---
>>>> tests/qemu-iotests/common.rc | 69 ++++++++++++++++++++++++++++++++------------
>>>> 4 files changed, 59 insertions(+), 58 deletions(-)
>>>>
>>>> diff --git a/tests/qemu-iotests/039.out b/tests/qemu-iotests/039.out
>>>> index 724d7b2..972c6c0 100644
>>>> --- a/tests/qemu-iotests/039.out
>>>> +++ b/tests/qemu-iotests/039.out
>>>> @@ -11,11 +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)
>>>> -./common.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
>>>> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>>> -else
>>>> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>>> -fi )
>>>> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>>>> incompatible_features 0x1
>>>> ERROR cluster 5 refcount=0 reference=1
>>>> ERROR OFLAG_COPIED data cluster: l2_entry=8000000000050000 refcount=0
>>>> @@ -50,11 +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)
>>>> -./common.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
>>>> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>>> -else
>>>> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>>> -fi )
>>>> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>>>> incompatible_features 0x1
>>>> ERROR cluster 5 refcount=0 reference=1
>>>> Rebuilding refcount structure
>>>> @@ -68,11 +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)
>>>> -./common.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
>>>> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>>> -else
>>>> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>>> -fi )
>>>> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>>>> incompatible_features 0x0
>>>> No errors were found on the image.
>>>>
>>>> @@ -91,11 +79,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)
>>>> -./common.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
>>>> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>>> -else
>>>> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>>> -fi )
>>>> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>>>> incompatible_features 0x1
>>>> ERROR cluster 5 refcount=0 reference=1
>>>> ERROR OFLAG_COPIED data cluster: l2_entry=8000000000050000 refcount=0
>>>> @@ -105,11 +89,7 @@ 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.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
>>>> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>>> -else
>>>> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>>> -fi )
>>>> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>>>> 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 1aa7d37..8cb57eb 100644
>>>> --- a/tests/qemu-iotests/061.out
>>>> +++ b/tests/qemu-iotests/061.out
>>>> @@ -118,11 +118,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.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
>>>> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>>> -else
>>>> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>>> -fi )
>>>> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>>>> magic 0x514649fb
>>>> version 3
>>>> backing_file_offset 0x0
>>>> @@ -280,11 +276,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.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
>>>> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>>> -else
>>>> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>>> -fi )
>>>> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>>>> magic 0x514649fb
>>>> version 3
>>>> backing_file_offset 0x0
>>>> diff --git a/tests/qemu-iotests/137.out b/tests/qemu-iotests/137.out
>>>> index 22d59df..7fed5e6 100644
>>>> --- a/tests/qemu-iotests/137.out
>>>> +++ b/tests/qemu-iotests/137.out
>>>> @@ -35,11 +35,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
>>>> qemu-io: 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.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ]; then
>>>> - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>>> -else
>>>> - exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>>> -fi )
>>>> +./common.rc: Killed ( _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>>>> 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.rc b/tests/qemu-iotests/common.rc
>>>> index 5502c3d..6e461a1 100644
>>>> --- a/tests/qemu-iotests/common.rc
>>>> +++ b/tests/qemu-iotests/common.rc
>>>> @@ -60,19 +60,52 @@ if ! . ./common.config
>>>> exit 1
>>>> fi
>>>>
>>>> +_qemu_proc_wrapper()
>>>> +{
>>>> + local VALGRIND_LOGFILE="$1"
>>>> + shift
>>>> + if [ "${VALGRIND_QEMU}" == "y" ]; then
>>>> + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$@"
>>>> + else
>>>> + exec "$@"
>>>> + fi
>>>> +}
>>>> +
>>>
>>> Why do we need a second wrapper? I get nervous with each new wrapper we
>>> make, because I feel like it has unintended consequences with pipe
>>> handling and so on in the test dispatcher scripts.
>>>
>>
>> The _qemu_proc_wrapper() is acctually the function rather than a new
>> wrapper. The suffix _wrapper can be changed to mislead a reader not. The
>> routine code originates from the _qemu_io_wrapper() to repeat the same
>> functionality for the QEMU processes other than qemu-io and to keep the
>> uniformity. It is also true for the function _qemu_proc_valgrind_log()
>> that dumps Valgrind reports in case of errors. The behavior is the same
>> to one in the _qemu_io_wrapper(). Actually, nothing new except that
>> _qemu_nbd_wrapper() gets the subshell exec to keep the uniformity.
>>
>> Andrey
>>
>>>> +_qemu_proc_valgrind_log()
>>>> +{
>>>> + local VALGRIND_LOGFILE="$1"
>>>> + local RETVAL="$2"
>>>> + if [ "${VALGRIND_QEMU}" == "y" ]; then
>>>> + if [ $RETVAL == 99 ]; then
>>>> + cat "${VALGRIND_LOGFILE}"
>>>> + fi
>>>> + rm -f "${VALGRIND_LOGFILE}"
>>>> + fi
>>>> +}
>>>> +
>>>> _qemu_wrapper()
>>>> {
>>>> + local VALGRIND_LOGFILE="${TEST_DIR}"/$$.valgrind
>>>> (
>>>> if [ -n "${QEMU_NEED_PID}" ]; then
>>>> echo $BASHPID > "${QEMU_TEST_DIR}/qemu-${_QEMU_HANDLE}.pid"
>>>> fi
>>>> - exec "$QEMU_PROG" $QEMU_OPTIONS "$@"
>>>> + _qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_PROG" $QEMU_OPTIONS "$@"
>>>
>>> Can we not inline that logic here? especially because:
>>>
>>>> )
>>>> + RETVAL=$?
>>>> + _qemu_proc_valgrind_log "${VALGRIND_LOGFILE}" $RETVAL
>>>
>>> We're already making valgrind calls here anyway.
>>>
>>
>> We can of cause, but we would repeat that for all other
>> _qemu_*_wrapper() functions as well. The _qemu_proc_wrapper() runs a
>> process under the Valgrind if VALGRIND_QEMU=y and the
>> _qemu_proc_valgrind_log() dumps Valgrind error reports, if any and
>> removes the Valgrind temporary log file. Again, the same pattern is used
>> as it is in the current implementation of _qemu_io_wrapper(). I have
>> just copied the functionality to all other QEMU wrapper functions.
>>
>> Andrey
>>
>
> Oh, I see now. I was just asleep at the wheel. Sorry for the hassle.
> This is fine then, but maybe name it like _valgrind_exec_wrapper or some
> such?
>
> --js
>
In v6, I removed the word 'wrapper' and renamed the function to
'_qemu_proc_exec'.
Message ID:
<1566834628-485525-1-git-send-email-andrey.shinkevich@virtuozzo.com>
Andrey
--
With the best regards,
Andrey Shinkevich
© 2016 - 2025 Red Hat, Inc.