All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/10] Acceptance Test: introduce base class for Linux based tests
@ 2021-03-23 22:15 Cleber Rosa
  2021-03-23 22:15 ` [PATCH v2 01/10] tests/acceptance/virtiofs_submounts.py: add missing accel tag Cleber Rosa
                   ` (10 more replies)
  0 siblings, 11 replies; 53+ messages in thread
From: Cleber Rosa @ 2021-03-23 22:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Max Reitz, Willian Rampazzo,
	Eric Auger, John Snow, Willian Rampazzo, Cleber Rosa,
	Thomas Huth, Marc-André Lureau, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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




^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 01/10] tests/acceptance/virtiofs_submounts.py: add missing accel tag
  2021-03-23 22:15 [PATCH v2 00/10] Acceptance Test: introduce base class for Linux based tests Cleber Rosa
@ 2021-03-23 22:15 ` Cleber Rosa
  2021-03-24  8:45   ` Auger Eric
  2021-03-23 22:15 ` [PATCH v2 02/10] tests/acceptance/virtiofs_submounts.py: evaluate string not length Cleber Rosa
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 53+ messages in thread
From: Cleber Rosa @ 2021-03-23 22:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Max Reitz, Willian Rampazzo,
	Eric Auger, John Snow, Willian Rampazzo, Cleber Rosa,
	Thomas Huth, Marc-André Lureau, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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



^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH v2 02/10] tests/acceptance/virtiofs_submounts.py: evaluate string not length
  2021-03-23 22:15 [PATCH v2 00/10] Acceptance Test: introduce base class for Linux based tests Cleber Rosa
  2021-03-23 22:15 ` [PATCH v2 01/10] tests/acceptance/virtiofs_submounts.py: add missing accel tag Cleber Rosa
@ 2021-03-23 22:15 ` Cleber Rosa
  2021-03-24  8:45   ` Auger Eric
  2021-03-24 19:54   ` Willian Rampazzo
  2021-03-23 22:15 ` [PATCH v2 03/10] Python: add utility function for retrieving port redirection Cleber Rosa
                   ` (8 subsequent siblings)
  10 siblings, 2 replies; 53+ messages in thread
From: Cleber Rosa @ 2021-03-23 22:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Max Reitz, Willian Rampazzo,
	Eric Auger, John Snow, Willian Rampazzo, Cleber Rosa,
	Thomas Huth, Marc-André Lureau, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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



^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH v2 03/10] Python: add utility function for retrieving port redirection
  2021-03-23 22:15 [PATCH v2 00/10] Acceptance Test: introduce base class for Linux based tests Cleber Rosa
  2021-03-23 22:15 ` [PATCH v2 01/10] tests/acceptance/virtiofs_submounts.py: add missing accel tag Cleber Rosa
  2021-03-23 22:15 ` [PATCH v2 02/10] tests/acceptance/virtiofs_submounts.py: evaluate string not length Cleber Rosa
@ 2021-03-23 22:15 ` Cleber Rosa
  2021-03-24  8:58   ` Auger Eric
                     ` (2 more replies)
  2021-03-23 22:15 ` [PATCH v2 04/10] Acceptance Tests: move useful ssh methods to base class Cleber Rosa
                   ` (7 subsequent siblings)
  10 siblings, 3 replies; 53+ messages in thread
From: Cleber Rosa @ 2021-03-23 22:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Max Reitz, Willian Rampazzo,
	Eric Auger, John Snow, Willian Rampazzo, Cleber Rosa,
	Thomas Huth, Marc-André Lureau, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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



^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH v2 04/10] Acceptance Tests: move useful ssh methods to base class
  2021-03-23 22:15 [PATCH v2 00/10] Acceptance Test: introduce base class for Linux based tests Cleber Rosa
                   ` (2 preceding siblings ...)
  2021-03-23 22:15 ` [PATCH v2 03/10] Python: add utility function for retrieving port redirection Cleber Rosa
@ 2021-03-23 22:15 ` Cleber Rosa
  2021-03-24  9:07   ` Auger Eric
  2021-03-23 22:15 ` [PATCH v2 05/10] Acceptance Tests: add port redirection for ssh by default Cleber Rosa
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 53+ messages in thread
From: Cleber Rosa @ 2021-03-23 22:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Max Reitz, Willian Rampazzo,
	Eric Auger, John Snow, Willian Rampazzo, Cleber Rosa,
	Thomas Huth, Marc-André Lureau, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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



^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH v2 05/10] Acceptance Tests: add port redirection for ssh by default
  2021-03-23 22:15 [PATCH v2 00/10] Acceptance Test: introduce base class for Linux based tests Cleber Rosa
                   ` (3 preceding siblings ...)
  2021-03-23 22:15 ` [PATCH v2 04/10] Acceptance Tests: move useful ssh methods to base class Cleber Rosa
@ 2021-03-23 22:15 ` Cleber Rosa
  2021-03-24  8:30   ` Marc-André Lureau
                     ` (3 more replies)
  2021-03-23 22:15 ` [PATCH v2 06/10] Acceptance Tests: make username/password configurable Cleber Rosa
                   ` (5 subsequent siblings)
  10 siblings, 4 replies; 53+ messages in thread
From: Cleber Rosa @ 2021-03-23 22:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Max Reitz, Willian Rampazzo,
	Eric Auger, John Snow, Willian Rampazzo, Cleber Rosa,
	Thomas Huth, Marc-André Lureau, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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



^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH v2 06/10] Acceptance Tests: make username/password configurable
  2021-03-23 22:15 [PATCH v2 00/10] Acceptance Test: introduce base class for Linux based tests Cleber Rosa
                   ` (4 preceding siblings ...)
  2021-03-23 22:15 ` [PATCH v2 05/10] Acceptance Tests: add port redirection for ssh by default Cleber Rosa
