Merge remote-tracking branch 'remotes/cleber-gitlab/tags/python-next-pull-request' into staging

Acceptance Tests and Python libs improvements

Along with the Acceptance Tests and Python libs improvements, a
improvement to the diff generation for Python code.

# gpg: Signature made Tue 16 Feb 2021 04:55:45 GMT
# gpg:                using RSA key 7ABB96EB8B46B94D5E0FE9BB657E8D33A5F209F3
# gpg: Good signature from "Cleber Rosa <crosa@redhat.com>" [marginal]
# gpg: WARNING: This key is not certified with sufficiently trusted signatures!
# gpg:          It is not certain that the signature belongs to the owner.
# Primary key fingerprint: 7ABB 96EB 8B46 B94D 5E0F  E9BB 657E 8D33 A5F2 09F3

* remotes/cleber-gitlab/tags/python-next-pull-request:
  Acceptance Tests: set up existing ssh keys by default
  Acceptance Tests: fix population of public key in cloudinit image
  Acceptance Tests: introduce method for requiring an accelerator
  Acceptance Tests: introduce LinuxTest base class
  maint: Tell git that *.py files should use python diff hunks
  tests/acceptance/virtio-gpu.py: preserve virtio-user-gpu log
  Python: close the log file kept by QEMUMachine before reading it
  virtiofs_submounts.py test: Note on vmlinuz param
  Acceptance Tests: bump Avocado version requirement to 85.0

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

7 files changed, 163 insertions, 127 deletions

diff --git a/.gitattributes b/.gitattributes
index 3d2fe2e..07f430e 100644
--- a/.gitattributes
+++ b/.gitattributes
@@ -1,2 +1,3 @@
 *.c.inc         diff=c
 *.h.inc         diff=c
+*.py            diff=python
diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 7a40f46..6e44bda 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -337,12 +337,12 @@ class QEMUMachine:
             self._qmp.close()
             self._qmp_connection = None
 
-        self._load_io_log()
-
         if self._qemu_log_file is not None:
             self._qemu_log_file.close()
             self._qemu_log_file = None
 
+        self._load_io_log()
+
         self._qemu_log_path = None
 
         if self._temp_dir is not None:
diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
index bf54e41..df167b1 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -10,12 +10,20 @@
 
 import logging
 import os
+import shutil
 import sys
 import uuid
 import tempfile
 
 import avocado
 
+from avocado.utils import cloudinit
+from avocado.utils import datadrainer
+from avocado.utils import network
+from avocado.utils import vmimage
+from avocado.utils.path import find_command
+
+
 #: The QEMU build root directory.  It may also be the source directory
 #: if building from the source dir, but it's safer to use BUILD_DIR for
 #: that purpose.  Be aware that if this code is moved outside of a source
@@ -32,6 +40,8 @@ else:
 
 sys.path.append(os.path.join(SOURCE_DIR, 'python'))
 
+from qemu.accel import kvm_available
+from qemu.accel import tcg_available
 from qemu.machine import QEMUMachine
 
 def is_readable_executable_file(path):
@@ -155,6 +165,28 @@ class Test(avocado.Test):
             return vals.pop()
         return None
 
+    def require_accelerator(self, accelerator):
+        """
+        Requires an accelerator to be available for the test to continue
+
+        It takes into account the currently set qemu binary.
+
+        If the check fails, the test is canceled.  If the check itself
+        for the given accelerator is not available, the test is also
+        canceled.
+
+        :param accelerator: name of the accelerator, such as "kvm" or "tcg"
+        :type accelerator: str
+        """
+        checker = {'tcg': tcg_available,
+                   'kvm': kvm_available}.get(accelerator)
+        if checker is None:
+            self.cancel("Don't know how to check for the presence "
+                        "of accelerator %s" % accelerator)
+        if not checker(qemu_bin=self.qemu_bin):
+            self.cancel("%s accelerator does not seem to be "
+                        "available" % accelerator)
+
     def setUp(self):
         self._vms = {}
 
