This introduces a base class for tests that need to interact with a Linux guest. It generalizes the "boot_linux.py" code, already been used by the "virtiofs_submounts.py" and also SSH related code being used by that and "linux_ssh_mips_malta.py". While at it, a number of fixes on hopeful improvements to those tests were added. Changes from v1: * Majority of v1 patches have been merged. * New patches: - Acceptance Tests: make username/password configurable - Acceptance Tests: set up SSH connection by default after boot for LinuxTest - tests/acceptance/virtiofs_submounts.py: remove launch_vm() * Allowed for the configuration of the network device type (defaulting to virtio-net) [Phil] * Fix module name typo (s/qemu.util/qemu.utils/) in the commit message [John] * Tests based on LinuxTest will have the SSH connection already prepared Cleber Rosa (10): tests/acceptance/virtiofs_submounts.py: add missing accel tag tests/acceptance/virtiofs_submounts.py: evaluate string not length Python: add utility function for retrieving port redirection Acceptance Tests: move useful ssh methods to base class Acceptance Tests: add port redirection for ssh by default Acceptance Tests: make username/password configurable Acceptance Tests: set up SSH connection by default after boot for LinuxTest tests/acceptance/virtiofs_submounts.py: remove launch_vm() Acceptance Tests: add basic documentation on LinuxTest base class Acceptance Tests: introduce CPU hotplug test docs/devel/testing.rst | 25 ++++++++ python/qemu/utils.py | 35 ++++++++++++ tests/acceptance/avocado_qemu/__init__.py | 63 +++++++++++++++++++-- tests/acceptance/hotplug_cpu.py | 37 ++++++++++++ tests/acceptance/info_usernet.py | 29 ++++++++++ tests/acceptance/linux_ssh_mips_malta.py | 44 ++------------- tests/acceptance/virtiofs_submounts.py | 69 +++-------------------- tests/vm/basevm.py | 7 +-- 8 files changed, 198 insertions(+), 111 deletions(-) create mode 100644 python/qemu/utils.py create mode 100644 tests/acceptance/hotplug_cpu.py create mode 100644 tests/acceptance/info_usernet.py -- 2.25.4
The tag is useful to select tests that depend/use a particular feature. Signed-off-by: Cleber Rosa <crosa@redhat.com> Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com> Reviewed-by: Willian Rampazzo <willianr@redhat.com> --- 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 test_something_that_needs_cmd1_and_cmd2(self): class VirtiofsSubmountsTest(LinuxTest): """ :avocado: tags=arch:x86_64 + :avocado: tags=accel:kvm """ def get_portfwd(self): -- 2.25.4
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 <crosa@redhat.com> Reviewed-by: Beraldo Leal <bleal@redhat.com> --- 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 @@ def setUp(self): super(VirtiofsSubmountsTest, self).setUp(pubkey) - if len(vmlinuz) > 0: + if vmlinuz: self.vm.add_args('-kernel', vmlinuz, '-append', 'console=ttyS0 root=/dev/sda1') -- 2.25.4
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 <crosa@redhat.com> Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com> --- python/qemu/utils.py | 35 ++++++++++++++++++++++++ tests/acceptance/info_usernet.py | 29 ++++++++++++++++++++ tests/acceptance/linux_ssh_mips_malta.py | 16 +++++------ tests/acceptance/virtiofs_submounts.py | 21 ++++---------- tests/vm/basevm.py | 7 ++--- 5 files changed, 78 insertions(+), 30 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..89a246ab30 --- /dev/null +++ b/python/qemu/utils.py @@ -0,0 +1,35 @@ +""" +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 <crosa@redhat.com> +# +# 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" + :param info_usernet_output: str + :return: the port number allocated by the hostfwd option + :rtype: int + """ + 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 <crosa@redhat.com> +# +# 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 archive from avocado.utils import ssh +from qemu.utils import get_info_usernet_hostfwd_port + class LinuxSSH(Test): @@ -70,18 +72,14 @@ def get_kernel_info(self, endianess, wordsize): def setUp(self): super(LinuxSSH, self).setUp() - def get_portfwd(self): + def ssh_connect(self, username, password): + self.ssh_logger = logging.getLogger('ssh') 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] + 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) - return port - - def ssh_connect(self, username, password): - self.ssh_logger = logging.getLogger('ssh') - port = self.get_portfwd() 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 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..75ce07df36 100644 --- a/tests/vm/basevm.py +++ b/tests/vm/basevm.py @@ -21,6 +21,7 @@ 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 @@ -306,11 +307,7 @@ def boot(self, img, extra_args=[]): 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] + 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) -- 2.25.4
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 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 <crosa@redhat.com> Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com> Reviewed-by: Willian Rampazzo <willianr@redhat.com> --- tests/acceptance/avocado_qemu/__init__.py | 48 ++++++++++++++++++++++- tests/acceptance/linux_ssh_mips_malta.py | 38 ++---------------- tests/acceptance/virtiofs_submounts.py | 37 ----------------- 3 files changed, 50 insertions(+), 73 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 @@ 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 @@ 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 @@ def fetch_asset(self, name, 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..3f590a081f 100644 --- a/tests/acceptance/linux_ssh_mips_malta.py +++ b/tests/acceptance/linux_ssh_mips_malta.py @@ -12,7 +12,7 @@ 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 @@ -21,7 +21,7 @@ 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 +72,9 @@ def get_kernel_info(self, endianess, wordsize): 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 +95,7 @@ def boot_debian_wheezy_image_and_ssh_login(self, endianess, kernel_path): 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 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) -- 2.25.4
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. Signed-off-by: Cleber Rosa <crosa@redhat.com> --- tests/acceptance/avocado_qemu/__init__.py | 4 +++- tests/acceptance/virtiofs_submounts.py | 4 ---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py index 67f75f66e5..e75b002c70 100644 --- a/tests/acceptance/avocado_qemu/__init__.py +++ b/tests/acceptance/avocado_qemu/__init__.py @@ -309,10 +309,12 @@ 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') + 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 @@ def setUp(self): 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') -- 2.25.4
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 <crosa@redhat.com> --- 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 e75b002c70..535f63a48d 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() @@ -370,8 +372,8 @@ def prepare_cloudinit(self, ssh_pubkey=None): 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, -- 2.25.4
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 <crosa@redhat.com> --- tests/acceptance/avocado_qemu/__init__.py | 5 ++++- tests/acceptance/virtiofs_submounts.py | 1 - 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py index 535f63a48d..4960142bcc 100644 --- a/tests/acceptance/avocado_qemu/__init__.py +++ b/tests/acceptance/avocado_qemu/__init__.py @@ -390,7 +390,7 @@ def set_up_cloudinit(self, ssh_pubkey=None): 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(), @@ -398,3 +398,6 @@ def launch_and_wait(self): 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/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 @@ def set_up_virtiofs(self): 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') -- 2.25.4
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 <crosa@redhat.com> --- 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 @@ def set_up_virtiofs(self): '-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 @@ def test_pre_virtiofsd_set_up(self): 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 @@ def test_pre_launch_set_up(self): 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 @@ def test_post_launch_set_up(self): 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 @@ def test_two_runs(self): self.set_up_nested_mounts() self.set_up_virtiofs() - self.launch_vm() + self.launch_and_wait() self.mount_in_guest() self.check_in_guest() -- 2.25.4
Signed-off-by: Cleber Rosa <crosa@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Willian Rampazzo <willianr@redhat.com> --- docs/devel/testing.rst | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst index 1da4c4e4c4..ed2a06db28 100644 --- a/docs/devel/testing.rst +++ b/docs/devel/testing.rst @@ -810,6 +810,31 @@ 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. 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 ~~~~~~~~~~~ -- 2.25.4
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 <crosa@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Willian Rampazzo <willianr@redhat.com> --- 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 <crosa@redhat.com> +# +# 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') -- 2.25.4
[-- Attachment #1: Type: text/plain, Size: 2269 bytes --] Hi On Wed, Mar 24, 2021 at 2:23 AM Cleber Rosa <crosa@redhat.com> wrote: > 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. > > Signed-off-by: Cleber Rosa <crosa@redhat.com> > --- > tests/acceptance/avocado_qemu/__init__.py | 4 +++- > tests/acceptance/virtiofs_submounts.py | 4 ---- > 2 files changed, 3 insertions(+), 5 deletions(-) > > diff --git a/tests/acceptance/avocado_qemu/__init__.py > b/tests/acceptance/avocado_qemu/__init__.py > index 67f75f66e5..e75b002c70 100644 > --- a/tests/acceptance/avocado_qemu/__init__.py > +++ b/tests/acceptance/avocado_qemu/__init__.py > @@ -309,10 +309,12 @@ 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') > + 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 @@ def setUp(self): > 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') > > -- > 2.25.4 > > Looks fine, I suppose it could eventually be in LinuxSSHMixIn too for other users. Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> -- Marc-André Lureau [-- Attachment #2: Type: text/html, Size: 3145 bytes --]
[-- Attachment #1: Type: text/plain, Size: 1753 bytes --] On Wed, Mar 24, 2021 at 2:21 AM Cleber Rosa <crosa@redhat.com> wrote: > 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 <crosa@redhat.com> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > 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 e75b002c70..535f63a48d 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() > @@ -370,8 +372,8 @@ def prepare_cloudinit(self, ssh_pubkey=None): > 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, > -- > 2.25.4 > > > -- Marc-André Lureau [-- Attachment #2: Type: text/html, Size: 2748 bytes --]
[-- Attachment #1: Type: text/plain, Size: 2254 bytes --] On Wed, Mar 24, 2021 at 2:34 AM Cleber Rosa <crosa@redhat.com> wrote: > 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 <crosa@redhat.com> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > tests/acceptance/avocado_qemu/__init__.py | 5 ++++- > tests/acceptance/virtiofs_submounts.py | 1 - > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/tests/acceptance/avocado_qemu/__init__.py > b/tests/acceptance/avocado_qemu/__init__.py > index 535f63a48d..4960142bcc 100644 > --- a/tests/acceptance/avocado_qemu/__init__.py > +++ b/tests/acceptance/avocado_qemu/__init__.py > @@ -390,7 +390,7 @@ def set_up_cloudinit(self, ssh_pubkey=None): > 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(), > @@ -398,3 +398,6 @@ def launch_and_wait(self): > 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/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 @@ def set_up_virtiofs(self): > > 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') > -- > 2.25.4 > > > -- Marc-André Lureau [-- Attachment #2: Type: text/html, Size: 3341 bytes --]
[-- Attachment #1: Type: text/plain, Size: 2419 bytes --] On Wed, Mar 24, 2021 at 2:32 AM Cleber Rosa <crosa@redhat.com> wrote: > 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 <crosa@redhat.com> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > 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 @@ def set_up_virtiofs(self): > '-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 @@ def test_pre_virtiofsd_set_up(self): > 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 @@ def test_pre_launch_set_up(self): > > 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 @@ def test_post_launch_set_up(self): > 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 @@ def test_two_runs(self): > self.set_up_nested_mounts() > > self.set_up_virtiofs() > - self.launch_vm() > + self.launch_and_wait() > self.mount_in_guest() > self.check_in_guest() > > -- > 2.25.4 > > > -- Marc-André Lureau [-- Attachment #2: Type: text/html, Size: 3469 bytes --]
Hi Cleber, On 3/23/21 11:15 PM, Cleber Rosa wrote: > 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 s/than/then > arguments will not be set.> > This keeps the same logic, but improves readability a bit. > > Signed-off-by: Cleber Rosa <crosa@redhat.com> > Reviewed-by: Beraldo Leal <bleal@redhat.com> > --- > 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 @@ def setUp(self): > > super(VirtiofsSubmountsTest, self).setUp(pubkey) > > - if len(vmlinuz) > 0: > + if vmlinuz: > self.vm.add_args('-kernel', vmlinuz, > '-append', 'console=ttyS0 root=/dev/sda1') > > Reviewed-by: Eric Auger <eric.auger@redhat.com> Thanks Eric
Hi Cleber,
On 3/23/21 11:15 PM, Cleber Rosa wrote:
> The tag is useful to select tests that depend/use a particular
> feature.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> Reviewed-by: Willian Rampazzo <willianr@redhat.com>
> ---
> 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 test_something_that_needs_cmd1_and_cmd2(self):
> class VirtiofsSubmountsTest(LinuxTest):
> """
> :avocado: tags=arch:x86_64
> + :avocado: tags=accel:kvm
> """
>
> def get_portfwd(self):
>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Thanks
Eric
Hi Cleber,
On 3/23/21 11:15 PM, Cleber Rosa wrote:
> 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 <crosa@redhat.com>
> Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> ---
> python/qemu/utils.py | 35 ++++++++++++++++++++++++
> tests/acceptance/info_usernet.py | 29 ++++++++++++++++++++
> tests/acceptance/linux_ssh_mips_malta.py | 16 +++++------
> tests/acceptance/virtiofs_submounts.py | 21 ++++----------
> tests/vm/basevm.py | 7 ++---
> 5 files changed, 78 insertions(+), 30 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..89a246ab30
> --- /dev/null
> +++ b/python/qemu/utils.py
> @@ -0,0 +1,35 @@
> +"""
> +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 <crosa@redhat.com>
> +#
> +# 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"
> + :param info_usernet_output: str
> + :return: the port number allocated by the hostfwd option
> + :rtype: int
> + """
> + 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 <crosa@redhat.com>
> +#
> +# 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 archive
> from avocado.utils import ssh
>
> +from qemu.utils import get_info_usernet_hostfwd_port
> +
>
> class LinuxSSH(Test):
>
> @@ -70,18 +72,14 @@ def get_kernel_info(self, endianess, wordsize):
> def setUp(self):
> super(LinuxSSH, self).setUp()
>
> - def get_portfwd(self):
> + def ssh_connect(self, username, password):
> + self.ssh_logger = logging.getLogger('ssh')
> 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]
> + 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)
> - return port
> -
> - def ssh_connect(self, username, password):
> - self.ssh_logger = logging.getLogger('ssh')
> - port = self.get_portfwd()
> 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 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..75ce07df36 100644
> --- a/tests/vm/basevm.py
> +++ b/tests/vm/basevm.py
> @@ -21,6 +21,7 @@
> 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
> @@ -306,11 +307,7 @@ def boot(self, img, extra_args=[]):
> 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]
> + 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)
>
Looks good to me
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Thanks
Eric
Hi Cleber, On 3/23/21 11:15 PM, Cleber Rosa wrote: > 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 the base LinuxTest nit: strictly speaking they are moved to another class which is inherited by LinuxTest, right? > 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 <crosa@redhat.com> > Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com> > Reviewed-by: Willian Rampazzo <willianr@redhat.com> > --- > tests/acceptance/avocado_qemu/__init__.py | 48 ++++++++++++++++++++++- > tests/acceptance/linux_ssh_mips_malta.py | 38 ++---------------- > tests/acceptance/virtiofs_submounts.py | 37 ----------------- > 3 files changed, 50 insertions(+), 73 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 @@ > 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 @@ > 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 @@ def fetch_asset(self, name, > 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..3f590a081f 100644 > --- a/tests/acceptance/linux_ssh_mips_malta.py > +++ b/tests/acceptance/linux_ssh_mips_malta.py > @@ -12,7 +12,7 @@ > 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 > @@ -21,7 +21,7 @@ > from qemu.utils import get_info_usernet_hostfwd_port Can't you remove this now? > > > -class LinuxSSH(Test): > +class LinuxSSH(Test, LinuxSSHMixIn): out of curiosity why can't it be migrated to a LinuxTest? > > timeout = 150 # Not for 'configure --enable-debug --enable-debug-tcg' > > @@ -72,41 +72,9 @@ def get_kernel_info(self, endianess, wordsize): > 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 +95,7 @@ def boot_debian_wheezy_image_and_ssh_login(self, endianess, kernel_path): > 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 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) > > Besides, Reviewed-by: Eric Auger <eric.auger@redhat.com> Thanks Eric
Hi Cleber, On 3/23/21 11:15 PM, Cleber Rosa wrote: > 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 also sets the network device to virtio-net. This may be worth mentioning here in the commit msg. > arguments. > > Signed-off-by: Cleber Rosa <crosa@redhat.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Thanks Eric > --- > tests/acceptance/avocado_qemu/__init__.py | 4 +++- > tests/acceptance/virtiofs_submounts.py | 4 ---- > 2 files changed, 3 insertions(+), 5 deletions(-) > > diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py > index 67f75f66e5..e75b002c70 100644 > --- a/tests/acceptance/avocado_qemu/__init__.py > +++ b/tests/acceptance/avocado_qemu/__init__.py > @@ -309,10 +309,12 @@ 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') > + 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 @@ def setUp(self): > 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') > >
On 3/23/21 11:15 PM, Cleber Rosa wrote: > 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 <crosa@redhat.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Eric > --- > 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 e75b002c70..535f63a48d 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() > @@ -370,8 +372,8 @@ def prepare_cloudinit(self, ssh_pubkey=None): > 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, >
Hi Cleber,
On 3/23/21 11:15 PM, Cleber Rosa wrote:
> 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 <crosa@redhat.com>
> ---
> tests/acceptance/avocado_qemu/__init__.py | 5 ++++-
> tests/acceptance/virtiofs_submounts.py | 1 -
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> index 535f63a48d..4960142bcc 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -390,7 +390,7 @@ def set_up_cloudinit(self, ssh_pubkey=None):
> 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(),
> @@ -398,3 +398,6 @@ def launch_and_wait(self):
> 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/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 @@ def set_up_virtiofs(self):
>
> 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')
>
what about launch_and_wait calls in boot_linux.py. Don't you want to
force ssh connection off there?
Thanks
Eric
Hi, On 3/23/21 11:15 PM, Cleber Rosa wrote: > 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 <crosa@redhat.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Eric > --- > 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 @@ def set_up_virtiofs(self): > '-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 @@ def test_pre_virtiofsd_set_up(self): > 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 @@ def test_pre_launch_set_up(self): > > 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 @@ def test_post_launch_set_up(self): > 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 @@ def test_two_runs(self): > self.set_up_nested_mounts() > > self.set_up_virtiofs() > - self.launch_vm() > + self.launch_and_wait() > self.mount_in_guest() > self.check_in_guest() > >
Hi, On 3/23/21 11:15 PM, Cleber Rosa wrote: > Signed-off-by: Cleber Rosa <crosa@redhat.com> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > Reviewed-by: Willian Rampazzo <willianr@redhat.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Eric > --- > docs/devel/testing.rst | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst > index 1da4c4e4c4..ed2a06db28 100644 > --- a/docs/devel/testing.rst > +++ b/docs/devel/testing.rst > @@ -810,6 +810,31 @@ 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. 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 > ~~~~~~~~~~~ > >
Hi Cleber, On 3/23/21 11:15 PM, Cleber Rosa wrote: > 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 <crosa@redhat.com> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > Reviewed-by: Willian Rampazzo <willianr@redhat.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Eric > --- > 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 <crosa@redhat.com> > +# > +# 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') >
Hi Cleber, On 3/23/21 11:15 PM, Cleber Rosa wrote: > 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. > > Signed-off-by: Cleber Rosa <crosa@redhat.com> > --- > tests/acceptance/avocado_qemu/__init__.py | 4 +++- > tests/acceptance/virtiofs_submounts.py | 4 ---- > 2 files changed, 3 insertions(+), 5 deletions(-) > > diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py > index 67f75f66e5..e75b002c70 100644 > --- a/tests/acceptance/avocado_qemu/__init__.py > +++ b/tests/acceptance/avocado_qemu/__init__.py > @@ -309,10 +309,12 @@ 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'): I would be interested in testing with HW bridging too, when a bridge is available. Do you think we could have the netdev configurable too? This would be helpful to test vhost for instance. With respect the network device type, I am currently working on SMMU test and I need to call the parent setUp-) with the following args now: super(IOMMU, self).setUp(pubkey, 'virtio-net-pci,iommu_platform=on,disable-modern=off,disable-legacy=on') It works but I am not sure you had such kind of scenario in mind? Thanks Eric > super(LinuxTest, self).setUp() > self.vm.add_args('-smp', '2') > self.vm.add_args('-m', '1024') > + 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 @@ def setUp(self): > 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') > >
On Tue, Mar 23, 2021 at 7:15 PM Cleber Rosa <crosa@redhat.com> wrote:
>
> 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 <crosa@redhat.com>
> Reviewed-by: Beraldo Leal <bleal@redhat.com>
> ---
> tests/acceptance/virtiofs_submounts.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Reviewed-by: Willian Rampazzo <willianr@redhat.com>
On Tue, Mar 23, 2021 at 7:16 PM Cleber Rosa <crosa@redhat.com> wrote: > > 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 <crosa@redhat.com> > Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com> > --- > python/qemu/utils.py | 35 ++++++++++++++++++++++++ > tests/acceptance/info_usernet.py | 29 ++++++++++++++++++++ > tests/acceptance/linux_ssh_mips_malta.py | 16 +++++------ > tests/acceptance/virtiofs_submounts.py | 21 ++++---------- > tests/vm/basevm.py | 7 ++--- > 5 files changed, 78 insertions(+), 30 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..89a246ab30 > --- /dev/null > +++ b/python/qemu/utils.py > @@ -0,0 +1,35 @@ > +""" > +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 <crosa@redhat.com> > +# > +# 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" > + :param info_usernet_output: str > + :return: the port number allocated by the hostfwd option > + :rtype: int > + """ > + 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 <crosa@redhat.com> > +# > +# 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') This pattern is repeated every time a test needs to call get_info_usernet_hostfwd_port. Do you see any downside on moving this line inside the function and passing self.vm as an argument? This would abstract the need to run info usernet before calling the function. > + 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 archive > from avocado.utils import ssh > > +from qemu.utils import get_info_usernet_hostfwd_port > + > > class LinuxSSH(Test): > > @@ -70,18 +72,14 @@ def get_kernel_info(self, endianess, wordsize): > def setUp(self): > super(LinuxSSH, self).setUp() > > - def get_portfwd(self): > + def ssh_connect(self, username, password): > + self.ssh_logger = logging.getLogger('ssh') > 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] > + 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) > - return port > - > - def ssh_connect(self, username, password): > - self.ssh_logger = logging.getLogger('ssh') > - port = self.get_portfwd() > 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 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..75ce07df36 100644 > --- a/tests/vm/basevm.py > +++ b/tests/vm/basevm.py > @@ -21,6 +21,7 @@ > 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 > @@ -306,11 +307,7 @@ def boot(self, img, extra_args=[]): > 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] > + 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) > -- > 2.25.4 > Other than maybe a small adjustment to the get_info_usernet_hostfwd_port function: Reviewed-by: Willian Rampazzo <willianr@redhat.com>
On Tue, Mar 23, 2021 at 7:16 PM Cleber Rosa <crosa@redhat.com> wrote:
>
> 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.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
> tests/acceptance/avocado_qemu/__init__.py | 4 +++-
> tests/acceptance/virtiofs_submounts.py | 4 ----
> 2 files changed, 3 insertions(+), 5 deletions(-)
>
Reviewed-by: Willian Rampazzo <willianr@redhat.com>
On Tue, Mar 23, 2021 at 7:16 PM Cleber Rosa <crosa@redhat.com> wrote:
>
> 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 <crosa@redhat.com>
> ---
> tests/acceptance/avocado_qemu/__init__.py | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
Reviewed-by: Willian Rampazzo <willianr@redhat.com>
On Tue, Mar 23, 2021 at 7:16 PM Cleber Rosa <crosa@redhat.com> wrote:
>
> 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 <crosa@redhat.com>
> ---
> tests/acceptance/avocado_qemu/__init__.py | 5 ++++-
> tests/acceptance/virtiofs_submounts.py | 1 -
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
Reviewed-by: Willian Rampazzo <willianr@redhat.com>
On Tue, Mar 23, 2021 at 7:16 PM Cleber Rosa <crosa@redhat.com> wrote:
>
> 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 <crosa@redhat.com>
> ---
> tests/acceptance/virtiofs_submounts.py | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
Reviewed-by: Willian Rampazzo <willianr@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 9535 bytes --] On Wed, Mar 24, 2021 at 05:35:07PM -0300, Willian Rampazzo wrote: > On Tue, Mar 23, 2021 at 7:16 PM Cleber Rosa <crosa@redhat.com> wrote: > > > > 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 <crosa@redhat.com> > > Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com> > > --- > > python/qemu/utils.py | 35 ++++++++++++++++++++++++ > > tests/acceptance/info_usernet.py | 29 ++++++++++++++++++++ > > tests/acceptance/linux_ssh_mips_malta.py | 16 +++++------ > > tests/acceptance/virtiofs_submounts.py | 21 ++++---------- > > tests/vm/basevm.py | 7 ++--- > > 5 files changed, 78 insertions(+), 30 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..89a246ab30 > > --- /dev/null > > +++ b/python/qemu/utils.py > > @@ -0,0 +1,35 @@ > > +""" > > +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 <crosa@redhat.com> > > +# > > +# 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" > > + :param info_usernet_output: str > > + :return: the port number allocated by the hostfwd option > > + :rtype: int > > + """ > > + 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 <crosa@redhat.com> > > +# > > +# 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') > > This pattern is repeated every time a test needs to call > get_info_usernet_hostfwd_port. Do you see any downside on moving this > line inside the function and passing self.vm as an argument? This > would abstract the need to run info usernet before calling the > function. > My original plan was to follow this up with a utility in QEMUMachine itself (because that is the vm). It can still be done, but: * I don't expect *tests* to be calling this often, rather a base class for tests; * I'm avoiding changing QEMUMachine too much, given it's a more generic class and used by other tests (iotests, vm, etc). Hopefully when we have a better laid out structure for adding tests to QEMUMachine itself, it'll be confortable to change it more drastically. > > + 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 archive > > from avocado.utils import ssh > > > > +from qemu.utils import get_info_usernet_hostfwd_port > > + > > > > class LinuxSSH(Test): > > > > @@ -70,18 +72,14 @@ def get_kernel_info(self, endianess, wordsize): > > def setUp(self): > > super(LinuxSSH, self).setUp() > > > > - def get_portfwd(self): > > + def ssh_connect(self, username, password): > > + self.ssh_logger = logging.getLogger('ssh') > > 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] > > + 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) > > - return port > > - > > - def ssh_connect(self, username, password): > > - self.ssh_logger = logging.getLogger('ssh') > > - port = self.get_portfwd() > > 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 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..75ce07df36 100644 > > --- a/tests/vm/basevm.py > > +++ b/tests/vm/basevm.py > > @@ -21,6 +21,7 @@ > > 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 > > @@ -306,11 +307,7 @@ def boot(self, img, extra_args=[]): > > 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] > > + 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) > > -- > > 2.25.4 > > > > Other than maybe a small adjustment to the > get_info_usernet_hostfwd_port function: > > Reviewed-by: Willian Rampazzo <willianr@redhat.com> > Thanks for the review! - Cleber. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #1: Type: text/plain, Size: 10367 bytes --] On Wed, Mar 24, 2021 at 10:07:31AM +0100, Auger Eric wrote: > Hi Cleber, > > On 3/23/21 11:15 PM, Cleber Rosa wrote: > > 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 the base LinuxTest > nit: strictly speaking they are moved to another class which is > inherited by LinuxTest, right? > > 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 <crosa@redhat.com> > > Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com> > > Reviewed-by: Willian Rampazzo <willianr@redhat.com> > > --- > > tests/acceptance/avocado_qemu/__init__.py | 48 ++++++++++++++++++++++- > > tests/acceptance/linux_ssh_mips_malta.py | 38 ++---------------- > > tests/acceptance/virtiofs_submounts.py | 37 ----------------- > > 3 files changed, 50 insertions(+), 73 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 @@ > > 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 @@ > > 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 @@ def fetch_asset(self, name, > > 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..3f590a081f 100644 > > --- a/tests/acceptance/linux_ssh_mips_malta.py > > +++ b/tests/acceptance/linux_ssh_mips_malta.py > > @@ -12,7 +12,7 @@ > > 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 > > @@ -21,7 +21,7 @@ > > from qemu.utils import get_info_usernet_hostfwd_port > Can't you remove this now? > > > > Yes, good catch! > > -class LinuxSSH(Test): > > +class LinuxSSH(Test, LinuxSSHMixIn): > out of curiosity why can't it be migrated to a LinuxTest? LinuxTest (currently) assumes that it'll boot via an image obtained with avocado.utils.vmimage, and configured with avocado.utils.cloudinit. Those are not the case for this test. I believe there are still some opportunities for advancing them towards each other, but LinuxTest needs to become more versatile. > > > > timeout = 150 # Not for 'configure --enable-debug --enable-debug-tcg' > > > > @@ -72,41 +72,9 @@ def get_kernel_info(self, endianess, wordsize): > > 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 +95,7 @@ def boot_debian_wheezy_image_and_ssh_login(self, endianess, kernel_path): > > 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 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) > > > > > > Besides, > > Reviewed-by: Eric Auger <eric.auger@redhat.com> > > Thanks > > Eric Thanks for the review! - Cleber. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #1: Type: text/plain, Size: 587 bytes --] On Wed, Mar 24, 2021 at 10:10:50AM +0100, Auger Eric wrote: > Hi Cleber, > > On 3/23/21 11:15 PM, Cleber Rosa wrote: > > 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 > also sets the network device to virtio-net. This may be worth mentioning > here in the commit msg. Absolutely, I've added that note. > > arguments. > > > > Signed-off-by: Cleber Rosa <crosa@redhat.com> > Reviewed-by: Eric Auger <eric.auger@redhat.com> > > Thanks > > Eric > Thank you! - Cleber [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #1: Type: text/plain, Size: 2552 bytes --] On Wed, Mar 24, 2021 at 12:30:18PM +0400, Marc-André Lureau wrote: > Hi > > On Wed, Mar 24, 2021 at 2:23 AM Cleber Rosa <crosa@redhat.com> wrote: > > > 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. > > > > Signed-off-by: Cleber Rosa <crosa@redhat.com> > > --- > > tests/acceptance/avocado_qemu/__init__.py | 4 +++- > > tests/acceptance/virtiofs_submounts.py | 4 ---- > > 2 files changed, 3 insertions(+), 5 deletions(-) > > > > diff --git a/tests/acceptance/avocado_qemu/__init__.py > > b/tests/acceptance/avocado_qemu/__init__.py > > index 67f75f66e5..e75b002c70 100644 > > --- a/tests/acceptance/avocado_qemu/__init__.py > > +++ b/tests/acceptance/avocado_qemu/__init__.py > > @@ -309,10 +309,12 @@ 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') > > + 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 @@ def setUp(self): > > 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') > > > > -- > > 2.25.4 > > > > > Looks fine, I suppose it could eventually be in LinuxSSHMixIn too for other > users. > That's a good point, should be possible. I'll look into that. > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > > -- > Marc-André Lureau Thanks, - Cleber. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #1: Type: text/plain, Size: 2297 bytes --] On Wed, Mar 24, 2021 at 11:36:53AM +0100, Auger Eric wrote: > Hi Cleber, > On 3/23/21 11:15 PM, Cleber Rosa wrote: > > 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. > > > > Signed-off-by: Cleber Rosa <crosa@redhat.com> > > --- > > tests/acceptance/avocado_qemu/__init__.py | 4 +++- > > tests/acceptance/virtiofs_submounts.py | 4 ---- > > 2 files changed, 3 insertions(+), 5 deletions(-) > > > > diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py > > index 67f75f66e5..e75b002c70 100644 > > --- a/tests/acceptance/avocado_qemu/__init__.py > > +++ b/tests/acceptance/avocado_qemu/__init__.py > > @@ -309,10 +309,12 @@ 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'): > I would be interested in testing with HW bridging too, when a bridge is > available. Do you think we could have the netdev configurable too? > This would be helpful to test vhost for instance. > Right, I knew from the start that the user mode network would only go so far. TBH, I think it went too further than I expected. But, requiring, or supporting, other network modes can add quite a bit of complexity, depending on how much you want the framework to do. Anyway, this is a valid point/request. For the lack of a better place, and given that this may be a larger effort, I'm tracking it here: https://gitlab.com/cleber.gnu/qemu/-/issues/3 > With respect the network device type, I am currently working on SMMU > test and I need to call the parent setUp-) with the following args now: > > super(IOMMU, self).setUp(pubkey, > 'virtio-net-pci,iommu_platform=on,disable-modern=off,disable-legacy=on') > > It works but I am not sure you had such kind of scenario in mind? > I see where you're coming from, and I share the slight feeling of abusing the feature... but I think it's OK at this point. I mean, I believe it's better to focus on say, the HW bridging support as this at least works. > Thanks > > Eric > Cheers, - Cleber. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #1: Type: text/plain, Size: 2674 bytes --] On Wed, Mar 24, 2021 at 10:22:47AM +0100, Auger Eric wrote: > Hi Cleber, > > On 3/23/21 11:15 PM, Cleber Rosa wrote: > > 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 <crosa@redhat.com> > > --- > > tests/acceptance/avocado_qemu/__init__.py | 5 ++++- > > tests/acceptance/virtiofs_submounts.py | 1 - > > 2 files changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py > > index 535f63a48d..4960142bcc 100644 > > --- a/tests/acceptance/avocado_qemu/__init__.py > > +++ b/tests/acceptance/avocado_qemu/__init__.py > > @@ -390,7 +390,7 @@ def set_up_cloudinit(self, ssh_pubkey=None): > > 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(), > > @@ -398,3 +398,6 @@ def launch_and_wait(self): > > 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/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 @@ def set_up_virtiofs(self): > > > > 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') > > > what about launch_and_wait calls in boot_linux.py. Don't you want to > force ssh connection off there? > Good point. I guess one could argue that it doesn't hurt those tests, and even that it "tests more". But, I'd argue that less is more here indeed. I'll change those launch_and_wait() to include set_up_ssh_connection=False for those tests. > Thanks > > Eric Thanks a lot! - Cleber. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --]
Hi Cleber, On 3/24/21 11:23 PM, Cleber Rosa wrote: > On Wed, Mar 24, 2021 at 10:07:31AM +0100, Auger Eric wrote: >> Hi Cleber, >> >> On 3/23/21 11:15 PM, Cleber Rosa wrote: >>> 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 the base LinuxTest >> nit: strictly speaking they are moved to another class which is >> inherited by LinuxTest, right? >>> 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 <crosa@redhat.com> >>> Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com> >>> Reviewed-by: Willian Rampazzo <willianr@redhat.com> >>> --- >>> tests/acceptance/avocado_qemu/__init__.py | 48 ++++++++++++++++++++++- >>> tests/acceptance/linux_ssh_mips_malta.py | 38 ++---------------- >>> tests/acceptance/virtiofs_submounts.py | 37 ----------------- >>> 3 files changed, 50 insertions(+), 73 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 @@ >>> 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 @@ >>> 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 @@ def fetch_asset(self, name, >>> 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..3f590a081f 100644 >>> --- a/tests/acceptance/linux_ssh_mips_malta.py >>> +++ b/tests/acceptance/linux_ssh_mips_malta.py >>> @@ -12,7 +12,7 @@ >>> 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 >>> @@ -21,7 +21,7 @@ >>> from qemu.utils import get_info_usernet_hostfwd_port >> Can't you remove this now? >>> >>> > > Yes, good catch! > >>> -class LinuxSSH(Test): >>> +class LinuxSSH(Test, LinuxSSHMixIn): >> out of curiosity why can't it be migrated to a LinuxTest? > > LinuxTest (currently) assumes that it'll boot via an image obtained > with avocado.utils.vmimage, and configured with > avocado.utils.cloudinit. Those are not the case for this test. I > believe there are still some opportunities for advancing them towards > each other, but LinuxTest needs to become more versatile. OK makes sense. By the way would it be possible to launch other types of cloud-init images? I see that LinuxTest download_boot() only uses a fedora 31 image. I would be interested in being able to run other types of images. Could we make it configurable? I can work on this if it makes sense. Thanks Eric > >>> >>> timeout = 150 # Not for 'configure --enable-debug --enable-debug-tcg' >>> >>> @@ -72,41 +72,9 @@ def get_kernel_info(self, endianess, wordsize): >>> 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 +95,7 @@ def boot_debian_wheezy_image_and_ssh_login(self, endianess, kernel_path): >>> 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 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) >>> >>> >> >> Besides, >> >> Reviewed-by: Eric Auger <eric.auger@redhat.com> >> >> Thanks >> >> Eric > > Thanks for the review! > > - Cleber. >
Hi, On 3/23/21 7:15 PM, Cleber Rosa wrote: > 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 <crosa@redhat.com> > --- > tests/acceptance/avocado_qemu/__init__.py | 5 ++++- > tests/acceptance/virtiofs_submounts.py | 1 - > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py > index 535f63a48d..4960142bcc 100644 > --- a/tests/acceptance/avocado_qemu/__init__.py > +++ b/tests/acceptance/avocado_qemu/__init__.py > @@ -390,7 +390,7 @@ def set_up_cloudinit(self, ssh_pubkey=None): > 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(), > @@ -398,3 +398,6 @@ def launch_and_wait(self): > 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) Where is self.username set? - Wainer > 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 @@ def set_up_virtiofs(self): > > 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')
On 3/25/21 11:31 AM, Wainer dos Santos Moschetta wrote: > Hi, > > On 3/23/21 7:15 PM, Cleber Rosa wrote: >> 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 <crosa@redhat.com> >> --- >> tests/acceptance/avocado_qemu/__init__.py | 5 ++++- >> tests/acceptance/virtiofs_submounts.py | 1 - >> 2 files changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/tests/acceptance/avocado_qemu/__init__.py >> b/tests/acceptance/avocado_qemu/__init__.py >> index 535f63a48d..4960142bcc 100644 >> --- a/tests/acceptance/avocado_qemu/__init__.py >> +++ b/tests/acceptance/avocado_qemu/__init__.py >> @@ -390,7 +390,7 @@ def set_up_cloudinit(self, ssh_pubkey=None): >> 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(), >> @@ -398,3 +398,6 @@ def launch_and_wait(self): >> 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) > > Where is self.username set? Never mind, I missed patch 06. > > > - Wainer > >> 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 @@ def set_up_virtiofs(self): >> 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') > >
Hi, On 3/25/21 9:50 AM, Auger Eric wrote: > Hi Cleber, > > On 3/24/21 11:23 PM, Cleber Rosa wrote: >> On Wed, Mar 24, 2021 at 10:07:31AM +0100, Auger Eric wrote: >>> Hi Cleber, >>> >>> On 3/23/21 11:15 PM, Cleber Rosa wrote: >>>> 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 the base LinuxTest >>> nit: strictly speaking they are moved to another class which is >>> inherited by LinuxTest, right? >>>> 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 <crosa@redhat.com> >>>> Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com> >>>> Reviewed-by: Willian Rampazzo <willianr@redhat.com> >>>> --- >>>> tests/acceptance/avocado_qemu/__init__.py | 48 ++++++++++++++++++++++- >>>> tests/acceptance/linux_ssh_mips_malta.py | 38 ++---------------- >>>> tests/acceptance/virtiofs_submounts.py | 37 ----------------- >>>> 3 files changed, 50 insertions(+), 73 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 @@ >>>> 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 @@ >>>> 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 @@ def fetch_asset(self, name, >>>> 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..3f590a081f 100644 >>>> --- a/tests/acceptance/linux_ssh_mips_malta.py >>>> +++ b/tests/acceptance/linux_ssh_mips_malta.py >>>> @@ -12,7 +12,7 @@ >>>> 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 >>>> @@ -21,7 +21,7 @@ >>>> from qemu.utils import get_info_usernet_hostfwd_port >>> Can't you remove this now? >>>> >>>> >> Yes, good catch! >> >>>> -class LinuxSSH(Test): >>>> +class LinuxSSH(Test, LinuxSSHMixIn): >>> out of curiosity why can't it be migrated to a LinuxTest? >> LinuxTest (currently) assumes that it'll boot via an image obtained >> with avocado.utils.vmimage, and configured with >> avocado.utils.cloudinit. Those are not the case for this test. I >> believe there are still some opportunities for advancing them towards >> each other, but LinuxTest needs to become more versatile. > OK makes sense. > > By the way would it be possible to launch other types of cloud-init > images? I see that LinuxTest download_boot() only uses a fedora 31 > image. I would be interested in being able to run other types of images. > Could we make it configurable? I can work on this if it makes sense. It is a good idea! Thanks! - Wainer > > Thanks > > Eric >>>> >>>> timeout = 150 # Not for 'configure --enable-debug --enable-debug-tcg' >>>> >>>> @@ -72,41 +72,9 @@ def get_kernel_info(self, endianess, wordsize): >>>> 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 +95,7 @@ def boot_debian_wheezy_image_and_ssh_login(self, endianess, kernel_path): >>>> 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 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) >>>> >>>> >>> Besides, >>> >>> Reviewed-by: Eric Auger <eric.auger@redhat.com> >>> >>> Thanks >>> >>> Eric >> Thanks for the review! >> >> - Cleber. >>
Hi, On 3/24/21 6:10 AM, Auger Eric wrote: > Hi Cleber, > > On 3/23/21 11:15 PM, Cleber Rosa wrote: >> 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 > also sets the network device to virtio-net. This may be worth mentioning > here in the commit msg. >> arguments. >> >> Signed-off-by: Cleber Rosa <crosa@redhat.com> > Reviewed-by: Eric Auger <eric.auger@redhat.com> > > Thanks > > Eric > >> --- >> tests/acceptance/avocado_qemu/__init__.py | 4 +++- >> tests/acceptance/virtiofs_submounts.py | 4 ---- >> 2 files changed, 3 insertions(+), 5 deletions(-) >> >> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py >> index 67f75f66e5..e75b002c70 100644 >> --- a/tests/acceptance/avocado_qemu/__init__.py >> +++ b/tests/acceptance/avocado_qemu/__init__.py >> @@ -309,10 +309,12 @@ 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') >> + 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 @@ def setUp(self): >> self.vm.add_args('-kernel', vmlinuz, >> '-append', 'console=ttyS0 root=/dev/sda1') >> >> - # Allow us to connect to SSH Somewhat related with Eric's suggestion: keep the above comment along with the netdev setup code. - Wainer >> - 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') >> >> >
On 3/23/21 6:15 PM, Cleber Rosa wrote: > 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 <crosa@redhat.com> > Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com> > --- > python/qemu/utils.py | 35 ++++++++++++++++++++++++ > tests/acceptance/info_usernet.py | 29 ++++++++++++++++++++ > tests/acceptance/linux_ssh_mips_malta.py | 16 +++++------ > tests/acceptance/virtiofs_submounts.py | 21 ++++---------- > tests/vm/basevm.py | 7 ++--- > 5 files changed, 78 insertions(+), 30 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..89a246ab30 > --- /dev/null > +++ b/python/qemu/utils.py > @@ -0,0 +1,35 @@ > +""" > +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 <crosa@redhat.com> > +# > +# 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" > + :param info_usernet_output: str > + :return: the port number allocated by the hostfwd option > + :rtype: int I think, unless you know something I don't, that I would prefer to keep type information in the "live" annotations where they can be checked against rot. > + """ > + 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 I wonder if more guest-specific code doesn't belong elsewhere, but I don't have a strong counter-suggestion, so I would probably ACK this for now. (Are you okay with the idea that we won't include the utils module in the PyPI upload? I think I would like to avoid shipping something like this outside of our castle walls, but agree that having it in the common code area somewhere for our own use is good.) > 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 <crosa@redhat.com> > +# > +# 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 archive > from avocado.utils import ssh > > +from qemu.utils import get_info_usernet_hostfwd_port > + > > class LinuxSSH(Test): > > @@ -70,18 +72,14 @@ def get_kernel_info(self, endianess, wordsize): > def setUp(self): > super(LinuxSSH, self).setUp() > > - def get_portfwd(self): > + def ssh_connect(self, username, password): > + self.ssh_logger = logging.getLogger('ssh') > 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] > + 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) > - return port > - > - def ssh_connect(self, username, password): > - self.ssh_logger = logging.getLogger('ssh') > - port = self.get_portfwd() > 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 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..75ce07df36 100644 > --- a/tests/vm/basevm.py > +++ b/tests/vm/basevm.py > @@ -21,6 +21,7 @@ > 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 > @@ -306,11 +307,7 @@ def boot(self, img, extra_args=[]): > 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] > + 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) >
Hi, On 3/23/21 7:15 PM, Cleber Rosa wrote: > Signed-off-by: Cleber Rosa <crosa@redhat.com> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > Reviewed-by: Willian Rampazzo <willianr@redhat.com> > --- > docs/devel/testing.rst | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst > index 1da4c4e4c4..ed2a06db28 100644 > --- a/docs/devel/testing.rst > +++ b/docs/devel/testing.rst > @@ -810,6 +810,31 @@ 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. The most > +basic example looks like this: I think it is worth mentioning currently it will boot a Fedora 31 cloud-init image. Apart from that, Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com> > + > +.. 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 > ~~~~~~~~~~~ >
Hi, On 3/23/21 7:15 PM, Cleber Rosa wrote: > This introduces a base class for tests that need to interact with a > Linux guest. It generalizes the "boot_linux.py" code, already been > used by the "virtiofs_submounts.py" and also SSH related code being > used by that and "linux_ssh_mips_malta.py". I ran the linux_ssh_mips_malta.py tests, they all passed: (11/34) tests/acceptance/linux_ssh_mips_malta.py:LinuxSSH.test_mips_malta32eb_kernel3_2_0: PASS (64.41 s) (12/34) tests/acceptance/linux_ssh_mips_malta.py:LinuxSSH.test_mips_malta32el_kernel3_2_0: PASS (63.43 s) (13/34) tests/acceptance/linux_ssh_mips_malta.py:LinuxSSH.test_mips_malta64eb_kernel3_2_0: PASS (63.76 s) (14/34) tests/acceptance/linux_ssh_mips_malta.py:LinuxSSH.test_mips_malta64el_kernel3_2_0: PASS (62.52 s) Then I tried the virtiofs_submounts.py tests, it finishes with error. Something like that fixes it: 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 @@ def setUp(self): 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) > > While at it, a number of fixes on hopeful improvements to those tests > were added. > > Changes from v1: > > * Majority of v1 patches have been merged. > > * New patches: > - Acceptance Tests: make username/password configurable > - Acceptance Tests: set up SSH connection by default after boot for LinuxTest > - tests/acceptance/virtiofs_submounts.py: remove launch_vm() > > * Allowed for the configuration of the network device type (defaulting > to virtio-net) [Phil] > > * Fix module name typo (s/qemu.util/qemu.utils/) in the commit message > [John] > > * Tests based on LinuxTest will have the SSH connection already prepared > > Cleber Rosa (10): > tests/acceptance/virtiofs_submounts.py: add missing accel tag > tests/acceptance/virtiofs_submounts.py: evaluate string not length > Python: add utility function for retrieving port redirection > Acceptance Tests: move useful ssh methods to base class > Acceptance Tests: add port redirection for ssh by default > Acceptance Tests: make username/password configurable > Acceptance Tests: set up SSH connection by default after boot for > LinuxTest > tests/acceptance/virtiofs_submounts.py: remove launch_vm() > Acceptance Tests: add basic documentation on LinuxTest base class > Acceptance Tests: introduce CPU hotplug test > > docs/devel/testing.rst | 25 ++++++++ > python/qemu/utils.py | 35 ++++++++++++ > tests/acceptance/avocado_qemu/__init__.py | 63 +++++++++++++++++++-- > tests/acceptance/hotplug_cpu.py | 37 ++++++++++++ > tests/acceptance/info_usernet.py | 29 ++++++++++ > tests/acceptance/linux_ssh_mips_malta.py | 44 ++------------- > tests/acceptance/virtiofs_submounts.py | 69 +++-------------------- > tests/vm/basevm.py | 7 +-- > 8 files changed, 198 insertions(+), 111 deletions(-) > create mode 100644 python/qemu/utils.py > create mode 100644 tests/acceptance/hotplug_cpu.py > create mode 100644 tests/acceptance/info_usernet.py >
Hi Wainer, On 3/25/21 8:45 PM, Wainer dos Santos Moschetta wrote: > Hi, > > On 3/23/21 7:15 PM, Cleber Rosa wrote: >> This introduces a base class for tests that need to interact with a >> Linux guest. It generalizes the "boot_linux.py" code, already been >> used by the "virtiofs_submounts.py" and also SSH related code being >> used by that and "linux_ssh_mips_malta.py". > > I ran the linux_ssh_mips_malta.py tests, they all passed: > > (11/34) > tests/acceptance/linux_ssh_mips_malta.py:LinuxSSH.test_mips_malta32eb_kernel3_2_0: > PASS (64.41 s) > (12/34) > tests/acceptance/linux_ssh_mips_malta.py:LinuxSSH.test_mips_malta32el_kernel3_2_0: > PASS (63.43 s) > (13/34) > tests/acceptance/linux_ssh_mips_malta.py:LinuxSSH.test_mips_malta64eb_kernel3_2_0: > PASS (63.76 s) > (14/34) > tests/acceptance/linux_ssh_mips_malta.py:LinuxSSH.test_mips_malta64el_kernel3_2_0: > PASS (62.52 s) > > Then I tried the virtiofs_submounts.py tests, it finishes with error. > Something like that fixes it: > > 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 @@ def setUp(self): > > self.run(('ssh-keygen', '-N', '', '-t', 'ed25519', '-f', > self.ssh_key)) > > - pubkey = open(self.ssh_key + '.pub').read() > + pubkey = self.ssh_key + '.pub' Yes I discovered that too when developping the SMMU test. Thanks for mentionning Eric > > super(VirtiofsSubmountsTest, self).setUp(pubkey) > > >> >> While at it, a number of fixes on hopeful improvements to those tests >> were added. >> >> Changes from v1: >> >> * Majority of v1 patches have been merged. >> >> * New patches: >> - Acceptance Tests: make username/password configurable >> - Acceptance Tests: set up SSH connection by default after boot for >> LinuxTest >> - tests/acceptance/virtiofs_submounts.py: remove launch_vm() >> >> * Allowed for the configuration of the network device type (defaulting >> to virtio-net) [Phil] >> >> * Fix module name typo (s/qemu.util/qemu.utils/) in the commit message >> [John] >> >> * Tests based on LinuxTest will have the SSH connection already prepared >> >> Cleber Rosa (10): >> tests/acceptance/virtiofs_submounts.py: add missing accel tag >> tests/acceptance/virtiofs_submounts.py: evaluate string not length >> Python: add utility function for retrieving port redirection >> Acceptance Tests: move useful ssh methods to base class >> Acceptance Tests: add port redirection for ssh by default >> Acceptance Tests: make username/password configurable >> Acceptance Tests: set up SSH connection by default after boot for >> LinuxTest >> tests/acceptance/virtiofs_submounts.py: remove launch_vm() >> Acceptance Tests: add basic documentation on LinuxTest base class >> Acceptance Tests: introduce CPU hotplug test >> >> docs/devel/testing.rst | 25 ++++++++ >> python/qemu/utils.py | 35 ++++++++++++ >> tests/acceptance/avocado_qemu/__init__.py | 63 +++++++++++++++++++-- >> tests/acceptance/hotplug_cpu.py | 37 ++++++++++++ >> tests/acceptance/info_usernet.py | 29 ++++++++++ >> tests/acceptance/linux_ssh_mips_malta.py | 44 ++------------- >> tests/acceptance/virtiofs_submounts.py | 69 +++-------------------- >> tests/vm/basevm.py | 7 +-- >> 8 files changed, 198 insertions(+), 111 deletions(-) >> create mode 100644 python/qemu/utils.py >> create mode 100644 tests/acceptance/hotplug_cpu.py >> create mode 100644 tests/acceptance/info_usernet.py >>
[-- Attachment #1: Type: text/plain, Size: 3652 bytes --] On Thu, Mar 25, 2021 at 02:10:19PM -0400, John Snow wrote: > On 3/23/21 6:15 PM, Cleber Rosa wrote: > > 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 <crosa@redhat.com> > > Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com> > > --- > > python/qemu/utils.py | 35 ++++++++++++++++++++++++ > > tests/acceptance/info_usernet.py | 29 ++++++++++++++++++++ > > tests/acceptance/linux_ssh_mips_malta.py | 16 +++++------ > > tests/acceptance/virtiofs_submounts.py | 21 ++++---------- > > tests/vm/basevm.py | 7 ++--- > > 5 files changed, 78 insertions(+), 30 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..89a246ab30 > > --- /dev/null > > +++ b/python/qemu/utils.py > > @@ -0,0 +1,35 @@ > > +""" > > +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 <crosa@redhat.com> > > +# > > +# 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" > > + :param info_usernet_output: str > > + :return: the port number allocated by the hostfwd option > > + :rtype: int > > I think, unless you know something I don't, that I would prefer to keep type > information in the "live" annotations where they can be checked against rot. > No, that's a good point. No need to have type information defined twice. > > + """ > > + 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 > > I wonder if more guest-specific code doesn't belong elsewhere, but I don't > have a strong counter-suggestion, so I would probably ACK this for now. > There are multiple users of this pattern, and they go beyond the acceptance tests, so I think unifying them is a bit more important then having a better location. Also, like you, I can't think, of a better place at this time. > (Are you okay with the idea that we won't include the utils module in the > PyPI upload? I think I would like to avoid shipping something like this > outside of our castle walls, but agree that having it in the common code > area somewhere for our own use is good.) > At this time I don't have a need for it in the PyPI upload, but I wonder if this exception is justified. I mean, what would be gained, besides dealing with the exception itself, by not including it? Thanks for the feedback! - Cleber [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #1: Type: text/plain, Size: 10095 bytes --] On Wed, Mar 24, 2021 at 10:07:31AM +0100, Auger Eric wrote: > Hi Cleber, > > On 3/23/21 11:15 PM, Cleber Rosa wrote: > > 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 the base LinuxTest > nit: strictly speaking they are moved to another class which is > inherited by LinuxTest, right? I forgot to address this comment previously. Yes, you're right. I'll reword it. Thanks! - Cleber. > > 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 <crosa@redhat.com> > > Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com> > > Reviewed-by: Willian Rampazzo <willianr@redhat.com> > > --- > > tests/acceptance/avocado_qemu/__init__.py | 48 ++++++++++++++++++++++- > > tests/acceptance/linux_ssh_mips_malta.py | 38 ++---------------- > > tests/acceptance/virtiofs_submounts.py | 37 ----------------- > > 3 files changed, 50 insertions(+), 73 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 @@ > > 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 @@ > > 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 @@ def fetch_asset(self, name, > > 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..3f590a081f 100644 > > --- a/tests/acceptance/linux_ssh_mips_malta.py > > +++ b/tests/acceptance/linux_ssh_mips_malta.py > > @@ -12,7 +12,7 @@ > > 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 > > @@ -21,7 +21,7 @@ > > from qemu.utils import get_info_usernet_hostfwd_port > Can't you remove this now? > > > > > > -class LinuxSSH(Test): > > +class LinuxSSH(Test, LinuxSSHMixIn): > out of curiosity why can't it be migrated to a LinuxTest? > > > > timeout = 150 # Not for 'configure --enable-debug --enable-debug-tcg' > > > > @@ -72,41 +72,9 @@ def get_kernel_info(self, endianess, wordsize): > > 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 +95,7 @@ def boot_debian_wheezy_image_and_ssh_login(self, endianess, kernel_path): > > 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 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) > > > > > > Besides, > > Reviewed-by: Eric Auger <eric.auger@redhat.com> > > Thanks > > Eric [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #1: Type: text/plain, Size: 2443 bytes --] On Thu, Mar 25, 2021 at 02:57:48PM -0300, Wainer dos Santos Moschetta wrote: > Hi, > > On 3/24/21 6:10 AM, Auger Eric wrote: > > Hi Cleber, > > > > On 3/23/21 11:15 PM, Cleber Rosa wrote: > > > 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 > > also sets the network device to virtio-net. This may be worth mentioning > > here in the commit msg. > > > arguments. > > > > > > Signed-off-by: Cleber Rosa <crosa@redhat.com> > > Reviewed-by: Eric Auger <eric.auger@redhat.com> > > > > Thanks > > > > Eric > > > > > --- > > > tests/acceptance/avocado_qemu/__init__.py | 4 +++- > > > tests/acceptance/virtiofs_submounts.py | 4 ---- > > > 2 files changed, 3 insertions(+), 5 deletions(-) > > > > > > diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py > > > index 67f75f66e5..e75b002c70 100644 > > > --- a/tests/acceptance/avocado_qemu/__init__.py > > > +++ b/tests/acceptance/avocado_qemu/__init__.py > > > @@ -309,10 +309,12 @@ 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') > > > + 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 @@ def setUp(self): > > > self.vm.add_args('-kernel', vmlinuz, > > > '-append', 'console=ttyS0 root=/dev/sda1') > > > - # Allow us to connect to SSH > > Somewhat related with Eric's suggestion: keep the above comment along with > the netdev setup code. > > - Wainer > Sure, good point. Thanks, - Cleber. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #1: Type: text/plain, Size: 1384 bytes --] On Thu, Mar 25, 2021 at 03:14:58PM -0300, Wainer dos Santos Moschetta wrote: > Hi, > > On 3/23/21 7:15 PM, Cleber Rosa wrote: > > Signed-off-by: Cleber Rosa <crosa@redhat.com> > > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > Reviewed-by: Willian Rampazzo <willianr@redhat.com> > > --- > > docs/devel/testing.rst | 25 +++++++++++++++++++++++++ > > 1 file changed, 25 insertions(+) > > > > diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst > > index 1da4c4e4c4..ed2a06db28 100644 > > --- a/docs/devel/testing.rst > > +++ b/docs/devel/testing.rst > > @@ -810,6 +810,31 @@ 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. The most > > +basic example looks like this: > > I think it is worth mentioning currently it will boot a Fedora 31 cloud-init > image. > Sure, makes sense. Thanks! - Cleber. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #1: Type: text/plain, Size: 1713 bytes --] On Thu, Mar 25, 2021 at 04:45:51PM -0300, Wainer dos Santos Moschetta wrote: > Hi, > > On 3/23/21 7:15 PM, Cleber Rosa wrote: > > This introduces a base class for tests that need to interact with a > > Linux guest. It generalizes the "boot_linux.py" code, already been > > used by the "virtiofs_submounts.py" and also SSH related code being > > used by that and "linux_ssh_mips_malta.py". > > I ran the linux_ssh_mips_malta.py tests, they all passed: > > (11/34) tests/acceptance/linux_ssh_mips_malta.py:LinuxSSH.test_mips_malta32eb_kernel3_2_0: > PASS (64.41 s) > (12/34) tests/acceptance/linux_ssh_mips_malta.py:LinuxSSH.test_mips_malta32el_kernel3_2_0: > PASS (63.43 s) > (13/34) tests/acceptance/linux_ssh_mips_malta.py:LinuxSSH.test_mips_malta64eb_kernel3_2_0: > PASS (63.76 s) > (14/34) tests/acceptance/linux_ssh_mips_malta.py:LinuxSSH.test_mips_malta64el_kernel3_2_0: > PASS (62.52 s) > > Then I tried the virtiofs_submounts.py tests, it finishes with error. > Something like that fixes it: > > 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 @@ def setUp(self): > > 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) > Hi Wainer, Yes, thank you so much for catching that and proposing a fix. I'm adding to the v3 of this series. Thanks again! - Cleber. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --]
On 4/11/21 10:09 PM, Cleber Rosa wrote:
> At this time I don't have a need for it in the PyPI upload, but I
> wonder if this exception is justified. I mean, what would be gained,
> besides dealing with the exception itself, by not including it?
>
I just don't want to support or maintain little one-off misc utilities
and things, but also don't wish to impose a larger design burden on you
to integrate it in a more holistic and subjectively pleasant way.
Having a misc/utils package where we just stick "kinda-sorta" stuff but
never intend to ship or support is a useful middle ground. It's for your
benefit so that we don't have to agonize about the interfaces, but still
create common code that the rest of the QEMU tree can use.
Even shipping 0.x stuff, releasing it onto PyPI imposes quite a design
burden. At least within the QEMU tree I can see who is using which
interfaces and how and avoid breakage. Once we pull that lever ... we
won't have that benefit anymore.
--js