[PATCH v2 0/3] Acceptance tests: boot Linux with KVM test

[PATCH v2 0/3] Acceptance tests: boot Linux with KVM test

[Wainer dos Santos Moschetta]

This adds boot Linux tests for x86_64, aarch64, ppc64, s390x
targets which, unlike others, tell QEMU to use KVM. Likewise
it was added cases for tcg.

The patch 01 introduce an infraestructure on avocado_qemu framework
so that:
a) simply tagging the test with `accel:kvm` (or `accel:tcg`) will
   automatically set the corresponding '--accel' on the launched
   QEMU;
a) test is canceled if the accelerator is not enabled in QEMU
   binary and not present in the host.

Changes v1 -> v2:
- old patch 01 was removed becase the same feature is merged on
  commit 74b56bb55395.
- old patch 02 became 01 on v2, and with a major re-implementation. The
  bits that added the accel module is merged on commit 8b272e00186.
- old patch 03 was removed. Based on comments of ehabkost and crosa,
  instead the boot_linux_console test suite got new cases which uses
  the 'accel' tag to specify the use of kvm or tcg.
- Added patch 03 which refactor the handler of 'machine' tag that
  was introduced on commit ba21bde930f.

v1: [PATCH 0/3] Acceptance tests: boot Linux with KVM test
- https://www.mail-archive.com/qemu-devel@nongnu.org/msg627498.html

Tree:
- Git: https://github.com/wainersm/qemu
- Branch: acceptance_kvm_test-v2

CI:
- Travis (FAIL): https://travis-ci.org/wainersm/qemu/builds/626703965
  Not related with those change. Acceptance tests on Travis ran just
fine.

Wainer dos Santos Moschetta (3):
  tests/acceptance: avocado_qemu: Introduce the 'accel' test parameter
  tests/acceptance: boot_linux_console: Add boot Linux with kvm tests
  tests/acceptance: avocado_qemu: Refactor the handler of 'machine'
    parameter

 docs/devel/testing.rst                    | 16 +++++
 tests/acceptance/avocado_qemu/__init__.py | 27 ++++++-
 tests/acceptance/boot_linux_console.py    | 88 +++++++++++++++++------
 3 files changed, 109 insertions(+), 22 deletions(-)

-- 
2.23.0

[PATCH v2 1/3] tests/acceptance: avocado_qemu: Introduce the 'accel' test parameter

[Wainer dos Santos Moschetta]

The test case may need to boot the VM with an accelerator that
isn't actually enabled on the QEMU binary and/or present in the host. In
this case the test behavior is undefined, and the best course of
action is to skip its execution.

This change introduced the 'accel' parameter (and the handler of
tag with same name) used to indicate the test case requires a
given accelerator available. It was implemented a mechanism to
skip the test case if the accelerator is not available. Moreover,
 the QEMU --accel argument is set automatically to any VM
launched if the parameter is present.

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

diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index 27f286858a..6c2e0718e1 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -757,6 +757,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=machine:VALUE`` tag, it will be set to ``VALUE``.
 
+accel
+~~~~~
+The accelerator that will be set to all QEMUMachine instances created
+by the test.
+
+The ``accel`` 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=accel:VALUE`` tag, it will be set to ``VALUE``. Currently
+``VALUE`` should be either ``kvm`` or ``tcg``.
+
 qemu_bin
 ~~~~~~~~
 
@@ -798,6 +809,11 @@ machine
 The machine type that will be set to all QEMUMachine instances created
 by the test.
 
+accel
+~~~~~
+The accelerator that will be set to all QEMUMachine instances created
+by the test. In case the accelerator is not available (both QEMU
+binary and the host system are checked) then the test is canceled.
 
 qemu_bin
 ~~~~~~~~
diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
index 6618ea67c1..aff32668d9 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -20,6 +20,7 @@ SRC_ROOT_DIR = os.path.join(os.path.dirname(__file__), '..', '..', '..')
 sys.path.append(os.path.join(SRC_ROOT_DIR, 'python'))
 
 from qemu.machine import QEMUMachine
+from qemu.accel import kvm_available, tcg_available
 
 def is_readable_executable_file(path):
     return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK)
@@ -111,6 +112,8 @@ class Test(avocado.Test):
 
     def setUp(self):
         self._vms = {}