@ 2021-03-23 22:15 ` Cleber Rosa
  2021-03-24  8:31   ` Marc-André Lureau
                     ` (2 more replies)
  2021-03-23 22:15 ` [PATCH v2 07/10] Acceptance Tests: set up SSH connection by default after boot for LinuxTest Cleber Rosa
                   ` (4 subsequent siblings)
  10 siblings, 3 replies; 53+ messages in thread
From: Cleber Rosa @ 2021-03-23 22:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Max Reitz, Willian Rampazzo,
	Eric Auger, John Snow, Willian Rampazzo, Cleber Rosa,
	Thomas Huth, Marc-André Lureau, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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



^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH v2 07/10] Acceptance Tests: set up SSH connection by default after boot for LinuxTest
  2021-03-23 22:15 [PATCH v2 00/10] Acceptance Test: introduce base class for Linux based tests Cleber Rosa
                   ` (5 preceding siblings ...)
  2021-03-23 22:15 ` [PATCH v2 06/10] Acceptance Tests: make username/password configurable Cleber Rosa
@ 2021-03-23 22:15 ` Cleber Rosa
  2021-03-24  8:33   ` Marc-André Lureau
                     ` (3 more replies)
  2021-03-23 22:15 ` [PATCH v2 08/10] tests/acceptance/virtiofs_submounts.py: remove launch_vm() Cleber Rosa
                   ` (3 subsequent siblings)
  10 siblings, 4 replies; 53+ messages in thread
From: Cleber Rosa @ 2021-03-23 22:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Max Reitz, Willian Rampazzo,
	Eric Auger, John Snow, Willian Rampazzo, Cleber Rosa,
	Thomas Huth, Marc-André Lureau, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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



^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH v2 08/10] tests/acceptance/virtiofs_submounts.py: remove launch_vm()
  2021-03-23 22:15 [PATCH v2 00/10] Acceptance Test: introduce base class for Linux based tests Cleber Rosa
                   ` (6 preceding siblings ...)
  2021-03-23 22:15 ` [PATCH v2 07/10] Acceptance Tests: set up SSH connection by default after boot for LinuxTest Cleber Rosa
@ 2021-03-23 22:15 ` Cleber Rosa
  2021-03-24  8:34   ` Marc-André Lureau
                     ` (2 more replies)
  2021-03-23 22:15 ` [PATCH v2 09/10] Acceptance Tests: add basic documentation on LinuxTest base class Cleber Rosa
                   ` (2 subsequent siblings)
  10 siblings, 3 replies; 53+ messages in thread
From: Cleber Rosa @ 2021-03-23 22:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Max Reitz, Willian Rampazzo,
	Eric Auger, John Snow, Willian Rampazzo, Cleber Rosa,
	Thomas Huth, Marc-André Lureau, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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



^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH v2 09/10] Acceptance Tests: add basic documentation on LinuxTest base class
  2021-03-23 22:15 [PATCH v2 00/10] Acceptance Test: introduce base class for Linux based tests Cleber Rosa
                   ` (7 preceding siblings ...)
  2021-03-23 22:15 ` [PATCH v2 08/10] tests/acceptance/virtiofs_submounts.py: remove launch_vm() Cleber Rosa
@ 2021-03-23 22:15 ` Cleber Rosa
  2021-03-24  9:25   ` Auger Eric
  2021-03-25 18:14   ` Wainer dos Santos Moschetta
  2021-03-23 22:15 ` [PATCH v2 10/10] Acceptance Tests: introduce CPU hotplug test Cleber Rosa
  2021-03-25 19:45 ` [PATCH v2 00/10] Acceptance Test: introduce base class for Linux based tests Wainer dos Santos Moschetta
  10 siblings, 2 replies; 53+ messages in thread
From: Cleber Rosa @ 2021-03-23 22:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Max Reitz, Willian Rampazzo,
	Eric Auger, John Snow, Willian Rampazzo, Cleber Rosa,
	Thomas Huth, Marc-André Lureau, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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



^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH v2 10/10] Acceptance Tests: introduce CPU hotplug test
  2021-03-23 22:15 [PATCH v2 00/10] Acceptance Test: introduce base class for Linux based tests Cleber Rosa
                   ` (8 preceding siblings ...)
  2021-03-23 22:15 ` [PATCH v2 09/10] Acceptance Tests: add basic documentation on LinuxTest base class Cleber Rosa
@ 2021-03-23 22:15 ` Cleber Rosa
  2021-03-24  9:29   ` Auger Eric
  2021-03-25 19:45 ` [PATCH v2 00/10] Acceptance Test: introduce base class for Linux based tests Wainer dos Santos Moschetta
  10 siblings, 1 reply; 53+ messages in thread
From: Cleber Rosa @ 2021-03-23 22:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Max Reitz, Willian Rampazzo,
	Eric Auger, John Snow, Willian Rampazzo, Cleber Rosa,
	Thomas Huth, Marc-André Lureau, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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



^ permalink raw reply related	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 05/10] Acceptance Tests: add port redirection for ssh by default
  2021-03-23 22:15 ` [PATCH v2 05/10] Acceptance Tests: add port redirection for ssh by default Cleber Rosa
@ 2021-03-24  8:30   ` Marc-André Lureau
  2021-03-24 22:28     ` Cleber Rosa
  2021-03-24  9:10   ` Auger Eric
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 53+ messages in thread
From: Marc-André Lureau @ 2021-03-24  8:30 UTC (permalink / raw)
  To: Cleber Rosa
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, John Snow,
	Philippe Mathieu-Daudé, Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, QEMU, Willian Rampazzo, Eric Auger,
	Willian Rampazzo, Thomas Huth, Max Reitz, Alex Bennée,
	Aurelien Jarno, Beraldo Leal

[-- 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 --]

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 06/10] Acceptance Tests: make username/password configurable
  2021-03-23 22:15 ` [PATCH v2 06/10] Acceptance Tests: make username/password configurable Cleber Rosa
