[Qemu-devel] [PATCH 00/18] Acceptance Tests: target architecture support

Cleber Rosa posted 18 patches 5 years, 3 months ago
Test asan passed
Test docker-clang@ubuntu passed
Test docker-mingw@fedora passed
Test checkpatch failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190117185628.21862-1-crosa@redhat.com
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, Eduardo Habkost <ehabkost@redhat.com>, Cleber Rosa <crosa@redhat.com>, Fam Zheng <fam@euphon.net>, "Philippe Mathieu-Daudé" <philmd@redhat.com>
There is a newer version of this series
.travis.yml                               |   4 +-
docs/devel/testing.rst                    |  23 ++-
scripts/qemu.py                           |  32 ++--
tests/Makefile.include                    |   5 +-
tests/acceptance/avocado_qemu/__init__.py |  22 ++-
tests/acceptance/boot_linux_console.py    | 208 ++++++++++++++++++++--
tests/requirements.txt                    |   2 +-
7 files changed, 254 insertions(+), 42 deletions(-)
[Qemu-devel] [PATCH 00/18] Acceptance Tests: target architecture support
Posted by Cleber Rosa 5 years, 3 months ago
The current acceptance tests don't provide any type of architecture
information that can be used to influence the selection of the QEMU
binary used on them[1].  If one is running tests on a x86_64 host, the
default QEMU binary will be "x86_64-softmmu/qemu-system-x86_64".

Given the nature of QEMU, some tests will be architecture agnostic,
while others will be architecture dependent.  The "check-qtest" and
"check-qtest-TARGET" make targets exemplify that pattern.

For the acceptance tests, the same requirement exists.  Tests should
be allowed to influence the binary used, and when they don't, a
default selection mechanism should kick in[2].  The proposed solution
here requires only that an Avocado tag is set, such as:

   class My(Test):
       def test_nx_cpu_flag(self):
           """
           :avocado: tags=arch:x86_64
           """
           test_code()

The value of the "arch" key, in this case, "x86_64" will be used when
selecting the QEMU binary to use in the test.  At the same time, if
"x86_64-softmmu" is not a built target, the test will be filtered out
by "make check-acceptance"[3].

Besides the convention explained above, where the binary will be
selected from the "arch" tag, it's also possible to set an "arch"
*parameter* that will also influence the QEMU binary selection:

  $ avocado run -p arch=aarch64 works-on-many-arches.py

Finally, it's also posible to set the "qemu_bin" parameter, which will
define (instead of just influencing) the QEMU binary to be used:

 $ avocado run -p qemu_bin=qemu-bin-aarch64 test.py

As examples for the idea proposed here, a number of "boot linux
console" tests have been added, for a number of different target
architectures.  When the build environment includes them (as it has
been added to Travis CI jobs) the architecture specific tests will be
automatically executed.

As mentioned previously, this patch series include ideas present in
other patch series, and from different authors.  I tried by best
to include the information about authorship, but if I missed any,
please accept my apologies and let me know.

---

[1] - The "boot_linux_console.py" contains a "x86_64" test tag, but
      that is informational only, in the sense that it's not consumed
      by the test itself, or used by "make check-acceptance" to filter
      out tests.

[2] - This patch series doesn't attempt to change the default selection
      mechanism.  Possible changes in this area may include looking for
      any one built binary first, no matter the host architecture.

[3] - On a previous proposed version, the test class would look at the
      "arch" parameter given, and would cancel the test if there wasn't
      a match.

---

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

Cleber Rosa (17):
  scripts/qemu.py: log QEMU launch command line
  Acceptance tests: show avocado test execution by default
  Acceptance tests: improve docstring on pick_default_qemu_bin()
  Acceptance tests: fix doc reference to avocado_qemu directory
  Acceptance tests: introduce arch parameter and attribute
  Acceptance tests: use "arch:" tag to filter target specific tests
  Acceptance tests: look for target architecture in test tags first
  Boot Linux Console Test: rename the x86_64 after the arch and machine
  Boot Linux Console Test: update the x86_64 kernel
  Boot Linux Console Test: refactor the console watcher into utility
    method
  scripts/qemu.py: support adding a console with the default serial
    device
  Boot Linux Console Test: add a test for mips64el + malta
  Boot Linux Console Test: add a test for ppc64 + pseries
  Boot Linux Console Test: add a test for aarch64 + virt
  Boot Linux Console Test: add a test for arm + virt
  Boot Linux Console Test: add a test for s390x + s390-ccw-virtio
  Boot Linux Console Test: add a test for alpha + clipper