+        # VM argumments that are mapped from parameters
+        self._param_to_vm_args = []
 
         self.arch = self.params.get('arch',
                                     default=self._get_unique_tag_val('arch'))
@@ -124,10 +127,30 @@ class Test(avocado.Test):
         if self.qemu_bin is None:
             self.cancel("No QEMU binary defined or found in the source tree")
 
+        self.accel = self.params.get('accel',
+                                     default=self._get_unique_tag_val('accel'))
+        if self.accel:
+            avail = False
+            if self.accel == 'kvm':
+                if kvm_available(self.arch, self.qemu_bin):
+                    self._param_to_vm_args.append('-enable-kvm')
+                    avail = True
+            elif self.accel == 'tcg':
+                if tcg_available(self.qemu_bin):
+                    self._param_to_vm_args.extend(['--accel', 'tcg'])
+                    avail = True
+            else:
+                self.cancel("Unknown accelerator: %s" % self.accel)
+
+            if not avail:
+                self.cancel("%s is not available" % self.accel)
+
     def _new_vm(self, *args):
         vm = QEMUMachine(self.qemu_bin, sock_dir=tempfile.mkdtemp())
         if args:
             vm.add_args(*args)
+        if self._param_to_vm_args:
+            vm.add_args(*self._param_to_vm_args)
         return vm
 
     @property
-- 
2.23.0

[PATCH v2 2/3] tests/acceptance: boot_linux_console: Add boot Linux with kvm tests

[Wainer dos Santos Moschetta]

Added boot Linux test cases that launch QEMU with kvm
enabled. Likewise it was added tests for tcg.

Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
---
 tests/acceptance/boot_linux_console.py | 88 ++++++++++++++++++++------
 1 file changed, 68 insertions(+), 20 deletions(-)

diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
index 9c6aa2040a..d3730b43dc 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -51,11 +51,7 @@ class BootLinuxConsole(Test):
         os.chdir(cwd)
         return self.workdir + path
 
-    def test_x86_64_pc(self):
-        """
-        :avocado: tags=arch:x86_64
-        :avocado: tags=machine:pc
-        """
+    def do_test_x86_64_pc(self):
         kernel_url = ('https://archives.fedoraproject.org/pub/archive/fedora'
                       '/linux/releases/29/Everything/x86_64/os/images/pxeboot'
                       '/vmlinuz')
@@ -70,6 +66,22 @@ class BootLinuxConsole(Test):
         console_pattern = 'Kernel command line: %s' % kernel_command_line
         self.wait_for_console_pattern(console_pattern)
 
+    def test_x86_64_pc_kvm(self):
+        """
+        :avocado: tags=arch:x86_64
+        :avocado: tags=machine:pc
+        :avocado: tags=accel:kvm
+        """
+        self.do_test_x86_64_pc()
+
+    def test_x86_64_pc_tcg(self):
+        """
+        :avocado: tags=arch:x86_64
+        :avocado: tags=machine:pc
+        :avocado: tags=accel:tcg
+        """
+        self.do_test_x86_64_pc()
+
     def test_mips_malta(self):
         """
         :avocado: tags=arch:mips
@@ -258,11 +270,7 @@ class BootLinuxConsole(Test):
         kernel_hash = '18d1c68f2e23429e266ca39ba5349ccd0aeb7180'
         self.do_test_mips_malta32el_nanomips(kernel_url, kernel_hash)
 
-    def test_aarch64_virt(self):
-        """
-        :avocado: tags=arch:aarch64
-        :avocado: tags=machine:virt
-        """
+    def do_test_aarch64_virt(self):
         kernel_url = ('https://archives.fedoraproject.org/pub/archive/fedora'
                       '/linux/releases/29/Everything/aarch64/os/images/pxeboot'
                       '/vmlinuz')
@@ -279,6 +287,22 @@ class BootLinuxConsole(Test):
         console_pattern = 'Kernel command line: %s' % kernel_command_line
         self.wait_for_console_pattern(console_pattern)
 
+    def test_aarch64_virt_kvm(self):
+        """
+        :avocado: tags=arch:aarch64
+        :avocado: tags=machine:virt
+        :avocado: tags=accel:kvm
+        """
+        self.do_test_aarch64_virt()
+
+    def test_aarch64_virt_tcg(self):
+        """
+        :avocado: tags=arch:aarch64
+        :avocado: tags=machine:virt
+        :avocado: tags=accel:tcg
+        """
+        self.do_test_aarch64_virt()
+
     def test_arm_virt(self):
         """
         :avocado: tags=arch:arm
