[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 14/15] qemu_iotests: add option to show qemu binary logs o
From: |
Max Reitz |
Subject: |
Re: [PATCH v3 14/15] qemu_iotests: add option to show qemu binary logs on stdout |
Date: |
Fri, 30 Apr 2021 15:50:34 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 |
On 14.04.21 19:03, Emanuele Giuseppe Esposito wrote:
Using the flag -p, allow the qemu binary to print to stdout.
This helps especially when doing print-debugging.
I think this shouldn’t refer to prints but to qemu’s stdout/stderr in
general, i.e....
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
tests/qemu-iotests/check | 3 ++-
tests/qemu-iotests/iotests.py | 9 +++++++++
tests/qemu-iotests/testenv.py | 9 +++++++--
3 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 489178d9a4..84483922eb 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -33,6 +33,7 @@ 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('-p', dest='print', action='store_true', help='shows qemu
binary prints to stdout')
I’d prefer for the description to be “redirects qemu's stdout and stderr
to the test output”.
Also, this line is too long.
p.add_argument('-gdb', action='store_true',
help="start gdbserver with $GDB_QEMU options. \
Default is localhost:12345")
@@ -117,7 +118,7 @@ if __name__ == '__main__':
aiomode=args.aiomode, cachemode=args.cachemode,
imgopts=args.imgopts, misalign=args.misalign,
debug=args.debug, valgrind=args.valgrind,
- gdb=args.gdb)
+ gdb=args.gdb, qprint=args.print)
testfinder = TestFinder(test_dir=env.source_iotests)
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index f9832558a0..52ff7332f8 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -79,6 +79,8 @@
if os.environ.get('GDB_QEMU'):
qemu_gdb = ['gdbserver'] + os.environ.get('GDB_QEMU').strip().split(' ')
+qemu_print = os.environ.get('PRINT_QEMU', False)
+
imgfmt = os.environ.get('IMGFMT', 'raw')
imgproto = os.environ.get('IMGPROTO', 'file')
output_dir = os.environ.get('OUTPUT_DIR', '.')
@@ -621,6 +623,13 @@ def _post_shutdown(self) -> None:
super()._post_shutdown()
self.subprocess_check_valgrind(qemu_valgrind)
+ def _pre_launch(self) -> None:
+ super()._pre_launch()
+ if qemu_print and self._qemu_log_file != None:
I think "is not None" is mostly used in Python. (I’m saying this in
this weird way because I’m not the one to ask what’s mostly used in
Python...)
(That also silences mypy, which otherwise complains and then fails 297.)
+ # set QEMU binary output to stdout
+ self._qemu_log_file.close()
+ self._qemu_log_file = None
I don’t know enough Python to know how this works. I suppose this does
access the super class’s member? (I could also imagine it creates a new
member, just for this child class, but that then effectively overwrites
the super class’s member.)
Considering _qemu_log_file is apparently meant to be a private attribute
(from the leading underscore), I think it would be better to have a
function provided by the super class for this.
+
def add_object(self, opts):
self._args.append('-object')
self._args.append(opts)
diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index 39ae7ace33..6ae099114e 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -73,7 +73,7 @@ class TestEnv(ContextManager['TestEnv']):
'AIOMODE', 'CACHEMODE', 'VALGRIND_QEMU',
'CACHEMODE_IS_DEFAULT', 'IMGFMT_GENERIC',
'IMGOPTSSYNTAX',
'IMGKEYSECRET', 'QEMU_DEFAULT_MACHINE',
'MALLOC_PERTURB_',
- 'GDB_QEMU']
+ 'GDB_QEMU', 'PRINT_QEMU']
def get_env(self) -> Dict[str, str]:
env = {}
@@ -165,7 +165,8 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: str,
misalign: bool = False,
debug: bool = False,
valgrind: bool = False,
- gdb: bool = False) -> None:
+ gdb: bool = False,
+ qprint: bool = False) -> None:
self.imgfmt = imgfmt
self.imgproto = imgproto
self.aiomode = aiomode
@@ -173,6 +174,9 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: str,
self.misalign = misalign
self.debug = debug
+ if qprint:
+ self.print_qemu = 'y'
+
if gdb:
self.gdb_qemu = os.environ.get('GDB_QEMU', 'localhost:12345')
elif 'GDB_QEMU' in os.environ:
@@ -278,6 +282,7 @@ def print_env(self) -> None:
SOCKET_SCM_HELPER -- {SOCKET_SCM_HELPER}
GDB_QEMU -- "{GDB_QEMU}"
VALGRIND_QEMU -- "{VALGRIND_QEMU}"
+PRINT_QEMU -- "{PRINT_QEMU}"
Again, I personally don’t like the quotes.
(I think “PRINT_QEMU -- y” and “PRINT_QEMU --” look better.)
Another thing: Here in this place, the name of the environment variable
is visible. “PRINT_QEMU” doesn’t really have meaning; now, if it
weren’t visible, I wouldn’t care, but here it is visible. Perhaps
“PRINT_QEMU_OUTPUT”, or "SHOW_QEMU_OUTPUT" would mean more?
Well, it’s all bike-shedding, so with “is not None” and the add_argument
line broken so it doesn’t exceed 79 characters:
Reviewed-by: Max Reitz <mreitz@redhat.com>
"""
args = collections.defaultdict(str, self.get_env())
- Re: [PATCH v3 05/15] qemu-iotests: delay QMP socket timers, (continued)
- [PATCH v3 06/15] qemu_iotests: insert gdbserver command line as wrapper for qemu binary, Emanuele Giuseppe Esposito, 2021/04/14
- [PATCH v3 07/15] qemu-iotests: add gdbserver option to script tests too, Emanuele Giuseppe Esposito, 2021/04/14
- [PATCH v3 08/15] docs/devel/testing: add -gdb option to the debugging section of QEMU iotests, Emanuele Giuseppe Esposito, 2021/04/14
- [PATCH v3 09/15] qemu_iotests: extend the check script to support valgrind for python tests, Emanuele Giuseppe Esposito, 2021/04/14
- [PATCH v3 14/15] qemu_iotests: add option to show qemu binary logs on stdout, Emanuele Giuseppe Esposito, 2021/04/14
- Re: [PATCH v3 14/15] qemu_iotests: add option to show qemu binary logs on stdout,
Max Reitz <=
- [PATCH v3 13/15] docs/devel/testing: add -valgrind option to the debug section of QEMU iotests, Emanuele Giuseppe Esposito, 2021/04/14
- [PATCH v3 10/15] qemu_iotests: extent QMP socket timeout when using valgrind, Emanuele Giuseppe Esposito, 2021/04/14
- [PATCH v3 11/15] qemu_iotests: allow valgrind to read/delete the generated log file, Emanuele Giuseppe Esposito, 2021/04/14
- [PATCH v3 15/15] docs/devel/testing: add -p option to the debug section of QEMU iotests, Emanuele Giuseppe Esposito, 2021/04/14