Re: [PATCH v3 14/15] qemu_iotests: add option to show qemu binary logs on stdout
On 30.04.21 23:04, Emanuele Giuseppe Esposito wrote:
On 30/04/2021 15:50, Max Reitz wrote:
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
---
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.
It should access the superclass member, and it's the same that
_post_shutdown does. I can make a function for that.
Understood.
Regarding all other feedback in all patches that I did not answer: agree
on all of them, will adjust everything in v4.
Thanks, looking forward to it! :)
Max
Re: [PATCH v3 14/15] qemu_iotests: add option to show qemu binary logs on stdout
On 30/04/2021 15:50, Max Reitz wrote:
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
---
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.
It should access the superclass member, and it's the same that
_post_shutdown does. I can make a function for that.
Regarding all other feedback in all patches that I did not answer: agree
on all of them, will adjust everything in v4.
Thank you,
Emanuele
+
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',
Re: [PATCH v3 14/15] qemu_iotests: add option to show qemu binary logs on stdout
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
---
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