@ 2021-03-24  8:31   ` Marc-André Lureau
  2021-03-24  9:18   ` Auger Eric
  2021-03-24 20:43   ` Willian Rampazzo
  2 siblings, 0 replies; 53+ messages in thread
From: Marc-André Lureau @ 2021-03-24  8:31 UTC (permalink / raw)
  To: Cleber Rosa
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, John Snow,
	Philippe Mathieu-Daudé, Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, QEMU, Willian Rampazzo, Eric Auger,
	Willian Rampazzo, Thomas Huth, Max Reitz, Alex Bennée,
	Aurelien Jarno, Beraldo Leal

[-- 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 --]

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 07/10] Acceptance Tests: set up SSH connection by default after boot for LinuxTest
  2021-03-23 22:15 ` [PATCH v2 07/10] Acceptance Tests: set up SSH connection by default after boot for LinuxTest Cleber Rosa
@ 2021-03-24  8:33   ` Marc-André Lureau
  2021-03-24  9:22   ` Auger Eric
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 53+ messages in thread
From: Marc-André Lureau @ 2021-03-24  8:33 UTC (permalink / raw)
  To: Cleber Rosa
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, John Snow,
	Philippe Mathieu-Daudé, Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, QEMU, Willian Rampazzo, Eric Auger,
	Willian Rampazzo, Thomas Huth, Max Reitz, Alex Bennée,
	Aurelien Jarno, Beraldo Leal

[-- 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 --]

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 08/10] tests/acceptance/virtiofs_submounts.py: remove launch_vm()
  2021-03-23 22:15 ` [PATCH v2 08/10] tests/acceptance/virtiofs_submounts.py: remove launch_vm() Cleber Rosa
@ 2021-03-24  8:34   ` Marc-André Lureau
  2021-03-24  9:24   ` Auger Eric
  2021-03-24 20:46   ` Willian Rampazzo
  2 siblings, 0 replies; 53+ messages in thread
From: Marc-André Lureau @ 2021-03-24  8:34 UTC (permalink / raw)
  To: Cleber Rosa
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, John Snow,
	Philippe Mathieu-Daudé, Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, QEMU, Willian Rampazzo, Eric Auger,
	Willian Rampazzo, Thomas Huth, Max Reitz, Alex Bennée,
	Aurelien Jarno, Beraldo Leal

[-- 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 --]

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 02/10] tests/acceptance/virtiofs_submounts.py: evaluate string not length
  2021-03-23 22:15 ` [PATCH v2 02/10] tests/acceptance/virtiofs_submounts.py: evaluate string not length Cleber Rosa
@ 2021-03-24  8:45   ` Auger Eric
  2021-03-24 19:54   ` Willian Rampazzo
  1 sibling, 0 replies; 53+ messages in thread
From: Auger Eric @ 2021-03-24  8:45 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Max Reitz, Willian Rampazzo,
	Eric Auger, John Snow, Willian Rampazzo, Thomas Huth,
	Marc-André Lureau, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 01/10] tests/acceptance/virtiofs_submounts.py: add missing accel tag
  2021-03-23 22:15 ` [PATCH v2 01/10] tests/acceptance/virtiofs_submounts.py: add missing accel tag Cleber Rosa
@ 2021-03-24  8:45   ` Auger Eric
  0 siblings, 0 replies; 53+ messages in thread
From: Auger Eric @ 2021-03-24  8:45 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Max Reitz, Willian Rampazzo,
	Eric Auger, John Snow, Willian Rampazzo, Thomas Huth,
	Marc-André Lureau, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 03/10] Python: add utility function for retrieving port redirection
  2021-03-23 22:15 ` [PATCH v2 03/10] Python: add utility function for retrieving port redirection Cleber Rosa
@ 2021-03-24  8:58   ` Auger Eric
  2021-03-24 20:35   ` Willian Rampazzo
  2021-03-25 18:10   ` John Snow
  2 siblings, 0 replies; 53+ messages in thread
From: Auger Eric @ 2021-03-24  8:58 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Max Reitz, Willian Rampazzo,
	Eric Auger, John Snow, Willian Rampazzo, Thomas Huth,
	Marc-André Lureau, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 04/10] Acceptance Tests: move useful ssh methods to base class
  2021-03-23 22:15 ` [PATCH v2 04/10] Acceptance Tests: move useful ssh methods to base class Cleber Rosa
@ 2021-03-24  9:07   ` Auger Eric
  2021-03-24 22:23     ` Cleber Rosa
  2021-04-12  2:18     ` Cleber Rosa
  0 siblings, 2 replies; 53+ messages in thread
From: Auger Eric @ 2021-03-24  9:07 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Max Reitz, Willian Rampazzo,
	Eric Auger, John Snow, Willian Rampazzo, Thomas Huth,
	Marc-André Lureau, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 05/10] Acceptance Tests: add port redirection for ssh by default
  2021-03-23 22:15 ` [PATCH v2 05/10] Acceptance Tests: add port redirection for ssh by default Cleber Rosa
  2021-03-24  8:30   ` Marc-André Lureau
@ 2021-03-24  9:10   ` Auger Eric
  2021-03-24 22:27     ` Cleber Rosa
  2021-03-25 17:57     ` Wainer dos Santos Moschetta
  2021-03-24 10:36   ` Auger Eric
  2021-03-24 20:39   ` Willian Rampazzo
  3 siblings, 2 replies; 53+ messages in thread
