[PATCH 0/3] avocado_qemu: allow cross-arch tests

[PATCH 0/3] avocado_qemu: allow cross-arch tests

[Daniel Henrique Barboza]

Hi,

In these changes I've introduced two new avocado env variables to allow
users to run 'check-avocado' with different archs and machines in tests
that doesn't set any arch/machine to run with.

This is useful for archs that doesn't have abundance of real hardware
available (e.g. RISC-V), meaning that we end up running 'check-avocado'
in non-RISCV hosts every time, and most tests ends up being cancelled
because the test always defaults to the host arch. For example, building
QEMU for riscv64 in a x86_64 host will cancel all tests:

$ make check-avocado
(...)
RESULTS    : PASS 0 | ERROR 0 | FAIL 0 | SKIP 1 | WARN 0 | INTERRUPT 0 | CANCEL 11

After the changes implemented here, one can use env variables to force
the tests to a default arch and machine. In the scenario mentioned
above:

$ AVOCADO_DEFAULT_ARCH=riscv64 AVOCADO_DEFAULT_MACHINE=virt make check-avocado
(...)
RESULTS    : PASS 11 | ERROR 0 | FAIL 0 | SKIP 1 | WARN 0 | INTERRUPT 0 | CANCEL 0

Note that test behavior changes only when the env vars are set. There is
no change made in the regular work of 'check-avocado'.

Cc: Cleber Rosa <crosa@redhat.com>
Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
Cc: Beraldo Leal <bleal@redhat.com>

Daniel Henrique Barboza (3):
  avocado_qemu: enhance CANCEL message in QemuBaseTest:setUp()
  avocado_qemu: add AVOCADO_DEFAULT_ARCH for cross-arch tests
  avocado_qemu: add AVOCADO_DEFAULT_MACHINE

 docs/devel/testing.rst                 | 15 ++++++++++++---
 tests/avocado/avocado_qemu/__init__.py | 22 +++++++++++++++-------
 2 files changed, 27 insertions(+), 10 deletions(-)

-- 
2.39.0

[PATCH 1/3] avocado_qemu: enhance CANCEL message in QemuBaseTest:setUp()

[Daniel Henrique Barboza]

Trying to run 'make check-avocado' while having only non-x86_64 QEMU
binaries built, in a x86_64 host machine, will give us the following
cancel message:

"CANCEL: No QEMU binary defined or found in the build tree"

Which is not quite what's happening here. Avocado defaults to the host
arch for every arch-agnostic test, and in the case mentioned above it
cancelled the test because there were no qemu-system-x86_64 binary to be
found.

Change pick_default_qemu_bin() to return a (qemu_bin, arch) tuple, then
use 'arch' in the CANCEL message. This will make the error more
informative:

"CANCEL: No QEMU binary defined or found in the build tree for arch x86_64"

Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
 tests/avocado/avocado_qemu/__init__.py | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tests/avocado/avocado_qemu/__init__.py b/tests/avocado/avocado_qemu/__init__.py
index 910f3ba1ea..8614ac3978 100644
--- a/tests/avocado/avocado_qemu/__init__.py
+++ b/tests/avocado/avocado_qemu/__init__.py
@@ -128,8 +128,8 @@ def pick_default_qemu_bin(bin_prefix='qemu-system-', arch=None):
     ]
     for path in qemu_bin_paths:
         if is_readable_executable_file(path):
