|
From: | Vladimir Sementsov-Ogievskiy |
Subject: | Re: [PATCH v6 08/11] iotests: add testenv.py |
Date: | Fri, 15 Jan 2021 16:10:50 +0300 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.1 |
15.01.2021 15:45, Kevin Wolf wrote:
Am 15.01.2021 um 13:19 hat Vladimir Sementsov-Ogievskiy geschrieben:15.01.2021 14:18, Kevin Wolf wrote:Am 09.01.2021 um 13:26 hat Vladimir Sementsov-Ogievskiy geschrieben:Add TestEnv class, which will handle test environment in a new python iotests running framework. Difference with current ./check interface: - -v (verbose) option dropped, as it is unused - -xdiff option is dropped, until somebody complains that it is needed - same for -n option Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- tests/qemu-iotests/testenv.py | 328 ++++++++++++++++++++++++++++++++++ 1 file changed, 328 insertions(+) create mode 100755 tests/qemu-iotests/testenv.py
[..]
+ def init_binaries(self): + """Init binary path variables: + PYTHON (for bash tests) + QEMU_PROG, QEMU_IMG_PROG, QEMU_IO_PROG, QEMU_NBD_PROG, QSD_PROG + SOCKET_SCM_HELPER + """ + self.python = '/usr/bin/python3 -B'This doesn't look right, we need to respect the Python binary set in configure (which I think we get from common.env)Oh, I missed the change. Then I should just drop this self.python.Do we still get the value from elsewhere or do we need to manually parse common.env?
Hmm.. Good question. We have either parse common.env, and still create self.python variable. Or drop it, and include common.env directly to bash tests. For this we'll need to export BUILD_IOTESTS, and do . $BUILD_IOTESTS/common.env in common.rc..
+ def root(*names): + return os.path.join(self.build_root, *names) + + arch = os.uname().machine + if 'ppc64' in arch: + arch = 'ppc64' + + self.qemu_prog = os.getenv('QEMU_PROG', root(f'qemu-system-{arch}')) + self.qemu_img_prog = os.getenv('QEMU_IMG_PROG', root('qemu-img')) + self.qemu_io_prog = os.getenv('QEMU_IO_PROG', root('qemu-io')) + self.qemu_nbd_prog = os.getenv('QEMU_NBD_PROG', root('qemu-nbd')) + self.qsd_prog = os.getenv('QSD_PROG', root('storage-daemon', + 'qemu-storage-daemon')) + + for b in [self.qemu_img_prog, self.qemu_io_prog, self.qemu_nbd_prog, + self.qemu_prog, self.qsd_prog]: + if not os.path.exists(b): + exit('Not such file: ' + b) + if not os.access(b, os.X_OK): + exit('Not executable: ' + b) + + helper_path = os.path.join(self.build_iotests, 'socket_scm_helper') + if os.access(helper_path, os.X_OK): + self.socket_scm_helper = helper_path # SOCKET_SCM_HELPER + + def __init__(self, argv: List[str]) -> None: + """Parse args and environment""" + + # Initialize generic paths: build_root, build_iotests, source_iotests, + # which are needed to initialize some environment variables. They are + # used by init_*() functions as well. + + + if os.path.islink(sys.argv[0]): + # called from the build tree + self.source_iotests = os.path.dirname(os.readlink(sys.argv[0])) + self.build_iotests = os.path.dirname(os.path.abspath(sys.argv[0])) + else: + # called from the source tree + self.source_iotests = os.getcwd() + self.build_iotests = self.source_iotests + + self.build_root = os.path.join(self.build_iotests, '..', '..') + + self.init_handle_argv(argv) + self.init_directories() + self.init_binaries() + + # QEMU_OPTIONS + self.qemu_options = '-nodefaults -display none -accel qtest' + machine_map = ( + (('arm', 'aarch64'), 'virt'),How does this work? Won't we check for "qemu-system-('arm', 'aarch64')" below, which we'll never find?Hmm. I just took existing logic from check: [..] case "$QEMU_PROG" in *qemu-system-arm|*qemu-system-aarch64) export QEMU_OPTIONS="$QEMU_OPTIONS -machine virt" ;; [..]What I mean is that the code below doesn't look like it's prepared to interpret a tuple like ('arm', 'aarch64'), it expects a single string:+ ('avr', 'mega2560'), + ('rx', 'gdbsim-r5f562n8'), + ('tricore', 'tricore_testboard') + ) + for suffix, machine in machine_map: + if self.qemu_prog.endswith(f'qemu-system-{suffix}'):Here we get effectively: suffix: Tuple[str, str] = ('arm', 'aarch64') The formatted string uses str(suffix), which makes the result "qemu-system-('arm', 'aarch64')". Or am I misunderstanding something here?
Ah, you are right:) Will fix. I interpreted your Won't we check for "qemu-system-('arm', 'aarch64')" asWon't we check for "qemu-system-arm" or "qemu-system-aarch64"
-- Best regards, Vladimir
[Prev in Thread] | Current Thread | [Next in Thread] |