[Qemu-devel] [PATCH 0/7] Acceptance Tests: basic architecture support

[Qemu-devel] [PATCH 0/7] Acceptance Tests: basic architecture support

[Cleber Rosa]

The current version of the Acceptance Tests have been basically tested
on x86_64.  Most of them should be valid tests on many different
architectures.

This introduces another standard test parameter, 'arch', and a public
test attribute with the same name.  Then, because of the different
behavior in different QEMU targets, it adds a more explicit
configuration of the QEMUMachine machine type used on the tests (the
self.vm attribute).

Finally, for tests that are known to be architecture specific, it
changes the approach, from using tags to canceling the test.  The
difference is that this reuses the same 'arch' parameter (so no need
to pass tags for the same reason), and instead of completely excluding
the test from the job, it just won't be executed on architectures that
are not supported.  More details about this on the last commit.

Cleber Rosa (7):
  Acceptance Tests: improve docstring on pick_default_qemu_bin()
  Acceptance Tests: introduce arch parameter and attribute
  scripts/qemu.py: add method and private attribute for arch
  scripts/qemu.py: set predefined machine type based on arch
  Acceptance Tests: set machine type
  Acceptance Tests: add variants definition for architectures
  Acceptance Tests: change the handling of tests for specific archs

 docs/devel/testing.rst                    | 18 ++++++++++++++
 scripts/qemu.py                           | 29 ++++++++++++++++++++++-
 tests/acceptance/avocado_qemu/__init__.py | 17 ++++++++++---
 tests/acceptance/boot_linux_console.py    |  6 +++--
 tests/acceptance/variants/arch.json       |  1 +
 tests/acceptance/version.py               |  2 ++
 tests/acceptance/vnc.py                   |  5 ++++
 7 files changed, 72 insertions(+), 6 deletions(-)
 create mode 100644 tests/acceptance/variants/arch.json

-- 
2.17.1

[Qemu-devel] [PATCH 1/7] Acceptance Tests: improve docstring on pick_default_qemu_bin()

[Cleber Rosa]

Making it clear what is returned by this utility function.

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

diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
index 1e54fd5932..d8d5b48dac 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -27,6 +27,10 @@ def pick_default_qemu_bin():
     """
     Picks the path of a QEMU binary, starting either in the current working
     directory or in the source tree root directory.
+
+    :returns: the path to the default QEMU binary or None if one could not
+              be found
+    :rtype: str or None
     """
     arch = os.uname()[4]
     qemu_bin_relative_path = os.path.join("%s-softmmu" % arch,
-- 
2.17.1

[Qemu-devel] [PATCH 2/7] Acceptance Tests: introduce arch parameter and attribute

[Cleber Rosa]

On a number of different scenarios, such as when choosing a QEMU
binary to be used on tests (or a image to use to boot a test VM), it's
useful to define the architecture that should be used.

This introduces both a test parameter and a test instance attribute,
that will contain such a value.

The selection of the default QEMU binary, if one is not given
explicitly, has also been changed to look at the arch attribute.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 docs/devel/testing.rst                    | 18 ++++++++++++++++++
 tests/acceptance/avocado_qemu/__init__.py | 13 ++++++++++---
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index 727c4019b5..4178ae5022 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -659,6 +659,17 @@ vm
 A QEMUMachine instance, initially configured according to the given
 ``qemu_bin`` parameter.
 
+arch
+~~~~
+
+The architecture that will be used on a number of different
+scenarios.  For instance, when a QEMU binary is not explicitly given,
+the one selected will depend on this attribute.
+
+The ``arch`` attribute will be set to the test parameter of the same
+name, and if one is not given explicitly, it will default to the
+system architecture (as given by ``uname``).
+
 qemu_bin
 ~~~~~~~~
 
@@ -681,6 +692,13 @@ like the following:
 
   PARAMS (key=qemu_bin, path=*, default=x86_64-softmmu/qemu-system-x86_64) => 'x86_64-softmmu/qemu-system-x86_64
 
+arch
+~~~~
+
+The architecture that will be used on a number of different scenarios.
+This parameter has a direct relation with the ``arch`` attribute.  If
+not given, it will default to None.
+
 qemu_bin
 ~~~~~~~~
 
diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
index d8d5b48dac..9329d9b9ec 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -23,16 +23,22 @@ def is_readable_executable_file(path):
     return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK)
 
 
-def pick_default_qemu_bin():
+def pick_default_qemu_bin(arch=None):
     """
     Picks the path of a QEMU binary, starting either in the current working
     directory or in the source tree root directory.
 
+    :param arch: the arch to use when looking for a QEMU binary (the target
+                 will match the arch given).  If None (the default) arch
+                 will be the current host system arch (as given by
+                 :func:`os.uname`).
+    :param arch: str
     :returns: the path to the default QEMU binary or None if one could not
               be found
     :rtype: str or None
     """
-    arch = os.uname()[4]
+    if arch is None:
+        arch = os.uname()[4]
     qemu_bin_relative_path = os.path.join("%s-softmmu" % arch,
                                           "qemu-system-%s" % arch)
     if is_readable_executable_file(qemu_bin_relative_path):
@@ -47,8 +53,9 @@ def pick_default_qemu_bin():
 class Test(avocado.Test):
     def setUp(self):
         self.vm = None
+        self.arch = self.params.get('arch', default=os.uname()[4])
         self.qemu_bin = self.params.get('qemu_bin',
-                                        default=pick_default_qemu_bin())
+                                        default=pick_default_qemu_bin(self.arch))
         if self.qemu_bin is None:
             self.cancel("No QEMU binary defined or found in the source tree")
         self.vm = QEMUMachine(self.qemu_bin)
-- 
2.17.1

[Qemu-devel] [PATCH 3/7] scripts/qemu.py: add method and private attribute for arch

[Cleber Rosa]

Because some sane defaults may require the knowledge of the arch,
let's give the QEMUMachine the opportunity to hold that information.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 scripts/qemu.py | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index f099ce7278..d9e24a0c1a 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -113,6 +113,7 @@ class QEMUMachine(object):
         self._test_dir = test_dir
         self._temp_dir = None
         self._launched = False
+        self._arch = None
         self._machine = None
         self._console_device_type = None
         self._console_address = None
@@ -406,6 +407,12 @@ class QEMUMachine(object):
         '''
         self._args.extend(args)
 
+    def set_arch(self, arch):
+        """
+        Sets the architecture of this machine
+        """
+        self._arch = arch
+
     def set_machine(self, machine_type):
         '''
         Sets the machine type
-- 
2.17.1

[Qemu-devel] [PATCH 4/7] scripts/qemu.py: set predefined machine type based on arch

[Cleber Rosa]

Some targets require a machine type to be set, as there's no default
(aarch64 is one example).  To give a consistent interface to users of
this API, this changes set_machine() so that a predefined default can
be used, if one is not given.  The approach used is exactly the same
with the console device type.

Also, even when there's a default machine type, for some purposes,
testing included, it's better if outside code is explicit about the
machine type, instead of relying on whatever is set internally.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 scripts/qemu.py | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index d9e24a0c1a..fca9b76990 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = {
     r'^s390-ccw-virtio.*': 'sclpconsole',
     }
 
+#: Maps archictures to the preferred machine type
+MACHINE_TYPES = {
+    r'^aarch64$': 'virt',
+    r'^ppc$': 'g3beige',
+    r'^ppc64$': 'pseries',
+    r'^s390x$': 's390-ccw-virtio',
+    r'^x86_64$': 'q35',
+    }
+
 
 class QEMUMachineError(Exception):
     """
@@ -413,13 +422,24 @@ class QEMUMachine(object):
         """
         self._arch = arch
 
