|
From: | Emanuele Giuseppe Esposito |
Subject: | Re: [PATCH v4 04/15] qemu-iotests: add option to attach gdbserver |
Date: | Thu, 27 May 2021 13:06:07 +0200 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 |
On 26/05/2021 21:15, Vladimir Sementsov-Ogievskiy wrote:
20.05.2021 10:52, Emanuele Giuseppe Esposito wrote:Define -gdb flag and GDB_OPTIONS environment variable to python tests to attach a gdbserver to each qemu instance. This patch only adds and parses this flag, it does not yet add the implementation for it. if -gdb is not provided but $GDB_OPTIONS is set, ignore the environment variable. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> --- tests/qemu-iotests/check | 6 +++++- tests/qemu-iotests/iotests.py | 5 +++++ tests/qemu-iotests/testenv.py | 19 ++++++++++++++++--- 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check index d1c87ceaf1..b9820fdaaf 100755 --- a/tests/qemu-iotests/check +++ b/tests/qemu-iotests/check @@ -33,6 +33,9 @@ def make_argparser() -> argparse.ArgumentParser: help='pretty print output for make check')p.add_argument('-d', dest='debug', action='store_true', help='debug')+ p.add_argument('-gdb', action='store_true', + help="start gdbserver with $GDB_OPTIONS options \ + ('localhost:12345' if $GDB_OPTIONS is empty)") p.add_argument('-misalign', action='store_true', help='misalign memory allocations') p.add_argument('--color', choices=['on', 'off', 'auto'], @@ -112,7 +115,8 @@ if __name__ == '__main__': env = TestEnv(imgfmt=args.imgfmt, imgproto=args.imgproto, aiomode=args.aiomode, cachemode=args.cachemode, imgopts=args.imgopts, misalign=args.misalign, - debug=args.debug, valgrind=args.valgrind) + debug=args.debug, valgrind=args.valgrind, + gdb=args.gdb) testfinder = TestFinder(test_dir=env.source_iotests)diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.pyindex 5d78de0f0b..d667fde6f8 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -75,6 +75,11 @@ qemu_prog = os.environ.get('QEMU_PROG', 'qemu') qemu_opts = os.environ.get('QEMU_OPTIONS', '').strip().split(' ') +gdb_qemu_env = os.environ.get('GDB_OPTIONS')should we specify a default? otherwise get() should raise an exception when GDB_OPTIONS is not set..or you need os.getenv, which will return None if variable is absent.
No, os.environ.get does not raise any exception. I tested it myself, plus: https://stackoverflow.com/questions/16924471/difference-between-os-getenv-and-os-environ-get
+qemu_gdb = [] +if gdb_qemu_env: + qemu_gdb = ['gdbserver'] + gdb_qemu_env.strip().split(' ') + imgfmt = os.environ.get('IMGFMT', 'raw') imgproto = os.environ.get('IMGPROTO', 'file') output_dir = os.environ.get('OUTPUT_DIR', '.')diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.pyindex 6d27712617..49ddd586ef 100644 --- a/tests/qemu-iotests/testenv.py +++ b/tests/qemu-iotests/testenv.py @@ -27,6 +27,7 @@ import glob from typing import Dict, Any, Optional, ContextManager +DEF_GDB_OPTIONS = 'localhost:12345' def isxfile(path: str) -> bool: return os.path.isfile(path) and os.access(path, os.X_OK) @@ -72,7 +73,8 @@ class TestEnv(ContextManager['TestEnv']):'QEMU_NBD_OPTIONS', 'IMGOPTS', 'IMGFMT', 'IMGPROTO','AIOMODE', 'CACHEMODE', 'VALGRIND_QEMU','CACHEMODE_IS_DEFAULT', 'IMGFMT_GENERIC', 'IMGOPTSSYNTAX', - 'IMGKEYSECRET', 'QEMU_DEFAULT_MACHINE', 'MALLOC_PERTURB_'] + 'IMGKEYSECRET', 'QEMU_DEFAULT_MACHINE', 'MALLOC_PERTURB_',+ 'GDB_OPTIONS'] def get_env(self) -> Dict[str, str]: env = {}@@ -163,7 +165,8 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: str,imgopts: Optional[str] = None, misalign: bool = False, debug: bool = False, - valgrind: bool = False) -> None: + valgrind: bool = False, + gdb: bool = False) -> None: self.imgfmt = imgfmt self.imgproto = imgproto self.aiomode = aiomode@@ -171,6 +174,14 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: str,self.misalign = misalign self.debug = debug + if gdb:+ self.gdb_options = os.environ.get('GDB_OPTIONS', DEF_GDB_OPTIONS)everywhere in the file os.getenv is used, let's be consistent on it.+ if not self.gdb_options: + # cover the case 'export GDB_OPTIONS=' + self.gdb_options = DEF_GDB_OPTIONSHmm, a bit strange to handle 'export X=' only for this new variable, we don't have such logic for other variables.. I'm not against still, if you need it.+ elif 'GDB_OPTIONS' in os.environ: + del os.environ['GDB_OPTIONS']Don't need to remove variable from envirton, it has no effect. The task of TestEnv class is to prepare environment in its attributes, listed in env_variables. Then prepared attributes are available through get_env(). So if you don't want to provide GDB_OPTIONS, it's enough to not initialize self.gdb_options. So, just drop "elif:" branch.
You forget that there are bash tests :) Think about the following case: # export GDB_OPTIONS="localhost:1236" # ./check -qcow2 007 # a script test The output will contain: GDB_OPTIONS -- VALGRIND_QEMU -- PRINT_QEMU_OUTPUT -- BUT in common.rc we will have: GDB="" if [ ! -z ${GDB_OPTIONS} ]; then # <---- still set! GDB="gdbserver ${GDB_OPTIONS}" fiso gsdbserver will be set anyways, and the test will block waiting for a gdb connection.
Therefore we need that elif.
+ if valgrind: self.valgrind_qemu = 'y' @@ -269,7 +280,9 @@ def print_env(self) -> None: PLATFORM -- {platform} TEST_DIR -- {TEST_DIR} SOCK_DIR -- {SOCK_DIR} -SOCKET_SCM_HELPER -- {SOCKET_SCM_HELPER}""" +SOCKET_SCM_HELPER -- {SOCKET_SCM_HELPER} +GDB_OPTIONS -- {GDB_OPTIONS}Not sure we need to see this additional line every time.. Maybe, show it only when gdb is set?
I think it can be helpful to remind the users what is not set and what is available to them (yes, one can also do ./check --help or read the docs but this is something even the laziest cannot unsee).
Thank you, Emanuele
+""" args = collections.defaultdict(str, self.get_env())
[Prev in Thread] | Current Thread | [Next in Thread] |