Philippe Mathieu-Daudé (1):
  Boot Linux Console Test: add a test for mips + malta

 .travis.yml                               |   4 +-
 docs/devel/testing.rst                    |  23 ++-
 scripts/qemu.py                           |  32 ++--
 tests/Makefile.include                    |   5 +-
 tests/acceptance/avocado_qemu/__init__.py |  22 ++-
 tests/acceptance/boot_linux_console.py    | 208 ++++++++++++++++++++--
 tests/requirements.txt                    |   2 +-
 7 files changed, 254 insertions(+), 42 deletions(-)

-- 
2.20.1


Re: [Qemu-devel] [PATCH 00/18] Acceptance Tests: target architecture support
Posted by Aleksandar Markovic 5 years, 3 months ago
> From: Cleber Rosa <crosa@redhat.com>

>   class My(Test):
>       def test_nx_cpu_flag(self):
>           """
>           :avocado: tags=arch:x86_64
>           """
>           test_code()

> The value of the "arch" key, in this case, "x86_64" will be used when
> selecting the QEMU binary to use in the test.  At the same time, if
> "x86_64-softmmu" is not a built target, the test will be filtered out
> by "make check-acceptance"[3].

I think, the term "arch" is a little problematic in QEMU parlance. IMHO,
"target" should be used instead. ("arch" is used in Linux kernel community)

The overall structure of the "tags" should be a little different. My
suggestion:

"target"
"isa" (instruction set architecture, determeines how the kernel and rootfs are built)
"cpu"
"machine"

This would allow clear view what a particular acceptance test tests, and will
enforce consistency and clarity in the test organization across the board.

That said, I am very excited about this series.

Regards,
Aleksandar


Re: [Qemu-devel] [PATCH 00/18] Acceptance Tests: target architecture support
Posted by Philippe Mathieu-Daudé 5 years, 3 months ago
Hi Aleksandar,

On 1/21/19 11:15 PM, Aleksandar Markovic wrote:
>> From: Cleber Rosa <crosa@redhat.com>
> 
>>   class My(Test):
>>       def test_nx_cpu_flag(self):
>>           """
>>           :avocado: tags=arch:x86_64
>>           """
>>           test_code()
> 
>> The value of the "arch" key, in this case, "x86_64" will be used when
>> selecting the QEMU binary to use in the test.  At the same time, if
>> "x86_64-softmmu" is not a built target, the test will be filtered out
>> by "make check-acceptance"[3].
> 
> I think, the term "arch" is a little problematic in QEMU parlance. IMHO,
> "target" should be used instead. ("arch" is used in Linux kernel community)

Using target_arch/host_arch might be more explicit, but host_arch is not
very relevant regarding Avocado (except to choose the correct TCG
binary), so usually arch implies target_arch.

I mean, it is unlikely you enforce --host_arch on the command line to
run tests, this using --arch instead of --target_arch looks OK to me,
since host_arch can be guessed.

> 
> The overall structure of the "tags" should be a little different. My
> suggestion:
> 
> "target"
> "isa" (instruction set architecture, determeines how the kernel and rootfs are built)

In QEMU, "isa" is implicit with CPU, isnt' it?

I.e. I use:

$ mips64el-softmmu/qemu-system-mips64el -M Malta -cpu mips64dspr2

> "cpu"
> "machine"
> 
> This would allow clear view what a particular acceptance test tests, and will
> enforce consistency and clarity in the test organization across the board.

Maybe the problem you are pointing out is not Avocado test organization
but QEMU CPU organization... (which also bother me with boards able to
use different SoC, with different peripherals or cpus).

What is your idea on passing arch/cpu/isa to start QEMU?

> 
> That said, I am very excited about this series.

Me too :)

Regards,

Phil.

Re: [Qemu-devel] [PATCH 00/18] Acceptance Tests: target architecture support
Posted by Cleber Rosa 5 years, 2 months ago

