[PATCH v2 0/7] tests/acceptance: Handle tests with "cpu" tag

[PATCH v2 0/7] tests/acceptance: Handle tests with "cpu" tag

[Wainer dos Santos Moschetta]

Currently the acceptance tests tagged with "machine" have the "-M TYPE"
automatically added to the list of arguments of the QEMUMachine object.
In other words, that option is passed to the launched QEMU. On this
series it is implemented the same feature but instead for tests marked
with "cpu".

There is a caveat, however, in case the test needs additional arguments to
the CPU type they cannot be passed via tag, because the tags parser split
values by comma. For example, in tests/acceptance/x86_cpu_model_versions.py,
there are cases where:

  * -cpu is set to "Cascadelake-Server,x-force-features=on,check=off,enforce=off"
  * if it was tagged like "cpu:Cascadelake-Server,x-force-features=on,check=off,enforce=off"
    then the parser would break it into 4 tags ("cpu:Cascadelake-Server",
    "x-force-features=on", "check=off", "enforce=off")
  * resulting on "-cpu Cascadelake-Server" and the remaining arguments are ignored.

It was introduced the avocado_qemu.Test.set_vm_arg() method to deal with
cases like the example above, so that one can tag it as "cpu:Cascadelake-Server"
AND call self.set_vm_args('-cpu', "Cascadelake-Server,x-force-features=on,check=off,enforce=off"),
and that results on the reset of the initial value of -cpu.

