Re: [PATCH 1/1] iotests: fix 051.out expected output after error text touchups
On 3/18/21 4:09 PM, Connor Kuehl wrote:
A patch was recently applied that touched up some error messages that
pertained to key names like 'node-name'. The trouble is it only updated
tests/qemu-iotests/051.pc.out and not tests/qemu-iotests/051.out as
well.
Do that now.
Fixes: 785ec4b1b9 ("block: Clarify error messages pertaining to
'node-name'")
Signed-off-by: Connor Kuehl
Reviewed-by: John Snow
---
tests/qemu-iotests/051.out | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index de4771bcb3..db8c14b903 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -61,13 +61,13 @@ QEMU X.Y.Z monitor - type 'help' for more information
(qemu) quit
Testing: -drive file=TEST_DIR/t.qcow2,node-name=123foo
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,node-name=123foo: Invalid node name
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,node-name=123foo: Invalid node-name:
'123foo'
Testing: -drive file=TEST_DIR/t.qcow2,node-name=_foo
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,node-name=_foo: Invalid node name
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,node-name=_foo: Invalid node-name:
'_foo'
Testing: -drive file=TEST_DIR/t.qcow2,node-name=foo#12
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,node-name=foo#12: Invalid node name
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,node-name=foo#12: Invalid node-name:
'foo#12'
=== Device without drive ===
Re: [PATCH v2 4/5] qemu-iotests: let "check" spawn an arbitrary test command
On 23/03/21 20:12, Vladimir Sementsov-Ogievskiy wrote: Move the trailing empty line to print_env(), since it always looks better and one caller was not adding it. Seems you've moved this fix from one unrelated commit to another.. And it touches two extra files. I'd just make it a separate commit. Nitpicking. Separate or as is: Well, now I add the third caller so it's time to make up our mind on the trailing line. A two-line commit didn't seem worth it. Paolo Reviewed-by: Vladimir Sementsov-Ogievskiy
Re: [PATCH v2 2/5] qemu-iotests: allow passing unittest.main arguments to the test scripts
On 23/03/21 20:17, Vladimir Sementsov-Ogievskiy wrote: + unittest.main(argv=argv, + testRunner=ReproducibleTestRunner, + verbosity=2 if debug else 1, + warnings=None if sys.warnoptions else 'ignore') def execute_setup_common(supported_fmts: Sequence[str] = (), supported_platforms: Sequence[str] = (), @@ -1350,7 +1354,7 @@ def execute_test(*args, test_function=None, **kwargs): debug = execute_setup_common(*args, **kwargs) if not test_function: - execute_unittest(debug) + execute_unittest(sys.argv, debug) else: test_function() If you decide to resend for some of my comments (or due to another reviewer be more careful), I think it would be nicer to merge part of this commit which moves us from passing object to passing ReproducibleTestRunner to the previous commit, to not remove line that we've added in the previous commit. And here only add argv argument. Well, it's the price to pay to make the previous commit as independent as possible. In particular in the previous patch there's no reason to add the complexity of warnings. I could make it three commits, but at some point too much splitting adds clutter, the patches are already pretty small. Paolo
Re: [PATCH v2 1/5] qemu-iotests: do not buffer the test output
23.03.2021 21:19, Paolo Bonzini wrote:
Instead of buffering the test output into a StringIO, patch it on
the fly by wrapping sys.stdout's write method. This can be
done unconditionally, even if using -d, which makes execute_unittest
a bit simpler.
Signed-off-by: Paolo Bonzini
Reviewed-by: Vladimir Sementsov-Ogievskiy
some non-critical notes below
---
tests/qemu-iotests/iotests.py | 68 ---
1 file changed, 40 insertions(+), 28 deletions(-)
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 90d0b62523..0521235030 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -32,7 +32,7 @@
import sys
import time
from typing import (Any, Callable, Dict, Iterable,
-List, Optional, Sequence, Tuple, TypeVar)
+List, Optional, Sequence, Tuple, Type, TypeVar)
import unittest
from contextlib import contextmanager
@@ -1271,37 +1271,49 @@ def func_wrapper(*args, **kwargs):
return func(*args, **kwargs)
return func_wrapper
You don't use typing everywhere.. But iotests.py doesn't use it everywhere as
well
+# We need to filter out the time taken from the output so that
+# qemu-iotest can reliably diff the results against master output,
+# and hide skipped tests from the reference output.
+
+class ReproducibleTestResult(unittest.TextTestResult):
+def addSkip(self, test, reason):
+# Same as TextTestResult, but print dot instead of "s"
+unittest.TestResult.addSkip(self, test, reason)
+if self.showAll:
+self.stream.writeln("skipped {0!r}".format(reason))
+elif self.dots:
+self.stream.write(".")
+self.stream.flush()
+
+class ReproducibleStreamWrapper(object):
+def __init__(self, stream):
+self.stream = stream
+
+def __getattr__(self, attr):
+if attr in ('stream', '__getstate__'):
why __getstate__ is here? It's something about pickle..
+raise AttributeError(attr)
+return getattr(self.stream,attr)
s/,/, /
+
+def write(self, arg=None):
+arg = re.sub(r'Ran (\d+) tests? in [\d.]+s', r'Ran \1 tests', arg)
+arg = re.sub(r' \(skipped=\d+\)', r'', arg)
+self.stream.write(arg)
+
+class ReproducibleTestRunner(unittest.TextTestRunner):
+def __init__(self, stream: Optional[io.TextIOBase] = None,
+ resultclass: Type = ReproducibleTestResult, *args, **kwargs):
actually, neither stream nor resultclass arguments are used in the code, so it
seems we don't need them?
+rstream = ReproducibleStreamWrapper(stream or sys.stdout)
+super().__init__(stream=rstream, # type: ignore
+ descriptions=True,
+ resultclass=resultclass,
+ *args, **kwargs)
+
def execute_unittest(debug=False):
"""Executes unittests within the calling module."""
verbosity = 2 if debug else 1
-
-if debug:
-output = sys.stdout
-else:
-# We need to filter out the time taken from the output so that
-# qemu-iotest can reliably diff the results against master output.
-output = io.StringIO()
-
-runner = unittest.TextTestRunner(stream=output, descriptions=True,
- verbosity=verbosity)
-try:
-# unittest.main() will use sys.exit(); so expect a SystemExit
-# exception
-unittest.main(testRunner=runner)
-finally:
-# We need to filter out the time taken from the output so that
-# qemu-iotest can reliably diff the results against master output.
-if not debug:
-out = output.getvalue()
-out = re.sub(r'Ran (\d+) tests? in [\d.]+s', r'Ran \1 tests', out)
-
-# Hide skipped tests from the reference output
-out = re.sub(r'OK \(skipped=\d+\)', 'OK', out)
-out_first_line, out_rest = out.split('\n', 1)
-out = out_first_line.replace('s', '.') + '\n' + out_rest
-
-sys.stderr.write(out)
+runner = unittest.ReproducibleTestRunner(verbosity=verbosity)
+unittest.main(testRunner=runner)
def execute_setup_common(supported_fmts: Sequence[str] = (),
supported_platforms: Sequence[str] = (),
--
Best regards,
Vladimir
Re: [PATCH v2 1/5] qemu-iotests: do not buffer the test output
23.03.2021 21:54, Vladimir Sementsov-Ogievskiy wrote: +class ReproducibleTestRunner(unittest.TextTestRunner): + def __init__(self, stream: Optional[io.TextIOBase] = None, + resultclass: Type = ReproducibleTestResult, *args, **kwargs): actually, neither stream nor resultclass arguments are used in the code, so it seems we don't need them? Ah, we probably should support them to be prepared for the following patch. -- Best regards, Vladimir
Re: [PATCH v2 2/5] qemu-iotests: allow passing unittest.main arguments to the test scripts
23.03.2021 21:19, Paolo Bonzini wrote: Python test scripts that use unittest consist of multiple tests. unittest.main allows selecting which tests to run, but currently this is not possible because the iotests wrapper ignores sys.argv. unittest.main command line options also allow the user to pick the desired options for verbosity, failfast mode, etc. While "-d" is currently translated to "-v", it also enables extra debug output, and other options are not available at all. These command line options only work if the unittest.main testRunner argument is a type, rather than a TestRunner instance. Therefore, pass the class name and "verbosity" argument to unittest.main, and adjust for the different default warnings between TextTestRunner and unittest.main. Signed-off-by: Paolo Bonzini --- tests/qemu-iotests/iotests.py | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index 0521235030..c7915684ba 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -1308,12 +1308,16 @@ def __init__(self, stream: Optional[io.TextIOBase] = None, resultclass=resultclass, *args, **kwargs) -def execute_unittest(debug=False): +def execute_unittest(argv: List[str], debug: bool= False): """Executes unittests within the calling module.""" -verbosity = 2 if debug else 1 -runner = unittest.ReproducibleTestRunner(verbosity=verbosity) -unittest.main(testRunner=runner) +# Some tests have warnings, especially ResourceWarnings for unclosed +# files and sockets. Ignore them for now to ensure reproducibility of +# the test output. +unittest.main(argv=argv, + testRunner=ReproducibleTestRunner, + verbosity=2 if debug else 1, + warnings=None if sys.warnoptions else 'ignore') def execute_setup_common(supported_fmts: Sequence[str] = (), supported_platforms: Sequence[str] = (), @@ -1350,7 +1354,7 @@ def execute_test(*args, test_function=None, **kwargs): debug = execute_setup_common(*args, **kwargs) if not test_function: -execute_unittest(debug) +execute_unittest(sys.argv, debug) else: test_function() If you decide to resend for some of my comments (or due to another reviewer be more careful), I think it would be nicer to merge part of this commit which moves us from passing object to passing ReproducibleTestRunner to the previous commit, to not remove line that we've added in the previous commit. And here only add argv argument. -- Best regards, Vladimir
Re: [PATCH v2 4/5] qemu-iotests: let "check" spawn an arbitrary test command
23.03.2021 21:19, Paolo Bonzini wrote:
Right now there is no easy way for "check" to print a reproducer command.
Because such a reproducer command line would be huge, we can instead teach
check to start a command of our choice. This can be for example a Python
unit test with arguments to only run a specific subtest.
Move the trailing empty line to print_env(), since it always looks better
and one caller was not adding it.
Seems you've moved this fix from one unrelated commit to another.. And it
touches two extra files. I'd just make it a separate commit. Nitpicking.
Separate or as is:
Reviewed-by: Vladimir Sementsov-Ogievskiy
Signed-off-by: Paolo Bonzini
---
tests/qemu-iotests/check | 15 ++-
tests/qemu-iotests/testenv.py| 3 ++-
tests/qemu-iotests/testrunner.py | 1 -
3 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index d1c87ceaf1..478d74e509 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -19,6 +19,7 @@
import os
import sys
import argparse
+import shutil
from findtests import TestFinder
from testenv import TestEnv
from testrunner import TestRunner
@@ -101,7 +102,7 @@ def make_argparser() -> argparse.ArgumentParser:
'rerun failed ./check command, starting from the '
'middle of the process.')
g_sel.add_argument('tests', metavar='TEST_FILES', nargs='*',
- help='tests to run')
+ help='tests to run, or "--" followed by a command')
return p
@@ -114,6 +115,18 @@ if __name__ == '__main__':
imgopts=args.imgopts, misalign=args.misalign,
debug=args.debug, valgrind=args.valgrind)
+if len(sys.argv) > 1 and sys.argv[-len(args.tests)-1] == '--':
+if not args.tests:
+sys.exit("missing command after '--'")
+cmd = args.tests
+env.print_env()
+exec_path = shutil.which(cmd[0])
+if exec_path is None:
+sys.exit('command not found: ' + cmd[0])
+cmd[0] = exec_path
+full_env = env.prepare_subprocess(cmd)
+os.execve(cmd[0], cmd, full_env)
+
testfinder = TestFinder(test_dir=env.source_iotests)
groups = args.groups.split(',') if args.groups else None
diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index 341a4af4e9..6767eeeb25 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -283,7 +283,8 @@ 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}
+"""
args = collections.defaultdict(str, self.get_env())
diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
index 519924dc81..2f56ac545d 100644
--- a/tests/qemu-iotests/testrunner.py
+++ b/tests/qemu-iotests/testrunner.py
@@ -316,7 +316,6 @@ def run_tests(self, tests: List[str]) -> bool:
if not self.makecheck:
self.env.print_env()
-print()
test_field_width = max(len(os.path.basename(t)) for t in tests) + 2
--
Best regards,
Vladimir
[PATCH v2 4/5] qemu-iotests: let "check" spawn an arbitrary test command
Right now there is no easy way for "check" to print a reproducer command.
Because such a reproducer command line would be huge, we can instead teach
check to start a command of our choice. This can be for example a Python
unit test with arguments to only run a specific subtest.
Move the trailing empty line to print_env(), since it always looks better
and one caller was not adding it.
Signed-off-by: Paolo Bonzini
---
tests/qemu-iotests/check | 15 ++-
tests/qemu-iotests/testenv.py| 3 ++-
tests/qemu-iotests/testrunner.py | 1 -
3 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index d1c87ceaf1..478d74e509 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -19,6 +19,7 @@
import os
import sys
import argparse
+import shutil
from findtests import TestFinder
from testenv import TestEnv
from testrunner import TestRunner
@@ -101,7 +102,7 @@ def make_argparser() -> argparse.ArgumentParser:
'rerun failed ./check command, starting from the '
'middle of the process.')
g_sel.add_argument('tests', metavar='TEST_FILES', nargs='*',
- help='tests to run')
+ help='tests to run, or "--" followed by a command')
return p
@@ -114,6 +115,18 @@ if __name__ == '__main__':
imgopts=args.imgopts, misalign=args.misalign,
debug=args.debug, valgrind=args.valgrind)
+if len(sys.argv) > 1 and sys.argv[-len(args.tests)-1] == '--':
+if not args.tests:
+sys.exit("missing command after '--'")
+cmd = args.tests
+env.print_env()
+exec_path = shutil.which(cmd[0])
+if exec_path is None:
+sys.exit('command not found: ' + cmd[0])
+cmd[0] = exec_path
+full_env = env.prepare_subprocess(cmd)
+os.execve(cmd[0], cmd, full_env)
+
testfinder = TestFinder(test_dir=env.source_iotests)
groups = args.groups.split(',') if args.groups else None
diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index 341a4af4e9..6767eeeb25 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -283,7 +283,8 @@ 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}
+"""
args = collections.defaultdict(str, self.get_env())
diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
index 519924dc81..2f56ac545d 100644
--- a/tests/qemu-iotests/testrunner.py
+++ b/tests/qemu-iotests/testrunner.py
@@ -316,7 +316,6 @@ def run_tests(self, tests: List[str]) -> bool:
if not self.makecheck:
self.env.print_env()
-print()
test_field_width = max(len(os.path.basename(t)) for t in tests) + 2
--
2.30.1
[PATCH v2 0/5] qemu-iotests: quality of life improvements
This series adds a few usability improvements to qemu-iotests, in particular: - arguments can be passed to Python unittests scripts, for example to run only a subset of the test cases (patches 1-2) - it is possible to do "./check -- ../../../tests/qemu-iotests/055 args..." and specify arbitrary arguments to be passed to a single test script. This allows to take advantage of the previous feature and ease debugging of Python tests. Paolo v1->v2: patches 1-2 are a rewrite of v1's patch 1 moved print_env change to patch 4 do not use argparse.REMAINDER Paolo Bonzini (5): qemu-iotests: do not buffer the test output qemu-iotests: allow passing unittest.main arguments to the test scripts qemu-iotests: move command line and environment handling from TestRunner to TestEnv qemu-iotests: let "check" spawn an arbitrary test command qemu-iotests: fix case of SOCK_DIR already in the environment tests/qemu-iotests/check | 15 +- tests/qemu-iotests/iotests.py| 78 +++- tests/qemu-iotests/testenv.py| 22 +++-- tests/qemu-iotests/testrunner.py | 15 +- 4 files changed, 81 insertions(+), 49 deletions(-) -- 2.30.1
[PATCH v2 3/5] qemu-iotests: move command line and environment handling from TestRunner to TestEnv
In the next patch, "check" will learn how to execute a test script without
going through TestRunner. To enable this, keep only the text output
and subprocess handling in the TestRunner; move into TestEnv the logic
to prepare for running a subprocess.
Reviewed-by: Vladimir Sementsov-Ogievskiy
Signed-off-by: Paolo Bonzini
---
tests/qemu-iotests/testenv.py| 17 -
tests/qemu-iotests/testrunner.py | 14 +-
2 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index 1fbec854c1..341a4af4e9 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -25,7 +25,7 @@
import random
import subprocess
import glob
-from typing import Dict, Any, Optional, ContextManager
+from typing import List, Dict, Any, Optional, ContextManager
def isxfile(path: str) -> bool:
@@ -74,6 +74,21 @@ class TestEnv(ContextManager['TestEnv']):
'CACHEMODE_IS_DEFAULT', 'IMGFMT_GENERIC', 'IMGOPTSSYNTAX',
'IMGKEYSECRET', 'QEMU_DEFAULT_MACHINE', 'MALLOC_PERTURB_']
+def prepare_subprocess(self, args: List[str]) -> Dict[str, str]:
+if self.debug:
+args.append('-d')
+
+with open(args[0], encoding="utf-8") as f:
+try:
+if f.readline().rstrip() == '#!/usr/bin/env python3':
+args.insert(0, self.python)
+except UnicodeDecodeError: # binary test? for future.
+pass
+
+os_env = os.environ.copy()
+os_env.update(self.get_env())
+return os_env
+
def get_env(self) -> Dict[str, str]:
env = {}
for v in self.env_variables:
diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
index 1fc61fcaa3..519924dc81 100644
--- a/tests/qemu-iotests/testrunner.py
+++ b/tests/qemu-iotests/testrunner.py
@@ -129,7 +129,6 @@ class TestRunner(ContextManager['TestRunner']):
def __init__(self, env: TestEnv, makecheck: bool = False,
color: str = 'auto') -> None:
self.env = env
-self.test_run_env = self.env.get_env()
self.makecheck = makecheck
self.last_elapsed = LastElapsedTime('.last-elapsed-cache', env)
@@ -243,18 +242,7 @@ def do_run_test(self, test: str) -> TestResult:
silent_unlink(p)
args = [str(f_test.resolve())]
-if self.env.debug:
-args.append('-d')
-
-with f_test.open(encoding="utf-8") as f:
-try:
-if f.readline().rstrip() == '#!/usr/bin/env python3':
-args.insert(0, self.env.python)
-except UnicodeDecodeError: # binary test? for future.
-pass
-
-env = os.environ.copy()
-env.update(self.test_run_env)
+env = self.env.prepare_subprocess(args)
t0 = time.time()
with f_bad.open('w', encoding="utf-8") as f:
--
2.30.1
[PATCH v2 5/5] qemu-iotests: fix case of SOCK_DIR already in the environment
Due to a typo, in this case the SOCK_DIR was not being created. Reviewed-by: Vladimir Sementsov-Ogievskiy Signed-off-by: Paolo Bonzini --- tests/qemu-iotests/testenv.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py index 6767eeeb25..169268f61a 100644 --- a/tests/qemu-iotests/testenv.py +++ b/tests/qemu-iotests/testenv.py @@ -120,7 +120,7 @@ def init_directories(self) -> None: try: self.sock_dir = os.environ['SOCK_DIR'] self.tmp_sock_dir = False -Path(self.test_dir).mkdir(parents=True, exist_ok=True) +Path(self.sock_dir).mkdir(parents=True, exist_ok=True) except KeyError: self.sock_dir = tempfile.mkdtemp() self.tmp_sock_dir = True -- 2.30.1
[PATCH v2 1/5] qemu-iotests: do not buffer the test output
Instead of buffering the test output into a StringIO, patch it on
the fly by wrapping sys.stdout's write method. This can be
done unconditionally, even if using -d, which makes execute_unittest
a bit simpler.
Signed-off-by: Paolo Bonzini
---
tests/qemu-iotests/iotests.py | 68 ---
1 file changed, 40 insertions(+), 28 deletions(-)
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 90d0b62523..0521235030 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -32,7 +32,7 @@
import sys
import time
from typing import (Any, Callable, Dict, Iterable,
-List, Optional, Sequence, Tuple, TypeVar)
+List, Optional, Sequence, Tuple, Type, TypeVar)
import unittest
from contextlib import contextmanager
@@ -1271,37 +1271,49 @@ def func_wrapper(*args, **kwargs):
return func(*args, **kwargs)
return func_wrapper
+# We need to filter out the time taken from the output so that
+# qemu-iotest can reliably diff the results against master output,
+# and hide skipped tests from the reference output.
+
+class ReproducibleTestResult(unittest.TextTestResult):
+def addSkip(self, test, reason):
+# Same as TextTestResult, but print dot instead of "s"
+unittest.TestResult.addSkip(self, test, reason)
+if self.showAll:
+self.stream.writeln("skipped {0!r}".format(reason))
+elif self.dots:
+self.stream.write(".")
+self.stream.flush()
+
+class ReproducibleStreamWrapper(object):
+def __init__(self, stream):
+self.stream = stream
+
+def __getattr__(self, attr):
+if attr in ('stream', '__getstate__'):
+raise AttributeError(attr)
+return getattr(self.stream,attr)
+
+def write(self, arg=None):
+arg = re.sub(r'Ran (\d+) tests? in [\d.]+s', r'Ran \1 tests', arg)
+arg = re.sub(r' \(skipped=\d+\)', r'', arg)
+self.stream.write(arg)
+
+class ReproducibleTestRunner(unittest.TextTestRunner):
+def __init__(self, stream: Optional[io.TextIOBase] = None,
+ resultclass: Type = ReproducibleTestResult, *args, **kwargs):
+rstream = ReproducibleStreamWrapper(stream or sys.stdout)
+super().__init__(stream=rstream, # type: ignore
+ descriptions=True,
+ resultclass=resultclass,
+ *args, **kwargs)
+
def execute_unittest(debug=False):
"""Executes unittests within the calling module."""
verbosity = 2 if debug else 1
-
-if debug:
-output = sys.stdout
-else:
-# We need to filter out the time taken from the output so that
-# qemu-iotest can reliably diff the results against master output.
-output = io.StringIO()
-
-runner = unittest.TextTestRunner(stream=output, descriptions=True,
- verbosity=verbosity)
-try:
-# unittest.main() will use sys.exit(); so expect a SystemExit
-# exception
-unittest.main(testRunner=runner)
-finally:
-# We need to filter out the time taken from the output so that
-# qemu-iotest can reliably diff the results against master output.
-if not debug:
-out = output.getvalue()
-out = re.sub(r'Ran (\d+) tests? in [\d.]+s', r'Ran \1 tests', out)
-
-# Hide skipped tests from the reference output
-out = re.sub(r'OK \(skipped=\d+\)', 'OK', out)
-out_first_line, out_rest = out.split('\n', 1)
-out = out_first_line.replace('s', '.') + '\n' + out_rest
-
-sys.stderr.write(out)
+runner = unittest.ReproducibleTestRunner(verbosity=verbosity)
+unittest.main(testRunner=runner)
def execute_setup_common(supported_fmts: Sequence[str] = (),
supported_platforms: Sequence[str] = (),
--
2.30.1
Re: [PATCH v2 2/5] qemu-iotests: allow passing unittest.main arguments to the test scripts
23.03.2021 21:19, Paolo Bonzini wrote: Python test scripts that use unittest consist of multiple tests. unittest.main allows selecting which tests to run, but currently this is not possible because the iotests wrapper ignores sys.argv. unittest.main command line options also allow the user to pick the desired options for verbosity, failfast mode, etc. While "-d" is currently translated to "-v", it also enables extra debug output, and other options are not available at all. These command line options only work if the unittest.main testRunner argument is a type, rather than a TestRunner instance. Therefore, pass the class name and "verbosity" argument to unittest.main, and adjust for the different default warnings between TextTestRunner and unittest.main. Signed-off-by: Paolo Bonzini Reviewed-by: Vladimir Sementsov-Ogievskiy -- Best regards, Vladimir
Re: [PATCH 3/4] qemu-iotests: let "check" spawn an arbitrary test command
23.03.2021 20:22, Paolo Bonzini wrote: On 23/03/21 18:11, Vladimir Sementsov-Ogievskiy wrote: If you have positional arguments that must begin with - and don’t look like negative numbers, you can insert the pseudo-argument '--' which tells parse_args() that everything after that is a positional argument: So, as I understand argparse supports '--' feature out of the box. So, we can keep '*' as is, and it would parse all remaining positional arguments which are either tests or the command, and '--' will be automatically dropped. So, we only need to check existing of '--' in original sys.argv to chose our behavior. There is still a difference with REMAINDER: ./check aa -- bb => REMAINDER: error because ./-- is not a test => look for '--': invoke "aa -- bb" So I think REMAINDER provides the best behavior overall. Ok, with '*' you need to check that exactly sys.argv[-len(agrs.tests)-1] == '--', which gives the same behavior as REMAINDER. I'm OK with REMAINDER too, still with we probably also need some comment to not embarrass next person who go into documentation and not find it. -- Best regards, Vladimir
Re: [PATCH 1/4] qemu-iotests: allow passing unittest.main arguments to the test scripts
On 23/03/21 18:27, Vladimir Sementsov-Ogievskiy wrote: Sounds good :) We'll see dots appearing dynamically during test run, yes? Yes, exactly! :) Paolo
Re: [PATCH 1/4] qemu-iotests: allow passing unittest.main arguments to the test scripts
23.03.2021 20:04, Paolo Bonzini wrote:
On 23/03/21 17:56, Vladimir Sementsov-Ogievskiy wrote:
hmm, just use a separate call of unittest.main, or honestly define a varaible
as Union[] or just Any ?
All the ugliness goes away if the implementation is done properly. :)
For me normal try-finally is a lot more clean than calling atexit() in a class
method. It's just a strange interface. Prior to the patch user can call
execute_unittest several times and expect that output will be printed during
the call. After the patch outputs of all calls to execute_unittest() will be
printed at program exit..
Yeah, I agree. I didn't like the finally, but I really didn't like it
because of the *behavior* of buffering the whole output. I have changed
it to drop the buffering altogether, that's much better code and more usable:
Me too. Never liked buffering of test output.
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 90d0b62523..a74f4f9488 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -32,7 +32,7 @@
import sys
import time
from typing import (Any, Callable, Dict, Iterable,
- List, Optional, Sequence, Tuple, TypeVar)
+ List, Optional, Sequence, Tuple, Type, TypeVar)
import unittest
from contextlib import contextmanager
@@ -1271,37 +1271,49 @@ def func_wrapper(*args, **kwargs):
return func(*args, **kwargs)
return func_wrapper
+# We need to filter out the time taken from the output so that
+# qemu-iotest can reliably diff the results against master output,
+# and hide skipped tests from the reference output.
+
+class ReproducibleTestResult(unittest.TextTestResult):
+ def addSkip(self, test, reason):
+ # Same as TextTestResult, but print dot instead of "s"
+ unittest.TestResult.addSkip(self, test, reason)
+ if self.showAll:
+ self.stream.writeln("skipped {0!r}".format(reason))
+ elif self.dots:
+ self.stream.write(".")
+ self.stream.flush()
+
+class ReproducibleStreamWrapper(object):
+ def __init__(self, stream):
+ self.stream = stream
+
+ def __getattr__(self, attr):
+ if attr in ('stream', '__getstate__'):
+ raise AttributeError(attr)
+ return getattr(self.stream,attr)
+
+ def write(self, arg=None):
+ arg = re.sub(r'Ran (\d+) tests? in [\d.]+s', r'Ran \1 tests', arg)
+ arg = re.sub(r' \(skipped=\d+\)', r'', arg)
+ self.stream.write(arg)
+
+class ReproducibleTestRunner(unittest.TextTestRunner):
+ def __init__(self, stream: Optional[io.TextIOBase] = None,
+ resultclass: Type = ReproducibleTestResult, *args, **kwargs):
+ rstream = ReproducibleStreamWrapper(stream or sys.stdout)
+ super().__init__(stream=rstream, # type: ignore
+ descriptions=True,
+ resultclass=resultclass,
+ *args, **kwargs)
+
def execute_unittest(debug=False):
"""Executes unittests within the calling module."""
verbosity = 2 if debug else 1
-
- if debug:
- output = sys.stdout
- else:
- # We need to filter out the time taken from the output so that
- # qemu-iotest can reliably diff the results against master output.
- output = io.StringIO()
-
- runner = unittest.TextTestRunner(stream=output, descriptions=True,
- verbosity=verbosity)
- try:
- # unittest.main() will use sys.exit(); so expect a SystemExit
- # exception
- unittest.main(testRunner=runner)
- finally:
- # We need to filter out the time taken from the output so that
- # qemu-iotest can reliably diff the results against master output.
- if not debug:
- out = output.getvalue()
- out = re.sub(r'Ran (\d+) tests? in [\d.]+s', r'Ran \1 tests', out)
-
- # Hide skipped tests from the reference output
- out = re.sub(r'OK \(skipped=\d+\)', 'OK', out)
- out_first_line, out_rest = out.split('\n', 1)
- out = out_first_line.replace('s', '.') + '\n' + out_rest
-
- sys.stderr.write(out)
+ runner = unittest.ReproducibleTestRunner(verbosity=verbosity)
+ unittest.main(testRunner=ReproducibleTestRunner)
def execute_setup_common(supported_fmts: Sequence[str] = (),
supported_platforms: Sequence[str] = (),
Sounds good :) We'll see dots appearing dynamically during test run, yes?
[anyway, I'd drop "" test outputs for unittest-based tests at some moment... But
after that patch I'll have to keep some kind of "progress bar" :]
--
Best regards,
Vladimir
Re: [PATCH 3/4] qemu-iotests: let "check" spawn an arbitrary test command
23.03.2021 20:00, Paolo Bonzini wrote:
On 23/03/21 17:43, Vladimir Sementsov-Ogievskiy wrote:
Interesting that REMAINDER documentation disappeared from latest (3.9) python
documentation https://docs.python.org/3.9/library/argparse.html , but exists
here https://docs.python.org/3.8/library/argparse.html (and no mark of
deprecation)
Whoa. https://bugs.python.org/issue17050 says:
---
Since this feature is buggy, and there isn't an easy fix, we should probably
remove any mention of it from the docs. We can still leave it as an
undocumented legacy feature.
There is precedent for leaving `nargs` constants undocumented.
`argparse.PARSER` ('+...') is used by the subparser mechanism, but is not
documented. https://bugs.python.org/issue16988
---
The problematic case appears to be when you have more than one positional
argument, which is exactly the case with the 3.8 documented use of REMAINDER.
Hence the decision to drop the documentation.
However, "check" works fine because the REMAINDER argument is the only
positional argument:
$ ./check 001 -d
Test "tests/-d" is not found
Another possibility is to pre-process sys.argv like this:
if '--' in sys.argv:
cmd = True
args = sys.argv[0:sys.argv.index('--')]
posargs = sys.argv[len(args)+1:]
else:
cmd = False
args = list(x for x in sys.argv if x.startswith('-'))
posargs = list(x for x in sys.argv if not x.startswith('-'))
But getting the help message right etc. would be messy.
Paolo
Hmm:
If you have positional arguments that must begin with - and don’t look like
negative numbers, you can insert the pseudo-argument '--' which tells
parse_args() that everything after that is a positional argument:
So, as I understand argparse supports '--' feature out of the box. So, we can
keep '*' as is, and it would parse all remaining positional arguments which are
either tests or the command, and '--' will be automatically dropped. So, we
only need to check existing of '--' in original sys.argv to chose our behavior.
--
Best regards,
Vladimir
[PATCH v2 2/5] qemu-iotests: allow passing unittest.main arguments to the test scripts
Python test scripts that use unittest consist of multiple tests. unittest.main allows selecting which tests to run, but currently this is not possible because the iotests wrapper ignores sys.argv. unittest.main command line options also allow the user to pick the desired options for verbosity, failfast mode, etc. While "-d" is currently translated to "-v", it also enables extra debug output, and other options are not available at all. These command line options only work if the unittest.main testRunner argument is a type, rather than a TestRunner instance. Therefore, pass the class name and "verbosity" argument to unittest.main, and adjust for the different default warnings between TextTestRunner and unittest.main. Signed-off-by: Paolo Bonzini --- tests/qemu-iotests/iotests.py | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index 0521235030..c7915684ba 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -1308,12 +1308,16 @@ def __init__(self, stream: Optional[io.TextIOBase] = None, resultclass=resultclass, *args, **kwargs) -def execute_unittest(debug=False): +def execute_unittest(argv: List[str], debug: bool= False): """Executes unittests within the calling module.""" -verbosity = 2 if debug else 1 -runner = unittest.ReproducibleTestRunner(verbosity=verbosity) -unittest.main(testRunner=runner) +# Some tests have warnings, especially ResourceWarnings for unclosed +# files and sockets. Ignore them for now to ensure reproducibility of +# the test output. +unittest.main(argv=argv, + testRunner=ReproducibleTestRunner, + verbosity=2 if debug else 1, + warnings=None if sys.warnoptions else 'ignore') def execute_setup_common(supported_fmts: Sequence[str] = (), supported_platforms: Sequence[str] = (), @@ -1350,7 +1354,7 @@ def execute_test(*args, test_function=None, **kwargs): debug = execute_setup_common(*args, **kwargs) if not test_function: -execute_unittest(debug) +execute_unittest(sys.argv, debug) else: test_function() -- 2.30.1
Re: [PATCH 1/4] qemu-iotests: allow passing unittest.main arguments to the test scripts
On 23/03/21 17:56, Vladimir Sementsov-Ogievskiy wrote:
hmm, just use a separate call of unittest.main, or honestly define a
varaible as Union[] or just Any ?
All the ugliness goes away if the implementation is done properly. :)
For me normal try-finally is a lot more clean than calling atexit() in a
class method. It's just a strange interface. Prior to the patch user can
call execute_unittest several times and expect that output will be
printed during the call. After the patch outputs of all calls to
execute_unittest() will be printed at program exit..
Yeah, I agree. I didn't like the finally, but I really didn't like it
because of the *behavior* of buffering the whole output. I have changed
it to drop the buffering altogether, that's much better code and more usable:
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 90d0b62523..a74f4f9488 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -32,7 +32,7 @@
import sys
import time
from typing import (Any, Callable, Dict, Iterable,
-List, Optional, Sequence, Tuple, TypeVar)
+List, Optional, Sequence, Tuple, Type, TypeVar)
import unittest
from contextlib import contextmanager
@@ -1271,37 +1271,49 @@ def func_wrapper(*args, **kwargs):
return func(*args, **kwargs)
return func_wrapper
+# We need to filter out the time taken from the output so that
+# qemu-iotest can reliably diff the results against master output,
+# and hide skipped tests from the reference output.
+
+class ReproducibleTestResult(unittest.TextTestResult):
+def addSkip(self, test, reason):
+# Same as TextTestResult, but print dot instead of "s"
+unittest.TestResult.addSkip(self, test, reason)
+if self.showAll:
+self.stream.writeln("skipped {0!r}".format(reason))
+elif self.dots:
+self.stream.write(".")
+self.stream.flush()
+
+class ReproducibleStreamWrapper(object):
+def __init__(self, stream):
+self.stream = stream
+
+def __getattr__(self, attr):
+if attr in ('stream', '__getstate__'):
+raise AttributeError(attr)
+return getattr(self.stream,attr)
+
+def write(self, arg=None):
+arg = re.sub(r'Ran (\d+) tests? in [\d.]+s', r'Ran \1 tests', arg)
+arg = re.sub(r' \(skipped=\d+\)', r'', arg)
+self.stream.write(arg)
+
+class ReproducibleTestRunner(unittest.TextTestRunner):
+def __init__(self, stream: Optional[io.TextIOBase] = None,
+ resultclass: Type = ReproducibleTestResult, *args, **kwargs):
+rstream = ReproducibleStreamWrapper(stream or sys.stdout)
+super().__init__(stream=rstream, # type: ignore
+ descriptions=True,
+ resultclass=resultclass,
+ *args, **kwargs)
+
def execute_unittest(debug=False):
"""Executes unittests within the calling module."""
verbosity = 2 if debug else 1
-
-if debug:
-output = sys.stdout
-else:
-# We need to filter out the time taken from the output so that
-# qemu-iotest can reliably diff the results against master output.
-output = io.StringIO()
-
-runner = unittest.TextTestRunner(stream=output, descriptions=True,
- verbosity=verbosity)
-try:
-# unittest.main() will use sys.exit(); so expect a SystemExit
-# exception
-unittest.main(testRunner=runner)
-finally:
-# We need to filter out the time taken from the output so that
-# qemu-iotest can reliably diff the results against master output.
-if not debug:
-out = output.getvalue()
-out = re.sub(r'Ran (\d+) tests? in [\d.]+s', r'Ran \1 tests', out)
-
-# Hide skipped tests from the reference output
-out = re.sub(r'OK \(skipped=\d+\)', 'OK', out)
-out_first_line, out_rest = out.split('\n', 1)
-out = out_first_line.replace('s', '.') + '\n' + out_rest
-
-sys.stderr.write(out)
+runner = unittest.ReproducibleTestRunner(verbosity=verbosity)
+unittest.main(testRunner=ReproducibleTestRunner)
def execute_setup_common(supported_fmts: Sequence[str] = (),
supported_platforms: Sequence[str] = (),
Re: [PATCH 3/4] qemu-iotests: let "check" spawn an arbitrary test command
On 23/03/21 17:43, Vladimir Sementsov-Ogievskiy wrote:
Interesting that REMAINDER documentation disappeared from latest (3.9)
python documentation https://docs.python.org/3.9/library/argparse.html ,
but exists here https://docs.python.org/3.8/library/argparse.html (and
no mark of deprecation)
Whoa. https://bugs.python.org/issue17050 says:
---
Since this feature is buggy, and there isn't an easy fix, we should
probably remove any mention of it from the docs. We can still leave it
as an undocumented legacy feature.
There is precedent for leaving `nargs` constants undocumented.
`argparse.PARSER` ('+...') is used by the subparser mechanism, but is
not documented. https://bugs.python.org/issue16988
---
The problematic case appears to be when you have more than one
positional argument, which is exactly the case with the 3.8 documented
use of REMAINDER. Hence the decision to drop the documentation.
However, "check" works fine because the REMAINDER argument is the only
positional argument:
$ ./check 001 -d
Test "tests/-d" is not found
Another possibility is to pre-process sys.argv like this:
if '--' in sys.argv:
cmd = True
args = sys.argv[0:sys.argv.index('--')]
posargs = sys.argv[len(args)+1:]
else:
cmd = False
args = list(x for x in sys.argv if x.startswith('-'))
posargs = list(x for x in sys.argv if not x.startswith('-'))
But getting the help message right etc. would be messy.
Paolo
[PATCH v2 21/22] iotests: iothreads need ioeventfd
From: Laurent Vivier
And ioeventfd are only available with virtio-scsi-pci or virtio-scsi-ccw,
use the alias but add a rule to require virtio-scsi-pci or virtio-scsi-ccw
for the tests that use iothreads.
Signed-off-by: Laurent Vivier
Reviewed-by: Thomas Huth
Message-Id: <20210319202335.2397060-7-laur...@vivier.eu>
Signed-off-by: Alex Bennée
---
tests/qemu-iotests/127| 3 ++-
tests/qemu-iotests/256| 6 --
tests/qemu-iotests/common.rc | 13 +
tests/qemu-iotests/iotests.py | 5 +
4 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/tests/qemu-iotests/127 b/tests/qemu-iotests/127
index 98e8e82a82..32edc3b068 100755
--- a/tests/qemu-iotests/127
+++ b/tests/qemu-iotests/127
@@ -44,7 +44,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
_supported_fmt qcow2
_supported_proto file fuse
-_require_devices virtio-scsi scsi-hd
+_require_devices scsi-hd
+_require_one_device_of virtio-scsi-pci virtio-scsi-ccw
IMG_SIZE=64K
diff --git a/tests/qemu-iotests/256 b/tests/qemu-iotests/256
index 8d82a1dd86..13666813bd 100755
--- a/tests/qemu-iotests/256
+++ b/tests/qemu-iotests/256
@@ -24,6 +24,8 @@ import os
import iotests
from iotests import log
+iotests._verify_virtio_scsi_pci_or_ccw()
+
iotests.script_initialize(supported_fmts=['qcow2'])
size = 64 * 1024 * 1024
@@ -61,8 +63,8 @@ with iotests.FilePath('img0') as img0_path, \
log('--- Preparing images & VM ---\n')
vm.add_object('iothread,id=iothread0')
vm.add_object('iothread,id=iothread1')
-vm.add_device('virtio-scsi-pci,id=scsi0,iothread=iothread0')
-vm.add_device('virtio-scsi-pci,id=scsi1,iothread=iothread1')
+vm.add_device('virtio-scsi,id=scsi0,iothread=iothread0')
+vm.add_device('virtio-scsi,id=scsi1,iothread=iothread1')
iotests.qemu_img_create('-f', iotests.imgfmt, img0_path, str(size))
iotests.qemu_img_create('-f', iotests.imgfmt, img1_path, str(size))
vm.add_drive(img0_path, interface='none')
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 65cdba5723..7f49c9716d 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -977,5 +977,18 @@ _require_devices()
done
}
+_require_one_device_of()
+{
+available=$($QEMU -M none -device help | \
+grep ^name | sed -e 's/^name "//' -e 's/".*$//')
+for device
+do
+if echo "$available" | grep -q "$device" ; then
+return
+fi
+done
+_notrun "$* not available"
+}
+
# make sure this script returns success
true
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 1e9e6a066e..5af0182895 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -1146,6 +1146,11 @@ def _verify_virtio_blk() -> None:
if 'virtio-blk' not in out:
notrun('Missing virtio-blk in QEMU binary')
+def _verify_virtio_scsi_pci_or_ccw() -> None:
+out = qemu_pipe('-M', 'none', '-device', 'help')
+if 'virtio-scsi-pci' not in out and 'virtio-scsi-ccw' not in out:
+notrun('Missing virtio-scsi-pci or virtio-scsi-ccw in QEMU binary')
+
def supports_quorum():
return 'quorum' in qemu_img_pipe('--help')
--
2.20.1
[PATCH v2 19/22] iotests: Revert "iotests: use -ccw on s390x for 040, 139, and 182"
From: Laurent Vivier
Commit f1d5516ab583 introduces a test in some iotests to check if
the machine is a s390-ccw-virtio and to select virtio-*-ccw rather
than virtio-*-pci.
We don't need that because QEMU already provides aliases to use the correct
virtio interface according to the machine type.
This patch removes all virtio-*-pci and virtio-*-ccw to use virtio-*
instead and remove get_virtio_scsi_device().
This also enables virtio-mmio devices (virtio-*-device)
Signed-off-by: Laurent Vivier
Tested-by: Philippe Mathieu-Daudé
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Cornelia Huck
Message-Id: <20210319202335.2397060-5-laur...@vivier.eu>
Signed-off-by: Alex Bennée
---
tests/qemu-iotests/040| 2 +-
tests/qemu-iotests/051| 12 +---
tests/qemu-iotests/051.out| 2 +-
tests/qemu-iotests/051.pc.out | 2 +-
tests/qemu-iotests/068| 4 +---
tests/qemu-iotests/093| 3 +--
tests/qemu-iotests/139| 9 ++---
tests/qemu-iotests/182| 13 ++---
tests/qemu-iotests/238| 4 +---
tests/qemu-iotests/240| 10 +-
tests/qemu-iotests/257| 4 ++--
tests/qemu-iotests/307| 4 +---
tests/qemu-iotests/iotests.py | 5 -
13 files changed, 19 insertions(+), 55 deletions(-)
diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index 336ff7c4f2..ba7cb34ce8 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -89,7 +89,7 @@ class TestSingleDrive(ImageCommitTestCase):
qemu_io('-f', 'raw', '-c', 'write -P 0xab 0 524288', backing_img)
qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0xef 524288 524288',
mid_img)
self.vm = iotests.VM().add_drive(test_img,
"node-name=top,backing.node-name=mid,backing.backing.node-name=base",
interface="none")
-self.vm.add_device(iotests.get_virtio_scsi_device())
+self.vm.add_device('virtio-scsi')
self.vm.add_device("scsi-hd,id=scsi0,drive=drive0")
self.vm.launch()
self.has_quit = False
diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
index f92161d8ef..333cc81818 100755
--- a/tests/qemu-iotests/051
+++ b/tests/qemu-iotests/051
@@ -119,17 +119,7 @@ echo
echo === Device without drive ===
echo
-case "$QEMU_DEFAULT_MACHINE" in
- s390-ccw-virtio)
- virtio_scsi=virtio-scsi-ccw
- ;;
- *)
- virtio_scsi=virtio-scsi-pci
- ;;
-esac
-
-run_qemu -device $virtio_scsi -device scsi-hd |
-sed -e "s/$virtio_scsi/VIRTIO_SCSI/"
+run_qemu -device virtio-scsi -device scsi-hd
echo
echo === Overriding backing file ===
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index de4771bcb3..437053c839 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -72,7 +72,7 @@ QEMU_PROG: -drive file=TEST_DIR/t.qcow2,node-name=foo#12:
Invalid node name
=== Device without drive ===
-Testing: -device VIRTIO_SCSI -device scsi-hd
+Testing: -device virtio-scsi -device scsi-hd
QEMU X.Y.Z monitor - type 'help' for more information
(qemu) QEMU_PROG: -device scsi-hd: drive property not set
diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out
index a28e3fc124..e95bd42b8d 100644
--- a/tests/qemu-iotests/051.pc.out
+++ b/tests/qemu-iotests/051.pc.out
@@ -72,7 +72,7 @@ QEMU_PROG: -drive file=TEST_DIR/t.qcow2,node-name=foo#12:
Invalid node-name: 'fo
=== Device without drive ===
-Testing: -device VIRTIO_SCSI -device scsi-hd
+Testing: -device virtio-scsi -device scsi-hd
QEMU X.Y.Z monitor - type 'help' for more information
(qemu) QEMU_PROG: -device scsi-hd: drive property not set
diff --git a/tests/qemu-iotests/068 b/tests/qemu-iotests/068
index 03e03508a6..54e49c8ffa 100755
--- a/tests/qemu-iotests/068
+++ b/tests/qemu-iotests/068
@@ -49,11 +49,9 @@ IMG_SIZE=128K
case "$QEMU_DEFAULT_MACHINE" in
s390-ccw-virtio)
platform_parm="-no-shutdown"
- hba=virtio-scsi-ccw
;;
*)
platform_parm=""
- hba=virtio-scsi-pci
;;
esac
@@ -61,7 +59,7 @@ _qemu()
{
$QEMU $platform_parm -nographic -monitor stdio -serial none \
-drive if=none,id=drive0,file="$TEST_IMG",format="$IMGFMT" \
- -device $hba,id=hba0 \
+ -device virtio-scsi,id=hba0 \
-device scsi-hd,drive=drive0 \
"$@" |\
_filter_qemu | _filter_hmp
diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093
index 7745cb04b6..93274dc8cb 100755
--- a/tests/qemu-iotests/093
+++ b/tests/qemu-iotests/093
@@ -371,8 +371,7 @@ class ThrottleTestGroupNames(iotests.QMPTestCase):
class ThrottleTestRemovableMedia(iotests.QMPTestCase):
def setUp(self):
self.vm = iotests.VM()
-self.vm.add_device("{},id=virtio-scsi".format(
-iotests.get_virtio_scsi_device()))
+self.vm.add_device("{},id=virtio-scsi".format('virtio-scsi'))
self.vm.launch()
def tearDown(self):
diff --git a/tests/qemu-iotests/139 b/tests/qemu-iotests/139
index
[PATCH v2 20/22] iotests: test m68k with the virt machine
From: Laurent Vivier
This allows to cover the virtio tests with a 32bit big-endian
virtio-mmio machine.
Signed-off-by: Laurent Vivier
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Cornelia Huck
Message-Id: <20210319202335.2397060-6-laur...@vivier.eu>
Signed-off-by: Alex Bennée
---
tests/qemu-iotests/testenv.py | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index 1fbec854c1..6d27712617 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -208,6 +208,7 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: str,
('arm', 'virt'),
('aarch64', 'virt'),
('avr', 'mega2560'),
+('m68k', 'virt'),
('rx', 'gdbsim-r5f562n8'),
('tricore', 'tricore_testboard')
)
--
2.20.1
[PATCH v2 18/22] blockdev: with -drive if=virtio, use generic virtio-blk
From: Laurent Vivier
Rather than checking if the machine is an s390x to use virtio-blk-ccw
instead of virtio-blk-pci, use the alias virtio-blk that is set to
the expected target.
This also enables the use of virtio-blk-device for targets without
PCI or CCW.
Signed-off-by: Laurent Vivier
Tested-by: Philippe Mathieu-Daudé
Reviewed-by: Philippe Mathieu-Daudé
Acked-by: Markus Armbruster
Message-Id: <20210319202335.2397060-4-laur...@vivier.eu>
Signed-off-by: Alex Bennée
---
blockdev.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index cf70bb4e43..413aacf604 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -962,11 +962,7 @@ DriveInfo *drive_new(QemuOpts *all_opts,
BlockInterfaceType block_default_type,
QemuOpts *devopts;
devopts = qemu_opts_create(qemu_find_opts("device"), NULL, 0,
_abort);
-if (arch_type == QEMU_ARCH_S390X) {
-qemu_opt_set(devopts, "driver", "virtio-blk-ccw", _abort);
-} else {
-qemu_opt_set(devopts, "driver", "virtio-blk-pci", _abort);
-}
+qemu_opt_set(devopts, "driver", "virtio-blk", _abort);
qemu_opt_set(devopts, "drive", qdict_get_str(bs_opts, "id"),
_abort);
}
--
2.20.1
Re: [PATCH 1/4] qemu-iotests: allow passing unittest.main arguments to the test scripts
23.03.2021 18:29, Paolo Bonzini wrote: On 23/03/21 15:34, Vladimir Sementsov-Ogievskiy wrote: + def __init__(self, *args, **kwargs): + super().__init__(stream=ReproducibleTextTestRunner.output, *args, **kwargs) over-79 line (PEP8) Ok, thanks. + +def execute_unittest(argv, debug=False): + """Executes unittests within the calling module.""" + + # If we see non-empty argv we must not be invoked as a test script, + # so do not bother making constant output; debuggability is more + # important. + if debug or len(argv) > 1: It's native to rely on converting sequences to bool. Empty sequence is False and non-empty is True, just like if debug or argv: No, this is checking if there is *more than one* element in argv, because argv contains the program name as argv[0]. It's trying to catch the case of "./run testclass.TestMethod" or "./run -v" and not buffer the output, but it sucks. Really this patchset should have been marked as RFC. A better implementation is to create a class that wraps sys.stdout and overrides write() to ensure reproducibility. There is no need to buffer the output in the StringIO at all. + argv += ['-v'] + runner = unittest.TextTestRunner + else: + runner = ReproducibleTextTestRunner + + unittest.main(argv=argv, testRunner=runner, + warnings=None if sys.warnoptions else 'ignore') This thing about warnings seems unrelated change and not described in the commit message The default settings for warnings is different when you instantiate TextTestRunner and when you use unittest.main. Currently the tests have some warnings, they need to be disabled otherwise the tests fail. Honestly, I don't have time to debug the warnings and they are pre-existing anyway. But you're right, at least I should have a comment describing the purpose of the warnings keyword argument. + def execute_setup_common(supported_fmts: Sequence[str] = (), supported_platforms: Sequence[str] = (), supported_cache_modes: Sequence[str] = (), @@ -1338,7 +1348,7 @@ def execute_test(*args, test_function=None, **kwargs): debug = execute_setup_common(*args, **kwargs) if not test_function: - execute_unittest(debug) + execute_unittest(sys.argv, debug) else: test_function() Why isn't it simpler just to add argv argument to original function, and on "debug or argv" path just pass unittest.TextTestRunner instead of constructing the object? Why do we need new class and switching to atexit()? mypy complains because you set the variable to two different types on the two branches. hmm, just use a separate call of unittest.main, or honestly define a varaible as Union[] or just Any ? So I decided it was cleaner to write a new runner class. I think this is the only remaining part of the patch that I like. :) For me normal try-finally is a lot more clean than calling atexit() in a class method. It's just a strange interface. Prior to the patch user can call execute_unittest several times and expect that output will be printed during the call. After the patch outputs of all calls to execute_unittest() will be printed at program exit.. -- Best regards, Vladimir
Re: [PATCH 3/4] qemu-iotests: let "check" spawn an arbitrary test command
On 23/03/21 18:11, Vladimir Sementsov-Ogievskiy wrote: If you have positional arguments that must begin with - and don’t look like negative numbers, you can insert the pseudo-argument '--' which tells parse_args() that everything after that is a positional argument: So, as I understand argparse supports '--' feature out of the box. So, we can keep '*' as is, and it would parse all remaining positional arguments which are either tests or the command, and '--' will be automatically dropped. So, we only need to check existing of '--' in original sys.argv to chose our behavior. There is still a difference with REMAINDER: ./check aa -- bb => REMAINDER: error because ./-- is not a test => look for '--': invoke "aa -- bb" So I think REMAINDER provides the best behavior overall. Paolo
Re: [PATCH 4/4] qemu-iotests: fix case of SOCK_DIR already in the environment
23.03.2021 16:06, Paolo Bonzini wrote: Due to a typo, in this case the SOCK_DIR was not being created. Signed-off-by: Paolo Bonzini --- tests/qemu-iotests/testenv.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py index 6767eeeb25..169268f61a 100644 --- a/tests/qemu-iotests/testenv.py +++ b/tests/qemu-iotests/testenv.py @@ -120,7 +120,7 @@ def init_directories(self) -> None: try: self.sock_dir = os.environ['SOCK_DIR'] self.tmp_sock_dir = False -Path(self.test_dir).mkdir(parents=True, exist_ok=True) +Path(self.sock_dir).mkdir(parents=True, exist_ok=True) except KeyError: self.sock_dir = tempfile.mkdtemp() self.tmp_sock_dir = True Reviewed-by: Vladimir Sementsov-Ogievskiy -- Best regards, Vladimir
Re: [PATCH 2/4] qemu-iotests: move command line and environment handling from TestRunner to TestEnv
23.03.2021 16:06, Paolo Bonzini wrote:
In the next patch, "check" will learn how to execute a test script without
going through TestRunner. To enable this, keep only the text output
and subprocess handling in the TestRunner; move into TestEnv the logic
to prepare for running a subprocess.
Signed-off-by: Paolo Bonzini
---
tests/qemu-iotests/testenv.py| 20 ++--
tests/qemu-iotests/testrunner.py | 15 +--
2 files changed, 19 insertions(+), 16 deletions(-)
diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index 1fbec854c1..6767eeeb25 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -25,7 +25,7 @@
import random
import subprocess
import glob
-from typing import Dict, Any, Optional, ContextManager
+from typing import List, Dict, Any, Optional, ContextManager
def isxfile(path: str) -> bool:
@@ -74,6 +74,21 @@ class TestEnv(ContextManager['TestEnv']):
'CACHEMODE_IS_DEFAULT', 'IMGFMT_GENERIC',
'IMGOPTSSYNTAX',
'IMGKEYSECRET', 'QEMU_DEFAULT_MACHINE',
'MALLOC_PERTURB_']
+def prepare_subprocess(self, args: List[str]) -> Dict[str, str]:
+if self.debug:
+args.append('-d')
+
+with open(args[0], encoding="utf-8") as f:
+try:
+if f.readline().rstrip() == '#!/usr/bin/env python3':
+args.insert(0, self.python)
+except UnicodeDecodeError: # binary test? for future.
+pass
+
+os_env = os.environ.copy()
+os_env.update(self.get_env())
+return os_env
+
def get_env(self) -> Dict[str, str]:
env = {}
for v in self.env_variables:
@@ -268,7 +283,8 @@ 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}
+"""
Unrelated change.. Better be in another commit or at least noted in commit msg.
You also updated only one of two callers, so output will change after this
patch. Seems it should become better..
args = collections.defaultdict(str, self.get_env())
diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
index 1fc61fcaa3..2f56ac545d 100644
--- a/tests/qemu-iotests/testrunner.py
+++ b/tests/qemu-iotests/testrunner.py
@@ -129,7 +129,6 @@ class TestRunner(ContextManager['TestRunner']):
def __init__(self, env: TestEnv, makecheck: bool = False,
color: str = 'auto') -> None:
self.env = env
-self.test_run_env = self.env.get_env()
self.makecheck = makecheck
self.last_elapsed = LastElapsedTime('.last-elapsed-cache', env)
@@ -243,18 +242,7 @@ def do_run_test(self, test: str) -> TestResult:
silent_unlink(p)
args = [str(f_test.resolve())]
-if self.env.debug:
-args.append('-d')
-
-with f_test.open(encoding="utf-8") as f:
-try:
-if f.readline().rstrip() == '#!/usr/bin/env python3':
-args.insert(0, self.env.python)
-except UnicodeDecodeError: # binary test? for future.
-pass
-
-env = os.environ.copy()
-env.update(self.test_run_env)
+env = self.env.prepare_subprocess(args)
t0 = time.time()
with f_bad.open('w', encoding="utf-8") as f:
@@ -328,7 +316,6 @@ def run_tests(self, tests: List[str]) -> bool:
if not self.makecheck:
self.env.print_env()
-print()
test_field_width = max(len(os.path.basename(t)) for t in tests) + 2
Better without changing empty line, but still I'm OK with the patch as is:
Reviewed-by: Vladimir Sementsov-Ogievskiy
--
Best regards,
Vladimir
Re: [PATCH 3/4] qemu-iotests: let "check" spawn an arbitrary test command
23.03.2021 16:06, Paolo Bonzini wrote:
Right now there is no easy way for "check" to print a reproducer command.
Because such a reproducer command line would be huge, we can instead teach
check to start a command of our choice. This can be for example a Python
unit test with arguments to only run a specific subtest.
Signed-off-by: Paolo Bonzini
---
tests/qemu-iotests/check | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index d1c87ceaf1..aab25dac6a 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -19,6 +19,7 @@
import os
import sys
import argparse
+import shutil
from findtests import TestFinder
from testenv import TestEnv
from testrunner import TestRunner
@@ -100,8 +101,8 @@ def make_argparser() -> argparse.ArgumentParser:
'one to TEST (not inclusive). This may be used to '
'rerun failed ./check command, starting from the '
'middle of the process.')
-g_sel.add_argument('tests', metavar='TEST_FILES', nargs='*',
- help='tests to run')
+g_sel.add_argument('tests', nargs=argparse.REMAINDER,
Interesting that REMAINDER documentation disappeared from latest (3.9) python
documentation https://docs.python.org/3.9/library/argparse.html , but exists
here https://docs.python.org/3.8/library/argparse.html (and no mark of
deprecation)
+ help='tests to run, or "--" followed by a command')
return p
@@ -114,6 +115,17 @@ if __name__ == '__main__':
imgopts=args.imgopts, misalign=args.misalign,
debug=args.debug, valgrind=args.valgrind)
+if args.tests[0] == '--':
+del args.tests[0]
+cmd = args.tests
+env.print_env()
+exec_path = shutil.which(cmd[0])
+if exec_path is None:
+sys.exit('command not found: ' + cmd[0])
+cmd[0] = exec_path
+full_env = env.prepare_subprocess(cmd)
+os.execve(cmd[0], cmd, full_env)
+
testfinder = TestFinder(test_dir=env.source_iotests)
groups = args.groups.split(',') if args.groups else None
I hope at some moment we'll run testcases with syntax like
./check -qcow2 test_name.ClassName.method_name
But a possibility to run any command under check script defined environment is
a good thing anyway:
Reviewed-by: Vladimir Sementsov-Ogievskiy
--
Best regards,
Vladimir
Re: [PATCH 1/4] qemu-iotests: allow passing unittest.main arguments to the test scripts
23.03.2021 18:29, Paolo Bonzini wrote: On 23/03/21 15:34, Vladimir Sementsov-Ogievskiy wrote: + def __init__(self, *args, **kwargs): + super().__init__(stream=ReproducibleTextTestRunner.output, *args, **kwargs) over-79 line (PEP8) Ok, thanks. + +def execute_unittest(argv, debug=False): + """Executes unittests within the calling module.""" + + # If we see non-empty argv we must not be invoked as a test script, + # so do not bother making constant output; debuggability is more + # important. + if debug or len(argv) > 1: It's native to rely on converting sequences to bool. Empty sequence is False and non-empty is True, just like if debug or argv: No, this is checking if there is *more than one* Ah, oops, yes, right :\ element in argv, because argv contains the program name as argv[0]. It's trying to catch the case of "./run testclass.TestMethod" or "./run -v" and not buffer the output, but it sucks. Really this patchset should have been marked as RFC. A better implementation is to create a class that wraps sys.stdout and overrides write() to ensure reproducibility. There is no need to buffer the output in the StringIO at all. + argv += ['-v'] + runner = unittest.TextTestRunner + else: + runner = ReproducibleTextTestRunner + + unittest.main(argv=argv, testRunner=runner, + warnings=None if sys.warnoptions else 'ignore') This thing about warnings seems unrelated change and not described in the commit message The default settings for warnings is different when you instantiate TextTestRunner and when you use unittest.main. Currently the tests have some warnings, they need to be disabled otherwise the tests fail. Honestly, I don't have time to debug the warnings and they are pre-existing anyway. But you're right, at least I should have a comment describing the purpose of the warnings keyword argument. + def execute_setup_common(supported_fmts: Sequence[str] = (), supported_platforms: Sequence[str] = (), supported_cache_modes: Sequence[str] = (), @@ -1338,7 +1348,7 @@ def execute_test(*args, test_function=None, **kwargs): debug = execute_setup_common(*args, **kwargs) if not test_function: - execute_unittest(debug) + execute_unittest(sys.argv, debug) else: test_function() Why isn't it simpler just to add argv argument to original function, and on "debug or argv" path just pass unittest.TextTestRunner instead of constructing the object? Why do we need new class and switching to atexit()? mypy complains because you set the variable to two different types on the two branches. So I decided it was cleaner to write a new runner class. I think this is the only remaining part of the patch that I like. :) Thanks, Paolo -- Best regards, Vladimir
Re: [PATCH 1/4] qemu-iotests: allow passing unittest.main arguments to the test scripts
On 23/03/21 15:34, Vladimir Sementsov-Ogievskiy wrote: + def __init__(self, *args, **kwargs): + super().__init__(stream=ReproducibleTextTestRunner.output, *args, **kwargs) over-79 line (PEP8) Ok, thanks. + +def execute_unittest(argv, debug=False): + """Executes unittests within the calling module.""" + + # If we see non-empty argv we must not be invoked as a test script, + # so do not bother making constant output; debuggability is more + # important. + if debug or len(argv) > 1: It's native to rely on converting sequences to bool. Empty sequence is False and non-empty is True, just like if debug or argv: No, this is checking if there is *more than one* element in argv, because argv contains the program name as argv[0]. It's trying to catch the case of "./run testclass.TestMethod" or "./run -v" and not buffer the output, but it sucks. Really this patchset should have been marked as RFC. A better implementation is to create a class that wraps sys.stdout and overrides write() to ensure reproducibility. There is no need to buffer the output in the StringIO at all. + argv += ['-v'] + runner = unittest.TextTestRunner + else: + runner = ReproducibleTextTestRunner + + unittest.main(argv=argv, testRunner=runner, + warnings=None if sys.warnoptions else 'ignore') This thing about warnings seems unrelated change and not described in the commit message The default settings for warnings is different when you instantiate TextTestRunner and when you use unittest.main. Currently the tests have some warnings, they need to be disabled otherwise the tests fail. Honestly, I don't have time to debug the warnings and they are pre-existing anyway. But you're right, at least I should have a comment describing the purpose of the warnings keyword argument. + def execute_setup_common(supported_fmts: Sequence[str] = (), supported_platforms: Sequence[str] = (), supported_cache_modes: Sequence[str] = (), @@ -1338,7 +1348,7 @@ def execute_test(*args, test_function=None, **kwargs): debug = execute_setup_common(*args, **kwargs) if not test_function: - execute_unittest(debug) + execute_unittest(sys.argv, debug) else: test_function() Why isn't it simpler just to add argv argument to original function, and on "debug or argv" path just pass unittest.TextTestRunner instead of constructing the object? Why do we need new class and switching to atexit()? mypy complains because you set the variable to two different types on the two branches. So I decided it was cleaner to write a new runner class. I think this is the only remaining part of the patch that I like. :) Thanks, Paolo
Re: [PATCH 1/4] qemu-iotests: allow passing unittest.main arguments to the test scripts
23.03.2021 16:06, Paolo Bonzini wrote:
Python test scripts that use unittest consist of multiple tests.
unittest.main allows selecting which tests to run, but currently this
is not possible because the iotests wrapper ignores sys.argv.
unittest.main command line options also allow the user to pick the
desired options for verbosity, failfast mode, etc. While "-d" is
currently translated to "-v", it also enables extra debug output,
and also there are no other options available. Unfortunately, these
command line options only work if the unittest.main testRunner argument
is a type, rather than a TestRunner instance, and right now iotests.py
is using a TextTestRunner instance in order to pass the output stream.
By moving the machinery to obtain reproducible unittest output into a
TextTestRunner subclass, we can just pass the class name to unittest.main.
Signed-off-by: Paolo Bonzini
Great that you are working on it!
I keep in mind the necessity of supporting running test-cases in separate, but
never actually start doing it.
And in my dreams I usually just drop the
"""
...
--
Ran 3 tests
OK
"""
output at all, as it gives NO information.
When unittest-based test fails we have a native backtrace, and nothing more
needed. (OK, information about crashed process counts too).
But actually, we don't need all these .out for unittest-based tests.
So, I'd drop it. But this is more work, and includes updating one-two
unittest-based iotests that still have reasonable output, so this patch is good
anyway.
---
tests/qemu-iotests/iotests.py | 60 ---
1 file changed, 35 insertions(+), 25 deletions(-)
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 90d0b62523..b9f4edfc42 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -1271,38 +1271,49 @@ def func_wrapper(*args, **kwargs):
return func(*args, **kwargs)
return func_wrapper
-def execute_unittest(debug=False):
-"""Executes unittests within the calling module."""
-
-verbosity = 2 if debug else 1
-
-if debug:
-output = sys.stdout
-else:
-# We need to filter out the time taken from the output so that
-# qemu-iotest can reliably diff the results against master output.
-output = io.StringIO()
-
-runner = unittest.TextTestRunner(stream=output, descriptions=True,
- verbosity=verbosity)
-try:
-# unittest.main() will use sys.exit(); so expect a SystemExit
-# exception
-unittest.main(testRunner=runner)
-finally:
-# We need to filter out the time taken from the output so that
-# qemu-iotest can reliably diff the results against master output.
-if not debug:
-out = output.getvalue()
+# We need to filter out the time taken from the output so that
+# qemu-iotest can reliably diff the results against master output.
+class ReproducibleTextTestRunner(unittest.TextTestRunner):
+__output = None
+
+@classmethod
+@property
+def output(cls):
+if cls.__output is not None:
+return cls.__output
+
+cls.__output = io.StringIO()
+def print_output():
+out = cls.__output.getvalue()
out = re.sub(r'Ran (\d+) tests? in [\d.]+s', r'Ran \1 tests', out)
# Hide skipped tests from the reference output
out = re.sub(r'OK \(skipped=\d+\)', 'OK', out)
out_first_line, out_rest = out.split('\n', 1)
out = out_first_line.replace('s', '.') + '\n' + out_rest
-
sys.stderr.write(out)
+atexit.register(print_output)
+return cls.__output
+
+def __init__(self, *args, **kwargs):
+super().__init__(stream=ReproducibleTextTestRunner.output, *args,
**kwargs)
over-79 line (PEP8)
+
+def execute_unittest(argv, debug=False):
+"""Executes unittests within the calling module."""
+
+# If we see non-empty argv we must not be invoked as a test script,
+# so do not bother making constant output; debuggability is more
+# important.
+if debug or len(argv) > 1:
It's native to rely on converting sequences to bool. Empty sequence is False
and non-empty is True, just like
if debug or argv:
+argv += ['-v']
+runner = unittest.TextTestRunner
+else:
+runner = ReproducibleTextTestRunner
+
+unittest.main(argv=argv, testRunner=runner,
+ warnings=None if sys.warnoptions else 'ignore')
This thing about warnings seems unrelated change and not described in the
commit message
+
def execute_setup_common(supported_fmts: Sequence[str] = (),
supported_platforms: Sequence[str] = (),
supported_cache_modes: Sequence[str] = (),
@@ -1338,7 +1348,7 @@ def execute_test(*args,
[PATCH v3 8/9] simplebench/bench-backup: add --drop-caches argument
Add an option to drop caches before each test run. It may probably
improve reliability of results when testing in cached mode.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
scripts/simplebench/bench-backup.py | 6 +-
scripts/simplebench/simplebench.py | 11 ++-
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/scripts/simplebench/bench-backup.py
b/scripts/simplebench/bench-backup.py
index 092fed5816..5a0675c593 100755
--- a/scripts/simplebench/bench-backup.py
+++ b/scripts/simplebench/bench-backup.py
@@ -156,7 +156,8 @@ def bench(args):
})
result = simplebench.bench(bench_func, test_envs, test_cases,
- count=args.count, initial_run=args.initial_run)
+ count=args.count, initial_run=args.initial_run,
+ drop_caches=args.drop_caches)
with open('results.json', 'w') as f:
json.dump(result, f, indent=4)
print(results_to_text(result))
@@ -221,4 +222,7 @@ def __call__(self, parser, namespace, values,
option_string=None):
Do additional initial run per cell which doesn't count in result,
default true''')
+p.add_argument('--drop-caches', action='store_true', help='''\
+Do "sync; echo 3 > /proc/sys/vm/drop_caches" before each test run''')
+
bench(p.parse_args())
diff --git a/scripts/simplebench/simplebench.py
b/scripts/simplebench/simplebench.py
index 27bc4d4715..8efca2af98 100644
--- a/scripts/simplebench/simplebench.py
+++ b/scripts/simplebench/simplebench.py
@@ -19,11 +19,17 @@
#
import statistics
+import subprocess
import time
+def do_drop_caches():
+subprocess.run('sync; echo 3 > /proc/sys/vm/drop_caches', shell=True,
+ check=True)
+
+
def bench_one(test_func, test_env, test_case, count=5, initial_run=True,
- slow_limit=100):
+ slow_limit=100, drop_caches=False):
"""Benchmark one test-case
test_func -- benchmarking function with prototype
@@ -40,6 +46,7 @@ def bench_one(test_func, test_env, test_case, count=5,
initial_run=True,
initial_run -- do initial run of test_func, which don't get into result
slow_limit -- stop at slow run (that exceedes the slow_limit by seconds).
(initial run is not measured)
+drop_caches -- drop caches before each run
Returns dict with the following fields:
'runs': list of test_func results
@@ -53,6 +60,7 @@ def bench_one(test_func, test_env, test_case, count=5,
initial_run=True,
"""
if initial_run:
print(' #initial run:')
+do_drop_caches()
print(' ', test_func(test_env, test_case))
runs = []
@@ -60,6 +68,7 @@ def bench_one(test_func, test_env, test_case, count=5,
initial_run=True,
t = time.time()
print(' #run {}'.format(i+1))
+do_drop_caches()
res = test_func(test_env, test_case)
print(' ', res)
runs.append(res)
--
2.29.2
[PATCH v3 7/9] simplebench/bench-backup: add --count and --no-initial-run
Add arguments to set number of test runs per table cell and to disable
initial run that is not counted in results.
It's convenient to set --count 1 --no-initial-run to fast run test
onece, and to set --count to some large enough number for good
precision of the results.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
scripts/simplebench/bench-backup.py | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/scripts/simplebench/bench-backup.py
b/scripts/simplebench/bench-backup.py
index a2120fcbf0..092fed5816 100755
--- a/scripts/simplebench/bench-backup.py
+++ b/scripts/simplebench/bench-backup.py
@@ -155,7 +155,8 @@ def bench(args):
'qemu-binary': path
})
-result = simplebench.bench(bench_func, test_envs, test_cases, count=3)
+result = simplebench.bench(bench_func, test_envs, test_cases,
+ count=args.count, initial_run=args.initial_run)
with open('results.json', 'w') as f:
json.dump(result, f, indent=4)
print(results_to_text(result))
@@ -211,4 +212,13 @@ def __call__(self, parser, namespace, values,
option_string=None):
both: generate two test cases for each src:dst pair''',
default='direct', choices=('direct', 'cached', 'both'))
+p.add_argument('--count', type=int, default=3, help='''\
+Number of test runs per table cell''')
+
+# BooleanOptionalAction helps to support --no-initial-run option
+p.add_argument('--initial-run', action=argparse.BooleanOptionalAction,
+ help='''\
+Do additional initial run per cell which doesn't count in result,
+default true''')
+
bench(p.parse_args())
--
2.29.2
[PATCH v3 5/9] simplebench/bench_block_job: handle error in BLOCK_JOB_COMPLETED
We should not report success if there is an error in final event.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: John Snow
---
scripts/simplebench/bench_block_job.py | 4
1 file changed, 4 insertions(+)
diff --git a/scripts/simplebench/bench_block_job.py
b/scripts/simplebench/bench_block_job.py
index 8f8385ccce..71d2e489c8 100755
--- a/scripts/simplebench/bench_block_job.py
+++ b/scripts/simplebench/bench_block_job.py
@@ -70,6 +70,10 @@ def bench_block_job(cmd, cmd_args, qemu_args):
vm.shutdown()
return {'error': 'block-job failed: ' + str(e),
'vm-log': vm.get_log()}
+if 'error' in e['data']:
+vm.shutdown()
+return {'error': 'block-job failed: ' + e['data']['error'],
+'vm-log': vm.get_log()}
end_ms = e['timestamp']['seconds'] * 100 + \
e['timestamp']['microseconds']
finally:
--
2.29.2
[PATCH v3 2/9] simplebench: bench_one(): support count=1
statistics.stdev raises if sequence length is less than two. Support that case by hand. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: John Snow --- scripts/simplebench/simplebench.py | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/simplebench/simplebench.py b/scripts/simplebench/simplebench.py index 0a3035732c..27bc4d4715 100644 --- a/scripts/simplebench/simplebench.py +++ b/scripts/simplebench/simplebench.py @@ -83,7 +83,10 @@ def bench_one(test_func, test_env, test_case, count=5, initial_run=True, dim = 'seconds' result['dimension'] = dim result['average'] = statistics.mean(r[dim] for r in succeeded) -result['stdev'] = statistics.stdev(r[dim] for r in succeeded) +if len(succeeded) == 1: +result['stdev'] = 0 +else: +result['stdev'] = statistics.stdev(r[dim] for r in succeeded) if len(succeeded) < count: result['n-failed'] = count - len(succeeded) -- 2.29.2
[PATCH v3 9/9] MAINTAINERS: update Benchmark util: add git tree
Signed-off-by: Vladimir Sementsov-Ogievskiy --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 9147e9a429..7d83d64d14 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2531,6 +2531,7 @@ Benchmark util M: Vladimir Sementsov-Ogievskiy S: Maintained F: scripts/simplebench/ +T: git https://src.openvz.org/scm/~vsementsov/qemu.git simplebench QAPI M: Markus Armbruster -- 2.29.2
[PATCH v3 6/9] simplebench/bench-backup: support qcow2 source files
Add support for qcow2 source. New option says to use test-source.qcow2
instead of test-source. Of course, test-source.qcow2 should be
precreated.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: John Snow
---
scripts/simplebench/bench-backup.py| 5 +
scripts/simplebench/bench_block_job.py | 7 ++-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/scripts/simplebench/bench-backup.py
b/scripts/simplebench/bench-backup.py
index fbc85f266f..a2120fcbf0 100755
--- a/scripts/simplebench/bench-backup.py
+++ b/scripts/simplebench/bench-backup.py
@@ -58,6 +58,8 @@ def bench(args):
if src == 'nbd':
source = nbd_drv
+elif args.qcow2_sources:
+source = drv_qcow2(drv_file(dirs[src] + '/test-source.qcow2'))
else:
source = drv_file(dirs[src] + '/test-source')
@@ -199,6 +201,9 @@ def __call__(self, parser, namespace, values,
option_string=None):
Use compressed backup. It automatically means
automatically creating qcow2 target with
lazy_refcounts for each test run''', action='store_true')
+p.add_argument('--qcow2-sources', help='''\
+Use test-source.qcow2 images as sources instead of
+test-source raw images''', action='store_true')
p.add_argument('--target-cache', help='''\
Setup cache for target nodes. Options:
direct: default, use O_DIRECT and aio=native
diff --git a/scripts/simplebench/bench_block_job.py
b/scripts/simplebench/bench_block_job.py
index 71d2e489c8..4f03c12169 100755
--- a/scripts/simplebench/bench_block_job.py
+++ b/scripts/simplebench/bench_block_job.py
@@ -88,6 +88,11 @@ def get_image_size(path):
return json.loads(out)['virtual-size']
+def get_blockdev_size(obj):
+img = obj['filename'] if 'filename' in obj else obj['file']['filename']
+return get_image_size(img)
+
+
# Bench backup or mirror
def bench_block_copy(qemu_binary, cmd, cmd_options, source, target):
"""Helper to run bench_block_job() for mirror or backup"""
@@ -101,7 +106,7 @@ def bench_block_copy(qemu_binary, cmd, cmd_options, source,
target):
subprocess.run(['qemu-img', 'create', '-f', 'qcow2',
target['file']['filename'],
-str(get_image_size(source['filename']))],
+str(get_blockdev_size(source))],
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL, check=True)
--
2.29.2
[PATCH v3 3/9] simplebench/bench-backup: add --compressed option
Allow bench compressed backup.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
scripts/simplebench/bench-backup.py| 55 ++
scripts/simplebench/bench_block_job.py | 23 +++
2 files changed, 62 insertions(+), 16 deletions(-)
diff --git a/scripts/simplebench/bench-backup.py
b/scripts/simplebench/bench-backup.py
index 33a1ecfefa..72eae85bb1 100755
--- a/scripts/simplebench/bench-backup.py
+++ b/scripts/simplebench/bench-backup.py
@@ -23,7 +23,7 @@
import simplebench
from results_to_text import results_to_text
-from bench_block_job import bench_block_copy, drv_file, drv_nbd
+from bench_block_job import bench_block_copy, drv_file, drv_nbd, drv_qcow2
def bench_func(env, case):
@@ -37,29 +37,41 @@ def bench_func(env, case):
def bench(args):
test_cases = []
-sources = {}
-targets = {}
-for d in args.dir:
-label, path = d.split(':') # paths with colon not supported
-sources[label] = drv_file(path + '/test-source')
-targets[label] = drv_file(path + '/test-target')
+# paths with colon not supported, so we just split by ':'
+dirs = dict(d.split(':') for d in args.dir)
+nbd_drv = None
if args.nbd:
nbd = args.nbd.split(':')
host = nbd[0]
port = '10809' if len(nbd) == 1 else nbd[1]
-drv = drv_nbd(host, port)
-sources['nbd'] = drv
-targets['nbd'] = drv
+nbd_drv = drv_nbd(host, port)
for t in args.test:
src, dst = t.split(':')
-test_cases.append({
-'id': t,
-'source': sources[src],
-'target': targets[dst]
-})
+if src == 'nbd' and dst == 'nbd':
+raise ValueError("Can't use 'nbd' label for both src and dst")
+
+if (src == 'nbd' or dst == 'nbd') and not nbd_drv:
+raise ValueError("'nbd' label used but --nbd is not given")
+
+if src == 'nbd':
+source = nbd_drv
+else:
+source = drv_file(dirs[src] + '/test-source')
+
+if dst == 'nbd':
+test_cases.append({'id': t, 'source': source, 'target': nbd_drv})
+continue
+
+fname = dirs[dst] + '/test-target'
+if args.compressed:
+fname += '.qcow2'
+target = drv_file(fname)
+if args.compressed:
+target = drv_qcow2(target)
+test_cases.append({'id': t, 'source': source, 'target': target})
binaries = [] # list of (, , [])
for i, q in enumerate(args.env):
@@ -106,6 +118,13 @@ def bench(args):
elif opt.startswith('max-workers='):
x_perf['max-workers'] = int(opt.split('=')[1])
+backup_options = {}
+if x_perf:
+backup_options['x-perf'] = x_perf
+
+if args.compressed:
+backup_options['compress'] = True
+
if is_mirror:
assert not x_perf
test_envs.append({
@@ -117,7 +136,7 @@ def bench(args):
test_envs.append({
'id': f'backup({label})\n' + '\n'.join(opts),
'cmd': 'blockdev-backup',
-'cmd-options': {'x-perf': x_perf} if x_perf else {},
+'cmd-options': backup_options,
'qemu-binary': path
})
@@ -163,5 +182,9 @@ def __call__(self, parser, namespace, values,
option_string=None):
p.add_argument('--test', nargs='+', help='''\
Tests, in form source-dir-label:target-dir-label''',
action=ExtendAction)
+p.add_argument('--compressed', help='''\
+Use compressed backup. It automatically means
+automatically creating qcow2 target with
+lazy_refcounts for each test run''', action='store_true')
bench(p.parse_args())
diff --git a/scripts/simplebench/bench_block_job.py
b/scripts/simplebench/bench_block_job.py
index 7332845c1c..08f86ed9c1 100755
--- a/scripts/simplebench/bench_block_job.py
+++ b/scripts/simplebench/bench_block_job.py
@@ -21,6 +21,7 @@
import sys
import os
+import subprocess
import socket
import json
@@ -77,11 +78,29 @@ def bench_block_job(cmd, cmd_args, qemu_args):
return {'seconds': (end_ms - start_ms) / 100.0}
+def get_image_size(path):
+out = subprocess.run(['qemu-img', 'info', '--out=json', path],
+ stdout=subprocess.PIPE, check=True).stdout
+return json.loads(out)['virtual-size']
+
+
# Bench backup or mirror
def bench_block_copy(qemu_binary, cmd, cmd_options, source, target):
"""Helper to run bench_block_job() for mirror or backup"""
assert cmd in ('blockdev-backup', 'blockdev-mirror')
+if target['driver'] == 'qcow2':
+try:
+os.remove(target['file']['filename'])
+except OSError:
+pass
+
+subprocess.run(['qemu-img', 'create', '-f', 'qcow2',
+target['file']['filename'],
+str(get_image_size(source['filename']))],
+
[PATCH v3 4/9] simplebench/bench-backup: add target-cache argument
Allow benchmark with different kinds of target cache.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: John Snow
---
scripts/simplebench/bench-backup.py| 33 --
scripts/simplebench/bench_block_job.py | 10 +---
2 files changed, 33 insertions(+), 10 deletions(-)
diff --git a/scripts/simplebench/bench-backup.py
b/scripts/simplebench/bench-backup.py
index 72eae85bb1..fbc85f266f 100755
--- a/scripts/simplebench/bench-backup.py
+++ b/scripts/simplebench/bench-backup.py
@@ -65,13 +65,26 @@ def bench(args):
test_cases.append({'id': t, 'source': source, 'target': nbd_drv})
continue
-fname = dirs[dst] + '/test-target'
-if args.compressed:
-fname += '.qcow2'
-target = drv_file(fname)
-if args.compressed:
-target = drv_qcow2(target)
-test_cases.append({'id': t, 'source': source, 'target': target})
+if args.target_cache == 'both':
+target_caches = ['direct', 'cached']
+else:
+target_caches = [args.target_cache]
+
+for c in target_caches:
+o_direct = c == 'direct'
+fname = dirs[dst] + '/test-target'
+if args.compressed:
+fname += '.qcow2'
+target = drv_file(fname, o_direct=o_direct)
+if args.compressed:
+target = drv_qcow2(target)
+
+test_id = t
+if args.target_cache == 'both':
+test_id += f'({c})'
+
+test_cases.append({'id': test_id, 'source': source,
+ 'target': target})
binaries = [] # list of (, , [])
for i, q in enumerate(args.env):
@@ -186,5 +199,11 @@ def __call__(self, parser, namespace, values,
option_string=None):
Use compressed backup. It automatically means
automatically creating qcow2 target with
lazy_refcounts for each test run''', action='store_true')
+p.add_argument('--target-cache', help='''\
+Setup cache for target nodes. Options:
+ direct: default, use O_DIRECT and aio=native
+ cached: use system cache (Qemu default) and aio=threads (Qemu default)
+ both: generate two test cases for each src:dst pair''',
+ default='direct', choices=('direct', 'cached', 'both'))
bench(p.parse_args())
diff --git a/scripts/simplebench/bench_block_job.py
b/scripts/simplebench/bench_block_job.py
index 08f86ed9c1..8f8385ccce 100755
--- a/scripts/simplebench/bench_block_job.py
+++ b/scripts/simplebench/bench_block_job.py
@@ -115,9 +115,13 @@ def bench_block_copy(qemu_binary, cmd, cmd_options,
source, target):
'-blockdev', json.dumps(target)])
-def drv_file(filename):
-return {'driver': 'file', 'filename': filename,
-'cache': {'direct': True}, 'aio': 'native'}
+def drv_file(filename, o_direct=True):
+node = {'driver': 'file', 'filename': filename}
+if o_direct:
+node['cache'] = {'direct': True}
+node['aio'] = 'native'
+
+return node
def drv_nbd(host, port):
--
2.29.2
[PATCH v3 1/9] simplebench: bench_one(): add slow_limit argument
Sometimes one of cells in a testing table runs too slow. And we really
don't want to wait so long. Limit number of runs in this case.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
scripts/simplebench/simplebench.py | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/scripts/simplebench/simplebench.py
b/scripts/simplebench/simplebench.py
index f61513af90..0a3035732c 100644
--- a/scripts/simplebench/simplebench.py
+++ b/scripts/simplebench/simplebench.py
@@ -19,9 +19,11 @@
#
import statistics
+import time
-def bench_one(test_func, test_env, test_case, count=5, initial_run=True):
+def bench_one(test_func, test_env, test_case, count=5, initial_run=True,
+ slow_limit=100):
"""Benchmark one test-case
test_func -- benchmarking function with prototype
@@ -36,6 +38,8 @@ def bench_one(test_func, test_env, test_case, count=5,
initial_run=True):
test_case -- test case - opaque second argument for test_func
count -- how many times to call test_func, to calculate average
initial_run -- do initial run of test_func, which don't get into result
+slow_limit -- stop at slow run (that exceedes the slow_limit by seconds).
+ (initial run is not measured)
Returns dict with the following fields:
'runs': list of test_func results
@@ -53,11 +57,19 @@ def bench_one(test_func, test_env, test_case, count=5,
initial_run=True):
runs = []
for i in range(count):
+t = time.time()
+
print(' #run {}'.format(i+1))
res = test_func(test_env, test_case)
print(' ', res)
runs.append(res)
+if time.time() - t > slow_limit:
+print('- run is too slow, stop here')
+break
+
+count = len(runs)
+
result = {'runs': runs}
succeeded = [r for r in runs if ('seconds' in r or 'iops' in r)]
--
2.29.2
[PATCH v3 0/9] simplebench improvements
Hi all! Here are some improvements to simplebench lib, to support my "qcow2: compressed write cache" series. v3: 01: use simpler logic 02,04-06: add John's r-b 07: use BooleanOptionalAction and initial_run=args.initial_run 08: rewrite so that we have a new --drop-caches option Vladimir Sementsov-Ogievskiy (9): simplebench: bench_one(): add slow_limit argument simplebench: bench_one(): support count=1 simplebench/bench-backup: add --compressed option simplebench/bench-backup: add target-cache argument simplebench/bench_block_job: handle error in BLOCK_JOB_COMPLETED simplebench/bench-backup: support qcow2 source files simplebench/bench-backup: add --count and --no-initial-run simplebench/bench-backup: add --drop-caches argument MAINTAINERS: update Benchmark util: add git tree MAINTAINERS| 1 + scripts/simplebench/bench-backup.py| 95 +- scripts/simplebench/bench_block_job.py | 42 +++- scripts/simplebench/simplebench.py | 28 +++- 4 files changed, 144 insertions(+), 22 deletions(-) -- 2.29.2
[PATCH 3/4] qemu-iotests: let "check" spawn an arbitrary test command
Right now there is no easy way for "check" to print a reproducer command.
Because such a reproducer command line would be huge, we can instead teach
check to start a command of our choice. This can be for example a Python
unit test with arguments to only run a specific subtest.
Signed-off-by: Paolo Bonzini
---
tests/qemu-iotests/check | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index d1c87ceaf1..aab25dac6a 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -19,6 +19,7 @@
import os
import sys
import argparse
+import shutil
from findtests import TestFinder
from testenv import TestEnv
from testrunner import TestRunner
@@ -100,8 +101,8 @@ def make_argparser() -> argparse.ArgumentParser:
'one to TEST (not inclusive). This may be used to '
'rerun failed ./check command, starting from the '
'middle of the process.')
-g_sel.add_argument('tests', metavar='TEST_FILES', nargs='*',
- help='tests to run')
+g_sel.add_argument('tests', nargs=argparse.REMAINDER,
+ help='tests to run, or "--" followed by a command')
return p
@@ -114,6 +115,17 @@ if __name__ == '__main__':
imgopts=args.imgopts, misalign=args.misalign,
debug=args.debug, valgrind=args.valgrind)
+if args.tests[0] == '--':
+del args.tests[0]
+cmd = args.tests
+env.print_env()
+exec_path = shutil.which(cmd[0])
+if exec_path is None:
+sys.exit('command not found: ' + cmd[0])
+cmd[0] = exec_path
+full_env = env.prepare_subprocess(cmd)
+os.execve(cmd[0], cmd, full_env)
+
testfinder = TestFinder(test_dir=env.source_iotests)
groups = args.groups.split(',') if args.groups else None
--
2.30.1
[PATCH 1/4] qemu-iotests: allow passing unittest.main arguments to the test scripts
Python test scripts that use unittest consist of multiple tests.
unittest.main allows selecting which tests to run, but currently this
is not possible because the iotests wrapper ignores sys.argv.
unittest.main command line options also allow the user to pick the
desired options for verbosity, failfast mode, etc. While "-d" is
currently translated to "-v", it also enables extra debug output,
and also there are no other options available. Unfortunately, these
command line options only work if the unittest.main testRunner argument
is a type, rather than a TestRunner instance, and right now iotests.py
is using a TextTestRunner instance in order to pass the output stream.
By moving the machinery to obtain reproducible unittest output into a
TextTestRunner subclass, we can just pass the class name to unittest.main.
Signed-off-by: Paolo Bonzini
---
tests/qemu-iotests/iotests.py | 60 ---
1 file changed, 35 insertions(+), 25 deletions(-)
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 90d0b62523..b9f4edfc42 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -1271,38 +1271,49 @@ def func_wrapper(*args, **kwargs):
return func(*args, **kwargs)
return func_wrapper
-def execute_unittest(debug=False):
-"""Executes unittests within the calling module."""
-
-verbosity = 2 if debug else 1
-
-if debug:
-output = sys.stdout
-else:
-# We need to filter out the time taken from the output so that
-# qemu-iotest can reliably diff the results against master output.
-output = io.StringIO()
-
-runner = unittest.TextTestRunner(stream=output, descriptions=True,
- verbosity=verbosity)
-try:
-# unittest.main() will use sys.exit(); so expect a SystemExit
-# exception
-unittest.main(testRunner=runner)
-finally:
-# We need to filter out the time taken from the output so that
-# qemu-iotest can reliably diff the results against master output.
-if not debug:
-out = output.getvalue()
+# We need to filter out the time taken from the output so that
+# qemu-iotest can reliably diff the results against master output.
+class ReproducibleTextTestRunner(unittest.TextTestRunner):
+__output = None
+
+@classmethod
+@property
+def output(cls):
+if cls.__output is not None:
+return cls.__output
+
+cls.__output = io.StringIO()
+def print_output():
+out = cls.__output.getvalue()
out = re.sub(r'Ran (\d+) tests? in [\d.]+s', r'Ran \1 tests', out)
# Hide skipped tests from the reference output
out = re.sub(r'OK \(skipped=\d+\)', 'OK', out)
out_first_line, out_rest = out.split('\n', 1)
out = out_first_line.replace('s', '.') + '\n' + out_rest
-
sys.stderr.write(out)
+atexit.register(print_output)
+return cls.__output
+
+def __init__(self, *args, **kwargs):
+super().__init__(stream=ReproducibleTextTestRunner.output, *args,
**kwargs)
+
+def execute_unittest(argv, debug=False):
+"""Executes unittests within the calling module."""
+
+# If we see non-empty argv we must not be invoked as a test script,
+# so do not bother making constant output; debuggability is more
+# important.
+if debug or len(argv) > 1:
+argv += ['-v']
+runner = unittest.TextTestRunner
+else:
+runner = ReproducibleTextTestRunner
+
+unittest.main(argv=argv, testRunner=runner,
+ warnings=None if sys.warnoptions else 'ignore')
+
def execute_setup_common(supported_fmts: Sequence[str] = (),
supported_platforms: Sequence[str] = (),
supported_cache_modes: Sequence[str] = (),
@@ -1338,7 +1348,7 @@ def execute_test(*args, test_function=None, **kwargs):
debug = execute_setup_common(*args, **kwargs)
if not test_function:
-execute_unittest(debug)
+execute_unittest(sys.argv, debug)
else:
test_function()
--
2.30.1
[PATCH 0/4] qemu-iotests: quality of life improvements
This series adds a few usability improvements to qemu-iotests, in particular: - arguments can be passed to Python unittests scripts, for example to run only a subset of the test cases (patch 1) - it is possible to do "./check -- ../../../tests/qemu-iotests/055 args..." and specify arbitrary arguments to be passed to a single test script. This allows to take advantage of the previous feature and ease debugging of Python tests. Paolo Paolo Bonzini (4): qemu-iotests: allow passing unittest.main arguments to the test scripts qemu-iotests: move command line and environment handling from TestRunner to TestEnv qemu-iotests: let "check" spawn an arbitrary test command qemu-iotests: fix case of SOCK_DIR already in the environment tests/qemu-iotests/check | 16 +++-- tests/qemu-iotests/iotests.py| 60 +++- tests/qemu-iotests/testenv.py| 22 ++-- tests/qemu-iotests/testrunner.py | 15 +--- 4 files changed, 69 insertions(+), 44 deletions(-) -- 2.30.1
[PATCH 4/4] qemu-iotests: fix case of SOCK_DIR already in the environment
Due to a typo, in this case the SOCK_DIR was not being created. Signed-off-by: Paolo Bonzini --- tests/qemu-iotests/testenv.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py index 6767eeeb25..169268f61a 100644 --- a/tests/qemu-iotests/testenv.py +++ b/tests/qemu-iotests/testenv.py @@ -120,7 +120,7 @@ def init_directories(self) -> None: try: self.sock_dir = os.environ['SOCK_DIR'] self.tmp_sock_dir = False -Path(self.test_dir).mkdir(parents=True, exist_ok=True) +Path(self.sock_dir).mkdir(parents=True, exist_ok=True) except KeyError: self.sock_dir = tempfile.mkdtemp() self.tmp_sock_dir = True -- 2.30.1
[PATCH 2/4] qemu-iotests: move command line and environment handling from TestRunner to TestEnv
In the next patch, "check" will learn how to execute a test script without
going through TestRunner. To enable this, keep only the text output
and subprocess handling in the TestRunner; move into TestEnv the logic
to prepare for running a subprocess.
Signed-off-by: Paolo Bonzini
---
tests/qemu-iotests/testenv.py| 20 ++--
tests/qemu-iotests/testrunner.py | 15 +--
2 files changed, 19 insertions(+), 16 deletions(-)
diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index 1fbec854c1..6767eeeb25 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -25,7 +25,7 @@
import random
import subprocess
import glob
-from typing import Dict, Any, Optional, ContextManager
+from typing import List, Dict, Any, Optional, ContextManager
def isxfile(path: str) -> bool:
@@ -74,6 +74,21 @@ class TestEnv(ContextManager['TestEnv']):
'CACHEMODE_IS_DEFAULT', 'IMGFMT_GENERIC', 'IMGOPTSSYNTAX',
'IMGKEYSECRET', 'QEMU_DEFAULT_MACHINE', 'MALLOC_PERTURB_']
+def prepare_subprocess(self, args: List[str]) -> Dict[str, str]:
+if self.debug:
+args.append('-d')
+
+with open(args[0], encoding="utf-8") as f:
+try:
+if f.readline().rstrip() == '#!/usr/bin/env python3':
+args.insert(0, self.python)
+except UnicodeDecodeError: # binary test? for future.
+pass
+
+os_env = os.environ.copy()
+os_env.update(self.get_env())
+return os_env
+
def get_env(self) -> Dict[str, str]:
env = {}
for v in self.env_variables:
@@ -268,7 +283,8 @@ 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}
+"""
args = collections.defaultdict(str, self.get_env())
diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
index 1fc61fcaa3..2f56ac545d 100644
--- a/tests/qemu-iotests/testrunner.py
+++ b/tests/qemu-iotests/testrunner.py
@@ -129,7 +129,6 @@ class TestRunner(ContextManager['TestRunner']):
def __init__(self, env: TestEnv, makecheck: bool = False,
color: str = 'auto') -> None:
self.env = env
-self.test_run_env = self.env.get_env()
self.makecheck = makecheck
self.last_elapsed = LastElapsedTime('.last-elapsed-cache', env)
@@ -243,18 +242,7 @@ def do_run_test(self, test: str) -> TestResult:
silent_unlink(p)
args = [str(f_test.resolve())]
-if self.env.debug:
-args.append('-d')
-
-with f_test.open(encoding="utf-8") as f:
-try:
-if f.readline().rstrip() == '#!/usr/bin/env python3':
-args.insert(0, self.env.python)
-except UnicodeDecodeError: # binary test? for future.
-pass
-
-env = os.environ.copy()
-env.update(self.test_run_env)
+env = self.env.prepare_subprocess(args)
t0 = time.time()
with f_bad.open('w', encoding="utf-8") as f:
@@ -328,7 +316,6 @@ def run_tests(self, tests: List[str]) -> bool:
if not self.makecheck:
self.env.print_env()
-print()
test_field_width = max(len(os.path.basename(t)) for t in tests) + 2
--
2.30.1
Re: "make check" broken with everything but tools disabled
On 3/18/21 11:03 AM, Paolo Bonzini wrote: > On 18/03/21 10:16, Claudio Fontana wrote: >> my experience with the new build system (meson-based), is that I have to do: >> >> make >> >> first, and then >> >> make check >> >> later, or bugs start happening > > This shouldn't be needed. > > Paolo > I am pretty sure it didn't work in some cases, maybe with check-tcg only.. will keep an eye on this. Ciao, Claudio
Re: [PATCH] hw/block/nvme: remove description for zoned.append_size_limit
On Mar 23 11:18, Niklas Cassel wrote:
> From: Niklas Cassel
>
> The description was originally removed in commit 578d914b263c
> ("hw/block/nvme: align zoned.zasl with mdts") together with the removal
> of the zoned.append_size_limit parameter itself.
>
> However, it was (most likely accidentally), re-added in commit
> f7dcd31885cb ("hw/block/nvme: add non-mdts command size limit for verify").
>
> Remove the description again, since the parameter it describes,
> zoned.append_size_limit, no longer exists.
>
> Signed-off-by: Niklas Cassel
> ---
> hw/block/nvme.c | 8
> 1 file changed, 8 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 6842b01ab5..205d3ec944 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -91,14 +91,6 @@
> * the minimum memory page size (CAP.MPSMIN). The default value is 0 (i.e.
> * defaulting to the value of `mdts`).
> *
> - * - `zoned.append_size_limit`
> - * The maximum I/O size in bytes that is allowed in Zone Append command.
> - * The default is 128KiB. Since internally this this value is maintained as
> - * ZASL = log2( / ), some values assigned
> - * to this property may be rounded down and result in a lower maximum ZA
> - * data size being in effect. By setting this property to 0, users can make
> - * ZASL to be equal to MDTS. This property only affects zoned namespaces.
> - *
> * nvme namespace device parameters
> *
> * - `subsys`
> --
> 2.30.2
Argh. Thanks Niklas, queing it up for fixes.
Reviewed-by: Klaus Jensen
signature.asc
Description: PGP signature
[PATCH] hw/block/nvme: remove description for zoned.append_size_limit
From: Niklas Cassel
The description was originally removed in commit 578d914b263c
("hw/block/nvme: align zoned.zasl with mdts") together with the removal
of the zoned.append_size_limit parameter itself.
However, it was (most likely accidentally), re-added in commit
f7dcd31885cb ("hw/block/nvme: add non-mdts command size limit for verify").
Remove the description again, since the parameter it describes,
zoned.append_size_limit, no longer exists.
Signed-off-by: Niklas Cassel
---
hw/block/nvme.c | 8
1 file changed, 8 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 6842b01ab5..205d3ec944 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -91,14 +91,6 @@
* the minimum memory page size (CAP.MPSMIN). The default value is 0 (i.e.
* defaulting to the value of `mdts`).
*
- * - `zoned.append_size_limit`
- * The maximum I/O size in bytes that is allowed in Zone Append command.
- * The default is 128KiB. Since internally this this value is maintained as
- * ZASL = log2( / ), some values assigned
- * to this property may be rounded down and result in a lower maximum ZA
- * data size being in effect. By setting this property to 0, users can make
- * ZASL to be equal to MDTS. This property only affects zoned namespaces.
- *
* nvme namespace device parameters
*
* - `subsys`
--
2.30.2
Re: [PULL 0/7] SD/MMC patches for 2021-03-21
On Mon, 22 Mar 2021 at 17:23, Philippe Mathieu-Daudé wrote: > > The following changes since commit b184750926812cb78ac0caf4c4b2b13683b5bde3: > > Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into > staging (2021-03-22 11:24:55 +) > > are available in the Git repository at: > > https://github.com/philmd/qemu.git tags/sdmmc-20210322 > > for you to fetch changes up to cffb446e8fd19a14e1634c7a3a8b07be3f01d5c9: > > hw/sd: sdhci: Reset the data pointer of s->fifo_buffer[] when a different > block size is programmed (2021-03-22 16:56:22 +0100) > > > SD/MMC patches queue > > - Fix build error when DEBUG_SD is on > - Perform SD ERASE operation > - SDHCI ADMA heap buffer overflow > (CVE-2020-17380, CVE-2020-25085, CVE-2021-3409) Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/6.0 for any user-visible changes. -- PMM
Re: [PATCH] block: Remove monitor command block_passwd
On Tue, Mar 23, 2021 at 11:19:51AM +0100, Markus Armbruster wrote: > Command block_passwd always fails since > > Commit c01c214b69 "block: remove all encryption handling APIs" > (v2.10.0) turned block_passwd into a stub that always fails, and > hardcoded encryption_key_missing to false in query-named-block-nodes > and query-block. > > Commit ad1324e044 "block: remove 'encryption_key_missing' flag from > QAPI" just landed. Complete the cleanup job: remove block_passwd. Oh, we never marked block_passwd as deprecated, so I missed this cleanup. None the less we're fine removing it, since anyone who has tried to use it since 2.10 will have found it useless, and have been unable to start their VMs with encrypted disks. > > Cc: Daniel P. Berrangé > Signed-off-by: Markus Armbruster > --- > This is based on my "[PATCH 00/28] qapi: Enforce naming rules". To > apply directly, simply drop the qapi/pragma.json hunk. > > Based-on: <20210323094025.3569441-1-arm...@redhat.com> > > > qapi/block-core.json | 14 -- > qapi/pragma.json | 1 - > block/monitor/block-hmp-cmds.c | 10 -- > blockdev.c | 8 > hmp-commands.hx| 15 --- > 5 files changed, 48 deletions(-) Reviewed-by: Daniel P. Berrangé Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PATCH] block: Remove monitor command block_passwd
Command block_passwd always fails since
Commit c01c214b69 "block: remove all encryption handling APIs"
(v2.10.0) turned block_passwd into a stub that always fails, and
hardcoded encryption_key_missing to false in query-named-block-nodes
and query-block.
Commit ad1324e044 "block: remove 'encryption_key_missing' flag from
QAPI" just landed. Complete the cleanup job: remove block_passwd.
Cc: Daniel P. Berrangé
Signed-off-by: Markus Armbruster
---
This is based on my "[PATCH 00/28] qapi: Enforce naming rules". To
apply directly, simply drop the qapi/pragma.json hunk.
Based-on: <20210323094025.3569441-1-arm...@redhat.com>
qapi/block-core.json | 14 --
qapi/pragma.json | 1 -
block/monitor/block-hmp-cmds.c | 10 --
blockdev.c | 8
hmp-commands.hx| 15 ---
5 files changed, 48 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 1c3f1deb03..6d227924d0 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1207,20 +1207,6 @@
##
{ 'command': 'query-block-jobs', 'returns': ['BlockJobInfo'] }
-##
-# @block_passwd:
-#
-# This command sets the password of a block device that has not been open
-# with a password and requires one.
-#
-# This command is now obsolete and will always return an error since 2.10
-#
-##
-{ 'command': 'block_passwd',
- 'data': { '*device': 'str',
-'*node-name': 'str',
-'password': 'str' } }
-
##
# @block_resize:
#
diff --git a/qapi/pragma.json b/qapi/pragma.json
index b4e17167e1..3bc0335d1f 100644
--- a/qapi/pragma.json
+++ b/qapi/pragma.json
@@ -6,7 +6,6 @@
# Commands allowed to return a non-dictionary:
'command-name-exceptions': [
'add_client',
-'block_passwd',
'block_resize',
'block_set_io_throttle',
'client_migrate_info',
diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index 75d7fa9510..ebf1033f31 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -515,16 +515,6 @@ void hmp_block_stream(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, error);
}
-void hmp_block_passwd(Monitor *mon, const QDict *qdict)
-{
-const char *device = qdict_get_str(qdict, "device");
-const char *password = qdict_get_str(qdict, "password");
-Error *err = NULL;
-
-qmp_block_passwd(true, device, false, NULL, password, );
-hmp_handle_error(mon, err);
-}
-
void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict)
{
Error *err = NULL;
diff --git a/blockdev.c b/blockdev.c
index cf70bb4e43..621cc3b7c4 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2407,14 +2407,6 @@ exit:
job_txn_unref(block_job_txn);
}
-void qmp_block_passwd(bool has_device, const char *device,
- bool has_node_name, const char *node_name,
- const char *password, Error **errp)
-{
-error_setg(errp,
- "Setting block passwords directly is no longer supported");
-}
-
BlockDirtyBitmapSha256 *qmp_x_debug_block_dirty_bitmap_sha256(const char *node,
const char *name,
Error **errp)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 9b88c45174..435c591a1c 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1493,21 +1493,6 @@ SRST
used by another monitor command.
ERST
-{
-.name = "block_passwd",
-.args_type = "device:B,password:s",
-.params = "block_passwd device password",
-.help = "set the password of encrypted block devices",
-.cmd= hmp_block_passwd,
-},
-
-SRST
-``block_passwd`` *device* *password*
- Set the encrypted device *device* password to *password*
-
- This command is now obsolete and will always return an error since 2.10
-ERST
-
{
.name = "block_set_io_throttle",
.args_type =
"device:B,bps:l,bps_rd:l,bps_wr:l,iops:l,iops_rd:l,iops_wr:l",
--
2.26.3
Re: [RFC v5 1/6] qmp: add QMP command x-debug-query-virtio
On 3/18/21 4:46 PM, Eric Blake wrote:
On 3/18/21 11:29 AM, Jonah Palmer wrote:
From: Laurent Vivier
This new command lists all the instances of VirtIODevice with
their path and virtio type
Signed-off-by: Laurent Vivier
Reviewed-by: Eric Blake
Signed-off-by: Jonah Palmer
---
We've missed soft freeze for 6.0, and this feels like a new feature;
therefore...
+++ b/hw/virtio/virtio.c
+VirtioInfoList *qmp_x_debug_query_virtio(Error **errp)
+{
+VirtioInfoList *list = NULL;
+VirtioInfoList *node;
+VirtIODevice *vdev;
+
+QTAILQ_FOREACH(vdev, _list, next) {
+DeviceState *dev = DEVICE(vdev);
+node = g_new0(VirtioInfoList, 1);
+node->value = g_new(VirtioInfo, 1);
+node->value->path = g_strdup(dev->canonical_path);
+node->value->type = qapi_enum_parse(_lookup, vdev->name,
+VIRTIO_TYPE_UNKNOWN, NULL);
+node->next = list;
+list = node;
This should be updated to use QAPI_LIST_PREPEND rather than open coding.
Ok! I'll update this.
+}
+
+return list;
+}
+
static const TypeInfo virtio_device_info = {
.name = TYPE_VIRTIO_DEVICE,
.parent = TYPE_DEVICE,
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index b7ece7a..2470e09 100644
--- a/include/hw/virtio/virtio.h
+++ b/qapi/virtio.json
@@ -0,0 +1,68 @@
+##
+# = Virtio devices
+##
+
+##
+# @VirtioType:
+#
+# An enumeration of Virtio device types.
+#
+# Since: 6.0
...this now needs to reference 6.1.
And these!
+##
+{ 'enum': 'VirtioType',
+ 'data': [ 'unknown', 'virtio-9p', 'virtio-blk', 'virtio-serial',
+'virtio-gpu', 'virtio-input', 'virtio-net', 'virtio-scsi',
+'vhost-user-fs', 'vhost-vsock', 'virtio-balloon', 'virtio-crypto',
+'virtio-iommu', 'virtio-pmem', 'virtio-rng' ]
+}
+
+##
+# @VirtioInfo:
+#
+# Information about a given VirtIODevice
+#
+# @path: VirtIO device canonical path.
+#
+# @type: VirtIO device type.
+#
+# Since: 6.0
and throughout the series (I'll quit pointing it out)
+##
+# @x-debug-query-virtio:
+#
+# Return the list of all VirtIO devices
+#
+# Returns: list of @VirtioInfo
+#
+# Since: 6.0
+#
+# Example:
+#
+# -> { "execute": "x-debug-query-virtio" }
That said, adding an 'x-' experimental feature is NOT locking us down,
so if some maintainer still wants to include this in -rc1 on the grounds
that it will help debugging other things, rather than pushing it out to
6.1 as a new feature, then keeping things as 6.0 is tolerable in that
border-line case. (If it misses -rc1, then I will become more adamant
that it does not belong in 6.0)
Right, this patch is not at all urgent for 6.0. I just wanted to send this out
to get
a review.
Best,
Jonah
