qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 0/8] Acceptance test: Add "boot_linux" acceptance test
@ 2019-11-04 15:13 Cleber Rosa
  2019-11-04 15:13 ` [PATCH v7 1/8] Acceptance test x86_cpu_model_versions: use default vm Cleber Rosa
                   ` (9 more replies)
  0 siblings, 10 replies; 39+ messages in thread
From: Cleber Rosa @ 2019-11-04 15:13 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Wainer dos Santos Moschetta,
	Willian Rampazzo, Cleber Rosa, qemu-ppc, Aleksandar Rikalo,
	Philippe Mathieu-Daudé,
	Aurelien Jarno, Eduardo Habkost

This acceptance test, validates that a full blown Linux guest can
successfully boot in QEMU.  In this specific case, the guest chosen is
Fedora version 31.

 * x86_64, pc and q35 machine types, with and without kvm as an
   accellerator

 * aarch64 and virt machine type, with and without kvm as an
   accellerator

 * ppc64 and pseries machine type

 * s390x and s390-ccw-virtio machine type

This has been tested on x86_64 and ppc64le hosts and has been running
reliably (in my experience) on Travis CI.

Some hopefully useful pointers for reviewers:
=============================================

Git Info:
  - URI: https://github.com/clebergnu/qemu/tree/test_boot_linux_v7
  - Remote: https://github.com/clebergnu/qemu
  - Branch: test_boot_linux_v7

Travis CI Info:
  - Build: https://travis-ci.org/clebergnu/qemu/builds/606191094
  - Job: https://travis-ci.org/clebergnu/qemu/jobs/606191120

Previous version:
  - v6: https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg01202.html
  - v5: https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg04652.html
  - v4: https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg02032.html
  - v3: https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg01677.html
  - v2: https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg04318.html
  - v1: http://lists.nongnu.org/archive/html/qemu-devel/2018-09/msg02530.html

Changes from v6:
================

 * Bumped Fedora to most recently released version (31).

 * Included new architectures (ppc64 and s390x), consolidating all
   tests into the same commit.

 * New commit: "Acceptance tests: use avocado tags for machine type"

 * New commit: "Acceptance tests: introduce utility method for tags
   unique vals"

 * New commit: "Acceptance test x86_cpu_model_versions: use default
   vm", needed to normalize the use of the machine type tags

 * Added a lot of leniency to the test setup (and reliability to the
   test/job), canceling the test if there are any failures while
   downloading/preparing the boot images.

 * Made use of Avocado's data drainer a regular feature (dropped the
   commit with RFC) and squashed it.

 * Bumped pycdlib version to 1.8.0

 * Dropped explicit "--enable-slirp=git" (added on v5) to Travis CI
   configure line, as the default configuration on Travis CI now
   results in user networking capabilities.

Changes from v5:
================

 * Added explicit "--enable-slirp=git" to Travis CI configure line, as
   these tests depend on "-netdev user" like networking.

 * Bumped Fedora to most recently released version (30).

 * Changed "checksum" parameter to 'sha256' and use the same hashes as
   provided by the Fedora project (instead of using Avocado's default
   sha1 and compute and use a different hash value).

 * New commit: Add "boot_linux" test for aarch64 and virt machine type

 * New commit: [RFC]: use Avocado data drainer for console logging

Changes from v4:
================

 * New commit "Acceptance tests: use relative location for tests"

 * New commit "Acceptance tests: keep a stable reference to the QEMU build dir"

 * Pinned the Fedora 29 image by adding a checksum.  The goal is to
   never allow more than one component to change at a time (the one
   allowed to change is QEMU itself).  Updates to the image should be
   manual. (Based on comments from Cornelia)

 * Moved the downloading of the Fedora 29 cloud image to the test
   setUp() method, canceling the test if the image can not be
   downloaded.

 * Removed the ":avocado: enable" tag, given that Avocado versions
   68.0 and later operate on a "recursive by default" manner, that
   is able to correctly identify this as an Avocado test.

Changes from v3:
================

 * New patch "Acceptance tests: depend on qemu-img"

Known Issues on v3 (no longer applicable):
==========================================

 * A recent TCG performance regression[1] affects this test in a
   number of ways:
   - The test execution may timeout by itself
   - The generation of SSH host keys in the guest's first boot is also
     affected (possibly also a timeout)
   - The cloud-init "phone home" feature attempts to read the host keys
     and fails, causing the test to timeout and fail

   These are not observed anymore once the fix[2] is applied.

[1] - https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg00338.html
[2] - https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg01129.html

Changes from v2:
================

 * Updated the tag to include the "arch:" key, in a similar fashion as to
   the tests in the "Acceptance Tests: target architecture support":
   - https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg00369.html

 * Renamed the test method name to test_x86_64_pc, again, similarly to the
   boot_linux_console.py tests in the series mentioned before.

 * Set the machine type explicitly, again similarly to the
   boot_linux_console.py tests in the series mentioned before.

 * Added messages after the launch of the VM, to let test runners know
   the test know waits for a boot confirmation from the the guest (Eduardo).

 * Updated commit message to reflect the fact that this version does
   not allow for parameterization of the guest OS, version, etc.

 * Dropped the RFC prefix on patch "RFC: Acceptance tests: add the
   build directory to the system PATH"

 * Changed the comments on "RFC: Acceptance tests: add the build
   directory to the system PATH" to make it clear the addition of a
   the build directory to the PATH may influence other utility code.

Changes from v1:
================

 * The commit message was adjusted, removing the reference to the
   avocado.utils.vmimage encoding issue on previous Avocado versions
   (<= 64.0) and the fix that would (and was) included in Avocado
   version 65.0.

 * Effectively added pycdlib==1.6.0 to the requirements.txt file,
   added on a56931eef3, and adjusted the commit message was also
   to reflect that.

 * Updated the default version of the guest OS, from Fedora 28 to 29.
   Besides possible improvements in the (virtual) hardware coverage,
   it brings a performance improvement in the order of 20% to the
   test.

 * Removed all direct parameters usage.  Because some parameters and
   its default values implemented in the test would prevent it from
   running on some environments.  Example: the "accel" parameter had a
   default value of "kvm", which would prevent this test, that boots a
   x86_64 OS, from running on a host arch different than x86_64.  I
   recognize that it's desirable to make tests reusable and
   parameterized (that was the reason for the first version doing so),
   but the mechanism to be used to define the architectures that a
   given test should support is still an open issue, and has been
   discussed in other threads.  I'll follow up those discussions with
   a proposal, and until then, removing those aspects from this test
   implementation seemed to be the best option.  A caveat: this test
   currently adds the same tag (x86_64) and follows other assumptions
   made on "boot_linux_console.py", that is, that a x86_64 target
   binary will be used to run it.  If a user is in an environment that
   does not have a x86_64 target binary, it could filter those tests
   out with: "avocado run --filter-by-tags='-x86_64' tests/acceptance".

 * Removed most arguments to the QEMU command line for pretty much the
   same reasons described above, and by following the general
   perception that I could grasp from other discussions that QEMU
   defaults should preferrably be used.  This test, as well as others,
   can and should be extended later to allow for different test
   scenarios by passing well documented parameter values.  That is,
   they should respect well-known parameters such as "accel" mentioned
   above, so that the same test can run with KVM or TCG.

 * Changed the value of the memory argument to 1024, which based on
   my experimentations and observations is the minimum amount of RAM
   for the Fedora 29 cloud image to sucessfully boot on QEMU.  I know
   there's no such thing as a "one size fits all", specially for QEMU,
   but this makes me wonder wether a x86_64 machine type shouldn't
   have its default_ram_size bumped to a number practical enough to
   run modern operating systems.

 * Added a new patch "RFC: Acceptance tests: add the build directory
   to the system PATH", which is supposed to gather feedback on how to
   enable the use of built binaries, such as qemu-img, to code used by
   the test code.  The specific situation here is that the vmimage,
   part of the avocado.utils libraries, makes use of qemu-img to create
   snapshot files.  Even though we could require qemu-img to be installed
   as a dependency of tests, system wide, it actually goes against the
   goal of testing all QEMU things from the source/build tree.  This
   became aparent with tests running on environments such as Travis CI,
   which don't necessarily have qemu-img available elsewhere.

Cleber Rosa (8):
  Acceptance test x86_cpu_model_versions: use default vm
  Acceptance tests: introduce utility method for tags unique vals
  Acceptance tests: use avocado tags for machine type
  Acceptance tests: use relative location for tests
  Acceptance tests: keep a stable reference to the QEMU build dir
  Acceptance tests: add the build directory to the system PATH
  Acceptance tests: depend on qemu-img
  Acceptance test: add "boot_linux" tests

 docs/devel/testing.rst                     |  18 +++
 tests/Makefile.include                     |   4 +-
 tests/acceptance/avocado_qemu/__init__.py  |  32 +++-
 tests/acceptance/boot_linux.py             | 175 +++++++++++++++++++++
 tests/acceptance/boot_linux_console.py     |  19 +--
 tests/acceptance/cpu_queries.py            |   2 +-
 tests/acceptance/linux_initrd.py           |   2 +-
 tests/acceptance/linux_ssh_mips_malta.py   |   5 -
 tests/acceptance/machine_m68k_nextcube.py  |  21 +--
 tests/acceptance/machine_sparc_leon3.py    |   3 +-
 tests/acceptance/ppc_prep_40p.py           |   3 -
 tests/acceptance/x86_cpu_model_versions.py | 137 +++++++++-------
 tests/requirements.txt                     |   1 +
 13 files changed, 308 insertions(+), 114 deletions(-)
 create mode 100644 tests/acceptance/boot_linux.py

-- 
2.21.0



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

* [PATCH v7 1/8] Acceptance test x86_cpu_model_versions: use default vm
  2019-11-04 15:13 [PATCH v7 0/8] Acceptance test: Add "boot_linux" acceptance test Cleber Rosa
@ 2019-11-04 15:13 ` Cleber Rosa
  2019-11-04 18:22   ` Philippe Mathieu-Daudé
  2019-11-04 15:13 ` [PATCH v7 2/8] Acceptance tests: introduce utility method for tags unique vals Cleber Rosa
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 39+ messages in thread
From: Cleber Rosa @ 2019-11-04 15:13 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Wainer dos Santos Moschetta,
	Willian Rampazzo, Cleber Rosa, qemu-ppc, Aleksandar Rikalo,
	Philippe Mathieu-Daudé,
	Aurelien Jarno, Eduardo Habkost

The default vm provided by the test, available as self.vm, serves the
same purpose of the one obtained by self.get_vm(), but saves a line
and matches the style of other tests.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/acceptance/x86_cpu_model_versions.py | 100 ++++++++++-----------
 1 file changed, 46 insertions(+), 54 deletions(-)

diff --git a/tests/acceptance/x86_cpu_model_versions.py b/tests/acceptance/x86_cpu_model_versions.py
index 5fc9ca4bc6..6eb977954d 100644
--- a/tests/acceptance/x86_cpu_model_versions.py
+++ b/tests/acceptance/x86_cpu_model_versions.py
@@ -25,10 +25,6 @@
 import avocado_qemu
 import re
 
-def get_cpu_prop(vm, prop):
-    cpu_path = vm.command('query-cpus')[0].get('qom_path')
-    return vm.command('qom-get', path=cpu_path, property=prop)
-
 class X86CPUModelAliases(avocado_qemu.Test):
     """
     Validation of PC CPU model versions and CPU model aliases
@@ -241,78 +237,74 @@ class CascadelakeArchCapabilities(avocado_qemu.Test):
 
     :avocado: tags=arch:x86_64
     """
+    def get_cpu_prop(self, prop):
+        cpu_path = self.vm.command('query-cpus')[0].get('qom_path')
+        return self.vm.command('qom-get', path=cpu_path, property=prop)
+
     def test_4_1(self):
         # machine-type only:
-        vm = self.get_vm()
-        vm.add_args('-S')
-        vm.set_machine('pc-i440fx-4.1')
-        vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off')
-        vm.launch()
-        self.assertFalse(get_cpu_prop(vm, 'arch-capabilities'),
+        self.vm.add_args('-S')
+        self.vm.set_machine('pc-i440fx-4.1')
+        self.vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off')
+        self.vm.launch()
+        self.assertFalse(self.get_cpu_prop('arch-capabilities'),
                          'pc-i440fx-4.1 + Cascadelake-Server should not have arch-capabilities')
 
     def test_4_0(self):
-        vm = self.get_vm()
-        vm.add_args('-S')
-        vm.set_machine('pc-i440fx-4.0')
-        vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off')
-        vm.launch()
-        self.assertFalse(get_cpu_prop(vm, 'arch-capabilities'),
+        self.vm.add_args('-S')
+        self.vm.set_machine('pc-i440fx-4.0')
+        self.vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off')
+        self.vm.launch()
+        self.assertFalse(self.get_cpu_prop('arch-capabilities'),
                          'pc-i440fx-4.0 + Cascadelake-Server should not have arch-capabilities')
 
     def test_set_4_0(self):
         # command line must override machine-type if CPU model is not versioned:
-        vm = self.get_vm()
-        vm.add_args('-S')
-        vm.set_machine('pc-i440fx-4.0')
-        vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off,+arch-capabilities')
-        vm.launch()
-        self.assertTrue(get_cpu_prop(vm, 'arch-capabilities'),
+        self.vm.add_args('-S')
+        self.vm.set_machine('pc-i440fx-4.0')
+        self.vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off,+arch-capabilities')
+        self.vm.launch()
+        self.assertTrue(self.get_cpu_prop('arch-capabilities'),
                         'pc-i440fx-4.0 + Cascadelake-Server,+arch-capabilities should have arch-capabilities')
 
     def test_unset_4_1(self):
-        vm = self.get_vm()
-        vm.add_args('-S')
-        vm.set_machine('pc-i440fx-4.1')
-        vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off,-arch-capabilities')
-        vm.launch()
-        self.assertFalse(get_cpu_prop(vm, 'arch-capabilities'),
+        self.vm.add_args('-S')
+        self.vm.set_machine('pc-i440fx-4.1')
+        self.vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off,-arch-capabilities')
+        self.vm.launch()
+        self.assertFalse(self.get_cpu_prop('arch-capabilities'),
                          'pc-i440fx-4.1 + Cascadelake-Server,-arch-capabilities should not have arch-capabilities')
 
     def test_v1_4_0(self):
         # versioned CPU model overrides machine-type:
-        vm = self.get_vm()
-        vm.add_args('-S')
-        vm.set_machine('pc-i440fx-4.0')
-        vm.add_args('-cpu', 'Cascadelake-Server-v1,x-force-features=on,check=off,enforce=off')
-        vm.launch()
-        self.assertFalse(get_cpu_prop(vm, 'arch-capabilities'),
+        self.vm.add_args('-S')
+        self.vm.set_machine('pc-i440fx-4.0')
+        self.vm.add_args('-cpu', 'Cascadelake-Server-v1,x-force-features=on,check=off,enforce=off')
+        self.vm.launch()
+        self.assertFalse(self.get_cpu_prop('arch-capabilities'),
                          'pc-i440fx-4.0 + Cascadelake-Server-v1 should not have arch-capabilities')
 
     def test_v2_4_0(self):
-        vm = self.get_vm()
-        vm.add_args('-S')
-        vm.set_machine('pc-i440fx-4.0')
-        vm.add_args('-cpu', 'Cascadelake-Server-v2,x-force-features=on,check=off,enforce=off')
-        vm.launch()
-        self.assertTrue(get_cpu_prop(vm, 'arch-capabilities'),
-                         'pc-i440fx-4.0 + Cascadelake-Server-v2 should have arch-capabilities')
+        self.vm.add_args('-S')
+        self.vm.set_machine('pc-i440fx-4.0')
+        self.vm.add_args('-cpu', 'Cascadelake-Server-v2,x-force-features=on,check=off,enforce=off')
+        self.vm.launch()
+        self.assertTrue(self.get_cpu_prop('arch-capabilities'),
+                        'pc-i440fx-4.0 + Cascadelake-Server-v2 should have arch-capabilities')
 
     def test_v1_set_4_0(self):
         # command line must override machine-type and versioned CPU model:
-        vm = self.get_vm()
-        vm.add_args('-S')
-        vm.set_machine('pc-i440fx-4.0')
-        vm.add_args('-cpu', 'Cascadelake-Server-v1,x-force-features=on,check=off,enforce=off,+arch-capabilities')
-        vm.launch()
-        self.assertTrue(get_cpu_prop(vm, 'arch-capabilities'),
-                         'pc-i440fx-4.0 + Cascadelake-Server-v1,+arch-capabilities should have arch-capabilities')
+        self.vm.add_args('-S')
+        self.vm.set_machine('pc-i440fx-4.0')
+        self.vm.add_args('-cpu', 'Cascadelake-Server-v1,x-force-features=on,check=off,enforce=off,+arch-capabilities')
+        self.vm.launch()
+        self.assertTrue(self.get_cpu_prop('arch-capabilities'),
+                        'pc-i440fx-4.0 + Cascadelake-Server-v1,+arch-capabilities should have arch-capabilities')
 
     def test_v2_unset_4_1(self):
-        vm = self.get_vm()
-        vm.add_args('-S')
-        vm.set_machine('pc-i440fx-4.1')
-        vm.add_args('-cpu', 'Cascadelake-Server-v2,x-force-features=on,check=off,enforce=off,-arch-capabilities')
-        vm.launch()
-        self.assertFalse(get_cpu_prop(vm, 'arch-capabilities'),
+        self.vm.add_args('-S')
+        self.vm.set_machine('pc-i440fx-4.1')
+        self.vm.add_args('-cpu', 'Cascadelake-Server-v2,x-force-features=on,check=off,enforce=off,-arch-capabilities')
+        self.vm.launch()
+        self.assertFalse(self.get_cpu_prop('arch-capabilities'),
                          'pc-i440fx-4.1 + Cascadelake-Server-v2,-arch-capabilities should not have arch-capabilities')
-- 
2.21.0



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

* [PATCH v7 2/8] Acceptance tests: introduce utility method for tags unique vals
  2019-11-04 15:13 [PATCH v7 0/8] Acceptance test: Add "boot_linux" acceptance test Cleber Rosa
  2019-11-04 15:13 ` [PATCH v7 1/8] Acceptance test x86_cpu_model_versions: use default vm Cleber Rosa
@ 2019-11-04 15:13 ` Cleber Rosa
  2019-11-08 13:14   ` Philippe Mathieu-Daudé
  2019-11-04 15:13 ` [PATCH v7 3/8] Acceptance tests: use avocado tags for machine type Cleber Rosa
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 39+ messages in thread
From: Cleber Rosa @ 2019-11-04 15:13 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Wainer dos Santos Moschetta,
	Willian Rampazzo, Cleber Rosa, qemu-ppc, Aleksandar Rikalo,
	Philippe Mathieu-Daudé,
	Aurelien Jarno, Eduardo Habkost

Currently a test can describe the target architecture binary that it
should primarily be run with, be setting a single tag value.

The same approach is expected to be done with other QEMU aspects to be
tested, for instance, the machine type and accelerator, so let's
generalize the logic into a utility method.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/acceptance/avocado_qemu/__init__.py | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
index 9a57c020d8..e676d9c4e7 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -100,14 +100,21 @@ def exec_command_and_wait_for_pattern(test, command,
 
 
 class Test(avocado.Test):
+    def _get_unique_tag_val(self, tag_name):
+        """
+        Gets a tag value, if unique for a key
+        """
+        vals = self.tags.get(tag_name, [])
+        if len(vals) == 1:
+            return vals.pop()
+        return None
+
     def setUp(self):
         self._vms = {}
-        arches = self.tags.get('arch', [])
-        if len(arches) == 1:
-            arch = arches.pop()
-        else:
-            arch = None
-        self.arch = self.params.get('arch', default=arch)
+
+        self.arch = self.params.get('arch',
+                                    default=self._get_unique_tag_val('arch'))
+
         default_qemu_bin = pick_default_qemu_bin(arch=self.arch)
         self.qemu_bin = self.params.get('qemu_bin',
                                         default=default_qemu_bin)
-- 
2.21.0



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

* [PATCH v7 3/8] Acceptance tests: use avocado tags for machine type
  2019-11-04 15:13 [PATCH v7 0/8] Acceptance test: Add "boot_linux" acceptance test Cleber Rosa
  2019-11-04 15:13 ` [PATCH v7 1/8] Acceptance test x86_cpu_model_versions: use default vm Cleber Rosa
  2019-11-04 15:13 ` [PATCH v7 2/8] Acceptance tests: introduce utility method for tags unique vals Cleber Rosa
@ 2019-11-04 15:13 ` Cleber Rosa
  2019-11-08 13:20   ` Philippe Mathieu-Daudé
  2019-11-04 15:13 ` [PATCH v7 4/8] Acceptance tests: use relative location for tests Cleber Rosa
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 39+ messages in thread
From: Cleber Rosa @ 2019-11-04 15:13 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Wainer dos Santos Moschetta,
	Willian Rampazzo, Cleber Rosa, qemu-ppc, Aleksandar Rikalo,
	Philippe Mathieu-Daudé,
	Aurelien Jarno, Eduardo Habkost

The same way the arch tag is being used as a fallback for the arch
parameter, let's do the same for QEMU's machine and avoid some boiler
plate code.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 docs/devel/testing.rst                     | 18 ++++++++
 tests/acceptance/avocado_qemu/__init__.py  |  5 ++
 tests/acceptance/boot_linux_console.py     | 19 +-------
 tests/acceptance/cpu_queries.py            |  2 +-
 tests/acceptance/linux_initrd.py           |  2 +-
 tests/acceptance/linux_ssh_mips_malta.py   |  5 --
 tests/acceptance/machine_m68k_nextcube.py  | 21 ++-------
 tests/acceptance/machine_sparc_leon3.py    |  3 +-
 tests/acceptance/ppc_prep_40p.py           |  3 --
 tests/acceptance/x86_cpu_model_versions.py | 53 ++++++++++++++++------
 10 files changed, 72 insertions(+), 59 deletions(-)

diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index 8e981e062d..27f286858a 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -746,6 +746,17 @@ name.  If one is not given explicitly, it will either be set to
 ``None``, or, if the test is tagged with one (and only one)
 ``:avocado: tags=arch:VALUE`` tag, it will be set to ``VALUE``.
 
+machine
+~~~~~~~
+
+The machine type that will be set to all QEMUMachine instances created
+by the test.
+
+The ``machine`` attribute will be set to the test parameter of the same
+name.  If one is not given explicitly, it will either be set to
+``None``, or, if the test is tagged with one (and only one)
+``:avocado: tags=machine:VALUE`` tag, it will be set to ``VALUE``.
+
 qemu_bin
 ~~~~~~~~
 
@@ -781,6 +792,13 @@ architecture of a kernel or disk image to boot a VM with.
 This parameter has a direct relation with the ``arch`` attribute.  If
 not given, it will default to None.
 
+machine
+~~~~~~~
+
+The machine type that will be set to all QEMUMachine instances created
+by the test.
+
+
 qemu_bin
 ~~~~~~~~
 
diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
index e676d9c4e7..6618ea67c1 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -115,6 +115,9 @@ class Test(avocado.Test):
         self.arch = self.params.get('arch',
                                     default=self._get_unique_tag_val('arch'))
 
+        self.machine = self.params.get('machine',
+                                       default=self._get_unique_tag_val('machine'))
+
         default_qemu_bin = pick_default_qemu_bin(arch=self.arch)
         self.qemu_bin = self.params.get('qemu_bin',
                                         default=default_qemu_bin)
@@ -136,6 +139,8 @@ class Test(avocado.Test):
             name = str(uuid.uuid4())
         if self._vms.get(name) is None:
             self._vms[name] = self._new_vm(*args)
+            if self.machine is not None:
+                self._vms[name].set_machine(self.machine)
         return self._vms[name]
 
     def tearDown(self):
diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
index 7e41cebd47..6732cc59ca 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -62,7 +62,6 @@ class BootLinuxConsole(Test):
         kernel_hash = '23bebd2680757891cf7adedb033532163a792495'
         kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
 
-        self.vm.set_machine('pc')
         self.vm.set_console()
         kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE + 'console=ttyS0'
         self.vm.add_args('-kernel', kernel_path,
@@ -85,7 +84,6 @@ class BootLinuxConsole(Test):
         kernel_path = self.extract_from_deb(deb_path,
                                             '/boot/vmlinux-2.6.32-5-4kc-malta')
 
-        self.vm.set_machine('malta')
         self.vm.set_console()
         kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE + 'console=ttyS0'
         self.vm.add_args('-kernel', kernel_path,
@@ -118,7 +116,6 @@ class BootLinuxConsole(Test):
         kernel_path = self.extract_from_deb(deb_path,
                                             '/boot/vmlinux-2.6.32-5-5kc-malta')
 
-        self.vm.set_machine('malta')
         self.vm.set_console()
         kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE + 'console=ttyS0'
         self.vm.add_args('-kernel', kernel_path,
@@ -148,7 +145,6 @@ class BootLinuxConsole(Test):
         initrd_path = self.workdir + "rootfs.cpio"
         archive.gzip_uncompress(initrd_path_gz, initrd_path)
 
-        self.vm.set_machine('malta')
         self.vm.set_console()
         kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE
                                + 'console=ttyS0 console=tty '
@@ -188,7 +184,6 @@ class BootLinuxConsole(Test):
         initrd_path = self.workdir + "rootfs.cpio"
         archive.gzip_uncompress(initrd_path_gz, initrd_path)
 
-        self.vm.set_machine('malta')
         self.vm.set_console()
         kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE
                                + 'console=ttyS0 console=tty '
@@ -215,7 +210,6 @@ class BootLinuxConsole(Test):
             with open(kernel_path, 'wb') as f_out:
                 shutil.copyfileobj(f_in, f_out)
 
-        self.vm.set_machine('malta')
         self.vm.set_console()
         kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE
                                + 'mem=256m@@0x0 '
@@ -275,7 +269,6 @@ class BootLinuxConsole(Test):
         kernel_hash = '8c73e469fc6ea06a58dc83a628fc695b693b8493'
         kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
 
-        self.vm.set_machine('virt')
         self.vm.set_console()
         kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
                                'console=ttyAMA0')
@@ -297,7 +290,6 @@ class BootLinuxConsole(Test):
         kernel_hash = 'e9826d741b4fb04cadba8d4824d1ed3b7fb8b4d4'
         kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
 
-        self.vm.set_machine('virt')
         self.vm.set_console()
         kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
                                'console=ttyAMA0')
@@ -310,7 +302,7 @@ class BootLinuxConsole(Test):
     def test_arm_emcraft_sf2(self):
         """
         :avocado: tags=arch:arm
-        :avocado: tags=machine:emcraft_sf2
+        :avocado: tags=machine:emcraft-sf2
         :avocado: tags=endian:little
         """
         uboot_url = ('https://raw.githubusercontent.com/'
@@ -324,7 +316,6 @@ class BootLinuxConsole(Test):
         spi_hash = '85f698329d38de63aea6e884a86fbde70890a78a'
         spi_path = self.fetch_asset(spi_url, asset_hash=spi_hash)
 
-        self.vm.set_machine('emcraft-sf2')
         self.vm.set_console()
         kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE
         self.vm.add_args('-kernel', uboot_path,
@@ -351,7 +342,6 @@ class BootLinuxConsole(Test):
         kernel_path = self.extract_from_deb(deb_path, '/boot/kernel7.img')
         dtb_path = self.extract_from_deb(deb_path, '/boot/bcm2709-rpi-2-b.dtb')
 
-        self.vm.set_machine('raspi2')
         self.vm.set_console()
         kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
                                serial_kernel_cmdline[uart_id])
@@ -393,7 +383,6 @@ class BootLinuxConsole(Test):
         initrd_path = os.path.join(self.workdir, 'rootfs.cpio')
         archive.gzip_uncompress(initrd_path_gz, initrd_path)
 
-        self.vm.set_machine('smdkc210')
         self.vm.set_console()
         kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
                                'earlycon=exynos4210,0x13800000 earlyprintk ' +
@@ -414,7 +403,7 @@ class BootLinuxConsole(Test):
     def test_s390x_s390_ccw_virtio(self):
         """
         :avocado: tags=arch:s390x
-        :avocado: tags=machine:s390_ccw_virtio
+        :avocado: tags=machine:s390-ccw-virtio
         """
         kernel_url = ('https://archives.fedoraproject.org/pub/archive'
                       '/fedora-secondary/releases/29/Everything/s390x/os/images'
@@ -422,7 +411,6 @@ class BootLinuxConsole(Test):
         kernel_hash = 'e8e8439103ef8053418ef062644ffd46a7919313'
         kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
 
-        self.vm.set_machine('s390-ccw-virtio')
         self.vm.set_console()
         kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE + 'console=sclp0'
         self.vm.add_args('-nodefaults',
@@ -444,7 +432,6 @@ class BootLinuxConsole(Test):
 
         uncompressed_kernel = archive.uncompress(kernel_path, self.workdir)
 
-        self.vm.set_machine('clipper')
         self.vm.set_console()
         kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE + 'console=ttyS0'
         self.vm.add_args('-vga', 'std',
@@ -465,7 +452,6 @@ class BootLinuxConsole(Test):
         kernel_hash = '3fe04abfc852b66653b8c3c897a59a689270bc77'
         kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
 
-        self.vm.set_machine('pseries')
         self.vm.set_console()
         kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE + 'console=hvc0'
         self.vm.add_args('-kernel', kernel_path,
@@ -489,7 +475,6 @@ class BootLinuxConsole(Test):
         kernel_path = self.extract_from_deb(deb_path,
                                             '/boot/vmlinux-5.3.0-1-m68k')
 
-        self.vm.set_machine('q800')
         self.vm.set_console()
         kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
                                'console=ttyS0 vga=off')
diff --git a/tests/acceptance/cpu_queries.py b/tests/acceptance/cpu_queries.py
index af47d2795a..293dccb89a 100644
--- a/tests/acceptance/cpu_queries.py
+++ b/tests/acceptance/cpu_queries.py
@@ -20,8 +20,8 @@ class QueryCPUModelExpansion(Test):
     def test(self):
         """
         :avocado: tags=arch:x86_64
+        :avocado: tags=machine:none
         """
-        self.vm.set_machine('none')
         self.vm.add_args('-S')
         self.vm.launch()
 
diff --git a/tests/acceptance/linux_initrd.py b/tests/acceptance/linux_initrd.py
index c61d9826a4..3a0ff7b098 100644
--- a/tests/acceptance/linux_initrd.py
+++ b/tests/acceptance/linux_initrd.py
@@ -20,6 +20,7 @@ class LinuxInitrd(Test):
     Checks QEMU evaluates correctly the initrd file passed as -initrd option.
 
     :avocado: tags=arch:x86_64
+    :avocado: tags=machine:pc
     """
 
     timeout = 300
@@ -66,7 +67,6 @@ class LinuxInitrd(Test):
             initrd.write(b'\0')
             initrd.flush()
 
-            self.vm.set_machine('pc')
             self.vm.set_console()
             kernel_command_line = 'console=ttyS0'
             self.vm.add_args('-kernel', kernel_path,
diff --git a/tests/acceptance/linux_ssh_mips_malta.py b/tests/acceptance/linux_ssh_mips_malta.py
index fc13f9e4d4..1d570deb00 100644
--- a/tests/acceptance/linux_ssh_mips_malta.py
+++ b/tests/acceptance/linux_ssh_mips_malta.py
@@ -111,7 +111,6 @@ class LinuxSSH(Test):
         image_url, image_hash = self.get_image_info(endianess)
         image_path = self.fetch_asset(image_url, asset_hash=image_hash)
 
-        self.vm.set_machine('malta')
         self.vm.set_console()
         kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE
                                + 'console=ttyS0 root=/dev/sda1')
@@ -215,7 +214,6 @@ class LinuxSSH(Test):
     def test_mips_malta32eb_kernel3_2_0(self):
         """
         :avocado: tags=arch:mips
-        :avocado: tags=machine:malta
         :avocado: tags=endian:big
         :avocado: tags=device:pcnet32
         """
@@ -224,7 +222,6 @@ class LinuxSSH(Test):
     def test_mips_malta32el_kernel3_2_0(self):
         """
         :avocado: tags=arch:mipsel
-        :avocado: tags=machine:malta
         :avocado: tags=endian:little
         :avocado: tags=device:pcnet32
         """
@@ -233,7 +230,6 @@ class LinuxSSH(Test):
     def test_mips_malta64eb_kernel3_2_0(self):
         """
         :avocado: tags=arch:mips64
-        :avocado: tags=machine:malta
         :avocado: tags=endian:big
         :avocado: tags=device:pcnet32
         """
@@ -242,7 +238,6 @@ class LinuxSSH(Test):
     def test_mips_malta64el_kernel3_2_0(self):
         """
         :avocado: tags=arch:mips64el
-        :avocado: tags=machine:malta
         :avocado: tags=endian:little
         :avocado: tags=device:pcnet32
         """
diff --git a/tests/acceptance/machine_m68k_nextcube.py b/tests/acceptance/machine_m68k_nextcube.py
index fcd2c58ee7..32cf571f94 100644
--- a/tests/acceptance/machine_m68k_nextcube.py
+++ b/tests/acceptance/machine_m68k_nextcube.py
@@ -43,6 +43,11 @@ def tesseract_available(expected_version):
 
 
 class NextCubeMachine(Test):
+    """
+    :avocado: tags=arch:m68k
+    :avocado: tags=machine:next-cube
+    :avocado: tags=device:framebuffer
+    """
 
     timeout = 15
 
@@ -52,7 +57,6 @@ class NextCubeMachine(Test):
         rom_hash = 'b3534796abae238a0111299fc406a9349f7fee24'
         rom_path = self.fetch_asset(rom_url, asset_hash=rom_hash)
 
-        self.vm.set_machine('next-cube')
         self.vm.add_args('-bios', rom_path)
         self.vm.launch()
 
@@ -66,11 +70,6 @@ class NextCubeMachine(Test):
 
     @skipUnless(PIL_AVAILABLE, 'Python PIL not installed')
     def test_bootrom_framebuffer_size(self):
-        """
-        :avocado: tags=arch:m68k
-        :avocado: tags=machine:next_cube
-        :avocado: tags=device:framebuffer
-        """
         screenshot_path = os.path.join(self.workdir, "dump.png")
         self.check_bootrom_framebuffer(screenshot_path)
 
@@ -80,11 +79,6 @@ class NextCubeMachine(Test):
 
     @skipUnless(tesseract_available(3), 'tesseract v3 OCR tool not available')
     def test_bootrom_framebuffer_ocr_with_tesseract_v3(self):
-        """
-        :avocado: tags=arch:m68k
-        :avocado: tags=machine:next_cube
-        :avocado: tags=device:framebuffer
-        """
         screenshot_path = os.path.join(self.workdir, "dump.png")
         self.check_bootrom_framebuffer(screenshot_path)
 
@@ -101,11 +95,6 @@ class NextCubeMachine(Test):
     # that it is still alpha-level software.
     @skipUnless(tesseract_available(4), 'tesseract v4 OCR tool not available')
     def test_bootrom_framebuffer_ocr_with_tesseract_v4(self):
-        """
-        :avocado: tags=arch:m68k
-        :avocado: tags=machine:next_cube
-        :avocado: tags=device:framebuffer
-        """
         screenshot_path = os.path.join(self.workdir, "dump.png")
         self.check_bootrom_framebuffer(screenshot_path)
 
diff --git a/tests/acceptance/machine_sparc_leon3.py b/tests/acceptance/machine_sparc_leon3.py
index 298f1e25e6..f77e210ccb 100644
--- a/tests/acceptance/machine_sparc_leon3.py
+++ b/tests/acceptance/machine_sparc_leon3.py
@@ -16,7 +16,7 @@ class Leon3Machine(Test):
     def test_leon3_helenos_uimage(self):
         """
         :avocado: tags=arch:sparc
-        :avocado: tags=machine:leon3
+        :avocado: tags=machine:leon3_generic
         :avocado: tags=binfmt:uimage
         """
         kernel_url = ('http://www.helenos.org/releases/'
@@ -24,7 +24,6 @@ class Leon3Machine(Test):
         kernel_hash = 'a88c9cfdb8430c66650e5290a08765f9bf049a30'
         kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
 
-        self.vm.set_machine('leon3_generic')
         self.vm.set_console()
         self.vm.add_args('-kernel', kernel_path)
 
diff --git a/tests/acceptance/ppc_prep_40p.py b/tests/acceptance/ppc_prep_40p.py
index 6f507fb0a6..b27572f212 100644
--- a/tests/acceptance/ppc_prep_40p.py
+++ b/tests/acceptance/ppc_prep_40p.py
@@ -39,7 +39,6 @@ class IbmPrep40pMachine(Test):
         drive_hash = 'dbcfc09912e71bd5f0d82c7c1ee43082fb596ceb'
         drive_path = self.fetch_asset(drive_url, asset_hash=drive_hash)
 
-        self.vm.set_machine('40p')
         self.vm.set_console()
         self.vm.add_args('-bios', bios_path,
                          '-fda', drive_path)
@@ -53,7 +52,6 @@ class IbmPrep40pMachine(Test):
         :avocado: tags=arch:ppc
         :avocado: tags=machine:40p
         """
-        self.vm.set_machine('40p')
         self.vm.set_console()
         self.vm.add_args('-m', '192') # test fw_cfg
 
@@ -73,7 +71,6 @@ class IbmPrep40pMachine(Test):
         drive_hash = 'ac6fa2707d888b36d6fa64de6e7fe48e'
         drive_path = self.fetch_asset(drive_url, asset_hash=drive_hash,
                                       algorithm='md5')
-        self.vm.set_machine('40p')
         self.vm.set_console()
         self.vm.add_args('-cdrom', drive_path,
                          '-boot', 'd')
diff --git a/tests/acceptance/x86_cpu_model_versions.py b/tests/acceptance/x86_cpu_model_versions.py
index 6eb977954d..90558d9a71 100644
--- a/tests/acceptance/x86_cpu_model_versions.py
+++ b/tests/acceptance/x86_cpu_model_versions.py
@@ -75,12 +75,15 @@ class X86CPUModelAliases(avocado_qemu.Test):
                          "EPYC-IBPB shouldn't be versioned")
 
     def test_4_0_alias_compatibility(self):
-        """Check if pc-*-4.0 unversioned CPU model won't be reported as aliases"""
+        """
+        Check if pc-*-4.0 unversioned CPU model won't be reported as aliases
+
+        :avocado: tags=machine:pc-i440fx-4.0
+        """
         # pc-*-4.0 won't expose non-versioned CPU models as aliases
         # We do this to help management software to keep compatibility
         # with older QEMU versions that didn't have the versioned CPU model
         self.vm.add_args('-S')
-        self.vm.set_machine('pc-i440fx-4.0')
         self.vm.launch()
         cpus = dict((m['name'], m) for m in self.vm.command('query-cpu-definitions'))
 
@@ -105,9 +108,12 @@ class X86CPUModelAliases(avocado_qemu.Test):
             self.assertNotIn('alias-of', c, "%s shouldn't be an alias" % (name))
 
     def test_4_1_alias(self):
-        """Check if unversioned CPU model is an alias pointing to right version"""
+        """
+        Check if unversioned CPU model is an alias pointing to right version
+
+        :avocado: tags=machine:pc-i440fx-4.1
+        """
         self.vm.add_args('-S')
-        self.vm.set_machine('pc-i440fx-4.1')
         self.vm.launch()
 
         cpus = dict((m['name'], m) for m in self.vm.command('query-cpu-definitions'))
@@ -207,9 +213,12 @@ class X86CPUModelAliases(avocado_qemu.Test):
         self.validate_aliases(cpus)
 
     def test_none_alias(self):
-        """Check if unversioned CPU model is an alias pointing to some version"""
+        """
+        Check if unversioned CPU model is an alias pointing to some version
+
+        :avocado: tags=machine:none
+        """
         self.vm.add_args('-S')
-        self.vm.set_machine('none')
         self.vm.launch()
 
         cpus = dict((m['name'], m) for m in self.vm.command('query-cpu-definitions'))
@@ -242,68 +251,84 @@ class CascadelakeArchCapabilities(avocado_qemu.Test):
         return self.vm.command('qom-get', path=cpu_path, property=prop)
 
     def test_4_1(self):
+        """
+        :avocado: tags=machine:pc-i440fx-4.1
+        """
         # machine-type only:
         self.vm.add_args('-S')
-        self.vm.set_machine('pc-i440fx-4.1')
         self.vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off')
         self.vm.launch()
         self.assertFalse(self.get_cpu_prop('arch-capabilities'),
                          'pc-i440fx-4.1 + Cascadelake-Server should not have arch-capabilities')
 
     def test_4_0(self):
+        """
+        :avocado: tags=machine:pc-i440fx-4.0
+        """
         self.vm.add_args('-S')
-        self.vm.set_machine('pc-i440fx-4.0')
         self.vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off')
         self.vm.launch()
         self.assertFalse(self.get_cpu_prop('arch-capabilities'),
                          'pc-i440fx-4.0 + Cascadelake-Server should not have arch-capabilities')
 
     def test_set_4_0(self):
+        """
+        :avocado: tags=machine:pc-i440fx-4.0
+        """
         # command line must override machine-type if CPU model is not versioned:
         self.vm.add_args('-S')
-        self.vm.set_machine('pc-i440fx-4.0')
         self.vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off,+arch-capabilities')
         self.vm.launch()
         self.assertTrue(self.get_cpu_prop('arch-capabilities'),
                         'pc-i440fx-4.0 + Cascadelake-Server,+arch-capabilities should have arch-capabilities')
 
     def test_unset_4_1(self):
+        """
+        :avocado: tags=machine:pc-i440fx-4.1
+        """
         self.vm.add_args('-S')
-        self.vm.set_machine('pc-i440fx-4.1')
         self.vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off,-arch-capabilities')
         self.vm.launch()
         self.assertFalse(self.get_cpu_prop('arch-capabilities'),
                          'pc-i440fx-4.1 + Cascadelake-Server,-arch-capabilities should not have arch-capabilities')
 
     def test_v1_4_0(self):
+        """
+        :avocado: tags=machine:pc-i440fx-4.0
+        """
         # versioned CPU model overrides machine-type:
         self.vm.add_args('-S')
-        self.vm.set_machine('pc-i440fx-4.0')
         self.vm.add_args('-cpu', 'Cascadelake-Server-v1,x-force-features=on,check=off,enforce=off')
         self.vm.launch()
         self.assertFalse(self.get_cpu_prop('arch-capabilities'),
                          'pc-i440fx-4.0 + Cascadelake-Server-v1 should not have arch-capabilities')
 
     def test_v2_4_0(self):
+        """
+        :avocado: tags=machine:pc-i440fx-4.0
+        """
         self.vm.add_args('-S')
-        self.vm.set_machine('pc-i440fx-4.0')
         self.vm.add_args('-cpu', 'Cascadelake-Server-v2,x-force-features=on,check=off,enforce=off')
         self.vm.launch()
         self.assertTrue(self.get_cpu_prop('arch-capabilities'),
                         'pc-i440fx-4.0 + Cascadelake-Server-v2 should have arch-capabilities')
 
     def test_v1_set_4_0(self):
+        """
+        :avocado: tags=machine:pc-i440fx-4.0
+        """
         # command line must override machine-type and versioned CPU model:
         self.vm.add_args('-S')
-        self.vm.set_machine('pc-i440fx-4.0')
         self.vm.add_args('-cpu', 'Cascadelake-Server-v1,x-force-features=on,check=off,enforce=off,+arch-capabilities')
         self.vm.launch()
         self.assertTrue(self.get_cpu_prop('arch-capabilities'),
                         'pc-i440fx-4.0 + Cascadelake-Server-v1,+arch-capabilities should have arch-capabilities')
 
     def test_v2_unset_4_1(self):
+        """
+        :avocado: tags=machine:pc-i440fx-4.1
+        """
         self.vm.add_args('-S')
-        self.vm.set_machine('pc-i440fx-4.1')
         self.vm.add_args('-cpu', 'Cascadelake-Server-v2,x-force-features=on,check=off,enforce=off,-arch-capabilities')
         self.vm.launch()
         self.assertFalse(self.get_cpu_prop('arch-capabilities'),
-- 
2.21.0



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

* [PATCH v7 4/8] Acceptance tests: use relative location for tests
  2019-11-04 15:13 [PATCH v7 0/8] Acceptance test: Add "boot_linux" acceptance test Cleber Rosa
                   ` (2 preceding siblings ...)
  2019-11-04 15:13 ` [PATCH v7 3/8] Acceptance tests: use avocado tags for machine type Cleber Rosa
@ 2019-11-04 15:13 ` Cleber Rosa
  2019-11-04 18:26   ` Philippe Mathieu-Daudé
  2019-11-07 18:52   ` Wainer dos Santos Moschetta
  2019-11-04 15:13 ` [PATCH v7 5/8] Acceptance tests: keep a stable reference to the QEMU build dir Cleber Rosa
                   ` (5 subsequent siblings)
  9 siblings, 2 replies; 39+ messages in thread
From: Cleber Rosa @ 2019-11-04 15:13 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Wainer dos Santos Moschetta,
	Willian Rampazzo, Cleber Rosa, qemu-ppc, Aleksandar Rikalo,
	Philippe Mathieu-Daudé,
	Aurelien Jarno, Eduardo Habkost

An Avocado Test ID[1] is composed by a number of components, but it
starts with the Test Name, usually a file system location that was
given to the loader.

Because the source directory is being given as a prefix to the
"tests/acceptance" directory containing the acceptance tests, the test
names will needlessly include the directory the user is using to host
the QEMU sources (and/or build tree).

Let's remove the source dir (or a build dir) from the path given to
the test loader.  This should give more constant names, and when using
result servers and databases, it should give the same test names
across executions from different people or from different directories.

[1] - https://avocado-framework.readthedocs.io/en/69.0/ReferenceGuide.html#test-id

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/Makefile.include | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 56f73b46e2..65e85f5275 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -1180,7 +1180,7 @@ check-acceptance: check-venv $(TESTS_RESULTS_DIR)
             --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \
             --filter-by-tags-include-empty --filter-by-tags-include-empty-key \
             $(AVOCADO_TAGS) \
-            --failfast=on $(SRC_PATH)/tests/acceptance, \
+            --failfast=on tests/acceptance, \
             "AVOCADO", "tests/acceptance")
 
 # Consolidated targets
-- 
2.21.0



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

* [PATCH v7 5/8] Acceptance tests: keep a stable reference to the QEMU build dir
  2019-11-04 15:13 [PATCH v7 0/8] Acceptance test: Add "boot_linux" acceptance test Cleber Rosa
                   ` (3 preceding siblings ...)
  2019-11-04 15:13 ` [PATCH v7 4/8] Acceptance tests: use relative location for tests Cleber Rosa
@ 2019-11-04 15:13 ` Cleber Rosa
  2019-11-07 19:22   ` Wainer dos Santos Moschetta
  2019-11-04 15:13 ` [PATCH v7 6/8] Acceptance tests: add the build directory to the system PATH Cleber Rosa
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 39+ messages in thread
From: Cleber Rosa @ 2019-11-04 15:13 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Wainer dos Santos Moschetta,
	Willian Rampazzo, Cleber Rosa, qemu-ppc, Aleksandar Rikalo,
	Philippe Mathieu-Daudé,
	Aurelien Jarno, Eduardo Habkost

This is related to the the differences in in-tree and out-of-tree
builds in QEMU.  For simplification, <BLD> means my build directory.

Currently, by running a `make check-acceptance` one gets (in
tests/acceptance/avocado_qemu/__init__.py):

   SRC_ROOT_DIR: <BLD>/tests/acceptance/avocado_qemu/../../..

This in itself is problematic, because after the parent directories
are applied, one may be left not with a pointer to the build directory
as intended, but with the location of the source tree (assuming they
differ). Built binaries, such as qemu-img, are of course not there and
can't be found.

Given that a Python '__file__' will contain the absolute path to the
file backing the module, say:

   __file__: <BLD>/tests/acceptance/avocado_qemu/__init__.py

                  |  4  |     3    |      2     |     1     |

A solution is to not "evaluate" the third parent dir (marked as 4
here) because that ends up following the "tests" directory symlink to
the source tree.  In fact, there's no need to keep or evaluate any of
the parent directories, we can just drop the rightmost 4 components,
and we'll keep a stable reference to the build directory (with no
symlink being followed).  This works for either a dedicated build
directory or also a combined source and build tree.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/acceptance/avocado_qemu/__init__.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
index 6618ea67c1..17ce583c87 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -16,7 +16,7 @@ import tempfile
 
 import avocado
 
-SRC_ROOT_DIR = os.path.join(os.path.dirname(__file__), '..', '..', '..')
+SRC_ROOT_DIR = os.path.dirname(os.path.dirname(os.path.dirname(os.path.dirname(__file__))))
 sys.path.append(os.path.join(SRC_ROOT_DIR, 'python'))
 
 from qemu.machine import QEMUMachine
-- 
2.21.0



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

* [PATCH v7 6/8] Acceptance tests: add the build directory to the system PATH
  2019-11-04 15:13 [PATCH v7 0/8] Acceptance test: Add "boot_linux" acceptance test Cleber Rosa
                   ` (4 preceding siblings ...)
  2019-11-04 15:13 ` [PATCH v7 5/8] Acceptance tests: keep a stable reference to the QEMU build dir Cleber Rosa
@ 2019-11-04 15:13 ` Cleber Rosa
  2019-11-07 19:46   ` Wainer dos Santos Moschetta
  2019-11-08 13:13   ` Philippe Mathieu-Daudé
  2019-11-04 15:13 ` [PATCH v7 7/8] Acceptance tests: depend on qemu-img Cleber Rosa
                   ` (3 subsequent siblings)
  9 siblings, 2 replies; 39+ messages in thread
From: Cleber Rosa @ 2019-11-04 15:13 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Wainer dos Santos Moschetta,
	Willian Rampazzo, Cleber Rosa, qemu-ppc, Aleksandar Rikalo,
	Philippe Mathieu-Daudé,
	Aurelien Jarno, Eduardo Habkost

So that when binaries such as qemu-img are searched for, those in the
build tree will be favored.  As a clarification, SRC_ROOT_DIR is
dependent on the location from where tests are executed, so they are
equal to the build directory if one is being used.

The original motivation is that Avocado libraries such as
avocado.utils.vmimage.get() may use the matching binaries, but it may
also apply to any other binary that test code may eventually attempt
to execute.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/acceptance/avocado_qemu/__init__.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
index 17ce583c87..a4bb796a47 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -110,6 +110,12 @@ class Test(avocado.Test):
         return None
 
     def setUp(self):
+        # Some utility code uses binaries from the system's PATH.  For
+        # instance, avocado.utils.vmimage.get() uses qemu-img, to
+        # create a snapshot image.  This is a transparent way of
+        # making sure those utilities find and use binaries on the
+        # build tree by default.
+        os.environ['PATH'] = '%s:%s' % (SRC_ROOT_DIR, os.environ['PATH'])
         self._vms = {}
 
         self.arch = self.params.get('arch',
-- 
2.21.0



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

* [PATCH v7 7/8] Acceptance tests: depend on qemu-img
  2019-11-04 15:13 [PATCH v7 0/8] Acceptance test: Add "boot_linux" acceptance test Cleber Rosa
                   ` (5 preceding siblings ...)
  2019-11-04 15:13 ` [PATCH v7 6/8] Acceptance tests: add the build directory to the system PATH Cleber Rosa
@ 2019-11-04 15:13 ` Cleber Rosa
  2019-11-07 20:31   ` Wainer dos Santos Moschetta
  2019-11-04 15:13 ` [PATCH v7 8/8] Acceptance test: add "boot_linux" tests Cleber Rosa
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 39+ messages in thread
From: Cleber Rosa @ 2019-11-04 15:13 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Wainer dos Santos Moschetta,
	Willian Rampazzo, Cleber Rosa, qemu-ppc, Aleksandar Rikalo,
	Philippe Mathieu-Daudé,
	Aurelien Jarno, Eduardo Habkost

Tests using the avocado.utils.vmimage library make use of qemu-img,
and because it makes sense to use the version matching the rest of the
source code, let's make sure it gets built.

Its selection, instead of a possible qemu-img binary installed system
wide, is already dealt with by the change that adds the build dir to
the PATH during the test execution.

This is based on the same work for qemu-iotests, and suggested by its
author:

  - https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg00951.html

CC: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Cleber Rosa <crosa@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 tests/Makefile.include | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 65e85f5275..559c3e6375 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -1174,7 +1174,7 @@ $(TESTS_RESULTS_DIR):
 
 check-venv: $(TESTS_VENV_DIR)
 
-check-acceptance: check-venv $(TESTS_RESULTS_DIR)
+check-acceptance: check-venv $(TESTS_RESULTS_DIR) qemu-img$(EXESUF)
 	$(call quiet-command, \
             $(TESTS_VENV_DIR)/bin/python -m avocado \
             --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \
-- 
2.21.0



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

* [PATCH v7 8/8] Acceptance test: add "boot_linux" tests
  2019-11-04 15:13 [PATCH v7 0/8] Acceptance test: Add "boot_linux" acceptance test Cleber Rosa
                   ` (6 preceding siblings ...)
  2019-11-04 15:13 ` [PATCH v7 7/8] Acceptance tests: depend on qemu-img Cleber Rosa
@ 2019-11-04 15:13 ` Cleber Rosa
  2019-11-08 19:42   ` Wainer dos Santos Moschetta
                     ` (2 more replies)
  2019-11-04 19:54 ` [PATCH v7 0/8] Acceptance test: Add "boot_linux" acceptance test no-reply
  2019-11-07 18:38 ` Wainer dos Santos Moschetta
  9 siblings, 3 replies; 39+ messages in thread
From: Cleber Rosa @ 2019-11-04 15:13 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Wainer dos Santos Moschetta,
	Willian Rampazzo, Cleber Rosa, qemu-ppc, Aleksandar Rikalo,
	Philippe Mathieu-Daudé,
	Aurelien Jarno, Eduardo Habkost

This acceptance test, validates that a full blown Linux guest can
successfully boot in QEMU.  In this specific case, the guest chosen is
Fedora version 31.

 * x86_64, pc and q35 machine types, with and without kvm as an
   accellerator

 * aarch64 and virt machine type, with and without kvm as an
   accellerator

 * ppc64 and pseries machine type

 * s390x and s390-ccw-virtio machine type

The method for checking the successful boot is based on "cloudinit"
and its "phone home" feature.  The guest is given an ISO image
with the location of the phone home server, and the information to
post (the instance ID).  Upon receiving the correct information,
from the guest, the test is considered to have PASSed.

This test is currently limited to user mode networking only, and
instructs the guest to connect to the "router" address that is hard
coded in QEMU.

To create the cloudinit ISO image that will be used to configure the
guest, the pycdlib library is also required and has been added as
requirement to the virtual environment created by "check-venv".

The console output is read by a separate thread, by means of the
Avocado datadrainer utility module.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/acceptance/boot_linux.py | 175 +++++++++++++++++++++++++++++++++
 tests/requirements.txt         |   1 +
 2 files changed, 176 insertions(+)
 create mode 100644 tests/acceptance/boot_linux.py

diff --git a/tests/acceptance/boot_linux.py b/tests/acceptance/boot_linux.py
new file mode 100644
index 0000000000..882f7dc5df
--- /dev/null
+++ b/tests/acceptance/boot_linux.py
@@ -0,0 +1,175 @@
+# Functional test that boots a complete Linux system via a cloud image
+#
+# Copyright (c) 2018-2019 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.
+
+import os
+
+from avocado_qemu import Test, SRC_ROOT_DIR
+
+from qemu import kvm_available
+
+from avocado.utils import cloudinit
+from avocado.utils import network
+from avocado.utils import vmimage
+from avocado.utils import datadrainer
+
+
+KVM_NOT_AVAILABLE = "KVM accelerator does not seem to be available"
+
+
+class BootLinux(Test):
+    """
+    Boots a Linux system, checking for a successful initialization
+    """
+
+    timeout = 600
+    chksum = None
+
+    def setUp(self):
+        super(BootLinux, self).setUp()
+        self.prepare_boot()
+        self.vm.add_args('-smp', '2')
+        self.vm.add_args('-m', '2048')
+        self.vm.add_args('-drive', 'file=%s' % self.boot.path)
+        self.prepare_cloudinit()
+
+    def prepare_boot(self):
+        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:
+            self.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')
+
+    def prepare_cloudinit(self):
+        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)
+            self.vm.add_args('-drive', 'file=%s,format=raw' % cloudinit_iso)
+        except Exception:
+            self.cancel('Failed to prepared cloudinit image')
+
+    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):
+    """
+    :avocado: tags=arch:x86_64
+    """
+
+    chksum = 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'
+
+    def test_pc(self):
+        """
+        :avocado: tags=machine:pc
+        """
+        self.launch_and_wait()
+
+    def test_kvm_pc(self):
+        """
+        :avocado: tags=machine:pc
+        :avocado: tags=accel:kvm
+        """
+        if not kvm_available(self.arch):
+            self.cancel(KVM_NOT_AVAILABLE)
+        self.vm.add_args("-accel", "kvm")
+        self.launch_and_wait()
+
+    def test_q35(self):
+        """
+        :avocado: tags=machine:q35
+        """
+        self.launch_and_wait()
+
+    def test_kvm_q35(self):
+        """
+        :avocado: tags=machine:q35
+        :avocado: tags=accel:kvm
+        """
+        if not kvm_available(self.arch):
+            self.cancel(KVM_NOT_AVAILABLE)
+        self.vm.add_args("-accel", "kvm")
+        self.launch_and_wait()
+
+
+class BootLinuxAarch64(BootLinux):
+    """
+    :avocado: tags=arch:aarch64
+    :avocado: tags=machine:virt
+    """
+
+    chksum = '1e18d9c0cf734940c4b5d5ec592facaed2af0ad0329383d5639c997fdf16fe49'
+
+    def test_virt(self):
+        self.vm.add_args('-cpu', 'cortex-a53')
+        self.vm.add_args('-bios',
+                         os.path.join(SRC_ROOT_DIR, 'pc-bios',
+                                      'edk2-aarch64-code.fd'))
+        self.vm.add_args('-device', 'virtio-rng-pci,rng=rng0')
+        self.vm.add_args('-object', 'rng-random,id=rng0,filename=/dev/urandom')
+        self.launch_and_wait()
+
+    def test_kvm_virt(self):
+        """
+        :avocado: tags=accel:kvm
+        """
+        if not kvm_available(self.arch):
+            self.cancel(KVM_NOT_AVAILABLE)
+        self.vm.add_args("-accel", "kvm")
+        self.test_virt()
+
+
+class BootLinuxPPC64(BootLinux):
+    """
+    :avocado: tags=arch:ppc64
+    """
+
+    chksum = '7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58'
+
+    def test_pseries(self):
+        """
+        :avocado: tags=machine:pseries
+        """
+        self.launch_and_wait()
+
+
+class BootLinuxS390X(BootLinux):
+    """
+    :avocado: tags=arch:s390x
+    """
+
+    chksum = '4caaab5a434fd4d1079149a072fdc7891e354f834d355069ca982fdcaf5a122d'
+
+    def test_s390_ccw_virtio(self):
+        """
+        :avocado: tags=machine:s390-ccw-virtio
+        """
+        self.launch_and_wait()
diff --git a/tests/requirements.txt b/tests/requirements.txt
index a2a587223a..3893361e0c 100644
--- a/tests/requirements.txt
+++ b/tests/requirements.txt
@@ -2,3 +2,4 @@
 # in the tests/venv Python virtual environment. For more info,
 # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
 avocado-framework==72.0
+pycdlib==1.8.0
-- 
2.21.0



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

* Re: [PATCH v7 1/8] Acceptance test x86_cpu_model_versions: use default vm
  2019-11-04 15:13 ` [PATCH v7 1/8] Acceptance test x86_cpu_model_versions: use default vm Cleber Rosa
@ 2019-11-04 18:22   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 39+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-11-04 18:22 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Beraldo Leal, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Wainer dos Santos Moschetta,
	Willian Rampazzo, qemu-ppc, Aleksandar Rikalo, Aurelien Jarno,
	Eduardo Habkost

On 11/4/19 4:13 PM, Cleber Rosa wrote:
> The default vm provided by the test, available as self.vm, serves the
> same purpose of the one obtained by self.get_vm(), but saves a line
> and matches the style of other tests.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>   tests/acceptance/x86_cpu_model_versions.py | 100 ++++++++++-----------
>   1 file changed, 46 insertions(+), 54 deletions(-)
> 
> diff --git a/tests/acceptance/x86_cpu_model_versions.py b/tests/acceptance/x86_cpu_model_versions.py
> index 5fc9ca4bc6..6eb977954d 100644
> --- a/tests/acceptance/x86_cpu_model_versions.py
> +++ b/tests/acceptance/x86_cpu_model_versions.py
> @@ -25,10 +25,6 @@
>   import avocado_qemu
>   import re
>   
> -def get_cpu_prop(vm, prop):
> -    cpu_path = vm.command('query-cpus')[0].get('qom_path')
> -    return vm.command('qom-get', path=cpu_path, property=prop)
> -
>   class X86CPUModelAliases(avocado_qemu.Test):
>       """
>       Validation of PC CPU model versions and CPU model aliases
> @@ -241,78 +237,74 @@ class CascadelakeArchCapabilities(avocado_qemu.Test):
>   
>       :avocado: tags=arch:x86_64
>       """
> +    def get_cpu_prop(self, prop):
> +        cpu_path = self.vm.command('query-cpus')[0].get('qom_path')
> +        return self.vm.command('qom-get', path=cpu_path, property=prop)
> +
>       def test_4_1(self):
>           # machine-type only:
> -        vm = self.get_vm()
> -        vm.add_args('-S')
> -        vm.set_machine('pc-i440fx-4.1')
> -        vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off')
> -        vm.launch()
> -        self.assertFalse(get_cpu_prop(vm, 'arch-capabilities'),
> +        self.vm.add_args('-S')
> +        self.vm.set_machine('pc-i440fx-4.1')
> +        self.vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off')
> +        self.vm.launch()
> +        self.assertFalse(self.get_cpu_prop('arch-capabilities'),
>                            'pc-i440fx-4.1 + Cascadelake-Server should not have arch-capabilities')
>   
>       def test_4_0(self):
> -        vm = self.get_vm()
> -        vm.add_args('-S')
> -        vm.set_machine('pc-i440fx-4.0')
> -        vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off')
> -        vm.launch()
> -        self.assertFalse(get_cpu_prop(vm, 'arch-capabilities'),
> +        self.vm.add_args('-S')
> +        self.vm.set_machine('pc-i440fx-4.0')
> +        self.vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off')
> +        self.vm.launch()
> +        self.assertFalse(self.get_cpu_prop('arch-capabilities'),
>                            'pc-i440fx-4.0 + Cascadelake-Server should not have arch-capabilities')
>   
>       def test_set_4_0(self):
>           # command line must override machine-type if CPU model is not versioned:
> -        vm = self.get_vm()
> -        vm.add_args('-S')
> -        vm.set_machine('pc-i440fx-4.0')
> -        vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off,+arch-capabilities')
> -        vm.launch()
> -        self.assertTrue(get_cpu_prop(vm, 'arch-capabilities'),
> +        self.vm.add_args('-S')
> +        self.vm.set_machine('pc-i440fx-4.0')
> +        self.vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off,+arch-capabilities')
> +        self.vm.launch()
> +        self.assertTrue(self.get_cpu_prop('arch-capabilities'),
>                           'pc-i440fx-4.0 + Cascadelake-Server,+arch-capabilities should have arch-capabilities')
>   
>       def test_unset_4_1(self):
> -        vm = self.get_vm()
> -        vm.add_args('-S')
> -        vm.set_machine('pc-i440fx-4.1')
> -        vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off,-arch-capabilities')
> -        vm.launch()
> -        self.assertFalse(get_cpu_prop(vm, 'arch-capabilities'),
> +        self.vm.add_args('-S')
> +        self.vm.set_machine('pc-i440fx-4.1')
> +        self.vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off,-arch-capabilities')
> +        self.vm.launch()
> +        self.assertFalse(self.get_cpu_prop('arch-capabilities'),
>                            'pc-i440fx-4.1 + Cascadelake-Server,-arch-capabilities should not have arch-capabilities')
>   
>       def test_v1_4_0(self):
>           # versioned CPU model overrides machine-type:
> -        vm = self.get_vm()
> -        vm.add_args('-S')
> -        vm.set_machine('pc-i440fx-4.0')
> -        vm.add_args('-cpu', 'Cascadelake-Server-v1,x-force-features=on,check=off,enforce=off')
> -        vm.launch()
> -        self.assertFalse(get_cpu_prop(vm, 'arch-capabilities'),
> +        self.vm.add_args('-S')
> +        self.vm.set_machine('pc-i440fx-4.0')
> +        self.vm.add_args('-cpu', 'Cascadelake-Server-v1,x-force-features=on,check=off,enforce=off')
> +        self.vm.launch()
> +        self.assertFalse(self.get_cpu_prop('arch-capabilities'),
>                            'pc-i440fx-4.0 + Cascadelake-Server-v1 should not have arch-capabilities')
>   
>       def test_v2_4_0(self):
> -        vm = self.get_vm()
> -        vm.add_args('-S')
> -        vm.set_machine('pc-i440fx-4.0')
> -        vm.add_args('-cpu', 'Cascadelake-Server-v2,x-force-features=on,check=off,enforce=off')
> -        vm.launch()
> -        self.assertTrue(get_cpu_prop(vm, 'arch-capabilities'),
> -                         'pc-i440fx-4.0 + Cascadelake-Server-v2 should have arch-capabilities')
> +        self.vm.add_args('-S')
> +        self.vm.set_machine('pc-i440fx-4.0')
> +        self.vm.add_args('-cpu', 'Cascadelake-Server-v2,x-force-features=on,check=off,enforce=off')
> +        self.vm.launch()
> +        self.assertTrue(self.get_cpu_prop('arch-capabilities'),
> +                        'pc-i440fx-4.0 + Cascadelake-Server-v2 should have arch-capabilities')
>   
>       def test_v1_set_4_0(self):
>           # command line must override machine-type and versioned CPU model:
> -        vm = self.get_vm()
> -        vm.add_args('-S')
> -        vm.set_machine('pc-i440fx-4.0')
> -        vm.add_args('-cpu', 'Cascadelake-Server-v1,x-force-features=on,check=off,enforce=off,+arch-capabilities')
> -        vm.launch()
> -        self.assertTrue(get_cpu_prop(vm, 'arch-capabilities'),
> -                         'pc-i440fx-4.0 + Cascadelake-Server-v1,+arch-capabilities should have arch-capabilities')
> +        self.vm.add_args('-S')
> +        self.vm.set_machine('pc-i440fx-4.0')
> +        self.vm.add_args('-cpu', 'Cascadelake-Server-v1,x-force-features=on,check=off,enforce=off,+arch-capabilities')
> +        self.vm.launch()
> +        self.assertTrue(self.get_cpu_prop('arch-capabilities'),
> +                        'pc-i440fx-4.0 + Cascadelake-Server-v1,+arch-capabilities should have arch-capabilities')
>   
>       def test_v2_unset_4_1(self):
> -        vm = self.get_vm()
> -        vm.add_args('-S')
> -        vm.set_machine('pc-i440fx-4.1')
> -        vm.add_args('-cpu', 'Cascadelake-Server-v2,x-force-features=on,check=off,enforce=off,-arch-capabilities')
> -        vm.launch()
> -        self.assertFalse(get_cpu_prop(vm, 'arch-capabilities'),
> +        self.vm.add_args('-S')
> +        self.vm.set_machine('pc-i440fx-4.1')
> +        self.vm.add_args('-cpu', 'Cascadelake-Server-v2,x-force-features=on,check=off,enforce=off,-arch-capabilities')
> +        self.vm.launch()
> +        self.assertFalse(self.get_cpu_prop('arch-capabilities'),
>                            'pc-i440fx-4.1 + Cascadelake-Server-v2,-arch-capabilities should not have arch-capabilities')
> 


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

* Re: [PATCH v7 4/8] Acceptance tests: use relative location for tests
  2019-11-04 15:13 ` [PATCH v7 4/8] Acceptance tests: use relative location for tests Cleber Rosa
@ 2019-11-04 18:26   ` Philippe Mathieu-Daudé
  2019-11-11 22:11     ` Cleber Rosa
  2019-11-07 18:52   ` Wainer dos Santos Moschetta
  1 sibling, 1 reply; 39+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-11-04 18:26 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Beraldo Leal, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Wainer dos Santos Moschetta,
	Willian Rampazzo, qemu-ppc, Aleksandar Rikalo, Aurelien Jarno,
	Eduardo Habkost

On 11/4/19 4:13 PM, Cleber Rosa wrote:
> An Avocado Test ID[1] is composed by a number of components, but it
> starts with the Test Name, usually a file system location that was
> given to the loader.
> 
> Because the source directory is being given as a prefix to the
> "tests/acceptance" directory containing the acceptance tests, the test
> names will needlessly include the directory the user is using to host
> the QEMU sources (and/or build tree).
> 
> Let's remove the source dir (or a build dir) from the path given to
> the test loader.  This should give more constant names, and when using
> result servers and databases, it should give the same test names
> across executions from different people or from different directories.

Can we strip the full path to directory and only keep the filename in 
the database? (Thinking about out-of-tree tests).

> 
> [1] - https://avocado-framework.readthedocs.io/en/69.0/ReferenceGuide.html#test-id
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>   tests/Makefile.include | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 56f73b46e2..65e85f5275 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -1180,7 +1180,7 @@ check-acceptance: check-venv $(TESTS_RESULTS_DIR)
>               --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \
>               --filter-by-tags-include-empty --filter-by-tags-include-empty-key \
>               $(AVOCADO_TAGS) \
> -            --failfast=on $(SRC_PATH)/tests/acceptance, \
> +            --failfast=on tests/acceptance, \
>               "AVOCADO", "tests/acceptance")
>   
>   # Consolidated targets
> 


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

* Re: [PATCH v7 0/8] Acceptance test: Add "boot_linux" acceptance test
  2019-11-04 15:13 [PATCH v7 0/8] Acceptance test: Add "boot_linux" acceptance test Cleber Rosa
                   ` (7 preceding siblings ...)
  2019-11-04 15:13 ` [PATCH v7 8/8] Acceptance test: add "boot_linux" tests Cleber Rosa
@ 2019-11-04 19:54 ` no-reply
  2019-11-07 18:38 ` Wainer dos Santos Moschetta
  9 siblings, 0 replies; 39+ messages in thread
From: no-reply @ 2019-11-04 19:54 UTC (permalink / raw)
  To: crosa
  Cc: bleal, qemu-devel, wainersm, frederic.konrad, hpoussin, chouteau,
	wrampazz, crosa, qemu-ppc, aleksandar.rikalo, philmd, aurelien,
	ehabkost

Patchew URL: https://patchew.org/QEMU/20191104151323.9883-1-crosa@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PATCH v7 0/8] Acceptance test: Add "boot_linux" acceptance test
Type: series
Message-id: 20191104151323.9883-1-crosa@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
8675afa Acceptance test: add "boot_linux" tests
409affc Acceptance tests: depend on qemu-img
58c0f47 Acceptance tests: add the build directory to the system PATH
236ecfa Acceptance tests: keep a stable reference to the QEMU build dir
6a147b4 Acceptance tests: use relative location for tests
bcd92a3 Acceptance tests: use avocado tags for machine type
0f52135 Acceptance tests: introduce utility method for tags unique vals
050f1d1 Acceptance test x86_cpu_model_versions: use default vm

=== OUTPUT BEGIN ===
1/8 Checking commit 050f1d1ea660 (Acceptance test x86_cpu_model_versions: use default vm)
ERROR: line over 90 characters
#47: FILE: tests/acceptance/x86_cpu_model_versions.py:248:
+        self.vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off')

ERROR: line over 90 characters
#61: FILE: tests/acceptance/x86_cpu_model_versions.py:256:
+        self.vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off')

ERROR: line over 90 characters
#76: FILE: tests/acceptance/x86_cpu_model_versions.py:265:
+        self.vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off,+arch-capabilities')

ERROR: line over 90 characters
#90: FILE: tests/acceptance/x86_cpu_model_versions.py:273:
+        self.vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off,-arch-capabilities')

ERROR: line over 90 characters
#105: FILE: tests/acceptance/x86_cpu_model_versions.py:282:
+        self.vm.add_args('-cpu', 'Cascadelake-Server-v1,x-force-features=on,check=off,enforce=off')

ERROR: line over 90 characters
#120: FILE: tests/acceptance/x86_cpu_model_versions.py:290:
+        self.vm.add_args('-cpu', 'Cascadelake-Server-v2,x-force-features=on,check=off,enforce=off')

ERROR: line over 90 characters
#123: FILE: tests/acceptance/x86_cpu_model_versions.py:293:
+                        'pc-i440fx-4.0 + Cascadelake-Server-v2 should have arch-capabilities')

ERROR: line over 90 characters
#136: FILE: tests/acceptance/x86_cpu_model_versions.py:299:
+        self.vm.add_args('-cpu', 'Cascadelake-Server-v1,x-force-features=on,check=off,enforce=off,+arch-capabilities')

ERROR: line over 90 characters
#139: FILE: tests/acceptance/x86_cpu_model_versions.py:302:
+                        'pc-i440fx-4.0 + Cascadelake-Server-v1,+arch-capabilities should have arch-capabilities')

ERROR: line over 90 characters
#150: FILE: tests/acceptance/x86_cpu_model_versions.py:307:
+        self.vm.add_args('-cpu', 'Cascadelake-Server-v2,x-force-features=on,check=off,enforce=off,-arch-capabilities')

total: 10 errors, 0 warnings, 134 lines checked

Patch 1/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

2/8 Checking commit 0f5213583c78 (Acceptance tests: introduce utility method for tags unique vals)
3/8 Checking commit bcd92a334692 (Acceptance tests: use avocado tags for machine type)
WARNING: line over 80 characters
#59: FILE: tests/acceptance/avocado_qemu/__init__.py:119:
+                                       default=self._get_unique_tag_val('machine'))

total: 0 errors, 1 warnings, 450 lines checked

Patch 3/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/8 Checking commit 6a147b4b2eb1 (Acceptance tests: use relative location for tests)
5/8 Checking commit 236ecfa68ee0 (Acceptance tests: keep a stable reference to the QEMU build dir)
ERROR: line over 90 characters
#48: FILE: tests/acceptance/avocado_qemu/__init__.py:19:
+SRC_ROOT_DIR = os.path.dirname(os.path.dirname(os.path.dirname(os.path.dirname(__file__))))

total: 1 errors, 0 warnings, 8 lines checked

Patch 5/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

6/8 Checking commit 58c0f475d5ec (Acceptance tests: add the build directory to the system PATH)
7/8 Checking commit 409affc034ce (Acceptance tests: depend on qemu-img)
8/8 Checking commit 8675afa0dead (Acceptance test: add "boot_linux" tests)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#42: 
new file mode 100644

WARNING: line over 80 characters
#122: FILE: tests/acceptance/boot_linux.py:76:
+        console_drainer = datadrainer.LineLogger(self.vm.console_socket.fileno(),

WARNING: line over 80 characters
#123: FILE: tests/acceptance/boot_linux.py:77:
+                                                 logger=self.log.getChild('console'))

WARNING: line over 80 characters
#126: FILE: tests/acceptance/boot_linux.py:80:
+        cloudinit.wait_for_phone_home(('0.0.0.0', self.phone_home_port), self.name)

total: 0 errors, 4 warnings, 179 lines checked

Patch 8/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20191104151323.9883-1-crosa@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

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

* Re: [PATCH v7 0/8] Acceptance test: Add "boot_linux" acceptance test
  2019-11-04 15:13 [PATCH v7 0/8] Acceptance test: Add "boot_linux" acceptance test Cleber Rosa
                   ` (8 preceding siblings ...)
  2019-11-04 19:54 ` [PATCH v7 0/8] Acceptance test: Add "boot_linux" acceptance test no-reply
@ 2019-11-07 18:38 ` Wainer dos Santos Moschetta
  9 siblings, 0 replies; 39+ messages in thread
From: Wainer dos Santos Moschetta @ 2019-11-07 18:38 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Beraldo Leal, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Willian Rampazzo, qemu-ppc,
	Aleksandar Rikalo, Philippe Mathieu-Daudé,
	Aurelien Jarno, Eduardo Habkost

Hi Cleber,

On 11/4/19 1:13 PM, Cleber Rosa wrote:
> This acceptance test, validates that a full blown Linux guest can
> successfully boot in QEMU.  In this specific case, the guest chosen is
> Fedora version 31.
>
>   * x86_64, pc and q35 machine types, with and without kvm as an
>     accellerator
>
>   * aarch64 and virt machine type, with and without kvm as an
>     accellerator
>
>   * ppc64 and pseries machine type
>
>   * s390x and s390-ccw-virtio machine type
>
> This has been tested on x86_64 and ppc64le hosts and has been running
> reliably (in my experience) on Travis CI.
>
> Some hopefully useful pointers for reviewers:
> =============================================
>
> Git Info:
>    - URI: https://github.com/clebergnu/qemu/tree/test_boot_linux_v7
>    - Remote: https://github.com/clebergnu/qemu
>    - Branch: test_boot_linux_v7
>
> Travis CI Info:
>    - Build: https://travis-ci.org/clebergnu/qemu/builds/606191094
>    - Job: https://travis-ci.org/clebergnu/qemu/jobs/606191120
>
> Previous version:
>    - v6: https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg01202.html
>    - v5: https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg04652.html
>    - v4: https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg02032.html
>    - v3: https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg01677.html
>    - v2: https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg04318.html
>    - v1: http://lists.nongnu.org/archive/html/qemu-devel/2018-09/msg02530.html
>
> Changes from v6:
> ================
>
>   * Bumped Fedora to most recently released version (31).
>
>   * Included new architectures (ppc64 and s390x), consolidating all
>     tests into the same commit.
>
>   * New commit: "Acceptance tests: use avocado tags for machine type"
>
>   * New commit: "Acceptance tests: introduce utility method for tags
>     unique vals"

It seems 02 and 03 are re-sending of patches originally sent in the 
series with Message-Id: 20190924194501.9303-1-crosa@redhat.com. On the 
original thread you can find my reviews.

- Wainer


>
>   * New commit: "Acceptance test x86_cpu_model_versions: use default
>     vm", needed to normalize the use of the machine type tags
>
>   * Added a lot of leniency to the test setup (and reliability to the
>     test/job), canceling the test if there are any failures while
>     downloading/preparing the boot images.
>
>   * Made use of Avocado's data drainer a regular feature (dropped the
>     commit with RFC) and squashed it.
>
>   * Bumped pycdlib version to 1.8.0
>
>   * Dropped explicit "--enable-slirp=git" (added on v5) to Travis CI
>     configure line, as the default configuration on Travis CI now
>     results in user networking capabilities.
>
> Changes from v5:
> ================
>
>   * Added explicit "--enable-slirp=git" to Travis CI configure line, as
>     these tests depend on "-netdev user" like networking.
>
>   * Bumped Fedora to most recently released version (30).
>
>   * Changed "checksum" parameter to 'sha256' and use the same hashes as
>     provided by the Fedora project (instead of using Avocado's default
>     sha1 and compute and use a different hash value).
>
>   * New commit: Add "boot_linux" test for aarch64 and virt machine type
>
>   * New commit: [RFC]: use Avocado data drainer for console logging
>
> Changes from v4:
> ================
>
>   * New commit "Acceptance tests: use relative location for tests"
>
>   * New commit "Acceptance tests: keep a stable reference to the QEMU build dir"
>
>   * Pinned the Fedora 29 image by adding a checksum.  The goal is to
>     never allow more than one component to change at a time (the one
>     allowed to change is QEMU itself).  Updates to the image should be
>     manual. (Based on comments from Cornelia)
>
>   * Moved the downloading of the Fedora 29 cloud image to the test
>     setUp() method, canceling the test if the image can not be
>     downloaded.
>
>   * Removed the ":avocado: enable" tag, given that Avocado versions
>     68.0 and later operate on a "recursive by default" manner, that
>     is able to correctly identify this as an Avocado test.
>
> Changes from v3:
> ================
>
>   * New patch "Acceptance tests: depend on qemu-img"
>
> Known Issues on v3 (no longer applicable):
> ==========================================
>
>   * A recent TCG performance regression[1] affects this test in a
>     number of ways:
>     - The test execution may timeout by itself
>     - The generation of SSH host keys in the guest's first boot is also
>       affected (possibly also a timeout)
>     - The cloud-init "phone home" feature attempts to read the host keys
>       and fails, causing the test to timeout and fail
>
>     These are not observed anymore once the fix[2] is applied.
>
> [1] - https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg00338.html
> [2] - https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg01129.html
>
> Changes from v2:
> ================
>
>   * Updated the tag to include the "arch:" key, in a similar fashion as to
>     the tests in the "Acceptance Tests: target architecture support":
>     - https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg00369.html
>
>   * Renamed the test method name to test_x86_64_pc, again, similarly to the
>     boot_linux_console.py tests in the series mentioned before.
>
>   * Set the machine type explicitly, again similarly to the
>     boot_linux_console.py tests in the series mentioned before.
>
>   * Added messages after the launch of the VM, to let test runners know
>     the test know waits for a boot confirmation from the the guest (Eduardo).
>
>   * Updated commit message to reflect the fact that this version does
>     not allow for parameterization of the guest OS, version, etc.
>
>   * Dropped the RFC prefix on patch "RFC: Acceptance tests: add the
>     build directory to the system PATH"
>
>   * Changed the comments on "RFC: Acceptance tests: add the build
>     directory to the system PATH" to make it clear the addition of a
>     the build directory to the PATH may influence other utility code.
>
> Changes from v1:
> ================
>
>   * The commit message was adjusted, removing the reference to the
>     avocado.utils.vmimage encoding issue on previous Avocado versions
>     (<= 64.0) and the fix that would (and was) included in Avocado
>     version 65.0.
>
>   * Effectively added pycdlib==1.6.0 to the requirements.txt file,
>     added on a56931eef3, and adjusted the commit message was also
>     to reflect that.
>
>   * Updated the default version of the guest OS, from Fedora 28 to 29.
>     Besides possible improvements in the (virtual) hardware coverage,
>     it brings a performance improvement in the order of 20% to the
>     test.
>
>   * Removed all direct parameters usage.  Because some parameters and
>     its default values implemented in the test would prevent it from
>     running on some environments.  Example: the "accel" parameter had a
>     default value of "kvm", which would prevent this test, that boots a
>     x86_64 OS, from running on a host arch different than x86_64.  I
>     recognize that it's desirable to make tests reusable and
>     parameterized (that was the reason for the first version doing so),
>     but the mechanism to be used to define the architectures that a
>     given test should support is still an open issue, and has been
>     discussed in other threads.  I'll follow up those discussions with
>     a proposal, and until then, removing those aspects from this test
>     implementation seemed to be the best option.  A caveat: this test
>     currently adds the same tag (x86_64) and follows other assumptions
>     made on "boot_linux_console.py", that is, that a x86_64 target
>     binary will be used to run it.  If a user is in an environment that
>     does not have a x86_64 target binary, it could filter those tests
>     out with: "avocado run --filter-by-tags='-x86_64' tests/acceptance".
>
>   * Removed most arguments to the QEMU command line for pretty much the
>     same reasons described above, and by following the general
>     perception that I could grasp from other discussions that QEMU
>     defaults should preferrably be used.  This test, as well as others,
>     can and should be extended later to allow for different test
>     scenarios by passing well documented parameter values.  That is,
>     they should respect well-known parameters such as "accel" mentioned
>     above, so that the same test can run with KVM or TCG.
>
>   * Changed the value of the memory argument to 1024, which based on
>     my experimentations and observations is the minimum amount of RAM
>     for the Fedora 29 cloud image to sucessfully boot on QEMU.  I know
>     there's no such thing as a "one size fits all", specially for QEMU,
>     but this makes me wonder wether a x86_64 machine type shouldn't
>     have its default_ram_size bumped to a number practical enough to
>     run modern operating systems.
>
>   * Added a new patch "RFC: Acceptance tests: add the build directory
>     to the system PATH", which is supposed to gather feedback on how to
>     enable the use of built binaries, such as qemu-img, to code used by
>     the test code.  The specific situation here is that the vmimage,
>     part of the avocado.utils libraries, makes use of qemu-img to create
>     snapshot files.  Even though we could require qemu-img to be installed
>     as a dependency of tests, system wide, it actually goes against the
>     goal of testing all QEMU things from the source/build tree.  This
>     became aparent with tests running on environments such as Travis CI,
>     which don't necessarily have qemu-img available elsewhere.
>
> Cleber Rosa (8):
>    Acceptance test x86_cpu_model_versions: use default vm
>    Acceptance tests: introduce utility method for tags unique vals
>    Acceptance tests: use avocado tags for machine type
>    Acceptance tests: use relative location for tests
>    Acceptance tests: keep a stable reference to the QEMU build dir
>    Acceptance tests: add the build directory to the system PATH
>    Acceptance tests: depend on qemu-img
>    Acceptance test: add "boot_linux" tests
>
>   docs/devel/testing.rst                     |  18 +++
>   tests/Makefile.include                     |   4 +-
>   tests/acceptance/avocado_qemu/__init__.py  |  32 +++-
>   tests/acceptance/boot_linux.py             | 175 +++++++++++++++++++++
>   tests/acceptance/boot_linux_console.py     |  19 +--
>   tests/acceptance/cpu_queries.py            |   2 +-
>   tests/acceptance/linux_initrd.py           |   2 +-
>   tests/acceptance/linux_ssh_mips_malta.py   |   5 -
>   tests/acceptance/machine_m68k_nextcube.py  |  21 +--
>   tests/acceptance/machine_sparc_leon3.py    |   3 +-
>   tests/acceptance/ppc_prep_40p.py           |   3 -
>   tests/acceptance/x86_cpu_model_versions.py | 137 +++++++++-------
>   tests/requirements.txt                     |   1 +
>   13 files changed, 308 insertions(+), 114 deletions(-)
>   create mode 100644 tests/acceptance/boot_linux.py
>



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

* Re: [PATCH v7 4/8] Acceptance tests: use relative location for tests
  2019-11-04 15:13 ` [PATCH v7 4/8] Acceptance tests: use relative location for tests Cleber Rosa
  2019-11-04 18:26   ` Philippe Mathieu-Daudé
@ 2019-11-07 18:52   ` Wainer dos Santos Moschetta
  1 sibling, 0 replies; 39+ messages in thread
From: Wainer dos Santos Moschetta @ 2019-11-07 18:52 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Beraldo Leal, Fabien Chouteau, KONRAD Frederic, qemu-ppc,
	Willian Rampazzo, Hervé Poussineau, Aleksandar Rikalo,
	Philippe Mathieu-Daudé,
	Aurelien Jarno, Eduardo Habkost


On 11/4/19 1:13 PM, Cleber Rosa wrote:
> An Avocado Test ID[1] is composed by a number of components, but it
> starts with the Test Name, usually a file system location that was
> given to the loader.
>
> Because the source directory is being given as a prefix to the
> "tests/acceptance" directory containing the acceptance tests, the test
> names will needlessly include the directory the user is using to host
> the QEMU sources (and/or build tree).
>
> Let's remove the source dir (or a build dir) from the path given to
> the test loader.  This should give more constant names, and when using
> result servers and databases, it should give the same test names
> across executions from different people or from different directories.

Completely agree.

>
> [1] - https://avocado-framework.readthedocs.io/en/69.0/ReferenceGuide.html#test-id
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   tests/Makefile.include | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

- Wainer

>
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 56f73b46e2..65e85f5275 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -1180,7 +1180,7 @@ check-acceptance: check-venv $(TESTS_RESULTS_DIR)
>               --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \
>               --filter-by-tags-include-empty --filter-by-tags-include-empty-key \
>               $(AVOCADO_TAGS) \
> -            --failfast=on $(SRC_PATH)/tests/acceptance, \
> +            --failfast=on tests/acceptance, \
>               "AVOCADO", "tests/acceptance")
>   
>   # Consolidated targets



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

* Re: [PATCH v7 5/8] Acceptance tests: keep a stable reference to the QEMU build dir
  2019-11-04 15:13 ` [PATCH v7 5/8] Acceptance tests: keep a stable reference to the QEMU build dir Cleber Rosa
@ 2019-11-07 19:22   ` Wainer dos Santos Moschetta
  2019-11-11 22:38     ` Cleber Rosa
  2019-11-15 21:36     ` Cleber Rosa
  0 siblings, 2 replies; 39+ messages in thread
From: Wainer dos Santos Moschetta @ 2019-11-07 19:22 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Beraldo Leal, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Willian Rampazzo, qemu-ppc,
	Aleksandar Rikalo, Philippe Mathieu-Daudé,
	Aurelien Jarno, Eduardo Habkost


On 11/4/19 1:13 PM, Cleber Rosa wrote:
> This is related to the the differences in in-tree and out-of-tree
> builds in QEMU.  For simplification, <BLD> means my build directory.
>
> Currently, by running a `make check-acceptance` one gets (in
> tests/acceptance/avocado_qemu/__init__.py):
>
>     SRC_ROOT_DIR: <BLD>/tests/acceptance/avocado_qemu/../../..
>
> This in itself is problematic, because after the parent directories
> are applied, one may be left not with a pointer to the build directory
> as intended, but with the location of the source tree (assuming they
> differ). Built binaries, such as qemu-img, are of course not there and
> can't be found.
>
> Given that a Python '__file__' will contain the absolute path to the
> file backing the module, say:
>
>     __file__: <BLD>/tests/acceptance/avocado_qemu/__init__.py
>
>                    |  4  |     3    |      2     |     1     |
>
> A solution is to not "evaluate" the third parent dir (marked as 4
> here) because that ends up following the "tests" directory symlink to
> the source tree.  In fact, there's no need to keep or evaluate any of
> the parent directories, we can just drop the rightmost 4 components,
> and we'll keep a stable reference to the build directory (with no
> symlink being followed).  This works for either a dedicated build
> directory or also a combined source and build tree.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   tests/acceptance/avocado_qemu/__init__.py | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> index 6618ea67c1..17ce583c87 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -16,7 +16,7 @@ import tempfile
>   
>   import avocado
>   
> -SRC_ROOT_DIR = os.path.join(os.path.dirname(__file__), '..', '..', '..')
> +SRC_ROOT_DIR = os.path.dirname(os.path.dirname(os.path.dirname(os.path.dirname(__file__))))

In this case, wouldn't make sense to rename the constant from 
SRC_ROOT_DIR to BUILD_ROOT_DIR?

This patch looks good to me besides that.

- Wainer

>   sys.path.append(os.path.join(SRC_ROOT_DIR, 'python'))
>   
>   from qemu.machine import QEMUMachine



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

* Re: [PATCH v7 6/8] Acceptance tests: add the build directory to the system PATH
  2019-11-04 15:13 ` [PATCH v7 6/8] Acceptance tests: add the build directory to the system PATH Cleber Rosa
@ 2019-11-07 19:46   ` Wainer dos Santos Moschetta
  2019-11-11 22:49     ` Cleber Rosa
  2019-11-08 13:13   ` Philippe Mathieu-Daudé
  1 sibling, 1 reply; 39+ messages in thread
From: Wainer dos Santos Moschetta @ 2019-11-07 19:46 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Beraldo Leal, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Willian Rampazzo, qemu-ppc,
	Aleksandar Rikalo, Philippe Mathieu-Daudé,
	Aurelien Jarno, Eduardo Habkost


On 11/4/19 1:13 PM, Cleber Rosa wrote:
> So that when binaries such as qemu-img are searched for, those in the
> build tree will be favored.  As a clarification, SRC_ROOT_DIR is
> dependent on the location from where tests are executed, so they are
> equal to the build directory if one is being used.
>
> The original motivation is that Avocado libraries such as
> avocado.utils.vmimage.get() may use the matching binaries, but it may
> also apply to any other binary that test code may eventually attempt
> to execute.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   tests/acceptance/avocado_qemu/__init__.py | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> index 17ce583c87..a4bb796a47 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -110,6 +110,12 @@ class Test(avocado.Test):
>           return None
>   
>       def setUp(self):
> +        # Some utility code uses binaries from the system's PATH.  For
> +        # instance, avocado.utils.vmimage.get() uses qemu-img, to
> +        # create a snapshot image.  This is a transparent way of

Because PATH is changed in a transparent way, wouldn't be better to also 
self.log.info() that fact?

> +        # making sure those utilities find and use binaries on the
> +        # build tree by default.
> +        os.environ['PATH'] = '%s:%s' % (SRC_ROOT_DIR, os.environ['PATH'])

I think PATH should be set only once at class initialization. Perhaps in 
setUpClass()?

- Wainer

>           self._vms = {}
>   
>           self.arch = self.params.get('arch',



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

* Re: [PATCH v7 7/8] Acceptance tests: depend on qemu-img
  2019-11-04 15:13 ` [PATCH v7 7/8] Acceptance tests: depend on qemu-img Cleber Rosa
@ 2019-11-07 20:31   ` Wainer dos Santos Moschetta
  2019-11-15 23:45     ` Cleber Rosa
  0 siblings, 1 reply; 39+ messages in thread
From: Wainer dos Santos Moschetta @ 2019-11-07 20:31 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Beraldo Leal, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Willian Rampazzo, qemu-ppc,
	Aleksandar Rikalo, Philippe Mathieu-Daudé,
	Aurelien Jarno, Eduardo Habkost


On 11/4/19 1:13 PM, Cleber Rosa wrote:
> Tests using the avocado.utils.vmimage library make use of qemu-img,
> and because it makes sense to use the version matching the rest of the
> source code, let's make sure it gets built.
>
> Its selection, instead of a possible qemu-img binary installed system
> wide, is already dealt with by the change that adds the build dir to
> the PATH during the test execution.
>
> This is based on the same work for qemu-iotests, and suggested by its
> author:
>
>    - https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg00951.html
>
> CC: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>   tests/Makefile.include | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 65e85f5275..559c3e6375 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -1174,7 +1174,7 @@ $(TESTS_RESULTS_DIR):
>   
>   check-venv: $(TESTS_VENV_DIR)
>   
> -check-acceptance: check-venv $(TESTS_RESULTS_DIR)
> +check-acceptance: check-venv $(TESTS_RESULTS_DIR) qemu-img$(EXESUF)

To be honest, I don't fell comfortable by the fact that the whole 
acceptance suite will depend on qemu-img which, in reality, is needed by 
only a sub-set of tests. Besides it, there might be some reason for 
someone to build QEMU with --disable-tools and this change will end up 
forcing the qemu-img built (of course if check-acceptance is issued).

What if we instead:

1. Warn the users in case qemu tools weren't built. Alerting that 
qemu-img and friends will be picked up from system-wide (if any).

2. The tests that rely on avocado.utils.vmimage check for the presence 
of dependent tools, possible canceling itself on their lack. This may be 
done at test code level or perhaps using Avocado's tag mechanism + 
tweaking avocado_qemu.

Thanks,

Wainer

>   	$(call quiet-command, \
>               $(TESTS_VENV_DIR)/bin/python -m avocado \
>               --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \



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

* Re: [PATCH v7 6/8] Acceptance tests: add the build directory to the system PATH
  2019-11-04 15:13 ` [PATCH v7 6/8] Acceptance tests: add the build directory to the system PATH Cleber Rosa
  2019-11-07 19:46   ` Wainer dos Santos Moschetta
@ 2019-11-08 13:13   ` Philippe Mathieu-Daudé
  2019-11-15 23:19     ` Cleber Rosa
  1 sibling, 1 reply; 39+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-11-08 13:13 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Beraldo Leal, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Wainer dos Santos Moschetta,
	Willian Rampazzo, qemu-ppc, Aleksandar Rikalo, Aurelien Jarno,
	Eduardo Habkost

On 11/4/19 4:13 PM, Cleber Rosa wrote:
> So that when binaries such as qemu-img are searched for, those in the
> build tree will be favored.  As a clarification, SRC_ROOT_DIR is
> dependent on the location from where tests are executed, so they are
> equal to the build directory if one is being used.
> 
> The original motivation is that Avocado libraries such as
> avocado.utils.vmimage.get() may use the matching binaries, but it may
> also apply to any other binary that test code may eventually attempt
> to execute.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   tests/acceptance/avocado_qemu/__init__.py | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> index 17ce583c87..a4bb796a47 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -110,6 +110,12 @@ class Test(avocado.Test):
>           return None
>   
>       def setUp(self):
> +        # Some utility code uses binaries from the system's PATH.  For
> +        # instance, avocado.utils.vmimage.get() uses qemu-img, to
> +        # create a snapshot image.  This is a transparent way of
> +        # making sure those utilities find and use binaries on the
> +        # build tree by default.
> +        os.environ['PATH'] = '%s:%s' % (SRC_ROOT_DIR, os.environ['PATH'])

But qemu-img is compiled in BUILD_ROOT_DIR, isn't it?

Maybe we should pass its path by argument, such --qemu-img /path/to/it.

>           self._vms = {}
>   
>           self.arch = self.params.get('arch',
> 


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

* Re: [PATCH v7 2/8] Acceptance tests: introduce utility method for tags unique vals
  2019-11-04 15:13 ` [PATCH v7 2/8] Acceptance tests: introduce utility method for tags unique vals Cleber Rosa
@ 2019-11-08 13:14   ` Philippe Mathieu-Daudé
  2019-11-11 21:44     ` Cleber Rosa
  0 siblings, 1 reply; 39+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-11-08 13:14 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Beraldo Leal, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Wainer dos Santos Moschetta,
	Willian Rampazzo, qemu-ppc, Aleksandar Rikalo, Aurelien Jarno,
	Eduardo Habkost

On 11/4/19 4:13 PM, Cleber Rosa wrote:
> Currently a test can describe the target architecture binary that it
> should primarily be run with, be setting a single tag value.
> 
> The same approach is expected to be done with other QEMU aspects to be
> tested, for instance, the machine type and accelerator, so let's
> generalize the logic into a utility method.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   tests/acceptance/avocado_qemu/__init__.py | 19 +++++++++++++------
>   1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> index 9a57c020d8..e676d9c4e7 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -100,14 +100,21 @@ def exec_command_and_wait_for_pattern(test, command,
>   
>   
>   class Test(avocado.Test):
> +    def _get_unique_tag_val(self, tag_name):
> +        """
> +        Gets a tag value, if unique for a key

'Get'?

> +        """
> +        vals = self.tags.get(tag_name, [])
> +        if len(vals) == 1:
> +            return vals.pop()
> +        return None
> +
>       def setUp(self):
>           self._vms = {}
> -        arches = self.tags.get('arch', [])
> -        if len(arches) == 1:
> -            arch = arches.pop()
> -        else:
> -            arch = None
> -        self.arch = self.params.get('arch', default=arch)
> +
> +        self.arch = self.params.get('arch',
> +                                    default=self._get_unique_tag_val('arch'))
> +
>           default_qemu_bin = pick_default_qemu_bin(arch=self.arch)
>           self.qemu_bin = self.params.get('qemu_bin',
>                                           default=default_qemu_bin)
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>


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

* Re: [PATCH v7 3/8] Acceptance tests: use avocado tags for machine type
  2019-11-04 15:13 ` [PATCH v7 3/8] Acceptance tests: use avocado tags for machine type Cleber Rosa
@ 2019-11-08 13:20   ` Philippe Mathieu-Daudé
  2019-11-11 21:49     ` Cleber Rosa
  2019-11-12  1:59     ` Cleber Rosa
  0 siblings, 2 replies; 39+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-11-08 13:20 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Beraldo Leal, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Wainer dos Santos Moschetta,
	Willian Rampazzo, qemu-ppc, Aleksandar Rikalo, Aurelien Jarno,
	Eduardo Habkost

On 11/4/19 4:13 PM, Cleber Rosa wrote:
> The same way the arch tag is being used as a fallback for the arch
> parameter, let's do the same for QEMU's machine and avoid some boiler
> plate code.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   docs/devel/testing.rst                     | 18 ++++++++
>   tests/acceptance/avocado_qemu/__init__.py  |  5 ++
>   tests/acceptance/boot_linux_console.py     | 19 +-------
>   tests/acceptance/cpu_queries.py            |  2 +-
>   tests/acceptance/linux_initrd.py           |  2 +-
>   tests/acceptance/linux_ssh_mips_malta.py   |  5 --
>   tests/acceptance/machine_m68k_nextcube.py  | 21 ++-------
>   tests/acceptance/machine_sparc_leon3.py    |  3 +-
>   tests/acceptance/ppc_prep_40p.py           |  3 --
>   tests/acceptance/x86_cpu_model_versions.py | 53 ++++++++++++++++------
>   10 files changed, 72 insertions(+), 59 deletions(-)
> 
> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
> index 8e981e062d..27f286858a 100644
> --- a/docs/devel/testing.rst
> +++ b/docs/devel/testing.rst
> @@ -746,6 +746,17 @@ name.  If one is not given explicitly, it will either be set to
>   ``None``, or, if the test is tagged with one (and only one)
>   ``:avocado: tags=arch:VALUE`` tag, it will be set to ``VALUE``.
>   
> +machine
> +~~~~~~~
> +
> +The machine type that will be set to all QEMUMachine instances created
> +by the test.
> +
> +The ``machine`` attribute will be set to the test parameter of the same
> +name.  If one is not given explicitly, it will either be set to
> +``None``, or, if the test is tagged with one (and only one)
> +``:avocado: tags=machine:VALUE`` tag, it will be set to ``VALUE``.
> +
>   qemu_bin
>   ~~~~~~~~
>   
> @@ -781,6 +792,13 @@ architecture of a kernel or disk image to boot a VM with.
>   This parameter has a direct relation with the ``arch`` attribute.  If
>   not given, it will default to None.
>   
> +machine
> +~~~~~~~
> +
> +The machine type that will be set to all QEMUMachine instances created
> +by the test.
> +
> +
>   qemu_bin
>   ~~~~~~~~
>   
> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> index e676d9c4e7..6618ea67c1 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -115,6 +115,9 @@ class Test(avocado.Test):
>           self.arch = self.params.get('arch',
>                                       default=self._get_unique_tag_val('arch'))
>   
> +        self.machine = self.params.get('machine',
> +                                       default=self._get_unique_tag_val('machine'))
> +
>           default_qemu_bin = pick_default_qemu_bin(arch=self.arch)
>           self.qemu_bin = self.params.get('qemu_bin',
>                                           default=default_qemu_bin)
> @@ -136,6 +139,8 @@ class Test(avocado.Test):
>               name = str(uuid.uuid4())
>           if self._vms.get(name) is None:
>               self._vms[name] = self._new_vm(*args)
> +            if self.machine is not None:
> +                self._vms[name].set_machine(self.machine)
>           return self._vms[name]
>   
>       def tearDown(self):
> diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
> index 7e41cebd47..6732cc59ca 100644
> --- a/tests/acceptance/boot_linux_console.py
> +++ b/tests/acceptance/boot_linux_console.py
> @@ -62,7 +62,6 @@ class BootLinuxConsole(Test):
>           kernel_hash = '23bebd2680757891cf7adedb033532163a792495'
>           kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
>   
> -        self.vm.set_machine('pc')
>           self.vm.set_console()
>           kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE + 'console=ttyS0'
>           self.vm.add_args('-kernel', kernel_path,
> @@ -85,7 +84,6 @@ class BootLinuxConsole(Test):
>           kernel_path = self.extract_from_deb(deb_path,
>                                               '/boot/vmlinux-2.6.32-5-4kc-malta')
>   
> -        self.vm.set_machine('malta')
>           self.vm.set_console()
>           kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE + 'console=ttyS0'
>           self.vm.add_args('-kernel', kernel_path,
> @@ -118,7 +116,6 @@ class BootLinuxConsole(Test):
>           kernel_path = self.extract_from_deb(deb_path,
>                                               '/boot/vmlinux-2.6.32-5-5kc-malta')
>   
> -        self.vm.set_machine('malta')
>           self.vm.set_console()
>           kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE + 'console=ttyS0'
>           self.vm.add_args('-kernel', kernel_path,
> @@ -148,7 +145,6 @@ class BootLinuxConsole(Test):
>           initrd_path = self.workdir + "rootfs.cpio"
>           archive.gzip_uncompress(initrd_path_gz, initrd_path)
>   
> -        self.vm.set_machine('malta')
>           self.vm.set_console()
>           kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE
>                                  + 'console=ttyS0 console=tty '
> @@ -188,7 +184,6 @@ class BootLinuxConsole(Test):
>           initrd_path = self.workdir + "rootfs.cpio"
>           archive.gzip_uncompress(initrd_path_gz, initrd_path)
>   
> -        self.vm.set_machine('malta')
>           self.vm.set_console()
>           kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE
>                                  + 'console=ttyS0 console=tty '
> @@ -215,7 +210,6 @@ class BootLinuxConsole(Test):
>               with open(kernel_path, 'wb') as f_out:
>                   shutil.copyfileobj(f_in, f_out)
>   
> -        self.vm.set_machine('malta')
>           self.vm.set_console()
>           kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE
>                                  + 'mem=256m@@0x0 '
> @@ -275,7 +269,6 @@ class BootLinuxConsole(Test):
>           kernel_hash = '8c73e469fc6ea06a58dc83a628fc695b693b8493'
>           kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
>   
> -        self.vm.set_machine('virt')
>           self.vm.set_console()
>           kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
>                                  'console=ttyAMA0')
> @@ -297,7 +290,6 @@ class BootLinuxConsole(Test):
>           kernel_hash = 'e9826d741b4fb04cadba8d4824d1ed3b7fb8b4d4'
>           kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
>   
> -        self.vm.set_machine('virt')
>           self.vm.set_console()
>           kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
>                                  'console=ttyAMA0')
> @@ -310,7 +302,7 @@ class BootLinuxConsole(Test):
>       def test_arm_emcraft_sf2(self):
>           """
>           :avocado: tags=arch:arm
> -        :avocado: tags=machine:emcraft_sf2
> +        :avocado: tags=machine:emcraft-sf2

Maybe add a comment about this change, "Since avocado 72(?) we can ... 
so use ..."

>           :avocado: tags=endian:little
>           """
>           uboot_url = ('https://raw.githubusercontent.com/'
> @@ -324,7 +316,6 @@ class BootLinuxConsole(Test):
>           spi_hash = '85f698329d38de63aea6e884a86fbde70890a78a'
>           spi_path = self.fetch_asset(spi_url, asset_hash=spi_hash)
>   
> -        self.vm.set_machine('emcraft-sf2')
>           self.vm.set_console()
>           kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE
>           self.vm.add_args('-kernel', uboot_path,
> @@ -351,7 +342,6 @@ class BootLinuxConsole(Test):
>           kernel_path = self.extract_from_deb(deb_path, '/boot/kernel7.img')
>           dtb_path = self.extract_from_deb(deb_path, '/boot/bcm2709-rpi-2-b.dtb')
>   
> -        self.vm.set_machine('raspi2')
>           self.vm.set_console()
>           kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
>                                  serial_kernel_cmdline[uart_id])
> @@ -393,7 +383,6 @@ class BootLinuxConsole(Test):
>           initrd_path = os.path.join(self.workdir, 'rootfs.cpio')
>           archive.gzip_uncompress(initrd_path_gz, initrd_path)
>   
> -        self.vm.set_machine('smdkc210')
>           self.vm.set_console()
>           kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
>                                  'earlycon=exynos4210,0x13800000 earlyprintk ' +
> @@ -414,7 +403,7 @@ class BootLinuxConsole(Test):
>       def test_s390x_s390_ccw_virtio(self):
>           """
>           :avocado: tags=arch:s390x
> -        :avocado: tags=machine:s390_ccw_virtio
> +        :avocado: tags=machine:s390-ccw-virtio
>           """
>           kernel_url = ('https://archives.fedoraproject.org/pub/archive'
>                         '/fedora-secondary/releases/29/Everything/s390x/os/images'
> @@ -422,7 +411,6 @@ class BootLinuxConsole(Test):
>           kernel_hash = 'e8e8439103ef8053418ef062644ffd46a7919313'
>           kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
>   
> -        self.vm.set_machine('s390-ccw-virtio')
>           self.vm.set_console()
>           kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE + 'console=sclp0'
>           self.vm.add_args('-nodefaults',
> @@ -444,7 +432,6 @@ class BootLinuxConsole(Test):
>   
>           uncompressed_kernel = archive.uncompress(kernel_path, self.workdir)
>   
> -        self.vm.set_machine('clipper')
>           self.vm.set_console()
>           kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE + 'console=ttyS0'
>           self.vm.add_args('-vga', 'std',
> @@ -465,7 +452,6 @@ class BootLinuxConsole(Test):
>           kernel_hash = '3fe04abfc852b66653b8c3c897a59a689270bc77'
>           kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
>   
> -        self.vm.set_machine('pseries')
>           self.vm.set_console()
>           kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE + 'console=hvc0'
>           self.vm.add_args('-kernel', kernel_path,
> @@ -489,7 +475,6 @@ class BootLinuxConsole(Test):
>           kernel_path = self.extract_from_deb(deb_path,
>                                               '/boot/vmlinux-5.3.0-1-m68k')
>   
> -        self.vm.set_machine('q800')
>           self.vm.set_console()
>           kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
>                                  'console=ttyS0 vga=off')
> diff --git a/tests/acceptance/cpu_queries.py b/tests/acceptance/cpu_queries.py
> index af47d2795a..293dccb89a 100644
> --- a/tests/acceptance/cpu_queries.py
> +++ b/tests/acceptance/cpu_queries.py
> @@ -20,8 +20,8 @@ class QueryCPUModelExpansion(Test):
>       def test(self):
>           """
>           :avocado: tags=arch:x86_64
> +        :avocado: tags=machine:none

Not to confuse with None :)

>           """
> -        self.vm.set_machine('none')
>           self.vm.add_args('-S')
>           self.vm.launch()
>   
> diff --git a/tests/acceptance/linux_initrd.py b/tests/acceptance/linux_initrd.py
> index c61d9826a4..3a0ff7b098 100644
> --- a/tests/acceptance/linux_initrd.py
> +++ b/tests/acceptance/linux_initrd.py
> @@ -20,6 +20,7 @@ class LinuxInitrd(Test):
>       Checks QEMU evaluates correctly the initrd file passed as -initrd option.
>   
>       :avocado: tags=arch:x86_64
> +    :avocado: tags=machine:pc

For some tests we can run on multiple machines (here q35), I was tempted 
to use multiple tags. How could I do that now?

>       """
>   
>       timeout = 300
> @@ -66,7 +67,6 @@ class LinuxInitrd(Test):
>               initrd.write(b'\0')
>               initrd.flush()
>   
> -            self.vm.set_machine('pc')
>               self.vm.set_console()
>               kernel_command_line = 'console=ttyS0'
>               self.vm.add_args('-kernel', kernel_path,
> diff --git a/tests/acceptance/linux_ssh_mips_malta.py b/tests/acceptance/linux_ssh_mips_malta.py
> index fc13f9e4d4..1d570deb00 100644
> --- a/tests/acceptance/linux_ssh_mips_malta.py
> +++ b/tests/acceptance/linux_ssh_mips_malta.py
> @@ -111,7 +111,6 @@ class LinuxSSH(Test):
>           image_url, image_hash = self.get_image_info(endianess)
>           image_path = self.fetch_asset(image_url, asset_hash=image_hash)
>   
> -        self.vm.set_machine('malta')
>           self.vm.set_console()
>           kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE
>                                  + 'console=ttyS0 root=/dev/sda1')
> @@ -215,7 +214,6 @@ class LinuxSSH(Test):
>       def test_mips_malta32eb_kernel3_2_0(self):
>           """
>           :avocado: tags=arch:mips
> -        :avocado: tags=machine:malta
>           :avocado: tags=endian:big
>           :avocado: tags=device:pcnet32
>           """
> @@ -224,7 +222,6 @@ class LinuxSSH(Test):
>       def test_mips_malta32el_kernel3_2_0(self):
>           """
>           :avocado: tags=arch:mipsel
> -        :avocado: tags=machine:malta
>           :avocado: tags=endian:little
>           :avocado: tags=device:pcnet32
>           """
> @@ -233,7 +230,6 @@ class LinuxSSH(Test):
>       def test_mips_malta64eb_kernel3_2_0(self):
>           """
>           :avocado: tags=arch:mips64
> -        :avocado: tags=machine:malta
>           :avocado: tags=endian:big
>           :avocado: tags=device:pcnet32
>           """
> @@ -242,7 +238,6 @@ class LinuxSSH(Test):
>       def test_mips_malta64el_kernel3_2_0(self):
>           """
>           :avocado: tags=arch:mips64el
> -        :avocado: tags=machine:malta
>           :avocado: tags=endian:little
>           :avocado: tags=device:pcnet32
>           """
> diff --git a/tests/acceptance/machine_m68k_nextcube.py b/tests/acceptance/machine_m68k_nextcube.py
> index fcd2c58ee7..32cf571f94 100644
> --- a/tests/acceptance/machine_m68k_nextcube.py
> +++ b/tests/acceptance/machine_m68k_nextcube.py
> @@ -43,6 +43,11 @@ def tesseract_available(expected_version):
>   
>   
>   class NextCubeMachine(Test):
> +    """
> +    :avocado: tags=arch:m68k
> +    :avocado: tags=machine:next-cube
> +    :avocado: tags=device:framebuffer
> +    """
>   
>       timeout = 15
>   
> @@ -52,7 +57,6 @@ class NextCubeMachine(Test):
>           rom_hash = 'b3534796abae238a0111299fc406a9349f7fee24'
>           rom_path = self.fetch_asset(rom_url, asset_hash=rom_hash)
>   
> -        self.vm.set_machine('next-cube')
>           self.vm.add_args('-bios', rom_path)
>           self.vm.launch()
>   
> @@ -66,11 +70,6 @@ class NextCubeMachine(Test):
>   
>       @skipUnless(PIL_AVAILABLE, 'Python PIL not installed')
>       def test_bootrom_framebuffer_size(self):
> -        """
> -        :avocado: tags=arch:m68k
> -        :avocado: tags=machine:next_cube
> -        :avocado: tags=device:framebuffer
> -        """
>           screenshot_path = os.path.join(self.workdir, "dump.png")
>           self.check_bootrom_framebuffer(screenshot_path)
>   
> @@ -80,11 +79,6 @@ class NextCubeMachine(Test):
>   
>       @skipUnless(tesseract_available(3), 'tesseract v3 OCR tool not available')
>       def test_bootrom_framebuffer_ocr_with_tesseract_v3(self):
> -        """
> -        :avocado: tags=arch:m68k
> -        :avocado: tags=machine:next_cube
> -        :avocado: tags=device:framebuffer
> -        """
>           screenshot_path = os.path.join(self.workdir, "dump.png")
>           self.check_bootrom_framebuffer(screenshot_path)
>   
> @@ -101,11 +95,6 @@ class NextCubeMachine(Test):
>       # that it is still alpha-level software.
>       @skipUnless(tesseract_available(4), 'tesseract v4 OCR tool not available')
>       def test_bootrom_framebuffer_ocr_with_tesseract_v4(self):
> -        """
> -        :avocado: tags=arch:m68k
> -        :avocado: tags=machine:next_cube
> -        :avocado: tags=device:framebuffer
> -        """
>           screenshot_path = os.path.join(self.workdir, "dump.png")
>           self.check_bootrom_framebuffer(screenshot_path)
>   
> diff --git a/tests/acceptance/machine_sparc_leon3.py b/tests/acceptance/machine_sparc_leon3.py
> index 298f1e25e6..f77e210ccb 100644
> --- a/tests/acceptance/machine_sparc_leon3.py
> +++ b/tests/acceptance/machine_sparc_leon3.py
> @@ -16,7 +16,7 @@ class Leon3Machine(Test):
>       def test_leon3_helenos_uimage(self):
>           """
>           :avocado: tags=arch:sparc
> -        :avocado: tags=machine:leon3
> +        :avocado: tags=machine:leon3_generic
>           :avocado: tags=binfmt:uimage
>           """
>           kernel_url = ('http://www.helenos.org/releases/'
> @@ -24,7 +24,6 @@ class Leon3Machine(Test):
>           kernel_hash = 'a88c9cfdb8430c66650e5290a08765f9bf049a30'
>           kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
>   
> -        self.vm.set_machine('leon3_generic')
>           self.vm.set_console()
>           self.vm.add_args('-kernel', kernel_path)
>   
> diff --git a/tests/acceptance/ppc_prep_40p.py b/tests/acceptance/ppc_prep_40p.py
> index 6f507fb0a6..b27572f212 100644
> --- a/tests/acceptance/ppc_prep_40p.py
> +++ b/tests/acceptance/ppc_prep_40p.py
> @@ -39,7 +39,6 @@ class IbmPrep40pMachine(Test):
>           drive_hash = 'dbcfc09912e71bd5f0d82c7c1ee43082fb596ceb'
>           drive_path = self.fetch_asset(drive_url, asset_hash=drive_hash)
>   
> -        self.vm.set_machine('40p')
>           self.vm.set_console()
>           self.vm.add_args('-bios', bios_path,
>                            '-fda', drive_path)
> @@ -53,7 +52,6 @@ class IbmPrep40pMachine(Test):
>           :avocado: tags=arch:ppc
>           :avocado: tags=machine:40p
>           """
> -        self.vm.set_machine('40p')
>           self.vm.set_console()
>           self.vm.add_args('-m', '192') # test fw_cfg
>   
> @@ -73,7 +71,6 @@ class IbmPrep40pMachine(Test):
>           drive_hash = 'ac6fa2707d888b36d6fa64de6e7fe48e'
>           drive_path = self.fetch_asset(drive_url, asset_hash=drive_hash,
>                                         algorithm='md5')
> -        self.vm.set_machine('40p')
>           self.vm.set_console()
>           self.vm.add_args('-cdrom', drive_path,
>                            '-boot', 'd')
> diff --git a/tests/acceptance/x86_cpu_model_versions.py b/tests/acceptance/x86_cpu_model_versions.py
> index 6eb977954d..90558d9a71 100644
> --- a/tests/acceptance/x86_cpu_model_versions.py
> +++ b/tests/acceptance/x86_cpu_model_versions.py
> @@ -75,12 +75,15 @@ class X86CPUModelAliases(avocado_qemu.Test):
>                            "EPYC-IBPB shouldn't be versioned")
>   
>       def test_4_0_alias_compatibility(self):
> -        """Check if pc-*-4.0 unversioned CPU model won't be reported as aliases"""
> +        """
> +        Check if pc-*-4.0 unversioned CPU model won't be reported as aliases
> +
> +        :avocado: tags=machine:pc-i440fx-4.0
> +        """
>           # pc-*-4.0 won't expose non-versioned CPU models as aliases
>           # We do this to help management software to keep compatibility
>           # with older QEMU versions that didn't have the versioned CPU model
>           self.vm.add_args('-S')
> -        self.vm.set_machine('pc-i440fx-4.0')
>           self.vm.launch()
>           cpus = dict((m['name'], m) for m in self.vm.command('query-cpu-definitions'))
>   
> @@ -105,9 +108,12 @@ class X86CPUModelAliases(avocado_qemu.Test):
>               self.assertNotIn('alias-of', c, "%s shouldn't be an alias" % (name))
>   
>       def test_4_1_alias(self):
> -        """Check if unversioned CPU model is an alias pointing to right version"""
> +        """
> +        Check if unversioned CPU model is an alias pointing to right version
> +
> +        :avocado: tags=machine:pc-i440fx-4.1
> +        """
>           self.vm.add_args('-S')
> -        self.vm.set_machine('pc-i440fx-4.1')
>           self.vm.launch()
>   
>           cpus = dict((m['name'], m) for m in self.vm.command('query-cpu-definitions'))
> @@ -207,9 +213,12 @@ class X86CPUModelAliases(avocado_qemu.Test):
>           self.validate_aliases(cpus)
>   
>       def test_none_alias(self):
> -        """Check if unversioned CPU model is an alias pointing to some version"""
> +        """
> +        Check if unversioned CPU model is an alias pointing to some version
> +
> +        :avocado: tags=machine:none
> +        """
>           self.vm.add_args('-S')
> -        self.vm.set_machine('none')
>           self.vm.launch()
>   
>           cpus = dict((m['name'], m) for m in self.vm.command('query-cpu-definitions'))
> @@ -242,68 +251,84 @@ class CascadelakeArchCapabilities(avocado_qemu.Test):
>           return self.vm.command('qom-get', path=cpu_path, property=prop)
>   
>       def test_4_1(self):
> +        """
> +        :avocado: tags=machine:pc-i440fx-4.1
> +        """
>           # machine-type only:
>           self.vm.add_args('-S')
> -        self.vm.set_machine('pc-i440fx-4.1')
>           self.vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off')
>           self.vm.launch()
>           self.assertFalse(self.get_cpu_prop('arch-capabilities'),
>                            'pc-i440fx-4.1 + Cascadelake-Server should not have arch-capabilities')
>   
>       def test_4_0(self):
> +        """
> +        :avocado: tags=machine:pc-i440fx-4.0
> +        """
>           self.vm.add_args('-S')
> -        self.vm.set_machine('pc-i440fx-4.0')
>           self.vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off')
>           self.vm.launch()
>           self.assertFalse(self.get_cpu_prop('arch-capabilities'),
>                            'pc-i440fx-4.0 + Cascadelake-Server should not have arch-capabilities')
>   
>       def test_set_4_0(self):
> +        """
> +        :avocado: tags=machine:pc-i440fx-4.0
> +        """
>           # command line must override machine-type if CPU model is not versioned:
>           self.vm.add_args('-S')
> -        self.vm.set_machine('pc-i440fx-4.0')
>           self.vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off,+arch-capabilities')
>           self.vm.launch()
>           self.assertTrue(self.get_cpu_prop('arch-capabilities'),
>                           'pc-i440fx-4.0 + Cascadelake-Server,+arch-capabilities should have arch-capabilities')
>   
>       def test_unset_4_1(self):
> +        """
> +        :avocado: tags=machine:pc-i440fx-4.1
> +        """
>           self.vm.add_args('-S')
> -        self.vm.set_machine('pc-i440fx-4.1')
>           self.vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off,-arch-capabilities')
>           self.vm.launch()
>           self.assertFalse(self.get_cpu_prop('arch-capabilities'),
>                            'pc-i440fx-4.1 + Cascadelake-Server,-arch-capabilities should not have arch-capabilities')
>   
>       def test_v1_4_0(self):
> +        """
> +        :avocado: tags=machine:pc-i440fx-4.0
> +        """
>           # versioned CPU model overrides machine-type:
>           self.vm.add_args('-S')
> -        self.vm.set_machine('pc-i440fx-4.0')
>           self.vm.add_args('-cpu', 'Cascadelake-Server-v1,x-force-features=on,check=off,enforce=off')
>           self.vm.launch()
>           self.assertFalse(self.get_cpu_prop('arch-capabilities'),
>                            'pc-i440fx-4.0 + Cascadelake-Server-v1 should not have arch-capabilities')
>   
>       def test_v2_4_0(self):
> +        """
> +        :avocado: tags=machine:pc-i440fx-4.0
> +        """
>           self.vm.add_args('-S')
> -        self.vm.set_machine('pc-i440fx-4.0')
>           self.vm.add_args('-cpu', 'Cascadelake-Server-v2,x-force-features=on,check=off,enforce=off')
>           self.vm.launch()
>           self.assertTrue(self.get_cpu_prop('arch-capabilities'),
>                           'pc-i440fx-4.0 + Cascadelake-Server-v2 should have arch-capabilities')
>   
>       def test_v1_set_4_0(self):
> +        """
> +        :avocado: tags=machine:pc-i440fx-4.0
> +        """
>           # command line must override machine-type and versioned CPU model:
>           self.vm.add_args('-S')
> -        self.vm.set_machine('pc-i440fx-4.0')
>           self.vm.add_args('-cpu', 'Cascadelake-Server-v1,x-force-features=on,check=off,enforce=off,+arch-capabilities')
>           self.vm.launch()
>           self.assertTrue(self.get_cpu_prop('arch-capabilities'),
>                           'pc-i440fx-4.0 + Cascadelake-Server-v1,+arch-capabilities should have arch-capabilities')
>   
>       def test_v2_unset_4_1(self):
> +        """
> +        :avocado: tags=machine:pc-i440fx-4.1
> +        """
>           self.vm.add_args('-S')
> -        self.vm.set_machine('pc-i440fx-4.1')
>           self.vm.add_args('-cpu', 'Cascadelake-Server-v2,x-force-features=on,check=off,enforce=off,-arch-capabilities')
>           self.vm.launch()
>           self.assertFalse(self.get_cpu_prop('arch-capabilities'),
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>


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

* Re: [PATCH v7 8/8] Acceptance test: add "boot_linux" tests
  2019-11-04 15:13 ` [PATCH v7 8/8] Acceptance test: add "boot_linux" tests Cleber Rosa
@ 2019-11-08 19:42   ` Wainer dos Santos Moschetta
  2019-11-15 23:47     ` Cleber Rosa
  2019-11-12 18:20   ` Philippe Mathieu-Daudé
  2019-12-03 19:19   ` Alex Bennée
  2 siblings, 1 reply; 39+ messages in thread
From: Wainer dos Santos Moschetta @ 2019-11-08 19:42 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Beraldo Leal, Fabien Chouteau, KONRAD Frederic, qemu-ppc,
	Willian Rampazzo, Hervé Poussineau, Aleksandar Rikalo,
	Philippe Mathieu-Daudé,
	Aurelien Jarno, Eduardo Habkost


On 11/4/19 1:13 PM, Cleber Rosa wrote:
> This acceptance test, validates that a full blown Linux guest can
> successfully boot in QEMU.  In this specific case, the guest chosen is
> Fedora version 31.
>
>   * x86_64, pc and q35 machine types, with and without kvm as an
>     accellerator
>
>   * aarch64 and virt machine type, with and without kvm as an
>     accellerator
>
>   * ppc64 and pseries machine type
>
>   * s390x and s390-ccw-virtio machine type
>
> The method for checking the successful boot is based on "cloudinit"
> and its "phone home" feature.  The guest is given an ISO image
> with the location of the phone home server, and the information to
> post (the instance ID).  Upon receiving the correct information,
> from the guest, the test is considered to have PASSed.
>
> This test is currently limited to user mode networking only, and
> instructs the guest to connect to the "router" address that is hard
> coded in QEMU.
>
> To create the cloudinit ISO image that will be used to configure the
> guest, the pycdlib library is also required and has been added as
> requirement to the virtual environment created by "check-venv".
>
> The console output is read by a separate thread, by means of the
> Avocado datadrainer utility module.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   tests/acceptance/boot_linux.py | 175 +++++++++++++++++++++++++++++++++
>   tests/requirements.txt         |   1 +
>   2 files changed, 176 insertions(+)
>   create mode 100644 tests/acceptance/boot_linux.py
>
> diff --git a/tests/acceptance/boot_linux.py b/tests/acceptance/boot_linux.py
> new file mode 100644
> index 0000000000..882f7dc5df
> --- /dev/null
> +++ b/tests/acceptance/boot_linux.py
> @@ -0,0 +1,175 @@
> +# Functional test that boots a complete Linux system via a cloud image
> +#
> +# Copyright (c) 2018-2019 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.
> +
> +import os
> +
> +from avocado_qemu import Test, SRC_ROOT_DIR
> +
> +from qemu import kvm_available
> +
> +from avocado.utils import cloudinit
> +from avocado.utils import network
> +from avocado.utils import vmimage
> +from avocado.utils import datadrainer
> +
> +
> +KVM_NOT_AVAILABLE = "KVM accelerator does not seem to be available"
> +
> +
> +class BootLinux(Test):
> +    """
> +    Boots a Linux system, checking for a successful initialization
> +    """
> +
> +    timeout = 600
> +    chksum = None
> +
> +    def setUp(self):
> +        super(BootLinux, self).setUp()
> +        self.prepare_boot()
> +        self.vm.add_args('-smp', '2')
> +        self.vm.add_args('-m', '2048')
> +        self.vm.add_args('-drive', 'file=%s' % self.boot.path)
> +        self.prepare_cloudinit()
> +
> +    def prepare_boot(self):
> +        self.log.info('Downloading/preparing boot image')
Cosmetic: replace 'Downloading/preparing' with 'Downloading' since this 
function does only download the boot image.
> +        # Fedora 31 only provides ppc64le images
> +        image_arch = self.arch
> +        if image_arch == 'ppc64':
> +            image_arch = 'ppc64le'
> +        try:
> +            self.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')

Likewise.

> +
> +    def prepare_cloudinit(self):
> +        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)
> +            self.vm.add_args('-drive', 'file=%s,format=raw' % cloudinit_iso)
> +        except Exception:
> +            self.cancel('Failed to prepared cloudinit image')
> +
> +    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):
> +    """
> +    :avocado: tags=arch:x86_64
> +    """
> +
> +    chksum = 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'
> +
> +    def test_pc(self):
> +        """
> +        :avocado: tags=machine:pc
> +        """
> +        self.launch_and_wait()
> +
> +    def test_kvm_pc(self):
> +        """
> +        :avocado: tags=machine:pc
> +        :avocado: tags=accel:kvm
> +        """
> +        if not kvm_available(self.arch):
> +            self.cancel(KVM_NOT_AVAILABLE)

kvm_available() solely checks for kvm presence on host, not if enabled 
in the target binary. That said, as a follow up we could adapt this test 
to use a strengthen approach as I proposed in patch 02 of '[PATCH 0/3] 
Acceptance tests: boot Linux with KVM test' [1] series.

[1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg627498.html

> +        self.vm.add_args("-accel", "kvm")
> +        self.launch_and_wait()
> +
> +    def test_q35(self):
> +        """
> +        :avocado: tags=machine:q35
> +        """
> +        self.launch_and_wait()
> +
> +    def test_kvm_q35(self):
> +        """
> +        :avocado: tags=machine:q35
> +        :avocado: tags=accel:kvm
> +        """
> +        if not kvm_available(self.arch):
> +            self.cancel(KVM_NOT_AVAILABLE)
> +        self.vm.add_args("-accel", "kvm")
> +        self.launch_and_wait()
> +
> +
> +class BootLinuxAarch64(BootLinux):
> +    """
> +    :avocado: tags=arch:aarch64
> +    :avocado: tags=machine:virt
> +    """
> +
> +    chksum = '1e18d9c0cf734940c4b5d5ec592facaed2af0ad0329383d5639c997fdf16fe49'
> +
> +    def test_virt(self):
> +        self.vm.add_args('-cpu', 'cortex-a53')
> +        self.vm.add_args('-bios',
> +                         os.path.join(SRC_ROOT_DIR, 'pc-bios',
> +                                      'edk2-aarch64-code.fd'))
> +        self.vm.add_args('-device', 'virtio-rng-pci,rng=rng0')
> +        self.vm.add_args('-object', 'rng-random,id=rng0,filename=/dev/urandom')
> +        self.launch_and_wait()
> +
> +    def test_kvm_virt(self):
> +        """
> +        :avocado: tags=accel:kvm
> +        """
> +        if not kvm_available(self.arch):
> +            self.cancel(KVM_NOT_AVAILABLE)
> +        self.vm.add_args("-accel", "kvm")
> +        self.test_virt()
> +
> +
> +class BootLinuxPPC64(BootLinux):
> +    """
> +    :avocado: tags=arch:ppc64
> +    """
> +
> +    chksum = '7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58'
> +
> +    def test_pseries(self):
> +        """
> +        :avocado: tags=machine:pseries
> +        """
> +        self.launch_and_wait()
> +
> +
> +class BootLinuxS390X(BootLinux):
> +    """
> +    :avocado: tags=arch:s390x
> +    """
> +
> +    chksum = '4caaab5a434fd4d1079149a072fdc7891e354f834d355069ca982fdcaf5a122d'
> +
> +    def test_s390_ccw_virtio(self):
> +        """
> +        :avocado: tags=machine:s390-ccw-virtio
> +        """
> +        self.launch_and_wait()
> diff --git a/tests/requirements.txt b/tests/requirements.txt
> index a2a587223a..3893361e0c 100644
> --- a/tests/requirements.txt
> +++ b/tests/requirements.txt
> @@ -2,3 +2,4 @@
>   # in the tests/venv Python virtual environment. For more info,
>   # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
>   avocado-framework==72.0
> +pycdlib==1.8.0

Besides the cosmetics suggestions, this test is ready to go.

In other to write powerful tests there will be needed a mechanism to 
boot a full blown guest which can be interacted with. I see the proposed 
BootLinux class here as the first milestone towards that goal. Another 
reason to merge this as soon as possible.

Ran the tests on my x86_68 machine with RHEL 8.1, and all pass 
(BootLinuxAarch64.test_kvm_virt correctly skipped).

Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

Thanks,

Wainer



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

* Re: [PATCH v7 2/8] Acceptance tests: introduce utility method for tags unique vals
  2019-11-08 13:14   ` Philippe Mathieu-Daudé
@ 2019-11-11 21:44     ` Cleber Rosa
  0 siblings, 0 replies; 39+ messages in thread
From: Cleber Rosa @ 2019-11-11 21:44 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Beraldo Leal, qemu-devel, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Wainer dos Santos Moschetta,
	Willian Rampazzo, qemu-ppc, Aleksandar Rikalo, Aurelien Jarno,
	Eduardo Habkost

On Fri, Nov 08, 2019 at 02:14:58PM +0100, Philippe Mathieu-Daudé wrote:
> On 11/4/19 4:13 PM, Cleber Rosa wrote:
> > Currently a test can describe the target architecture binary that it
> > should primarily be run with, be setting a single tag value.
> > 
> > The same approach is expected to be done with other QEMU aspects to be
> > tested, for instance, the machine type and accelerator, so let's
> > generalize the logic into a utility method.
> > 
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > ---
> >   tests/acceptance/avocado_qemu/__init__.py | 19 +++++++++++++------
> >   1 file changed, 13 insertions(+), 6 deletions(-)
> > 
> > diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> > index 9a57c020d8..e676d9c4e7 100644
> > --- a/tests/acceptance/avocado_qemu/__init__.py
> > +++ b/tests/acceptance/avocado_qemu/__init__.py
> > @@ -100,14 +100,21 @@ def exec_command_and_wait_for_pattern(test, command,
> >   class Test(avocado.Test):
> > +    def _get_unique_tag_val(self, tag_name):
> > +        """
> > +        Gets a tag value, if unique for a key
> 
> 'Get'?
> 

Not sure what's better, I was thinking along the lines that *this
method* gets (one) a tag value...  But you may be right.

- Cleber.



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

* Re: [PATCH v7 3/8] Acceptance tests: use avocado tags for machine type
  2019-11-08 13:20   ` Philippe Mathieu-Daudé
@ 2019-11-11 21:49     ` Cleber Rosa
  2019-11-12 18:15       ` Philippe Mathieu-Daudé
  2019-11-12  1:59     ` Cleber Rosa
  1 sibling, 1 reply; 39+ messages in thread
From: Cleber Rosa @ 2019-11-11 21:49 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Beraldo Leal, qemu-devel, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Wainer dos Santos Moschetta,
	Willian Rampazzo, qemu-ppc, Aleksandar Rikalo, Aurelien Jarno,
	Eduardo Habkost

On Fri, Nov 08, 2019 at 02:20:45PM +0100, Philippe Mathieu-Daudé wrote:
> > @@ -310,7 +302,7 @@ class BootLinuxConsole(Test):
> >       def test_arm_emcraft_sf2(self):
> >           """
> >           :avocado: tags=arch:arm
> > -        :avocado: tags=machine:emcraft_sf2
> > +        :avocado: tags=machine:emcraft-sf2
> 
> Maybe add a comment about this change, "Since avocado 72(?) we can ... so
> use ..."
> 

You mean on this specific test docstring?  I'm confused if there's a
special reason for doing it here, of if you're suggesting adding a
similar command to all tag entries which make use of the extended
character set (which I think would be too verbose, repetitve, and hard
to maintain).

> > diff --git a/tests/acceptance/cpu_queries.py b/tests/acceptance/cpu_queries.py
> > index af47d2795a..293dccb89a 100644
> > --- a/tests/acceptance/cpu_queries.py
> > +++ b/tests/acceptance/cpu_queries.py
> > @@ -20,8 +20,8 @@ class QueryCPUModelExpansion(Test):
> >       def test(self):
> >           """
> >           :avocado: tags=arch:x86_64
> > +        :avocado: tags=machine:none
> 
> Not to confuse with None :)
> 

Yep! :)

- Cleber.



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

* Re: [PATCH v7 4/8] Acceptance tests: use relative location for tests
  2019-11-04 18:26   ` Philippe Mathieu-Daudé
@ 2019-11-11 22:11     ` Cleber Rosa
  2019-11-12 18:17       ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 39+ messages in thread
From: Cleber Rosa @ 2019-11-11 22:11 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Beraldo Leal, qemu-devel, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Wainer dos Santos Moschetta,
	Willian Rampazzo, qemu-ppc, Aleksandar Rikalo, Aurelien Jarno,
	Eduardo Habkost

On Mon, Nov 04, 2019 at 07:26:23PM +0100, Philippe Mathieu-Daudé wrote:
> On 11/4/19 4:13 PM, Cleber Rosa wrote:
> > An Avocado Test ID[1] is composed by a number of components, but it
> > starts with the Test Name, usually a file system location that was
> > given to the loader.
> > 
> > Because the source directory is being given as a prefix to the
> > "tests/acceptance" directory containing the acceptance tests, the test
> > names will needlessly include the directory the user is using to host
> > the QEMU sources (and/or build tree).
> > 
> > Let's remove the source dir (or a build dir) from the path given to
> > the test loader.  This should give more constant names, and when using
> > result servers and databases, it should give the same test names
> > across executions from different people or from different directories.
> 
> Can we strip the full path to directory and only keep the filename in the
> database? (Thinking about out-of-tree tests).
>

Yes, absolutely, but this needs to be done one the Avocado side.  TBH,
I have ideas to make this go even further, such as:

 1) the stripping of the "test_" prefix of the test method

 2) replace the full path to a directory with tests for a "test suite"
    alias (default to the directory name itself)

 3) test suite alias will be persisted on test result such as reports
    or machine, but ommited from the human UI

 4) full path to directory, exact version of test files (git hash) will
    all be metadata and not part of the test ID

Roughly speaking, something which is listed like this currently:

  $ avocado list tests/acceptance/
  INSTRUMENTED tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_x86_64_pc
  ...

When executed, would be shown as:

  JOB ID     : fb885e9c3e7dc50534ec380a7e988cbf94233f07
  JOB LOG    : /home/cleber/avocado/job-results/job-2019-11-11T17.07-fb885e9/job.log
   (1/1) acceptance/boot_linux_console.py:BootLinuxConsole.x86_64_pc: PASS (2.17 s)
  RESULTS    : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
  JOB TIME   : 2.35 s

How does that sound?

- Cleber.



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

* Re: [PATCH v7 5/8] Acceptance tests: keep a stable reference to the QEMU build dir
  2019-11-07 19:22   ` Wainer dos Santos Moschetta
@ 2019-11-11 22:38     ` Cleber Rosa
  2019-11-15 21:36     ` Cleber Rosa
  1 sibling, 0 replies; 39+ messages in thread
From: Cleber Rosa @ 2019-11-11 22:38 UTC (permalink / raw)
  To: Wainer dos Santos Moschetta
  Cc: Beraldo Leal, qemu-devel, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Willian Rampazzo, qemu-ppc,
	Aleksandar Rikalo, Philippe Mathieu-Daudé,
	Aurelien Jarno, Eduardo Habkost

On Thu, Nov 07, 2019 at 05:22:24PM -0200, Wainer dos Santos Moschetta wrote:
> 
> On 11/4/19 1:13 PM, Cleber Rosa wrote:
> > This is related to the the differences in in-tree and out-of-tree
> > builds in QEMU.  For simplification, <BLD> means my build directory.
> > 
> > Currently, by running a `make check-acceptance` one gets (in
> > tests/acceptance/avocado_qemu/__init__.py):
> > 
> >     SRC_ROOT_DIR: <BLD>/tests/acceptance/avocado_qemu/../../..
> > 
> > This in itself is problematic, because after the parent directories
> > are applied, one may be left not with a pointer to the build directory
> > as intended, but with the location of the source tree (assuming they
> > differ). Built binaries, such as qemu-img, are of course not there and
> > can't be found.
> > 
> > Given that a Python '__file__' will contain the absolute path to the
> > file backing the module, say:
> > 
> >     __file__: <BLD>/tests/acceptance/avocado_qemu/__init__.py
> > 
> >                    |  4  |     3    |      2     |     1     |
> > 
> > A solution is to not "evaluate" the third parent dir (marked as 4
> > here) because that ends up following the "tests" directory symlink to
> > the source tree.  In fact, there's no need to keep or evaluate any of
> > the parent directories, we can just drop the rightmost 4 components,
> > and we'll keep a stable reference to the build directory (with no
> > symlink being followed).  This works for either a dedicated build
> > directory or also a combined source and build tree.
> > 
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > ---
> >   tests/acceptance/avocado_qemu/__init__.py | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> > index 6618ea67c1..17ce583c87 100644
> > --- a/tests/acceptance/avocado_qemu/__init__.py
> > +++ b/tests/acceptance/avocado_qemu/__init__.py
> > @@ -16,7 +16,7 @@ import tempfile
> >   import avocado
> > -SRC_ROOT_DIR = os.path.join(os.path.dirname(__file__), '..', '..', '..')
> > +SRC_ROOT_DIR = os.path.dirname(os.path.dirname(os.path.dirname(os.path.dirname(__file__))))
> 
> In this case, wouldn't make sense to rename the constant from SRC_ROOT_DIR
> to BUILD_ROOT_DIR?
>

True.  I remember thinking about doing that as a separate change and
ended up forgetting.  Maybe it's better to just do it here.

> This patch looks good to me besides that.
> 
> - Wainer
>

Thanks!
- Cleber.



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

* Re: [PATCH v7 6/8] Acceptance tests: add the build directory to the system PATH
  2019-11-07 19:46   ` Wainer dos Santos Moschetta
@ 2019-11-11 22:49     ` Cleber Rosa
  2019-11-12 14:00       ` Wainer dos Santos Moschetta
  0 siblings, 1 reply; 39+ messages in thread
From: Cleber Rosa @ 2019-11-11 22:49 UTC (permalink / raw)
  To: Wainer dos Santos Moschetta
  Cc: Beraldo Leal, qemu-devel, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Willian Rampazzo, qemu-ppc,
	Aleksandar Rikalo, Philippe Mathieu-Daudé,
	Aurelien Jarno, Eduardo Habkost

On Thu, Nov 07, 2019 at 05:46:13PM -0200, Wainer dos Santos Moschetta wrote:
> 
> On 11/4/19 1:13 PM, Cleber Rosa wrote:
> > So that when binaries such as qemu-img are searched for, those in the
> > build tree will be favored.  As a clarification, SRC_ROOT_DIR is
> > dependent on the location from where tests are executed, so they are
> > equal to the build directory if one is being used.
> > 
> > The original motivation is that Avocado libraries such as
> > avocado.utils.vmimage.get() may use the matching binaries, but it may
> > also apply to any other binary that test code may eventually attempt
> > to execute.
> > 
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > ---
> >   tests/acceptance/avocado_qemu/__init__.py | 6 ++++++
> >   1 file changed, 6 insertions(+)
> > 
> > diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> > index 17ce583c87..a4bb796a47 100644
> > --- a/tests/acceptance/avocado_qemu/__init__.py
> > +++ b/tests/acceptance/avocado_qemu/__init__.py
> > @@ -110,6 +110,12 @@ class Test(avocado.Test):
> >           return None
> >       def setUp(self):
> > +        # Some utility code uses binaries from the system's PATH.  For
> > +        # instance, avocado.utils.vmimage.get() uses qemu-img, to
> > +        # create a snapshot image.  This is a transparent way of
> 
> Because PATH is changed in a transparent way, wouldn't be better to also
> self.log.info() that fact?
>

I don't have a problem with logging it, but because it will happen for
*every single* test, it seems like it will become noise.  I think it's
better to properly document this aspect of "avocado_qemu.Test" instead
(which is currently missing here).  Something like:

"Tests based on avocado_qemu.Test will have, as a convenience, the 
QEMU build directory added to their PATH environment variable.  The goal
is to allow tests to seamless use matching built binaries, instead of
binaries installed elsewhere in the system".

How does it sound?

> > +        # making sure those utilities find and use binaries on the
> > +        # build tree by default.
> > +        os.environ['PATH'] = '%s:%s' % (SRC_ROOT_DIR, os.environ['PATH'])
> 
> I think PATH should be set only once at class initialization. Perhaps in
> setUpClass()?
> 
> - Wainer
>

The Avocado test isolation model makes setUpClass() unnecessary,
unsupported and pointless, so we only support setUp().

Every test already runs on its own process, and with the nrunner
model, should be able to run on completely different systems.  That's
why we don't want to support a setUpClass() like approach.

- Cleber.



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

* Re: [PATCH v7 3/8] Acceptance tests: use avocado tags for machine type
  2019-11-08 13:20   ` Philippe Mathieu-Daudé
  2019-11-11 21:49     ` Cleber Rosa
@ 2019-11-12  1:59     ` Cleber Rosa
  2019-11-12 18:15       ` Philippe Mathieu-Daudé
  1 sibling, 1 reply; 39+ messages in thread
From: Cleber Rosa @ 2019-11-12  1:59 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Beraldo Leal, Jan Richter, qemu-devel, Fabien Chouteau,
	KONRAD Frederic, Hervé Poussineau,
	Wainer dos Santos Moschetta, Willian Rampazzo, qemu-ppc,
	Aleksandar Rikalo, Aurelien Jarno, Eduardo Habkost

On Fri, Nov 08, 2019 at 02:20:45PM +0100, Philippe Mathieu-Daudé wrote:
> On 11/4/19 4:13 PM, Cleber Rosa wrote:
> >           """
> > -        self.vm.set_machine('none')
> >           self.vm.add_args('-S')
> >           self.vm.launch()
> > diff --git a/tests/acceptance/linux_initrd.py b/tests/acceptance/linux_initrd.py
> > index c61d9826a4..3a0ff7b098 100644
> > --- a/tests/acceptance/linux_initrd.py
> > +++ b/tests/acceptance/linux_initrd.py
> > @@ -20,6 +20,7 @@ class LinuxInitrd(Test):
> >       Checks QEMU evaluates correctly the initrd file passed as -initrd option.
> >       :avocado: tags=arch:x86_64
> > +    :avocado: tags=machine:pc
> 
> For some tests we can run on multiple machines (here q35), I was tempted to
> use multiple tags. How could I do that now?
>

I missed this comment: you can add many tag values here to *classify*
the test as being "q35 machine type capable".

But, Avocado will only run a test multiple times with a varianter
plugin active.  In that case, a "machine" *parameter* with different
values will be passed to the tests.  This tag value is being used
as a default value for the parameter, so it has a lower precedence.

We have a pending task[1] to create an initial CIT file for arch and
machine types.

CC'ing Jan Richter, who is supposed to start working on it soon.

- Cleber.

[1] - https://trello.com/c/1wvzcxHY/105-create-cit-parameter-for-acceptance-tests



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

* Re: [PATCH v7 6/8] Acceptance tests: add the build directory to the system PATH
  2019-11-11 22:49     ` Cleber Rosa
@ 2019-11-12 14:00       ` Wainer dos Santos Moschetta
  2019-11-15 23:17         ` Cleber Rosa
  0 siblings, 1 reply; 39+ messages in thread
From: Wainer dos Santos Moschetta @ 2019-11-12 14:00 UTC (permalink / raw)
  To: Cleber Rosa
  Cc: Beraldo Leal, qemu-devel, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Willian Rampazzo, qemu-ppc,
	Aleksandar Rikalo, Philippe Mathieu-Daudé,
	Aurelien Jarno, Eduardo Habkost


On 11/11/19 8:49 PM, Cleber Rosa wrote:
> On Thu, Nov 07, 2019 at 05:46:13PM -0200, Wainer dos Santos Moschetta wrote:
>> On 11/4/19 1:13 PM, Cleber Rosa wrote:
>>> So that when binaries such as qemu-img are searched for, those in the
>>> build tree will be favored.  As a clarification, SRC_ROOT_DIR is
>>> dependent on the location from where tests are executed, so they are
>>> equal to the build directory if one is being used.
>>>
>>> The original motivation is that Avocado libraries such as
>>> avocado.utils.vmimage.get() may use the matching binaries, but it may
>>> also apply to any other binary that test code may eventually attempt
>>> to execute.
>>>
>>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>>> ---
>>>    tests/acceptance/avocado_qemu/__init__.py | 6 ++++++
>>>    1 file changed, 6 insertions(+)
>>>
>>> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
>>> index 17ce583c87..a4bb796a47 100644
>>> --- a/tests/acceptance/avocado_qemu/__init__.py
>>> +++ b/tests/acceptance/avocado_qemu/__init__.py
>>> @@ -110,6 +110,12 @@ class Test(avocado.Test):
>>>            return None
>>>        def setUp(self):
>>> +        # Some utility code uses binaries from the system's PATH.  For
>>> +        # instance, avocado.utils.vmimage.get() uses qemu-img, to
>>> +        # create a snapshot image.  This is a transparent way of
>> Because PATH is changed in a transparent way, wouldn't be better to also
>> self.log.info() that fact?
>>
> I don't have a problem with logging it, but because it will happen for
> *every single* test, it seems like it will become noise.  I think it's
> better to properly document this aspect of "avocado_qemu.Test" instead
> (which is currently missing here).  Something like:
>
> "Tests based on avocado_qemu.Test will have, as a convenience, the
> QEMU build directory added to their PATH environment variable.  The goal
> is to allow tests to seamless use matching built binaries, instead of
> binaries installed elsewhere in the system".
>
> How does it sound?


It does.


>
>>> +        # making sure those utilities find and use binaries on the
>>> +        # build tree by default.
>>> +        os.environ['PATH'] = '%s:%s' % (SRC_ROOT_DIR, os.environ['PATH'])
>> I think PATH should be set only once at class initialization. Perhaps in
>> setUpClass()?
>>
>> - Wainer
>>
> The Avocado test isolation model makes setUpClass() unnecessary,
> unsupported and pointless, so we only support setUp().
>
> Every test already runs on its own process, and with the nrunner
> model, should be able to run on completely different systems.  That's
> why we don't want to support a setUpClass() like approach.

Okay, thanks for the explanation.

Thanks,

Wainer

>
> - Cleber.
>
>



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

* Re: [PATCH v7 3/8] Acceptance tests: use avocado tags for machine type
  2019-11-11 21:49     ` Cleber Rosa
@ 2019-11-12 18:15       ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 39+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-11-12 18:15 UTC (permalink / raw)
  To: Cleber Rosa
  Cc: Beraldo Leal, qemu-devel, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Wainer dos Santos Moschetta,
	Willian Rampazzo, qemu-ppc, Aleksandar Rikalo, Aurelien Jarno,
	Eduardo Habkost

On 11/11/19 10:49 PM, Cleber Rosa wrote:
> On Fri, Nov 08, 2019 at 02:20:45PM +0100, Philippe Mathieu-Daudé wrote:
>>> @@ -310,7 +302,7 @@ class BootLinuxConsole(Test):
>>>        def test_arm_emcraft_sf2(self):
>>>            """
>>>            :avocado: tags=arch:arm
>>> -        :avocado: tags=machine:emcraft_sf2
>>> +        :avocado: tags=machine:emcraft-sf2
>>
>> Maybe add a comment about this change, "Since avocado 72(?) we can ... so
>> use ..."
>>
> 
> You mean on this specific test docstring?  I'm confused if there's a

No! Just in the commit description :)

> special reason for doing it here, of if you're suggesting adding a
> similar command to all tag entries which make use of the extended
> character set (which I think would be too verbose, repetitve, and hard
> to maintain).
> 
>>> diff --git a/tests/acceptance/cpu_queries.py b/tests/acceptance/cpu_queries.py
>>> index af47d2795a..293dccb89a 100644
>>> --- a/tests/acceptance/cpu_queries.py
>>> +++ b/tests/acceptance/cpu_queries.py
>>> @@ -20,8 +20,8 @@ class QueryCPUModelExpansion(Test):
>>>        def test(self):
>>>            """
>>>            :avocado: tags=arch:x86_64
>>> +        :avocado: tags=machine:none
>>
>> Not to confuse with None :)
>>
> 
> Yep! :)
> 
> - Cleber.
> 



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

* Re: [PATCH v7 3/8] Acceptance tests: use avocado tags for machine type
  2019-11-12  1:59     ` Cleber Rosa
@ 2019-11-12 18:15       ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 39+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-11-12 18:15 UTC (permalink / raw)
  To: Cleber Rosa
  Cc: Beraldo Leal, Jan Richter, qemu-devel, Fabien Chouteau,
	KONRAD Frederic, Hervé Poussineau,
	Wainer dos Santos Moschetta, Willian Rampazzo, qemu-ppc,
	Aleksandar Rikalo, Aurelien Jarno, Eduardo Habkost

On 11/12/19 2:59 AM, Cleber Rosa wrote:
> On Fri, Nov 08, 2019 at 02:20:45PM +0100, Philippe Mathieu-Daudé wrote:
>> On 11/4/19 4:13 PM, Cleber Rosa wrote:
>>>            """
>>> -        self.vm.set_machine('none')
>>>            self.vm.add_args('-S')
>>>            self.vm.launch()
>>> diff --git a/tests/acceptance/linux_initrd.py b/tests/acceptance/linux_initrd.py
>>> index c61d9826a4..3a0ff7b098 100644
>>> --- a/tests/acceptance/linux_initrd.py
>>> +++ b/tests/acceptance/linux_initrd.py
>>> @@ -20,6 +20,7 @@ class LinuxInitrd(Test):
>>>        Checks QEMU evaluates correctly the initrd file passed as -initrd option.
>>>        :avocado: tags=arch:x86_64
>>> +    :avocado: tags=machine:pc
>>
>> For some tests we can run on multiple machines (here q35), I was tempted to
>> use multiple tags. How could I do that now?
>>
> 
> I missed this comment: you can add many tag values here to *classify*
> the test as being "q35 machine type capable".
> 
> But, Avocado will only run a test multiple times with a varianter
> plugin active.  In that case, a "machine" *parameter* with different
> values will be passed to the tests.  This tag value is being used
> as a default value for the parameter, so it has a lower precedence.
> 
> We have a pending task[1] to create an initial CIT file for arch and
> machine types.
> 
> CC'ing Jan Richter, who is supposed to start working on it soon.
> 
> - Cleber.
> 
> [1] - https://trello.com/c/1wvzcxHY/105-create-cit-parameter-for-acceptance-tests

Good news, thanks for the trello link.



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

* Re: [PATCH v7 4/8] Acceptance tests: use relative location for tests
  2019-11-11 22:11     ` Cleber Rosa
@ 2019-11-12 18:17       ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 39+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-11-12 18:17 UTC (permalink / raw)
  To: Cleber Rosa
  Cc: Beraldo Leal, qemu-devel, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Wainer dos Santos Moschetta,
	Willian Rampazzo, qemu-ppc, Aleksandar Rikalo, Aurelien Jarno,
	Eduardo Habkost

On 11/11/19 11:11 PM, Cleber Rosa wrote:
> On Mon, Nov 04, 2019 at 07:26:23PM +0100, Philippe Mathieu-Daudé wrote:
>> On 11/4/19 4:13 PM, Cleber Rosa wrote:
>>> An Avocado Test ID[1] is composed by a number of components, but it
>>> starts with the Test Name, usually a file system location that was
>>> given to the loader.
>>>
>>> Because the source directory is being given as a prefix to the
>>> "tests/acceptance" directory containing the acceptance tests, the test
>>> names will needlessly include the directory the user is using to host
>>> the QEMU sources (and/or build tree).
>>>
>>> Let's remove the source dir (or a build dir) from the path given to
>>> the test loader.  This should give more constant names, and when using
>>> result servers and databases, it should give the same test names
>>> across executions from different people or from different directories.
>>
>> Can we strip the full path to directory and only keep the filename in the
>> database? (Thinking about out-of-tree tests).
>>
> 
> Yes, absolutely, but this needs to be done one the Avocado side.  TBH,
> I have ideas to make this go even further, such as:
> 
>   1) the stripping of the "test_" prefix of the test method
> 
>   2) replace the full path to a directory with tests for a "test suite"
>      alias (default to the directory name itself)
> 
>   3) test suite alias will be persisted on test result such as reports
>      or machine, but ommited from the human UI
> 
>   4) full path to directory, exact version of test files (git hash) will
>      all be metadata and not part of the test ID
> 
> Roughly speaking, something which is listed like this currently:
> 
>    $ avocado list tests/acceptance/
>    INSTRUMENTED tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_x86_64_pc
>    ...
> 
> When executed, would be shown as:
> 
>    JOB ID     : fb885e9c3e7dc50534ec380a7e988cbf94233f07
>    JOB LOG    : /home/cleber/avocado/job-results/job-2019-11-11T17.07-fb885e9/job.log
>     (1/1) acceptance/boot_linux_console.py:BootLinuxConsole.x86_64_pc: PASS (2.17 s)

For the particular use case of QEMU, we can also strip the "acceptance/" 
part (and eventually ".py").

>    RESULTS    : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
>    JOB TIME   : 2.35 s
> 
> How does that sound?
> 
> - Cleber.
> 



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

* Re: [PATCH v7 8/8] Acceptance test: add "boot_linux" tests
  2019-11-04 15:13 ` [PATCH v7 8/8] Acceptance test: add "boot_linux" tests Cleber Rosa
  2019-11-08 19:42   ` Wainer dos Santos Moschetta
@ 2019-11-12 18:20   ` Philippe Mathieu-Daudé
  2019-11-15 23:48     ` Cleber Rosa
  2019-12-03 19:19   ` Alex Bennée
  2 siblings, 1 reply; 39+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-11-12 18:20 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Beraldo Leal, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Wainer dos Santos Moschetta,
	Willian Rampazzo, qemu-ppc, Aleksandar Rikalo, Aurelien Jarno,
	Eduardo Habkost

On 11/4/19 4:13 PM, Cleber Rosa wrote:
> This acceptance test, validates that a full blown Linux guest can
> successfully boot in QEMU.  In this specific case, the guest chosen is
> Fedora version 31.
> 
>   * x86_64, pc and q35 machine types, with and without kvm as an
>     accellerator

typo "accelerator"

> 
>   * aarch64 and virt machine type, with and without kvm as an
>     accellerator

Ditto.

> 
>   * ppc64 and pseries machine type
> 
>   * s390x and s390-ccw-virtio machine type
> 
> The method for checking the successful boot is based on "cloudinit"
> and its "phone home" feature.  The guest is given an ISO image
> with the location of the phone home server, and the information to
> post (the instance ID).  Upon receiving the correct information,
> from the guest, the test is considered to have PASSed.
> 
> This test is currently limited to user mode networking only, and
> instructs the guest to connect to the "router" address that is hard
> coded in QEMU.
> 
> To create the cloudinit ISO image that will be used to configure the
> guest, the pycdlib library is also required and has been added as
> requirement to the virtual environment created by "check-venv".
> 
> The console output is read by a separate thread, by means of the
> Avocado datadrainer utility module.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---



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

* Re: [PATCH v7 5/8] Acceptance tests: keep a stable reference to the QEMU build dir
  2019-11-07 19:22   ` Wainer dos Santos Moschetta
  2019-11-11 22:38     ` Cleber Rosa
@ 2019-11-15 21:36     ` Cleber Rosa
  1 sibling, 0 replies; 39+ messages in thread
From: Cleber Rosa @ 2019-11-15 21:36 UTC (permalink / raw)
  To: Wainer dos Santos Moschetta
  Cc: Beraldo Leal, qemu-devel, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Willian Rampazzo, qemu-ppc,
	Aleksandar Rikalo, Philippe Mathieu-Daudé,
	Aurelien Jarno, Eduardo Habkost

On Thu, Nov 07, 2019 at 05:22:24PM -0200, Wainer dos Santos Moschetta wrote:
> 
> On 11/4/19 1:13 PM, Cleber Rosa wrote:
> > This is related to the the differences in in-tree and out-of-tree
> > builds in QEMU.  For simplification, <BLD> means my build directory.
> > 
> > Currently, by running a `make check-acceptance` one gets (in
> > tests/acceptance/avocado_qemu/__init__.py):
> > 
> >     SRC_ROOT_DIR: <BLD>/tests/acceptance/avocado_qemu/../../..
> > 
> > This in itself is problematic, because after the parent directories
> > are applied, one may be left not with a pointer to the build directory
> > as intended, but with the location of the source tree (assuming they
> > differ). Built binaries, such as qemu-img, are of course not there and
> > can't be found.
> > 
> > Given that a Python '__file__' will contain the absolute path to the
> > file backing the module, say:
> > 
> >     __file__: <BLD>/tests/acceptance/avocado_qemu/__init__.py
> > 
> >                    |  4  |     3    |      2     |     1     |
> > 
> > A solution is to not "evaluate" the third parent dir (marked as 4
> > here) because that ends up following the "tests" directory symlink to
> > the source tree.  In fact, there's no need to keep or evaluate any of
> > the parent directories, we can just drop the rightmost 4 components,
> > and we'll keep a stable reference to the build directory (with no
> > symlink being followed).  This works for either a dedicated build
> > directory or also a combined source and build tree.
> > 
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > ---
> >   tests/acceptance/avocado_qemu/__init__.py | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> > index 6618ea67c1..17ce583c87 100644
> > --- a/tests/acceptance/avocado_qemu/__init__.py
> > +++ b/tests/acceptance/avocado_qemu/__init__.py
> > @@ -16,7 +16,7 @@ import tempfile
> >   import avocado
> > -SRC_ROOT_DIR = os.path.join(os.path.dirname(__file__), '..', '..', '..')
> > +SRC_ROOT_DIR = os.path.dirname(os.path.dirname(os.path.dirname(os.path.dirname(__file__))))
> 
> In this case, wouldn't make sense to rename the constant from SRC_ROOT_DIR
> to BUILD_ROOT_DIR?
> 
> This patch looks good to me besides that.
> 
> - Wainer
>

So, based on your review, I went back and forward with this.  I'm
going to propose something different on v8, basically distinguishing
between source and build directories.  I'm keeping this patch as is on
v8 though, just for the sake of making the indivdual changes easier to
track.

Thanks,
- Cleber.

> >   sys.path.append(os.path.join(SRC_ROOT_DIR, 'python'))
> >   from qemu.machine import QEMUMachine



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

* Re: [PATCH v7 6/8] Acceptance tests: add the build directory to the system PATH
  2019-11-12 14:00       ` Wainer dos Santos Moschetta
@ 2019-11-15 23:17         ` Cleber Rosa
  0 siblings, 0 replies; 39+ messages in thread
From: Cleber Rosa @ 2019-11-15 23:17 UTC (permalink / raw)
  To: Wainer dos Santos Moschetta
  Cc: Beraldo Leal, qemu-devel, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Willian Rampazzo, qemu-ppc,
	Aleksandar Rikalo, Philippe Mathieu-Daudé,
	Aurelien Jarno, Eduardo Habkost

On Tue, Nov 12, 2019 at 12:00:20PM -0200, Wainer dos Santos Moschetta wrote:
> 
> On 11/11/19 8:49 PM, Cleber Rosa wrote:
> > On Thu, Nov 07, 2019 at 05:46:13PM -0200, Wainer dos Santos Moschetta wrote:
> > > On 11/4/19 1:13 PM, Cleber Rosa wrote:
> > > > So that when binaries such as qemu-img are searched for, those in the
> > > > build tree will be favored.  As a clarification, SRC_ROOT_DIR is
> > > > dependent on the location from where tests are executed, so they are
> > > > equal to the build directory if one is being used.
> > > > 
> > > > The original motivation is that Avocado libraries such as
> > > > avocado.utils.vmimage.get() may use the matching binaries, but it may
> > > > also apply to any other binary that test code may eventually attempt
> > > > to execute.
> > > > 
> > > > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > > > ---
> > > >    tests/acceptance/avocado_qemu/__init__.py | 6 ++++++
> > > >    1 file changed, 6 insertions(+)
> > > > 
> > > > diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> > > > index 17ce583c87..a4bb796a47 100644
> > > > --- a/tests/acceptance/avocado_qemu/__init__.py
> > > > +++ b/tests/acceptance/avocado_qemu/__init__.py
> > > > @@ -110,6 +110,12 @@ class Test(avocado.Test):
> > > >            return None
> > > >        def setUp(self):
> > > > +        # Some utility code uses binaries from the system's PATH.  For
> > > > +        # instance, avocado.utils.vmimage.get() uses qemu-img, to
> > > > +        # create a snapshot image.  This is a transparent way of
> > > Because PATH is changed in a transparent way, wouldn't be better to also
> > > self.log.info() that fact?
> > > 
> > I don't have a problem with logging it, but because it will happen for
> > *every single* test, it seems like it will become noise.  I think it's
> > better to properly document this aspect of "avocado_qemu.Test" instead
> > (which is currently missing here).  Something like:
> > 
> > "Tests based on avocado_qemu.Test will have, as a convenience, the
> > QEMU build directory added to their PATH environment variable.  The goal
> > is to allow tests to seamless use matching built binaries, instead of
> > binaries installed elsewhere in the system".
> > 
> > How does it sound?
> 
> 
> It does.
> 
> 
> > 
> > > > +        # making sure those utilities find and use binaries on the
> > > > +        # build tree by default.
> > > > +        os.environ['PATH'] = '%s:%s' % (SRC_ROOT_DIR, os.environ['PATH'])
> > > I think PATH should be set only once at class initialization. Perhaps in
> > > setUpClass()?
> > > 
> > > - Wainer
> > > 
> > The Avocado test isolation model makes setUpClass() unnecessary,
> > unsupported and pointless, so we only support setUp().
> > 
> > Every test already runs on its own process, and with the nrunner
> > model, should be able to run on completely different systems.  That's
> > why we don't want to support a setUpClass() like approach.
> 
> Okay, thanks for the explanation.
>

And thanks for the review.  Given the level of controversy here, I've
decided to take a different approach on v8.  Basically, I'm adding an
interface to avocado.utils.vmimage[1], so that we can explicitly
control the qemu-img binary used.

Looking forward to your opinion on v8.

Thanks,
- Cleber.

[1] - https://github.com/avocado-framework/avocado/pull/3374

> Thanks,
> 
> Wainer
> 
> > 
> > - Cleber.
> > 
> > 
> 
> 



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

* Re: [PATCH v7 6/8] Acceptance tests: add the build directory to the system PATH
  2019-11-08 13:13   ` Philippe Mathieu-Daudé
@ 2019-11-15 23:19     ` Cleber Rosa
  0 siblings, 0 replies; 39+ messages in thread
From: Cleber Rosa @ 2019-11-15 23:19 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Beraldo Leal, qemu-devel, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Wainer dos Santos Moschetta,
	Willian Rampazzo, qemu-ppc, Aleksandar Rikalo, Aurelien Jarno,
	Eduardo Habkost

On Fri, Nov 08, 2019 at 02:13:02PM +0100, Philippe Mathieu-Daudé wrote:
> On 11/4/19 4:13 PM, Cleber Rosa wrote:
> > So that when binaries such as qemu-img are searched for, those in the
> > build tree will be favored.  As a clarification, SRC_ROOT_DIR is
> > dependent on the location from where tests are executed, so they are
> > equal to the build directory if one is being used.
> > 
> > The original motivation is that Avocado libraries such as
> > avocado.utils.vmimage.get() may use the matching binaries, but it may
> > also apply to any other binary that test code may eventually attempt
> > to execute.
> > 
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > ---
> >   tests/acceptance/avocado_qemu/__init__.py | 6 ++++++
> >   1 file changed, 6 insertions(+)
> > 
> > diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> > index 17ce583c87..a4bb796a47 100644
> > --- a/tests/acceptance/avocado_qemu/__init__.py
> > +++ b/tests/acceptance/avocado_qemu/__init__.py
> > @@ -110,6 +110,12 @@ class Test(avocado.Test):
> >           return None
> >       def setUp(self):
> > +        # Some utility code uses binaries from the system's PATH.  For
> > +        # instance, avocado.utils.vmimage.get() uses qemu-img, to
> > +        # create a snapshot image.  This is a transparent way of
> > +        # making sure those utilities find and use binaries on the
> > +        # build tree by default.
> > +        os.environ['PATH'] = '%s:%s' % (SRC_ROOT_DIR, os.environ['PATH'])
> 
> But qemu-img is compiled in BUILD_ROOT_DIR, isn't it?
> 
> Maybe we should pass its path by argument, such --qemu-img /path/to/it.
>

Hi Philippe,

On the next version we should see a properly named variable for the
build directory, and (as explained in the previous response) also
a more explicit setting of the qemu-img binary used (although not
a parameter or command line argument at this point).

Looking forward for your opinion on the next version, and thanks
again!

- Cleber.

> >           self._vms = {}
> >           self.arch = self.params.get('arch',
> > 



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

* Re: [PATCH v7 7/8] Acceptance tests: depend on qemu-img
  2019-11-07 20:31   ` Wainer dos Santos Moschetta
@ 2019-11-15 23:45     ` Cleber Rosa
  0 siblings, 0 replies; 39+ messages in thread
From: Cleber Rosa @ 2019-11-15 23:45 UTC (permalink / raw)
  To: Wainer dos Santos Moschetta
  Cc: Beraldo Leal, qemu-devel, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Willian Rampazzo, qemu-ppc,
	Aleksandar Rikalo, Philippe Mathieu-Daudé,
	Aurelien Jarno, Eduardo Habkost

On Thu, Nov 07, 2019 at 06:31:03PM -0200, Wainer dos Santos Moschetta wrote:
> 
> On 11/4/19 1:13 PM, Cleber Rosa wrote:
> > Tests using the avocado.utils.vmimage library make use of qemu-img,
> > and because it makes sense to use the version matching the rest of the
> > source code, let's make sure it gets built.
> > 
> > Its selection, instead of a possible qemu-img binary installed system
> > wide, is already dealt with by the change that adds the build dir to
> > the PATH during the test execution.
> > 
> > This is based on the same work for qemu-iotests, and suggested by its
> > author:
> > 
> >    - https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg00951.html
> > 
> > CC: Philippe Mathieu-Daudé <philmd@redhat.com>
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > ---
> >   tests/Makefile.include | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > index 65e85f5275..559c3e6375 100644
> > --- a/tests/Makefile.include
> > +++ b/tests/Makefile.include
> > @@ -1174,7 +1174,7 @@ $(TESTS_RESULTS_DIR):
> >   check-venv: $(TESTS_VENV_DIR)
> > -check-acceptance: check-venv $(TESTS_RESULTS_DIR)
> > +check-acceptance: check-venv $(TESTS_RESULTS_DIR) qemu-img$(EXESUF)
> 
> To be honest, I don't fell comfortable by the fact that the whole acceptance
> suite will depend on qemu-img which, in reality, is needed by only a sub-set
> of tests. Besides it, there might be some reason for someone to build QEMU
> with --disable-tools and this change will end up forcing the qemu-img built
> (of course if check-acceptance is issued).
>

Fair enough.

> What if we instead:
> 
> 1. Warn the users in case qemu tools weren't built. Alerting that qemu-img
> and friends will be picked up from system-wide (if any).
>

I'll propose on v8 that we try the locally built qemu-img, or fallback to the
system-wide install if available...

> 2. The tests that rely on avocado.utils.vmimage check for the presence of
> dependent tools, possible canceling itself on their lack. This may be done
> at test code level or perhaps using Avocado's tag mechanism + tweaking
> avocado_qemu.
>

... if not available, the test will automatically cancel because the
image creation (which depends on qemu-img) behaves like that.

Thanks!
- Cleber.

> Thanks,
> 
> Wainer
> 
> >   	$(call quiet-command, \
> >               $(TESTS_VENV_DIR)/bin/python -m avocado \
> >               --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \



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

* Re: [PATCH v7 8/8] Acceptance test: add "boot_linux" tests
  2019-11-08 19:42   ` Wainer dos Santos Moschetta
@ 2019-11-15 23:47     ` Cleber Rosa
  0 siblings, 0 replies; 39+ messages in thread
From: Cleber Rosa @ 2019-11-15 23:47 UTC (permalink / raw)
  To: Wainer dos Santos Moschetta
  Cc: Beraldo Leal, qemu-devel, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Willian Rampazzo, qemu-ppc,
	Aleksandar Rikalo, Philippe Mathieu-Daudé,
	Aurelien Jarno, Eduardo Habkost

On Fri, Nov 08, 2019 at 05:42:14PM -0200, Wainer dos Santos Moschetta wrote:
> 
> On 11/4/19 1:13 PM, Cleber Rosa wrote:
> > This acceptance test, validates that a full blown Linux guest can
> > successfully boot in QEMU.  In this specific case, the guest chosen is
> > Fedora version 31.
> > 
> >   * x86_64, pc and q35 machine types, with and without kvm as an
> >     accellerator
> > 
> >   * aarch64 and virt machine type, with and without kvm as an
> >     accellerator
> > 
> >   * ppc64 and pseries machine type
> > 
> >   * s390x and s390-ccw-virtio machine type
> > 
> > The method for checking the successful boot is based on "cloudinit"
> > and its "phone home" feature.  The guest is given an ISO image
> > with the location of the phone home server, and the information to
> > post (the instance ID).  Upon receiving the correct information,
> > from the guest, the test is considered to have PASSed.
> > 
> > This test is currently limited to user mode networking only, and
> > instructs the guest to connect to the "router" address that is hard
> > coded in QEMU.
> > 
> > To create the cloudinit ISO image that will be used to configure the
> > guest, the pycdlib library is also required and has been added as
> > requirement to the virtual environment created by "check-venv".
> > 
> > The console output is read by a separate thread, by means of the
> > Avocado datadrainer utility module.
> > 
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > ---
> >   tests/acceptance/boot_linux.py | 175 +++++++++++++++++++++++++++++++++
> >   tests/requirements.txt         |   1 +
> >   2 files changed, 176 insertions(+)
> >   create mode 100644 tests/acceptance/boot_linux.py
> > 
> > diff --git a/tests/acceptance/boot_linux.py b/tests/acceptance/boot_linux.py
> > new file mode 100644
> > index 0000000000..882f7dc5df
> > --- /dev/null
> > +++ b/tests/acceptance/boot_linux.py
> > @@ -0,0 +1,175 @@
> > +# Functional test that boots a complete Linux system via a cloud image
> > +#
> > +# Copyright (c) 2018-2019 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.
> > +
> > +import os
> > +
> > +from avocado_qemu import Test, SRC_ROOT_DIR
> > +
> > +from qemu import kvm_available
> > +
> > +from avocado.utils import cloudinit
> > +from avocado.utils import network
> > +from avocado.utils import vmimage
> > +from avocado.utils import datadrainer
> > +
> > +
> > +KVM_NOT_AVAILABLE = "KVM accelerator does not seem to be available"
> > +
> > +
> > +class BootLinux(Test):
> > +    """
> > +    Boots a Linux system, checking for a successful initialization
> > +    """
> > +
> > +    timeout = 600
> > +    chksum = None
> > +
> > +    def setUp(self):
> > +        super(BootLinux, self).setUp()
> > +        self.prepare_boot()
> > +        self.vm.add_args('-smp', '2')
> > +        self.vm.add_args('-m', '2048')
> > +        self.vm.add_args('-drive', 'file=%s' % self.boot.path)
> > +        self.prepare_cloudinit()
> > +
> > +    def prepare_boot(self):
> > +        self.log.info('Downloading/preparing boot image')
> Cosmetic: replace 'Downloading/preparing' with 'Downloading' since this
> function does only download the boot image.

Actually the "preparing" is intentional, because this will also
prepare the snapshot image (the original vmimage is never touched).

> > +        # Fedora 31 only provides ppc64le images
> > +        image_arch = self.arch
> > +        if image_arch == 'ppc64':
> > +            image_arch = 'ppc64le'
> > +        try:
> > +            self.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')
> 
> Likewise.
>

Likewise too :)

> > +
> > +    def prepare_cloudinit(self):
> > +        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)
> > +            self.vm.add_args('-drive', 'file=%s,format=raw' % cloudinit_iso)
> > +        except Exception:
> > +            self.cancel('Failed to prepared cloudinit image')
> > +
> > +    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):
> > +    """
> > +    :avocado: tags=arch:x86_64
> > +    """
> > +
> > +    chksum = 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'
> > +
> > +    def test_pc(self):
> > +        """
> > +        :avocado: tags=machine:pc
> > +        """
> > +        self.launch_and_wait()
> > +
> > +    def test_kvm_pc(self):
> > +        """
> > +        :avocado: tags=machine:pc
> > +        :avocado: tags=accel:kvm
> > +        """
> > +        if not kvm_available(self.arch):
> > +            self.cancel(KVM_NOT_AVAILABLE)
> 
> kvm_available() solely checks for kvm presence on host, not if enabled in
> the target binary. That said, as a follow up we could adapt this test to use
> a strengthen approach as I proposed in patch 02 of '[PATCH 0/3] Acceptance
> tests: boot Linux with KVM test' [1] series.
> 
> [1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg627498.html
>

Yes, I'll revisit that.  Thanks for the reminder.

> > +        self.vm.add_args("-accel", "kvm")
> > +        self.launch_and_wait()
> > +
> > +    def test_q35(self):
> > +        """
> > +        :avocado: tags=machine:q35
> > +        """
> > +        self.launch_and_wait()
> > +
> > +    def test_kvm_q35(self):
> > +        """
> > +        :avocado: tags=machine:q35
> > +        :avocado: tags=accel:kvm
> > +        """
> > +        if not kvm_available(self.arch):
> > +            self.cancel(KVM_NOT_AVAILABLE)
> > +        self.vm.add_args("-accel", "kvm")
> > +        self.launch_and_wait()
> > +
> > +
> > +class BootLinuxAarch64(BootLinux):
> > +    """
> > +    :avocado: tags=arch:aarch64
> > +    :avocado: tags=machine:virt
> > +    """
> > +
> > +    chksum = '1e18d9c0cf734940c4b5d5ec592facaed2af0ad0329383d5639c997fdf16fe49'
> > +
> > +    def test_virt(self):
> > +        self.vm.add_args('-cpu', 'cortex-a53')
> > +        self.vm.add_args('-bios',
> > +                         os.path.join(SRC_ROOT_DIR, 'pc-bios',
> > +                                      'edk2-aarch64-code.fd'))
> > +        self.vm.add_args('-device', 'virtio-rng-pci,rng=rng0')
> > +        self.vm.add_args('-object', 'rng-random,id=rng0,filename=/dev/urandom')
> > +        self.launch_and_wait()
> > +
> > +    def test_kvm_virt(self):
> > +        """
> > +        :avocado: tags=accel:kvm
> > +        """
> > +        if not kvm_available(self.arch):
> > +            self.cancel(KVM_NOT_AVAILABLE)
> > +        self.vm.add_args("-accel", "kvm")
> > +        self.test_virt()
> > +
> > +
> > +class BootLinuxPPC64(BootLinux):
> > +    """
> > +    :avocado: tags=arch:ppc64
> > +    """
> > +
> > +    chksum = '7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58'
> > +
> > +    def test_pseries(self):
> > +        """
> > +        :avocado: tags=machine:pseries
> > +        """
> > +        self.launch_and_wait()
> > +
> > +
> > +class BootLinuxS390X(BootLinux):
> > +    """
> > +    :avocado: tags=arch:s390x
> > +    """
> > +
> > +    chksum = '4caaab5a434fd4d1079149a072fdc7891e354f834d355069ca982fdcaf5a122d'
> > +
> > +    def test_s390_ccw_virtio(self):
> > +        """
> > +        :avocado: tags=machine:s390-ccw-virtio
> > +        """
> > +        self.launch_and_wait()
> > diff --git a/tests/requirements.txt b/tests/requirements.txt
> > index a2a587223a..3893361e0c 100644
> > --- a/tests/requirements.txt
> > +++ b/tests/requirements.txt
> > @@ -2,3 +2,4 @@
> >   # in the tests/venv Python virtual environment. For more info,
> >   # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
> >   avocado-framework==72.0
> > +pycdlib==1.8.0
> 
> Besides the cosmetics suggestions, this test is ready to go.
> 
> In other to write powerful tests there will be needed a mechanism to boot a
> full blown guest which can be interacted with. I see the proposed BootLinux
> class here as the first milestone towards that goal. Another reason to merge
> this as soon as possible.
>

Yes, agreed.

> Ran the tests on my x86_68 machine with RHEL 8.1, and all pass
> (BootLinuxAarch64.test_kvm_virt correctly skipped).
> 
> Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> 
> Thanks,
> 
> Wainer
> 

Thanks!
- Cleber.



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

* Re: [PATCH v7 8/8] Acceptance test: add "boot_linux" tests
  2019-11-12 18:20   ` Philippe Mathieu-Daudé
@ 2019-11-15 23:48     ` Cleber Rosa
  0 siblings, 0 replies; 39+ messages in thread
From: Cleber Rosa @ 2019-11-15 23:48 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Beraldo Leal, qemu-devel, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Wainer dos Santos Moschetta,
	Willian Rampazzo, qemu-ppc, Aleksandar Rikalo, Aurelien Jarno,
	Eduardo Habkost

On Tue, Nov 12, 2019 at 07:20:38PM +0100, Philippe Mathieu-Daudé wrote:
> On 11/4/19 4:13 PM, Cleber Rosa wrote:
> > This acceptance test, validates that a full blown Linux guest can
> > successfully boot in QEMU.  In this specific case, the guest chosen is
> > Fedora version 31.
> > 
> >   * x86_64, pc and q35 machine types, with and without kvm as an
> >     accellerator
> 
> typo "accelerator"
>

Fixed, thanks!

> > 
> >   * aarch64 and virt machine type, with and without kvm as an
> >     accellerator
> 
> Ditto.
>

Also fixed, thanks again!

- Cleber.



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

* Re: [PATCH v7 8/8] Acceptance test: add "boot_linux" tests
  2019-11-04 15:13 ` [PATCH v7 8/8] Acceptance test: add "boot_linux" tests Cleber Rosa
  2019-11-08 19:42   ` Wainer dos Santos Moschetta
  2019-11-12 18:20   ` Philippe Mathieu-Daudé
@ 2019-12-03 19:19   ` Alex Bennée
  2 siblings, 0 replies; 39+ messages in thread
From: Alex Bennée @ 2019-12-03 19:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Fabien Chouteau, KONRAD Frederic,
	Hervé Poussineau, Wainer dos Santos Moschetta,
	Willian Rampazzo, Cleber Rosa, qemu-ppc, Aleksandar Rikalo,
	Philippe Mathieu-Daudé,
	Aurelien Jarno, Eduardo Habkost


Cleber Rosa <crosa@redhat.com> writes:

> This acceptance test, validates that a full blown Linux guest can
> successfully boot in QEMU.  In this specific case, the guest chosen is
> Fedora version 31.
>
>  * x86_64, pc and q35 machine types, with and without kvm as an
>    accellerator
>
>  * aarch64 and virt machine type, with and without kvm as an
>    accellerator
>
>  * ppc64 and pseries machine type
>
>  * s390x and s390-ccw-virtio machine type
>
> The method for checking the successful boot is based on "cloudinit"
> and its "phone home" feature.  The guest is given an ISO image
> with the location of the phone home server, and the information to
> post (the instance ID).  Upon receiving the correct information,
> from the guest, the test is considered to have PASSed.
>
> This test is currently limited to user mode networking only, and
> instructs the guest to connect to the "router" address that is hard
> coded in QEMU.
>
> To create the cloudinit ISO image that will be used to configure the
> guest, the pycdlib library is also required and has been added as
> requirement to the virtual environment created by "check-venv".
>
> The console output is read by a separate thread, by means of the
> Avocado datadrainer utility module.

Hmm I'm seeing breakage with this final commit:

19:16:36 [alex@zen:~/l/qemu.git] review/boot-linux-acceptance|… + /bin/sh -c "cd builds/all; make check-acceptance"
make[1]: Entering directory '/home/alex/lsrc/qemu.git/slirp'
make[1]: Nothing to be done for 'all'.
make[1]: Leaving directory '/home/alex/lsrc/qemu.git/slirp'
  VENV    /home/alex/lsrc/qemu.git/builds/all/tests/venv
  PIP     /home/alex/lsrc/qemu.git/tests/requirements.txt
  CC      qemu-img.o
  LINK    qemu-img
/usr/bin/ld: //lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from //lib/x86_64-linux-gnu/libc.so.6
  AVOCADO tests/acceptance
JOB ID     : f564c8a207a556b04abceed09e51cf945b48ca56
JOB LOG    : /home/alex/lsrc/qemu.git/builds/all/tests/results/job-2019-12-03T19.16-f564c8a/job.log
 (01/60) tests/acceptance/boot_linux.py:BootLinuxX8664.test_pc: ERROR: Traceback (most recent call last):\n  File "/usr/lib/python3.7/imp.py", line 234, in load_module\n
return load_source(name, filename, file)\n  File "/usr/lib/python3.7/imp.py", line 171, in load_source\n    module = _load(spec)\n  File "<frozen importlib._b... (0.03 s)
Interrupting job (failfast).
RESULTS    : PASS 0 | ERROR 1 | FAIL 0 | SKIP 59 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB TIME   : 0.15 s
make: *** [/home/alex/lsrc/qemu.git/tests/Makefile.include:1183: check-acceptance] Error 9


-- 
Alex Bennée


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

end of thread, other threads:[~2019-12-03 20:36 UTC | newest]

Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-04 15:13 [PATCH v7 0/8] Acceptance test: Add "boot_linux" acceptance test Cleber Rosa
2019-11-04 15:13 ` [PATCH v7 1/8] Acceptance test x86_cpu_model_versions: use default vm Cleber Rosa
2019-11-04 18:22   ` Philippe Mathieu-Daudé
2019-11-04 15:13 ` [PATCH v7 2/8] Acceptance tests: introduce utility method for tags unique vals Cleber Rosa
2019-11-08 13:14   ` Philippe Mathieu-Daudé
2019-11-11 21:44     ` Cleber Rosa
2019-11-04 15:13 ` [PATCH v7 3/8] Acceptance tests: use avocado tags for machine type Cleber Rosa
2019-11-08 13:20   ` Philippe Mathieu-Daudé
2019-11-11 21:49     ` Cleber Rosa
2019-11-12 18:15       ` Philippe Mathieu-Daudé
2019-11-12  1:59     ` Cleber Rosa
2019-11-12 18:15       ` Philippe Mathieu-Daudé
2019-11-04 15:13 ` [PATCH v7 4/8] Acceptance tests: use relative location for tests Cleber Rosa
2019-11-04 18:26   ` Philippe Mathieu-Daudé
2019-11-11 22:11     ` Cleber Rosa
2019-11-12 18:17       ` Philippe Mathieu-Daudé
2019-11-07 18:52   ` Wainer dos Santos Moschetta
2019-11-04 15:13 ` [PATCH v7 5/8] Acceptance tests: keep a stable reference to the QEMU build dir Cleber Rosa
2019-11-07 19:22   ` Wainer dos Santos Moschetta
2019-11-11 22:38     ` Cleber Rosa
2019-11-15 21:36     ` Cleber Rosa
2019-11-04 15:13 ` [PATCH v7 6/8] Acceptance tests: add the build directory to the system PATH Cleber Rosa
2019-11-07 19:46   ` Wainer dos Santos Moschetta
2019-11-11 22:49     ` Cleber Rosa
2019-11-12 14:00       ` Wainer dos Santos Moschetta
2019-11-15 23:17         ` Cleber Rosa
2019-11-08 13:13   ` Philippe Mathieu-Daudé
2019-11-15 23:19     ` Cleber Rosa
2019-11-04 15:13 ` [PATCH v7 7/8] Acceptance tests: depend on qemu-img Cleber Rosa
2019-11-07 20:31   ` Wainer dos Santos Moschetta
2019-11-15 23:45     ` Cleber Rosa
2019-11-04 15:13 ` [PATCH v7 8/8] Acceptance test: add "boot_linux" tests Cleber Rosa
2019-11-08 19:42   ` Wainer dos Santos Moschetta
2019-11-15 23:47     ` Cleber Rosa
2019-11-12 18:20   ` Philippe Mathieu-Daudé
2019-11-15 23:48     ` Cleber Rosa
2019-12-03 19:19   ` Alex Bennée
2019-11-04 19:54 ` [PATCH v7 0/8] Acceptance test: Add "boot_linux" acceptance test no-reply
2019-11-07 18:38 ` Wainer dos Santos Moschetta

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).