From: Auger Eric @ 2021-03-24  9:10 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Max Reitz, Willian Rampazzo,
	Eric Auger, John Snow, Willian Rampazzo, Thomas Huth,
	Marc-André Lureau, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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')
>  
> 



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 06/10] Acceptance Tests: make username/password configurable
  2021-03-23 22:15 ` [PATCH v2 06/10] Acceptance Tests: make username/password configurable Cleber Rosa
  2021-03-24  8:31   ` Marc-André Lureau
@ 2021-03-24  9:18   ` Auger Eric
  2021-03-24 20:43   ` Willian Rampazzo
  2 siblings, 0 replies; 53+ messages in thread
From: Auger Eric @ 2021-03-24  9:18 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Max Reitz, Willian Rampazzo,
	Eric Auger, John Snow, Willian Rampazzo, Thomas Huth,
	Marc-André Lureau, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal



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,
> 



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 07/10] Acceptance Tests: set up SSH connection by default after boot for LinuxTest
  2021-03-23 22:15 ` [PATCH v2 07/10] Acceptance Tests: set up SSH connection by default after boot for LinuxTest Cleber Rosa
  2021-03-24  8:33   ` Marc-André Lureau
@ 2021-03-24  9:22   ` Auger Eric
  2021-03-24 22:45     ` Cleber Rosa
  2021-03-24 20:45   ` Willian Rampazzo
  2021-03-25 14:31   ` Wainer dos Santos Moschetta
  3 siblings, 1 reply; 53+ messages in thread
From: Auger Eric @ 2021-03-24  9:22 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Max Reitz, Willian Rampazzo,
	Eric Auger, John Snow, Willian Rampazzo, Thomas Huth,
	Marc-André Lureau, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 08/10] tests/acceptance/virtiofs_submounts.py: remove launch_vm()
  2021-03-23 22:15 ` [PATCH v2 08/10] tests/acceptance/virtiofs_submounts.py: remove launch_vm() Cleber Rosa
  2021-03-24  8:34   ` Marc-André Lureau
@ 2021-03-24  9:24   ` Auger Eric
  2021-03-24 20:46   ` Willian Rampazzo
  2 siblings, 0 replies; 53+ messages in thread
From: Auger Eric @ 2021-03-24  9:24 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Max Reitz, Willian Rampazzo,
	Eric Auger, John Snow, Willian Rampazzo, Thomas Huth,
	Marc-André Lureau, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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()
>  
> 



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 09/10] Acceptance Tests: add basic documentation on LinuxTest base class
  2021-03-23 22:15 ` [PATCH v2 09/10] Acceptance Tests: add basic documentation on LinuxTest base class Cleber Rosa
@ 2021-03-24  9:25   ` Auger Eric
  2021-03-25 18:14   ` Wainer dos Santos Moschetta
  1 sibling, 0 replies; 53+ messages in thread
From: Auger Eric @ 2021-03-24  9:25 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Max Reitz, Willian Rampazzo,
	Eric Auger, John Snow, Willian Rampazzo, Thomas Huth,
	Marc-André Lureau, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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
>  ~~~~~~~~~~~
>  
> 



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 10/10] Acceptance Tests: introduce CPU hotplug test
  2021-03-23 22:15 ` [PATCH v2 10/10] Acceptance Tests: introduce CPU hotplug test Cleber Rosa
@ 2021-03-24  9:29   ` Auger Eric
  0 siblings, 0 replies; 53+ messages in thread
From: Auger Eric @ 2021-03-24  9:29 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Max Reitz, Willian Rampazzo,
	Eric Auger, John Snow, Willian Rampazzo, Thomas Huth,
	Marc-André Lureau, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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')
> 



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 05/10] Acceptance Tests: add port redirection for ssh by default
  2021-03-23 22:15 ` [PATCH v2 05/10] Acceptance Tests: add port redirection for ssh by default Cleber Rosa
  2021-03-24  8:30   ` Marc-André Lureau
  2021-03-24  9:10   ` Auger Eric
@ 2021-03-24 10:36   ` Auger Eric
  2021-03-24 22:40     ` Cleber Rosa
  2021-03-24 20:39   ` Willian Rampazzo
  3 siblings, 1 reply; 53+ messages in thread
From: Auger Eric @ 2021-03-24 10:36 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Max Reitz, Willian Rampazzo,
	Eric Auger, John Snow, Willian Rampazzo, Thomas Huth,
	Marc-André Lureau, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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')
>  
> 



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 02/10] tests/acceptance/virtiofs_submounts.py: evaluate string not length
  2021-03-23 22:15 ` [PATCH v2 02/10] tests/acceptance/virtiofs_submounts.py: evaluate string not length Cleber Rosa
  2021-03-24  8:45   ` Auger Eric
@ 2021-03-24 19:54   ` Willian Rampazzo
  1 sibling, 0 replies; 53+ messages in thread
From: Willian Rampazzo @ 2021-03-24 19:54 UTC (permalink / raw)
  To: Cleber Rosa
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, qemu-devel, Willian Rampazzo,
	Eric Auger, John Snow, Thomas Huth, Marc-André Lureau,
	Max Reitz, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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>



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 03/10] Python: add utility function for retrieving port redirection
  2021-03-23 22:15 ` [PATCH v2 03/10] Python: add utility function for retrieving port redirection Cleber Rosa
  2021-03-24  8:58   ` Auger Eric