@@ -400,11 +424,7 @@ class BootLinuxConsole(Test):
         self.wait_for_console_pattern('Boot successful.')
         # TODO user command, for now the uart is stuck
 
-    def test_s390x_s390_ccw_virtio(self):
-        """
-        :avocado: tags=arch:s390x
-        :avocado: tags=machine:s390-ccw-virtio
-        """
+    def do_test_s390x_s390_ccw_virtio(self):
         kernel_url = ('https://archives.fedoraproject.org/pub/archive'
                       '/fedora-secondary/releases/29/Everything/s390x/os/images'
                       '/kernel.img')
@@ -420,6 +440,22 @@ class BootLinuxConsole(Test):
         console_pattern = 'Kernel command line: %s' % kernel_command_line
         self.wait_for_console_pattern(console_pattern)
 
+    def test_s390x_s390_ccw_virtio_kvm(self):
+        """
+        :avocado: tags=arch:s390x
+        :avocado: tags=machine:s390-ccw-virtio
+        :avocado: tags=accel:kvm
+        """
+        self.do_test_s390x_s390_ccw_virtio()
+
+    def test_s390x_s390_ccw_virtio_tcg(self):
+        """
+        :avocado: tags=arch:s390x
+        :avocado: tags=machine:s390-ccw-virtio
+        :avocado: tags=accel:tcg
+        """
+        self.do_test_s390x_s390_ccw_virtio()
+
     def test_alpha_clipper(self):
         """
         :avocado: tags=arch:alpha
@@ -441,11 +477,7 @@ class BootLinuxConsole(Test):
         console_pattern = 'Kernel command line: %s' % kernel_command_line
         self.wait_for_console_pattern(console_pattern)
 
-    def test_ppc64_pseries(self):
-        """
-        :avocado: tags=arch:ppc64
-        :avocado: tags=machine:pseries
-        """
+    def do_test_ppc64_pseries(self):
         kernel_url = ('https://archives.fedoraproject.org/pub/archive'
                       '/fedora-secondary/releases/29/Everything/ppc64le/os'
                       '/ppc/ppc64/vmlinuz')
@@ -460,6 +492,22 @@ class BootLinuxConsole(Test):
         console_pattern = 'Kernel command line: %s' % kernel_command_line
         self.wait_for_console_pattern(console_pattern)
 
+    def test_ppc64le_pseries_kvm(self):
+        """
+        :avocado: tags=arch:ppc64
+        :avocado: tags=machine:pseries
+        :avocado: tags=accel:kvm
+        """
+        self.do_test_ppc64_pseries()
+
+    def test_ppc64le_pseries_tcg(self):
+        """
+        :avocado: tags=arch:ppc64
+        :avocado: tags=machine:pseries
+        :avocado: tags=accel:tcg
+        """
+        self.do_test_ppc64_pseries()
+
     def test_m68k_q800(self):
         """
         :avocado: tags=arch:m68k
-- 
2.23.0

[PATCH v2 3/3] tests/acceptance: avocado_qemu: Refactor the handler of 'machine' parameter

[Wainer dos Santos Moschetta]

The Test._param_to_vm_args variable contain VM arguments that should be added
at launch which were originated from test parameters. Use this variable
to set -M from 'machine' parameter as well.

Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
---
 tests/acceptance/avocado_qemu/__init__.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
index aff32668d9..ba9539d511 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -120,6 +120,8 @@ class Test(avocado.Test):
 
         self.machine = self.params.get('machine',
                                        default=self._get_unique_tag_val('machine'))
+        if self.machine:
+            self._param_to_vm_args.extend(['-M', self.machine])
 
         default_qemu_bin = pick_default_qemu_bin(arch=self.arch)
         self.qemu_bin = self.params.get('qemu_bin',
@@ -162,8 +164,6 @@ 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):
-- 
2.23.0

Re: [PATCH v2 1/3] tests/acceptance: avocado_qemu: Introduce the 'accel' test parameter

[Thomas Huth]

