qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Alex Bennée" <alex.bennee@linaro.org>
To: Cleber Rosa <crosa@redhat.com>
Cc: fam@euphon.net, qemu-devel@nongnu.org,
	Wainer dos Santos Moschetta <wainersm@redhat.com>,
	wrampazz@redhat.com, philmd@redhat.com, sgarzare@redhat.com
Subject: Re: [PATCH 0/2] tests/acceptance: Add boot vmlinux test
Date: Tue, 10 Dec 2019 11:05:18 +0000	[thread overview]
Message-ID: <87a78030gh.fsf@linaro.org> (raw)
In-Reply-To: <20191206164204.GB23522@dhcp-17-72.bos.redhat.com>


Cleber Rosa <crosa@redhat.com> writes:

> On Fri, Dec 06, 2019 at 09:00:10AM -0500, Wainer dos Santos Moschetta wrote:
>> This series add a new acceptance test: boot an uncompressed
>> Linux kernel built with CONFIG_PVH, so checking the PVH
>> capability introduced in QEMU 4.0 works.
>> 
>> The test implementation depends on [1] which is likely released
>> on next Avocado. So that will need a version 2 of this
>> series to bump Avocado version.
>>
>
> Right, the Avocado bits have been merged, so unless a major reversal
> comes (not expected at all), it will be on Avocado 74.0.
>
>> Also I want to use this as an example of a scenario that test
>> assets could be better managed. As you see on patch 01 the
>> kernel is built at test time on localhost. While Avocado provides
>> an API to easily fetch and build it, the whole process takes
>> reasonable time - besides the fact that localhost must have
>> all build dependencies installed. How could it be done better?
>>
>
> This is clearly not a "kernel build" test, so we should avoid building
> the kernel as part of the "PVH boot" test.  The problem you expose
> here is a very real, and each possible solution has its own problems,
> unfortunately.
>
> The best solution IMO would be to find a "well known" distribution of
> such a kernel.  Maybe something maintained by the Xen project or one
> of its commercial products?
>
> The second best solution is to have a helper script (using the Avocado
> utils API is fine) that will build a kernel and create/populate the
> test's data directory (tests/acceptance/pvh.py.data/) with a vmlinux.
> The test can cancel itself if it doesn't find a kernel there.
>
> The third best solution IMO is for this test to require a parameter
> telling where the CONFIG_PVH enabled vmlinux kernel image is.
>
> Also notice that, with a custom built kernel image (a different one
> for each user), the result of the test as a general indicator to the
> QEMU codebase diminishes quite a bit.

I can see a use case for making it easier for developers to build test
cases otherwise everyone has their own particular variant of the kernel
and buildroot/busybox which they have in their own farm. However as
Cleber has noted they make poor standardised tests given the variation
in kernel builds you can get.

That said I think there are better targets. kvm-unit-tests can be built
against a range of architectures and are fashioned as individual unit
tests for specific functionality. It would be useful to make it easy for
a developer to regenerate the test assets to re-run a test someone else
has found to fail.

>> Nonetheless I argue in favor of merging this as is, and
>> gradually implement corrections to improve the tests assets
>> management. Also if eventually the test is proven to unacceptable
>> slow down the Travis CI then I can add a guard to skip it.
>>
>
> I'm pretty sure that building the kernel will cause a major slow down
> of the Travis CI results for the "acceptance" block (and thus for the
> overall results).  But, it may also time it out (there's a 50 minutes
> deadline).
>
> I hope this brings some ideas, and thanks for coming up with
> interesting problems! :)
>
> - Cleber.
>
>> [1] https://github.com/avocado-framework/avocado/pull/3389
>> 
>> Wainer dos Santos Moschetta (2):
>>   tests/acceptance: Add PVH boot test
>>   .travis.yml: Add kernel build deps for acceptance tests
>> 
>>  .travis.yml             |  2 ++
>>  tests/acceptance/pvh.py | 48 +++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 50 insertions(+)
>>  create mode 100644 tests/acceptance/pvh.py
>> 
>> -- 
>> 2.21.0
>> 


-- 
Alex Bennée


      reply	other threads:[~2019-12-10 11:06 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-06 14:00 [PATCH 0/2] tests/acceptance: Add boot vmlinux test Wainer dos Santos Moschetta
2019-12-06 14:00 ` [PATCH 1/2] tests/acceptance: Add PVH boot test Wainer dos Santos Moschetta
2019-12-06 15:04   ` Willian Rampazzo
2019-12-06 16:54   ` Cleber Rosa
2019-12-09 14:43     ` Wainer dos Santos Moschetta
2019-12-10  2:21       ` Cleber Rosa
2019-12-10 11:16   ` Philippe Mathieu-Daudé
2019-12-06 14:00 ` [PATCH 2/2] .travis.yml: Add kernel build deps for acceptance tests Wainer dos Santos Moschetta
2019-12-12 12:22   ` Alex Bennée
2019-12-06 16:42 ` [PATCH 0/2] tests/acceptance: Add boot vmlinux test Cleber Rosa
2019-12-10 11:05   ` Alex Bennée [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87a78030gh.fsf@linaro.org \
    --to=alex.bennee@linaro.org \
    --cc=crosa@redhat.com \
    --cc=fam@euphon.net \
    --cc=philmd@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=sgarzare@redhat.com \
    --cc=wainersm@redhat.com \
    --cc=wrampazz@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).