@ 2021-03-24 20:35   ` Willian Rampazzo
  2021-03-24 21:58     ` Cleber Rosa
  2021-03-25 18:10   ` John Snow
  2 siblings, 1 reply; 53+ messages in thread
From: Willian Rampazzo @ 2021-03-24 20:35 UTC (permalink / raw)
  To: Cleber Rosa
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, qemu-devel, Willian Rampazzo,
	Eric Auger, John Snow, Thomas Huth, Marc-André Lureau,
	Max Reitz, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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>



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 05/10] Acceptance Tests: add port redirection for ssh by default
  2021-03-23 22:15 ` [PATCH v2 05/10] Acceptance Tests: add port redirection for ssh by default Cleber Rosa
                     ` (2 preceding siblings ...)
  2021-03-24 10:36   ` Auger Eric
@ 2021-03-24 20:39   ` Willian Rampazzo
  3 siblings, 0 replies; 53+ messages in thread
From: Willian Rampazzo @ 2021-03-24 20:39 UTC (permalink / raw)
  To: Cleber Rosa
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, qemu-devel, Willian Rampazzo,
	Eric Auger, John Snow, Thomas Huth, Marc-André Lureau,
	Max Reitz, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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>



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 06/10] Acceptance Tests: make username/password configurable
  2021-03-23 22:15 ` [PATCH v2 06/10] Acceptance Tests: make username/password configurable Cleber Rosa
  2021-03-24  8:31   ` Marc-André Lureau
  2021-03-24  9:18   ` Auger Eric
@ 2021-03-24 20:43   ` Willian Rampazzo
  2 siblings, 0 replies; 53+ messages in thread
From: Willian Rampazzo @ 2021-03-24 20:43 UTC (permalink / raw)
  To: Cleber Rosa
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, qemu-devel, Willian Rampazzo,
	Eric Auger, John Snow, Thomas Huth, Marc-André Lureau,
	Max Reitz, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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>



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 07/10] Acceptance Tests: set up SSH connection by default after boot for LinuxTest
  2021-03-23 22:15 ` [PATCH v2 07/10] Acceptance Tests: set up SSH connection by default after boot for LinuxTest Cleber Rosa
  2021-03-24  8:33   ` Marc-André Lureau
  2021-03-24  9:22   ` Auger Eric
@ 2021-03-24 20:45   ` Willian Rampazzo
  2021-03-25 14:31   ` Wainer dos Santos Moschetta
  3 siblings, 0 replies; 53+ messages in thread
From: Willian Rampazzo @ 2021-03-24 20:45 UTC (permalink / raw)
  To: Cleber Rosa
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, qemu-devel, Willian Rampazzo,
	Eric Auger, John Snow, Thomas Huth, Marc-André Lureau,
	Max Reitz, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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>



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 08/10] tests/acceptance/virtiofs_submounts.py: remove launch_vm()
  2021-03-23 22:15 ` [PATCH v2 08/10] tests/acceptance/virtiofs_submounts.py: remove launch_vm() Cleber Rosa
  2021-03-24  8:34   ` Marc-André Lureau
  2021-03-24  9:24   ` Auger Eric
@ 2021-03-24 20:46   ` Willian Rampazzo
  2 siblings, 0 replies; 53+ messages in thread
From: Willian Rampazzo @ 2021-03-24 20:46 UTC (permalink / raw)
  To: Cleber Rosa
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, qemu-devel, Willian Rampazzo,
	Eric Auger, John Snow, Thomas Huth, Marc-André Lureau,
	Max Reitz, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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>



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 03/10] Python: add utility function for retrieving port redirection
  2021-03-24 20:35   ` Willian Rampazzo
@ 2021-03-24 21:58     ` Cleber Rosa
  0 siblings, 0 replies; 53+ messages in thread
From: Cleber Rosa @ 2021-03-24 21:58 UTC (permalink / raw)
  To: Willian Rampazzo
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, qemu-devel, Willian Rampazzo,
	Eric Auger, John Snow, Thomas Huth, Marc-André Lureau,
	Max Reitz, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

[-- 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 --]

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 04/10] Acceptance Tests: move useful ssh methods to base class
  2021-03-24  9:07   ` Auger Eric
@ 2021-03-24 22:23     ` Cleber Rosa
  2021-03-25 12:50       ` Auger Eric
  2021-04-12  2:18     ` Cleber Rosa
  1 sibling, 1 reply; 53+ messages in thread
From: Cleber Rosa @ 2021-03-24 22:23 UTC (permalink / raw)
  To: Auger Eric
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	qemu-devel, Wainer dos Santos Moschetta,
	Philippe Mathieu-Daudé,
	Willian Rampazzo, Eric Auger, John Snow, Willian Rampazzo,
	Thomas Huth, Marc-André Lureau, Max Reitz,
	Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

[-- 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 --]

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 05/10] Acceptance Tests: add port redirection for ssh by default
  2021-03-24  9:10   ` Auger Eric
@ 2021-03-24 22:27     ` Cleber Rosa
  2021-03-25 17:57     ` Wainer dos Santos Moschetta
  1 sibling, 0 replies; 53+ messages in thread
From: Cleber Rosa @ 2021-03-24 22:27 UTC (permalink / raw)
  To: Auger Eric
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	qemu-devel, Wainer dos Santos Moschetta,
	Philippe Mathieu-Daudé,
	Willian Rampazzo, Eric Auger, John Snow, Willian Rampazzo,
	Thomas Huth, Marc-André Lureau, Max Reitz,
	Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

[-- 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 --]

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 05/10] Acceptance Tests: add port redirection for ssh by default
  2021-03-24  8:30   ` Marc-André Lureau
@ 2021-03-24 22:28     ` Cleber Rosa
  0 siblings, 0 replies; 53+ messages in thread
From: Cleber Rosa @ 2021-03-24 22:28 UTC (permalink / raw)
  To: Marc-André Lureau
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, John Snow,
	Philippe Mathieu-Daudé, Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, QEMU, Willian Rampazzo, Eric Auger,
	Willian Rampazzo, Thomas Huth, Max Reitz, Alex Bennée,
	Aurelien Jarno, Beraldo Leal

[-- 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 --]

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 05/10] Acceptance Tests: add port redirection for ssh by default
  2021-03-24 10:36   ` Auger Eric
