Re: [Qemu-devel] [PATCH 1/7] iotests: allow Valgrind checking all QEMU processes
On 6/10/19 10:02 AM, Andrey Shinkevich wrote: > > > On 10/06/2019 17:24, Eric Blake wrote: >> On 6/9/19 1:35 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 >> >> Let's spell this --valgrind; long options should prefer the use of -- >> (as in getopt_long), whether or not we also have a reason to support >> -valgrind (as in getopt_long_only). Yes, qemu is an oddball in this >> regards, but no need to make it worse. >> > > Thank you, Eric. That sounds good but the short option'-valgrind' is > preexisting in QEMU. Should I create a new patch for the long option? > If so, will we have both options supported by QEMU? Oh, you're talking about qemu-iotests/check already supporting merely '-valgrind', not 'qemu-kvm' or '*/qemu-system-*'. ./check is already an oddball for not permitting double dash, but at this point, normalizing it is a lot of churn. So it becomes a tradeoff on how much grunt work do you really want to do. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 1/7] iotests: allow Valgrind checking all QEMU processes
On 10/06/2019 17:24, Eric Blake wrote: > On 6/9/19 1:35 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 > > Let's spell this --valgrind; long options should prefer the use of -- > (as in getopt_long), whether or not we also have a reason to support > -valgrind (as in getopt_long_only). Yes, qemu is an oddball in this > regards, but no need to make it worse. > Thank you, Eric. That sounds good but the short option'-valgrind' is preexisting in QEMU. Should I create a new patch for the long option? If so, will we have both options supported by QEMU? Andrey >> >> Signed-off-by: Andrey Shinkevich >> --- >> tests/qemu-iotests/common.rc | 65 >> >> 1 file changed, 48 insertions(+), 17 deletions(-) >> >> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc >> index 93f8738..3caaca4 100644 > > -- With the best regards, Andrey Shinkevich
Re: [Qemu-devel] [PATCH 1/7] iotests: allow Valgrind checking all QEMU processes
On 6/9/19 1:35 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 Let's spell this --valgrind; long options should prefer the use of -- (as in getopt_long), whether or not we also have a reason to support -valgrind (as in getopt_long_only). Yes, qemu is an oddball in this regards, but no need to make it worse. > > Signed-off-by: Andrey Shinkevich > --- > tests/qemu-iotests/common.rc | 65 > > 1 file changed, 48 insertions(+), 17 deletions(-) > > diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc > index 93f8738..3caaca4 100644 -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH 1/7] iotests: allow Valgrind checking all QEMU processes
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
Signed-off-by: Andrey Shinkevich
---
tests/qemu-iotests/common.rc | 65
1 file changed, 48 insertions(+), 17 deletions(-)
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 93f8738..3caaca4 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,38 +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()
{
+local VALGRIND_LOGFILE="${TEST_DIR}"/$$.valgrind
(
echo $BASHPID > "${QEMU_TEST_DIR}/qemu-nbd.pid"
-exec "$QEMU_NBD_PROG" $QEMU_NBD_OPTIONS "$@"
+_qemu_proc_wrapper "${VALGRIND_LOGFILE}" "$QEMU_NBD_PROG"
$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
