From 2ca6e26cea73fa1d270f73392e8b87f3e67e6a2b Mon Sep 17 00:00:00 2001 From: Cleber Rosa Date: Thu, 11 Feb 2021 17:01:42 -0500 Subject: [PATCH 01/44] Python: expose QEMUMachine's temporary directory Each instance of qemu.machine.QEMUMachine currently has a "test directory", which may not have any relation to a "test", and it's really a temporary directory. Users instantiating the QEMUMachine class will be able to set the location of the directory that will *contain* the QEMUMachine unique temporary directory, so that parameter name has been changed from test_dir to base_temp_dir. A property has been added to allow users to access it without using private attributes, and with that, the directory is created on first use of the property. Signed-off-by: Cleber Rosa Message-Id: <20210211220146.2525771-3-crosa@redhat.com> Reviewed-by: Wainer dos Santos Moschetta Signed-off-by: Cleber Rosa Signed-off-by: John Snow --- python/qemu/machine.py | 24 ++++++++++++++++-------- python/qemu/qtest.py | 6 +++--- tests/qemu-iotests/iotests.py | 2 +- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/python/qemu/machine.py b/python/qemu/machine.py index 6e44bda337..b379fcbe72 100644 --- a/python/qemu/machine.py +++ b/python/qemu/machine.py @@ -84,7 +84,7 @@ class QEMUMachine: args: Sequence[str] = (), wrapper: Sequence[str] = (), name: Optional[str] = None, - test_dir: str = "/var/tmp", + base_temp_dir: str = "/var/tmp", monitor_address: Optional[SocketAddrT] = None, socket_scm_helper: Optional[str] = None, sock_dir: Optional[str] = None, @@ -97,10 +97,10 @@ class QEMUMachine: @param args: list of extra arguments @param wrapper: list of arguments used as prefix to qemu binary @param name: prefix for socket and log file names (default: qemu-PID) - @param test_dir: where to create socket and log file + @param base_temp_dir: default location where temporary files are created @param monitor_address: address for QMP monitor @param socket_scm_helper: helper program, required for send_fd_scm() - @param sock_dir: where to create socket (overrides test_dir for sock) + @param sock_dir: where to create socket (defaults to base_temp_dir) @param drain_console: (optional) True to drain console socket to buffer @param console_log: (optional) path to console log file @note: Qemu process is not started until launch() is used. @@ -112,8 +112,8 @@ class QEMUMachine: self._wrapper = wrapper self._name = name or "qemu-%d" % os.getpid() - self._test_dir = test_dir - self._sock_dir = sock_dir or self._test_dir + self._base_temp_dir = base_temp_dir + self._sock_dir = sock_dir or self._base_temp_dir self._socket_scm_helper = socket_scm_helper if monitor_address is not None: @@ -303,9 +303,7 @@ class QEMUMachine: return args def _pre_launch(self) -> None: - self._temp_dir = tempfile.mkdtemp(prefix="qemu-machine-", - dir=self._test_dir) - self._qemu_log_path = os.path.join(self._temp_dir, self._name + ".log") + self._qemu_log_path = os.path.join(self.temp_dir, self._name + ".log") self._qemu_log_file = open(self._qemu_log_path, 'wb') if self._console_set: @@ -744,3 +742,13 @@ class QEMUMachine: file=self._console_log_path, drain=self._drain_console) return self._console_socket + + @property + def temp_dir(self) -> str: + """ + Returns a temporary directory to be used for this machine + """ + if self._temp_dir is None: + self._temp_dir = tempfile.mkdtemp(prefix="qemu-machine-", + dir=self._base_temp_dir) + return self._temp_dir diff --git a/python/qemu/qtest.py b/python/qemu/qtest.py index 39a0cf62fe..78b97d13cf 100644 --- a/python/qemu/qtest.py +++ b/python/qemu/qtest.py @@ -112,14 +112,14 @@ class QEMUQtestMachine(QEMUMachine): binary: str, args: Sequence[str] = (), name: Optional[str] = None, - test_dir: str = "/var/tmp", + base_temp_dir: str = "/var/tmp", socket_scm_helper: Optional[str] = None, sock_dir: Optional[str] = None): if name is None: name = "qemu-%d" % os.getpid() if sock_dir is None: - sock_dir = test_dir - super().__init__(binary, args, name=name, test_dir=test_dir, + sock_dir = base_temp_dir + super().__init__(binary, args, name=name, base_temp_dir=base_temp_dir, socket_scm_helper=socket_scm_helper, sock_dir=sock_dir) self._qtest: Optional[QEMUQtestProtocol] = None diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index 777fa2ec0e..92681907ed 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -571,7 +571,7 @@ class VM(qtest.QEMUQtestMachine): def __init__(self, path_suffix=''): name = "qemu%s-%d" % (path_suffix, os.getpid()) super().__init__(qemu_prog, qemu_opts, name=name, - test_dir=test_dir, + base_temp_dir=test_dir, socket_scm_helper=socket_scm_helper, sock_dir=sock_dir) self._num_drives = 0 From f084e148aa401f71c33da4268f73815e0dc2b262 Mon Sep 17 00:00:00 2001 From: Cleber Rosa Date: Mon, 12 Apr 2021 00:46:34 -0400 Subject: [PATCH 02/44] tests/acceptance/virtiofs_submounts.py: add missing accel tag The tag is useful to select tests that depend/use a particular feature. Signed-off-by: Cleber Rosa Reviewed-by: Wainer dos Santos Moschetta Reviewed-by: Willian Rampazzo Reviewed-by: Eric Auger Message-Id: <20210412044644.55083-2-crosa@redhat.com> Signed-off-by: John Snow --- tests/acceptance/virtiofs_submounts.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/acceptance/virtiofs_submounts.py b/tests/acceptance/virtiofs_submounts.py index 46fa65392a..5b74ce2929 100644 --- a/tests/acceptance/virtiofs_submounts.py +++ b/tests/acceptance/virtiofs_submounts.py @@ -70,6 +70,7 @@ def has_cmds(*cmds): class VirtiofsSubmountsTest(LinuxTest): """ :avocado: tags=arch:x86_64 + :avocado: tags=accel:kvm """ def get_portfwd(self): From c028691e65b9f45e7a8ff8ffbfb9a3818478b572 Mon Sep 17 00:00:00 2001 From: Cleber Rosa Date: Mon, 12 Apr 2021 00:46:35 -0400 Subject: [PATCH 03/44] tests/acceptance/virtiofs_submounts.py: evaluate string not length If the vmlinuz variable is set to anything that evaluates to True, then the respective arguments should be set. If the variable contains an empty string, than it will evaluate to False, and the extra arguments will not be set. This keeps the same logic, but improves readability a bit. Signed-off-by: Cleber Rosa Reviewed-by: Beraldo Leal Reviewed-by: Eric Auger Reviewed-by: Willian Rampazzo Message-Id: <20210412044644.55083-3-crosa@redhat.com> Signed-off-by: John Snow --- tests/acceptance/virtiofs_submounts.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/acceptance/virtiofs_submounts.py b/tests/acceptance/virtiofs_submounts.py index 5b74ce2929..ca64b76301 100644 --- a/tests/acceptance/virtiofs_submounts.py +++ b/tests/acceptance/virtiofs_submounts.py @@ -251,7 +251,7 @@ class VirtiofsSubmountsTest(LinuxTest): super(VirtiofsSubmountsTest, self).setUp(pubkey) - if len(vmlinuz) > 0: + if vmlinuz: self.vm.add_args('-kernel', vmlinuz, '-append', 'console=ttyS0 root=/dev/sda1') From 976218cbe792c37c1af7840ca5113e37b5a51d95 Mon Sep 17 00:00:00 2001 From: Cleber Rosa Date: Mon, 12 Apr 2021 00:46:36 -0400 Subject: [PATCH 04/44] Python: add utility function for retrieving port redirection Slightly different versions for the same utility code are currently present on different locations. This unifies them all, giving preference to the version from virtiofs_submounts.py, because of the last tweaks added to it. While at it, this adds a "qemu.utils" module to host the utility function and a test. Signed-off-by: Cleber Rosa Reviewed-by: Wainer dos Santos Moschetta Reviewed-by: Eric Auger Reviewed-by: Willian Rampazzo Message-Id: <20210412044644.55083-4-crosa@redhat.com> Signed-off-by: John Snow [Squashed in below fix. --js] Signed-off-by: John Snow Signed-off-by: Cleber Rosa Message-Id: <20210601154546.130870-2-crosa@redhat.com> Signed-off-by: John Snow --- python/qemu/utils.py | 33 ++++++++++++++++++++++++ tests/acceptance/info_usernet.py | 29 +++++++++++++++++++++ tests/acceptance/linux_ssh_mips_malta.py | 18 ++++++------- tests/acceptance/virtiofs_submounts.py | 21 ++++----------- tests/vm/basevm.py | 11 +++----- 5 files changed, 79 insertions(+), 33 deletions(-) create mode 100644 python/qemu/utils.py create mode 100644 tests/acceptance/info_usernet.py diff --git a/python/qemu/utils.py b/python/qemu/utils.py new file mode 100644 index 0000000000..5ed789275e --- /dev/null +++ b/python/qemu/utils.py @@ -0,0 +1,33 @@ +""" +QEMU utility library + +This offers miscellaneous utility functions, which may not be easily +distinguishable or numerous to be in their own module. +""" + +# Copyright (C) 2021 Red Hat Inc. +# +# Authors: +# Cleber Rosa +# +# This work is licensed under the terms of the GNU GPL, version 2. See +# the COPYING file in the top-level directory. +# + +import re +from typing import Optional + + +def get_info_usernet_hostfwd_port(info_usernet_output: str) -> Optional[int]: + """ + Returns the port given to the hostfwd parameter via info usernet + + :param info_usernet_output: output generated by hmp command "info usernet" + :return: the port number allocated by the hostfwd option + """ + for line in info_usernet_output.split('\r\n'): + regex = r'TCP.HOST_FORWARD.*127\.0\.0\.1\s+(\d+)\s+10\.' + match = re.search(regex, line) + if match is not None: + return int(match[1]) + return None diff --git a/tests/acceptance/info_usernet.py b/tests/acceptance/info_usernet.py new file mode 100644 index 0000000000..9c1fd903a0 --- /dev/null +++ b/tests/acceptance/info_usernet.py @@ -0,0 +1,29 @@ +# Test for the hmp command "info usernet" +# +# Copyright (c) 2021 Red Hat, Inc. +# +# Author: +# Cleber Rosa +# +# This work is licensed under the terms of the GNU GPL, version 2 or +# later. See the COPYING file in the top-level directory. + +from avocado_qemu import Test + +from qemu.utils import get_info_usernet_hostfwd_port + + +class InfoUsernet(Test): + + def test_hostfwd(self): + self.vm.add_args('-netdev', 'user,id=vnet,hostfwd=:127.0.0.1:0-:22') + self.vm.launch() + res = self.vm.command('human-monitor-command', + command_line='info usernet') + port = get_info_usernet_hostfwd_port(res) + self.assertIsNotNone(port, + ('"info usernet" output content does not seem to ' + 'contain the redirected port')) + self.assertGreater(port, 0, + ('Found a redirected port that is not greater than' + ' zero')) diff --git a/tests/acceptance/linux_ssh_mips_malta.py b/tests/acceptance/linux_ssh_mips_malta.py index 6dbd02d49d..052008f02d 100644 --- a/tests/acceptance/linux_ssh_mips_malta.py +++ b/tests/acceptance/linux_ssh_mips_malta.py @@ -18,6 +18,8 @@ from avocado.utils import process from avocado.utils import archive from avocado.utils import ssh +from qemu.utils import get_info_usernet_hostfwd_port + class LinuxSSH(Test): @@ -70,18 +72,14 @@ class LinuxSSH(Test): def setUp(self): super(LinuxSSH, self).setUp() - def get_portfwd(self): - res = self.vm.command('human-monitor-command', - command_line='info usernet') - line = res.split('\r\n')[2] - port = re.split(r'.*TCP.HOST_FORWARD.*127\.0\.0\.1 (\d+)\s+10\..*', - line)[1] - self.log.debug("sshd listening on port:" + port) - return port - def ssh_connect(self, username, password): self.ssh_logger = logging.getLogger('ssh') - port = self.get_portfwd() + res = self.vm.command('human-monitor-command', + command_line='info usernet') + port = get_info_usernet_hostfwd_port(res) + if not port: + self.cancel("Failed to retrieve SSH port") + self.log.debug("sshd listening on port:" + port) self.ssh_session = ssh.Session(self.VM_IP, port=int(port), user=username, password=password) for i in range(10): diff --git a/tests/acceptance/virtiofs_submounts.py b/tests/acceptance/virtiofs_submounts.py index ca64b76301..57a7047342 100644 --- a/tests/acceptance/virtiofs_submounts.py +++ b/tests/acceptance/virtiofs_submounts.py @@ -9,6 +9,8 @@ from avocado_qemu import LinuxTest, BUILD_DIR from avocado_qemu import wait_for_console_pattern from avocado.utils import ssh +from qemu.utils import get_info_usernet_hostfwd_port + def run_cmd(args): subp = subprocess.Popen(args, @@ -73,27 +75,14 @@ class VirtiofsSubmountsTest(LinuxTest): :avocado: tags=accel:kvm """ - def get_portfwd(self): - port = None - + def ssh_connect(self, username, keyfile): + self.ssh_logger = logging.getLogger('ssh') res = self.vm.command('human-monitor-command', command_line='info usernet') - for line in res.split('\r\n'): - match = \ - re.search(r'TCP.HOST_FORWARD.*127\.0\.0\.1\s+(\d+)\s+10\.', - line) - if match is not None: - port = int(match[1]) - break - + port = get_info_usernet_hostfwd_port(res) self.assertIsNotNone(port) self.assertGreater(port, 0) self.log.debug('sshd listening on port: %d', port) - return port - - def ssh_connect(self, username, keyfile): - self.ssh_logger = logging.getLogger('ssh') - port = self.get_portfwd() self.ssh_session = ssh.Session('127.0.0.1', port=port, user=username, key=keyfile) for i in range(10): diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py index 00f1d5ca8d..995e642465 100644 --- a/tests/vm/basevm.py +++ b/tests/vm/basevm.py @@ -21,6 +21,7 @@ import datetime sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python')) from qemu.accel import kvm_available from qemu.machine import QEMUMachine +from qemu.utils import get_info_usernet_hostfwd_port import subprocess import hashlib import argparse @@ -227,7 +228,7 @@ class BaseVM(object): "-o", "UserKnownHostsFile=" + os.devnull, "-o", "ConnectTimeout={}".format(self._config["ssh_timeout"]), - "-p", self.ssh_port, "-i", self._ssh_tmp_key_file] + "-p", str(self.ssh_port), "-i", self._ssh_tmp_key_file] # If not in debug mode, set ssh to quiet mode to # avoid printing the results of commands. if not self.debug: @@ -305,12 +306,8 @@ class BaseVM(object): # Init console so we can start consuming the chars. self.console_init() usernet_info = guest.qmp("human-monitor-command", - command_line="info usernet") - self.ssh_port = None - for l in usernet_info["return"].splitlines(): - fields = l.split() - if "TCP[HOST_FORWARD]" in fields and "22" in fields: - self.ssh_port = l.split()[3] + command_line="info usernet").get("return") + self.ssh_port = get_info_usernet_hostfwd_port(usernet_info) if not self.ssh_port: raise Exception("Cannot find ssh port from 'info usernet':\n%s" % \ usernet_info) From 7edee7ad9408696b9b8d40b5842a07a0c4e9b7a2 Mon Sep 17 00:00:00 2001 From: Cleber Rosa Date: Mon, 12 Apr 2021 00:46:37 -0400 Subject: [PATCH 05/44] Acceptance Tests: move useful ssh methods to base class Both the virtiofs submounts and the linux ssh mips malta tests contains useful methods related to ssh that deserve to be made available to other tests. Let's move them to an auxiliary, mix-in class that will be used on the base LinuxTest class. The method that helps with setting up an ssh connection will now support both key and password based authentication, defaulting to key based. Signed-off-by: Cleber Rosa Reviewed-by: Wainer dos Santos Moschetta Reviewed-by: Willian Rampazzo Reviewed-by: Eric Auger Signed-off-by: Cleber Rosa Message-Id: <20210412044644.55083-5-crosa@redhat.com> Signed-off-by: John Snow --- tests/acceptance/avocado_qemu/__init__.py | 48 ++++++++++++++++++++++- tests/acceptance/linux_ssh_mips_malta.py | 40 ++----------------- tests/acceptance/virtiofs_submounts.py | 37 ----------------- 3 files changed, 50 insertions(+), 75 deletions(-) diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py index 83b1741ec8..67f75f66e5 100644 --- a/tests/acceptance/avocado_qemu/__init__.py +++ b/tests/acceptance/avocado_qemu/__init__.py @@ -20,6 +20,7 @@ import avocado from avocado.utils import cloudinit from avocado.utils import datadrainer from avocado.utils import network +from avocado.utils import ssh from avocado.utils import vmimage from avocado.utils.path import find_command @@ -43,6 +44,8 @@ sys.path.append(os.path.join(SOURCE_DIR, 'python')) from qemu.accel import kvm_available from qemu.accel import tcg_available from qemu.machine import QEMUMachine +from qemu.utils import get_info_usernet_hostfwd_port + def is_readable_executable_file(path): return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK) @@ -253,7 +256,50 @@ class Test(avocado.Test): cancel_on_missing=cancel_on_missing) -class LinuxTest(Test): +class LinuxSSHMixIn: + """Contains utility methods for interacting with a guest via SSH.""" + + def ssh_connect(self, username, credential, credential_is_key=True): + self.ssh_logger = logging.getLogger('ssh') + res = self.vm.command('human-monitor-command', + command_line='info usernet') + port = get_info_usernet_hostfwd_port(res) + self.assertIsNotNone(port) + self.assertGreater(port, 0) + self.log.debug('sshd listening on port: %d', port) + if credential_is_key: + self.ssh_session = ssh.Session('127.0.0.1', port=port, + user=username, key=credential) + else: + self.ssh_session = ssh.Session('127.0.0.1', port=port, + user=username, password=credential) + for i in range(10): + try: + self.ssh_session.connect() + return + except: + time.sleep(4) + pass + self.fail('ssh connection timeout') + + def ssh_command(self, command): + self.ssh_logger.info(command) + result = self.ssh_session.cmd(command) + stdout_lines = [line.rstrip() for line + in result.stdout_text.splitlines()] + for line in stdout_lines: + self.ssh_logger.info(line) + stderr_lines = [line.rstrip() for line + in result.stderr_text.splitlines()] + for line in stderr_lines: + self.ssh_logger.warning(line) + + self.assertEqual(result.exit_status, 0, + f'Guest command failed: {command}') + return stdout_lines, stderr_lines + + +class LinuxTest(Test, LinuxSSHMixIn): """Facilitates having a cloud-image Linux based available. For tests that indend to interact with guests, this is a better choice diff --git a/tests/acceptance/linux_ssh_mips_malta.py b/tests/acceptance/linux_ssh_mips_malta.py index 052008f02d..61c9079d04 100644 --- a/tests/acceptance/linux_ssh_mips_malta.py +++ b/tests/acceptance/linux_ssh_mips_malta.py @@ -12,16 +12,14 @@ import logging import time from avocado import skipUnless -from avocado_qemu import Test +from avocado_qemu import Test, LinuxSSHMixIn from avocado_qemu import wait_for_console_pattern from avocado.utils import process from avocado.utils import archive from avocado.utils import ssh -from qemu.utils import get_info_usernet_hostfwd_port - -class LinuxSSH(Test): +class LinuxSSH(Test, LinuxSSHMixIn): timeout = 150 # Not for 'configure --enable-debug --enable-debug-tcg' @@ -72,41 +70,9 @@ class LinuxSSH(Test): def setUp(self): super(LinuxSSH, self).setUp() - def ssh_connect(self, username, password): - self.ssh_logger = logging.getLogger('ssh') - res = self.vm.command('human-monitor-command', - command_line='info usernet') - port = get_info_usernet_hostfwd_port(res) - if not port: - self.cancel("Failed to retrieve SSH port") - self.log.debug("sshd listening on port:" + port) - self.ssh_session = ssh.Session(self.VM_IP, port=int(port), - user=username, password=password) - for i in range(10): - try: - self.ssh_session.connect() - return - except: - time.sleep(4) - pass - self.fail("ssh connection timeout") - def ssh_disconnect_vm(self): self.ssh_session.quit() - def ssh_command(self, command, is_root=True): - self.ssh_logger.info(command) - result = self.ssh_session.cmd(command) - stdout_lines = [line.rstrip() for line - in result.stdout_text.splitlines()] - for line in stdout_lines: - self.ssh_logger.info(line) - stderr_lines = [line.rstrip() for line - in result.stderr_text.splitlines()] - for line in stderr_lines: - self.ssh_logger.warning(line) - return stdout_lines, stderr_lines - def boot_debian_wheezy_image_and_ssh_login(self, endianess, kernel_path): image_url, image_hash = self.get_image_info(endianess) image_path = self.fetch_asset(image_url, asset_hash=image_hash) @@ -127,7 +93,7 @@ class LinuxSSH(Test): wait_for_console_pattern(self, console_pattern, 'Oops') self.log.info('sshd ready') - self.ssh_connect('root', 'root') + self.ssh_connect('root', 'root', False) def shutdown_via_ssh(self): self.ssh_command('poweroff') diff --git a/tests/acceptance/virtiofs_submounts.py b/tests/acceptance/virtiofs_submounts.py index 57a7047342..bed8ce44df 100644 --- a/tests/acceptance/virtiofs_submounts.py +++ b/tests/acceptance/virtiofs_submounts.py @@ -9,8 +9,6 @@ from avocado_qemu import LinuxTest, BUILD_DIR from avocado_qemu import wait_for_console_pattern from avocado.utils import ssh -from qemu.utils import get_info_usernet_hostfwd_port - def run_cmd(args): subp = subprocess.Popen(args, @@ -75,41 +73,6 @@ class VirtiofsSubmountsTest(LinuxTest): :avocado: tags=accel:kvm """ - def ssh_connect(self, username, keyfile): - self.ssh_logger = logging.getLogger('ssh') - res = self.vm.command('human-monitor-command', - command_line='info usernet') - port = get_info_usernet_hostfwd_port(res) - self.assertIsNotNone(port) - self.assertGreater(port, 0) - self.log.debug('sshd listening on port: %d', port) - self.ssh_session = ssh.Session('127.0.0.1', port=port, - user=username, key=keyfile) - for i in range(10): - try: - self.ssh_session.connect() - return - except: - time.sleep(4) - pass - self.fail('ssh connection timeout') - - def ssh_command(self, command): - self.ssh_logger.info(command) - result = self.ssh_session.cmd(command) - stdout_lines = [line.rstrip() for line - in result.stdout_text.splitlines()] - for line in stdout_lines: - self.ssh_logger.info(line) - stderr_lines = [line.rstrip() for line - in result.stderr_text.splitlines()] - for line in stderr_lines: - self.ssh_logger.warning(line) - - self.assertEqual(result.exit_status, 0, - f'Guest command failed: {command}') - return stdout_lines, stderr_lines - def run(self, args, ignore_error=False): stdout, stderr, ret = run_cmd(args) From 54914114aff5008b58d3cf01bf9e2274144875ca Mon Sep 17 00:00:00 2001 From: Cleber Rosa Date: Mon, 12 Apr 2021 00:46:38 -0400 Subject: [PATCH 06/44] Acceptance Tests: add port redirection for ssh by default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For users of the LinuxTest class, let's set up the VM with the port redirection for SSH, instead of requiring each test to set the same arguments. It also sets the network device, by default, to virtio-net. Signed-off-by: Cleber Rosa Reviewed-by: Marc-André Lureau Reviewed-by: Eric Auger Reviewed-by: Willian Rampazzo Message-Id: <20210412044644.55083-6-crosa@redhat.com> Signed-off-by: John Snow --- tests/acceptance/avocado_qemu/__init__.py | 5 ++++- tests/acceptance/virtiofs_submounts.py | 4 ---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py index 67f75f66e5..0856880000 100644 --- a/tests/acceptance/avocado_qemu/__init__.py +++ b/tests/acceptance/avocado_qemu/__init__.py @@ -309,10 +309,13 @@ class LinuxTest(Test, LinuxSSHMixIn): timeout = 900 chksum = None - def setUp(self, ssh_pubkey=None): + def setUp(self, ssh_pubkey=None, network_device_type='virtio-net'): super(LinuxTest, self).setUp() self.vm.add_args('-smp', '2') self.vm.add_args('-m', '1024') + # The following network device allows for SSH connections + self.vm.add_args('-netdev', 'user,id=vnet,hostfwd=:127.0.0.1:0-:22', + '-device', '%s,netdev=vnet' % network_device_type) self.set_up_boot() if ssh_pubkey is None: ssh_pubkey, self.ssh_key = self.set_up_existing_ssh_keys() diff --git a/tests/acceptance/virtiofs_submounts.py b/tests/acceptance/virtiofs_submounts.py index bed8ce44df..e10a935ac4 100644 --- a/tests/acceptance/virtiofs_submounts.py +++ b/tests/acceptance/virtiofs_submounts.py @@ -207,10 +207,6 @@ class VirtiofsSubmountsTest(LinuxTest): self.vm.add_args('-kernel', vmlinuz, '-append', 'console=ttyS0 root=/dev/sda1') - # Allow us to connect to SSH - self.vm.add_args('-netdev', 'user,id=vnet,hostfwd=:127.0.0.1:0-:22', - '-device', 'virtio-net,netdev=vnet') - self.require_accelerator("kvm") self.vm.add_args('-accel', 'kvm') From d8c6a89968906af24ab27acd936013d3f937fc16 Mon Sep 17 00:00:00 2001 From: Cleber Rosa Date: Mon, 12 Apr 2021 00:46:39 -0400 Subject: [PATCH 07/44] Acceptance Tests: make username/password configurable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This makes the username/password used for authentication configurable, because some guest operating systems may have restrictions on accounts to be used for logins, and it just makes it better documented. Signed-off-by: Cleber Rosa Reviewed-by: Marc-André Lureau Reviewed-by: Eric Auger Reviewed-by: Willian Rampazzo Message-Id: <20210412044644.55083-7-crosa@redhat.com> Signed-off-by: John Snow --- tests/acceptance/avocado_qemu/__init__.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py index 0856880000..25f871f5bc 100644 --- a/tests/acceptance/avocado_qemu/__init__.py +++ b/tests/acceptance/avocado_qemu/__init__.py @@ -308,6 +308,8 @@ class LinuxTest(Test, LinuxSSHMixIn): timeout = 900 chksum = None + username = 'root' + password = 'password' def setUp(self, ssh_pubkey=None, network_device_type='virtio-net'): super(LinuxTest, self).setUp() @@ -371,8 +373,8 @@ class LinuxTest(Test, LinuxSSHMixIn): with open(ssh_pubkey) as pubkey: pubkey_content = pubkey.read() cloudinit.iso(cloudinit_iso, self.name, - username='root', - password='password', + username=self.username, + password=self.password, # QEMU's hard coded usermode router address phone_home_host='10.0.2.2', phone_home_port=self.phone_home_port, From c6620c443d076bc0c80357e41f8f8d7fcdade6df Mon Sep 17 00:00:00 2001 From: Cleber Rosa Date: Mon, 12 Apr 2021 00:46:40 -0400 Subject: [PATCH 08/44] Acceptance Tests: set up SSH connection by default after boot for LinuxTest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The LinuxTest specifically targets users that need to interact with Linux guests. So, it makes sense to give a connection by default, and avoid requiring it as boiler-plate code. Signed-off-by: Cleber Rosa Reviewed-by: Marc-André Lureau Reviewed-by: Willian Rampazzo Message-Id: <20210412044644.55083-8-crosa@redhat.com> Signed-off-by: John Snow --- tests/acceptance/avocado_qemu/__init__.py | 5 ++++- tests/acceptance/boot_linux.py | 18 +++++++++--------- tests/acceptance/virtiofs_submounts.py | 1 - 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py index 25f871f5bc..1062a851b9 100644 --- a/tests/acceptance/avocado_qemu/__init__.py +++ b/tests/acceptance/avocado_qemu/__init__.py @@ -391,7 +391,7 @@ class LinuxTest(Test, LinuxSSHMixIn): cloudinit_iso = self.prepare_cloudinit(ssh_pubkey) self.vm.add_args('-drive', 'file=%s,format=raw' % cloudinit_iso) - def launch_and_wait(self): + def launch_and_wait(self, set_up_ssh_connection=True): self.vm.set_console() self.vm.launch() console_drainer = datadrainer.LineLogger(self.vm.console_socket.fileno(), @@ -399,3 +399,6 @@ class LinuxTest(Test, LinuxSSHMixIn): console_drainer.start() self.log.info('VM launched, waiting for boot confirmation from guest') cloudinit.wait_for_phone_home(('0.0.0.0', self.phone_home_port), self.name) + if set_up_ssh_connection: + self.log.info('Setting up the SSH connection') + self.ssh_connect(self.username, self.ssh_key) diff --git a/tests/acceptance/boot_linux.py b/tests/acceptance/boot_linux.py index 0d178038a0..314370fd1f 100644 --- a/tests/acceptance/boot_linux.py +++ b/tests/acceptance/boot_linux.py @@ -29,7 +29,7 @@ class BootLinuxX8664(LinuxTest): """ self.require_accelerator("tcg") self.vm.add_args("-accel", "tcg") - self.launch_and_wait() + self.launch_and_wait(set_up_ssh_connection=False) def test_pc_i440fx_kvm(self): """ @@ -38,7 +38,7 @@ class BootLinuxX8664(LinuxTest): """ self.require_accelerator("kvm") self.vm.add_args("-accel", "kvm") - self.launch_and_wait() + self.launch_and_wait(set_up_ssh_connection=False) def test_pc_q35_tcg(self): """ @@ -47,7 +47,7 @@ class BootLinuxX8664(LinuxTest): """ self.require_accelerator("tcg") self.vm.add_args("-accel", "tcg") - self.launch_and_wait() + self.launch_and_wait(set_up_ssh_connection=False) def test_pc_q35_kvm(self): """ @@ -56,7 +56,7 @@ class BootLinuxX8664(LinuxTest): """ self.require_accelerator("kvm") self.vm.add_args("-accel", "kvm") - self.launch_and_wait() + self.launch_and_wait(set_up_ssh_connection=False) class BootLinuxAarch64(LinuxTest): @@ -85,7 +85,7 @@ class BootLinuxAarch64(LinuxTest): self.vm.add_args("-cpu", "max") self.vm.add_args("-machine", "virt,gic-version=2") self.add_common_args() - self.launch_and_wait() + self.launch_and_wait(set_up_ssh_connection=False) def test_virt_kvm_gicv2(self): """ @@ -98,7 +98,7 @@ class BootLinuxAarch64(LinuxTest): self.vm.add_args("-cpu", "host") self.vm.add_args("-machine", "virt,gic-version=2") self.add_common_args() - self.launch_and_wait() + self.launch_and_wait(set_up_ssh_connection=False) def test_virt_kvm_gicv3(self): """ @@ -111,7 +111,7 @@ class BootLinuxAarch64(LinuxTest): self.vm.add_args("-cpu", "host") self.vm.add_args("-machine", "virt,gic-version=3") self.add_common_args() - self.launch_and_wait() + self.launch_and_wait(set_up_ssh_connection=False) class BootLinuxPPC64(LinuxTest): @@ -128,7 +128,7 @@ class BootLinuxPPC64(LinuxTest): """ self.require_accelerator("tcg") self.vm.add_args("-accel", "tcg") - self.launch_and_wait() + self.launch_and_wait(set_up_ssh_connection=False) class BootLinuxS390X(LinuxTest): @@ -146,4 +146,4 @@ class BootLinuxS390X(LinuxTest): """ self.require_accelerator("tcg") self.vm.add_args("-accel", "tcg") - self.launch_and_wait() + self.launch_and_wait(set_up_ssh_connection=False) diff --git a/tests/acceptance/virtiofs_submounts.py b/tests/acceptance/virtiofs_submounts.py index e10a935ac4..e019d3b896 100644 --- a/tests/acceptance/virtiofs_submounts.py +++ b/tests/acceptance/virtiofs_submounts.py @@ -136,7 +136,6 @@ class VirtiofsSubmountsTest(LinuxTest): def launch_vm(self): self.launch_and_wait() - self.ssh_connect('root', self.ssh_key) def set_up_nested_mounts(self): scratch_dir = os.path.join(self.shared_dir, 'scratch') From a273387aec43d2f2cff19b232c8c3e569a669971 Mon Sep 17 00:00:00 2001 From: Cleber Rosa Date: Mon, 12 Apr 2021 00:46:41 -0400 Subject: [PATCH 09/44] tests/acceptance/virtiofs_submounts.py: remove launch_vm() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The LinuxTest class' launch_and_wait() method now behaves the same way as this test's custom launch_vm(), so let's just use the upper layer (common) method. Signed-off-by: Cleber Rosa Reviewed-by: Marc-André Lureau Reviewed-by: Eric Auger Reviewed-by: Willian Rampazzo Message-Id: <20210412044644.55083-9-crosa@redhat.com> Signed-off-by: John Snow --- tests/acceptance/virtiofs_submounts.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/tests/acceptance/virtiofs_submounts.py b/tests/acceptance/virtiofs_submounts.py index e019d3b896..d77ee35674 100644 --- a/tests/acceptance/virtiofs_submounts.py +++ b/tests/acceptance/virtiofs_submounts.py @@ -134,9 +134,6 @@ class VirtiofsSubmountsTest(LinuxTest): '-numa', 'node,memdev=mem') - def launch_vm(self): - self.launch_and_wait() - def set_up_nested_mounts(self): scratch_dir = os.path.join(self.shared_dir, 'scratch') try: @@ -225,7 +222,7 @@ class VirtiofsSubmountsTest(LinuxTest): self.set_up_nested_mounts() self.set_up_virtiofs() - self.launch_vm() + self.launch_and_wait() self.mount_in_guest() self.check_in_guest() @@ -235,14 +232,14 @@ class VirtiofsSubmountsTest(LinuxTest): self.set_up_nested_mounts() - self.launch_vm() + self.launch_and_wait() self.mount_in_guest() self.check_in_guest() def test_post_launch_set_up(self): self.set_up_shared_dir() self.set_up_virtiofs() - self.launch_vm() + self.launch_and_wait() self.set_up_nested_mounts() @@ -252,7 +249,7 @@ class VirtiofsSubmountsTest(LinuxTest): def test_post_mount_set_up(self): self.set_up_shared_dir() self.set_up_virtiofs() - self.launch_vm() + self.launch_and_wait() self.mount_in_guest() self.set_up_nested_mounts() @@ -265,7 +262,7 @@ class VirtiofsSubmountsTest(LinuxTest): self.set_up_nested_mounts() self.set_up_virtiofs() - self.launch_vm() + self.launch_and_wait() self.mount_in_guest() self.check_in_guest() From 1e4e7efa01f021e7abeb0c12ff7bb3758da22134 Mon Sep 17 00:00:00 2001 From: Cleber Rosa Date: Mon, 12 Apr 2021 00:46:42 -0400 Subject: [PATCH 10/44] Acceptance Tests: add basic documentation on LinuxTest base class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Cleber Rosa Reviewed-by: Marc-André Lureau Reviewed-by: Willian Rampazzo Reviewed-by: Eric Auger Reviewed-by: Wainer dos Santos Moschetta Message-Id: <20210412044644.55083-10-crosa@redhat.com> Signed-off-by: John Snow --- docs/devel/testing.rst | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst index 1da4c4e4c4..4e42392810 100644 --- a/docs/devel/testing.rst +++ b/docs/devel/testing.rst @@ -810,6 +810,32 @@ and hypothetical example follows: At test "tear down", ``avocado_qemu.Test`` handles all the QEMUMachines shutdown. +The ``avocado_qemu.LinuxTest`` base test class +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The ``avocado_qemu.LinuxTest`` is further specialization of the +``avocado_qemu.Test`` class, so it contains all the characteristics of +the later plus some extra features. + +First of all, this base class is intended for tests that need to +interact with a fully booted and operational Linux guest. At this +time, it uses a Fedora 31 guest image. The most basic example looks +like this: + +.. code:: + + from avocado_qemu import LinuxTest + + + class SomeTest(LinuxTest): + + def test(self): + self.launch_and_wait() + self.ssh_command('some_command_to_be_run_in_the_guest') + +Please refer to tests that use ``avocado_qemu.LinuxTest`` under +``tests/acceptance`` for more examples. + QEMUMachine ~~~~~~~~~~~ From fd1ce58d901bbe982db8c19ca6e1a63b30643150 Mon Sep 17 00:00:00 2001 From: Cleber Rosa Date: Mon, 12 Apr 2021 00:46:43 -0400 Subject: [PATCH 11/44] Acceptance Tests: introduce CPU hotplug test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Even though there are qtest based tests for hotplugging CPUs (from which this test took some inspiration from), this one adds checks from a Linux guest point of view. It should also serve as an example for tests that follow a similar pattern and need to interact with QEMU (via qmp) and with the Linux guest via SSH. Signed-off-by: Cleber Rosa Reviewed-by: Marc-André Lureau Reviewed-by: Willian Rampazzo Reviewed-by: Eric Auger Message-Id: <20210412044644.55083-11-crosa@redhat.com> Signed-off-by: John Snow --- tests/acceptance/hotplug_cpu.py | 37 +++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 tests/acceptance/hotplug_cpu.py diff --git a/tests/acceptance/hotplug_cpu.py b/tests/acceptance/hotplug_cpu.py new file mode 100644 index 0000000000..6374bf1b54 --- /dev/null +++ b/tests/acceptance/hotplug_cpu.py @@ -0,0 +1,37 @@ +# Functional test that hotplugs a CPU and checks it on a Linux guest +# +# Copyright (c) 2021 Red Hat, Inc. +# +# Author: +# Cleber Rosa +# +# This work is licensed under the terms of the GNU GPL, version 2 or +# later. See the COPYING file in the top-level directory. + +from avocado_qemu import LinuxTest + + +class HotPlugCPU(LinuxTest): + + def test(self): + """ + :avocado: tags=arch:x86_64 + :avocado: tags=machine:q35 + :avocado: tags=accel:kvm + """ + self.require_accelerator('kvm') + self.vm.add_args('-accel', 'kvm') + self.vm.add_args('-cpu', 'Haswell') + self.vm.add_args('-smp', '1,sockets=1,cores=2,threads=1,maxcpus=2') + self.launch_and_wait() + + self.ssh_command('test -e /sys/devices/system/cpu/cpu0') + with self.assertRaises(AssertionError): + self.ssh_command('test -e /sys/devices/system/cpu/cpu1') + + self.vm.command('device_add', + driver='Haswell-x86_64-cpu', + socket_id=0, + core_id=1, + thread_id=0) + self.ssh_command('test -e /sys/devices/system/cpu/cpu1') From d214740c994f51370112ceda33a9d5546ff21c84 Mon Sep 17 00:00:00 2001 From: Cleber Rosa Date: Mon, 12 Apr 2021 00:46:44 -0400 Subject: [PATCH 12/44] tests/acceptance/virtiofs_submounts.py: fix setup of SSH pubkey The public key argument should be a path to a file, and not the public key data. Reported-by: Wainer dos Santos Moschetta Signed-off-by: Cleber Rosa Message-Id: <20210412044644.55083-12-crosa@redhat.com> Reviewed-by: Willian Rampazzo Reviewed-by: Wainer dos Santos Moschetta Signed-off-by: John Snow --- tests/acceptance/virtiofs_submounts.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/acceptance/virtiofs_submounts.py b/tests/acceptance/virtiofs_submounts.py index d77ee35674..21ad7d792e 100644 --- a/tests/acceptance/virtiofs_submounts.py +++ b/tests/acceptance/virtiofs_submounts.py @@ -195,7 +195,7 @@ class VirtiofsSubmountsTest(LinuxTest): self.run(('ssh-keygen', '-N', '', '-t', 'ed25519', '-f', self.ssh_key)) - pubkey = open(self.ssh_key + '.pub').read() + pubkey = self.ssh_key + '.pub' super(VirtiofsSubmountsTest, self).setUp(pubkey) From 41787552de447733debe0616b716a0aa138242c7 Mon Sep 17 00:00:00 2001 From: Willian Rampazzo Date: Thu, 20 May 2021 17:47:47 -0300 Subject: [PATCH 13/44] acceptance tests: bump Avocado version to 88.1 Besides some internal changes, new features, and bug fixes, on the QEMU side, this version fixes the following message seen when running the acceptance tests: "Error running method "pre_tests" of plugin "fetchasset": 'bytes' object has no attribute 'encode'". The release notes are available at https://avocado-framework.readthedocs.io/en/latest/releases/88_0.html. Signed-off-by: Willian Rampazzo Message-Id: <20210520204747.210764-2-willianr@redhat.com> Acked-by: Cleber Rosa Signed-off-by: John Snow --- tests/requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/requirements.txt b/tests/requirements.txt index 91f3a343b9..a21b59b443 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -1,5 +1,5 @@ # Add Python module requirements, one per line, to be installed # in the tests/venv Python virtual environment. For more info, # refer to: https://pip.pypa.io/en/stable/user_guide/#id1 -avocado-framework==85.0 +avocado-framework==88.1 pycdlib==1.11.0 From ee1a27235b7965bc5514555eec898f4d067fced2 Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:16:45 -0400 Subject: [PATCH 14/44] python/console_socket: avoid one-letter variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes pylint warnings. Signed-off-by: John Snow Reviewed-by: Cleber Rosa Reviewed-by: Philippe Mathieu-Daudé Message-id: 20210527211715.394144-2-jsnow@redhat.com Message-id: 20210517184808.3562549-2-jsnow@redhat.com Signed-off-by: John Snow --- python/qemu/console_socket.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/python/qemu/console_socket.py b/python/qemu/console_socket.py index ac21130e44..87237bebef 100644 --- a/python/qemu/console_socket.py +++ b/python/qemu/console_socket.py @@ -46,11 +46,11 @@ class ConsoleSocket(socket.socket): self._drain_thread = self._thread_start() def __repr__(self) -> str: - s = super().__repr__() - s = s.rstrip(">") - s = "%s, logfile=%s, drain_thread=%s>" % (s, self._logfile, - self._drain_thread) - return s + tmp = super().__repr__() + tmp = tmp.rstrip(">") + tmp = "%s, logfile=%s, drain_thread=%s>" % (tmp, self._logfile, + self._drain_thread) + return tmp def _drain_fn(self) -> None: """Drains the socket and runs while the socket is open.""" From 07b71233a7ea77c0ec3687c3a3451865b3b899d3 Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:16:46 -0400 Subject: [PATCH 15/44] python/machine: use subprocess.DEVNULL instead of open(os.path.devnull) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One less file resource to manage, and it helps quiet some pylint >= 2.8.0 warnings about not using a with-context manager for the open call. Signed-off-by: John Snow Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Cleber Rosa Message-id: 20210527211715.394144-3-jsnow@redhat.com Message-id: 20210517184808.3562549-3-jsnow@redhat.com Signed-off-by: John Snow --- python/qemu/machine.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/python/qemu/machine.py b/python/qemu/machine.py index b379fcbe72..5b87e9ce02 100644 --- a/python/qemu/machine.py +++ b/python/qemu/machine.py @@ -223,9 +223,8 @@ class QEMUMachine: assert fd is not None fd_param.append(str(fd)) - devnull = open(os.path.devnull, 'rb') proc = subprocess.Popen( - fd_param, stdin=devnull, stdout=subprocess.PIPE, + fd_param, stdin=subprocess.DEVNULL, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, close_fds=False ) output = proc.communicate()[0] @@ -391,7 +390,6 @@ class QEMUMachine: """ Launch the VM and establish a QMP connection """ - devnull = open(os.path.devnull, 'rb') self._pre_launch() self._qemu_full_args = tuple( chain(self._wrapper, @@ -401,7 +399,7 @@ class QEMUMachine: ) LOG.debug('VM launch command: %r', ' '.join(self._qemu_full_args)) self._popen = subprocess.Popen(self._qemu_full_args, - stdin=devnull, + stdin=subprocess.DEVNULL, stdout=self._qemu_log_file, stderr=subprocess.STDOUT, shell=False, From 14b41797d5eb20fb9c6d0a1fe809e8422938f230 Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:16:47 -0400 Subject: [PATCH 16/44] python/machine: use subprocess.run instead of subprocess.Popen use run() instead of Popen() -- to assert to pylint that we are not forgetting to close a long-running program. Signed-off-by: John Snow Reviewed-by: Cleber Rosa Tested-by: Cleber Rosa Message-id: 20210527211715.394144-4-jsnow@redhat.com Message-id: 20210517184808.3562549-4-jsnow@redhat.com Signed-off-by: John Snow --- python/qemu/machine.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/python/qemu/machine.py b/python/qemu/machine.py index 5b87e9ce02..04e005f381 100644 --- a/python/qemu/machine.py +++ b/python/qemu/machine.py @@ -223,13 +223,16 @@ class QEMUMachine: assert fd is not None fd_param.append(str(fd)) - proc = subprocess.Popen( - fd_param, stdin=subprocess.DEVNULL, stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, close_fds=False + proc = subprocess.run( + fd_param, + stdin=subprocess.DEVNULL, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + check=False, + close_fds=False, ) - output = proc.communicate()[0] - if output: - LOG.debug(output) + if proc.stdout: + LOG.debug(proc.stdout) return proc.returncode From 8825fed82a1949ed74f103c2ff26c4d71d2e4845 Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:16:48 -0400 Subject: [PATCH 17/44] python/console_socket: Add a pylint ignore We manage cleaning up this resource ourselves. Pylint should shush. Signed-off-by: John Snow Reviewed-by: Cleber Rosa Message-id: 20210527211715.394144-5-jsnow@redhat.com Message-id: 20210517184808.3562549-5-jsnow@redhat.com Signed-off-by: John Snow --- python/qemu/console_socket.py | 1 + 1 file changed, 1 insertion(+) diff --git a/python/qemu/console_socket.py b/python/qemu/console_socket.py index 87237bebef..8c4ff598ad 100644 --- a/python/qemu/console_socket.py +++ b/python/qemu/console_socket.py @@ -39,6 +39,7 @@ class ConsoleSocket(socket.socket): self.connect(address) self._logfile = None if file: + # pylint: disable=consider-using-with self._logfile = open(file, "bw") self._open = True self._drain_thread = None From 63c33f3c286efe4c6474b53ae97915c9d1a6923a Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:16:49 -0400 Subject: [PATCH 18/44] python/machine: Disable pylint warning for open() in _pre_launch Shift the open() call later so that the pylint pragma applies *only* to that one open() call. Add a note that suggests why this is safe: the resource is unconditionally cleaned up in _post_shutdown(). _post_shutdown is called after failed launches (see launch()), and unconditionally after every call to shutdown(), and therefore also on __exit__. Signed-off-by: John Snow Reviewed-by: Wainer dos Santos Moschetta Reviewed-by: Cleber Rosa Message-id: 20210527211715.394144-6-jsnow@redhat.com Message-id: 20210517184808.3562549-6-jsnow@redhat.com Signed-off-by: John Snow --- python/qemu/machine.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/python/qemu/machine.py b/python/qemu/machine.py index 04e005f381..c66bc6a9c6 100644 --- a/python/qemu/machine.py +++ b/python/qemu/machine.py @@ -306,7 +306,6 @@ class QEMUMachine: def _pre_launch(self) -> None: self._qemu_log_path = os.path.join(self.temp_dir, self._name + ".log") - self._qemu_log_file = open(self._qemu_log_path, 'wb') if self._console_set: self._remove_files.append(self._console_address) @@ -321,6 +320,11 @@ class QEMUMachine: nickname=self._name ) + # NOTE: Make sure any opened resources are *definitely* freed in + # _post_shutdown()! + # pylint: disable=consider-using-with + self._qemu_log_file = open(self._qemu_log_path, 'wb') + def _post_launch(self) -> None: if self._qmp_connection: self._qmp.accept() From a0eae17a59fcbcdc96af2ea2a6767d758ff4a916 Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:16:50 -0400 Subject: [PATCH 19/44] python/machine: disable warning for Popen in _launch() We handle this resource rather meticulously in shutdown/kill/wait/__exit__ et al, through the laborious mechanisms in _do_shutdown(). Quiet this pylint warning here. Signed-off-by: John Snow Reviewed-by: Cleber Rosa Message-id: 20210527211715.394144-7-jsnow@redhat.com Message-id: 20210517184808.3562549-7-jsnow@redhat.com Signed-off-by: John Snow --- python/qemu/machine.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/python/qemu/machine.py b/python/qemu/machine.py index c66bc6a9c6..5d72c4ca36 100644 --- a/python/qemu/machine.py +++ b/python/qemu/machine.py @@ -405,6 +405,9 @@ class QEMUMachine: self._args) ) LOG.debug('VM launch command: %r', ' '.join(self._qemu_full_args)) + + # Cleaning up of this subprocess is guaranteed by _do_shutdown. + # pylint: disable=consider-using-with self._popen = subprocess.Popen(self._qemu_full_args, stdin=subprocess.DEVNULL, stdout=self._qemu_log_file, From 859aeb67d7372e63bd7bb2c7d063c2a49f2507ab Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:16:51 -0400 Subject: [PATCH 20/44] python/machine: Trim line length to below 80 chars One more little delinting fix that snuck in. Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Cleber Rosa Message-id: 20210527211715.394144-8-jsnow@redhat.com Signed-off-by: John Snow --- python/qemu/machine.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/qemu/machine.py b/python/qemu/machine.py index 5d72c4ca36..a8837b36e4 100644 --- a/python/qemu/machine.py +++ b/python/qemu/machine.py @@ -97,7 +97,7 @@ class QEMUMachine: @param args: list of extra arguments @param wrapper: list of arguments used as prefix to qemu binary @param name: prefix for socket and log file names (default: qemu-PID) - @param base_temp_dir: default location where temporary files are created + @param base_temp_dir: default location where temp files are created @param monitor_address: address for QMP monitor @param socket_scm_helper: helper program, required for send_fd_scm() @param sock_dir: where to create socket (defaults to base_temp_dir) From 7f0a143b0cd7b2b7c05b55b1b6814747ef612ce3 Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:16:52 -0400 Subject: [PATCH 21/44] iotests/297: add --namespace-packages to mypy arguments mypy is kind of weird about how it handles imports. For legacy reasons, it won't load PEP 420 namespaces, because of logic implemented prior to that becoming a standard. So, if you plan on using any, you have to pass --namespace-packages. Alright, fine. Signed-off-by: John Snow Reviewed-by: Cleber Rosa Message-id: 20210527211715.394144-9-jsnow@redhat.com Signed-off-by: John Snow --- tests/qemu-iotests/297 | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297 index a37910b42d..433b732336 100755 --- a/tests/qemu-iotests/297 +++ b/tests/qemu-iotests/297 @@ -95,6 +95,7 @@ def run_linters(): '--warn-redundant-casts', '--warn-unused-ignores', '--no-implicit-reexport', + '--namespace-packages', filename), env=env, check=False, From beb6b57b3b1a1fe6ebc208d2edc12b504f69e29f Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:16:53 -0400 Subject: [PATCH 22/44] python: create qemu packages move python/qemu/*.py to python/qemu/[machine, qmp, utils]/*.py and update import directives across the tree. This is done to create a PEP420 namespace package, in which we may create subpackages. To do this, the namespace directory ("qemu") should not have any modules in it. Those files will go into new 'machine', 'qmp' and 'utils' subpackages instead. Implement machine/__init__.py making the top-level classes and functions from its various modules available directly inside the package. Change qmp.py to qmp/__init__.py similarly, such that all of the useful QMP library classes are available directly from "qemu.qmp" instead of "qemu.qmp.qmp". Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Cleber Rosa Message-id: 20210527211715.394144-10-jsnow@redhat.com Signed-off-by: John Snow --- python/{qemu => }/.isort.cfg | 0 python/qemu/__init__.py | 11 ------- python/qemu/{ => machine}/.flake8 | 0 python/qemu/machine/__init__.py | 33 +++++++++++++++++++++ python/qemu/{ => machine}/console_socket.py | 0 python/qemu/{ => machine}/machine.py | 16 ++++++---- python/qemu/{ => machine}/pylintrc | 0 python/qemu/{ => machine}/qtest.py | 3 +- python/qemu/{qmp.py => qmp/__init__.py} | 12 +++++++- python/qemu/{utils.py => utils/__init__.py} | 18 +++++++++-- python/qemu/{ => utils}/accel.py | 0 tests/acceptance/avocado_qemu/__init__.py | 9 +++--- tests/acceptance/virtio-gpu.py | 2 +- tests/qemu-iotests/300 | 4 +-- tests/qemu-iotests/iotests.py | 2 +- tests/vm/aarch64vm.py | 2 +- tests/vm/basevm.py | 3 +- 17 files changed, 83 insertions(+), 32 deletions(-) rename python/{qemu => }/.isort.cfg (100%) delete mode 100644 python/qemu/__init__.py rename python/qemu/{ => machine}/.flake8 (100%) create mode 100644 python/qemu/machine/__init__.py rename python/qemu/{ => machine}/console_socket.py (100%) rename python/qemu/{ => machine}/machine.py (98%) rename python/qemu/{ => machine}/pylintrc (100%) rename python/qemu/{ => machine}/qtest.py (99%) rename python/qemu/{qmp.py => qmp/__init__.py} (96%) rename python/qemu/{utils.py => utils/__init__.py} (66%) rename python/qemu/{ => utils}/accel.py (100%) diff --git a/python/qemu/.isort.cfg b/python/.isort.cfg similarity index 100% rename from python/qemu/.isort.cfg rename to python/.isort.cfg diff --git a/python/qemu/__init__.py b/python/qemu/__init__.py deleted file mode 100644 index 4ca06c34a4..0000000000 --- a/python/qemu/__init__.py +++ /dev/null @@ -1,11 +0,0 @@ -# QEMU library -# -# Copyright (C) 2015-2016 Red Hat Inc. -# Copyright (C) 2012 IBM Corp. -# -# Authors: -# Fam Zheng -# -# This work is licensed under the terms of the GNU GPL, version 2. See -# the COPYING file in the top-level directory. -# diff --git a/python/qemu/.flake8 b/python/qemu/machine/.flake8 similarity index 100% rename from python/qemu/.flake8 rename to python/qemu/machine/.flake8 diff --git a/python/qemu/machine/__init__.py b/python/qemu/machine/__init__.py new file mode 100644 index 0000000000..98302ea31e --- /dev/null +++ b/python/qemu/machine/__init__.py @@ -0,0 +1,33 @@ +""" +QEMU development and testing library. + +This library provides a few high-level classes for driving QEMU from a +test suite, not intended for production use. + +- QEMUMachine: Configure and Boot a QEMU VM + - QEMUQtestMachine: VM class, with a qtest socket. + +- QEMUQtestProtocol: Connect to, send/receive qtest messages. +""" + +# Copyright (C) 2020-2021 John Snow for Red Hat Inc. +# Copyright (C) 2015-2016 Red Hat Inc. +# Copyright (C) 2012 IBM Corp. +# +# Authors: +# John Snow +# Fam Zheng +# +# This work is licensed under the terms of the GNU GPL, version 2. See +# the COPYING file in the top-level directory. +# + +from .machine import QEMUMachine +from .qtest import QEMUQtestMachine, QEMUQtestProtocol + + +__all__ = ( + 'QEMUMachine', + 'QEMUQtestProtocol', + 'QEMUQtestMachine', +) diff --git a/python/qemu/console_socket.py b/python/qemu/machine/console_socket.py similarity index 100% rename from python/qemu/console_socket.py rename to python/qemu/machine/console_socket.py diff --git a/python/qemu/machine.py b/python/qemu/machine/machine.py similarity index 98% rename from python/qemu/machine.py rename to python/qemu/machine/machine.py index a8837b36e4..d33b02d2ce 100644 --- a/python/qemu/machine.py +++ b/python/qemu/machine/machine.py @@ -38,8 +38,14 @@ from typing import ( Type, ) -from . import console_socket, qmp -from .qmp import QMPMessage, QMPReturnValue, SocketAddrT +from qemu.qmp import ( + QEMUMonitorProtocol, + QMPMessage, + QMPReturnValue, + SocketAddrT, +) + +from . import console_socket LOG = logging.getLogger(__name__) @@ -139,7 +145,7 @@ class QEMUMachine: self._events: List[QMPMessage] = [] self._iolog: Optional[str] = None self._qmp_set = True # Enable QMP monitor by default. - self._qmp_connection: Optional[qmp.QEMUMonitorProtocol] = None + self._qmp_connection: Optional[QEMUMonitorProtocol] = None self._qemu_full_args: Tuple[str, ...] = () self._temp_dir: Optional[str] = None self._launched = False @@ -314,7 +320,7 @@ class QEMUMachine: if self._remove_monitor_sockfile: assert isinstance(self._monitor_address, str) self._remove_files.append(self._monitor_address) - self._qmp_connection = qmp.QEMUMonitorProtocol( + self._qmp_connection = QEMUMonitorProtocol( self._monitor_address, server=True, nickname=self._name @@ -541,7 +547,7 @@ class QEMUMachine: self._qmp_set = enabled @property - def _qmp(self) -> qmp.QEMUMonitorProtocol: + def _qmp(self) -> QEMUMonitorProtocol: if self._qmp_connection is None: raise QEMUMachineError("Attempt to access QMP with no connection") return self._qmp_connection diff --git a/python/qemu/pylintrc b/python/qemu/machine/pylintrc similarity index 100% rename from python/qemu/pylintrc rename to python/qemu/machine/pylintrc diff --git a/python/qemu/qtest.py b/python/qemu/machine/qtest.py similarity index 99% rename from python/qemu/qtest.py rename to python/qemu/machine/qtest.py index 78b97d13cf..e893ca3697 100644 --- a/python/qemu/qtest.py +++ b/python/qemu/machine/qtest.py @@ -26,8 +26,9 @@ from typing import ( TextIO, ) +from qemu.qmp import SocketAddrT + from .machine import QEMUMachine -from .qmp import SocketAddrT class QEMUQtestProtocol: diff --git a/python/qemu/qmp.py b/python/qemu/qmp/__init__.py similarity index 96% rename from python/qemu/qmp.py rename to python/qemu/qmp/__init__.py index 2cd4d43036..9606248a3d 100644 --- a/python/qemu/qmp.py +++ b/python/qemu/qmp/__init__.py @@ -1,4 +1,14 @@ -""" QEMU Monitor Protocol Python class """ +""" +QEMU Monitor Protocol (QMP) development library & tooling. + +This package provides a fairly low-level class for communicating to QMP +protocol servers, as implemented by QEMU, the QEMU Guest Agent, and the +QEMU Storage Daemon. This library is not intended for production use. + +`QEMUMonitorProtocol` is the primary class of interest, and all errors +raised derive from `QMPError`. +""" + # Copyright (C) 2009, 2010 Red Hat Inc. # # Authors: diff --git a/python/qemu/utils.py b/python/qemu/utils/__init__.py similarity index 66% rename from python/qemu/utils.py rename to python/qemu/utils/__init__.py index 5ed789275e..7f1a5138c4 100644 --- a/python/qemu/utils.py +++ b/python/qemu/utils/__init__.py @@ -1,13 +1,14 @@ """ -QEMU utility library +QEMU development and testing utilities -This offers miscellaneous utility functions, which may not be easily -distinguishable or numerous to be in their own module. +This package provides a small handful of utilities for performing +various tasks not directly related to the launching of a VM. """ # Copyright (C) 2021 Red Hat Inc. # # Authors: +# John Snow # Cleber Rosa # # This work is licensed under the terms of the GNU GPL, version 2. See @@ -17,6 +18,17 @@ distinguishable or numerous to be in their own module. import re from typing import Optional +# pylint: disable=import-error +from .accel import kvm_available, list_accel, tcg_available + + +__all__ = ( + 'get_info_usernet_hostfwd_port', + 'kvm_available', + 'list_accel', + 'tcg_available', +) + def get_info_usernet_hostfwd_port(info_usernet_output: str) -> Optional[int]: """ diff --git a/python/qemu/accel.py b/python/qemu/utils/accel.py similarity index 100% rename from python/qemu/accel.py rename to python/qemu/utils/accel.py diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py index 1062a851b9..93c4b9851f 100644 --- a/tests/acceptance/avocado_qemu/__init__.py +++ b/tests/acceptance/avocado_qemu/__init__.py @@ -41,11 +41,12 @@ else: sys.path.append(os.path.join(SOURCE_DIR, 'python')) -from qemu.accel import kvm_available -from qemu.accel import tcg_available from qemu.machine import QEMUMachine -from qemu.utils import get_info_usernet_hostfwd_port - +from qemu.utils import ( + get_info_usernet_hostfwd_port, + kvm_available, + tcg_available, +) def is_readable_executable_file(path): return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK) diff --git a/tests/acceptance/virtio-gpu.py b/tests/acceptance/virtio-gpu.py index ab18cddbb7..e7979343e9 100644 --- a/tests/acceptance/virtio-gpu.py +++ b/tests/acceptance/virtio-gpu.py @@ -10,7 +10,7 @@ from avocado_qemu import wait_for_console_pattern from avocado_qemu import exec_command_and_wait_for_pattern from avocado_qemu import is_readable_executable_file -from qemu.accel import kvm_available +from qemu.utils import kvm_available import os import socket diff --git a/tests/qemu-iotests/300 b/tests/qemu-iotests/300 index b475a92c47..fe94de84ed 100755 --- a/tests/qemu-iotests/300 +++ b/tests/qemu-iotests/300 @@ -28,7 +28,7 @@ import iotests # Import qemu after iotests.py has amended sys.path # pylint: disable=wrong-import-order -import qemu +from qemu.machine import machine BlockBitmapMapping = List[Dict[str, object]] @@ -466,7 +466,7 @@ class TestBlockBitmapMappingErrors(TestDirtyBitmapMigration): # the failed migration try: self.vm_b.shutdown() - except qemu.machine.AbnormalShutdown: + except machine.AbnormalShutdown: pass def test_aliased_bitmap_name_too_long(self) -> None: diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index 92681907ed..89663dac06 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -38,7 +38,7 @@ from contextlib import contextmanager # pylint: disable=import-error, wrong-import-position sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python')) -from qemu import qtest +from qemu.machine import qtest from qemu.qmp import QMPMessage # Use this logger for logging messages directly from the iotests module diff --git a/tests/vm/aarch64vm.py b/tests/vm/aarch64vm.py index d70ab843b6..b00cce07eb 100644 --- a/tests/vm/aarch64vm.py +++ b/tests/vm/aarch64vm.py @@ -14,7 +14,7 @@ import os import sys import subprocess import basevm -from qemu.accel import kvm_available +from qemu.utils import kvm_available # This is the config needed for current version of QEMU. # This works for both kvm and tcg. diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py index 995e642465..0f2e436ed3 100644 --- a/tests/vm/basevm.py +++ b/tests/vm/basevm.py @@ -19,9 +19,8 @@ import logging import time import datetime sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python')) -from qemu.accel import kvm_available from qemu.machine import QEMUMachine -from qemu.utils import get_info_usernet_hostfwd_port +from qemu.utils import get_info_usernet_hostfwd_port, kvm_available import subprocess import hashlib import argparse From ea1213b7ccc7c24a3c704dc88bd855df45203fed Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:16:54 -0400 Subject: [PATCH 23/44] python: add qemu package installer Add setup.cfg and setup.py, necessary for installing a package via pip. Add a ReST document (PACKAGE.rst) explaining the basics of what this package is for and who to contact for more information. This document will be used as the landing page for the package on PyPI. List the subpackages we intend to package by name instead of using find_namespace because find_namespace will naively also packages tests, things it finds in the dist/ folder, etc. I could not figure out how to modify this behavior; adding allow/deny lists to setuptools kept changing the packaged hierarchy. This works, roll with it. I am not yet using a pyproject.toml style package manifest, because "editable" installs are not defined (yet?) by PEP-517/518. I consider editable installs crucial for development, though they have (apparently) always been somewhat poorly defined. Pip now (19.2 and later) now supports editable installs for projects using pyproject.toml manifests, but might require the use of the --no-use-pep517 flag, which somewhat defeats the point. Full support for setup.py-less editable installs was not introduced until pip 21.1.1: https://github.com/pypa/pip/pull/9547/commits/7a95720e796a5e56481c1cc20b6ce6249c50f357 For now, while the dust settles, stick with the de-facto setup.py/setup.cfg combination supported by setuptools. It will be worth re-evaluating this point again in the future when our supported build platforms all ship a fairly modern pip. Additional reading on this matter: https://github.com/pypa/packaging-problems/issues/256 https://github.com/pypa/pip/issues/6334 https://github.com/pypa/pip/issues/6375 https://github.com/pypa/pip/issues/6434 https://github.com/pypa/pip/issues/6438 Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Cleber Rosa Message-id: 20210527211715.394144-11-jsnow@redhat.com Signed-off-by: John Snow --- python/PACKAGE.rst | 33 +++++++++++++++++++++++++++++++++ python/setup.cfg | 22 ++++++++++++++++++++++ python/setup.py | 23 +++++++++++++++++++++++ 3 files changed, 78 insertions(+) create mode 100644 python/PACKAGE.rst create mode 100644 python/setup.cfg create mode 100755 python/setup.py diff --git a/python/PACKAGE.rst b/python/PACKAGE.rst new file mode 100644 index 0000000000..1bbfe1b58e --- /dev/null +++ b/python/PACKAGE.rst @@ -0,0 +1,33 @@ +QEMU Python Tooling +=================== + +This package provides QEMU tooling used by the QEMU project to build, +configure, and test QEMU. It is not a fully-fledged SDK and it is subject +to change at any time. + +Usage +----- + +The ``qemu.qmp`` subpackage provides a library for communicating with +QMP servers. The ``qemu.machine`` subpackage offers rudimentary +facilities for launching and managing QEMU processes. Refer to each +package's documentation +(``>>> help(qemu.qmp)``, ``>>> help(qemu.machine)``) +for more information. + +Contributing +------------ + +This package is maintained by John Snow as part of +the QEMU source tree. Contributions are welcome and follow the `QEMU +patch submission process +`_, which involves +sending patches to the QEMU development mailing list. + +John maintains a `GitLab staging branch +`_, and there is an +official `GitLab mirror `_. + +Please report bugs on the `QEMU issue tracker +`_ and tag ``@jsnow`` in +the report. diff --git a/python/setup.cfg b/python/setup.cfg new file mode 100644 index 0000000000..3fa92a2e73 --- /dev/null +++ b/python/setup.cfg @@ -0,0 +1,22 @@ +[metadata] +name = qemu +maintainer = QEMU Developer Team +maintainer_email = qemu-devel@nongnu.org +url = https://www.qemu.org/ +download_url = https://www.qemu.org/download/ +description = QEMU Python Build, Debug and SDK tooling. +long_description = file:PACKAGE.rst +long_description_content_type = text/x-rst +classifiers = + Development Status :: 3 - Alpha + License :: OSI Approved :: GNU General Public License v2 (GPLv2) + Natural Language :: English + Operating System :: OS Independent + Programming Language :: Python :: 3 :: Only + +[options] +python_requires = >= 3.6 +packages = + qemu.qmp + qemu.machine + qemu.utils diff --git a/python/setup.py b/python/setup.py new file mode 100755 index 0000000000..2014f81b75 --- /dev/null +++ b/python/setup.py @@ -0,0 +1,23 @@ +#!/usr/bin/env python3 +""" +QEMU tooling installer script +Copyright (c) 2020-2021 John Snow for Red Hat, Inc. +""" + +import setuptools +import pkg_resources + + +def main(): + """ + QEMU tooling installer + """ + + # https://medium.com/@daveshawley/safely-using-setup-cfg-for-metadata-1babbe54c108 + pkg_resources.require('setuptools>=39.2') + + setuptools.setup() + + +if __name__ == '__main__': + main() From 3afc32906f7bffd8e09b7d247d60b55c49665bd3 Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:16:55 -0400 Subject: [PATCH 24/44] python: add VERSION file Python infrastructure as it exists today is not capable reliably of single-sourcing a package version from a parent directory. The authors of pip are working to correct this, but as of today this is not possible. The problem is that when using pip to build and install a python package, it copies files over to a temporary directory and performs its build there. This loses access to any information in the parent directory, including git itself. Further, Python versions have a standard (PEP 440) that may or may not follow QEMU's versioning. In general, it does; but naturally QEMU does not follow PEP 440. To avoid any automatically-generated conflict, a manual version file is preferred. I am proposing: - Python tooling follows the QEMU version, indirectly, but with a major version of 0 to indicate that the API is not expected to be stable. This would mean version 0.5.2.0, 0.5.1.1, 0.5.3.0, etc. - In the event that a Python package needs to be updated independently of the QEMU version, a pre-release alpha version should be preferred, but *only* after inclusion to the qemu development or stable branches. e.g. 0.5.2.0a1, 0.5.2.0a2, and so on should be preferred prior to 5.2.0's release. - The Python core tooling makes absolutely no version compatibility checks or constraints. It *may* work with releases of QEMU from the past or future, but it is not required to. i.e., "qemu.machine" will, for now, remain in lock-step with QEMU. - We reserve the right to split the qemu package into independently versioned subpackages at a later date. This might allow for us to begin versioning QMP independently from QEMU at a later date, if we so choose. Implement this versioning scheme by adding a VERSION file and setting it to 0.6.0.0a1. Signed-off-by: John Snow Reviewed-by: Cleber Rosa Message-id: 20210527211715.394144-12-jsnow@redhat.com Signed-off-by: John Snow --- python/VERSION | 1 + python/setup.cfg | 1 + 2 files changed, 2 insertions(+) create mode 100644 python/VERSION diff --git a/python/VERSION b/python/VERSION new file mode 100644 index 0000000000..c19f3b832b --- /dev/null +++ b/python/VERSION @@ -0,0 +1 @@ +0.6.1.0a1 diff --git a/python/setup.cfg b/python/setup.cfg index 3fa92a2e73..b0010e0188 100644 --- a/python/setup.cfg +++ b/python/setup.cfg @@ -1,5 +1,6 @@ [metadata] name = qemu +version = file:VERSION maintainer = QEMU Developer Team maintainer_email = qemu-devel@nongnu.org url = https://www.qemu.org/ From 93128815af4efcaba03a5581c959bc7f98ee2725 Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:16:56 -0400 Subject: [PATCH 25/44] python: add directory structure README.rst files Add short readmes to python/, python/qemu/, python/qemu/machine, python/qemu/qmp, and python/qemu/utils that explain the directory hierarchy. These readmes are visible when browsing the source on e.g. gitlab/github and are designed to help new developers/users quickly make sense of the source tree. They are not designed for inclusion in a published manual. Signed-off-by: John Snow Reviewed-by: Cleber Rosa Message-id: 20210527211715.394144-13-jsnow@redhat.com Signed-off-by: John Snow --- python/README.rst | 41 ++++++++++++++++++++++++++++++++++ python/qemu/README.rst | 8 +++++++ python/qemu/machine/README.rst | 9 ++++++++ python/qemu/qmp/README.rst | 9 ++++++++ python/qemu/utils/README.rst | 7 ++++++ 5 files changed, 74 insertions(+) create mode 100644 python/README.rst create mode 100644 python/qemu/README.rst create mode 100644 python/qemu/machine/README.rst create mode 100644 python/qemu/qmp/README.rst create mode 100644 python/qemu/utils/README.rst diff --git a/python/README.rst b/python/README.rst new file mode 100644 index 0000000000..38b0c83f32 --- /dev/null +++ b/python/README.rst @@ -0,0 +1,41 @@ +QEMU Python Tooling +=================== + +This directory houses Python tooling used by the QEMU project to build, +configure, and test QEMU. It is organized by namespace (``qemu``), and +then by package (e.g. ``qemu/machine``, ``qemu/qmp``, etc). + +``setup.py`` is used by ``pip`` to install this tooling to the current +environment. ``setup.cfg`` provides the packaging configuration used by +``setup.py`` in a setuptools specific format. You will generally invoke +it by doing one of the following: + +1. ``pip3 install .`` will install these packages to your current + environment. If you are inside a virtual environment, they will + install there. If you are not, it will attempt to install to the + global environment, which is **not recommended**. + +2. ``pip3 install --user .`` will install these packages to your user's + local python packages. If you are inside of a virtual environment, + this will fail; you likely want the first invocation above. + +If you append the ``-e`` argument, pip will install in "editable" mode; +which installs a version of the package that installs a forwarder +pointing to these files, such that the package always reflects the +latest version in your git tree. + +See `Installing packages using pip and virtual environments +`_ +for more information. + + +Files in this directory +----------------------- + +- ``qemu/`` Python package source directory. +- ``PACKAGE.rst`` is used as the README file that is visible on PyPI.org. +- ``README.rst`` you are here! +- ``VERSION`` contains the PEP-440 compliant version used to describe + this package; it is referenced by ``setup.cfg``. +- ``setup.cfg`` houses setuptools package configuration. +- ``setup.py`` is the setuptools installer used by pip; See above. diff --git a/python/qemu/README.rst b/python/qemu/README.rst new file mode 100644 index 0000000000..d04943f526 --- /dev/null +++ b/python/qemu/README.rst @@ -0,0 +1,8 @@ +QEMU Python Namespace +===================== + +This directory serves as the root of a `Python PEP 420 implicit +namespace package `_. + +Each directory below is assumed to be an installable Python package that +is available under the ``qemu.`` namespace. diff --git a/python/qemu/machine/README.rst b/python/qemu/machine/README.rst new file mode 100644 index 0000000000..ac2b4fffb4 --- /dev/null +++ b/python/qemu/machine/README.rst @@ -0,0 +1,9 @@ +qemu.machine package +==================== + +This package provides core utilities used for testing and debugging +QEMU. It is used by the iotests, vm tests, acceptance tests, and several +other utilities in the ./scripts directory. It is not a fully-fledged +SDK and it is subject to change at any time. + +See the documentation in ``__init__.py`` for more information. diff --git a/python/qemu/qmp/README.rst b/python/qemu/qmp/README.rst new file mode 100644 index 0000000000..c21951491c --- /dev/null +++ b/python/qemu/qmp/README.rst @@ -0,0 +1,9 @@ +qemu.qmp package +================ + +This package provides a library used for connecting to and communicating +with QMP servers. It is used extensively by iotests, vm tests, +acceptance tests, and other utilities in the ./scripts directory. It is +not a fully-fledged SDK and is subject to change at any time. + +See the documentation in ``__init__.py`` for more information. diff --git a/python/qemu/utils/README.rst b/python/qemu/utils/README.rst new file mode 100644 index 0000000000..975fbf4d7d --- /dev/null +++ b/python/qemu/utils/README.rst @@ -0,0 +1,7 @@ +qemu.utils package +================== + +This package provides miscellaneous utilities used for testing and +debugging QEMU. It is used primarily by the vm and acceptance tests. + +See the documentation in ``__init__.py`` for more information. From eae4e442caa087b2ef292a5fb6377236fa8423f2 Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:16:57 -0400 Subject: [PATCH 26/44] python: add MANIFEST.in When creating a source or binary distribution via 'python3 setup.py ', the VERSION and PACKAGE.rst files aren't bundled by default. Create a MANIFEST.in file that instructs the build tools to include these so that installation from these files won't fail. This is required by 'tox', as well as by the tooling needed to upload packages to PyPI. Exclude the 'README.rst' file -- that's intended as a guidebook to our source tree, not a file that needs to be distributed. Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Cleber Rosa Message-id: 20210527211715.394144-14-jsnow@redhat.com Signed-off-by: John Snow --- python/MANIFEST.in | 3 +++ python/README.rst | 2 ++ 2 files changed, 5 insertions(+) create mode 100644 python/MANIFEST.in diff --git a/python/MANIFEST.in b/python/MANIFEST.in new file mode 100644 index 0000000000..7059ad2822 --- /dev/null +++ b/python/MANIFEST.in @@ -0,0 +1,3 @@ +include VERSION +include PACKAGE.rst +exclude README.rst diff --git a/python/README.rst b/python/README.rst index 38b0c83f32..0099646ae2 100644 --- a/python/README.rst +++ b/python/README.rst @@ -33,6 +33,8 @@ Files in this directory ----------------------- - ``qemu/`` Python package source directory. +- ``MANIFEST.in`` is read by python setuptools, it specifies additional files + that should be included by a source distribution. - ``PACKAGE.rst`` is used as the README file that is visible on PyPI.org. - ``README.rst`` you are here! - ``VERSION`` contains the PEP-440 compliant version used to describe From 41c1d81cf2a9bfdb310576a716f3777e8feb1822 Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:16:58 -0400 Subject: [PATCH 27/44] python: Add pipenv support pipenv is a tool used for managing virtual environments with pinned, explicit dependencies. It is used for precisely recreating python virtual environments. pipenv uses two files to do this: (1) Pipfile, which is similar in purpose and scope to what setup.cfg lists. It specifies the requisite minimum to get a functional environment for using this package. (2) Pipfile.lock, which is similar in purpose to `pip freeze > requirements.txt`. It specifies a canonical virtual environment used for deployment or testing. This ensures that all users have repeatable results. The primary benefit of using this tool is to ensure *rock solid* repeatable CI results with a known set of packages. Although I endeavor to support as many versions as I can, the fluid nature of the Python toolchain often means tailoring code for fairly specific versions. Note that pipenv is *not* required to install or use this module; this is purely for the sake of repeatable testing by CI or developers. Here, a "blank" pipfile is added with no dependencies, but specifies Python 3.6 for the virtual environment. Pipfile will specify our version minimums, while Pipfile.lock specifies an exact loadout of packages that were known to operate correctly. This latter file provides the real value for easy setup of container images and CI environments. Signed-off-by: John Snow Reviewed-by: Cleber Rosa Message-id: 20210527211715.394144-15-jsnow@redhat.com Signed-off-by: John Snow --- python/Pipfile | 11 +++++++++++ python/README.rst | 3 +++ 2 files changed, 14 insertions(+) create mode 100644 python/Pipfile diff --git a/python/Pipfile b/python/Pipfile new file mode 100644 index 0000000000..9534830b5e --- /dev/null +++ b/python/Pipfile @@ -0,0 +1,11 @@ +[[source]] +name = "pypi" +url = "https://pypi.org/simple" +verify_ssl = true + +[dev-packages] + +[packages] + +[requires] +python_version = "3.6" diff --git a/python/README.rst b/python/README.rst index 0099646ae2..bf9bbca979 100644 --- a/python/README.rst +++ b/python/README.rst @@ -36,6 +36,9 @@ Files in this directory - ``MANIFEST.in`` is read by python setuptools, it specifies additional files that should be included by a source distribution. - ``PACKAGE.rst`` is used as the README file that is visible on PyPI.org. +- ``Pipfile`` is used by Pipenv to generate ``Pipfile.lock``. +- ``Pipfile.lock`` is a set of pinned package dependencies that this package + is tested under in our CI suite. It is used by ``make venv-check``. - ``README.rst`` you are here! - ``VERSION`` contains the PEP-440 compliant version used to describe this package; it is referenced by ``setup.cfg``. From d1e0476958cd275419754b8acf31a9f1dc62d3dd Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:16:59 -0400 Subject: [PATCH 28/44] python: add pylint import exceptions Pylint 2.5.x - 2.7.x have regressions that make import checking inconsistent, see: https://github.com/PyCQA/pylint/issues/3609 https://github.com/PyCQA/pylint/issues/3624 https://github.com/PyCQA/pylint/issues/3651 Pinning to 2.4.4 is worse, because it mandates versions of shared dependencies that are too old for features we want in isort and mypy. Oh well. Signed-off-by: John Snow Reviewed-by: Cleber Rosa Message-id: 20210527211715.394144-16-jsnow@redhat.com Signed-off-by: John Snow --- python/qemu/machine/__init__.py | 3 +++ python/qemu/machine/machine.py | 2 +- python/qemu/machine/qtest.py | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/python/qemu/machine/__init__.py b/python/qemu/machine/__init__.py index 98302ea31e..728f27adbe 100644 --- a/python/qemu/machine/__init__.py +++ b/python/qemu/machine/__init__.py @@ -22,6 +22,9 @@ test suite, not intended for production use. # the COPYING file in the top-level directory. # +# pylint: disable=import-error +# see: https://github.com/PyCQA/pylint/issues/3624 +# see: https://github.com/PyCQA/pylint/issues/3651 from .machine import QEMUMachine from .qtest import QEMUQtestMachine, QEMUQtestProtocol diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py index d33b02d2ce..b62435528e 100644 --- a/python/qemu/machine/machine.py +++ b/python/qemu/machine/machine.py @@ -38,7 +38,7 @@ from typing import ( Type, ) -from qemu.qmp import ( +from qemu.qmp import ( # pylint: disable=import-error QEMUMonitorProtocol, QMPMessage, QMPReturnValue, diff --git a/python/qemu/machine/qtest.py b/python/qemu/machine/qtest.py index e893ca3697..93700684d1 100644 --- a/python/qemu/machine/qtest.py +++ b/python/qemu/machine/qtest.py @@ -26,7 +26,7 @@ from typing import ( TextIO, ) -from qemu.qmp import SocketAddrT +from qemu.qmp import SocketAddrT # pylint: disable=import-error from .machine import QEMUMachine From ef42440d797a1549dd64fe2a51500ba55fe54c3f Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:17:00 -0400 Subject: [PATCH 29/44] python: move pylintrc into setup.cfg Delete the empty settings now that it's sharing a home with settings for other tools. pylint can now be run from this folder as "pylint qemu". Signed-off-by: John Snow Reviewed-by: Cleber Rosa Tested-by: Cleber Rosa Message-id: 20210527211715.394144-17-jsnow@redhat.com Signed-off-by: John Snow --- python/qemu/machine/pylintrc | 58 ------------------------------------ python/setup.cfg | 29 ++++++++++++++++++ 2 files changed, 29 insertions(+), 58 deletions(-) delete mode 100644 python/qemu/machine/pylintrc diff --git a/python/qemu/machine/pylintrc b/python/qemu/machine/pylintrc deleted file mode 100644 index 3f69205000..0000000000 --- a/python/qemu/machine/pylintrc +++ /dev/null @@ -1,58 +0,0 @@ -[MASTER] - -[MESSAGES CONTROL] - -# Disable the message, report, category or checker with the given id(s). You -# can either give multiple identifiers separated by comma (,) or put this -# option multiple times (only on the command line, not in the configuration -# file where it should appear only once). You can also use "--disable=all" to -# disable everything first and then reenable specific checks. For example, if -# you want to run only the similarities checker, you can use "--disable=all -# --enable=similarities". If you want to run only the classes checker, but have -# no Warning level messages displayed, use "--disable=all --enable=classes -# --disable=W". -disable=too-many-arguments, - too-many-instance-attributes, - too-many-public-methods, - -[REPORTS] - -[REFACTORING] - -[MISCELLANEOUS] - -[LOGGING] - -[BASIC] - -# Good variable names which should always be accepted, separated by a comma. -good-names=i, - j, - k, - ex, - Run, - _, - fd, - c, -[VARIABLES] - -[STRING] - -[SPELLING] - -[FORMAT] - -[SIMILARITIES] - -# Ignore imports when computing similarities. -ignore-imports=yes - -[TYPECHECK] - -[CLASSES] - -[IMPORTS] - -[DESIGN] - -[EXCEPTIONS] diff --git a/python/setup.cfg b/python/setup.cfg index b0010e0188..36b4253e93 100644 --- a/python/setup.cfg +++ b/python/setup.cfg @@ -21,3 +21,32 @@ packages = qemu.qmp qemu.machine qemu.utils + +[pylint.messages control] +# Disable the message, report, category or checker with the given id(s). You +# can either give multiple identifiers separated by comma (,) or put this +# option multiple times (only on the command line, not in the configuration +# file where it should appear only once). You can also use "--disable=all" to +# disable everything first and then reenable specific checks. For example, if +# you want to run only the similarities checker, you can use "--disable=all +# --enable=similarities". If you want to run only the classes checker, but have +# no Warning level messages displayed, use "--disable=all --enable=classes +# --disable=W". +disable=too-many-arguments, + too-many-instance-attributes, + too-many-public-methods, + +[pylint.basic] +# Good variable names which should always be accepted, separated by a comma. +good-names=i, + j, + k, + ex, + Run, + _, + fd, + c, + +[pylint.similarities] +# Ignore imports when computing similarities. +ignore-imports=yes From b4d37d8188dbff34f0bf88279eeb5b6cb6d1ff82 Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:17:01 -0400 Subject: [PATCH 30/44] python: add pylint to pipenv We are specifying >= pylint 2.8.x for several reasons: 1. For setup.cfg support, added in pylint 2.5.x 2. To specify a version that has incompatibly dropped bad-whitespace checks (2.6.x) 3. 2.7.x fixes "unsubscriptable" warnings in Python 3.9 4. 2.8.x adds a new, incompatible 'consider-using-with' warning that must be disabled in some cases. These pragmas cause warnings themselves in 2.7.x. Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Cleber Rosa Tested-by: Cleber Rosa Message-id: 20210527211715.394144-18-jsnow@redhat.com Signed-off-by: John Snow --- python/Pipfile | 1 + python/Pipfile.lock | 130 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+) create mode 100644 python/Pipfile.lock diff --git a/python/Pipfile b/python/Pipfile index 9534830b5e..285e2c8e67 100644 --- a/python/Pipfile +++ b/python/Pipfile @@ -4,6 +4,7 @@ url = "https://pypi.org/simple" verify_ssl = true [dev-packages] +pylint = ">=2.8.0" [packages] diff --git a/python/Pipfile.lock b/python/Pipfile.lock new file mode 100644 index 0000000000..c9debd0950 --- /dev/null +++ b/python/Pipfile.lock @@ -0,0 +1,130 @@ +{ + "_meta": { + "hash": { + "sha256": "bd4fb76fcdd145bbf23c3a9dd7ad966113c5ce43ca51cc2d828aa7e73d572901" + }, + "pipfile-spec": 6, + "requires": { + "python_version": "3.6" + }, + "sources": [ + { + "name": "pypi", + "url": "https://pypi.org/simple", + "verify_ssl": true + } + ] + }, + "default": {}, + "develop": { + "astroid": { + "hashes": [ + "sha256:4db03ab5fc3340cf619dbc25e42c2cc3755154ce6009469766d7143d1fc2ee4e", + "sha256:8a398dfce302c13f14bab13e2b14fe385d32b73f4e4853b9bdfb64598baa1975" + ], + "markers": "python_version ~= '3.6'", + "version": "==2.5.6" + }, + "isort": { + "hashes": [ + "sha256:0a943902919f65c5684ac4e0154b1ad4fac6dcaa5d9f3426b732f1c8b5419be6", + "sha256:2bb1680aad211e3c9944dbce1d4ba09a989f04e238296c87fe2139faa26d655d" + ], + "markers": "python_version >= '3.6' and python_version < '4.0'", + "version": "==5.8.0" + }, + "lazy-object-proxy": { + "hashes": [ + "sha256:17e0967ba374fc24141738c69736da90e94419338fd4c7c7bef01ee26b339653", + "sha256:1fee665d2638491f4d6e55bd483e15ef21f6c8c2095f235fef72601021e64f61", + "sha256:22ddd618cefe54305df49e4c069fa65715be4ad0e78e8d252a33debf00f6ede2", + "sha256:24a5045889cc2729033b3e604d496c2b6f588c754f7a62027ad4437a7ecc4837", + "sha256:410283732af311b51b837894fa2f24f2c0039aa7f220135192b38fcc42bd43d3", + "sha256:4732c765372bd78a2d6b2150a6e99d00a78ec963375f236979c0626b97ed8e43", + "sha256:489000d368377571c6f982fba6497f2aa13c6d1facc40660963da62f5c379726", + "sha256:4f60460e9f1eb632584c9685bccea152f4ac2130e299784dbaf9fae9f49891b3", + "sha256:5743a5ab42ae40caa8421b320ebf3a998f89c85cdc8376d6b2e00bd12bd1b587", + "sha256:85fb7608121fd5621cc4377a8961d0b32ccf84a7285b4f1d21988b2eae2868e8", + "sha256:9698110e36e2df951c7c36b6729e96429c9c32b3331989ef19976592c5f3c77a", + "sha256:9d397bf41caad3f489e10774667310d73cb9c4258e9aed94b9ec734b34b495fd", + "sha256:b579f8acbf2bdd9ea200b1d5dea36abd93cabf56cf626ab9c744a432e15c815f", + "sha256:b865b01a2e7f96db0c5d12cfea590f98d8c5ba64ad222300d93ce6ff9138bcad", + "sha256:bf34e368e8dd976423396555078def5cfc3039ebc6fc06d1ae2c5a65eebbcde4", + "sha256:c6938967f8528b3668622a9ed3b31d145fab161a32f5891ea7b84f6b790be05b", + "sha256:d1c2676e3d840852a2de7c7d5d76407c772927addff8d742b9808fe0afccebdf", + "sha256:d7124f52f3bd259f510651450e18e0fd081ed82f3c08541dffc7b94b883aa981", + "sha256:d900d949b707778696fdf01036f58c9876a0d8bfe116e8d220cfd4b15f14e741", + "sha256:ebfd274dcd5133e0afae738e6d9da4323c3eb021b3e13052d8cbd0e457b1256e", + "sha256:ed361bb83436f117f9917d282a456f9e5009ea12fd6de8742d1a4752c3017e93", + "sha256:f5144c75445ae3ca2057faac03fda5a902eff196702b0a24daf1d6ce0650514b" + ], + "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3, 3.4, 3.5'", + "version": "==1.6.0" + }, + "mccabe": { + "hashes": [ + "sha256:ab8a6258860da4b6677da4bd2fe5dc2c659cff31b3ee4f7f5d64e79735b80d42", + "sha256:dd8d182285a0fe56bace7f45b5e7d1a6ebcbf524e8f3bd87eb0f125271b8831f" + ], + "version": "==0.6.1" + }, + "pylint": { + "hashes": [ + "sha256:586d8fa9b1891f4b725f587ef267abe2a1bad89d6b184520c7f07a253dd6e217", + "sha256:f7e2072654a6b6afdf5e2fb38147d3e2d2d43c89f648637baab63e026481279b" + ], + "index": "pypi", + "version": "==2.8.2" + }, + "toml": { + "hashes": [ + "sha256:806143ae5bfb6a3c6e736a764057db0e6a0e05e338b5630894a5f779cabb4f9b", + "sha256:b3bda1d108d5dd99f4a20d24d9c348e91c4db7ab1b749200bded2f839ccbe68f" + ], + "markers": "python_version >= '2.6' and python_version not in '3.0, 3.1, 3.2'", + "version": "==0.10.2" + }, + "typed-ast": { + "hashes": [ + "sha256:01ae5f73431d21eead5015997ab41afa53aa1fbe252f9da060be5dad2c730ace", + "sha256:067a74454df670dcaa4e59349a2e5c81e567d8d65458d480a5b3dfecec08c5ff", + "sha256:0fb71b8c643187d7492c1f8352f2c15b4c4af3f6338f21681d3681b3dc31a266", + "sha256:1b3ead4a96c9101bef08f9f7d1217c096f31667617b58de957f690c92378b528", + "sha256:2068531575a125b87a41802130fa7e29f26c09a2833fea68d9a40cf33902eba6", + "sha256:209596a4ec71d990d71d5e0d312ac935d86930e6eecff6ccc7007fe54d703808", + "sha256:2c726c276d09fc5c414693a2de063f521052d9ea7c240ce553316f70656c84d4", + "sha256:398e44cd480f4d2b7ee8d98385ca104e35c81525dd98c519acff1b79bdaac363", + "sha256:52b1eb8c83f178ab787f3a4283f68258525f8d70f778a2f6dd54d3b5e5fb4341", + "sha256:5feca99c17af94057417d744607b82dd0a664fd5e4ca98061480fd8b14b18d04", + "sha256:7538e495704e2ccda9b234b82423a4038f324f3a10c43bc088a1636180f11a41", + "sha256:760ad187b1041a154f0e4d0f6aae3e40fdb51d6de16e5c99aedadd9246450e9e", + "sha256:777a26c84bea6cd934422ac2e3b78863a37017618b6e5c08f92ef69853e765d3", + "sha256:95431a26309a21874005845c21118c83991c63ea800dd44843e42a916aec5899", + "sha256:9ad2c92ec681e02baf81fdfa056fe0d818645efa9af1f1cd5fd6f1bd2bdfd805", + "sha256:9c6d1a54552b5330bc657b7ef0eae25d00ba7ffe85d9ea8ae6540d2197a3788c", + "sha256:aee0c1256be6c07bd3e1263ff920c325b59849dc95392a05f258bb9b259cf39c", + "sha256:af3d4a73793725138d6b334d9d247ce7e5f084d96284ed23f22ee626a7b88e39", + "sha256:b36b4f3920103a25e1d5d024d155c504080959582b928e91cb608a65c3a49e1a", + "sha256:b9574c6f03f685070d859e75c7f9eeca02d6933273b5e69572e5ff9d5e3931c3", + "sha256:bff6ad71c81b3bba8fa35f0f1921fb24ff4476235a6e94a26ada2e54370e6da7", + "sha256:c190f0899e9f9f8b6b7863debfb739abcb21a5c054f911ca3596d12b8a4c4c7f", + "sha256:c907f561b1e83e93fad565bac5ba9c22d96a54e7ea0267c708bffe863cbe4075", + "sha256:cae53c389825d3b46fb37538441f75d6aecc4174f615d048321b716df2757fb0", + "sha256:dd4a21253f42b8d2b48410cb31fe501d32f8b9fbeb1f55063ad102fe9c425e40", + "sha256:dde816ca9dac1d9c01dd504ea5967821606f02e510438120091b84e852367428", + "sha256:f2362f3cb0f3172c42938946dbc5b7843c2a28aec307c49100c8b38764eb6927", + "sha256:f328adcfebed9f11301eaedfa48e15bdece9b519fb27e6a8c01aa52a17ec31b3", + "sha256:f8afcf15cc511ada719a88e013cec87c11aff7b91f019295eb4530f96fe5ef2f", + "sha256:fb1bbeac803adea29cedd70781399c99138358c26d05fcbd23c13016b7f5ec65" + ], + "markers": "implementation_name == 'cpython' and python_version < '3.8'", + "version": "==1.4.3" + }, + "wrapt": { + "hashes": [ + "sha256:b62ffa81fb85f4332a4f609cab4ac40709470da05643a082ec1eb88e6d9b97d7" + ], + "version": "==1.12.1" + } + } +} From 81f8c4467c1899ef1ba984c70c328ac0c32af10c Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:17:02 -0400 Subject: [PATCH 31/44] python: move flake8 config to setup.cfg Update the comment concerning the flake8 exception to match commit 42c0dd12, whose commit message stated: A note on the flake8 exception: flake8 will warn on *any* bare except, but pylint's is context-aware and will suppress the warning if you re-raise the exception. Signed-off-by: John Snow Reviewed-by: Cleber Rosa Message-id: 20210527211715.394144-19-jsnow@redhat.com Signed-off-by: John Snow --- python/qemu/machine/.flake8 | 2 -- python/setup.cfg | 3 +++ 2 files changed, 3 insertions(+), 2 deletions(-) delete mode 100644 python/qemu/machine/.flake8 diff --git a/python/qemu/machine/.flake8 b/python/qemu/machine/.flake8 deleted file mode 100644 index 45d8146f3f..0000000000 --- a/python/qemu/machine/.flake8 +++ /dev/null @@ -1,2 +0,0 @@ -[flake8] -extend-ignore = E722 # Pylint handles this, but smarter. \ No newline at end of file diff --git a/python/setup.cfg b/python/setup.cfg index 36b4253e93..52a89a0a29 100644 --- a/python/setup.cfg +++ b/python/setup.cfg @@ -22,6 +22,9 @@ packages = qemu.machine qemu.utils +[flake8] +extend-ignore = E722 # Prefer pylint's bare-except checks to flake8's + [pylint.messages control] # Disable the message, report, category or checker with the given id(s). You # can either give multiple identifiers separated by comma (,) or put this From 21d0b8667981e386cdfec18ad7d3eb4d9a33b088 Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:17:03 -0400 Subject: [PATCH 32/44] python: add excluded dirs to flake8 config Instruct flake8 to avoid certain well-known directories created by python tooling that it ought not check. Note that at-present, nothing actually creates a ".venv" directory; but it is in such widespread usage as a de-facto location for a developer's virtual environment that it should be excluded anyway. A forthcoming commit canonizes this with a "make venv" command. Signed-off-by: John Snow Reviewed-by: Cleber Rosa Message-id: 20210527211715.394144-20-jsnow@redhat.com Signed-off-by: John Snow --- python/setup.cfg | 2 ++ 1 file changed, 2 insertions(+) diff --git a/python/setup.cfg b/python/setup.cfg index 52a89a0a29..9aeab2bb0d 100644 --- a/python/setup.cfg +++ b/python/setup.cfg @@ -24,6 +24,8 @@ packages = [flake8] extend-ignore = E722 # Prefer pylint's bare-except checks to flake8's +exclude = __pycache__, + .venv, [pylint.messages control] # Disable the message, report, category or checker with the given id(s). You From 6d17d910434568626c1c739b7d3d8433618b19eb Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:17:04 -0400 Subject: [PATCH 33/44] python: Add flake8 to pipenv flake8 3.5.x does not support the --extend-ignore syntax used in the .flake8 file to gracefully extend default ignores, so 3.6.x is our minimum requirement. There is no known upper bound. flake8 can be run from the python/ directory with no arguments. Signed-off-by: John Snow Reviewed-by: Cleber Rosa Tested-by: Cleber Rosa Message-id: 20210527211715.394144-21-jsnow@redhat.com Signed-off-by: John Snow --- python/Pipfile | 1 + python/Pipfile.lock | 51 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/python/Pipfile b/python/Pipfile index 285e2c8e67..053f344dcb 100644 --- a/python/Pipfile +++ b/python/Pipfile @@ -4,6 +4,7 @@ url = "https://pypi.org/simple" verify_ssl = true [dev-packages] +flake8 = ">=3.6.0" pylint = ">=2.8.0" [packages] diff --git a/python/Pipfile.lock b/python/Pipfile.lock index c9debd0950..5c34019060 100644 --- a/python/Pipfile.lock +++ b/python/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "bd4fb76fcdd145bbf23c3a9dd7ad966113c5ce43ca51cc2d828aa7e73d572901" + "sha256": "3c842ab9c72c40d24d146349aa144e00e4dec1c358c812cfa96489411f5b3f87" }, "pipfile-spec": 6, "requires": { @@ -25,6 +25,22 @@ "markers": "python_version ~= '3.6'", "version": "==2.5.6" }, + "flake8": { + "hashes": [ + "sha256:07528381786f2a6237b061f6e96610a4167b226cb926e2aa2b6b1d78057c576b", + "sha256:bf8fd333346d844f616e8d47905ef3a3384edae6b4e9beb0c5101e25e3110907" + ], + "index": "pypi", + "version": "==3.9.2" + }, + "importlib-metadata": { + "hashes": [ + "sha256:8c501196e49fb9df5df43833bdb1e4328f64847763ec8a50703148b73784d581", + "sha256:d7eb1dea6d6a6086f8be21784cc9e3bcfa55872b52309bc5fad53a8ea444465d" + ], + "markers": "python_version < '3.8'", + "version": "==4.0.1" + }, "isort": { "hashes": [ "sha256:0a943902919f65c5684ac4e0154b1ad4fac6dcaa5d9f3426b732f1c8b5419be6", @@ -68,6 +84,22 @@ ], "version": "==0.6.1" }, + "pycodestyle": { + "hashes": [ + "sha256:514f76d918fcc0b55c6680472f0a37970994e07bbb80725808c17089be302068", + "sha256:c389c1d06bf7904078ca03399a4816f974a1d590090fecea0c63ec26ebaf1cef" + ], + "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3'", + "version": "==2.7.0" + }, + "pyflakes": { + "hashes": [ + "sha256:7893783d01b8a89811dd72d7dfd4d84ff098e5eed95cfa8905b22bbffe52efc3", + "sha256:f5bc8ecabc05bb9d291eb5203d6810b49040f6ff446a756326104746cc00c1db" + ], + "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3'", + "version": "==2.3.1" + }, "pylint": { "hashes": [ "sha256:586d8fa9b1891f4b725f587ef267abe2a1bad89d6b184520c7f07a253dd6e217", @@ -120,11 +152,28 @@ "markers": "implementation_name == 'cpython' and python_version < '3.8'", "version": "==1.4.3" }, + "typing-extensions": { + "hashes": [ + "sha256:0ac0f89795dd19de6b97debb0c6af1c70987fd80a2d62d1958f7e56fcc31b497", + "sha256:50b6f157849174217d0656f99dc82fe932884fb250826c18350e159ec6cdf342", + "sha256:779383f6086d90c99ae41cf0ff39aac8a7937a9283ce0a414e5dd782f4c94a84" + ], + "markers": "python_version < '3.8'", + "version": "==3.10.0.0" + }, "wrapt": { "hashes": [ "sha256:b62ffa81fb85f4332a4f609cab4ac40709470da05643a082ec1eb88e6d9b97d7" ], "version": "==1.12.1" + }, + "zipp": { + "hashes": [ + "sha256:3607921face881ba3e026887d8150cca609d517579abe052ac81fc5aeffdbd76", + "sha256:51cb66cc54621609dd593d1787f286ee42a5c0adbb4b29abea5a63edc3e03098" + ], + "markers": "python_version >= '3.6'", + "version": "==3.4.1" } } } From e941c844e4446b6200ac102ef285544c406a2fcd Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:17:05 -0400 Subject: [PATCH 34/44] python: move mypy.ini into setup.cfg mypy supports reading its configuration values from a central project configuration file; do so. Signed-off-by: John Snow Reviewed-by: Cleber Rosa Message-id: 20210527211715.394144-22-jsnow@redhat.com Signed-off-by: John Snow --- python/mypy.ini | 4 ---- python/setup.cfg | 5 +++++ 2 files changed, 5 insertions(+), 4 deletions(-) delete mode 100644 python/mypy.ini diff --git a/python/mypy.ini b/python/mypy.ini deleted file mode 100644 index 1a581c5f1e..0000000000 --- a/python/mypy.ini +++ /dev/null @@ -1,4 +0,0 @@ -[mypy] -strict = True -python_version = 3.6 -warn_unused_configs = True diff --git a/python/setup.cfg b/python/setup.cfg index 9aeab2bb0d..bd88b44ad8 100644 --- a/python/setup.cfg +++ b/python/setup.cfg @@ -27,6 +27,11 @@ extend-ignore = E722 # Prefer pylint's bare-except checks to flake8's exclude = __pycache__, .venv, +[mypy] +strict = True +python_version = 3.6 +warn_unused_configs = True + [pylint.messages control] # Disable the message, report, category or checker with the given id(s). You # can either give multiple identifiers separated by comma (,) or put this From 0542a4c95767b2370cb6622efe723bb6197aa04c Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:17:06 -0400 Subject: [PATCH 35/44] python: add mypy to pipenv 0.730 appears to be about the oldest version that works with the features we want, including nice human readable output (to make sure iotest 297 passes), and type-parameterized Popen generics. 0.770, however, supports adding 'strict' to the config file, so require at least 0.770. Now that we are checking a namespace package, we need to tell mypy to allow PEP420 namespaces, so modify the mypy config as part of the move. mypy can now be run from the python root by typing 'mypy -p qemu'. A note on mypy invocation: Running it as "mypy qemu/" changes the import path detection mechanisms in mypy slightly, and it will fail. See https://github.com/python/mypy/issues/8584 for a decent entry point with more breadcrumbs on the various behaviors that contribute to this subtle difference. Signed-off-by: John Snow Reviewed-by: Cleber Rosa Tested-by: Cleber Rosa Message-id: 20210527211715.394144-23-jsnow@redhat.com Signed-off-by: John Snow --- python/Pipfile | 1 + python/Pipfile.lock | 37 ++++++++++++++++++++++++++++++++++++- python/setup.cfg | 1 + 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/python/Pipfile b/python/Pipfile index 053f344dcb..796c6282e1 100644 --- a/python/Pipfile +++ b/python/Pipfile @@ -5,6 +5,7 @@ verify_ssl = true [dev-packages] flake8 = ">=3.6.0" +mypy = ">=0.770" pylint = ">=2.8.0" [packages] diff --git a/python/Pipfile.lock b/python/Pipfile.lock index 5c34019060..626e68403f 100644 --- a/python/Pipfile.lock +++ b/python/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "3c842ab9c72c40d24d146349aa144e00e4dec1c358c812cfa96489411f5b3f87" + "sha256": "14d171b3d86759e1fdfb9e55f66be4a696b6afa8f627d6c4778f8398c6a66b98" }, "pipfile-spec": 6, "requires": { @@ -84,6 +84,41 @@ ], "version": "==0.6.1" }, + "mypy": { + "hashes": [ + "sha256:0d0a87c0e7e3a9becdfbe936c981d32e5ee0ccda3e0f07e1ef2c3d1a817cf73e", + "sha256:25adde9b862f8f9aac9d2d11971f226bd4c8fbaa89fb76bdadb267ef22d10064", + "sha256:28fb5479c494b1bab244620685e2eb3c3f988d71fd5d64cc753195e8ed53df7c", + "sha256:2f9b3407c58347a452fc0736861593e105139b905cca7d097e413453a1d650b4", + "sha256:33f159443db0829d16f0a8d83d94df3109bb6dd801975fe86bacb9bf71628e97", + "sha256:3f2aca7f68580dc2508289c729bd49ee929a436208d2b2b6aab15745a70a57df", + "sha256:499c798053cdebcaa916eef8cd733e5584b5909f789de856b482cd7d069bdad8", + "sha256:4eec37370483331d13514c3f55f446fc5248d6373e7029a29ecb7b7494851e7a", + "sha256:552a815579aa1e995f39fd05dde6cd378e191b063f031f2acfe73ce9fb7f9e56", + "sha256:5873888fff1c7cf5b71efbe80e0e73153fe9212fafdf8e44adfe4c20ec9f82d7", + "sha256:61a3d5b97955422964be6b3baf05ff2ce7f26f52c85dd88db11d5e03e146a3a6", + "sha256:674e822aa665b9fd75130c6c5f5ed9564a38c6cea6a6432ce47eafb68ee578c5", + "sha256:7ce3175801d0ae5fdfa79b4f0cfed08807af4d075b402b7e294e6aa72af9aa2a", + "sha256:9743c91088d396c1a5a3c9978354b61b0382b4e3c440ce83cf77994a43e8c521", + "sha256:9f94aac67a2045ec719ffe6111df543bac7874cee01f41928f6969756e030564", + "sha256:a26f8ec704e5a7423c8824d425086705e381b4f1dfdef6e3a1edab7ba174ec49", + "sha256:abf7e0c3cf117c44d9285cc6128856106183938c68fd4944763003decdcfeb66", + "sha256:b09669bcda124e83708f34a94606e01b614fa71931d356c1f1a5297ba11f110a", + "sha256:cd07039aa5df222037005b08fbbfd69b3ab0b0bd7a07d7906de75ae52c4e3119", + "sha256:d23e0ea196702d918b60c8288561e722bf437d82cb7ef2edcd98cfa38905d506", + "sha256:d65cc1df038ef55a99e617431f0553cd77763869eebdf9042403e16089fe746c", + "sha256:d7da2e1d5f558c37d6e8c1246f1aec1e7349e4913d8fb3cb289a35de573fe2eb" + ], + "index": "pypi", + "version": "==0.812" + }, + "mypy-extensions": { + "hashes": [ + "sha256:090fedd75945a69ae91ce1303b5824f428daf5a028d2f6ab8a299250a846f15d", + "sha256:2d82818f5bb3e369420cb3c4060a7970edba416647068eb4c5343488a6c604a8" + ], + "version": "==0.4.3" + }, "pycodestyle": { "hashes": [ "sha256:514f76d918fcc0b55c6680472f0a37970994e07bbb80725808c17089be302068", diff --git a/python/setup.cfg b/python/setup.cfg index bd88b44ad8..b485d6161d 100644 --- a/python/setup.cfg +++ b/python/setup.cfg @@ -31,6 +31,7 @@ exclude = __pycache__, strict = True python_version = 3.6 warn_unused_configs = True +namespace_packages = True [pylint.messages control] # Disable the message, report, category or checker with the given id(s). You From 158ac451b9e1029798f8fdc103fef64830e4314e Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:17:07 -0400 Subject: [PATCH 36/44] python: move .isort.cfg into setup.cfg Signed-off-by: John Snow Reviewed-by: Cleber Rosa Message-id: 20210527211715.394144-24-jsnow@redhat.com Signed-off-by: John Snow --- python/.isort.cfg | 7 ------- python/setup.cfg | 8 ++++++++ 2 files changed, 8 insertions(+), 7 deletions(-) delete mode 100644 python/.isort.cfg diff --git a/python/.isort.cfg b/python/.isort.cfg deleted file mode 100644 index 6d0fd6cc0d..0000000000 --- a/python/.isort.cfg +++ /dev/null @@ -1,7 +0,0 @@ -[settings] -force_grid_wrap=4 -force_sort_within_sections=True -include_trailing_comma=True -line_length=72 -lines_after_imports=2 -multi_line_output=3 \ No newline at end of file diff --git a/python/setup.cfg b/python/setup.cfg index b485d6161d..3f07bd2752 100644 --- a/python/setup.cfg +++ b/python/setup.cfg @@ -61,3 +61,11 @@ good-names=i, [pylint.similarities] # Ignore imports when computing similarities. ignore-imports=yes + +[isort] +force_grid_wrap=4 +force_sort_within_sections=True +include_trailing_comma=True +line_length=72 +lines_after_imports=2 +multi_line_output=3 From 22a973cb1d365f6c506e190d26e2261a65066e15 Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:17:08 -0400 Subject: [PATCH 37/44] python/qemu: add isort to pipenv isort 5.0.0 through 5.0.4 has a bug that causes it to misinterpret certain "from ..." clauses that are not related to imports. isort < 5.1.1 has a bug where it does not handle comments near import statements correctly. Require 5.1.2 or greater. isort can be run (in "check" mode) with 'isort -c qemu' from the python root. isort can also be used to fix/rewrite import order automatically by using 'isort qemu'. Signed-off-by: John Snow Reviewed-by: Cleber Rosa Message-id: 20210527211715.394144-25-jsnow@redhat.com Signed-off-by: John Snow --- python/Pipfile | 1 + python/Pipfile.lock | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/python/Pipfile b/python/Pipfile index 796c6282e1..79c74dd8db 100644 --- a/python/Pipfile +++ b/python/Pipfile @@ -5,6 +5,7 @@ verify_ssl = true [dev-packages] flake8 = ">=3.6.0" +isort = ">=5.1.2" mypy = ">=0.770" pylint = ">=2.8.0" diff --git a/python/Pipfile.lock b/python/Pipfile.lock index 626e68403f..57a5befb10 100644 --- a/python/Pipfile.lock +++ b/python/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "14d171b3d86759e1fdfb9e55f66be4a696b6afa8f627d6c4778f8398c6a66b98" + "sha256": "8173290ad57aab0b722c9b4f109519de4e0dd7cd1bad1e16806b78bb169bce08" }, "pipfile-spec": 6, "requires": { @@ -46,7 +46,7 @@ "sha256:0a943902919f65c5684ac4e0154b1ad4fac6dcaa5d9f3426b732f1c8b5419be6", "sha256:2bb1680aad211e3c9944dbce1d4ba09a989f04e238296c87fe2139faa26d655d" ], - "markers": "python_version >= '3.6' and python_version < '4.0'", + "index": "pypi", "version": "==5.8.0" }, "lazy-object-proxy": { From a4dd49d40536b7ad70ab9c2e25a7810773ca32bc Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:17:09 -0400 Subject: [PATCH 38/44] python/qemu: add qemu package itself to pipenv This adds the python qemu packages themselves to the pipenv manifest. 'pipenv sync' will create a virtual environment sufficient to use the SDK. 'pipenv sync --dev' will create a virtual environment sufficient to use and test the SDK (with pylint, mypy, isort, flake8, etc.) The qemu packages are installed in 'editable' mode; all changes made to the python package inside the git tree will be reflected in the installed package without reinstallation. This includes changes made via git pull and so on. Signed-off-by: John Snow Reviewed-by: Cleber Rosa Tested-by: Cleber Rosa Message-id: 20210527211715.394144-26-jsnow@redhat.com Signed-off-by: John Snow --- python/Pipfile | 1 + python/Pipfile.lock | 9 +++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/python/Pipfile b/python/Pipfile index 79c74dd8db..dbe96f71c4 100644 --- a/python/Pipfile +++ b/python/Pipfile @@ -10,6 +10,7 @@ mypy = ">=0.770" pylint = ">=2.8.0" [packages] +qemu = {editable = true,path = "."} [requires] python_version = "3.6" diff --git a/python/Pipfile.lock b/python/Pipfile.lock index 57a5befb10..f0bf576c31 100644 --- a/python/Pipfile.lock +++ b/python/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "8173290ad57aab0b722c9b4f109519de4e0dd7cd1bad1e16806b78bb169bce08" + "sha256": "7c74cc4c2db3a75c954a6686411cda6fd60e464620bb6d5f1ed9a54be61db4cc" }, "pipfile-spec": 6, "requires": { @@ -15,7 +15,12 @@ } ] }, - "default": {}, + "default": { + "qemu": { + "editable": true, + "path": "." + } + }, "develop": { "astroid": { "hashes": [ From dbe75f55669a4e2295b0dae161b8f796e6dbaded Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:17:10 -0400 Subject: [PATCH 39/44] python: add devel package requirements to setuptools setuptools doesn't have a formal understanding of development requires, but it has an optional feataures section. Fine; add a "devel" feature and add the requirements to it. To avoid duplication, we can modify pipenv to install qemu[devel] instead. This enables us to run invocations like "pip install -e .[devel]" and test the package on bleeding-edge packages beyond those specified in Pipfile.lock. Importantly, this also allows us to install the qemu development packages in a non-networked mode: `pip3 install --no-index -e .[devel]` will now fail if the proper development dependencies are not already met. This can be useful for automated build scripts where fetching network packages may be undesirable. Signed-off-by: John Snow Reviewed-by: Cleber Rosa Message-id: 20210527211715.394144-27-jsnow@redhat.com Signed-off-by: John Snow --- python/PACKAGE.rst | 4 ++++ python/Pipfile | 5 +---- python/Pipfile.lock | 14 +++++++++----- python/README.rst | 4 ++++ python/setup.cfg | 9 +++++++++ 5 files changed, 27 insertions(+), 9 deletions(-) diff --git a/python/PACKAGE.rst b/python/PACKAGE.rst index 1bbfe1b58e..05ea7789fc 100644 --- a/python/PACKAGE.rst +++ b/python/PACKAGE.rst @@ -31,3 +31,7 @@ official `GitLab mirror `_. Please report bugs on the `QEMU issue tracker `_ and tag ``@jsnow`` in the report. + +Optional packages necessary for running code quality analysis for this +package can be installed with the optional dependency group "devel": +``pip install qemu[devel]``. diff --git a/python/Pipfile b/python/Pipfile index dbe96f71c4..e7acb8cefa 100644 --- a/python/Pipfile +++ b/python/Pipfile @@ -4,10 +4,7 @@ url = "https://pypi.org/simple" verify_ssl = true [dev-packages] -flake8 = ">=3.6.0" -isort = ">=5.1.2" -mypy = ">=0.770" -pylint = ">=2.8.0" +qemu = {editable = true, extras = ["devel"], path = "."} [packages] qemu = {editable = true,path = "."} diff --git a/python/Pipfile.lock b/python/Pipfile.lock index f0bf576c31..a2cdc1c50e 100644 --- a/python/Pipfile.lock +++ b/python/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "7c74cc4c2db3a75c954a6686411cda6fd60e464620bb6d5f1ed9a54be61db4cc" + "sha256": "eff562a688ebc6f3ffe67494dbb804b883e2159ad81c4d55d96da9f7aec13e91" }, "pipfile-spec": 6, "requires": { @@ -35,7 +35,7 @@ "sha256:07528381786f2a6237b061f6e96610a4167b226cb926e2aa2b6b1d78057c576b", "sha256:bf8fd333346d844f616e8d47905ef3a3384edae6b4e9beb0c5101e25e3110907" ], - "index": "pypi", + "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3, 3.4'", "version": "==3.9.2" }, "importlib-metadata": { @@ -51,7 +51,7 @@ "sha256:0a943902919f65c5684ac4e0154b1ad4fac6dcaa5d9f3426b732f1c8b5419be6", "sha256:2bb1680aad211e3c9944dbce1d4ba09a989f04e238296c87fe2139faa26d655d" ], - "index": "pypi", + "markers": "python_version >= '3.6' and python_version < '4.0'", "version": "==5.8.0" }, "lazy-object-proxy": { @@ -114,7 +114,7 @@ "sha256:d65cc1df038ef55a99e617431f0553cd77763869eebdf9042403e16089fe746c", "sha256:d7da2e1d5f558c37d6e8c1246f1aec1e7349e4913d8fb3cb289a35de573fe2eb" ], - "index": "pypi", + "markers": "python_version >= '3.5'", "version": "==0.812" }, "mypy-extensions": { @@ -145,9 +145,13 @@ "sha256:586d8fa9b1891f4b725f587ef267abe2a1bad89d6b184520c7f07a253dd6e217", "sha256:f7e2072654a6b6afdf5e2fb38147d3e2d2d43c89f648637baab63e026481279b" ], - "index": "pypi", + "markers": "python_version ~= '3.6'", "version": "==2.8.2" }, + "qemu": { + "editable": true, + "path": "." + }, "toml": { "hashes": [ "sha256:806143ae5bfb6a3c6e736a764057db0e6a0e05e338b5630894a5f779cabb4f9b", diff --git a/python/README.rst b/python/README.rst index bf9bbca979..954870973d 100644 --- a/python/README.rst +++ b/python/README.rst @@ -24,6 +24,10 @@ which installs a version of the package that installs a forwarder pointing to these files, such that the package always reflects the latest version in your git tree. +Installing ".[devel]" instead of "." will additionally pull in required +packages for testing this package. They are not runtime requirements, +and are not needed to simply use these libraries. + See `Installing packages using pip and virtual environments `_ for more information. diff --git a/python/setup.cfg b/python/setup.cfg index 3f07bd2752..39dc135e60 100644 --- a/python/setup.cfg +++ b/python/setup.cfg @@ -22,6 +22,15 @@ packages = qemu.machine qemu.utils +[options.extras_require] +# Run `pipenv lock --dev` when changing these requirements. +devel = + flake8 >= 3.6.0 + isort >= 5.1.2 + mypy >= 0.770 + pylint >= 2.8.0 + + [flake8] extend-ignore = E722 # Prefer pylint's bare-except checks to flake8's exclude = __pycache__, From 31622b2a8ac769b3cef730d3a24ed209e3861cbc Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:17:11 -0400 Subject: [PATCH 40/44] python: add avocado-framework and tests Try using avocado to manage our various tests; even though right now they're only invoking shell scripts and not really running any python-native code. Create tests/, and add shell scripts which call out to mypy, flake8, pylint and isort to enforce the standards in this directory. Add avocado-framework to the setup.cfg development dependencies, and add avocado.cfg to store some preferences for how we'd like the test output to look. Finally, add avocado-framework to the Pipfile environment and lock the new dependencies. We are using avocado >= 87.0 here to take advantage of some features that Cleber has helpfully added to make the test output here *very* friendly and easy to read for developers that might chance upon the output in Gitlab CI. [Note: ALL of the dependencies get updated to the most modern versions that exist at the time of this writing. No way around it that I have seen. Not ideal, but so it goes.] Provided you have the right development dependencies (mypy, flake8, isort, pylint, and now avocado-framework) You should be able to run "avocado --config avocado.cfg run tests/" from the python folder to run all of these linters with the correct arguments. (A forthcoming commit adds the much easier 'make check'.) Signed-off-by: John Snow Reviewed-by: Cleber Rosa Tested-by: Cleber Rosa Message-id: 20210527211715.394144-28-jsnow@redhat.com Signed-off-by: John Snow --- python/Pipfile.lock | 8 ++++++++ python/README.rst | 2 ++ python/avocado.cfg | 10 ++++++++++ python/setup.cfg | 1 + python/tests/flake8.sh | 2 ++ python/tests/isort.sh | 2 ++ python/tests/mypy.sh | 2 ++ python/tests/pylint.sh | 2 ++ 8 files changed, 29 insertions(+) create mode 100644 python/avocado.cfg create mode 100755 python/tests/flake8.sh create mode 100755 python/tests/isort.sh create mode 100755 python/tests/mypy.sh create mode 100755 python/tests/pylint.sh diff --git a/python/Pipfile.lock b/python/Pipfile.lock index a2cdc1c50e..6e344f5fad 100644 --- a/python/Pipfile.lock +++ b/python/Pipfile.lock @@ -30,6 +30,14 @@ "markers": "python_version ~= '3.6'", "version": "==2.5.6" }, + "avocado-framework": { + "hashes": [ + "sha256:42aa7962df98d6b78d4efd9afa2177226dc630f3d83a2a7d5baf7a0a7da7fa1b", + "sha256:d96ae343abf890e1ef3b3a6af5ce49e35f6bded0715770c4acb325bca555c515" + ], + "markers": "python_version >= '3.6'", + "version": "==88.1" + }, "flake8": { "hashes": [ "sha256:07528381786f2a6237b061f6e96610a4167b226cb926e2aa2b6b1d78057c576b", diff --git a/python/README.rst b/python/README.rst index 954870973d..6bd2c6b354 100644 --- a/python/README.rst +++ b/python/README.rst @@ -37,6 +37,8 @@ Files in this directory ----------------------- - ``qemu/`` Python package source directory. +- ``tests/`` Python package tests directory. +- ``avocado.cfg`` Configuration for the Avocado test-runner. - ``MANIFEST.in`` is read by python setuptools, it specifies additional files that should be included by a source distribution. - ``PACKAGE.rst`` is used as the README file that is visible on PyPI.org. diff --git a/python/avocado.cfg b/python/avocado.cfg new file mode 100644 index 0000000000..10dc6fb605 --- /dev/null +++ b/python/avocado.cfg @@ -0,0 +1,10 @@ +[simpletests] +# Don't show stdout/stderr in the test *summary* +status.failure_fields = ['status'] + +[job] +# Don't show the full debug.log output; only select stdout/stderr. +output.testlogs.logfiles = ['stdout', 'stderr'] + +# Show full stdout/stderr only on tests that FAIL +output.testlogs.statuses = ['FAIL'] diff --git a/python/setup.cfg b/python/setup.cfg index 39dc135e60..fd32519490 100644 --- a/python/setup.cfg +++ b/python/setup.cfg @@ -25,6 +25,7 @@ packages = [options.extras_require] # Run `pipenv lock --dev` when changing these requirements. devel = + avocado-framework >= 87.0 flake8 >= 3.6.0 isort >= 5.1.2 mypy >= 0.770 diff --git a/python/tests/flake8.sh b/python/tests/flake8.sh new file mode 100755 index 0000000000..51e0788462 --- /dev/null +++ b/python/tests/flake8.sh @@ -0,0 +1,2 @@ +#!/bin/sh -e +python3 -m flake8 diff --git a/python/tests/isort.sh b/python/tests/isort.sh new file mode 100755 index 0000000000..4480405bfb --- /dev/null +++ b/python/tests/isort.sh @@ -0,0 +1,2 @@ +#!/bin/sh -e +python3 -m isort -c qemu/ diff --git a/python/tests/mypy.sh b/python/tests/mypy.sh new file mode 100755 index 0000000000..5f980f563b --- /dev/null +++ b/python/tests/mypy.sh @@ -0,0 +1,2 @@ +#!/bin/sh -e +python3 -m mypy -p qemu diff --git a/python/tests/pylint.sh b/python/tests/pylint.sh new file mode 100755 index 0000000000..4b10b34db7 --- /dev/null +++ b/python/tests/pylint.sh @@ -0,0 +1,2 @@ +#!/bin/sh -e +python3 -m pylint qemu/ From 6560379facf40e66fd8fbf4578f3d28f510167d8 Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:17:12 -0400 Subject: [PATCH 41/44] python: add Makefile for some common tasks Add "make venv" to create the pipenv-managed virtual environment that contains our explicitly pinned dependencies. Add "make check" to run the python linters [in the host execution environment]. Add "make venv-check" which combines the above two: create/update the venv, then run the linters in that explicitly managed environment. Add "make develop" which canonizes the runes needed to get both the linting pre-requisites (the "[devel]" part), and the editable live-install (the "-e" part) of these python libraries. make clean: delete miscellaneous python packaging output possibly created by pipenv, pip, or other python packaging utilities make distclean: delete the above, the .venv, and the editable "qemu" package forwarder (qemu.egg-info) if there is one. Signed-off-by: John Snow Reviewed-by: Cleber Rosa Tested-by: Cleber Rosa Message-id: 20210527211715.394144-29-jsnow@redhat.com Signed-off-by: John Snow --- python/Makefile | 43 +++++++++++++++++++++++++++++++++++++++++++ python/PACKAGE.rst | 6 ++++++ python/README.rst | 6 ++++++ 3 files changed, 55 insertions(+) create mode 100644 python/Makefile diff --git a/python/Makefile b/python/Makefile new file mode 100644 index 0000000000..a9da168955 --- /dev/null +++ b/python/Makefile @@ -0,0 +1,43 @@ +.PHONY: help venv venv-check check clean distclean develop + +help: + @echo "python packaging help:" + @echo "" + @echo "make venv: Create pipenv's virtual environment." + @echo " NOTE: Requires Python 3.6 and pipenv." + @echo " Will download packages from PyPI." + @echo " Hint: (On Fedora): 'sudo dnf install python36 pipenv'" + @echo "" + @echo "make venv-check: run linters using pipenv's virtual environment." + @echo " Hint: If you don't know which test to run, run this one!" + @echo "" + @echo "make develop: Install deps for 'make check', and" + @echo " the qemu libs in editable/development mode." + @echo "" + @echo "make check: run linters using the current environment." + @echo "" + @echo "make clean: remove package build output." + @echo "" + @echo "make distclean: remove venv files, qemu package forwarder," + @echo " built distribution files, and everything" + @echo " from 'make clean'." + +venv: .venv +.venv: Pipfile.lock + @PIPENV_VENV_IN_PROJECT=1 pipenv sync --dev --keep-outdated + @touch .venv + +venv-check: venv + @pipenv run make check + +develop: + pip3 install -e .[devel] + +check: + @avocado --config avocado.cfg run tests/ + +clean: + python3 setup.py clean --all + +distclean: clean + rm -rf qemu.egg-info/ .venv/ dist/ diff --git a/python/PACKAGE.rst b/python/PACKAGE.rst index 05ea7789fc..b0b86cc4c3 100644 --- a/python/PACKAGE.rst +++ b/python/PACKAGE.rst @@ -35,3 +35,9 @@ the report. Optional packages necessary for running code quality analysis for this package can be installed with the optional dependency group "devel": ``pip install qemu[devel]``. + +``make develop`` can be used to install this package in editable mode +(to the current environment) *and* bring in testing dependencies in one +command. + +``make check`` can be used to run the available tests. diff --git a/python/README.rst b/python/README.rst index 6bd2c6b354..dcf993819d 100644 --- a/python/README.rst +++ b/python/README.rst @@ -28,6 +28,9 @@ Installing ".[devel]" instead of "." will additionally pull in required packages for testing this package. They are not runtime requirements, and are not needed to simply use these libraries. +Running ``make develop`` will pull in all testing dependencies and +install QEMU in editable mode to the current environment. + See `Installing packages using pip and virtual environments `_ for more information. @@ -39,6 +42,9 @@ Files in this directory - ``qemu/`` Python package source directory. - ``tests/`` Python package tests directory. - ``avocado.cfg`` Configuration for the Avocado test-runner. + Used by ``make check`` et al. +- ``Makefile`` provides some common testing/installation invocations. + Try ``make help`` to see available targets. - ``MANIFEST.in`` is read by python setuptools, it specifies additional files that should be included by a source distribution. - ``PACKAGE.rst`` is used as the README file that is visible on PyPI.org. From f9c0600f0200528921c43ccb8a8a44c81825a343 Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:17:13 -0400 Subject: [PATCH 42/44] python: add .gitignore Ignore *Python* build and package output (build, dist, qemu.egg-info); these files are not created as part of a QEMU build. They are created by running the commands 'python3 setup.py ' when preparing tarballs to upload to e.g. PyPI. Ignore miscellaneous cached python confetti (mypy, pylint, et al) Ignore .idea (pycharm) .vscode, and .venv (pipenv et al). Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Cleber Rosa Message-id: 20210527211715.394144-30-jsnow@redhat.com Signed-off-by: John Snow --- python/.gitignore | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 python/.gitignore diff --git a/python/.gitignore b/python/.gitignore new file mode 100644 index 0000000000..4ed144ceac --- /dev/null +++ b/python/.gitignore @@ -0,0 +1,15 @@ +# linter/tooling cache +.mypy_cache/ +.cache/ + +# python packaging +build/ +dist/ +qemu.egg-info/ + +# editor config +.idea/ +.vscode/ + +# virtual environments (pipenv et al) +.venv/ From 3c8de38c8515a300b7842d95893b9e95caaa0ad6 Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:17:14 -0400 Subject: [PATCH 43/44] python: add tox support This is intended to be a manually run, non-CI script. Use tox to test the linters against all python versions from 3.6 to 3.10. This will only work if you actually have those versions installed locally, but Fedora makes this easy: > sudo dnf install python3.6 python3.7 python3.8 python3.9 python3.10 Unlike the pipenv tests (make venv-check), this pulls "whichever" versions of the python packages, so they are unpinned and may break as time goes on. In the case that breakages are found, setup.cfg should be amended accordingly to avoid the bad dependant versions, or the code should be amended to work around the issue. With confidence that the tests pass on 3.6 through 3.10 inclusive, add the appropriate classifiers to setup.cfg to indicate which versions we claim to support. Tox 3.18.0 or above is required to use the 'allowlist_externals' option. Signed-off-by: John Snow Reviewed-by: Cleber Rosa Tested-by: Cleber Rosa Message-id: 20210527211715.394144-31-jsnow@redhat.com Signed-off-by: John Snow --- python/.gitignore | 1 + python/Makefile | 7 ++++++- python/setup.cfg | 23 ++++++++++++++++++++++- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/python/.gitignore b/python/.gitignore index 4ed144ceac..272ed223a8 100644 --- a/python/.gitignore +++ b/python/.gitignore @@ -13,3 +13,4 @@ qemu.egg-info/ # virtual environments (pipenv et al) .venv/ +.tox/ diff --git a/python/Makefile b/python/Makefile index a9da168955..b5621b0d54 100644 --- a/python/Makefile +++ b/python/Makefile @@ -16,6 +16,8 @@ help: @echo "" @echo "make check: run linters using the current environment." @echo "" + @echo "make check-tox: run linters using multiple python versions." + @echo "" @echo "make clean: remove package build output." @echo "" @echo "make distclean: remove venv files, qemu package forwarder," @@ -36,8 +38,11 @@ develop: check: @avocado --config avocado.cfg run tests/ +check-tox: + @tox + clean: python3 setup.py clean --all distclean: clean - rm -rf qemu.egg-info/ .venv/ dist/ + rm -rf qemu.egg-info/ .venv/ .tox/ dist/ diff --git a/python/setup.cfg b/python/setup.cfg index fd32519490..0fcdec6f32 100644 --- a/python/setup.cfg +++ b/python/setup.cfg @@ -14,6 +14,11 @@ classifiers = Natural Language :: English Operating System :: OS Independent Programming Language :: Python :: 3 :: Only + Programming Language :: Python :: 3.6 + Programming Language :: Python :: 3.7 + Programming Language :: Python :: 3.8 + Programming Language :: Python :: 3.9 + Programming Language :: Python :: 3.10 [options] python_requires = >= 3.6 @@ -30,12 +35,13 @@ devel = isort >= 5.1.2 mypy >= 0.770 pylint >= 2.8.0 - + tox >= 3.18.0 [flake8] extend-ignore = E722 # Prefer pylint's bare-except checks to flake8's exclude = __pycache__, .venv, + .tox, [mypy] strict = True @@ -79,3 +85,18 @@ include_trailing_comma=True line_length=72 lines_after_imports=2 multi_line_output=3 + +# tox (https://tox.readthedocs.io/) is a tool for running tests in +# multiple virtualenvs. This configuration file will run the test suite +# on all supported python versions. To use it, "pip install tox" and +# then run "tox" from this directory. You will need all of these versions +# of python available on your system to run this test. + +[tox:tox] +envlist = py36, py37, py38, py39, py310 + +[testenv] +allowlist_externals = make +deps = .[devel] +commands = + make check From 6b9c277797879ce41ed20deb6737f4156cc279b3 Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 May 2021 17:17:15 -0400 Subject: [PATCH 44/44] gitlab: add python linters to CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a Python container that has just enough juice for us to run the Python code quality analysis tools. Base this container on Fedora, because Fedora has very convenient packaging for testing multiple Python versions. We need python3, pip (for pulling packages), pipenv and virtualenv for creating virtual environments, and tox for running tests. make is needed for running 'make check-tox' and 'make venv-check' targets. Python3.10 is needed explicitly because the tox package only pulls in 3.6-3.9, but we wish to test the forthcoming release of Python as well to help predict any problems. Lastly, we need gcc to compile PyPI packages that may not have a binary distribution available. Add two tests: check-python-pipenv uses pipenv to test a frozen, very explicit set of packages against our minimum supported python version, Python 3.6. This test is not allowed to fail. The dependencies this test uses do not change unless python/Pipfile.lock is changed. check-python-tox uses tox to install the latest versions of required python dependencies against a wide array of Python versions from 3.6 to 3.9, even including the yet-to-be-released Python 3.10. This test is allowed to fail with a warning. Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alex Bennée Reviewed-by: Cleber Rosa Message-id: 20210527211715.394144-32-jsnow@redhat.com [Fix rebase conflict over .gitlab-ci.yml --js] Signed-off-by: John Snow --- .gitlab-ci.d/containers.yml | 5 +++++ .gitlab-ci.d/static_checks.yml | 21 +++++++++++++++++++++ tests/docker/dockerfiles/python.docker | 18 ++++++++++++++++++ 3 files changed, 44 insertions(+) create mode 100644 tests/docker/dockerfiles/python.docker diff --git a/.gitlab-ci.d/containers.yml b/.gitlab-ci.d/containers.yml index bd01ae8f80..1ca455f8e1 100644 --- a/.gitlab-ci.d/containers.yml +++ b/.gitlab-ci.d/containers.yml @@ -43,3 +43,8 @@ amd64-opensuse-leap-container: extends: .container_job_template variables: NAME: opensuse-leap + +python-container: + extends: .container_job_template + variables: + NAME: python diff --git a/.gitlab-ci.d/static_checks.yml b/.gitlab-ci.d/static_checks.yml index 91247a6f67..8e30872164 100644 --- a/.gitlab-ci.d/static_checks.yml +++ b/.gitlab-ci.d/static_checks.yml @@ -24,3 +24,24 @@ check-dco: - if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH' when: never - when: on_success + +check-python-pipenv: + stage: test + image: $CI_REGISTRY_IMAGE/qemu/python:latest + script: + - make -C python venv-check + variables: + GIT_DEPTH: 1 + needs: + job: python-container + +check-python-tox: + stage: test + image: $CI_REGISTRY_IMAGE/qemu/python:latest + script: + - make -C python check-tox + variables: + GIT_DEPTH: 1 + needs: + job: python-container + allow_failure: true diff --git a/tests/docker/dockerfiles/python.docker b/tests/docker/dockerfiles/python.docker new file mode 100644 index 0000000000..56d88417df --- /dev/null +++ b/tests/docker/dockerfiles/python.docker @@ -0,0 +1,18 @@ +# Python library testing environment + +FROM fedora:latest +MAINTAINER John Snow + +# Please keep this list sorted alphabetically +ENV PACKAGES \ + gcc \ + make \ + pipenv \ + python3 \ + python3-pip \ + python3-tox \ + python3-virtualenv \ + python3.10 + +RUN dnf install -y $PACKAGES +RUN rpm -q $PACKAGES | sort > /packages.txt