@ 2021-03-24 22:40     ` Cleber Rosa
  0 siblings, 0 replies; 53+ messages in thread
From: Cleber Rosa @ 2021-03-24 22:40 UTC (permalink / raw)
  To: Auger Eric
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	qemu-devel, Wainer dos Santos Moschetta,
	Philippe Mathieu-Daudé,
	Willian Rampazzo, Eric Auger, John Snow, Willian Rampazzo,
	Thomas Huth, Marc-André Lureau, Max Reitz,
	Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

[-- 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 --]

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 07/10] Acceptance Tests: set up SSH connection by default after boot for LinuxTest
  2021-03-24  9:22   ` Auger Eric
@ 2021-03-24 22:45     ` Cleber Rosa
  0 siblings, 0 replies; 53+ messages in thread
From: Cleber Rosa @ 2021-03-24 22:45 UTC (permalink / raw)
  To: Auger Eric
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	qemu-devel, Wainer dos Santos Moschetta,
	Philippe Mathieu-Daudé,
	Willian Rampazzo, Eric Auger, John Snow, Willian Rampazzo,
	Thomas Huth, Marc-André Lureau, Max Reitz,
	Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

[-- 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 --]

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 04/10] Acceptance Tests: move useful ssh methods to base class
  2021-03-24 22:23     ` Cleber Rosa
@ 2021-03-25 12:50       ` Auger Eric
  2021-03-25 17:43         ` Wainer dos Santos Moschetta
  0 siblings, 1 reply; 53+ messages in thread
From: Auger Eric @ 2021-03-25 12:50 UTC (permalink / raw)
  To: Cleber Rosa
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	qemu-devel, Wainer dos Santos Moschetta,
	Philippe Mathieu-Daudé,
	Willian Rampazzo, Eric Auger, John Snow, Willian Rampazzo,
	Thomas Huth, Marc-André Lureau, Max Reitz,
	Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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.
> 



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 07/10] Acceptance Tests: set up SSH connection by default after boot for LinuxTest
  2021-03-23 22:15 ` [PATCH v2 07/10] Acceptance Tests: set up SSH connection by default after boot for LinuxTest Cleber Rosa
                     ` (2 preceding siblings ...)
  2021-03-24 20:45   ` Willian Rampazzo
@ 2021-03-25 14:31   ` Wainer dos Santos Moschetta
  2021-03-25 14:53     ` Wainer dos Santos Moschetta
  3 siblings, 1 reply; 53+ messages in thread
From: Wainer dos Santos Moschetta @ 2021-03-25 14:31 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, John Snow,
	Philippe Mathieu-Daudé, Philippe Mathieu-Daudé,
	Max Reitz, Willian Rampazzo, Eric Auger, Willian Rampazzo,
	Thomas Huth, Marc-André Lureau, Alex Bennée,
	Aurelien Jarno, Beraldo Leal

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')



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 07/10] Acceptance Tests: set up SSH connection by default after boot for LinuxTest
  2021-03-25 14:31   ` Wainer dos Santos Moschetta
@ 2021-03-25 14:53     ` Wainer dos Santos Moschetta
  0 siblings, 0 replies; 53+ messages in thread
From: Wainer dos Santos Moschetta @ 2021-03-25 14:53 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	John Snow, Philippe Mathieu-Daudé,
	Max Reitz, Willian Rampazzo, Eric Auger, Willian Rampazzo,
	Thomas Huth, Marc-André Lureau, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal


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')
>
>



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 04/10] Acceptance Tests: move useful ssh methods to base class
  2021-03-25 12:50       ` Auger Eric
@ 2021-03-25 17:43         ` Wainer dos Santos Moschetta
  0 siblings, 0 replies; 53+ messages in thread
From: Wainer dos Santos Moschetta @ 2021-03-25 17:43 UTC (permalink / raw)
  To: Auger Eric, Cleber Rosa
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	qemu-devel, Philippe Mathieu-Daudé,
	Willian Rampazzo, John Snow, Eric Auger, Willian Rampazzo,
	Thomas Huth, Marc-André Lureau, Max Reitz,
	Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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.
>>



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 05/10] Acceptance Tests: add port redirection for ssh by default
  2021-03-24  9:10   ` Auger Eric
  2021-03-24 22:27     ` Cleber Rosa
@ 2021-03-25 17:57     ` Wainer dos Santos Moschetta
  2021-04-12  2:47       ` Cleber Rosa
  1 sibling, 1 reply; 53+ messages in thread
From: Wainer dos Santos Moschetta @ 2021-03-25 17:57 UTC (permalink / raw)
  To: Auger Eric, Cleber Rosa, qemu-devel
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, John Snow,
	Philippe Mathieu-Daudé, Philippe Mathieu-Daudé,
	Max Reitz, Willian Rampazzo, Eric Auger, Willian Rampazzo,
	Thomas Huth, Marc-André Lureau, Alex Bennée,
	Aurelien Jarno, Beraldo Leal

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')
>>   
>>
>



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 03/10] Python: add utility function for retrieving port redirection
  2021-03-23 22:15 ` [PATCH v2 03/10] Python: add utility function for retrieving port redirection Cleber Rosa
  2021-03-24  8:58   ` Auger Eric
  2021-03-24 20:35   ` Willian Rampazzo
@ 2021-03-25 18:10   ` John Snow
  2021-04-12  2:09     ` Cleber Rosa
  2 siblings, 1 reply; 53+ messages in thread
From: John Snow @ 2021-03-25 18:10 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Max Reitz, Willian Rampazzo,
	Eric Auger, Willian Rampazzo, Thomas Huth,
	Marc-André Lureau, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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)