On 18/12/2019 18.00, Wainer dos Santos Moschetta wrote:
> The test case may need to boot the VM with an accelerator that
> isn't actually enabled on the QEMU binary and/or present in the host. In
> this case the test behavior is undefined, and the best course of
> action is to skip its execution.
> 
> This change introduced the 'accel' parameter (and the handler of
> tag with same name) used to indicate the test case requires a
> given accelerator available. It was implemented a mechanism to
> skip the test case if the accelerator is not available. Moreover,
>  the QEMU --accel argument is set automatically to any VM
> launched if the parameter is present.
> 
> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> ---
>  docs/devel/testing.rst                    | 16 ++++++++++++++++
>  tests/acceptance/avocado_qemu/__init__.py | 23 +++++++++++++++++++++++
>  2 files changed, 39 insertions(+)
> 
> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
> index 27f286858a..6c2e0718e1 100644
> --- a/docs/devel/testing.rst
> +++ b/docs/devel/testing.rst
> @@ -757,6 +757,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=machine:VALUE`` tag, it will be set to ``VALUE``.
>  
> +accel
> +~~~~~
> +The accelerator that will be set to all QEMUMachine instances created
> +by the test.
> +
> +The ``accel`` 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=accel:VALUE`` tag, it will be set to ``VALUE``. Currently
> +``VALUE`` should be either ``kvm`` or ``tcg``.
> +
>  qemu_bin
>  ~~~~~~~~
>  
> @@ -798,6 +809,11 @@ machine
>  The machine type that will be set to all QEMUMachine instances created
>  by the test.
>  
> +accel
> +~~~~~
> +The accelerator that will be set to all QEMUMachine instances created
> +by the test. In case the accelerator is not available (both QEMU
> +binary and the host system are checked) then the test is canceled.
>  
>  qemu_bin
>  ~~~~~~~~
> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> index 6618ea67c1..aff32668d9 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -20,6 +20,7 @@ SRC_ROOT_DIR = os.path.join(os.path.dirname(__file__), '..', '..', '..')
>  sys.path.append(os.path.join(SRC_ROOT_DIR, 'python'))
>  
>  from qemu.machine import QEMUMachine
> +from qemu.accel import kvm_available, tcg_available
>  
>  def is_readable_executable_file(path):
>      return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK)
> @@ -111,6 +112,8 @@ class Test(avocado.Test):
>  
>      def setUp(self):
>          self._vms = {}
> +        # VM argumments that are mapped from parameters
> +        self._param_to_vm_args = []
>  
>          self.arch = self.params.get('arch',
>                                      default=self._get_unique_tag_val('arch'))
> @@ -124,10 +127,30 @@ class Test(avocado.Test):
>          if self.qemu_bin is None:
>              self.cancel("No QEMU binary defined or found in the source tree")
>  
> +        self.accel = self.params.get('accel',
> +                                     default=self._get_unique_tag_val('accel'))
> +        if self.accel:
> +            avail = False
> +            if self.accel == 'kvm':
> +                if kvm_available(self.arch, self.qemu_bin):
> +                    self._param_to_vm_args.append('-enable-kvm')

Could you please use "-accel kvm" instead? "-accel" is now our official
way to configure an accelerator ... so we should not use the old
wrappers in new code anymore if possible.

 Thanks,
  Thomas


PS: Travis supports KVM now, too (with some tweaking of the permissions)
... maybe we should now try to get some QEMU tests running with KVM
there, too...

Re: [PATCH v2 1/3] tests/acceptance: avocado_qemu: Introduce the 'accel' test parameter

[Wainer dos Santos Moschetta]

Hi Thomas,