@@ -206,3 +238,98 @@ class Test(avocado.Test):
                         expire=expire,
                         find_only=find_only,
                         cancel_on_missing=cancel_on_missing)
+
+
+class LinuxTest(Test):
+    """Facilitates having a cloud-image Linux based available.
+
+    For tests that indend to interact with guests, this is a better choice
+    to start with than the more vanilla `Test` class.
+    """
+
+    timeout = 900
+    chksum = None
+
+    def setUp(self, ssh_pubkey=None):
+        super(LinuxTest, self).setUp()
+        self.vm.add_args('-smp', '2')
+        self.vm.add_args('-m', '1024')
+        self.set_up_boot()
+        if ssh_pubkey is None:
+            ssh_pubkey, self.ssh_key = self.set_up_existing_ssh_keys()
+        self.set_up_cloudinit(ssh_pubkey)
+
+    def set_up_existing_ssh_keys(self):
+        ssh_public_key = os.path.join(SOURCE_DIR, 'tests', 'keys', 'id_rsa.pub')
+        source_private_key = os.path.join(SOURCE_DIR, 'tests', 'keys', 'id_rsa')
+        ssh_dir = os.path.join(self.workdir, '.ssh')
+        os.mkdir(ssh_dir, mode=0o700)
+        ssh_private_key = os.path.join(ssh_dir,
+                                       os.path.basename(source_private_key))
+        shutil.copyfile(source_private_key, ssh_private_key)
+        os.chmod(ssh_private_key, 0o600)
+        return (ssh_public_key, ssh_private_key)
+
+    def download_boot(self):
+        self.log.debug('Looking for and selecting a qemu-img binary to be '
+                       'used to create the bootable snapshot image')
+        # If qemu-img has been built, use it, otherwise the system wide one
+        # will be used.  If none is available, the test will cancel.
+        qemu_img = os.path.join(BUILD_DIR, 'qemu-img')
+        if not os.path.exists(qemu_img):
+            qemu_img = find_command('qemu-img', False)
+        if qemu_img is False:
+            self.cancel('Could not find "qemu-img", which is required to '
+                        'create the bootable image')
+        vmimage.QEMU_IMG = qemu_img
+
+        self.log.info('Downloading/preparing boot image')
+        # Fedora 31 only provides ppc64le images
+        image_arch = self.arch
+        if image_arch == 'ppc64':
+            image_arch = 'ppc64le'
+        try:
+            boot = vmimage.get(
+                'fedora', arch=image_arch, version='31',
+                checksum=self.chksum,
+                algorithm='sha256',
+                cache_dir=self.cache_dirs[0],
+                snapshot_dir=self.workdir)
+        except:
+            self.cancel('Failed to download/prepare boot image')
+        return boot.path
+
+    def prepare_cloudinit(self, ssh_pubkey=None):
+        self.log.info('Preparing cloudinit image')
+        try:
+            cloudinit_iso = os.path.join(self.workdir, 'cloudinit.iso')
+            self.phone_home_port = network.find_free_port()
+            with open(ssh_pubkey) as pubkey:
+                pubkey_content = pubkey.read()
+            cloudinit.iso(cloudinit_iso, self.name,
+                          username='root',
+                          password='password',
+                          # QEMU's hard coded usermode router address
+                          phone_home_host='10.0.2.2',
+                          phone_home_port=self.phone_home_port,
+                          authorized_key=pubkey_content)
+        except Exception:
+            self.cancel('Failed to prepare the cloudinit image')
+        return cloudinit_iso
+
+    def set_up_boot(self):
+        path = self.download_boot()
+        self.vm.add_args('-drive', 'file=%s' % path)
+
+    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):
+        self.vm.set_console()
+        self.vm.launch()
+        console_drainer = datadrainer.LineLogger(self.vm.console_socket.fileno(),
+                                                 logger=self.log.getChild('console'))
+        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)
diff --git a/tests/acceptance/boot_linux.py b/tests/acceptance/boot_linux.py
index bcd923b..0d17803 100644
--- a/tests/acceptance/boot_linux.py
+++ b/tests/acceptance/boot_linux.py
@@ -10,103 +10,12 @@
 
 import os
 
