Re: [PATCH v4 0/8] Acceptance tests: boot Linux with KVM test - Philippe Mathieu-Daudé

From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
To: Wainer dos Santos Moschetta <wainersm@redhat.com>, qemu-devel@nongnu.org
Cc: drjones@redhat.com, thuth@redhat.com, alex.bennee@linaro.org,
	ehabkost@redhat.com, crosa@redhat.com
Subject: Re: [PATCH v4 0/8] Acceptance tests: boot Linux with KVM test
Date: Mon, 10 Feb 2020 15:53:05 +0100	[thread overview]
Message-ID: <305480ea-feeb-683a-c611-0643f4460e6c@redhat.com> (raw)
In-Reply-To: <20200205203250.30526-1-wainersm@redhat.com>

Hi Wainer,

On 2/5/20 9:32 PM, Wainer dos Santos Moschetta wrote:
> This adds boot Linux tests for x86_64, aarch64, ppc64, and s390x
> targets which, unlike others, enable the KVM acceleration. Likewise
> it was added test cases for tcg.
> 
> It is introduced an infraestructure on avocado_qemu framework
> so that:
> a) simply tagging the test with `accel:kvm` (or `accel:tcg`) will
> automatically set the corresponding '-accel' on the launched
> QEMU;
> b) test is canceled if the accelerator is not enabled on the QEMU
> binary or not available in the host. In special, it checks if SMT
> is disabled on POWER8.
> 
> The acceptance builder on Travis was changed too in order to make
> the test run.
> 
> Changes v3 -> v4:
> - Broke changes per-arch to ease the reviews. Resulting on
>    patches 02, 03, 05, 06.
> - The test for aarch64 now passes '-cpu max' and
>    -M 'virt,gic-version=max'. (patch 03) [drjones]
> - Added a fix to accel.kvm_available() so that it detects
>    correctly the availability of kvm on ppc64le. (patch 05)
> - The test for ppc64le now checks if SMT is enabled on
>    POWER8 then skip.
> 
> v3: [PATCH v3 0/4] Acceptance tests: boot Linux with KVM test
> - https://www.mail-archive.com/qemu-devel@nongnu.org/msg672635.html
> v2: [PATCH v2 0/3] Acceptance tests: boot Linux with KVM test
> - https://www.mail-archive.com/qemu-devel@nongnu.org/msg666238.html
> v1: [PATCH 0/3] Acceptance tests: boot Linux with KVM test
> - https://www.mail-archive.com/qemu-devel@nongnu.org/msg627498.html
> 
> Tree:
> - Git: https://github.com/wainersm/qemu
> - Branch: acceptance_kvm_test-v4
> 
> CI:
> - Travis (FAIL): https://travis-ci.org/wainersm/qemu/builds/646154220
>    Failed jobs are not related with this series changes.
> 
> Wainer dos Santos Moschetta (8):
>    tests/acceptance: avocado_qemu: Introduce the 'accel' test parameter
>    tests/acceptance: boot_linux_console: Add boot Linux/x86 with KVM
>    tests/acceptance: boot_linux_console: Add boot Linux/aarch64 with KVM
>    python/qemu: accel: Fix kvm_available() on ppc64le
>    test/acceptance: boot_linux_console: Add boot Linux/ppc64le with KVM
>    tests/acceptance: boot_linux_console: Add boot Linux/s390x with KVM
>    tests/acceptance: avocado_qemu: Refactor the handler of 'machine'
>      parameter
>    travis.yml: Enable acceptance KVM tests
> 
>   .travis.yml                               |   7 +-
>   docs/devel/testing.rst                    |  16 ++++
>   python/qemu/accel.py                      |   3 +-
>   tests/acceptance/avocado_qemu/__init__.py |  27 +++++-
>   tests/acceptance/boot_linux_console.py    | 108 +++++++++++++++++-----
>   5 files changed, 136 insertions(+), 25 deletions(-)

As I'm not sure this is the correct to do, I'll defer this review to Cleber.
We can also have a look at it during the next Avocado call.