-            return path
-    return None
+            return path, arch
+    return None, arch
 
 
 def _console_interaction(test, success_message, failure_message,
@@ -247,11 +247,13 @@ def setUp(self, bin_prefix):
         self.cpu = self.params.get('cpu',
                                    default=self._get_unique_tag_val('cpu'))
 
-        default_qemu_bin = pick_default_qemu_bin(bin_prefix, arch=self.arch)
+        default_qemu_bin, arch = pick_default_qemu_bin(bin_prefix,
+                                                       arch=self.arch)
         self.qemu_bin = self.params.get('qemu_bin',
                                         default=default_qemu_bin)
         if self.qemu_bin is None:
-            self.cancel("No QEMU binary defined or found in the build tree")
+            self.cancel("No QEMU binary defined or found in the "
+                        "build tree for arch %s" % arch)
 
     def fetch_asset(self, name,
                     asset_hash=None, algorithm=None,
-- 
2.39.0

Re: [PATCH 1/3] avocado_qemu: enhance CANCEL message in QemuBaseTest:setUp()

[Fabiano Rosas]

Daniel Henrique Barboza <dbarboza@ventanamicro.com> writes:

> Trying to run 'make check-avocado' while having only non-x86_64 QEMU
> binaries built, in a x86_64 host machine, will give us the following
> cancel message:
>
> "CANCEL: No QEMU binary defined or found in the build tree"
>
> Which is not quite what's happening here. Avocado defaults to the host
> arch for every arch-agnostic test, and in the case mentioned above it
> cancelled the test because there were no qemu-system-x86_64 binary to be
> found.
>
> Change pick_default_qemu_bin() to return a (qemu_bin, arch) tuple, then
> use 'arch' in the CANCEL message. This will make the error more
> informative:
>
> "CANCEL: No QEMU binary defined or found in the build tree for arch x86_64"
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

Reviewed-by: Fabiano Rosas <farosas@suse.de>

[PATCH 2/3] avocado_qemu: add AVOCADO_DEFAULT_ARCH for cross-arch tests

[Daniel Henrique Barboza]

All avocado tests that are arch agnostic (i.e. does not set an 'arch'
tag) are run with arch=None in pick_default_qemu_bin(), and then 'arch'
is set to os.uname()[4], meaning that it will take the arch of the
running host.

This means that if one compiles QEMU binaries for non-x86 targets on an
x86 machine, and then run 'make check-avocado', all arch agnostic tests
will be cancelled because there's no qemu-system-x86_64 to be found.

There is no particular reason to not allow these tests to be run with
other arch binaries in a x86_64 host. Allow the developer to do it by
adding a a new env variable called AVOCADO_DEFAULT_ARCH. Any 'arch' that
is set by this variable will take precedence of setting it via
os.uname()[4]. We can then run non-x86 binaries tests in a x86_64 host
as follows:

$ AVOCADO_DEFAULT_ARCH=riscv64 make check-avocado
(...)
RESULTS: PASS 11 | ERROR 0 | FAIL 0 | SKIP 1 | WARN 0 | INTERRUPT 0 | CANCEL 0

Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
 docs/devel/testing.rst                 | 7 +++++++
 tests/avocado/avocado_qemu/__init__.py | 9 ++++++---
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index e10c47b5a7..95d0a3e626 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -1144,6 +1144,13 @@ the framework or by the test itself.  At the framework level, it will
 currently influence the selection of a QEMU binary (when one is not
 explicitly given).
 
+When ``arch`` is not set, an env variable AVOCADO_DEFAULT_ARCH can
+be used as default value if set.  This allows hosts of different
+architectures to run arch-agnostic tests using binaries from other
+archs (i.e. a x86_64 host can use aarch64/riscv64 binaries as
+default).  If this variable isn't set, ``arch`` defaults to the
+host system arch given by ``os.uname``.
+
 Tests are also free to use this attribute value, for their own needs.
 A test may, for instance, use the same value when selecting the
 architecture of a kernel or disk image to boot a VM with.
diff --git a/tests/avocado/avocado_qemu/__init__.py b/tests/avocado/avocado_qemu/__init__.py
index 8614ac3978..bc42985cbb 100644
--- a/tests/avocado/avocado_qemu/__init__.py
+++ b/tests/avocado/avocado_qemu/__init__.py
@@ -107,14 +107,17 @@ def pick_default_qemu_bin(bin_prefix='qemu-system-', arch=None):
     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`).
+                 will match the arch given).  If None (the default), check
+                 if the AVOCADO_DEFAULT_ARCH env var is set and use it as
+                 arch.  If it's not set, arch will be the current host
+                 system arch (as given by :func:`os.uname`).
     :type arch: str
     :returns: the path to the default QEMU binary or None if one could not
               be found
     :rtype: str or None
     """
+    if arch is None:
+        arch = os.getenv('AVOCADO_DEFAULT_ARCH')
     if arch is None:
         arch = os.uname()[4]
     # qemu binary path does not match arch for powerpc, handle it
-- 
2.39.0

Re: [PATCH 2/3] avocado_qemu: add AVOCADO_DEFAULT_ARCH for cross-arch tests

[Fabiano Rosas]

Daniel Henrique Barboza <dbarboza@ventanamicro.com> writes:

> All avocado tests that are arch agnostic (i.e. does not set an 'arch'
> tag) are run with arch=None in pick_default_qemu_bin(), and then 'arch'
> is set to os.uname()[4], meaning that it will take the arch of the
> running host.
>
> This means that if one compiles QEMU binaries for non-x86 targets on an
> x86 machine, and then run 'make check-avocado', all arch agnostic tests
> will be cancelled because there's no qemu-system-x86_64 to be found.
>
> There is no particular reason to not allow these tests to be run with
> other arch binaries in a x86_64 host. Allow the developer to do it by
> adding a a new env variable called AVOCADO_DEFAULT_ARCH. Any 'arch' that
> is set by this variable will take precedence of setting it via
> os.uname()[4]. We can then run non-x86 binaries tests in a x86_64 host
> as follows:
>
> $ AVOCADO_DEFAULT_ARCH=riscv64 make check-avocado
> (...)
> RESULTS: PASS 11 | ERROR 0 | FAIL 0 | SKIP 1 | WARN 0 | INTERRUPT 0 | CANCEL 0

I don't understand why tags don't solve the problem. We
are already passing a tag for each target:

ifndef AVOCADO_TAGS
	AVOCADO_CMDLINE_TAGS=$(patsubst %-softmmu,-t arch:%, \
						 $(filter %-softmmu,$(TARGETS)))
else
	AVOCADO_CMDLINE_TAGS=$(addprefix -t , $(AVOCADO_TAGS))
endif

I then tried to tag migration.py with:

    :avocado: tags=arch:x86_64
    :avocado: tags=arch:aarch64

On an x86_64 machine with target-list=x86_64-softmmu,aarch64-softmmu,
only the x86_64 test runs. Even if I remove the x86_64 tag from the
avocado line. Possibly due to the --filter-by-tags-include-empty
options. But I would expect a second run with aarch64, even if it
failed.

If I use only:

    :avocado: tags=arch:riscv

and run:

python3 -m avocado --show=app run -t arch:riscv -t arch:x86_64 --failfast ../tests/avocado/migration.py

Then it complains about the binary, but the x86_64 binary is present! So
it looked at the tag after all:

CANCEL: No QEMU binary defined or found in the build tree for arch riscv
                                                                   ^^^^^

I don't know how to make this work, but I feel there should be a way to
have the framework select the correct test AND pass the correct arch
parameter along. 

Re: [PATCH 2/3] avocado_qemu: add AVOCADO_DEFAULT_ARCH for cross-arch tests

[Daniel Henrique Barboza]

On 1/18/23 12:23, Fabiano Rosas wrote:
> Daniel Henrique Barboza <dbarboza@ventanamicro.com> writes:
>
>> All avocado tests that are arch agnostic (i.e. does not set an 'arch'
>> tag) are run with arch=None in pick_default_qemu_bin(), and then 'arch'
>> is set to os.uname()[4], meaning that it will take the arch of the
>> running host.
>>
>> This means that if one compiles QEMU binaries for non-x86 targets on an
>> x86 machine, and then run 'make check-avocado', all arch agnostic tests
>> will be cancelled because there's no qemu-system-x86_64 to be found.
>>
>> There is no particular reason to not allow these tests to be run with
>> other arch binaries in a x86_64 host. Allow the developer to do it by
>> adding a a new env variable called AVOCADO_DEFAULT_ARCH. Any 'arch' that
>> is set by this variable will take precedence of setting it via
>> os.uname()[4]. We can then run non-x86 binaries tests in a x86_64 host
>> as follows:
>>
>> $ AVOCADO_DEFAULT_ARCH=riscv64 make check-avocado
>> (...)
>> RESULTS: PASS 11 | ERROR 0 | FAIL 0 | SKIP 1 | WARN 0 | INTERRUPT 0 | CANCEL 0
> I don't understand why tags don't solve the problem. We
> are already passing a tag for each target:
>
> ifndef AVOCADO_TAGS
> 	AVOCADO_CMDLINE_TAGS=$(patsubst %-softmmu,-t arch:%, \
> 						 $(filter %-softmmu,$(TARGETS)))
> else
> 	AVOCADO_CMDLINE_TAGS=$(addprefix -t , $(AVOCADO_TAGS))
> endif
>
> I then tried to tag migration.py with:
>
>      :avocado: tags=arch:x86_64
>      :avocado: tags=arch:aarch64
>
> On an x86_64 machine with target-list=x86_64-softmmu,aarch64-softmmu,
> only the x86_64 test runs. Even if I remove the x86_64 tag from the
> avocado line. Possibly due to the --filter-by-tags-include-empty
> options. But I would expect a second run with aarch64, even if it
> failed.

I can give some light in this bit. tests/avocado/avocado_qemu/__init__.py,
setUp() from  "class QemuBaseTest":


     def setUp(self, bin_prefix):
         self.arch = self.params.get('arch',
default=self._get_unique_tag_val('arch'))


So what you did there:

:avocado: tags=arch:x86_64
:avocado: tags=arch:aarch64

Will result in "self.arch = None"  because 'arch' is not an unique tag. This has the
same effect as giving no 'arch' tags at all.

In fact, all avocado tags fetched from the test files works the same way. If the tag
isn't unique it's considered None.

It is worth mentioning that the docs mentions that the 'arch' tag must be unique:

=====
The ``arch`` 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=arch:VALUE`` tag, it will be set to ``VALUE``.
=====

But it doesn't mention that having multiple 'arch' tags will have the same effect
as having no 'arch' tag though.


>
> If I use only:
>
>      :avocado: tags=arch:riscv
>
> and run:
>
> python3 -m avocado --show=app run -t arch:riscv -t arch:x86_64 --failfast ../tests/avocado/migration.py
>
> Then it complains about the binary, but the x86_64 binary is present! So
> it looked at the tag after all:
>
> CANCEL: No QEMU binary defined or found in the build tree for arch riscv
>                                                                     ^^^^^

Yeah, in this case it looked at the 'arch:riscv' tag in the tes,t because it was the only 'arch' tag,
and complained that you didn't have RISC-V binaries because it actually tried to find them.

Also, what you did there with the "-t arch:riscv -t arch:x86_64" is "run this avocado
test if the test tags matches these". There is no way of "assigning" a different
arch/machine/etc via these command line tags. Probably this wasn't your intention
at all but I'd rather mention it to be safe.


Thanks,

Daniel



>
> I don't know how to make this work, but I feel there should be a way to
> have the framework select the correct test AND pass the correct arch
> parameter along.

[PATCH 3/3] avocado_qemu: add AVOCADO_DEFAULT_MACHINE

[Daniel Henrique Barboza]

Allow users to control which machine to use in tests where no particular
machine is specified. This is a good way to run tests in a machine
that's not the arch default, e.g. for qemu-system-x86_64 tests can be
run using a machine other than pc-i440fx.

This env var has greater effect when used together with the recently
added AVOCADO_DEFAULT_ARCH, allowing full control of what the tests
should use if no default arch/machine is given. In this example, a
check-avocado run will use the RISC-V 'virt' machine if no default
arch/machine is set:

AVOCADO_DEFAULT_ARCH=riscv64 AVOCADO_DEFAULT_MACHINE=virt make check-avocado

Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
 docs/devel/testing.rst                 | 8 +++++---
 tests/avocado/avocado_qemu/__init__.py | 3 +++
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index 95d0a3e626..128b6ae964 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -1178,9 +1178,11 @@ The machine type that will be set to all QEMUMachine instances created
 by the test.
 
 The ``machine`` attribute will be set to the test parameter of the same
-name.  If one is not given explicitly, it will either be set to
-``None``, or, if the test is tagged with one (and only one)
-``:avocado: tags=machine:VALUE`` tag, it will be set to ``VALUE``.
+name.  If one is not given explicitly, an env variable
+AVOCADO_DEFAULT_MACHINE can be used as default value if set.  Otherwise
+it will either be set to ``None``, or, if the test is tagged with one
+(and only one) ``:avocado: tags=machine:VALUE`` tag, it will be set to
+``VALUE``.
 
 qemu_bin
 ''''''''
diff --git a/tests/avocado/avocado_qemu/__init__.py b/tests/avocado/avocado_qemu/__init__.py
index bc42985cbb..75eaaa9265 100644
--- a/tests/avocado/avocado_qemu/__init__.py
+++ b/tests/avocado/avocado_qemu/__init__.py
@@ -282,6 +282,9 @@ def setUp(self):
         self.machine = self.params.get('machine',
                                        default=self._get_unique_tag_val('machine'))
 
+        if self.machine is None:
+            self.machine = os.getenv('AVOCADO_DEFAULT_MACHINE')
+
     def require_accelerator(self, accelerator):
         """
         Requires an accelerator to be available for the test to continue
-- 
2.39.0

Re: [PATCH 3/3] avocado_qemu: add AVOCADO_DEFAULT_MACHINE

[Fabiano Rosas]

Daniel Henrique Barboza <dbarboza@ventanamicro.com> writes:

> Allow users to control which machine to use in tests where no particular
> machine is specified. This is a good way to run tests in a machine
> that's not the arch default, e.g. for qemu-system-x86_64 tests can be
> run using a machine other than pc-i440fx.
>
> This env var has greater effect when used together with the recently
> added AVOCADO_DEFAULT_ARCH, allowing full control of what the tests
> should use if no default arch/machine is given. In this example, a
> check-avocado run will use the RISC-V 'virt' machine if no default
> arch/machine is set:
>
> AVOCADO_DEFAULT_ARCH=riscv64 AVOCADO_DEFAULT_MACHINE=virt make check-avocado
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>  docs/devel/testing.rst                 | 8 +++++---
>  tests/avocado/avocado_qemu/__init__.py | 3 +++
>  2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
> index 95d0a3e626..128b6ae964 100644
> --- a/docs/devel/testing.rst
> +++ b/docs/devel/testing.rst
> @@ -1178,9 +1178,11 @@ The machine type that will be set to all QEMUMachine instances created
>  by the test.
>  
>  The ``machine`` attribute will be set to the test parameter of the same
> -name.  If one is not given explicitly, it will either be set to
> -``None``, or, if the test is tagged with one (and only one)
> -``:avocado: tags=machine:VALUE`` tag, it will be set to ``VALUE``.
> +name.  If one is not given explicitly, an env variable
> +AVOCADO_DEFAULT_MACHINE can be used as default value if set.  Otherwise
> +it will either be set to ``None``, or, if the test is tagged with one
> +(and only one) ``:avocado: tags=machine:VALUE`` tag, it will be set to
> +``VALUE``.
>  
>  qemu_bin
>  ''''''''
> diff --git a/tests/avocado/avocado_qemu/__init__.py b/tests/avocado/avocado_qemu/__init__.py
> index bc42985cbb..75eaaa9265 100644
> --- a/tests/avocado/avocado_qemu/__init__.py
> +++ b/tests/avocado/avocado_qemu/__init__.py
> @@ -282,6 +282,9 @@ def setUp(self):
>          self.machine = self.params.get('machine',
>                                         default=self._get_unique_tag_val('machine'))
>  

Would it work for you to replicate the AVOCADO_CMDLINE_TAGS behavior?

In Makefile.include:

ifdef AVOCADO_PARAMS
	AVOCADO_CMDLINE_PARAMS=$(addprefix -p , $(AVOCADO_PARAMS))
endif

make V=1 check-avocado AVOCADO_PARAMS=machine=foo AVOCADO_TESTS=../tests/avocado/migration.py

Results in:

./qemu-system-x86_64 -display none -vga none ... -machine foo

Re: [PATCH 3/3] avocado_qemu: add AVOCADO_DEFAULT_MACHINE

[Daniel Henrique Barboza]

On 1/18/23 12:42, Fabiano Rosas wrote:
> Daniel Henrique Barboza <dbarboza@ventanamicro.com> writes:
>
>> Allow users to control which machine to use in tests where no particular
>> machine is specified. This is a good way to run tests in a machine
>> that's not the arch default, e.g. for qemu-system-x86_64 tests can be
>> run using a machine other than pc-i440fx.
>>
>> This env var has greater effect when used together with the recently
>> added AVOCADO_DEFAULT_ARCH, allowing full control of what the tests
>> should use if no default arch/machine is given. In this example, a
>> check-avocado run will use the RISC-V 'virt' machine if no default
>> arch/machine is set:
>>
>> AVOCADO_DEFAULT_ARCH=riscv64 AVOCADO_DEFAULT_MACHINE=virt make check-avocado
>>
>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>> ---
>>   docs/devel/testing.rst                 | 8 +++++---
>>   tests/avocado/avocado_qemu/__init__.py | 3 +++
>>   2 files changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
>> index 95d0a3e626..128b6ae964 100644
>> --- a/docs/devel/testing.rst
>> +++ b/docs/devel/testing.rst
>> @@ -1178,9 +1178,11 @@ The machine type that will be set to all QEMUMachine instances created
>>   by the test.
>>   
>>   The ``machine`` attribute will be set to the test parameter of the same
>> -name.  If one is not given explicitly, it will either be set to
>> -``None``, or, if the test is tagged with one (and only one)
>> -``:avocado: tags=machine:VALUE`` tag, it will be set to ``VALUE``.
>> +name.  If one is not given explicitly, an env variable
>> +AVOCADO_DEFAULT_MACHINE can be used as default value if set.  Otherwise
>> +it will either be set to ``None``, or, if the test is tagged with one
>> +(and only one) ``:avocado: tags=machine:VALUE`` tag, it will be set to
>> +``VALUE``.
>>   
>>   qemu_bin
>>   ''''''''
>> diff --git a/tests/avocado/avocado_qemu/__init__.py b/tests/avocado/avocado_qemu/__init__.py
>> index bc42985cbb..75eaaa9265 100644
>> --- a/tests/avocado/avocado_qemu/__init__.py
>> +++ b/tests/avocado/avocado_qemu/__init__.py
>> @@ -282,6 +282,9 @@ def setUp(self):
>>           self.machine = self.params.get('machine',
>>                                          default=self._get_unique_tag_val('machine'))
>>   
> Would it work for you to replicate the AVOCADO_CMDLINE_TAGS behavior?
>
> In Makefile.include:
>
> ifdef AVOCADO_PARAMS
> 	AVOCADO_CMDLINE_PARAMS=$(addprefix -p , $(AVOCADO_PARAMS))
> endif
>
> make V=1 check-avocado AVOCADO_PARAMS=machine=foo AVOCADO_TESTS=../tests/avocado/migration.py
>
> Results in:
>
> ./qemu-system-x86_64 -display none -vga none ... -machine foo

This doesn't work as is (tested with no patches applied, only added AVOCADO_PARAMS in
Makefile.include).

At this moment it seems that there is no way of changing Avocado internal behavior
by using command line parameters/tags.


Thanks,


Daniel