-    def set_machine(self, machine_type):
+    def set_machine(self, machine_type=None):
         '''
         Sets the machine type
 
         If set, the machine type will be added to the base arguments
         of the resulting QEMU command line.
         '''
+        if machine_type is None:
+            if self._arch is None:
+                raise QEMUMachineError("Can not set a default machine type: "
+                                       "QEMU instance without a defined arch")
+            for regex, machine in MACHINE_TYPES.items():
+                if re.match(regex, self._arch):
+                    machine_type = machine
+                    break
+            if machine_type is None:
+                raise QEMUMachineError("Can not set a machine type: no "
+                                       "matching machine type definition")
         self._machine = machine_type
 
     def set_console(self, device_type=None):
-- 
2.17.1

[Qemu-devel] [PATCH 5/7] Acceptance Tests: set machine type

[Cleber Rosa]

By setting the machine type, even if it's the one that will be picked
based on the arch, it's possible to run the same tests with targets
that require a machine type (in addition to those that don't).

Given that only boot_linux_console.py contains code specific to x86_64
(an explicit reference to the kernel image that will be used) the
other tests can be used to test different targets:

  $ avocado run -p arch=aarch64 --filter-by-tags='-x86_64' -- tests/acceptance/

Eventually, to reduce boiler plate code, the idea is to concentrate
the basic configuration (arch, machine, console) in either another
utility method, or make that happen by default.  This is of course the
subject of a future discussion.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/acceptance/boot_linux_console.py | 3 ++-
 tests/acceptance/version.py            | 2 ++
 tests/acceptance/vnc.py                | 5 +++++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
index 98324f7591..58032f971c 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -30,7 +30,8 @@ class BootLinuxConsole(Test):
         kernel_hash = '238e083e114c48200f80d889f7e32eeb2793e02a'
         kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
 
-        self.vm.set_machine('pc')
+        self.vm.set_arch(self.arch)
+        self.vm.set_machine()
         self.vm.set_console()
         kernel_command_line = 'console=ttyS0'
         self.vm.add_args('-kernel', kernel_path,
diff --git a/tests/acceptance/version.py b/tests/acceptance/version.py
index 13b0a7440d..7a3a20945f 100644
--- a/tests/acceptance/version.py
+++ b/tests/acceptance/version.py
@@ -18,6 +18,8 @@ class Version(Test):
     :avocado: tags=quick
     """
     def test_qmp_human_info_version(self):
+        self.vm.set_arch(self.arch)
+        self.vm.set_machine()
         self.vm.launch()
         res = self.vm.command('human-monitor-command',
                               command_line='info version')
diff --git a/tests/acceptance/vnc.py b/tests/acceptance/vnc.py
index b1ef9d71b1..4a8a83025f 100644
--- a/tests/acceptance/vnc.py
+++ b/tests/acceptance/vnc.py
@@ -16,6 +16,11 @@ class Vnc(Test):
     :avocado: enable
     :avocado: tags=vnc,quick
     """
+    def setUp(self):
+        super(Vnc, self).setUp()
+        self.vm.set_arch(self.arch)
+        self.vm.set_machine()
+
     def test_no_vnc(self):
         self.vm.add_args('-nodefaults', '-S')
         self.vm.launch()
-- 
2.17.1

[Qemu-devel] [PATCH 6/7] Acceptance Tests: add variants definition for architectures

[Cleber Rosa]

One of the Avocado features relevant to virtualization testing is the
ability to reuse tests in different scenarios, known as variants.
This adds a JSON based variants file, that can be used to run most
tests in a number of different architectures.  It can be run with:

   $ avocado run \
     --json-variants-load=tests/acceptance/variants/arch.json \
     --filter-by-tags='-x86_64' -- tests/acceptance/

Currently this covers 5 architectures, resulting in the execution
of 25 different combinations.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/acceptance/variants/arch.json | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 tests/acceptance/variants/arch.json

diff --git a/tests/acceptance/variants/arch.json b/tests/acceptance/variants/arch.json
new file mode 100644
index 0000000000..a7a2570553
--- /dev/null
+++ b/tests/acceptance/variants/arch.json
@@ -0,0 +1 @@
+[{"paths":["/run/*"],"variant":[["/run/aarch64",[["/run/aarch64", "arch", "aarch64"]]]],"variant_id": "aarch64"},{"paths":["/run/*"],"variant":[["/run/ppc",[["/run/ppc", "arch", "ppc"]]]],"variant_id": "ppc"},{"paths":["/run/*"],"variant":[["/run/ppc64",[["/run/ppc64", "arch", "ppc64"]]]],"variant_id": "ppc64"},{"paths":["/run/*"],"variant":[["/run/s390x",[["/run/s390x", "arch", "s390x"]]]],"variant_id": "s390x"},{"paths":["/run/*"],"variant":[["/run/x86_64",[["/run/x86_64", "arch", "x86_64"]]]],"variant_id": "x86_64"}]
-- 
2.17.1

[Qemu-devel] [PATCH 7/7] Acceptance Tests: change the handling of tests for specific archs

[Cleber Rosa]

With the introduction of a variants file that can run the same
tests on various architectures, it makes sense to make most tests
to be reusable on those environments.  The exception should be
when a test is really testing a specific architecture feature.

With the change proposed here, on a command line such as:

  $ avocado run \
     --json-variants-load=tests/acceptance/variants/arch.json \
     -- tests/acceptance/

The boot_linux_console.py tests will appear as "CANCELED: Currently
specific to the x86_64 arch", which is as a good thing when compared
to being ignored by tags because:

 * The architecture specific parts can be addressed
 * It will be run on the matching architecture (as opposed to always
   being filtered out by the tags mechanism)
 * CANCELED tests do no influence negatively the overall job results,
   they're not considered an error or failure

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

diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
index 58032f971c..2fb5da63cb 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -19,12 +19,13 @@ class BootLinuxConsole(Test):
     and the kernel command line is properly passed from QEMU to the kernel
 
     :avocado: enable
-    :avocado: tags=x86_64
     """
 
     timeout = 60
 
     def test(self):
+        if self.arch != 'x86_64':
+            self.cancel('Currently specific to the x86_64 arch')
         kernel_url = ('https://mirrors.kernel.org/fedora/releases/28/'
                       'Everything/x86_64/os/images/pxeboot/vmlinuz')
         kernel_hash = '238e083e114c48200f80d889f7e32eeb2793e02a'
-- 
2.17.1

Re: [Qemu-devel] [PATCH 7/7] Acceptance Tests: change the handling of tests for specific archs

[Philippe Mathieu-Daudé]

On 04/10/2018 17:14, Cleber Rosa wrote:
> With the introduction of a variants file that can run the same
> tests on various architectures, it makes sense to make most tests
> to be reusable on those environments.  The exception should be
> when a test is really testing a specific architecture feature.
> 
> With the change proposed here, on a command line such as:
> 
>   $ avocado run \
>      --json-variants-load=tests/acceptance/variants/arch.json \
>      -- tests/acceptance/
> 
> The boot_linux_console.py tests will appear as "CANCELED: Currently
> specific to the x86_64 arch", which is as a good thing when compared
> to being ignored by tags because:
> 
>  * The architecture specific parts can be addressed
>  * It will be run on the matching architecture (as opposed to always
>    being filtered out by the tags mechanism)
>  * CANCELED tests do no influence negatively the overall job results,
>    they're not considered an error or failure
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/acceptance/boot_linux_console.py | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
> index 58032f971c..2fb5da63cb 100644
> --- a/tests/acceptance/boot_linux_console.py
> +++ b/tests/acceptance/boot_linux_console.py
> @@ -19,12 +19,13 @@ class BootLinuxConsole(Test):
>      and the kernel command line is properly passed from QEMU to the kernel
>  
>      :avocado: enable
> -    :avocado: tags=x86_64
>      """
>  
>      timeout = 60
>  
>      def test(self):
> +        if self.arch != 'x86_64':
> +            self.cancel('Currently specific to the x86_64 arch')

Watch out the difference between the HOST arch and the TARGET arch.

This test is specific to the x86_64 TARGET arch (the one emulated),
_but_ it can be run on any HOST arch with TCG enabled:

$ uname -m
aarch64

$ avocado \
      --show=console \
    run \
      -p qemu_bin=x86_64-softmmu/qemu-system-x86_64 \
   tests/acceptance/boot_linux_console.py
JOB ID     : f6e023ac4ee0f999ab01bfb1b196f43fde8538b9

JOB LOG    :
/home/phil/avocado/job-results/job-2018-10-04T15.41-f6e023a/job.log
 (1/1) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test: /
console: [    0.000000] Linux version 4.16.3-301.fc28.x86_64
(mockbuild@bkernel02.phx2.fedoraproject.org) (gcc version 8.0.1 20180324
(Red Hat 8.0.1-0.20) (GCC)) #1 SMP Mon Apr 23 21:59:58 8
console: [    0.000000] Command line: console=ttyS0
console: [    0.000000] x86/fpu: x87 FPU will use FXSAVE
...
console: [    0.000000] NX (Execute Disable) protection: active
console: [    0.000000] random: fast init done
console: [    0.000000] SMBIOS 2.8 present.

console: [    0.000000] DMI: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.11.2-0-gf9626ccb91-prebuilt.qemu-project.org 04/01/2014
...
console: [    0.000000] Booting paravirtualized kernel on bare hardware
...
console: [    0.000000] Kernel command line: console=ttyS0
PASS (12.89 s)
RESULTS    : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 |
CANCEL 0
JOB TIME   : 13.12 s

>          kernel_url = ('https://mirrors.kernel.org/fedora/releases/28/'
>                        'Everything/x86_64/os/images/pxeboot/vmlinuz')
>          kernel_hash = '238e083e114c48200f80d889f7e32eeb2793e02a'
> 

Re: [Qemu-devel] [PATCH 7/7] Acceptance Tests: change the handling of tests for specific archs

[Cleber Rosa]

On 10/4/18 11:42 AM, Philippe Mathieu-Daudé wrote:
> On 04/10/2018 17:14, Cleber Rosa wrote:
>> With the introduction of a variants file that can run the same
>> tests on various architectures, it makes sense to make most tests
>> to be reusable on those environments.  The exception should be
>> when a test is really testing a specific architecture feature.
>>
>> With the change proposed here, on a command line such as:
>>
>>   $ avocado run \
>>      --json-variants-load=tests/acceptance/variants/arch.json \
>>      -- tests/acceptance/
>>
>> The boot_linux_console.py tests will appear as "CANCELED: Currently
>> specific to the x86_64 arch", which is as a good thing when compared
>> to being ignored by tags because:
>>
>>  * The architecture specific parts can be addressed
>>  * It will be run on the matching architecture (as opposed to always
>>    being filtered out by the tags mechanism)
>>  * CANCELED tests do no influence negatively the overall job results,
>>    they're not considered an error or failure
>>
>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>> ---
>>  tests/acceptance/boot_linux_console.py | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
>> index 58032f971c..2fb5da63cb 100644
>> --- a/tests/acceptance/boot_linux_console.py
>> +++ b/tests/acceptance/boot_linux_console.py
>> @@ -19,12 +19,13 @@ class BootLinuxConsole(Test):
>>      and the kernel command line is properly passed from QEMU to the kernel
>>  
>>      :avocado: enable
>> -    :avocado: tags=x86_64
>>      """
>>  
>>      timeout = 60
>>  
>>      def test(self):
>> +        if self.arch != 'x86_64':
>> +            self.cancel('Currently specific to the x86_64 arch')
> 
> Watch out the difference between the HOST arch and the TARGET arch.
> 
> This test is specific to the x86_64 TARGET arch (the one emulated),
> _but_ it can be run on any HOST arch with TCG enabled:
> 

Right, do you think we should be more explicit about the terminology
here and use "Currently specific to a x86_64 target arch" ?

> $ uname -m
> aarch64
> 
> $ avocado \
>       --show=console \
>     run \
>       -p qemu_bin=x86_64-softmmu/qemu-system-x86_64 \
>    tests/acceptance/boot_linux_console.py

The arch will only be based on `uname -m` if not given explicitly.  What
I mean is that on your aarch64 host, you should be able to do:

 $ avocado run -p arch=x86_64 ...

And that test should run the same way.  So yes, arch is about the
target, but it defaults to the same arch as the host.

Again, should we be more explicit about it?

Thanks!
- Cleber.

> JOB ID     : f6e023ac4ee0f999ab01bfb1b196f43fde8538b9
> 
> JOB LOG    :
> /home/phil/avocado/job-results/job-2018-10-04T15.41-f6e023a/job.log
>  (1/1) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test: /
> console: [    0.000000] Linux version 4.16.3-301.fc28.x86_64
> (mockbuild@bkernel02.phx2.fedoraproject.org) (gcc version 8.0.1 20180324
> (Red Hat 8.0.1-0.20) (GCC)) #1 SMP Mon Apr 23 21:59:58 8
> console: [    0.000000] Command line: console=ttyS0
> console: [    0.000000] x86/fpu: x87 FPU will use FXSAVE
> ...
> console: [    0.000000] NX (Execute Disable) protection: active
> console: [    0.000000] random: fast init done
> console: [    0.000000] SMBIOS 2.8 present.
> 
> console: [    0.000000] DMI: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS rel-1.11.2-0-gf9626ccb91-prebuilt.qemu-project.org 04/01/2014
> ...
> console: [    0.000000] Booting paravirtualized kernel on bare hardware
> ...
> console: [    0.000000] Kernel command line: console=ttyS0
> PASS (12.89 s)
> RESULTS    : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 |
> CANCEL 0
> JOB TIME   : 13.12 s
> 
>>          kernel_url = ('https://mirrors.kernel.org/fedora/releases/28/'
>>                        'Everything/x86_64/os/images/pxeboot/vmlinuz')
>>          kernel_hash = '238e083e114c48200f80d889f7e32eeb2793e02a'
>>

Re: [Qemu-devel] [PATCH 6/7] Acceptance Tests: add variants definition for architectures

[Laszlo Ersek]

On 10/04/18 17:14, Cleber Rosa wrote:
> One of the Avocado features relevant to virtualization testing is the
> ability to reuse tests in different scenarios, known as variants.
> This adds a JSON based variants file, that can be used to run most
> tests in a number of different architectures.  It can be run with:
> 
>    $ avocado run \
>      --json-variants-load=tests/acceptance/variants/arch.json \
>      --filter-by-tags='-x86_64' -- tests/acceptance/
> 
> Currently this covers 5 architectures, resulting in the execution
> of 25 different combinations.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/acceptance/variants/arch.json | 1 +
>  1 file changed, 1 insertion(+)
>  create mode 100644 tests/acceptance/variants/arch.json
> 
> diff --git a/tests/acceptance/variants/arch.json b/tests/acceptance/variants/arch.json
> new file mode 100644
> index 0000000000..a7a2570553
> --- /dev/null
> +++ b/tests/acceptance/variants/arch.json
> @@ -0,0 +1 @@
> +[{"paths":["/run/*"],"variant":[["/run/aarch64",[["/run/aarch64", "arch", "aarch64"]]]],"variant_id": "aarch64"},{"paths":["/run/*"],"variant":[["/run/ppc",[["/run/ppc", "arch", "ppc"]]]],"variant_id": "ppc"},{"paths":["/run/*"],"variant":[["/run/ppc64",[["/run/ppc64", "arch", "ppc64"]]]],"variant_id": "ppc64"},{"paths":["/run/*"],"variant":[["/run/s390x",[["/run/s390x", "arch", "s390x"]]]],"variant_id": "s390x"},{"paths":["/run/*"],"variant":[["/run/x86_64",[["/run/x86_64", "arch", "x86_64"]]]],"variant_id": "x86_64"}]
> 

(Side comment: you can parse json? :) That's awesome. Then I *am*
tempted to suggest that Philippe's work parse the firmware metadata
format, in the long run.)

Laszlo

Re: [Qemu-devel] [PATCH 2/7] Acceptance Tests: introduce arch parameter and attribute

[Murilo Opsfelder Araujo]

Hi, Cleber.

On Thu, Oct 04, 2018 at 11:14:24AM -0400, Cleber Rosa wrote:
> On a number of different scenarios, such as when choosing a QEMU
> binary to be used on tests (or a image to use to boot a test VM), it's
> useful to define the architecture that should be used.
>
> This introduces both a test parameter and a test instance attribute,
> that will contain such a value.
>
> The selection of the default QEMU binary, if one is not given
> explicitly, has also been changed to look at the arch attribute.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  docs/devel/testing.rst                    | 18 ++++++++++++++++++
>  tests/acceptance/avocado_qemu/__init__.py | 13 ++++++++++---
>  2 files changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
> index 727c4019b5..4178ae5022 100644
> --- a/docs/devel/testing.rst
> +++ b/docs/devel/testing.rst
> @@ -659,6 +659,17 @@ vm
>  A QEMUMachine instance, initially configured according to the given
>  ``qemu_bin`` parameter.
>
> +arch
> +~~~~
> +
> +The architecture that will be used on a number of different
> +scenarios.  For instance, when a QEMU binary is not explicitly given,
> +the one selected will depend on this attribute.
> +
> +The ``arch`` attribute will be set to the test parameter of the same
> +name, and if one is not given explicitly, it will default to the
> +system architecture (as given by ``uname``).
> +
>  qemu_bin
>  ~~~~~~~~
>
> @@ -681,6 +692,13 @@ like the following:
>
>    PARAMS (key=qemu_bin, path=*, default=x86_64-softmmu/qemu-system-x86_64) => 'x86_64-softmmu/qemu-system-x86_64
>
> +arch
> +~~~~
> +
> +The architecture that will be used on a number of different scenarios.
> +This parameter has a direct relation with the ``arch`` attribute.  If
> +not given, it will default to None.
> +
>  qemu_bin
>  ~~~~~~~~
>
> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> index d8d5b48dac..9329d9b9ec 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -23,16 +23,22 @@ def is_readable_executable_file(path):
>      return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK)
>
>
> -def pick_default_qemu_bin():
> +def pick_default_qemu_bin(arch=None):
>      """
>      Picks the path of a QEMU binary, starting either in the current working
>      directory or in the source tree root directory.
>
> +    :param arch: the arch to use when looking for a QEMU binary (the target
> +                 will match the arch given).  If None (the default) arch
> +                 will be the current host system arch (as given by
> +                 :func:`os.uname`).
> +    :param arch: str

Do you mean

    :type arch: str

?

>      :returns: the path to the default QEMU binary or None if one could not
>                be found
>      :rtype: str or None
>      """
> -    arch = os.uname()[4]
> +    if arch is None:
> +        arch = os.uname()[4]
>      qemu_bin_relative_path = os.path.join("%s-softmmu" % arch,
>                                            "qemu-system-%s" % arch)

On ppc64le systems, this can lead to an unexisting path.

For instance, os.uname() returns:

    >>> os.uname()
    ('Linux', 'localhost', '4.15.0-34-generic', '#37-Ubuntu SMP Mon Aug 27 15:18:58 UTC 2018', 'ppc64le')

Then, qemu_bin_relative_path would be set to:

    ppc64le-softmmu/qemu-system-ppc64le

However, the correct path is:

    ppc64-softmmu/qemu-system-ppc64

I'm not sure if ppc64le is the only case where the target name is different from
the OS architecture.

If you decide not to handle this now, at least add a FIXME, if possible.

>      if is_readable_executable_file(qemu_bin_relative_path):
> @@ -47,8 +53,9 @@ def pick_default_qemu_bin():
>  class Test(avocado.Test):
>      def setUp(self):
>          self.vm = None
> +        self.arch = self.params.get('arch', default=os.uname()[4])
>          self.qemu_bin = self.params.get('qemu_bin',
> -                                        default=pick_default_qemu_bin())
> +                                        default=pick_default_qemu_bin(self.arch))
>          if self.qemu_bin is None:
>              self.cancel("No QEMU binary defined or found in the source tree")
>          self.vm = QEMUMachine(self.qemu_bin)
> --
> 2.17.1
>
>

--
Murilo

Re: [Qemu-devel] [PATCH 1/7] Acceptance Tests: improve docstring on pick_default_qemu_bin()

[Philippe Mathieu-Daudé]

On 04/10/2018 17:14, Cleber Rosa wrote:
> Making it clear what is returned by this utility function.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>

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

> ---
>  tests/acceptance/avocado_qemu/__init__.py | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> index 1e54fd5932..d8d5b48dac 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -27,6 +27,10 @@ def pick_default_qemu_bin():
>      """
>      Picks the path of a QEMU binary, starting either in the current working
>      directory or in the source tree root directory.
> +
> +    :returns: the path to the default QEMU binary or None if one could not
> +              be found
> +    :rtype: str or None
>      """
>      arch = os.uname()[4]
>      qemu_bin_relative_path = os.path.join("%s-softmmu" % arch,
> 

Re: [Qemu-devel] [PATCH 3/7] scripts/qemu.py: add method and private attribute for arch

[Philippe Mathieu-Daudé]

On 04/10/2018 17:14, Cleber Rosa wrote:
> Because some sane defaults may require the knowledge of the arch,
> let's give the QEMUMachine the opportunity to hold that information.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>

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

> ---
>  scripts/qemu.py | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index f099ce7278..d9e24a0c1a 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -113,6 +113,7 @@ class QEMUMachine(object):
>          self._test_dir = test_dir
>          self._temp_dir = None
>          self._launched = False
> +        self._arch = None
>          self._machine = None
>          self._console_device_type = None
>          self._console_address = None
> @@ -406,6 +407,12 @@ class QEMUMachine(object):
>          '''
>          self._args.extend(args)
>  
> +    def set_arch(self, arch):
> +        """
> +        Sets the architecture of this machine
> +        """
> +        self._arch = arch
> +
>      def set_machine(self, machine_type):
>          '''
>          Sets the machine type
> 

Re: [Qemu-devel] [PATCH 5/7] Acceptance Tests: set machine type

[Philippe Mathieu-Daudé]

On 04/10/2018 17:14, Cleber Rosa wrote:
> By setting the machine type, even if it's the one that will be picked
> based on the arch, it's possible to run the same tests with targets
> that require a machine type (in addition to those that don't).
> 
> Given that only boot_linux_console.py contains code specific to x86_64
> (an explicit reference to the kernel image that will be used) the
> other tests can be used to test different targets:
> 
>   $ avocado run -p arch=aarch64 --filter-by-tags='-x86_64' -- tests/acceptance/
> 
> Eventually, to reduce boiler plate code, the idea is to concentrate
> the basic configuration (arch, machine, console) in either another
> utility method, or make that happen by default.  This is of course the
> subject of a future discussion.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/acceptance/boot_linux_console.py | 3 ++-
>  tests/acceptance/version.py            | 2 ++
>  tests/acceptance/vnc.py                | 5 +++++
>  3 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
> index 98324f7591..58032f971c 100644
> --- a/tests/acceptance/boot_linux_console.py
> +++ b/tests/acceptance/boot_linux_console.py
> @@ -30,7 +30,8 @@ class BootLinuxConsole(Test):
>          kernel_hash = '238e083e114c48200f80d889f7e32eeb2793e02a'
>          kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
>  
> -        self.vm.set_machine('pc')
> +        self.vm.set_arch(self.arch)
> +        self.vm.set_machine()
>          self.vm.set_console()
>          kernel_command_line = 'console=ttyS0'
>          self.vm.add_args('-kernel', kernel_path,
> diff --git a/tests/acceptance/version.py b/tests/acceptance/version.py
> index 13b0a7440d..7a3a20945f 100644
> --- a/tests/acceptance/version.py
> +++ b/tests/acceptance/version.py
> @@ -18,6 +18,8 @@ class Version(Test):
>      :avocado: tags=quick
>      """
>      def test_qmp_human_info_version(self):
> +        self.vm.set_arch(self.arch)
> +        self.vm.set_machine()
>          self.vm.launch()
>          res = self.vm.command('human-monitor-command',
>                                command_line='info version')
> diff --git a/tests/acceptance/vnc.py b/tests/acceptance/vnc.py
> index b1ef9d71b1..4a8a83025f 100644
> --- a/tests/acceptance/vnc.py
> +++ b/tests/acceptance/vnc.py
> @@ -16,6 +16,11 @@ class Vnc(Test):
>      :avocado: enable
>      :avocado: tags=vnc,quick
>      """
> +    def setUp(self):
> +        super(Vnc, self).setUp()
> +        self.vm.set_arch(self.arch)
> +        self.vm.set_machine()
> +
>      def test_no_vnc(self):
>          self.vm.add_args('-nodefaults', '-S')
>          self.vm.launch()
> 

$ uname -m
aarch64
$ avocado --show=app run
--json-variants-load=tests/acceptance/variants/arch.json
tests/acceptance/vnc.py
JOB ID     : 06f38999b7386afbd66023e7c1daee84d7991060
JOB LOG    :
/home/phil/avocado/job-results/job-2018-10-05T15.40-06f3899/job.log
 (01/20) tests/acceptance/vnc.py:Vnc.test_no_vnc;aarch64: PASS (0.34 s)
 (02/20) tests/acceptance/vnc.py:Vnc.test_no_vnc;ppc: PASS (0.31 s)
 (03/20) tests/acceptance/vnc.py:Vnc.test_no_vnc;ppc64: PASS (0.35 s)
 (04/20) tests/acceptance/vnc.py:Vnc.test_no_vnc;s390x: PASS (0.24 s)
 (05/20) tests/acceptance/vnc.py:Vnc.test_no_vnc;x86_64: PASS (0.34 s)
 (06/20)
tests/acceptance/vnc.py:Vnc.test_no_vnc_change_password;aarch64: PASS
(0.34 s)
 (07/20) tests/acceptance/vnc.py:Vnc.test_no_vnc_change_password;ppc:
PASS (0.27 s)
 (08/20) tests/acceptance/vnc.py:Vnc.test_no_vnc_change_password;ppc64:
PASS (0.34 s)
 (09/20) tests/acceptance/vnc.py:Vnc.test_no_vnc_change_password;s390x:
PASS (0.24 s)
 (10/20) tests/acceptance/vnc.py:Vnc.test_no_vnc_change_password;x86_64:
PASS (0.35 s)
 (11/20)
tests/acceptance/vnc.py:Vnc.test_vnc_change_password_requires_a_password;aarch64:
PASS (0.33 s)
 (12/20)
tests/acceptance/vnc.py:Vnc.test_vnc_change_password_requires_a_password;ppc:
PASS (0.28 s)
 (13/20)
tests/acceptance/vnc.py:Vnc.test_vnc_change_password_requires_a_password;ppc64:
PASS (0.38 s)
 (14/20)
tests/acceptance/vnc.py:Vnc.test_vnc_change_password_requires_a_password;s390x:
PASS (0.26 s)
 (15/20)
tests/acceptance/vnc.py:Vnc.test_vnc_change_password_requires_a_password;x86_64:
PASS (0.35 s)
 (16/20) tests/acceptance/vnc.py:Vnc.test_vnc_change_password;aarch64:
PASS (0.35 s)
 (17/20) tests/acceptance/vnc.py:Vnc.test_vnc_change_password;ppc: PASS
(0.28 s)
 (18/20) tests/acceptance/vnc.py:Vnc.test_vnc_change_password;ppc64:
PASS (0.36 s)
 (19/20) tests/acceptance/vnc.py:Vnc.test_vnc_change_password;s390x:
PASS (0.26 s)
 (20/20) tests/acceptance/vnc.py:Vnc.test_vnc_change_password;x86_64:
PASS (0.34 s)
RESULTS    : PASS 20 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0
| CANCEL 0
JOB TIME   : 8.30 s

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

Re: [Qemu-devel] [PATCH 6/7] Acceptance Tests: add variants definition for architectures

[Philippe Mathieu-Daudé]

Hi Cleber,

On 04/10/2018 17:14, Cleber Rosa wrote:
> One of the Avocado features relevant to virtualization testing is the
> ability to reuse tests in different scenarios, known as variants.
> This adds a JSON based variants file, that can be used to run most
> tests in a number of different architectures.  It can be run with:
> 
>    $ avocado run \
>      --json-variants-load=tests/acceptance/variants/arch.json \
>      --filter-by-tags='-x86_64' -- tests/acceptance/
> 
> Currently this covers 5 architectures, resulting in the execution
> of 25 different combinations.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/acceptance/variants/arch.json | 1 +
>  1 file changed, 1 insertion(+)
>  create mode 100644 tests/acceptance/variants/arch.json
> 
> diff --git a/tests/acceptance/variants/arch.json b/tests/acceptance/variants/arch.json
> new file mode 100644
> index 0000000000..a7a2570553
> --- /dev/null
> +++ b/tests/acceptance/variants/arch.json
> @@ -0,0 +1 @@
> +[{"paths":["/run/*"],"variant":[["/run/aarch64",[["/run/aarch64", "arch", "aarch64"]]]],"variant_id": "aarch64"},{"paths":["/run/*"],"variant":[["/run/ppc",[["/run/ppc", "arch", "ppc"]]]],"variant_id": "ppc"},{"paths":["/run/*"],"variant":[["/run/ppc64",[["/run/ppc64", "arch", "ppc64"]]]],"variant_id": "ppc64"},{"paths":["/run/*"],"variant":[["/run/s390x",[["/run/s390x", "arch", "s390x"]]]],"variant_id": "s390x"},{"paths":["/run/*"],"variant":[["/run/x86_64",[["/run/x86_64", "arch", "x86_64"]]]],"variant_id": "x86_64"}]
> 

Is this generated? (thinking about the other archs supported).

You should use some linter ;)

Re: [Qemu-devel] [PATCH 6/7] Acceptance Tests: add variants definition for architectures

[Eric Blake]

On 10/5/18 11:24 AM, Philippe Mathieu-Daudé wrote:
> Hi Cleber,
> 
> On 04/10/2018 17:14, Cleber Rosa wrote:
>> One of the Avocado features relevant to virtualization testing is the
>> ability to reuse tests in different scenarios, known as variants.
>> This adds a JSON based variants file, that can be used to run most
>> tests in a number of different architectures.  It can be run with:
>>
>>     $ avocado run \
>>       --json-variants-load=tests/acceptance/variants/arch.json \
>>       --filter-by-tags='-x86_64' -- tests/acceptance/

>> +++ b/tests/acceptance/variants/arch.json
>> @@ -0,0 +1 @@
>> +[{"paths":["/run/*"],"variant":[["/run/aarch64",[["/run/aarch64", "arch", "aarch64"]]]],"variant_id": "aarch64"},{"paths":["/run/*"],"variant":[["/run/ppc",[["/run/ppc", "arch", "ppc"]]]],"variant_id": "ppc"},{"paths":["/run/*"],"variant":[["/run/ppc64",[["/run/ppc64", "arch", "ppc64"]]]],"variant_id": "ppc64"},{"paths":["/run/*"],"variant":[["/run/s390x",[["/run/s390x", "arch", "s390x"]]]],"variant_id": "s390x"},{"paths":["/run/*"],"variant":[["/run/x86_64",[["/run/x86_64", "arch", "x86_64"]]]],"variant_id": "x86_64"}]
>>
> 
> Is this generated? (thinking about the other archs supported).
> 
> You should use some linter ;)

Also, that's a long line, which will probably get longer as more support 
is added. Beyond 990 bytes, it starts risking problems with corruption 
over email. It's also hard to view what changes incrementally if the 
single line changes. Is there a way to pretty-print things across 
multiple lines, for shorter lines and easier reading of future diffs?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 6/7] Acceptance Tests: add variants definition for architectures

[Cleber Rosa]

On 10/5/18 12:32 PM, Eric Blake wrote:
> On 10/5/18 11:24 AM, Philippe Mathieu-Daudé wrote:
>> Hi Cleber,
>>
>> On 04/10/2018 17:14, Cleber Rosa wrote:
>>> One of the Avocado features relevant to virtualization testing is the
>>> ability to reuse tests in different scenarios, known as variants.
>>> This adds a JSON based variants file, that can be used to run most
>>> tests in a number of different architectures.  It can be run with:
>>>
>>>     $ avocado run \
>>>       --json-variants-load=tests/acceptance/variants/arch.json \
>>>       --filter-by-tags='-x86_64' -- tests/acceptance/
> 
>>> +++ b/tests/acceptance/variants/arch.json
>>> @@ -0,0 +1 @@
>>> +[{"paths":["/run/*"],"variant":[["/run/aarch64",[["/run/aarch64",
>>> "arch", "aarch64"]]]],"variant_id":
>>> "aarch64"},{"paths":["/run/*"],"variant":[["/run/ppc",[["/run/ppc",
>>> "arch", "ppc"]]]],"variant_id":
>>> "ppc"},{"paths":["/run/*"],"variant":[["/run/ppc64",[["/run/ppc64",
>>> "arch", "ppc64"]]]],"variant_id":
>>> "ppc64"},{"paths":["/run/*"],"variant":[["/run/s390x",[["/run/s390x",
>>> "arch", "s390x"]]]],"variant_id":
>>> "s390x"},{"paths":["/run/*"],"variant":[["/run/x86_64",[["/run/x86_64",
>>> "arch", "x86_64"]]]],"variant_id": "x86_64"}]
>>>
>>
>> Is this generated? (thinking about the other archs supported).
>>

It's generated and kept on every job result (jobdata/variants.json).
Basically, you'd use any varianter plugin on a job, and then you can
reuse the JSON generated on other jobs.

TBH, I tweaked this one a bit.

>> You should use some linter ;)

I missed your point here... do you mean the style is not ideal?

> 
> Also, that's a long line, which will probably get longer as more support
> is added. Beyond 990 bytes, it starts risking problems with corruption
> over email. It's also hard to view what changes incrementally if the
> single line changes. Is there a way to pretty-print things across
> multiple lines, for shorter lines and easier reading of future diffs?
> 

Yes, good point.  I'll pretty print it.

Just a disclaimer: I've chosen to use a JSON variants because it's a
core Avocado feature (doesn't require any external plugin), and the
results are 100% reproducible (the variants are static).  In the future,
we may consider also shipping (and depending) on other variants.

One idea that is being maturing (and prototype) is a native QEMU
varianter.  There's some info here:

https://trello.com/c/qW4kMw50/32-guest-abi-machine-type-cpu-model-test-cases

Regards,
- Cleber.

Re: [Qemu-devel] [PATCH 6/7] Acceptance Tests: add variants definition for architectures

[Philippe Mathieu-Daudé]

On 05/10/2018 19:07, Cleber Rosa wrote:
> On 10/5/18 12:32 PM, Eric Blake wrote:
>> On 10/5/18 11:24 AM, Philippe Mathieu-Daudé wrote:
>>> Hi Cleber,
>>>
>>> On 04/10/2018 17:14, Cleber Rosa wrote:
>>>> One of the Avocado features relevant to virtualization testing is the
>>>> ability to reuse tests in different scenarios, known as variants.
>>>> This adds a JSON based variants file, that can be used to run most
>>>> tests in a number of different architectures.  It can be run with:
>>>>
>>>>     $ avocado run \
>>>>       --json-variants-load=tests/acceptance/variants/arch.json \
>>>>       --filter-by-tags='-x86_64' -- tests/acceptance/
>>
>>>> +++ b/tests/acceptance/variants/arch.json
>>>> @@ -0,0 +1 @@
>>>> +[{"paths":["/run/*"],"variant":[["/run/aarch64",[["/run/aarch64",
>>>> "arch", "aarch64"]]]],"variant_id":
>>>> "aarch64"},{"paths":["/run/*"],"variant":[["/run/ppc",[["/run/ppc",
>>>> "arch", "ppc"]]]],"variant_id":
>>>> "ppc"},{"paths":["/run/*"],"variant":[["/run/ppc64",[["/run/ppc64",
>>>> "arch", "ppc64"]]]],"variant_id":
>>>> "ppc64"},{"paths":["/run/*"],"variant":[["/run/s390x",[["/run/s390x",
>>>> "arch", "s390x"]]]],"variant_id":
>>>> "s390x"},{"paths":["/run/*"],"variant":[["/run/x86_64",[["/run/x86_64",
>>>> "arch", "x86_64"]]]],"variant_id": "x86_64"}]
>>>>
>>>
>>> Is this generated? (thinking about the other archs supported).
>>>
> 
> It's generated and kept on every job result (jobdata/variants.json).
> Basically, you'd use any varianter plugin on a job, and then you can
> reuse the JSON generated on other jobs.
> 
> TBH, I tweaked this one a bit.
> 
>>> You should use some linter ;)
> 
> I missed your point here... do you mean the style is not ideal?

As meant "pretty printer" :)

[
  {
    "paths": [
      "/run/*"
    ],
    "variant": [
      [
        "/run/aarch64",
        [
          [
            "/run/aarch64",
            "arch",
            "aarch64"
          ]
        ]
      ]
    ],
    "variant_id": "aarch64"
  },
  ...

But since it is generated I'd rather generate it...

>>
>> Also, that's a long line, which will probably get longer as more support
>> is added. Beyond 990 bytes, it starts risking problems with corruption
>> over email. It's also hard to view what changes incrementally if the
>> single line changes. Is there a way to pretty-print things across
>> multiple lines, for shorter lines and easier reading of future diffs?
>>
> 
> Yes, good point.  I'll pretty print it.
> 
> Just a disclaimer: I've chosen to use a JSON variants because it's a
> core Avocado feature (doesn't require any external plugin), and the
> results are 100% reproducible (the variants are static).  In the future,
> we may consider also shipping (and depending) on other variants.
> 
> One idea that is being maturing (and prototype) is a native QEMU
> varianter.  There's some info here:
> 
> https://trello.com/c/qW4kMw50/32-guest-abi-machine-type-cpu-model-test-cases
> 
> Regards,
> - Cleber.
> 

Re: [Qemu-devel] [PATCH 6/7] Acceptance Tests: add variants definition for architectures

[Cleber Rosa]

On 10/5/18 1:30 PM, Philippe Mathieu-Daudé wrote:
> On 05/10/2018 19:07, Cleber Rosa wrote:
>> On 10/5/18 12:32 PM, Eric Blake wrote:
>>> On 10/5/18 11:24 AM, Philippe Mathieu-Daudé wrote:
>>>> Hi Cleber,
>>>>
>>>> On 04/10/2018 17:14, Cleber Rosa wrote:
>>>>> One of the Avocado features relevant to virtualization testing is the
>>>>> ability to reuse tests in different scenarios, known as variants.
>>>>> This adds a JSON based variants file, that can be used to run most
>>>>> tests in a number of different architectures.  It can be run with:
>>>>>
>>>>>     $ avocado run \
>>>>>       --json-variants-load=tests/acceptance/variants/arch.json \
>>>>>       --filter-by-tags='-x86_64' -- tests/acceptance/
>>>
>>>>> +++ b/tests/acceptance/variants/arch.json
>>>>> @@ -0,0 +1 @@
>>>>> +[{"paths":["/run/*"],"variant":[["/run/aarch64",[["/run/aarch64",
>>>>> "arch", "aarch64"]]]],"variant_id":
>>>>> "aarch64"},{"paths":["/run/*"],"variant":[["/run/ppc",[["/run/ppc",
>>>>> "arch", "ppc"]]]],"variant_id":
>>>>> "ppc"},{"paths":["/run/*"],"variant":[["/run/ppc64",[["/run/ppc64",
>>>>> "arch", "ppc64"]]]],"variant_id":
>>>>> "ppc64"},{"paths":["/run/*"],"variant":[["/run/s390x",[["/run/s390x",
>>>>> "arch", "s390x"]]]],"variant_id":
>>>>> "s390x"},{"paths":["/run/*"],"variant":[["/run/x86_64",[["/run/x86_64",
>>>>> "arch", "x86_64"]]]],"variant_id": "x86_64"}]
>>>>>
>>>>
>>>> Is this generated? (thinking about the other archs supported).
>>>>
>>
>> It's generated and kept on every job result (jobdata/variants.json).
>> Basically, you'd use any varianter plugin on a job, and then you can
>> reuse the JSON generated on other jobs.
>>
>> TBH, I tweaked this one a bit.
>>
>>>> You should use some linter ;)
>>
>> I missed your point here... do you mean the style is not ideal?
> 
> As meant "pretty printer" :)
> 
> [
>   {
>     "paths": [
>       "/run/*"
>     ],
>     "variant": [
>       [
>         "/run/aarch64",
>         [
>           [
>             "/run/aarch64",
>             "arch",
>             "aarch64"
>           ]
>         ]
>       ]
>     ],
>     "variant_id": "aarch64"
>   },
>   ...
> 
> But since it is generated I'd rather generate it...

Actually, I think it's a good idea indeed to pretty print it, "python -m
json.tool" should do the job.

Thanks,
- Cleber.

> 
>>>
>>> Also, that's a long line, which will probably get longer as more support
>>> is added. Beyond 990 bytes, it starts risking problems with corruption
>>> over email. It's also hard to view what changes incrementally if the
>>> single line changes. Is there a way to pretty-print things across
>>> multiple lines, for shorter lines and easier reading of future diffs?
>>>
>>
>> Yes, good point.  I'll pretty print it.
>>
>> Just a disclaimer: I've chosen to use a JSON variants because it's a
>> core Avocado feature (doesn't require any external plugin), and the
>> results are 100% reproducible (the variants are static).  In the future,
>> we may consider also shipping (and depending) on other variants.
>>
>> One idea that is being maturing (and prototype) is a native QEMU
>> varianter.  There's some info here:
>>
>> https://trello.com/c/qW4kMw50/32-guest-abi-machine-type-cpu-model-test-cases
>>
>> Regards,
>> - Cleber.
>>

Re: [Qemu-devel] [PATCH 5/7] Acceptance Tests: set machine type

[Cleber Rosa]

On 10/5/18 11:42 AM, Philippe Mathieu-Daudé wrote:
> On 04/10/2018 17:14, Cleber Rosa wrote:
>> By setting the machine type, even if it's the one that will be picked
>> based on the arch, it's possible to run the same tests with targets
>> that require a machine type (in addition to those that don't).
>>
>> Given that only boot_linux_console.py contains code specific to x86_64
>> (an explicit reference to the kernel image that will be used) the
>> other tests can be used to test different targets:
>>
>>   $ avocado run -p arch=aarch64 --filter-by-tags='-x86_64' -- tests/acceptance/
>>
>> Eventually, to reduce boiler plate code, the idea is to concentrate
>> the basic configuration (arch, machine, console) in either another
>> utility method, or make that happen by default.  This is of course the
>> subject of a future discussion.
>>
>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>> ---
>>  tests/acceptance/boot_linux_console.py | 3 ++-
>>  tests/acceptance/version.py            | 2 ++
>>  tests/acceptance/vnc.py                | 5 +++++
>>  3 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
>> index 98324f7591..58032f971c 100644
>> --- a/tests/acceptance/boot_linux_console.py
>> +++ b/tests/acceptance/boot_linux_console.py
>> @@ -30,7 +30,8 @@ class BootLinuxConsole(Test):
>>          kernel_hash = '238e083e114c48200f80d889f7e32eeb2793e02a'
>>          kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
>>  
>> -        self.vm.set_machine('pc')
>> +        self.vm.set_arch(self.arch)
>> +        self.vm.set_machine()
>>          self.vm.set_console()
>>          kernel_command_line = 'console=ttyS0'
>>          self.vm.add_args('-kernel', kernel_path,
>> diff --git a/tests/acceptance/version.py b/tests/acceptance/version.py
>> index 13b0a7440d..7a3a20945f 100644
>> --- a/tests/acceptance/version.py
>> +++ b/tests/acceptance/version.py
>> @@ -18,6 +18,8 @@ class Version(Test):
>>      :avocado: tags=quick
>>      """
>>      def test_qmp_human_info_version(self):
>> +        self.vm.set_arch(self.arch)
>> +        self.vm.set_machine()
>>          self.vm.launch()
>>          res = self.vm.command('human-monitor-command',
>>                                command_line='info version')
>> diff --git a/tests/acceptance/vnc.py b/tests/acceptance/vnc.py
>> index b1ef9d71b1..4a8a83025f 100644
>> --- a/tests/acceptance/vnc.py
>> +++ b/tests/acceptance/vnc.py
>> @@ -16,6 +16,11 @@ class Vnc(Test):
>>      :avocado: enable
>>      :avocado: tags=vnc,quick
>>      """
>> +    def setUp(self):
>> +        super(Vnc, self).setUp()
>> +        self.vm.set_arch(self.arch)
>> +        self.vm.set_machine()
>> +
>>      def test_no_vnc(self):
>>          self.vm.add_args('-nodefaults', '-S')
>>          self.vm.launch()
>>
> 
> $ uname -m
> aarch64
> $ avocado --show=app run
> --json-variants-load=tests/acceptance/variants/arch.json
> tests/acceptance/vnc.py
> JOB ID     : 06f38999b7386afbd66023e7c1daee84d7991060
> JOB LOG    :
> /home/phil/avocado/job-results/job-2018-10-05T15.40-06f3899/job.log
>  (01/20) tests/acceptance/vnc.py:Vnc.test_no_vnc;aarch64: PASS (0.34 s)
>  (02/20) tests/acceptance/vnc.py:Vnc.test_no_vnc;ppc: PASS (0.31 s)
>  (03/20) tests/acceptance/vnc.py:Vnc.test_no_vnc;ppc64: PASS (0.35 s)
>  (04/20) tests/acceptance/vnc.py:Vnc.test_no_vnc;s390x: PASS (0.24 s)
>  (05/20) tests/acceptance/vnc.py:Vnc.test_no_vnc;x86_64: PASS (0.34 s)
>  (06/20)
> tests/acceptance/vnc.py:Vnc.test_no_vnc_change_password;aarch64: PASS
> (0.34 s)
>  (07/20) tests/acceptance/vnc.py:Vnc.test_no_vnc_change_password;ppc:
> PASS (0.27 s)
>  (08/20) tests/acceptance/vnc.py:Vnc.test_no_vnc_change_password;ppc64:
> PASS (0.34 s)
>  (09/20) tests/acceptance/vnc.py:Vnc.test_no_vnc_change_password;s390x:
> PASS (0.24 s)
>  (10/20) tests/acceptance/vnc.py:Vnc.test_no_vnc_change_password;x86_64:
> PASS (0.35 s)
>  (11/20)
> tests/acceptance/vnc.py:Vnc.test_vnc_change_password_requires_a_password;aarch64:
> PASS (0.33 s)
>  (12/20)
> tests/acceptance/vnc.py:Vnc.test_vnc_change_password_requires_a_password;ppc:
> PASS (0.28 s)
>  (13/20)
> tests/acceptance/vnc.py:Vnc.test_vnc_change_password_requires_a_password;ppc64:
> PASS (0.38 s)
>  (14/20)
> tests/acceptance/vnc.py:Vnc.test_vnc_change_password_requires_a_password;s390x:
> PASS (0.26 s)
>  (15/20)
> tests/acceptance/vnc.py:Vnc.test_vnc_change_password_requires_a_password;x86_64:
> PASS (0.35 s)
>  (16/20) tests/acceptance/vnc.py:Vnc.test_vnc_change_password;aarch64:
> PASS (0.35 s)
>  (17/20) tests/acceptance/vnc.py:Vnc.test_vnc_change_password;ppc: PASS
> (0.28 s)
>  (18/20) tests/acceptance/vnc.py:Vnc.test_vnc_change_password;ppc64:
> PASS (0.36 s)
>  (19/20) tests/acceptance/vnc.py:Vnc.test_vnc_change_password;s390x:
> PASS (0.26 s)
>  (20/20) tests/acceptance/vnc.py:Vnc.test_vnc_change_password;x86_64:
> PASS (0.34 s)
> RESULTS    : PASS 20 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0
> | CANCEL 0
> JOB TIME   : 8.30 s
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 

I think this is a "tested by" for the next commit (the one that adds the
JSON file), right?

Thanks for testing it!
- Cleber.

Re: [Qemu-devel] [PATCH 2/7] Acceptance Tests: introduce arch parameter and attribute

[Cleber Rosa]

On 10/4/18 7:56 PM, Murilo Opsfelder Araujo wrote:
> Hi, Cleber.
> 
> On Thu, Oct 04, 2018 at 11:14:24AM -0400, Cleber Rosa wrote:
>> On a number of different scenarios, such as when choosing a QEMU
>> binary to be used on tests (or a image to use to boot a test VM), it's
>> useful to define the architecture that should be used.
>>
>> This introduces both a test parameter and a test instance attribute,
>> that will contain such a value.
>>
>> The selection of the default QEMU binary, if one is not given
>> explicitly, has also been changed to look at the arch attribute.
>>
>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>> ---
>>  docs/devel/testing.rst                    | 18 ++++++++++++++++++
>>  tests/acceptance/avocado_qemu/__init__.py | 13 ++++++++++---
>>  2 files changed, 28 insertions(+), 3 deletions(-)
>>
>> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
>> index 727c4019b5..4178ae5022 100644
>> --- a/docs/devel/testing.rst
>> +++ b/docs/devel/testing.rst
>> @@ -659,6 +659,17 @@ vm
>>  A QEMUMachine instance, initially configured according to the given
>>  ``qemu_bin`` parameter.
>>
>> +arch
>> +~~~~
>> +
>> +The architecture that will be used on a number of different
>> +scenarios.  For instance, when a QEMU binary is not explicitly given,
>> +the one selected will depend on this attribute.
>> +
>> +The ``arch`` attribute will be set to the test parameter of the same
>> +name, and if one is not given explicitly, it will default to the
>> +system architecture (as given by ``uname``).
>> +
>>  qemu_bin
>>  ~~~~~~~~
>>
>> @@ -681,6 +692,13 @@ like the following:
>>
>>    PARAMS (key=qemu_bin, path=*, default=x86_64-softmmu/qemu-system-x86_64) => 'x86_64-softmmu/qemu-system-x86_64
>>
>> +arch
>> +~~~~
>> +
>> +The architecture that will be used on a number of different scenarios.
>> +This parameter has a direct relation with the ``arch`` attribute.  If
>> +not given, it will default to None.
>> +
>>  qemu_bin
>>  ~~~~~~~~
>>
>> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
>> index d8d5b48dac..9329d9b9ec 100644
>> --- a/tests/acceptance/avocado_qemu/__init__.py
>> +++ b/tests/acceptance/avocado_qemu/__init__.py
>> @@ -23,16 +23,22 @@ def is_readable_executable_file(path):
>>      return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK)
>>
>>
>> -def pick_default_qemu_bin():
>> +def pick_default_qemu_bin(arch=None):
>>      """
>>      Picks the path of a QEMU binary, starting either in the current working
>>      directory or in the source tree root directory.
>>
>> +    :param arch: the arch to use when looking for a QEMU binary (the target
>> +                 will match the arch given).  If None (the default) arch
>> +                 will be the current host system arch (as given by
>> +                 :func:`os.uname`).
>> +    :param arch: str
> 
> Do you mean
> 
>     :type arch: str
> 
> ?
> 

Hi Murilo,

Yes, thanks for catching that.

>>      :returns: the path to the default QEMU binary or None if one could not
>>                be found
>>      :rtype: str or None
>>      """
>> -    arch = os.uname()[4]
>> +    if arch is None:
>> +        arch = os.uname()[4]
>>      qemu_bin_relative_path = os.path.join("%s-softmmu" % arch,
>>                                            "qemu-system-%s" % arch)
> 
> On ppc64le systems, this can lead to an unexisting path.
> 
> For instance, os.uname() returns:
> 
>     >>> os.uname()
>     ('Linux', 'localhost', '4.15.0-34-generic', '#37-Ubuntu SMP Mon Aug 27 15:18:58 UTC 2018', 'ppc64le')
> 
> Then, qemu_bin_relative_path would be set to:
> 
>     ppc64le-softmmu/qemu-system-ppc64le
> 
> However, the correct path is:
> 
>     ppc64-softmmu/qemu-system-ppc64
> 
> I'm not sure if ppc64le is the only case where the target name is different from
> the OS architecture.
> 
> If you decide not to handle this now, at least add a FIXME, if possible.
> 

You're correct, but this behavior is also unrelated to the *this*
specific change (the uname() based arch was already being used). Because
of that I'd rather send the fix in another series.

To track the fix I've created the following card:

https://trello.com/c/0quVUfKi/48-support-target-binary-selection-on-ppc64le

Thanks!
- Cleber.

>>      if is_readable_executable_file(qemu_bin_relative_path):
>> @@ -47,8 +53,9 @@ def pick_default_qemu_bin():
>>  class Test(avocado.Test):
>>      def setUp(self):
>>          self.vm = None
>> +        self.arch = self.params.get('arch', default=os.uname()[4])
>>          self.qemu_bin = self.params.get('qemu_bin',
>> -                                        default=pick_default_qemu_bin())
>> +                                        default=pick_default_qemu_bin(self.arch))
>>          if self.qemu_bin is None:
>>              self.cancel("No QEMU binary defined or found in the source tree")
>>          self.vm = QEMUMachine(self.qemu_bin)
>> --
>> 2.17.1
>>
>>
> 
> --
> Murilo
>