05.06.2021 20:38, Emanuele Giuseppe Esposito wrote:
On 05/06/2021 15:28, Vladimir Sementsov-Ogievskiy wrote:
04.06.2021 12:17, Emanuele Giuseppe Esposito wrote:
Currently, the check script only parses the option and sets the
VALGRIND_QEMU environmental variable to "y".
Add another local python variable that prepares the command line,
identical to the one provided in the test scripts.
Because the python script does not know in advance the valgrind
PID to assign to the log file name, use the "%p" flag in valgrind
log file name that automatically puts the process PID at runtime.
Signed-off-by: Emanuele Giuseppe Esposito <eespo...@redhat.com>
Reviewed-by: Max Reitz <mre...@redhat.com>
---
tests/qemu-iotests/check | 7 ++++---
tests/qemu-iotests/iotests.py | 11 +++++++++++
tests/qemu-iotests/testenv.py | 1 +
3 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 1dba4218c0..e6aa110715 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -39,6 +39,10 @@ def make_argparser() -> argparse.ArgumentParser:
p.add_argument('--gdb', action='store_true',
help="start gdbserver with $GDB_OPTIONS options \
('localhost:12345' if $GDB_OPTIONS is empty)")
+ p.add_argument('--valgrind', action='store_true',
+ help='use valgrind, sets VALGRIND_QEMU environment '
+ 'variable')
+
p.add_argument('--misalign', action='store_true',
help='misalign memory allocations')
p.add_argument('--color', choices=['on', 'off', 'auto'],
@@ -88,9 +92,6 @@ def make_argparser() -> argparse.ArgumentParser:
g_bash.add_argument('-o', dest='imgopts',
help='options to pass to qemu-img create/convert, '
'sets IMGOPTS environment variable')
- g_bash.add_argument('--valgrind', action='store_true',
- help='use valgrind, sets VALGRIND_QEMU environment '
- 'variable')
g_sel = p.add_argument_group('test selecting options',
'The following options specify test set '
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index c547e8c07b..3fa1bd0ab5 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -96,6 +96,17 @@
sys.stderr.write('Please run this test via the "check" script\n')
sys.exit(os.EX_USAGE)
+qemu_valgrind = []
+if os.environ.get('VALGRIND_QEMU') == "y" and \
+ os.environ.get('NO_VALGRIND') != "y":
Hmm, interesting, why do you need additional NO_VALGRIND variable
To maintain consistency with the bash tests, where we have:
# Valgrind bug #409141 https://bugs.kde.org/show_bug.cgi?id=409141
# Until valgrind 3.16+ is ubiquitous, we must work around a hang in
# valgrind when issuing sigkill. Disable valgrind for this invocation.
_NO_VALGRIND()
{
NO_VALGRIND="y" "$@"
}
A, hm, I see it in bash tests. So, it's intended to not set by user but by
test.. But I doubt that python test will want to set environment variable to
disable valgrind. Most probably they will want to set some
valgrind_supported=False near supported_fmts=['qcow2']. So, we'll need
valgrind_supported argument for execute_setup_common. But no reason to
implement it until we don't need.
+ valgrind_logfile = "--log-file=" + test_dir
+ # %p allows to put the valgrind process PID, since
+ # we don't know it a priori (subprocess.Popen is
+ # not yet invoked)
+ valgrind_logfile += "/%p.valgrind"
+
+ qemu_valgrind = ['valgrind', valgrind_logfile, '--error-exitcode=99']
+
socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', 'socket_scm_helper')
luks_default_secret_object = 'secret,id=keysec0,data=' + \
diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index 8501c6caf5..8bf154376f 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -298,6 +298,7 @@ def print_env(self) -> None:
SOCK_DIR -- {SOCK_DIR}
SOCKET_SCM_HELPER -- {SOCKET_SCM_HELPER}
GDB_OPTIONS -- {GDB_OPTIONS}
+VALGRIND_QEMU -- {VALGRIND_QEMU}
"""
args = collections.defaultdict(str, self.get_env())
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
--
Best regards,
Vladimir