On 12/18/19 4:48 PM, Thomas Huth wrote:
> On 18/12/2019 18.00, Wainer dos Santos Moschetta wrote:
>> The test case may need to boot the VM with an accelerator that
>> isn't actually enabled on the QEMU binary and/or present in the host. In
>> this case the test behavior is undefined, and the best course of
>> action is to skip its execution.
>>
>> This change introduced the 'accel' parameter (and the handler of
>> tag with same name) used to indicate the test case requires a
>> given accelerator available. It was implemented a mechanism to
>> skip the test case if the accelerator is not available. Moreover,
>>   the QEMU --accel argument is set automatically to any VM
>> launched if the parameter is present.
>>
>> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
>> ---
>>   docs/devel/testing.rst                    | 16 ++++++++++++++++
>>   tests/acceptance/avocado_qemu/__init__.py | 23 +++++++++++++++++++++++
>>   2 files changed, 39 insertions(+)
>>
>> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
>> index 27f286858a..6c2e0718e1 100644
>> --- a/docs/devel/testing.rst
>> +++ b/docs/devel/testing.rst
>> @@ -757,6 +757,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=machine:VALUE`` tag, it will be set to ``VALUE``.
>>   
>> +accel
>> +~~~~~
>> +The accelerator that will be set to all QEMUMachine instances created
>> +by the test.
>> +
>> +The ``accel`` 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=accel:VALUE`` tag, it will be set to ``VALUE``. Currently
>> +``VALUE`` should be either ``kvm`` or ``tcg``.
>> +
>>   qemu_bin
>>   ~~~~~~~~
>>   
>> @@ -798,6 +809,11 @@ machine
>>   The machine type that will be set to all QEMUMachine instances created
>>   by the test.
>>   
>> +accel
>> +~~~~~
>> +The accelerator that will be set to all QEMUMachine instances created
>> +by the test. In case the accelerator is not available (both QEMU
>> +binary and the host system are checked) then the test is canceled.
>>   
>>   qemu_bin
>>   ~~~~~~~~
>> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
>> index 6618ea67c1..aff32668d9 100644
>> --- a/tests/acceptance/avocado_qemu/__init__.py
>> +++ b/tests/acceptance/avocado_qemu/__init__.py
>> @@ -20,6 +20,7 @@ SRC_ROOT_DIR = os.path.join(os.path.dirname(__file__), '..', '..', '..')
>>   sys.path.append(os.path.join(SRC_ROOT_DIR, 'python'))
>>   
>>   from qemu.machine import QEMUMachine
>> +from qemu.accel import kvm_available, tcg_available
>>   
>>   def is_readable_executable_file(path):
>>       return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK)
>> @@ -111,6 +112,8 @@ class Test(avocado.Test):
>>   
>>       def setUp(self):
>>           self._vms = {}
>> +        # VM argumments that are mapped from parameters
>> +        self._param_to_vm_args = []
>>   
>>           self.arch = self.params.get('arch',
>>                                       default=self._get_unique_tag_val('arch'))
>> @@ -124,10 +127,30 @@ class Test(avocado.Test):
>>           if self.qemu_bin is None:
>>               self.cancel("No QEMU binary defined or found in the source tree")
>>   
>> +        self.accel = self.params.get('accel',
>> +                                     default=self._get_unique_tag_val('accel'))
>> +        if self.accel:
>> +            avail = False
>> +            if self.accel == 'kvm':
>> +                if kvm_available(self.arch, self.qemu_bin):
>> +                    self._param_to_vm_args.append('-enable-kvm')
> Could you please use "-accel kvm" instead? "-accel" is now our official
> way to configure an accelerator ... so we should not use the old
> wrappers in new code anymore if possible.
Sure, I am going to adjust that on v3.
>
>   Thanks,
>    Thomas
>
>
> PS: Travis supports KVM now, too (with some tweaking of the permissions)
> ... maybe we should now try to get some QEMU tests running with KVM
> there, too...

I heard that but I failed miserably to enable nested virt on Travis. 
Actually I was expecting it enabled by default but not the case. I did 
not find documentation so I tried some tweaks like setting 
'sudo:required' and using bionic but none of that worked out.

Do you know what needs to be done?

Thanks!

- Wainer

Re: [PATCH v2 1/3] tests/acceptance: avocado_qemu: Introduce the 'accel' test parameter

[Thomas Huth]