This series was tested on CI (https://gitlab.com/wainersm/qemu/-/pipelines/277376246)
and with the following code:

from avocado_qemu import Test

class CPUTest(Test):
    def test_cpu(self):
        """
        :avocado: tags=cpu:host
        """
        # The cpu property is set to the tag value, or None on its absence
        self.assertEqual(self.cpu, "host")
        # The created VM has the '-cpu host' option
        self.assertIn("-cpu host", " ".join(self.vm._args))
        self.vm.launch()

    def test_cpu_none(self):
        self.assertEqual(self.cpu, None)
        self.assertNotIn('-cpu', self.vm._args)

    def test_cpu_reset(self):
        """
        :avocado: tags=cpu:host
        """
        self.assertIn("-cpu host", " ".join(self.vm._args))
        self.set_vm_arg("-cpu", "Cascadelake-Server,x-force-features=on")
        self.assertNotIn("-cpu host", " ".join(self.vm._args))
        self.assertIn("-cpu Cascadelake-Server,x-force-features=on", " ".join(self.vm._args))

Changes:
 - v1 -> v2:
   - Recognize the cpu value passed via test parameter [crosa]
   - Fixed tags (patch 02) on preparation to patch 03 [crosa]
   - Added QEMUMachine.args property (patch 04) so that _args could be handled
     without pylint complaining (protected property) 
   - Added Test.set_vm_arg() (patch 05) to handle the corner case [crosa]

Wainer dos Santos Moschetta (7):
  tests/acceptance: Automatic set -cpu to the test vm
  tests/acceptance: Fix mismatch on cpu tagged tests
  tests/acceptance: Let the framework handle "cpu:VALUE" tagged tests
  tests/acceptance: Tagging tests with "cpu:VALUE"
  python/qemu: Add args property to the QEMUMachine class
  tests/acceptance: Add set_vm_arg() to the Test class
  tests/acceptance: Handle cpu tag on x86_cpu_model_versions tests

 docs/devel/testing.rst                     | 17 +++++++++
 python/qemu/machine.py                     |  5 +++
 tests/acceptance/avocado_qemu/__init__.py  | 21 ++++++++++++
 tests/acceptance/boot_linux.py             |  3 --
 tests/acceptance/boot_linux_console.py     | 16 +++++----
 tests/acceptance/machine_mips_malta.py     |  7 ++--
 tests/acceptance/pc_cpu_hotplug_props.py   |  2 +-
 tests/acceptance/replay_kernel.py          | 17 ++++-----
 tests/acceptance/reverse_debugging.py      |  2 +-
 tests/acceptance/tcg_plugins.py            | 15 ++++----
 tests/acceptance/virtio-gpu.py             |  4 +--
 tests/acceptance/x86_cpu_model_versions.py | 40 +++++++++++++++++-----
 12 files changed, 107 insertions(+), 42 deletions(-)

-- 
2.29.2

[PATCH v2 1/7] tests/acceptance: Automatic set -cpu to the test vm

[Wainer dos Santos Moschetta]

This introduces a new feature to the functional tests: automatic setting of
the '-cpu VALUE' option to the created vm if the test is tagged with
'cpu:VALUE'. The 'cpu' property is made available to the test object as well.

For example, for a simple test as:

    def test(self):
        """
        :avocado: tags=cpu:host
        """
        self.assertEqual(self.cpu, "host")
        self.vm.launch()

The resulting QEMU evocation will be like:

    qemu-system-x86_64 -display none -vga none -chardev socket,id=mon,path=/var/tmp/avo_qemu_sock_pdgzbgd_/qemu-1135557-monitor.sock -mon chardev=mon,mode=control -cpu host

Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
---
 docs/devel/testing.rst                    | 17 +++++++++++++++++
 tests/acceptance/avocado_qemu/__init__.py |  5 +++++
 2 files changed, 22 insertions(+)

diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index 1da4c4e4c4..e139a618f5 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -878,6 +878,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``.
 
+cpu
+~~~
+
+The cpu model that will be set to all QEMUMachine instances created
+by the test.
+
+The ``cpu`` 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=cpu:VALUE`` tag, it will be set to ``VALUE``.
+
 machine
 ~~~~~~~
 
@@ -924,6 +935,12 @@ 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.
 
+cpu
+~~~
+
+The cpu model that will be set to all QEMUMachine instances created
+by the test.
+
 machine
 ~~~~~~~
 
diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
index 83b1741ec8..7f8e703757 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -206,6 +206,9 @@ def setUp(self):
         self.arch = self.params.get('arch',
                                     default=self._get_unique_tag_val('arch'))
 
+        self.cpu = self.params.get('cpu',
+                                   default=self._get_unique_tag_val('cpu'))
+
         self.machine = self.params.get('machine',
                                        default=self._get_unique_tag_val('machine'))
 
@@ -231,6 +234,8 @@ def get_vm(self, *args, name=None):
             name = str(uuid.uuid4())
         if self._vms.get(name) is None:
             self._vms[name] = self._new_vm(*args)
+            if self.cpu is not None:
+                self._vms[name].add_args('-cpu', self.cpu)
             if self.machine is not None:
                 self._vms[name].set_machine(self.machine)
         return self._vms[name]
-- 
2.29.2

[PATCH v2 2/7] tests/acceptance: Fix mismatch on cpu tagged tests

[Wainer dos Santos Moschetta]

There are test cases on machine_mips_malta.py and tcg_plugins.py files
where the cpu tag does not correspond to the value actually given to the QEMU
binary. This fixed those tests tags.

Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
---
 tests/acceptance/machine_mips_malta.py | 6 +++---
 tests/acceptance/tcg_plugins.py        | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/acceptance/machine_mips_malta.py b/tests/acceptance/machine_mips_malta.py
index 7c9a4ee4d2..b1fd075f51 100644
--- a/tests/acceptance/machine_mips_malta.py
+++ b/tests/acceptance/machine_mips_malta.py
@@ -96,7 +96,7 @@ def test_mips_malta_i6400_framebuffer_logo_1core(self):
         """
         :avocado: tags=arch:mips64el
         :avocado: tags=machine:malta
-        :avocado: tags=cpu:i6400
+        :avocado: tags=cpu:I6400
         """
         self.do_test_i6400_framebuffer_logo(1)
 
@@ -105,7 +105,7 @@ def test_mips_malta_i6400_framebuffer_logo_7cores(self):
         """
         :avocado: tags=arch:mips64el
         :avocado: tags=machine:malta
-        :avocado: tags=cpu:i6400
+        :avocado: tags=cpu:I6400
         :avocado: tags=mips:smp
         """
         self.do_test_i6400_framebuffer_logo(7)
@@ -115,7 +115,7 @@ def test_mips_malta_i6400_framebuffer_logo_8cores(self):
         """
         :avocado: tags=arch:mips64el
         :avocado: tags=machine:malta
-        :avocado: tags=cpu:i6400
+        :avocado: tags=cpu:I6400
         :avocado: tags=mips:smp
         """
         self.do_test_i6400_framebuffer_logo(8)
diff --git a/tests/acceptance/tcg_plugins.py b/tests/acceptance/tcg_plugins.py
index c21bf9e52a..aa6e18b62d 100644
--- a/tests/acceptance/tcg_plugins.py
+++ b/tests/acceptance/tcg_plugins.py
@@ -68,7 +68,7 @@ def test_aarch64_virt_insn(self):
         :avocado: tags=accel:tcg
         :avocado: tags=arch:aarch64
         :avocado: tags=machine:virt
-        :avocado: tags=cpu:cortex-a57
+        :avocado: tags=cpu:cortex-a53
         """
         kernel_path = self._grab_aarch64_kernel()
         kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
@@ -95,7 +95,7 @@ def test_aarch64_virt_insn_icount(self):
         :avocado: tags=accel:tcg
         :avocado: tags=arch:aarch64
         :avocado: tags=machine:virt
-        :avocado: tags=cpu:cortex-a57
+        :avocado: tags=cpu:cortex-a53
         """
         kernel_path = self._grab_aarch64_kernel()
         kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
@@ -121,7 +121,7 @@ def test_aarch64_virt_mem_icount(self):
         :avocado: tags=accel:tcg
         :avocado: tags=arch:aarch64
         :avocado: tags=machine:virt
-        :avocado: tags=cpu:cortex-a57
+        :avocado: tags=cpu:cortex-a53
         """
         kernel_path = self._grab_aarch64_kernel()
         kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
-- 
2.29.2

[PATCH v2 3/7] tests/acceptance: Let the framework handle "cpu:VALUE" tagged tests

[Wainer dos Santos Moschetta]

The tests that are already tagged with "cpu:VALUE" don't need to add
"-cpu VALUE" to the list of arguments of the vm object because the avocado_qemu
framework is able to handle it automatically.

Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
---
 tests/acceptance/boot_linux.py         | 3 ---
 tests/acceptance/machine_mips_malta.py | 1 -
 tests/acceptance/replay_kernel.py      | 8 +++-----
 tests/acceptance/reverse_debugging.py  | 2 +-
 tests/acceptance/tcg_plugins.py        | 9 ++++-----
 5 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/tests/acceptance/boot_linux.py b/tests/acceptance/boot_linux.py
index 0d178038a0..55637d126e 100644
--- a/tests/acceptance/boot_linux.py
+++ b/tests/acceptance/boot_linux.py
@@ -82,7 +82,6 @@ def test_virt_tcg(self):
         """
         self.require_accelerator("tcg")
         self.vm.add_args("-accel", "tcg")
-        self.vm.add_args("-cpu", "max")
         self.vm.add_args("-machine", "virt,gic-version=2")
         self.add_common_args()
         self.launch_and_wait()
@@ -95,7 +94,6 @@ def test_virt_kvm_gicv2(self):
         """
         self.require_accelerator("kvm")
         self.vm.add_args("-accel", "kvm")
-        self.vm.add_args("-cpu", "host")
         self.vm.add_args("-machine", "virt,gic-version=2")
         self.add_common_args()
         self.launch_and_wait()
@@ -108,7 +106,6 @@ def test_virt_kvm_gicv3(self):
         """
         self.require_accelerator("kvm")
         self.vm.add_args("-accel", "kvm")
-        self.vm.add_args("-cpu", "host")
         self.vm.add_args("-machine", "virt,gic-version=3")
         self.add_common_args()
         self.launch_and_wait()
diff --git a/tests/acceptance/machine_mips_malta.py b/tests/acceptance/machine_mips_malta.py
index b1fd075f51..b67d8cb141 100644
--- a/tests/acceptance/machine_mips_malta.py
+++ b/tests/acceptance/machine_mips_malta.py
@@ -62,7 +62,6 @@ def do_test_i6400_framebuffer_logo(self, cpu_cores_count):
         kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
                                'clocksource=GIC console=tty0 console=ttyS0')
         self.vm.add_args('-kernel', kernel_path,
-                         '-cpu', 'I6400',
                          '-smp', '%u' % cpu_cores_count,
                          '-vga', 'std',
                          '-append', kernel_command_line)
diff --git a/tests/acceptance/replay_kernel.py b/tests/acceptance/replay_kernel.py
index 71facdaa75..75f80506c1 100644
--- a/tests/acceptance/replay_kernel.py
+++ b/tests/acceptance/replay_kernel.py
@@ -156,8 +156,7 @@ def test_aarch64_virt(self):
                                'console=ttyAMA0')
         console_pattern = 'VFS: Cannot open root device'
 
-        self.run_rr(kernel_path, kernel_command_line, console_pattern,
-                    args=('-cpu', 'cortex-a53'))
+        self.run_rr(kernel_path, kernel_command_line, console_pattern)
 
     def test_arm_virt(self):
         """
@@ -301,7 +300,7 @@ def test_ppc64_e500(self):
         tar_url = ('https://www.qemu-advent-calendar.org'
                    '/2018/download/day19.tar.xz')
         file_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
-        self.do_test_advcal_2018(file_path, 'uImage', ('-cpu', 'e5500'))
+        self.do_test_advcal_2018(file_path, 'uImage')
 
     def test_ppc_g3beige(self):
         """
