From f7afa7daa08c09d7c8435a95a13f6bd9dd11255e Mon Sep 17 00:00:00 2001 From: Connor Kuehl Date: Wed, 21 Apr 2021 16:23:42 -0500 Subject: iotests/231: Update expected deprecation message The deprecation message in the expected output has technically been wrong since the wrong version of a patch was applied to it. Because of this, the test fails. Correct the expected output so that it passes. Signed-off-by: Connor Kuehl Reviewed-by: Max Reitz Reviewed-by: Stefano Garzarella Message-Id: <20210421212343.85524-2-ckuehl@redhat.com> Signed-off-by: Max Reitz --- tests/qemu-iotests/231.out | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'tests') diff --git a/tests/qemu-iotests/231.out b/tests/qemu-iotests/231.out index 579ba11..747dd22 100644 --- a/tests/qemu-iotests/231.out +++ b/tests/qemu-iotests/231.out @@ -1,9 +1,7 @@ QA output created by 231 -qemu-img: RBD options encoded in the filename as keyvalue pairs is deprecated. Future versions may cease to parse these options in the future. +qemu-img: warning: RBD options encoded in the filename as keyvalue pairs is deprecated unable to get monitor info from DNS SRV with service name: ceph-mon -no monitors specified to connect to. qemu-img: Could not open 'json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=BOGUS_CONF'}': error connecting: No such file or directory unable to get monitor info from DNS SRV with service name: ceph-mon -no monitors specified to connect to. qemu-img: Could not open 'json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'BOGUS_CONF'}': error connecting: No such file or directory *** done -- cgit v1.1 From 2b99cfce08da53a07e86271747fe465556ac7eb4 Mon Sep 17 00:00:00 2001 From: Connor Kuehl Date: Wed, 21 Apr 2021 16:23:43 -0500 Subject: block/rbd: Add an escape-aware strchr helper Sometimes the parser needs to further split a token it has collected from the token input stream. Right now, it does a cursory check to see if the relevant characters appear in the token to determine if it should break it down further. However, qemu_rbd_next_tok() will escape characters as it removes tokens from the token stream and plain strchr() won't. This can make the initial strchr() check slightly misleading since it implies qemu_rbd_next_tok() will find the token and split on it, except the reality is that qemu_rbd_next_tok() will pass over it if it is escaped. Use a custom strchr to avoid mixing escaped and unescaped string operations. Furthermore, this code is identical to how qemu_rbd_next_tok() seeks its next token, so incorporate this custom strchr into the body of that function to reduce duplication. Reported-by: Han Han Fixes: https://bugzilla.redhat.com/1873913 Signed-off-by: Connor Kuehl Message-Id: <20210421212343.85524-3-ckuehl@redhat.com> Reviewed-by: Stefano Garzarella Signed-off-by: Max Reitz --- tests/qemu-iotests/231 | 4 ++++ tests/qemu-iotests/231.out | 3 +++ 2 files changed, 7 insertions(+) (limited to 'tests') diff --git a/tests/qemu-iotests/231 b/tests/qemu-iotests/231 index 0f66d0c..8e6c644 100755 --- a/tests/qemu-iotests/231 +++ b/tests/qemu-iotests/231 @@ -55,6 +55,10 @@ _filter_conf() $QEMU_IMG info "json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=${BOGUS_CONF}'}" 2>&1 | _filter_conf $QEMU_IMG info "json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'${BOGUS_CONF}'}" 2>&1 | _filter_conf +# Regression test: the qemu-img invocation is expected to fail, but it should +# not seg fault the parser. +$QEMU_IMG create "rbd:rbd/aa\/bb:conf=${BOGUS_CONF}" 1M 2>&1 | _filter_conf + # success, all done echo "*** done" rm -f $seq.full diff --git a/tests/qemu-iotests/231.out b/tests/qemu-iotests/231.out index 747dd22..a785a6e 100644 --- a/tests/qemu-iotests/231.out +++ b/tests/qemu-iotests/231.out @@ -4,4 +4,7 @@ unable to get monitor info from DNS SRV with service name: ceph-mon qemu-img: Could not open 'json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=BOGUS_CONF'}': error connecting: No such file or directory unable to get monitor info from DNS SRV with service name: ceph-mon qemu-img: Could not open 'json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'BOGUS_CONF'}': error connecting: No such file or directory +Formatting 'rbd:rbd/aa\/bb:conf=BOGUS_CONF', fmt=raw size=1048576 +unable to get monitor info from DNS SRV with service name: ceph-mon +qemu-img: rbd:rbd/aa\/bb:conf=BOGUS_CONF: error connecting: No such file or directory *** done -- cgit v1.1 From 9c785cd714b3c368503ba3aed4266a77056cae29 Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Wed, 21 Apr 2021 10:58:58 +0300 Subject: mirror: stop cancelling in-flight requests on non-force cancel in READY If mirror is READY than cancel operation is not discarding the whole result of the operation, but instead it's a documented way get a point-in-time snapshot of source disk. So, we should not cancel any requests if mirror is READ and force=false. Let's fix that case. Note, that bug that we have before this commit is not critical, as the only .bdrv_cancel_in_flight implementation is nbd_cancel_in_flight() and it cancels only requests waiting for reconnection, so it should be rare case. Fixes: 521ff8b779b11c394dbdc43f02e158dd99df308a Signed-off-by: Vladimir Sementsov-Ogievskiy Message-Id: <20210421075858.40197-1-vsementsov@virtuozzo.com> Signed-off-by: Max Reitz --- tests/qemu-iotests/264 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/qemu-iotests/264 b/tests/qemu-iotests/264 index 4f96825..bc431d1 100755 --- a/tests/qemu-iotests/264 +++ b/tests/qemu-iotests/264 @@ -95,7 +95,7 @@ class TestNbdReconnect(iotests.QMPTestCase): self.assert_qmp(result, 'return', {}) def cancel_job(self): - result = self.vm.qmp('block-job-cancel', device='drive0') + result = self.vm.qmp('block-job-cancel', device='drive0', force=True) self.assert_qmp(result, 'return', {}) start_t = time.time() -- cgit v1.1 From f29f4c25eb9f11d78d62185b69456681f9c703b2 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 3 May 2021 13:01:06 +0200 Subject: 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 Reviewed-by: Vladimir Sementsov-Ogievskiy Tested-by: Emanuele Giuseppe Esposito Message-Id: <20210323181928.311862-2-pbonzini@redhat.com> Message-Id: <20210503110110.476887-2-pbonzini@redhat.com> Signed-off-by: Max Reitz --- tests/qemu-iotests/240.out | 8 ++--- tests/qemu-iotests/245.out | 8 ++--- tests/qemu-iotests/295.out | 6 ++-- tests/qemu-iotests/296.out | 8 ++--- tests/qemu-iotests/iotests.py | 70 +++++++++++++++++++++++++------------------ 5 files changed, 56 insertions(+), 44 deletions(-) (limited to 'tests') diff --git a/tests/qemu-iotests/240.out b/tests/qemu-iotests/240.out index e098283..89ed25e 100644 --- a/tests/qemu-iotests/240.out +++ b/tests/qemu-iotests/240.out @@ -15,7 +15,7 @@ {"return": {}} {"execute": "blockdev-del", "arguments": {"node-name": "hd0"}} {"return": {}} -==Attach two SCSI disks using the same block device and the same iothread== +.==Attach two SCSI disks using the same block device and the same iothread== {"execute": "blockdev-add", "arguments": {"driver": "null-co", "node-name": "hd0", "read-only": true, "read-zeroes": true}} {"return": {}} {"execute": "object-add", "arguments": {"id": "iothread0", "qom-type": "iothread"}} @@ -32,7 +32,7 @@ {"return": {}} {"execute": "blockdev-del", "arguments": {"node-name": "hd0"}} {"return": {}} -==Attach two SCSI disks using the same block device but different iothreads== +.==Attach two SCSI disks using the same block device but different iothreads== {"execute": "blockdev-add", "arguments": {"driver": "null-co", "node-name": "hd0", "read-only": true, "read-zeroes": true}} {"return": {}} {"execute": "object-add", "arguments": {"id": "iothread0", "qom-type": "iothread"}} @@ -55,7 +55,7 @@ {"return": {}} {"execute": "blockdev-del", "arguments": {"node-name": "hd0"}} {"return": {}} -==Attach a SCSI disks using the same block device as a NBD server== +.==Attach a SCSI disks using the same block device as a NBD server== {"execute": "blockdev-add", "arguments": {"driver": "null-co", "node-name": "hd0", "read-only": true, "read-zeroes": true}} {"return": {}} {"execute": "nbd-server-start", "arguments": {"addr": {"data": {"path": "SOCK_DIR/PID-nbd.sock"}, "type": "unix"}}} @@ -68,7 +68,7 @@ {"return": {}} {"execute": "device_add", "arguments": {"drive": "hd0", "driver": "scsi-hd", "id": "scsi-hd0"}} {"return": {}} -.... +. ---------------------------------------------------------------------- Ran 4 tests diff --git a/tests/qemu-iotests/245.out b/tests/qemu-iotests/245.out index 4b33dca..99c12f4 100644 --- a/tests/qemu-iotests/245.out +++ b/tests/qemu-iotests/245.out @@ -1,16 +1,16 @@ -{"execute": "job-finalize", "arguments": {"id": "commit0"}} +..{"execute": "job-finalize", "arguments": {"id": "commit0"}} {"return": {}} {"data": {"id": "commit0", "type": "commit"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} {"data": {"device": "commit0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "commit"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} -{"execute": "job-finalize", "arguments": {"id": "stream0"}} +...{"execute": "job-finalize", "arguments": {"id": "stream0"}} {"return": {}} {"data": {"id": "stream0", "type": "stream"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} {"data": {"device": "stream0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "stream"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} -{"execute": "job-finalize", "arguments": {"id": "stream0"}} +.{"execute": "job-finalize", "arguments": {"id": "stream0"}} {"return": {}} {"data": {"id": "stream0", "type": "stream"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} {"data": {"device": "stream0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "stream"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} -..................... +............... ---------------------------------------------------------------------- Ran 21 tests diff --git a/tests/qemu-iotests/295.out b/tests/qemu-iotests/295.out index ad34b2c..5ff91f1 100644 --- a/tests/qemu-iotests/295.out +++ b/tests/qemu-iotests/295.out @@ -4,7 +4,7 @@ {"return": {}} {"execute": "job-dismiss", "arguments": {"id": "job_erase_key"}} {"return": {}} -{"execute": "job-dismiss", "arguments": {"id": "job_add_key"}} +.{"execute": "job-dismiss", "arguments": {"id": "job_add_key"}} {"return": {}} {"execute": "job-dismiss", "arguments": {"id": "job_erase_key"}} {"return": {}} @@ -13,7 +13,7 @@ Job failed: Invalid password, cannot unlock any keyslot {"return": {}} {"execute": "job-dismiss", "arguments": {"id": "job_add_key"}} {"return": {}} -{"execute": "job-dismiss", "arguments": {"id": "job_add_key"}} +.{"execute": "job-dismiss", "arguments": {"id": "job_add_key"}} {"return": {}} {"execute": "job-dismiss", "arguments": {"id": "job_add_key"}} {"return": {}} @@ -33,7 +33,7 @@ Job failed: All the active keyslots match the (old) password that was given and {"return": {}} {"execute": "job-dismiss", "arguments": {"id": "job_erase_key"}} {"return": {}} -... +. ---------------------------------------------------------------------- Ran 3 tests diff --git a/tests/qemu-iotests/296.out b/tests/qemu-iotests/296.out index cb2859a..6c69735 100644 --- a/tests/qemu-iotests/296.out +++ b/tests/qemu-iotests/296.out @@ -13,7 +13,7 @@ Job failed: Failed to get shared "consistent read" lock qemu-img: Failed to get shared "consistent read" lock Is another process using the image [TEST_DIR/test.img]? -Formatting 'TEST_DIR/test.img', fmt=luks size=1048576 key-secret=keysec0 iter-time=10 +.Formatting 'TEST_DIR/test.img', fmt=luks size=1048576 key-secret=keysec0 iter-time=10 Job failed: Block node is read-only {"execute": "job-dismiss", "arguments": {"id": "job0"}} @@ -26,15 +26,15 @@ Job failed: Failed to get shared "consistent read" lock {"return": {}} {"execute": "job-dismiss", "arguments": {"id": "job0"}} {"return": {}} -Formatting 'TEST_DIR/test.img', fmt=luks size=1048576 key-secret=keysec0 iter-time=10 +.Formatting 'TEST_DIR/test.img', fmt=luks size=1048576 key-secret=keysec0 iter-time=10 {"return": {}} {"error": {"class": "GenericError", "desc": "Failed to get \"write\" lock"}} -Formatting 'TEST_DIR/test.img', fmt=luks size=1048576 key-secret=keysec0 iter-time=10 +.Formatting 'TEST_DIR/test.img', fmt=luks size=1048576 key-secret=keysec0 iter-time=10 {"return": {}} {"return": {}} -.... +. ---------------------------------------------------------------------- Ran 4 tests diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index 5af0182..55a0175 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -20,7 +20,6 @@ import atexit import bz2 from collections import OrderedDict import faulthandler -import io import json import logging import os @@ -32,7 +31,7 @@ import subprocess import sys import time from typing import (Any, Callable, Dict, Iterable, - List, Optional, Sequence, Tuple, TypeVar) + List, Optional, Sequence, TextIO, Tuple, Type, TypeVar) import unittest from contextlib import contextmanager @@ -1271,37 +1270,50 @@ def skip_if_user_is_root(func): 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: + def __init__(self, stream: TextIO): + 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[TextIO] = None, + resultclass: Type[unittest.TestResult] = ReproducibleTestResult, + **kwargs: Any) -> None: + rstream = ReproducibleStreamWrapper(stream or sys.stdout) + super().__init__(stream=rstream, # type: ignore + descriptions=True, + resultclass=resultclass, + **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 = ReproducibleTestRunner(verbosity=verbosity) + unittest.main(testRunner=runner) def execute_setup_common(supported_fmts: Sequence[str] = (), supported_platforms: Sequence[str] = (), -- cgit v1.1 From 00dbc85e0efd863498d52408b248039816c10532 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 3 May 2021 13:01:07 +0200 Subject: 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 Reviewed-by: Vladimir Sementsov-Ogievskiy Tested-by: Emanuele Giuseppe Esposito Message-Id: <20210323181928.311862-3-pbonzini@redhat.com> Message-Id: <20210503110110.476887-3-pbonzini@redhat.com> Signed-off-by: Max Reitz --- tests/qemu-iotests/iotests.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) (limited to 'tests') diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index 55a0175..5ead942 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -1308,12 +1308,16 @@ class ReproducibleTestRunner(unittest.TextTestRunner): resultclass=resultclass, **kwargs) -def execute_unittest(debug=False): +def execute_unittest(argv: List[str], debug: bool = False) -> None: """Executes unittests within the calling module.""" - verbosity = 2 if debug else 1 - runner = 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() -- cgit v1.1 From c64430d2386d9968342a8e1ae00ed34ff0b98bbb Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 3 May 2021 13:01:08 +0200 Subject: 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 Tested-by: Emanuele Giuseppe Esposito Message-Id: <20210323181928.311862-4-pbonzini@redhat.com> Message-Id: <20210503110110.476887-4-pbonzini@redhat.com> Signed-off-by: Max Reitz --- tests/qemu-iotests/testenv.py | 17 ++++++++++++++++- tests/qemu-iotests/testrunner.py | 14 +------------- 2 files changed, 17 insertions(+), 14 deletions(-) (limited to 'tests') diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py index 6d27712..fca3a60 100644 --- a/tests/qemu-iotests/testenv.py +++ b/tests/qemu-iotests/testenv.py @@ -25,7 +25,7 @@ import collections 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 1fc61fc..519924d 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 @@ class TestRunner(ContextManager['TestRunner']): 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: -- cgit v1.1 From 480b75ee1423ee6d8aba59cb8090d60eb97676ff Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 3 May 2021 13:01:09 +0200 Subject: 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 Reviewed-by: Vladimir Sementsov-Ogievskiy Tested-by: Emanuele Giuseppe Esposito Message-Id: <20210323181928.311862-5-pbonzini@redhat.com> Message-Id: <20210503110110.476887-5-pbonzini@redhat.com> Signed-off-by: Max Reitz --- tests/qemu-iotests/check | 19 ++++++++++++++++++- tests/qemu-iotests/testenv.py | 3 ++- tests/qemu-iotests/testrunner.py | 1 - 3 files changed, 20 insertions(+), 3 deletions(-) (limited to 'tests') diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check index 08f5136..2dd529e 100755 --- a/tests/qemu-iotests/check +++ b/tests/qemu-iotests/check @@ -19,6 +19,9 @@ import os import sys import argparse +import shutil +from pathlib import Path + from findtests import TestFinder from testenv import TestEnv from testrunner import TestRunner @@ -100,7 +103,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 @@ -113,6 +116,20 @@ 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_pathstr = shutil.which(cmd[0]) + if exec_pathstr is None: + sys.exit('command not found: ' + cmd[0]) + exec_path = Path(exec_pathstr).resolve() + cmd[0] = str(exec_path) + full_env = env.prepare_subprocess(cmd) + os.chdir(exec_path.parent) + 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 fca3a60..cd0e39b 100644 --- a/tests/qemu-iotests/testenv.py +++ b/tests/qemu-iotests/testenv.py @@ -284,7 +284,8 @@ IMGPROTO -- {IMGPROTO} 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 519924d..2f56ac5 100644 --- a/tests/qemu-iotests/testrunner.py +++ b/tests/qemu-iotests/testrunner.py @@ -316,7 +316,6 @@ class TestRunner(ContextManager['TestRunner']): if not self.makecheck: self.env.print_env() - print() test_field_width = max(len(os.path.basename(t)) for t in tests) + 2 -- cgit v1.1 From c3d479aab9d72fa0d6a3aafdaf0729140e6eae51 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 3 May 2021 13:01:10 +0200 Subject: 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 Tested-by: Emanuele Giuseppe Esposito Message-Id: <20210323181928.311862-6-pbonzini@redhat.com> Message-Id: <20210503110110.476887-6-pbonzini@redhat.com> Signed-off-by: Max Reitz --- tests/qemu-iotests/testenv.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py index cd0e39b..0c3fe75 100644 --- a/tests/qemu-iotests/testenv.py +++ b/tests/qemu-iotests/testenv.py @@ -120,7 +120,7 @@ class TestEnv(ContextManager['TestEnv']): 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 -- cgit v1.1 From ac4e14f5dc93d6b4bc5d4849bb61810023330380 Mon Sep 17 00:00:00 2001 From: Emanuele Giuseppe Esposito Date: Mon, 10 May 2021 21:04:49 +0200 Subject: qemu-iotests: fix pylint 2.8 consider-using-with error pylint 2.8 introduces consider-using-with error, suggesting to use the 'with' block statement when possible. Modify all subprocess.Popen call to use the 'with' statement, except the one in __init__ of QemuIoInteractive class, since it is assigned to a class field and used in other methods. Signed-off-by: Emanuele Giuseppe Esposito Message-Id: <20210510190449.65948-1-eesposit@redhat.com> [mreitz: Disable bad-option-value warning in the iotests' pylintrc, so that disabling consider-using-with in QemuIoInteractive will not produce a warning in pre-2.8 pylint versions] Signed-off-by: Max Reitz --- tests/qemu-iotests/iotests.py | 65 ++++++++++++++++++++-------------------- tests/qemu-iotests/pylintrc | 3 ++ tests/qemu-iotests/testrunner.py | 22 +++++++------- 3 files changed, 47 insertions(+), 43 deletions(-) (limited to 'tests') diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index 5ead942..777fa2e 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -112,15 +112,14 @@ def qemu_tool_pipe_and_status(tool: str, args: Sequence[str], Run a tool and return both its output and its exit code """ stderr = subprocess.STDOUT if connect_stderr else None - subp = subprocess.Popen(args, - stdout=subprocess.PIPE, - stderr=stderr, - universal_newlines=True) - output = subp.communicate()[0] - if subp.returncode < 0: - cmd = ' '.join(args) - sys.stderr.write(f'{tool} received signal {-subp.returncode}: {cmd}\n') - return (output, subp.returncode) + with subprocess.Popen(args, stdout=subprocess.PIPE, + stderr=stderr, universal_newlines=True) as subp: + output = subp.communicate()[0] + if subp.returncode < 0: + cmd = ' '.join(args) + sys.stderr.write(f'{tool} received signal \ + {-subp.returncode}: {cmd}\n') + return (output, subp.returncode) def qemu_img_pipe_and_status(*args: str) -> Tuple[str, int]: """ @@ -236,6 +235,9 @@ def qemu_io_silent_check(*args): class QemuIoInteractive: def __init__(self, *args): self.args = qemu_io_args_no_fmt + list(args) + # We need to keep the Popen objext around, and not + # close it immediately. Therefore, disable the pylint check: + # pylint: disable=consider-using-with self._p = subprocess.Popen(self.args, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, @@ -309,22 +311,22 @@ def qemu_nbd_popen(*args): cmd.extend(args) log('Start NBD server') - p = subprocess.Popen(cmd) - try: - while not os.path.exists(pid_file): - if p.poll() is not None: - raise RuntimeError( - "qemu-nbd terminated with exit code {}: {}" - .format(p.returncode, ' '.join(cmd))) - - time.sleep(0.01) - yield - finally: - if os.path.exists(pid_file): - os.remove(pid_file) - log('Kill NBD server') - p.kill() - p.wait() + with subprocess.Popen(cmd) as p: + try: + while not os.path.exists(pid_file): + if p.poll() is not None: + raise RuntimeError( + "qemu-nbd terminated with exit code {}: {}" + .format(p.returncode, ' '.join(cmd))) + + time.sleep(0.01) + yield + finally: + if os.path.exists(pid_file): + os.remove(pid_file) + log('Kill NBD server') + p.kill() + p.wait() def compare_images(img1, img2, fmt1=imgfmt, fmt2=imgfmt): '''Return True if two image files are identical''' @@ -333,13 +335,12 @@ def compare_images(img1, img2, fmt1=imgfmt, fmt2=imgfmt): def create_image(name, size): '''Create a fully-allocated raw image with sector markers''' - file = open(name, 'wb') - i = 0 - while i < size: - sector = struct.pack('>l504xl', i // 512, i // 512) - file.write(sector) - i = i + 512 - file.close() + with open(name, 'wb') as file: + i = 0 + while i < size: + sector = struct.pack('>l504xl', i // 512, i // 512) + file.write(sector) + i = i + 512 def image_size(img): '''Return image's virtual size''' diff --git a/tests/qemu-iotests/pylintrc b/tests/qemu-iotests/pylintrc index 7a6c0a9..f2c0b52 100644 --- a/tests/qemu-iotests/pylintrc +++ b/tests/qemu-iotests/pylintrc @@ -19,6 +19,9 @@ disable=invalid-name, too-many-public-methods, # pylint warns about Optional[] etc. as unsubscriptable in 3.9 unsubscriptable-object, + # Sometimes we need to disable a newly introduced pylint warning. + # Doing so should not produce a warning in older versions of pylint. + bad-option-value, # These are temporary, and should be removed: missing-docstring, too-many-return-statements, diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py index 2f56ac5..4a6ec42 100644 --- a/tests/qemu-iotests/testrunner.py +++ b/tests/qemu-iotests/testrunner.py @@ -246,17 +246,17 @@ class TestRunner(ContextManager['TestRunner']): t0 = time.time() with f_bad.open('w', encoding="utf-8") as f: - proc = subprocess.Popen(args, cwd=str(f_test.parent), env=env, - stdout=f, stderr=subprocess.STDOUT) - try: - proc.wait() - except KeyboardInterrupt: - proc.terminate() - proc.wait() - return TestResult(status='not run', - description='Interrupted by user', - interrupted=True) - ret = proc.returncode + with subprocess.Popen(args, cwd=str(f_test.parent), env=env, + stdout=f, stderr=subprocess.STDOUT) as proc: + try: + proc.wait() + except KeyboardInterrupt: + proc.terminate() + proc.wait() + return TestResult(status='not run', + description='Interrupted by user', + interrupted=True) + ret = proc.returncode elapsed = round(time.time() - t0, 1) -- cgit v1.1 From e46354a8aeefe4637c689de27532bc00712f9c60 Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Thu, 6 May 2021 12:06:16 +0300 Subject: test-write-threshold: rewrite test_threshold_(not_)trigger tests These tests use bdrv_write_threshold_exceeded() API, which is used only for test (since pre-previous commit). Better is testing real API, which is used in block.c as well. So, let's call bdrv_write_threshold_check_write(), and check is bs->write_threshold_offset cleared or not (it's cleared iff threshold triggered). Also we get rid of BdrvTrackedRequest use here. Note, that paranoiac bdrv_check_request() calls were added in 8b1170012b1 to protect BdrvTrackedRequest. Drop them now. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz Message-Id: <20210506090621.11848-4-vsementsov@virtuozzo.com> Reviewed-by: Stefan Hajnoczi Signed-off-by: Max Reitz --- tests/unit/test-write-threshold.c | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-) (limited to 'tests') diff --git a/tests/unit/test-write-threshold.c b/tests/unit/test-write-threshold.c index fc1c45a..fd40a81 100644 --- a/tests/unit/test-write-threshold.c +++ b/tests/unit/test-write-threshold.c @@ -55,41 +55,27 @@ static void test_threshold_multi_set_get(void) static void test_threshold_not_trigger(void) { - uint64_t amount = 0; uint64_t threshold = 4 * 1024 * 1024; BlockDriverState bs; - BdrvTrackedRequest req; memset(&bs, 0, sizeof(bs)); - memset(&req, 0, sizeof(req)); - req.offset = 1024; - req.bytes = 1024; - - bdrv_check_request(req.offset, req.bytes, &error_abort); bdrv_write_threshold_set(&bs, threshold); - amount = bdrv_write_threshold_exceeded(&bs, &req); - g_assert_cmpuint(amount, ==, 0); + bdrv_write_threshold_check_write(&bs, 1024, 1024); + g_assert_cmpuint(bdrv_write_threshold_get(&bs), ==, threshold); } static void test_threshold_trigger(void) { - uint64_t amount = 0; uint64_t threshold = 4 * 1024 * 1024; BlockDriverState bs; - BdrvTrackedRequest req; memset(&bs, 0, sizeof(bs)); - memset(&req, 0, sizeof(req)); - req.offset = (4 * 1024 * 1024) - 1024; - req.bytes = 2 * 1024; - - bdrv_check_request(req.offset, req.bytes, &error_abort); bdrv_write_threshold_set(&bs, threshold); - amount = bdrv_write_threshold_exceeded(&bs, &req); - g_assert_cmpuint(amount, >=, 1024); + bdrv_write_threshold_check_write(&bs, threshold - 1024, 2 * 1024); + g_assert_cmpuint(bdrv_write_threshold_get(&bs), ==, 0); } typedef struct TestStruct { -- cgit v1.1 From 2e0e9cbd897078fea2ad3772106e0cbd1f0c239a Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Thu, 6 May 2021 12:06:17 +0300 Subject: block/write-threshold: drop extra APIs bdrv_write_threshold_exceeded() is unused. bdrv_write_threshold_is_set() is used only to double check the value of bs->write_threshold_offset in tests. No real sense in it (both tests do check real value with help of bdrv_write_threshold_get()) Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz Message-Id: <20210506090621.11848-5-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake Reviewed-by: Stefan Hajnoczi [mreitz: Adjusted commit message as per Eric's suggestion] Signed-off-by: Max Reitz --- tests/unit/test-write-threshold.c | 4 ---- 1 file changed, 4 deletions(-) (limited to 'tests') diff --git a/tests/unit/test-write-threshold.c b/tests/unit/test-write-threshold.c index fd40a81..bb5c1a5 100644 --- a/tests/unit/test-write-threshold.c +++ b/tests/unit/test-write-threshold.c @@ -18,8 +18,6 @@ static void test_threshold_not_set_on_init(void) BlockDriverState bs; memset(&bs, 0, sizeof(bs)); - g_assert(!bdrv_write_threshold_is_set(&bs)); - res = bdrv_write_threshold_get(&bs); g_assert_cmpint(res, ==, 0); } @@ -33,8 +31,6 @@ static void test_threshold_set_get(void) bdrv_write_threshold_set(&bs, threshold); - g_assert(bdrv_write_threshold_is_set(&bs)); - res = bdrv_write_threshold_get(&bs); g_assert_cmpint(res, ==, threshold); } -- cgit v1.1 From 935129223ce2a2eb61c59b585869724570d3a349 Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Thu, 6 May 2021 12:06:19 +0300 Subject: test-write-threshold: drop extra tests Testing set/get of one 64bit variable doesn't seem necessary. We have a lot of such variables. Also remaining tests do test set/get anyway. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz Message-Id: <20210506090621.11848-7-vsementsov@virtuozzo.com> Reviewed-by: Stefan Hajnoczi Signed-off-by: Max Reitz --- tests/unit/test-write-threshold.c | 43 --------------------------------------- 1 file changed, 43 deletions(-) (limited to 'tests') diff --git a/tests/unit/test-write-threshold.c b/tests/unit/test-write-threshold.c index bb5c1a5..9e9986a 100644 --- a/tests/unit/test-write-threshold.c +++ b/tests/unit/test-write-threshold.c @@ -12,43 +12,6 @@ #include "block/write-threshold.h" -static void test_threshold_not_set_on_init(void) -{ - uint64_t res; - BlockDriverState bs; - memset(&bs, 0, sizeof(bs)); - - res = bdrv_write_threshold_get(&bs); - g_assert_cmpint(res, ==, 0); -} - -static void test_threshold_set_get(void) -{ - uint64_t threshold = 4 * 1024 * 1024; - uint64_t res; - BlockDriverState bs; - memset(&bs, 0, sizeof(bs)); - - bdrv_write_threshold_set(&bs, threshold); - - res = bdrv_write_threshold_get(&bs); - g_assert_cmpint(res, ==, threshold); -} - -static void test_threshold_multi_set_get(void) -{ - uint64_t threshold1 = 4 * 1024 * 1024; - uint64_t threshold2 = 15 * 1024 * 1024; - uint64_t res; - BlockDriverState bs; - memset(&bs, 0, sizeof(bs)); - - bdrv_write_threshold_set(&bs, threshold1); - bdrv_write_threshold_set(&bs, threshold2); - res = bdrv_write_threshold_get(&bs); - g_assert_cmpint(res, ==, threshold2); -} - static void test_threshold_not_trigger(void) { uint64_t threshold = 4 * 1024 * 1024; @@ -84,12 +47,6 @@ int main(int argc, char **argv) { size_t i; TestStruct tests[] = { - { "/write-threshold/not-set-on-init", - test_threshold_not_set_on_init }, - { "/write-threshold/set-get", - test_threshold_set_get }, - { "/write-threshold/multi-set-get", - test_threshold_multi_set_get }, { "/write-threshold/not-trigger", test_threshold_not_trigger }, { "/write-threshold/trigger", -- cgit v1.1 From 23357b93c7bbeec65aef75f798cacbcda0ca5f4d Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Thu, 6 May 2021 12:06:20 +0300 Subject: test-write-threshold: drop extra TestStruct structure We don't need this extra logic: it doesn't make code simpler. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz Message-Id: <20210506090621.11848-8-vsementsov@virtuozzo.com> Reviewed-by: Stefan Hajnoczi Signed-off-by: Max Reitz --- tests/unit/test-write-threshold.c | 20 +++----------------- 1 file changed, 3 insertions(+), 17 deletions(-) (limited to 'tests') diff --git a/tests/unit/test-write-threshold.c b/tests/unit/test-write-threshold.c index 9e9986a..49b1ef7 100644 --- a/tests/unit/test-write-threshold.c +++ b/tests/unit/test-write-threshold.c @@ -37,26 +37,12 @@ static void test_threshold_trigger(void) g_assert_cmpuint(bdrv_write_threshold_get(&bs), ==, 0); } -typedef struct TestStruct { - const char *name; - void (*func)(void); -} TestStruct; - int main(int argc, char **argv) { - size_t i; - TestStruct tests[] = { - { "/write-threshold/not-trigger", - test_threshold_not_trigger }, - { "/write-threshold/trigger", - test_threshold_trigger }, - { NULL, NULL } - }; - g_test_init(&argc, &argv, NULL); - for (i = 0; tests[i].name != NULL; i++) { - g_test_add_func(tests[i].name, tests[i].func); - } + g_test_add_func("/write-threshold/not-trigger", test_threshold_not_trigger); + g_test_add_func("/write-threshold/trigger", test_threshold_trigger); + return g_test_run(); } -- cgit v1.1 From c61ebf362d0abf288ce266845519d5a550a1d89f Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Thu, 6 May 2021 12:06:21 +0300 Subject: write-threshold: deal with includes "qemu/typedefs.h" is enough for include/block/write-threshold.h header with forward declaration of BlockDriverState. Also drop extra includes from block/write-threshold.c and tests/unit/test-write-threshold.c Signed-off-by: Vladimir Sementsov-Ogievskiy Message-Id: <20210506090621.11848-9-vsementsov@virtuozzo.com> Reviewed-by: Stefan Hajnoczi Signed-off-by: Max Reitz --- tests/unit/test-write-threshold.c | 1 - 1 file changed, 1 deletion(-) (limited to 'tests') diff --git a/tests/unit/test-write-threshold.c b/tests/unit/test-write-threshold.c index 49b1ef7..0158e46 100644 --- a/tests/unit/test-write-threshold.c +++ b/tests/unit/test-write-threshold.c @@ -7,7 +7,6 @@ */ #include "qemu/osdep.h" -#include "qapi/error.h" #include "block/block_int.h" #include "block/write-threshold.h" -- cgit v1.1