-from avocado_qemu import Test, BUILD_DIR
+from avocado_qemu import LinuxTest, BUILD_DIR
 
-from qemu.accel import kvm_available
-from qemu.accel import tcg_available
-
-from avocado.utils import cloudinit
-from avocado.utils import network
-from avocado.utils import vmimage
-from avocado.utils import datadrainer
-from avocado.utils.path import find_command
 from avocado import skipIf
 
-ACCEL_NOT_AVAILABLE_FMT = "%s accelerator does not seem to be available"
-KVM_NOT_AVAILABLE = ACCEL_NOT_AVAILABLE_FMT % "KVM"
-TCG_NOT_AVAILABLE = ACCEL_NOT_AVAILABLE_FMT % "TCG"
-
-
-class BootLinuxBase(Test):
-    def download_boot(self):
-        self.log.debug('Looking for and selecting a qemu-img binary to be '
-                       'used to create the bootable snapshot image')
-        # If qemu-img has been built, use it, otherwise the system wide one
-        # will be used.  If none is available, the test will cancel.
-        qemu_img = os.path.join(BUILD_DIR, 'qemu-img')
-        if not os.path.exists(qemu_img):
-            qemu_img = find_command('qemu-img', False)
-        if qemu_img is False:
-            self.cancel('Could not find "qemu-img", which is required to '
-                        'create the bootable image')
-        vmimage.QEMU_IMG = qemu_img
-
-        self.log.info('Downloading/preparing boot image')
-        # Fedora 31 only provides ppc64le images
-        image_arch = self.arch
-        if image_arch == 'ppc64':
-            image_arch = 'ppc64le'
-        try:
-            boot = vmimage.get(
-                'fedora', arch=image_arch, version='31',
-                checksum=self.chksum,
-                algorithm='sha256',
-                cache_dir=self.cache_dirs[0],
-                snapshot_dir=self.workdir)
-        except:
-            self.cancel('Failed to download/prepare boot image')
-        return boot.path
-
-    def prepare_cloudinit(self, ssh_pubkey=None):
-        self.log.info('Preparing cloudinit image')
-        try:
-            cloudinit_iso = os.path.join(self.workdir, 'cloudinit.iso')
-            self.phone_home_port = network.find_free_port()
-            cloudinit.iso(cloudinit_iso, self.name,
-                          username='root',
-                          password='password',
-                          # QEMU's hard coded usermode router address
-                          phone_home_host='10.0.2.2',
-                          phone_home_port=self.phone_home_port,
-                          authorized_key=ssh_pubkey)
-        except Exception:
-            self.cancel('Failed to prepare the cloudinit image')
-        return cloudinit_iso
-
-class BootLinux(BootLinuxBase):
-    """
-    Boots a Linux system, checking for a successful initialization
-    """
-
-    timeout = 900
-    chksum = None
-
-    def setUp(self, ssh_pubkey=None):
-        super(BootLinux, self).setUp()
-        self.vm.add_args('-smp', '2')
-        self.vm.add_args('-m', '1024')
-        self.set_up_boot()
-        self.set_up_cloudinit(ssh_pubkey)
-
-    def set_up_boot(self):
-        path = self.download_boot()
-        self.vm.add_args('-drive', 'file=%s' % path)
-
-    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):
-        self.vm.set_console()
-        self.vm.launch()
-        console_drainer = datadrainer.LineLogger(self.vm.console_socket.fileno(),
-                                                 logger=self.log.getChild('console'))
-        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)
-
 
-class BootLinuxX8664(BootLinux):
+class BootLinuxX8664(LinuxTest):
     """
     :avocado: tags=arch:x86_64
     """
@@ -118,8 +27,7 @@ class BootLinuxX8664(BootLinux):
         :avocado: tags=machine:pc
         :avocado: tags=accel:tcg
         """
-        if not tcg_available(self.qemu_bin):
-            self.cancel(TCG_NOT_AVAILABLE)
+        self.require_accelerator("tcg")
         self.vm.add_args("-accel", "tcg")
         self.launch_and_wait()
 
@@ -128,8 +36,7 @@ class BootLinuxX8664(BootLinux):
         :avocado: tags=machine:pc
         :avocado: tags=accel:kvm
         """