> 



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 09/10] Acceptance Tests: add basic documentation on LinuxTest base class
  2021-03-23 22:15 ` [PATCH v2 09/10] Acceptance Tests: add basic documentation on LinuxTest base class Cleber Rosa
  2021-03-24  9:25   ` Auger Eric
@ 2021-03-25 18:14   ` Wainer dos Santos Moschetta
  2021-04-12  2:59     ` Cleber Rosa
  1 sibling, 1 reply; 53+ messages in thread
From: Wainer dos Santos Moschetta @ 2021-03-25 18:14 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, John Snow,
	Philippe Mathieu-Daudé, Philippe Mathieu-Daudé,
	Max Reitz, Willian Rampazzo, Eric Auger, Willian Rampazzo,
	Thomas Huth, Marc-André Lureau, Alex Bennée,
	Aurelien Jarno, Beraldo Leal

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
>   ~~~~~~~~~~~
>   



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 00/10] Acceptance Test: introduce base class for Linux based tests
  2021-03-23 22:15 [PATCH v2 00/10] Acceptance Test: introduce base class for Linux based tests Cleber Rosa
                   ` (9 preceding siblings ...)
  2021-03-23 22:15 ` [PATCH v2 10/10] Acceptance Tests: introduce CPU hotplug test Cleber Rosa
@ 2021-03-25 19:45 ` Wainer dos Santos Moschetta
  2021-03-26  8:06   ` Auger Eric
  2021-04-12  3:22   ` Cleber Rosa
  10 siblings, 2 replies; 53+ messages in thread
From: Wainer dos Santos Moschetta @ 2021-03-25 19:45 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Max Reitz, Willian Rampazzo, John Snow, Eric Auger,
	Willian Rampazzo, Thomas Huth, Marc-André Lureau,
	Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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
>



^ permalink raw reply related	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 00/10] Acceptance Test: introduce base class for Linux based tests
  2021-03-25 19:45 ` [PATCH v2 00/10] Acceptance Test: introduce base class for Linux based tests Wainer dos Santos Moschetta
@ 2021-03-26  8:06   ` Auger Eric
  2021-04-12  3:22   ` Cleber Rosa
  1 sibling, 0 replies; 53+ messages in thread
From: Auger Eric @ 2021-03-26  8:06 UTC (permalink / raw)
  To: Wainer dos Santos Moschetta, Cleber Rosa, qemu-devel
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	Philippe Mathieu-Daudé,
	Max Reitz, Willian Rampazzo, John Snow, Eric Auger,
	Willian Rampazzo, Thomas Huth, Marc-André Lureau,
	Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

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
>>



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 03/10] Python: add utility function for retrieving port redirection
  2021-03-25 18:10   ` John Snow
@ 2021-04-12  2:09     ` Cleber Rosa
  2021-05-13 19:16       ` John Snow
  0 siblings, 1 reply; 53+ messages in thread
From: Cleber Rosa @ 2021-04-12  2:09 UTC (permalink / raw)
  To: John Snow
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	qemu-devel, Wainer dos Santos Moschetta,
	Philippe Mathieu-Daudé,
	Willian Rampazzo, Eric Auger, Willian Rampazzo, Thomas Huth,
	Marc-André Lureau, Max Reitz, Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

[-- 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 --]

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 04/10] Acceptance Tests: move useful ssh methods to base class
  2021-03-24  9:07   ` Auger Eric
  2021-03-24 22:23     ` Cleber Rosa
@ 2021-04-12  2:18     ` Cleber Rosa
  1 sibling, 0 replies; 53+ messages in thread
From: Cleber Rosa @ 2021-04-12  2:18 UTC (permalink / raw)
  To: Auger Eric
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, Alex Bennée,
	qemu-devel, Wainer dos Santos Moschetta,
	Philippe Mathieu-Daudé,
	Willian Rampazzo, Eric Auger, John Snow, Willian Rampazzo,
	Thomas Huth, Marc-André Lureau, Max Reitz,
	Philippe Mathieu-Daudé,
	Aurelien Jarno, Beraldo Leal

[-- 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 --]

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 05/10] Acceptance Tests: add port redirection for ssh by default
  2021-03-25 17:57     ` Wainer dos Santos Moschetta
@ 2021-04-12  2:47       ` Cleber Rosa
  0 siblings, 0 replies; 53+ messages in thread
From: Cleber Rosa @ 2021-04-12  2:47 UTC (permalink / raw)
  To: Wainer dos Santos Moschetta
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, John Snow,
	Philippe Mathieu-Daudé,
	qemu-devel, Philippe Mathieu-Daudé,
	Willian Rampazzo, Auger Eric, Eric Auger, Willian Rampazzo,
	Thomas Huth, Marc-André Lureau, Max Reitz, Alex Bennée,
	Aurelien Jarno, Beraldo Leal

[-- 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 --]

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 09/10] Acceptance Tests: add basic documentation on LinuxTest base class
  2021-03-25 18:14   ` Wainer dos Santos Moschetta
@ 2021-04-12  2:59     ` Cleber Rosa
  0 siblings, 0 replies; 53+ messages in thread
From: Cleber Rosa @ 2021-04-12  2:59 UTC (permalink / raw)
  To: Wainer dos Santos Moschetta
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, John Snow,
	Philippe Mathieu-Daudé, Philippe Mathieu-Daudé,
	qemu-devel, Willian Rampazzo, Eric Auger, Willian Rampazzo,
	Thomas Huth, Marc-André Lureau, Max Reitz, Alex Bennée,
	Aurelien Jarno, Beraldo Leal