On 1/22/19 5:48 AM, Philippe Mathieu-Daudé wrote:
> Hi Aleksandar,
> 
> On 1/21/19 11:15 PM, Aleksandar Markovic wrote:
>>> From: Cleber Rosa <crosa@redhat.com>
>>
>>>   class My(Test):
>>>       def test_nx_cpu_flag(self):
>>>           """
>>>           :avocado: tags=arch:x86_64
>>>           """
>>>           test_code()
>>
>>> The value of the "arch" key, in this case, "x86_64" will be used when
>>> selecting the QEMU binary to use in the test.  At the same time, if
>>> "x86_64-softmmu" is not a built target, the test will be filtered out
>>> by "make check-acceptance"[3].
>>
>> I think, the term "arch" is a little problematic in QEMU parlance. IMHO,
>> "target" should be used instead. ("arch" is used in Linux kernel community)
> 
> Using target_arch/host_arch might be more explicit, but host_arch is not
> very relevant regarding Avocado (except to choose the correct TCG
> binary), so usually arch implies target_arch.
> 
> I mean, it is unlikely you enforce --host_arch on the command line to
> run tests, this using --arch instead of --target_arch looks OK to me,
> since host_arch can be guessed.
> 

Naming things is hard, so this is a valid discussion.  But, I have to
say that I also find "arch" in this context to be descriptive enough.

>>
>> The overall structure of the "tags" should be a little different. My
>> suggestion:
>>
>> "target"
>> "isa" (instruction set architecture, determeines how the kernel and rootfs are built)
> 
> In QEMU, "isa" is implicit with CPU, isnt' it?
> 
> I.e. I use:
> 
> $ mips64el-softmmu/qemu-system-mips64el -M Malta -cpu mips64dspr2
> 
>> "cpu"
>> "machine"
>>
>> This would allow clear view what a particular acceptance test tests, and will
>> enforce consistency and clarity in the test organization across the board.
> 
> Maybe the problem you are pointing out is not Avocado test organization
> but QEMU CPU organization... (which also bother me with boards able to
> use different SoC, with different peripherals or cpus).
> 
> What is your idea on passing arch/cpu/isa to start QEMU?
> 
>>
>> That said, I am very excited about this series.
> 
> Me too :)
> 
> Regards,
> 
> Phil.
> 

Thanks! For the encouragement, help and input!

- Cleber.

Re: [Qemu-devel] [PATCH 00/18] Acceptance Tests: target architecture support
Posted by Aleksandar Markovic 5 years, 2 months ago
>> I think, the term "arch" is a little problematic in QEMU parlance. IMHO,
>> "target" should be used instead. ("arch" is used in Linux kernel community)

> Naming things is hard, so this is a valid discussion.  But, I have to
> say that I also find "arch" in this context to be descriptive enough.

This is not a "naming" problem, but a fundamental terminology convention used in QEMU. I am truly dissapointed that you chose to disrespect it.

Aleksandar

Re: [Qemu-devel] [PATCH 00/18] Acceptance Tests: target architecture support
Posted by Cleber Rosa 5 years, 2 months ago

On 2/1/19 12:32 AM, Aleksandar Markovic wrote:
>>> I think, the term "arch" is a little problematic in QEMU parlance. IMHO,
>>> "target" should be used instead. ("arch" is used in Linux kernel community)
> 
>> Naming things is hard, so this is a valid discussion.  But, I have to
>> say that I also find "arch" in this context to be descriptive enough.
> 
> This is not a "naming" problem, but a fundamental terminology convention used in QEMU. I am truly dissapointed that you chose to disrespect it.
> 
> Aleksandar
> 

Hi Aleksandar,

I'm basically listening to more experienced people on the matter, as I
clearly lack the background on the "terminology conventions", and thus,
I'm unable to make the best naming choices by myself.

Now, we have had participation from few people, and there doesn't seem
to be consensus.  So, to make it clear, I didn't *choose* to disrespect
it, I chose to respect how I perceived the community input.

If this makes you truly disappointed, please ask more people to give
their input, and I'll gladly steer the direction of this humble ship, I
mean, patch series. :)

Regards,
- Cleber.

Re: [Qemu-devel] [PATCH 00/18] Acceptance Tests: target architecture support
Posted by no-reply@patchew.org 5 years, 2 months ago
Patchew URL: https://patchew.org/QEMU/20190117185628.21862-1-crosa@redhat.com/



Hi,

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