-        if not kvm_available(self.arch, self.qemu_bin):
-            self.cancel(KVM_NOT_AVAILABLE)
+        self.require_accelerator("kvm")
         self.vm.add_args("-accel", "kvm")
         self.launch_and_wait()
 
@@ -138,8 +45,7 @@ class BootLinuxX8664(BootLinux):
         :avocado: tags=machine:q35
         :avocado: tags=accel:tcg
         """
-        if not tcg_available(self.qemu_bin):
-            self.cancel(TCG_NOT_AVAILABLE)
+        self.require_accelerator("tcg")
         self.vm.add_args("-accel", "tcg")
         self.launch_and_wait()
 
@@ -148,13 +54,12 @@ class BootLinuxX8664(BootLinux):
         :avocado: tags=machine:q35
         :avocado: tags=accel:kvm
         """
-        if not kvm_available(self.arch, self.qemu_bin):
-            self.cancel(KVM_NOT_AVAILABLE)
+        self.require_accelerator("kvm")
         self.vm.add_args("-accel", "kvm")
         self.launch_and_wait()
 
 
-class BootLinuxAarch64(BootLinux):
+class BootLinuxAarch64(LinuxTest):
     """
     :avocado: tags=arch:aarch64
     :avocado: tags=machine:virt
@@ -175,8 +80,7 @@ class BootLinuxAarch64(BootLinux):
         :avocado: tags=accel:tcg
         :avocado: tags=cpu:max
         """
-        if not tcg_available(self.qemu_bin):
-            self.cancel(TCG_NOT_AVAILABLE)
+        self.require_accelerator("tcg")
         self.vm.add_args("-accel", "tcg")
         self.vm.add_args("-cpu", "max")
         self.vm.add_args("-machine", "virt,gic-version=2")
@@ -189,8 +93,7 @@ class BootLinuxAarch64(BootLinux):
         :avocado: tags=cpu:host
         :avocado: tags=device:gicv2
         """
-        if not kvm_available(self.arch, self.qemu_bin):
-            self.cancel(KVM_NOT_AVAILABLE)
+        self.require_accelerator("kvm")
         self.vm.add_args("-accel", "kvm")
         self.vm.add_args("-cpu", "host")
         self.vm.add_args("-machine", "virt,gic-version=2")
@@ -203,8 +106,7 @@ class BootLinuxAarch64(BootLinux):
         :avocado: tags=cpu:host
         :avocado: tags=device:gicv3
         """
-        if not kvm_available(self.arch, self.qemu_bin):
-            self.cancel(KVM_NOT_AVAILABLE)
+        self.require_accelerator("kvm")
         self.vm.add_args("-accel", "kvm")
         self.vm.add_args("-cpu", "host")
         self.vm.add_args("-machine", "virt,gic-version=3")
@@ -212,7 +114,7 @@ class BootLinuxAarch64(BootLinux):
         self.launch_and_wait()
 
 
-class BootLinuxPPC64(BootLinux):
+class BootLinuxPPC64(LinuxTest):
     """
     :avocado: tags=arch:ppc64
     """
@@ -224,13 +126,12 @@ class BootLinuxPPC64(BootLinux):
         :avocado: tags=machine:pseries
         :avocado: tags=accel:tcg
         """
-        if not tcg_available(self.qemu_bin):
-            self.cancel(TCG_NOT_AVAILABLE)
+        self.require_accelerator("tcg")
         self.vm.add_args("-accel", "tcg")
         self.launch_and_wait()
 
 
-class BootLinuxS390X(BootLinux):
+class BootLinuxS390X(LinuxTest):
     """
     :avocado: tags=arch:s390x
     """
@@ -243,7 +144,6 @@ class BootLinuxS390X(BootLinux):
         :avocado: tags=machine:s390-ccw-virtio
         :avocado: tags=accel:tcg
         """