[-- 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 --]

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 00/10] Acceptance Test: introduce base class for Linux based tests
  2021-03-25 19:45 ` [PATCH v2 00/10] Acceptance Test: introduce base class for Linux based tests Wainer dos Santos Moschetta
  2021-03-26  8:06   ` Auger Eric
@ 2021-04-12  3:22   ` Cleber Rosa
  1 sibling, 0 replies; 53+ messages in thread
From: Cleber Rosa @ 2021-04-12  3:22 UTC (permalink / raw)
  To: Wainer dos Santos Moschetta
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost, John Snow,
	Philippe Mathieu-Daudé, Philippe Mathieu-Daudé,
	qemu-devel, Willian Rampazzo, Eric Auger, Willian Rampazzo,
	Thomas Huth, Marc-André Lureau, Max Reitz, Alex Bennée,
	Aurelien Jarno, Beraldo Leal

[-- 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 --]

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 03/10] Python: add utility function for retrieving port redirection
  2021-04-12  2:09     ` Cleber Rosa
@ 2021-05-13 19:16       ` John Snow
  0 siblings, 0 replies; 53+ messages in thread
From: John Snow @ 2021-05-13 19:16 UTC (permalink / raw)
  To: Cleber Rosa
  Cc: Fam Zheng, Aleksandar Rikalo, Eduardo Habkost,
	Philippe Mathieu-Daudé,
	qemu-devel, Wainer dos Santos Moschetta,
	Philippe Mathieu-Daudé,
	Willian Rampazzo, Eric Auger, Willian Rampazzo, Thomas Huth,
	Marc-André Lureau, Max Reitz, Alex Bennée,
	Aurelien Jarno, Beraldo Leal

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



^ permalink raw reply	[flat|nested] 53+ messages in thread

end of thread, other threads:[~2021-05-13 19:18 UTC | newest]

Thread overview: 53+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-23 22:15 [PATCH v2 00/10] Acceptance Test: introduce base class for Linux based tests Cleber Rosa
2021-03-23 22:15 ` [PATCH v2 01/10] tests/acceptance/virtiofs_submounts.py: add missing accel tag Cleber Rosa
2021-03-24  8:45   ` Auger Eric
2021-03-23 22:15 ` [PATCH v2 02/10] tests/acceptance/virtiofs_submounts.py: evaluate string not length Cleber Rosa
2021-03-24  8:45   ` Auger Eric
2021-03-24 19:54   ` Willian Rampazzo
2021-03-23 22:15 ` [PATCH v2 03/10] Python: add utility function for retrieving port redirection Cleber Rosa
2021-03-24  8:58   ` Auger Eric
2021-03-24 20:35   ` Willian Rampazzo
2021-03-24 21:58     ` Cleber Rosa
2021-03-25 18:10   ` John Snow
2021-04-12  2:09     ` Cleber Rosa
2021-05-13 19:16       ` John Snow
2021-03-23 22:15 ` [PATCH v2 04/10] Acceptance Tests: move useful ssh methods to base class Cleber Rosa
2021-03-24  9:07   ` Auger Eric
2021-03-24 22:23     ` Cleber Rosa
2021-03-25 12:50       ` Auger Eric
2021-03-25 17:43         ` Wainer dos Santos Moschetta
2021-04-12  2:18     ` Cleber Rosa
2021-03-23 22:15 ` [PATCH v2 05/10] Acceptance Tests: add port redirection for ssh by default Cleber Rosa
2021-03-24  8:30   ` Marc-André Lureau
2021-03-24 22:28     ` Cleber Rosa
2021-03-24  9:10   ` Auger Eric
2021-03-24 22:27     ` Cleber Rosa
2021-03-25 17:57     ` Wainer dos Santos Moschetta
2021-04-12  2:47       ` Cleber Rosa
2021-03-24 10:36   ` Auger Eric
2021-03-24 22:40     ` Cleber Rosa
2021-03-24 20:39   ` Willian Rampazzo
2021-03-23 22:15 ` [PATCH v2 06/10] Acceptance Tests: make username/password configurable Cleber Rosa
2021-03-24  8:31   ` Marc-André Lureau
2021-03-24  9:18   ` Auger Eric
2021-03-24 20:43   ` Willian Rampazzo
2021-03-23 22:15 ` [PATCH v2 07/10] Acceptance Tests: set up SSH connection by default after boot for LinuxTest Cleber Rosa
2021-03-24  8:33   ` Marc-André Lureau
2021-03-24  9:22   ` Auger Eric
2021-03-24 22:45     ` Cleber Rosa
2021-03-24 20:45   ` Willian Rampazzo
2021-03-25 14:31   ` Wainer dos Santos Moschetta
2021-03-25 14:53     ` Wainer dos Santos Moschetta
2021-03-23 22:15 ` [PATCH v2 08/10] tests/acceptance/virtiofs_submounts.py: remove launch_vm() Cleber Rosa
2021-03-24  8:34   ` Marc-André Lureau
2021-03-24  9:24   ` Auger Eric
2021-03-24 20:46   ` Willian Rampazzo
2021-03-23 22:15 ` [PATCH v2 09/10] Acceptance Tests: add basic documentation on LinuxTest base class Cleber Rosa
2021-03-24  9:25   ` Auger Eric
2021-03-25 18:14   ` Wainer dos Santos Moschetta
2021-04-12  2:59     ` Cleber Rosa
2021-03-23 22:15 ` [PATCH v2 10/10] Acceptance Tests: introduce CPU hotplug test Cleber Rosa
2021-03-24  9:29   ` Auger Eric
2021-03-25 19:45 ` [PATCH v2 00/10] Acceptance Test: introduce base class for Linux based tests Wainer dos Santos Moschetta
2021-03-26  8:06   ` Auger Eric
2021-04-12  3:22   ` Cleber Rosa

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.