@@ -348,8 +347,7 @@ def test_xtensa_lx60(self):
         tar_url = ('https://www.qemu-advent-calendar.org'
                    '/2018/download/day02.tar.xz')
         file_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
-        self.do_test_advcal_2018(file_path, 'santas-sleigh-ride.elf',
-                                 args=('-cpu', 'dc233c'))
+        self.do_test_advcal_2018(file_path, 'santas-sleigh-ride.elf')
 
 @skipUnless(os.getenv('AVOCADO_TIMEOUT_EXPECTED'), 'Test might timeout')
 class ReplayKernelSlow(ReplayKernelBase):
diff --git a/tests/acceptance/reverse_debugging.py b/tests/acceptance/reverse_debugging.py
index be01aca217..d2921e70c3 100644
--- a/tests/acceptance/reverse_debugging.py
+++ b/tests/acceptance/reverse_debugging.py
@@ -207,4 +207,4 @@ def test_aarch64_virt(self):
         kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
 
         self.reverse_debugging(
-            args=('-kernel', kernel_path, '-cpu', 'cortex-a53'))
+            args=('-kernel', kernel_path))
diff --git a/tests/acceptance/tcg_plugins.py b/tests/acceptance/tcg_plugins.py
index aa6e18b62d..9ca1515c3b 100644
--- a/tests/acceptance/tcg_plugins.py
+++ b/tests/acceptance/tcg_plugins.py
@@ -25,7 +25,7 @@ class PluginKernelBase(LinuxKernelTest):
     KERNEL_COMMON_COMMAND_LINE = 'printk.time=1 panic=-1 '
 
     def run_vm(self, kernel_path, kernel_command_line,
-               plugin, plugin_log, console_pattern, args):
+               plugin, plugin_log, console_pattern, args=None):
 
         vm = self.get_vm()
         vm.set_console()
@@ -80,8 +80,7 @@ def test_aarch64_virt_insn(self):
 
         self.run_vm(kernel_path, kernel_command_line,
                     "tests/plugin/libinsn.so", plugin_log.name,
-                    console_pattern,
-                    args=('-cpu', 'cortex-a53'))
+                    console_pattern)
 
         with plugin_log as lf, \
              mmap.mmap(lf.fileno(), 0, access=mmap.ACCESS_READ) as s:
@@ -108,7 +107,7 @@ def test_aarch64_virt_insn_icount(self):
         self.run_vm(kernel_path, kernel_command_line,
                     "tests/plugin/libinsn.so", plugin_log.name,
                     console_pattern,
-                    args=('-cpu', 'cortex-a53', '-icount', 'shift=1'))
+                    args=('-icount', 'shift=1'))
 
         with plugin_log as lf, \
              mmap.mmap(lf.fileno(), 0, access=mmap.ACCESS_READ) as s:
@@ -134,7 +133,7 @@ def test_aarch64_virt_mem_icount(self):
         self.run_vm(kernel_path, kernel_command_line,
                     "tests/plugin/libmem.so,arg=both", plugin_log.name,
                     console_pattern,
-                    args=('-cpu', 'cortex-a53', '-icount', 'shift=1'))
+                    args=('-icount', 'shift=1'))
 
         with plugin_log as lf, \
              mmap.mmap(lf.fileno(), 0, access=mmap.ACCESS_READ) as s:
-- 
2.29.2

[PATCH v2 4/7] tests/acceptance: Tagging tests with "cpu:VALUE"

[Wainer dos Santos Moschetta]

The existing tests which are passing "-cpu VALUE" argument to the vm object
are now properly "cpu:VALUE" tagged, so letting the avocado_qemu framework to
handle that automatically.

Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
---
 tests/acceptance/boot_linux_console.py   | 16 +++++++++-------
 tests/acceptance/pc_cpu_hotplug_props.py |  2 +-
 tests/acceptance/replay_kernel.py        |  9 ++++++---
 tests/acceptance/virtio-gpu.py           |  4 ++--
 4 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
index 1ca32ecf25..b7a856d871 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -238,6 +238,7 @@ def test_mips64el_malta_5KEc_cpio(self):
         :avocado: tags=arch:mips64el
         :avocado: tags=machine:malta
         :avocado: tags=endian:little