-        if not tcg_available(self.qemu_bin):
-            self.cancel(TCG_NOT_AVAILABLE)
+        self.require_accelerator("tcg")
         self.vm.add_args("-accel", "tcg")
         self.launch_and_wait()
diff --git a/tests/acceptance/virtio-gpu.py b/tests/acceptance/virtio-gpu.py
index 211f029..ab1a4c1 100644
--- a/tests/acceptance/virtio-gpu.py
+++ b/tests/acceptance/virtio-gpu.py
@@ -119,10 +119,11 @@ class VirtioGPUx86(Test):
         os.set_inheritable(vug_sock.fileno(), True)
 
         self._vug_log_path = os.path.join(
-            self.vm._test_dir, "vhost-user-gpu.log"
+            self.logdir, "vhost-user-gpu.log"
         )
         self._vug_log_file = open(self._vug_log_path, "wb")
-        print(self._vug_log_path)
+        self.log.info('Complete vhost-user-gpu.log file can be '
+                      'found at %s', self._vug_log_path)
 
         vugp = subprocess.Popen(
             [vug, "--virgl", "--fd=%d" % vug_sock.fileno()],
diff --git a/tests/acceptance/virtiofs_submounts.py b/tests/acceptance/virtiofs_submounts.py
index 949ca87..46fa653 100644
--- a/tests/acceptance/virtiofs_submounts.py
+++ b/tests/acceptance/virtiofs_submounts.py
@@ -5,14 +5,10 @@ import subprocess
 import time
 
 from avocado import skipUnless
-from avocado_qemu import Test, BUILD_DIR
+from avocado_qemu import LinuxTest, BUILD_DIR
 from avocado_qemu import wait_for_console_pattern
 from avocado.utils import ssh
 
-from qemu.accel import kvm_available
-
-from boot_linux import BootLinux
-
 
 def run_cmd(args):
     subp = subprocess.Popen(args,
@@ -71,7 +67,7 @@ def has_cmds(*cmds):
     return (True, '')
 
 
-class VirtiofsSubmountsTest(BootLinux):
+class VirtiofsSubmountsTest(LinuxTest):
     """
     :avocado: tags=arch:x86_64
     """
@@ -228,6 +224,18 @@ class VirtiofsSubmountsTest(BootLinux):
     def setUp(self):
         vmlinuz = self.params.get('vmlinuz')
         if vmlinuz is None:
+            """
+            The Linux kernel supports FUSE auto-submounts only as of 5.10.
+            boot_linux.py currently provides Fedora 31, whose kernel is too
+            old, so this test cannot pass with the on-image kernel (you are
+            welcome to try, hence the option to force such a test with
+            -p vmlinuz='').  Therefore, for now the user must provide a
+            sufficiently new custom kernel, or effectively explicitly
+            request failure with -p vmlinuz=''.
+            Once an image with a sufficiently new kernel is available
+            (probably Fedora 34), we can make -p vmlinuz='' the default, so
+            that this parameter no longer needs to be specified.
+            """
             self.cancel('vmlinuz parameter not set; you must point it to a '
                         'Linux kernel binary to test (to run this test with ' \
                         'the on-image kernel, set it to an empty string)')
@@ -250,8 +258,7 @@ class VirtiofsSubmountsTest(BootLinux):
         self.vm.add_args('-netdev', 'user,id=vnet,hostfwd=:127.0.0.1:0-:22',
                          '-device', 'virtio-net,netdev=vnet')
 
-        if not kvm_available(self.arch, self.qemu_bin):
-            self.cancel(KVM_NOT_AVAILABLE)
+        self.require_accelerator("kvm")
         self.vm.add_args('-accel', 'kvm')
 
     def tearDown(self):
diff --git a/tests/requirements.txt b/tests/requirements.txt
index 62e8ffd..91f3a34 100644
--- a/tests/requirements.txt
+++ b/tests/requirements.txt
@@ -1,5 +1,5 @@
 # Add Python module requirements, one per line, to be installed
 # in the tests/venv Python virtual environment. For more info,
 # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
-avocado-framework==83.0
+avocado-framework==85.0
 pycdlib==1.11.0