On 10/01/2020 21.02, Wainer dos Santos Moschetta wrote:
> Hi Thomas,
> 
> On 12/18/19 4:48 PM, Thomas Huth wrote:
>> On 18/12/2019 18.00, Wainer dos Santos Moschetta wrote:
>>> The test case may need to boot the VM with an accelerator that
>>> isn't actually enabled on the QEMU binary and/or present in the host. In
>>> this case the test behavior is undefined, and the best course of
>>> action is to skip its execution.
>>>
>>> This change introduced the 'accel' parameter (and the handler of
>>> tag with same name) used to indicate the test case requires a
>>> given accelerator available. It was implemented a mechanism to
>>> skip the test case if the accelerator is not available. Moreover,
>>>   the QEMU --accel argument is set automatically to any VM
>>> launched if the parameter is present.
>>>
>>> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
>>> ---
>>>   docs/devel/testing.rst                    | 16 ++++++++++++++++
>>>   tests/acceptance/avocado_qemu/__init__.py | 23 +++++++++++++++++++++++
>>>   2 files changed, 39 insertions(+)
>>>
>>> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
>>> index 27f286858a..6c2e0718e1 100644
>>> --- a/docs/devel/testing.rst
>>> +++ b/docs/devel/testing.rst
>>> @@ -757,6 +757,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=machine:VALUE`` tag, it will be set to ``VALUE``.
>>>   +accel
>>> +~~~~~
>>> +The accelerator that will be set to all QEMUMachine instances created
>>> +by the test.
>>> +
>>> +The ``accel`` 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=accel:VALUE`` tag, it will be set to ``VALUE``.
>>> Currently
>>> +``VALUE`` should be either ``kvm`` or ``tcg``.
>>> +
>>>   qemu_bin
>>>   ~~~~~~~~
>>>   @@ -798,6 +809,11 @@ machine
>>>   The machine type that will be set to all QEMUMachine instances created
>>>   by the test.
>>>   +accel
>>> +~~~~~
>>> +The accelerator that will be set to all QEMUMachine instances created
>>> +by the test. In case the accelerator is not available (both QEMU
>>> +binary and the host system are checked) then the test is canceled.
>>>     qemu_bin
>>>   ~~~~~~~~
>>> diff --git a/tests/acceptance/avocado_qemu/__init__.py
>>> b/tests/acceptance/avocado_qemu/__init__.py
>>> index 6618ea67c1..aff32668d9 100644
>>> --- a/tests/acceptance/avocado_qemu/__init__.py
>>> +++ b/tests/acceptance/avocado_qemu/__init__.py
>>> @@ -20,6 +20,7 @@ SRC_ROOT_DIR =
>>> os.path.join(os.path.dirname(__file__), '..', '..', '..')
>>>   sys.path.append(os.path.join(SRC_ROOT_DIR, 'python'))
>>>     from qemu.machine import QEMUMachine
>>> +from qemu.accel import kvm_available, tcg_available
>>>     def is_readable_executable_file(path):
>>>       return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK)
>>> @@ -111,6 +112,8 @@ class Test(avocado.Test):
>>>         def setUp(self):
>>>           self._vms = {}
>>> +        # VM argumments that are mapped from parameters
>>> +        self._param_to_vm_args = []
>>>             self.arch = self.params.get('arch',
>>>                                      
>>> default=self._get_unique_tag_val('arch'))
>>> @@ -124,10 +127,30 @@ class Test(avocado.Test):
>>>           if self.qemu_bin is None:
>>>               self.cancel("No QEMU binary defined or found in the
>>> source tree")
>>>   +        self.accel = self.params.get('accel',
>>> +                                    
>>> default=self._get_unique_tag_val('accel'))
>>> +        if self.accel:
>>> +            avail = False
>>> +            if self.accel == 'kvm':
>>> +                if kvm_available(self.arch, self.qemu_bin):
>>> +                    self._param_to_vm_args.append('-enable-kvm')
>> Could you please use "-accel kvm" instead? "-accel" is now our official
>> way to configure an accelerator ... so we should not use the old
>> wrappers in new code anymore if possible.
> Sure, I am going to adjust that on v3.
>>
>>   Thanks,
>>    Thomas
>>
>>
>> PS: Travis supports KVM now, too (with some tweaking of the permissions)
>> ... maybe we should now try to get some QEMU tests running with KVM
>> there, too...
> 
> I heard that but I failed miserably to enable nested virt on Travis.
> Actually I was expecting it enabled by default but not the case. I did
> not find documentation so I tried some tweaks like setting
> 'sudo:required' and using bionic but none of that worked out.
> 
> Do you know what needs to be done?

Yes, I recently enabled it for the kvm-unit-tests ... and yes, it's a
bit ugly: The user has to be in the "kvm" group which is not the case
for the user that runs the travis scripts. Tweaking the access rights of
/dev/kvm unfortunately does not work (at least not directly via chmod
o+rwx /dev/kvm ... but maybe there is a way via udev or something
similar?), so I ended up with:

      sudo chgrp kvm /usr/bin/qemu-system-*
      sudo chmod g+s /usr/bin/qemu-system-*

With that, the kvm-unit-tests are running now fine with KVM on Travis.

 Thomas

Re: [PATCH v2 1/3] tests/acceptance: avocado_qemu: Introduce the 'accel' test parameter

[Wainer dos Santos Moschetta]

On 1/11/20 6:56 AM, Thomas Huth wrote:
> On 10/01/2020 21.02, Wainer dos Santos Moschetta wrote:
>> Hi Thomas,
>>
>> On 12/18/19 4:48 PM, Thomas Huth wrote:
>>> On 18/12/2019 18.00, Wainer dos Santos Moschetta wrote:
>>>> The test case may need to boot the VM with an accelerator that
>>>> isn't actually enabled on the QEMU binary and/or present in the host. In
>>>> this case the test behavior is undefined, and the best course of
>>>> action is to skip its execution.
>>>>
>>>> This change introduced the 'accel' parameter (and the handler of
>>>> tag with same name) used to indicate the test case requires a
>>>> given accelerator available. It was implemented a mechanism to
>>>> skip the test case if the accelerator is not available. Moreover,
>>>>    the QEMU --accel argument is set automatically to any VM
>>>> launched if the parameter is present.
>>>>
>>>> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
>>>> ---
>>>>    docs/devel/testing.rst                    | 16 ++++++++++++++++
>>>>    tests/acceptance/avocado_qemu/__init__.py | 23 +++++++++++++++++++++++
>>>>    2 files changed, 39 insertions(+)
>>>>
>>>> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
>>>> index 27f286858a..6c2e0718e1 100644
>>>> --- a/docs/devel/testing.rst
>>>> +++ b/docs/devel/testing.rst
>>>> @@ -757,6 +757,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=machine:VALUE`` tag, it will be set to ``VALUE``.
>>>>    +accel
>>>> +~~~~~
>>>> +The accelerator that will be set to all QEMUMachine instances created
>>>> +by the test.
>>>> +
>>>> +The ``accel`` 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=accel:VALUE`` tag, it will be set to ``VALUE``.
>>>> Currently
>>>> +``VALUE`` should be either ``kvm`` or ``tcg``.
>>>> +
>>>>    qemu_bin
>>>>    ~~~~~~~~
>>>>    @@ -798,6 +809,11 @@ machine
>>>>    The machine type that will be set to all QEMUMachine instances created
>>>>    by the test.
>>>>    +accel
>>>> +~~~~~
>>>> +The accelerator that will be set to all QEMUMachine instances created
>>>> +by the test. In case the accelerator is not available (both QEMU
>>>> +binary and the host system are checked) then the test is canceled.
>>>>      qemu_bin
>>>>    ~~~~~~~~
>>>> diff --git a/tests/acceptance/avocado_qemu/__init__.py
>>>> b/tests/acceptance/avocado_qemu/__init__.py
>>>> index 6618ea67c1..aff32668d9 100644
>>>> --- a/tests/acceptance/avocado_qemu/__init__.py
>>>> +++ b/tests/acceptance/avocado_qemu/__init__.py
>>>> @@ -20,6 +20,7 @@ SRC_ROOT_DIR =
>>>> os.path.join(os.path.dirname(__file__), '..', '..', '..')
>>>>    sys.path.append(os.path.join(SRC_ROOT_DIR, 'python'))
>>>>      from qemu.machine import QEMUMachine
>>>> +from qemu.accel import kvm_available, tcg_available
>>>>      def is_readable_executable_file(path):
>>>>        return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK)
>>>> @@ -111,6 +112,8 @@ class Test(avocado.Test):
>>>>          def setUp(self):
>>>>            self._vms = {}
>>>> +        # VM argumments that are mapped from parameters
>>>> +        self._param_to_vm_args = []
>>>>              self.arch = self.params.get('arch',
>>>>                                       
>>>> default=self._get_unique_tag_val('arch'))
>>>> @@ -124,10 +127,30 @@ class Test(avocado.Test):
>>>>            if self.qemu_bin is None:
>>>>                self.cancel("No QEMU binary defined or found in the
>>>> source tree")
>>>>    +        self.accel = self.params.get('accel',
>>>> +
>>>> default=self._get_unique_tag_val('accel'))
>>>> +        if self.accel:
>>>> +            avail = False
>>>> +            if self.accel == 'kvm':
>>>> +                if kvm_available(self.arch, self.qemu_bin):
>>>> +                    self._param_to_vm_args.append('-enable-kvm')
>>> Could you please use "-accel kvm" instead? "-accel" is now our official
>>> way to configure an accelerator ... so we should not use the old
>>> wrappers in new code anymore if possible.
>> Sure, I am going to adjust that on v3.
>>>    Thanks,
>>>     Thomas
>>>
>>>
>>> PS: Travis supports KVM now, too (with some tweaking of the permissions)
>>> ... maybe we should now try to get some QEMU tests running with KVM
>>> there, too...
>> I heard that but I failed miserably to enable nested virt on Travis.
>> Actually I was expecting it enabled by default but not the case. I did
>> not find documentation so I tried some tweaks like setting
>> 'sudo:required' and using bionic but none of that worked out.
>>
>> Do you know what needs to be done?
> Yes, I recently enabled it for the kvm-unit-tests ... and yes, it's a
> bit ugly: The user has to be in the "kvm" group which is not the case
> for the user that runs the travis scripts. Tweaking the access rights of
> /dev/kvm unfortunately does not work (at least not directly via chmod
> o+rwx /dev/kvm ... but maybe there is a way via udev or something
> similar?), so I ended up with:
>
>        sudo chgrp kvm /usr/bin/qemu-system-*
>        sudo chmod g+s /usr/bin/qemu-system-*
>
> With that, the kvm-unit-tests are running now fine with KVM on Travis.

Hi Thomas,

Thanks for pointing out how you did setup Travis+KVM on kvm-unit-tests.

I just sent a v3. You can see that I changed the Travis's acceptance 
tests builder to run on Ubuntu Bionic, and tweaked the access rights 
(chmod o+rw /dev/kvm) which, unlike on kvm-unit-tests, it is enough to 
make KVM available for the acceptance tests.

- Wainer

>
>   Thomas

Re: [PATCH v2 3/3] tests/acceptance: avocado_qemu: Refactor the handler of 'machine' parameter

[Philippe Mathieu-Daudé]

On 12/18/19 6:00 PM, Wainer dos Santos Moschetta wrote:
> The Test._param_to_vm_args variable contain VM arguments that should be added
> at launch which were originated from test parameters. Use this variable
> to set -M from 'machine' parameter as well.
> 
> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> ---
>   tests/acceptance/avocado_qemu/__init__.py | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> index aff32668d9..ba9539d511 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -120,6 +120,8 @@ class Test(avocado.Test):
>   
>           self.machine = self.params.get('machine',
>                                          default=self._get_unique_tag_val('machine'))
> +        if self.machine:
> +            self._param_to_vm_args.extend(['-M', self.machine])
>   
>           default_qemu_bin = pick_default_qemu_bin(arch=self.arch)
>           self.qemu_bin = self.params.get('qemu_bin',
> @@ -162,8 +164,6 @@ 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):
> 

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

Also, applied to my python-next tree:
https://gitlab.com/philmd/qemu/commits/python-next

Re: [PATCH v2 3/3] tests/acceptance: avocado_qemu: Refactor the handler of 'machine' parameter

[Philippe Mathieu-Daudé]

On Thu, Jan 30, 2020 at 11:56 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
> On 12/18/19 6:00 PM, Wainer dos Santos Moschetta wrote:
> > The Test._param_to_vm_args variable contain VM arguments that should be added
> > at launch which were originated from test parameters. Use this variable
> > to set -M from 'machine' parameter as well.
> >
> > Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> > ---
> >   tests/acceptance/avocado_qemu/__init__.py | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> > index aff32668d9..ba9539d511 100644
> > --- a/tests/acceptance/avocado_qemu/__init__.py
> > +++ b/tests/acceptance/avocado_qemu/__init__.py
> > @@ -120,6 +120,8 @@ class Test(avocado.Test):
> >
> >           self.machine = self.params.get('machine',
> >                                          default=self._get_unique_tag_val('machine'))
> > +        if self.machine:
> > +            self._param_to_vm_args.extend(['-M', self.machine])
> >
> >           default_qemu_bin = pick_default_qemu_bin(arch=self.arch)
> >           self.qemu_bin = self.params.get('qemu_bin',
> > @@ -162,8 +164,6 @@ 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):
> >
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
> Also, applied to my python-next tree:
> https://gitlab.com/philmd/qemu/commits/python-next

Oops, this depends of the previous patch (which has a v3).
I'm removing this patch.