+        :avocado: tags=cpu:5KEc
         """
         kernel_url = ('https://github.com/philmd/qemu-testing-blob/'
                       'raw/9ad2df38/mips/malta/mips64el/'
@@ -257,8 +258,7 @@ def test_mips64el_malta_5KEc_cpio(self):
         kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE
                                + 'console=ttyS0 console=tty '
                                + 'rdinit=/sbin/init noreboot')
-        self.vm.add_args('-cpu', '5KEc',
-                         '-kernel', kernel_path,
+        self.vm.add_args('-kernel', kernel_path,
                          '-initrd', initrd_path,
                          '-append', kernel_command_line,
                          '-no-reboot')
@@ -286,7 +286,6 @@ def do_test_mips_malta32el_nanomips(self, kernel_url, kernel_hash):
                                + 'mem=256m@@0x0 '
                                + 'console=ttyS0')
         self.vm.add_args('-no-reboot',
-                         '-cpu', 'I7200',
                          '-kernel', kernel_path,
                          '-append', kernel_command_line)
         self.vm.launch()
@@ -298,6 +297,7 @@ def test_mips_malta32el_nanomips_4k(self):
         :avocado: tags=arch:mipsel
         :avocado: tags=machine:malta
         :avocado: tags=endian:little
+        :avocado: tags=cpu:I7200
         """
         kernel_url = ('https://mipsdistros.mips.com/LinuxDistro/nanomips/'
                       'kernels/v4.15.18-432-gb2eb9a8b07a1-20180627102142/'
@@ -310,6 +310,7 @@ def test_mips_malta32el_nanomips_16k_up(self):
         :avocado: tags=arch:mipsel
         :avocado: tags=machine:malta
         :avocado: tags=endian:little
+        :avocado: tags=cpu:I7200
         """
         kernel_url = ('https://mipsdistros.mips.com/LinuxDistro/nanomips/'
                       'kernels/v4.15.18-432-gb2eb9a8b07a1-20180627102142/'
@@ -322,6 +323,7 @@ def test_mips_malta32el_nanomips_64k_dbg(self):
         :avocado: tags=arch:mipsel
         :avocado: tags=machine:malta
         :avocado: tags=endian:little
+        :avocado: tags=cpu:I7200
         """
         kernel_url = ('https://mipsdistros.mips.com/LinuxDistro/nanomips/'
                       'kernels/v4.15.18-432-gb2eb9a8b07a1-20180627102142/'
@@ -333,6 +335,7 @@ def test_aarch64_virt(self):
         """
         :avocado: tags=arch:aarch64
         :avocado: tags=machine:virt
+        :avocado: tags=cpu:cortex-a53
         """
         kernel_url = ('https://archives.fedoraproject.org/pub/archive/fedora'
                       '/linux/releases/29/Everything/aarch64/os/images/pxeboot'
@@ -343,8 +346,7 @@ def test_aarch64_virt(self):
         self.vm.set_console()
         kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
                                'console=ttyAMA0')
-        self.vm.add_args('-cpu', 'cortex-a53',
-                         '-kernel', kernel_path,
+        self.vm.add_args('-kernel', kernel_path,
                          '-append', kernel_command_line)
         self.vm.launch()
         console_pattern = 'Kernel command line: %s' % kernel_command_line
@@ -1038,9 +1040,9 @@ def test_ppc64_e500(self):
         """
         :avocado: tags=arch:ppc64
         :avocado: tags=machine:ppce500
+        :avocado: tags=cpu:e5500
         """
         tar_hash = '6951d86d644b302898da2fd701739c9406527fe1'
-        self.vm.add_args('-cpu', 'e5500')
         self.do_test_advcal_2018('19', tar_hash, 'uImage')
 
     def test_ppc_g3beige(self):
@@ -1082,7 +1084,7 @@ def test_xtensa_lx60(self):
         """
         :avocado: tags=arch:xtensa
         :avocado: tags=machine:lx60
+        :avocado: tags=cpu:dc233c
         """
         tar_hash = '49e88d9933742f0164b60839886c9739cb7a0d34'
-        self.vm.add_args('-cpu', 'dc233c')
         self.do_test_advcal_2018('02', tar_hash, 'santas-sleigh-ride.elf')
diff --git a/tests/acceptance/pc_cpu_hotplug_props.py b/tests/acceptance/pc_cpu_hotplug_props.py
index f48f68fc6b..2e86d5017a 100644
--- a/tests/acceptance/pc_cpu_hotplug_props.py
+++ b/tests/acceptance/pc_cpu_hotplug_props.py
@@ -25,11 +25,11 @@
 class OmittedCPUProps(Test):
     """
     :avocado: tags=arch:x86_64
+    :avocado: tags=cpu:qemu64
     """
     def test_no_die_id(self):
         self.vm.add_args('-nodefaults', '-S')
         self.vm.add_args('-smp', '1,sockets=2,cores=2,threads=2,maxcpus=8')
-        self.vm.add_args('-cpu', 'qemu64')
         self.vm.add_args('-device', 'qemu64-x86_64-cpu,socket-id=1,core-id=0,thread-id=0')
         self.vm.launch()
         self.assertEquals(len(self.vm.command('query-cpus-fast')), 2)
diff --git a/tests/acceptance/replay_kernel.py b/tests/acceptance/replay_kernel.py
index 75f80506c1..bb32b31240 100644
--- a/tests/acceptance/replay_kernel.py
+++ b/tests/acceptance/replay_kernel.py
@@ -392,6 +392,7 @@ def test_mips64el_malta_5KEc_cpio(self):
         :avocado: tags=machine:malta
         :avocado: tags=endian:little
         :avocado: tags=slowness:high
+        :avocado: tags=cpu:5KEc
         """
         kernel_url = ('https://github.com/philmd/qemu-testing-blob/'
                       'raw/9ad2df38/mips/malta/mips64el/'
@@ -412,7 +413,7 @@ def test_mips64el_malta_5KEc_cpio(self):
                                'rdinit=/sbin/init noreboot')
         console_pattern = 'Boot successful.'
         self.run_rr(kernel_path, kernel_command_line, console_pattern, shift=5,
-                    args=('-initrd', initrd_path, '-cpu', '5KEc'))
+                    args=('-initrd', initrd_path))
 
     def do_test_mips_malta32el_nanomips(self, kernel_path_xz):
         kernel_path = self.workdir + "kernel"
@@ -424,14 +425,14 @@ def do_test_mips_malta32el_nanomips(self, kernel_path_xz):
                                'mem=256m@@0x0 '
                                'console=ttyS0')
         console_pattern = 'Kernel command line: %s' % kernel_command_line
-        self.run_rr(kernel_path, kernel_command_line, console_pattern, shift=5,
-                    args=('-cpu', 'I7200'))
+        self.run_rr(kernel_path, kernel_command_line, console_pattern, shift=5)
 
     def test_mips_malta32el_nanomips_4k(self):
         """
         :avocado: tags=arch:mipsel
         :avocado: tags=machine:malta
         :avocado: tags=endian:little
+        :avocado: tags=cpu:I7200
         """
         kernel_url = ('https://mipsdistros.mips.com/LinuxDistro/nanomips/'
                       'kernels/v4.15.18-432-gb2eb9a8b07a1-20180627102142/'
@@ -445,6 +446,7 @@ def test_mips_malta32el_nanomips_16k_up(self):
         :avocado: tags=arch:mipsel
         :avocado: tags=machine:malta
         :avocado: tags=endian:little
+        :avocado: tags=cpu:I7200
         """
         kernel_url = ('https://mipsdistros.mips.com/LinuxDistro/nanomips/'
                       'kernels/v4.15.18-432-gb2eb9a8b07a1-20180627102142/'
@@ -458,6 +460,7 @@ def test_mips_malta32el_nanomips_64k_dbg(self):
         :avocado: tags=arch:mipsel
         :avocado: tags=machine:malta
         :avocado: tags=endian:little
+        :avocado: tags=cpu:I7200
         """
         kernel_url = ('https://mipsdistros.mips.com/LinuxDistro/nanomips/'
                       'kernels/v4.15.18-432-gb2eb9a8b07a1-20180627102142/'
diff --git a/tests/acceptance/virtio-gpu.py b/tests/acceptance/virtio-gpu.py
index ab18cddbb7..3494297b22 100644
--- a/tests/acceptance/virtio-gpu.py
+++ b/tests/acceptance/virtio-gpu.py
@@ -60,6 +60,7 @@ def test_virtio_vga_virgl(self):
         """
         :avocado: tags=arch:x86_64
         :avocado: tags=device:virtio-vga
+        :avocado: tags=cpu:host
         """
         kernel_command_line = (
             self.KERNEL_COMMON_COMMAND_LINE + "console=ttyS0 rdinit=/bin/bash"
@@ -72,7 +73,6 @@ def test_virtio_vga_virgl(self):
         initrd_path = self.fetch_asset(self.INITRD_URL)
 
         self.vm.set_console()
-        self.vm.add_args("-cpu", "host")
         self.vm.add_args("-m", "2G")
         self.vm.add_args("-machine", "pc,accel=kvm")
         self.vm.add_args("-device", "virtio-vga,virgl=on")
@@ -101,6 +101,7 @@ def test_vhost_user_vga_virgl(self):
         """
         :avocado: tags=arch:x86_64
         :avocado: tags=device:vhost-user-vga
+        :avocado: tags=cpu:host
         """
         kernel_command_line = (
             self.KERNEL_COMMON_COMMAND_LINE + "console=ttyS0 rdinit=/bin/bash"
@@ -140,7 +141,6 @@ def test_vhost_user_vga_virgl(self):
         )
 
         self.vm.set_console()
-        self.vm.add_args("-cpu", "host")
         self.vm.add_args("-m", "2G")
         self.vm.add_args("-object", "memory-backend-memfd,id=mem,size=2G")
         self.vm.add_args("-machine", "pc,memory-backend=mem,accel=kvm")
-- 
2.29.2

[PATCH v2 5/7] python/qemu: Add args property to the QEMUMachine class

[Wainer dos Santos Moschetta]

This added the args property to QEMUMachine so that users of the class
can access and handle the list of arguments to be given to the QEMU
binary.

Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
---
 python/qemu/machine.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 6e44bda337..1c30bde99d 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -302,6 +302,11 @@ def _base_args(self) -> List[str]:
                 args.extend(['-device', device])
         return args
 
+    @property
+    def args(self) -> List[str]:
+        """Returns the list of arguments given to the QEMU binary."""
+        return self._args
+
     def _pre_launch(self) -> None:
         self._temp_dir = tempfile.mkdtemp(prefix="qemu-machine-",
                                           dir=self._test_dir)
-- 
2.29.2

[PATCH v2 6/7] tests/acceptance: Add set_vm_arg() to the Test class

[Wainer dos Santos Moschetta]

The set_vm_arg method is added to avocado_qemu.Test class on this
change. Use that method to set (or replace) an argument to the list of
arguments given to the QEMU binary.

Suggested-by: Cleber Rosa <crosa@redhat.com>
Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
---
 tests/acceptance/avocado_qemu/__init__.py | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
index 7f8e703757..5314ce70eb 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -240,6 +240,22 @@ def get_vm(self, *args, name=None):
                 self._vms[name].set_machine(self.machine)
         return self._vms[name]
 
+    def set_vm_arg(self, arg, value):
+        """
+        Set an argument to list of extra arguments to be given to the QEMU
+        binary. If the argument already exists then its value is replaced.
+
+        :param arg: the QEMU argument, such as "-cpu" in "-cpu host"
+        :type arg: str
+        :param value: the argument value, such as "host" in "-cpu host"
+        :type value: str
+        """
+        if arg not in self.vm.args:
+            self.vm.args.extend([arg, value])
+        else:
+            idx = self.vm.args.index(arg)
+            self.vm.args[idx + 1] = value
+
     def tearDown(self):
         for vm in self._vms.values():
             vm.shutdown()
-- 
2.29.2

[PATCH v2 7/7] tests/acceptance: Handle cpu tag on x86_cpu_model_versions tests

[Wainer dos Santos Moschetta]

Some test cases on x86_cpu_model_versions.py are corner cases because they
need to pass extra options to the -cpu argument. Once the avocado_qemu
framework will set -cpu automatically, the value should be reset. This changed
those tests so to call set_vm_arg() to overwrite the -cpu value.

Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
---
 tests/acceptance/x86_cpu_model_versions.py | 40 +++++++++++++++++-----
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/tests/acceptance/x86_cpu_model_versions.py b/tests/acceptance/x86_cpu_model_versions.py
index 77ed8597a4..0e9feda62d 100644
--- a/tests/acceptance/x86_cpu_model_versions.py
+++ b/tests/acceptance/x86_cpu_model_versions.py
@@ -252,10 +252,13 @@ def get_cpu_prop(self, prop):
     def test_4_1(self):
         """
         :avocado: tags=machine:pc-i440fx-4.1
+        :avocado: tags=cpu:Cascadelake-Server
         """
         # machine-type only:
         self.vm.add_args('-S')
-        self.vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off')
+        self.set_vm_arg('-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')
@@ -263,9 +266,12 @@ def test_4_1(self):
     def test_4_0(self):
         """
         :avocado: tags=machine:pc-i440fx-4.0
+        :avocado: tags=cpu:Cascadelake-Server
         """
         self.vm.add_args('-S')
-        self.vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off')
+        self.set_vm_arg('-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')
@@ -273,10 +279,13 @@ def test_4_0(self):
     def test_set_4_0(self):
         """
         :avocado: tags=machine:pc-i440fx-4.0
+        :avocado: tags=cpu:Cascadelake-Server
         """
         # command line must override machine-type if CPU model is not versioned:
         self.vm.add_args('-S')
-        self.vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off,+arch-capabilities')
+        self.set_vm_arg('-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')
@@ -284,9 +293,12 @@ def test_set_4_0(self):
     def test_unset_4_1(self):
         """
         :avocado: tags=machine:pc-i440fx-4.1
+        :avocado: tags=cpu:Cascadelake-Server
         """
         self.vm.add_args('-S')
-        self.vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off,-arch-capabilities')
+        self.set_vm_arg('-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')
@@ -294,10 +306,13 @@ def test_unset_4_1(self):
     def test_v1_4_0(self):
         """
         :avocado: tags=machine:pc-i440fx-4.0
+        :avocado: tags=cpu:Cascadelake-Server
         """
         # versioned CPU model overrides machine-type:
         self.vm.add_args('-S')
-        self.vm.add_args('-cpu', 'Cascadelake-Server-v1,x-force-features=on,check=off,enforce=off')
+        self.set_vm_arg('-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')
@@ -305,9 +320,12 @@ def test_v1_4_0(self):
     def test_v2_4_0(self):
         """
         :avocado: tags=machine:pc-i440fx-4.0
+        :avocado: tags=cpu:Cascadelake-Server
         """
         self.vm.add_args('-S')
-        self.vm.add_args('-cpu', 'Cascadelake-Server-v2,x-force-features=on,check=off,enforce=off')
+        self.set_vm_arg('-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')
@@ -315,10 +333,13 @@ def test_v2_4_0(self):
     def test_v1_set_4_0(self):
         """
         :avocado: tags=machine:pc-i440fx-4.0
+        :avocado: tags=cpu:Cascadelake-Server
         """
         # command line must override machine-type and versioned CPU model:
         self.vm.add_args('-S')
-        self.vm.add_args('-cpu', 'Cascadelake-Server-v1,x-force-features=on,check=off,enforce=off,+arch-capabilities')
+        self.set_vm_arg('-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')
@@ -326,9 +347,12 @@ def test_v1_set_4_0(self):
     def test_v2_unset_4_1(self):
         """
         :avocado: tags=machine:pc-i440fx-4.1
+        :avocado: tags=cpu:Cascadelake-Server
         """
         self.vm.add_args('-S')
-        self.vm.add_args('-cpu', 'Cascadelake-Server-v2,x-force-features=on,check=off,enforce=off,-arch-capabilities')
+        self.set_vm_arg('-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.29.2

Re: [PATCH v2 0/7] tests/acceptance: Handle tests with "cpu" tag

[Cleber Rosa]

[-- Attachment #1: Type: text/plain, Size: 2904 bytes --]

On Thu, Apr 08, 2021 at 04:52:30PM -0300, Wainer dos Santos Moschetta wrote:
> Currently the acceptance tests tagged with "machine" have the "-M TYPE"
> automatically added to the list of arguments of the QEMUMachine object.
> In other words, that option is passed to the launched QEMU. On this
> series it is implemented the same feature but instead for tests marked
> with "cpu".
> 
> There is a caveat, however, in case the test needs additional arguments to
> the CPU type they cannot be passed via tag, because the tags parser split
> values by comma. For example, in tests/acceptance/x86_cpu_model_versions.py,
> there are cases where:

Hi Wainer,

I've created an Avocado issue to hopefully get rid of this limitation:

   https://github.com/avocado-framework/avocado/issues/4541

> 
>   * -cpu is set to "Cascadelake-Server,x-force-features=on,check=off,enforce=off"
>   * if it was tagged like "cpu:Cascadelake-Server,x-force-features=on,check=off,enforce=off"
>     then the parser would break it into 4 tags ("cpu:Cascadelake-Server",
>     "x-force-features=on", "check=off", "enforce=off")
>   * resulting on "-cpu Cascadelake-Server" and the remaining arguments are ignored.
> 
> It was introduced the avocado_qemu.Test.set_vm_arg() method to deal with
> cases like the example above, so that one can tag it as "cpu:Cascadelake-Server"
> AND call self.set_vm_args('-cpu', "Cascadelake-Server,x-force-features=on,check=off,enforce=off"),
> and that results on the reset of the initial value of -cpu.
>

So for now this seems reasonable enough.

> This series was tested on CI (https://gitlab.com/wainersm/qemu/-/pipelines/277376246)
> and with the following code:
> 
> from avocado_qemu import Test
> 
> class CPUTest(Test):
>     def test_cpu(self):
>         """
>         :avocado: tags=cpu:host
>         """
>         # The cpu property is set to the tag value, or None on its absence
>         self.assertEqual(self.cpu, "host")
>         # The created VM has the '-cpu host' option
>         self.assertIn("-cpu host", " ".join(self.vm._args))
>         self.vm.launch()
> 
>     def test_cpu_none(self):
>         self.assertEqual(self.cpu, None)
>         self.assertNotIn('-cpu', self.vm._args)
> 
>     def test_cpu_reset(self):
>         """
>         :avocado: tags=cpu:host
>         """
>         self.assertIn("-cpu host", " ".join(self.vm._args))
>         self.set_vm_arg("-cpu", "Cascadelake-Server,x-force-features=on")
>         self.assertNotIn("-cpu host", " ".join(self.vm._args))
>         self.assertIn("-cpu Cascadelake-Server,x-force-features=on", " ".join(self.vm._args))
>

We should not let this type of testing go to waste, so it's about time
to set aside a directory for tests that are about the framework,
rather than end user functionality.  I'll take a look at that.

Cheers,
- Cleber.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 1/7] tests/acceptance: Automatic set -cpu to the test vm

[Cleber Rosa]

[-- Attachment #1: Type: text/plain, Size: 1587 bytes --]

On Thu, Apr 08, 2021 at 04:52:31PM -0300, Wainer dos Santos Moschetta wrote:
> This introduces a new feature to the functional tests: automatic setting of
> the '-cpu VALUE' option to the created vm if the test is tagged with
> 'cpu:VALUE'. The 'cpu' property is made available to the test object as well.
> 
> For example, for a simple test as:
> 
>     def test(self):
>         """
>         :avocado: tags=cpu:host
>         """
>         self.assertEqual(self.cpu, "host")
>         self.vm.launch()
>

So I tried a few tests with different CPU models and it works as
expected.  One minor caveat is that using "host" has side effects
in some cases, causing tests to fail because they may also require
KVM to be enabled.

But this is a generic mechanism so I don't think it should be
concerned with that.

> The resulting QEMU evocation will be like:
> 
>     qemu-system-x86_64 -display none -vga none -chardev socket,id=mon,path=/var/tmp/avo_qemu_sock_pdgzbgd_/qemu-1135557-monitor.sock -mon chardev=mon,mode=control -cpu host

Only thing is: can we please just break this line (I could not ignore
a 174 character line :).

> 
> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> ---
>  docs/devel/testing.rst                    | 17 +++++++++++++++++
>  tests/acceptance/avocado_qemu/__init__.py |  5 +++++
>  2 files changed, 22 insertions(+)

With the line broken mentioned above (which I can take care of when
queueing this patch):

Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 2/7] tests/acceptance: Fix mismatch on cpu tagged tests

[Cleber Rosa]

[-- Attachment #1: Type: text/plain, Size: 4354 bytes --]

On Thu, Apr 08, 2021 at 04:52:32PM -0300, Wainer dos Santos Moschetta wrote:
> There are test cases on machine_mips_malta.py and tcg_plugins.py files
> where the cpu tag does not correspond to the value actually given to the QEMU
> binary. This fixed those tests tags.
> 
> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> ---
>  tests/acceptance/machine_mips_malta.py | 6 +++---
>  tests/acceptance/tcg_plugins.py        | 6 +++---
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/acceptance/machine_mips_malta.py b/tests/acceptance/machine_mips_malta.py
> index 7c9a4ee4d2..b1fd075f51 100644
> --- a/tests/acceptance/machine_mips_malta.py
> +++ b/tests/acceptance/machine_mips_malta.py
> @@ -96,7 +96,7 @@ def test_mips_malta_i6400_framebuffer_logo_1core(self):
>          """
>          :avocado: tags=arch:mips64el
>          :avocado: tags=machine:malta
> -        :avocado: tags=cpu:i6400
> +        :avocado: tags=cpu:I6400
>          """
>          self.do_test_i6400_framebuffer_logo(1)
>  
> @@ -105,7 +105,7 @@ def test_mips_malta_i6400_framebuffer_logo_7cores(self):
>          """
>          :avocado: tags=arch:mips64el
>          :avocado: tags=machine:malta
> -        :avocado: tags=cpu:i6400
> +        :avocado: tags=cpu:I6400
>          :avocado: tags=mips:smp
>          """
>          self.do_test_i6400_framebuffer_logo(7)
> @@ -115,7 +115,7 @@ def test_mips_malta_i6400_framebuffer_logo_8cores(self):
>          """
>          :avocado: tags=arch:mips64el
>          :avocado: tags=machine:malta
> -        :avocado: tags=cpu:i6400
> +        :avocado: tags=cpu:I6400
>          :avocado: tags=mips:smp
>          """

What about also changing the test names for consistency?

>          self.do_test_i6400_framebuffer_logo(8)
> diff --git a/tests/acceptance/tcg_plugins.py b/tests/acceptance/tcg_plugins.py
> index c21bf9e52a..aa6e18b62d 100644
> --- a/tests/acceptance/tcg_plugins.py
> +++ b/tests/acceptance/tcg_plugins.py
> @@ -68,7 +68,7 @@ def test_aarch64_virt_insn(self):
>          :avocado: tags=accel:tcg
>          :avocado: tags=arch:aarch64
>          :avocado: tags=machine:virt
> -        :avocado: tags=cpu:cortex-a57
> +        :avocado: tags=cpu:cortex-a53
>          """
>          kernel_path = self._grab_aarch64_kernel()
>          kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
> @@ -95,7 +95,7 @@ def test_aarch64_virt_insn_icount(self):
>          :avocado: tags=accel:tcg
>          :avocado: tags=arch:aarch64
>          :avocado: tags=machine:virt
> -        :avocado: tags=cpu:cortex-a57
> +        :avocado: tags=cpu:cortex-a53
>          """
>          kernel_path = self._grab_aarch64_kernel()
>          kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
> @@ -121,7 +121,7 @@ def test_aarch64_virt_mem_icount(self):
>          :avocado: tags=accel:tcg
>          :avocado: tags=arch:aarch64
>          :avocado: tags=machine:virt
> -        :avocado: tags=cpu:cortex-a57
> +        :avocado: tags=cpu:cortex-a53
>          """
>          kernel_path = self._grab_aarch64_kernel()
>          kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
> -- 
> 2.29.2
> 

Despite the suggestion, this is working fine:

JOB ID     : 90e4ddaa4f5b7d2b7d53f9f7a7300e9e9a94e8f4
JOB LOG    : /home/cleber/avocado/job-results/job-2021-04-21T17.05-90e4dda/job.log
 (1/6) tests/acceptance/machine_mips_malta.py:MaltaMachineFramebuffer.test_mips_malta_i6400_framebuffer_logo_1core: PASS (3.59 s)
 (2/6) tests/acceptance/machine_mips_malta.py:MaltaMachineFramebuffer.test_mips_malta_i6400_framebuffer_logo_7cores: PASS (16.54 s)
 (3/6) tests/acceptance/machine_mips_malta.py:MaltaMachineFramebuffer.test_mips_malta_i6400_framebuffer_logo_8cores: PASS (17.95 s)
 (4/6) tests/acceptance/tcg_plugins.py:PluginKernelNormal.test_aarch64_virt_insn: PASS (17.05 s)
 (5/6) tests/acceptance/tcg_plugins.py:PluginKernelNormal.test_aarch64_virt_insn_icount: PASS (14.94 s)
 (6/6) tests/acceptance/tcg_plugins.py:PluginKernelNormal.test_aarch64_virt_mem_icount: PASS (14.39 s)
RESULTS    : PASS 6 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB TIME   : 87.47 s

Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 3/7] tests/acceptance: Let the framework handle "cpu:VALUE" tagged tests

[Cleber Rosa]

[-- Attachment #1: Type: text/plain, Size: 771 bytes --]

On Thu, Apr 08, 2021 at 04:52:33PM -0300, Wainer dos Santos Moschetta wrote:
> The tests that are already tagged with "cpu:VALUE" don't need to add
> "-cpu VALUE" to the list of arguments of the vm object because the avocado_qemu
> framework is able to handle it automatically.
> 
> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> ---
>  tests/acceptance/boot_linux.py         | 3 ---
>  tests/acceptance/machine_mips_malta.py | 1 -
>  tests/acceptance/replay_kernel.py      | 8 +++-----
>  tests/acceptance/reverse_debugging.py  | 2 +-
>  tests/acceptance/tcg_plugins.py        | 9 ++++-----
>  5 files changed, 8 insertions(+), 15 deletions(-)
>

Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 4/7] tests/acceptance: Tagging tests with "cpu:VALUE"

[Cleber Rosa]

[-- Attachment #1: Type: text/plain, Size: 687 bytes --]

On Thu, Apr 08, 2021 at 04:52:34PM -0300, Wainer dos Santos Moschetta wrote:
> The existing tests which are passing "-cpu VALUE" argument to the vm object
> are now properly "cpu:VALUE" tagged, so letting the avocado_qemu framework to
> handle that automatically.
> 
> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> ---
>  tests/acceptance/boot_linux_console.py   | 16 +++++++++-------
>  tests/acceptance/pc_cpu_hotplug_props.py |  2 +-
>  tests/acceptance/replay_kernel.py        |  9 ++++++---
>  tests/acceptance/virtio-gpu.py           |  4 ++--
>  4 files changed, 18 insertions(+), 13 deletions(-)
>

Reviewed-by: Cleber Rosa <crosa@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 6/7] tests/acceptance: Add set_vm_arg() to the Test class

[Cleber Rosa]

[-- Attachment #1: Type: text/plain, Size: 1753 bytes --]

On Thu, Apr 08, 2021 at 04:52:36PM -0300, Wainer dos Santos Moschetta wrote:
> The set_vm_arg method is added to avocado_qemu.Test class on this
> change. Use that method to set (or replace) an argument to the list of
> arguments given to the QEMU binary.
> 
> Suggested-by: Cleber Rosa <crosa@redhat.com>
> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> ---
>  tests/acceptance/avocado_qemu/__init__.py | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> index 7f8e703757..5314ce70eb 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -240,6 +240,22 @@ def get_vm(self, *args, name=None):
>                  self._vms[name].set_machine(self.machine)
>          return self._vms[name]
>  
> +    def set_vm_arg(self, arg, value):
> +        """
> +        Set an argument to list of extra arguments to be given to the QEMU
> +        binary. If the argument already exists then its value is replaced.
> +
> +        :param arg: the QEMU argument, such as "-cpu" in "-cpu host"
> +        :type arg: str
> +        :param value: the argument value, such as "host" in "-cpu host"
> +        :type value: str
> +        """
> +        if arg not in self.vm.args:
> +            self.vm.args.extend([arg, value])
> +        else:
> +            idx = self.vm.args.index(arg)
> +            self.vm.args[idx + 1] = value
> +

This assumes that the arg will have a value, but that's not always the
case.  And, even if you were to pass an empty string, the logic would
overwrite the next (unrelated) arg.

Regards,
- Cleber.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 5/7] python/qemu: Add args property to the QEMUMachine class

[Cleber Rosa]

[-- Attachment #1: Type: text/plain, Size: 436 bytes --]

On Thu, Apr 08, 2021 at 04:52:35PM -0300, Wainer dos Santos Moschetta wrote:
> This added the args property to QEMUMachine so that users of the class
> can access and handle the list of arguments to be given to the QEMU
> binary.
> 
> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> ---
>  python/qemu/machine.py | 5 +++++
>  1 file changed, 5 insertions(+)
>

Reviewed-by: Cleber Rosa <crosa@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 7/7] tests/acceptance: Handle cpu tag on x86_cpu_model_versions tests

[Cleber Rosa]

[-- Attachment #1: Type: text/plain, Size: 738 bytes --]

On Thu, Apr 08, 2021 at 04:52:37PM -0300, Wainer dos Santos Moschetta wrote:
> Some test cases on x86_cpu_model_versions.py are corner cases because they
> need to pass extra options to the -cpu argument. Once the avocado_qemu
> framework will set -cpu automatically, the value should be reset. This changed
> those tests so to call set_vm_arg() to overwrite the -cpu value.
> 
> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> ---
>  tests/acceptance/x86_cpu_model_versions.py | 40 +++++++++++++++++-----
>  1 file changed, 32 insertions(+), 8 deletions(-)
>

This LGTM, but, since it depends on the previous patch, I'll hold my
R-b/T-b until I can test with the modified behavior.

Thanks!
- Cleber.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 1/7] tests/acceptance: Automatic set -cpu to the test vm

[Willian Rampazzo]

On Thu, Apr 8, 2021 at 5:01 PM Wainer dos Santos Moschetta
<wainersm@redhat.com> wrote:
>
> This introduces a new feature to the functional tests: automatic setting of
> the '-cpu VALUE' option to the created vm if the test is tagged with
> 'cpu:VALUE'. The 'cpu' property is made available to the test object as well.
>
> For example, for a simple test as:
>
>     def test(self):
>         """
>         :avocado: tags=cpu:host
>         """
>         self.assertEqual(self.cpu, "host")
>         self.vm.launch()
>
> The resulting QEMU evocation will be like:
>
>     qemu-system-x86_64 -display none -vga none -chardev socket,id=mon,path=/var/tmp/avo_qemu_sock_pdgzbgd_/qemu-1135557-monitor.sock -mon chardev=mon,mode=control -cpu host
>
> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> ---
>  docs/devel/testing.rst                    | 17 +++++++++++++++++
>  tests/acceptance/avocado_qemu/__init__.py |  5 +++++
>  2 files changed, 22 insertions(+)
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

Re: [PATCH v2 2/7] tests/acceptance: Fix mismatch on cpu tagged tests

[Willian Rampazzo]

On Thu, Apr 8, 2021 at 5:00 PM Wainer dos Santos Moschetta
<wainersm@redhat.com> wrote:
>
> There are test cases on machine_mips_malta.py and tcg_plugins.py files
> where the cpu tag does not correspond to the value actually given to the QEMU
> binary. This fixed those tests tags.
>
> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> ---
>  tests/acceptance/machine_mips_malta.py | 6 +++---
>  tests/acceptance/tcg_plugins.py        | 6 +++---
>  2 files changed, 6 insertions(+), 6 deletions(-)
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

Re: [PATCH v2 3/7] tests/acceptance: Let the framework handle "cpu:VALUE" tagged tests

[Willian Rampazzo]

On Thu, Apr 8, 2021 at 5:01 PM Wainer dos Santos Moschetta
<wainersm@redhat.com> wrote:
>
> The tests that are already tagged with "cpu:VALUE" don't need to add
> "-cpu VALUE" to the list of arguments of the vm object because the avocado_qemu
> framework is able to handle it automatically.
>
> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> ---
>  tests/acceptance/boot_linux.py         | 3 ---
>  tests/acceptance/machine_mips_malta.py | 1 -
>  tests/acceptance/replay_kernel.py      | 8 +++-----
>  tests/acceptance/reverse_debugging.py  | 2 +-
>  tests/acceptance/tcg_plugins.py        | 9 ++++-----
>  5 files changed, 8 insertions(+), 15 deletions(-)
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

Re: [PATCH v2 4/7] tests/acceptance: Tagging tests with "cpu:VALUE"

[Willian Rampazzo]

On Thu, Apr 8, 2021 at 4:59 PM Wainer dos Santos Moschetta
<wainersm@redhat.com> wrote:
>
> The existing tests which are passing "-cpu VALUE" argument to the vm object
> are now properly "cpu:VALUE" tagged, so letting the avocado_qemu framework to
> handle that automatically.
>
> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> ---
>  tests/acceptance/boot_linux_console.py   | 16 +++++++++-------
>  tests/acceptance/pc_cpu_hotplug_props.py |  2 +-
>  tests/acceptance/replay_kernel.py        |  9 ++++++---
>  tests/acceptance/virtio-gpu.py           |  4 ++--
>  4 files changed, 18 insertions(+), 13 deletions(-)
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

Re: [PATCH v2 5/7] python/qemu: Add args property to the QEMUMachine class

[Willian Rampazzo]

On Thu, Apr 8, 2021 at 5:00 PM Wainer dos Santos Moschetta
<wainersm@redhat.com> wrote:
>
> This added the args property to QEMUMachine so that users of the class
> can access and handle the list of arguments to be given to the QEMU
> binary.
>
> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> ---
>  python/qemu/machine.py | 5 +++++
>  1 file changed, 5 insertions(+)
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

Re: [PATCH v2 1/7] tests/acceptance: Automatic set -cpu to the test vm

[Wainer dos Santos Moschetta]

Hi,

On 4/21/21 5:16 PM, Cleber Rosa wrote:
> On Thu, Apr 08, 2021 at 04:52:31PM -0300, Wainer dos Santos Moschetta wrote:
>> This introduces a new feature to the functional tests: automatic setting of
>> the '-cpu VALUE' option to the created vm if the test is tagged with
>> 'cpu:VALUE'. The 'cpu' property is made available to the test object as well.
>>
>> For example, for a simple test as:
>>
>>      def test(self):
>>          """
>>          :avocado: tags=cpu:host
>>          """
>>          self.assertEqual(self.cpu, "host")
>>          self.vm.launch()
>>
> So I tried a few tests with different CPU models and it works as
> expected.  One minor caveat is that using "host" has side effects
> in some cases, causing tests to fail because they may also require
> KVM to be enabled.
>
> But this is a generic mechanism so I don't think it should be
> concerned with that.


Good point. Certainly I will consider this when reviewing new tests.


>
>> The resulting QEMU evocation will be like:
>>
>>      qemu-system-x86_64 -display none -vga none -chardev socket,id=mon,path=/var/tmp/avo_qemu_sock_pdgzbgd_/qemu-1135557-monitor.sock -mon chardev=mon,mode=control -cpu host
> Only thing is: can we please just break this line (I could not ignore
> a 174 character line :).
>
>> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
>> ---
>>   docs/devel/testing.rst                    | 17 +++++++++++++++++
>>   tests/acceptance/avocado_qemu/__init__.py |  5 +++++
>>   2 files changed, 22 insertions(+)
> With the line broken mentioned above (which I can take care of when
> queueing this patch):


I will send a v3 to address your review for patch 06, so I can take care 
of it.


>
> Reviewed-by: Cleber Rosa <crosa@redhat.com>
> Tested-by: Cleber Rosa <crosa@redhat.com>


Thanks for the reviews!

- Wainer