Subject: [Qemu-devel] [PATCH 00/18] Acceptance Tests: target architecture support
Type: series
Message-id: 20190117185628.21862-1-crosa@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
a27efa3fec Boot Linux Console Test: add a test for alpha + clipper
9c5d101f9b Boot Linux Console Test: add a test for s390x + s390-ccw-virtio
7d584982d7 Boot Linux Console Test: add a test for arm + virt
039f09b5e5 Boot Linux Console Test: add a test for aarch64 + virt
a52a2dd405 Boot Linux Console Test: add a test for ppc64 + pseries
1995edd0d0 Boot Linux Console Test: add a test for mips64el + malta
cbd4fb29d7 Boot Linux Console Test: add a test for mips + malta
62ca82a551 scripts/qemu.py: support adding a console with the default serial device
8f31626ddd Boot Linux Console Test: refactor the console watcher into utility method
c257e982f1 Boot Linux Console Test: update the x86_64 kernel
9859ac0034 Boot Linux Console Test: rename the x86_64 after the arch and machine
4fa0bc613a Acceptance tests: look for target architecture in test tags first
6051a67e66 Acceptance tests: use "arch:" tag to filter target specific tests
8e86083ca9 Acceptance tests: introduce arch parameter and attribute
6ec8dc9d2e Acceptance tests: fix doc reference to avocado_qemu directory
5c8a9897ba Acceptance tests: improve docstring on pick_default_qemu_bin()
0e98f722c6 Acceptance tests: show avocado test execution by default
7f31c568c2 scripts/qemu.py: log QEMU launch command line

=== OUTPUT BEGIN ===
1/18 Checking commit 7f31c568c2e0 (scripts/qemu.py: log QEMU launch command line)
2/18 Checking commit 0e98f722c669 (Acceptance tests: show avocado test execution by default)
3/18 Checking commit 5c8a9897ba6a (Acceptance tests: improve docstring on pick_default_qemu_bin())
4/18 Checking commit 6ec8dc9d2eed (Acceptance tests: fix doc reference to avocado_qemu directory)
5/18 Checking commit 8e86083ca9e8 (Acceptance tests: introduce arch parameter and attribute)
WARNING: line over 80 characters
#96: FILE: tests/acceptance/avocado_qemu/__init__.py:58:
+                                        default=pick_default_qemu_bin(self.arch))

total: 0 errors, 1 warnings, 64 lines checked

Patch 5/18 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/18 Checking commit 6051a67e66ea (Acceptance tests: use "arch:" tag to filter target specific tests)
7/18 Checking commit 4fa0bc613a2e (Acceptance tests: look for target architecture in test tags first)
8/18 Checking commit 9859ac003400 (Boot Linux Console Test: rename the x86_64 after the arch and machine)
9/18 Checking commit c257e982f129 (Boot Linux Console Test: update the x86_64 kernel)
10/18 Checking commit 8f31626ddd45 (Boot Linux Console Test: refactor the console watcher into utility method)
11/18 Checking commit 62ca82a551c2 (scripts/qemu.py: support adding a console with the default serial device)
12/18 Checking commit cbd4fb29d7c8 (Boot Linux Console Test: add a test for mips + malta)
13/18 Checking commit 1995edd0d009 (Boot Linux Console Test: add a test for mips64el + malta)
ERROR: line over 90 characters
#66: FILE: tests/acceptance/boot_linux_console.py:95:
+        [1] https://kernel-team.pages.debian.net/kernel-handbook/ch-common-tasks.html#s-common-official

ERROR: line over 90 characters
#67: FILE: tests/acceptance/boot_linux_console.py:96:
+        [2] http://snapshot.debian.org/package/linux-2.6/2.6.32-48/#linux-source-2.6.32_2.6.32-48

total: 2 errors, 0 warnings, 58 lines checked

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

14/18 Checking commit a52a2dd405ae (Boot Linux Console Test: add a test for ppc64 + pseries)
15/18 Checking commit 039f09b5e5a4 (Boot Linux Console Test: add a test for aarch64 + virt)
16/18 Checking commit 7d584982d75c (Boot Linux Console Test: add a test for arm + virt)
17/18 Checking commit 9c5d101f9bfb (Boot Linux Console Test: add a test for s390x + s390-ccw-virtio)
18/18 Checking commit a27efa3fec59 (Boot Linux Console Test: add a test for alpha + clipper)
=== OUTPUT END ===

Test command exited with code: 1


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