[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/2] tests/acceptance: Add PVH boot test
From: |
Cleber Rosa |
Subject: |
Re: [PATCH 1/2] tests/acceptance: Add PVH boot test |
Date: |
Mon, 9 Dec 2019 21:21:10 -0500 |
User-agent: |
Mutt/1.12.1 (2019-06-15) |
On Mon, Dec 09, 2019 at 12:43:22PM -0200, Wainer dos Santos Moschetta wrote:
>
> On 12/6/19 2:54 PM, Cleber Rosa wrote:
> > On Fri, Dec 06, 2019 at 09:00:11AM -0500, Wainer dos Santos Moschetta wrote:
> > > QEMU 4.0 onward is able to boot an uncompressed kernel
> > > image by using the x86/HVM direct boot ABI. It needs
> > > Linux >= 4.21 built with CONFIG_PVH=y.
> > >
> > > This introduces an acceptance test which checks an
> > > uncompressed Linux kernel image boots properly.
> > >
> > > Signed-off-by: Wainer dos Santos Moschetta <address@hidden>
> > > ---
> > > tests/acceptance/pvh.py | 48 +++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 48 insertions(+)
> > > create mode 100644 tests/acceptance/pvh.py
> > >
> > > diff --git a/tests/acceptance/pvh.py b/tests/acceptance/pvh.py
> > > new file mode 100644
> > > index 0000000000..c68489c273
> > > --- /dev/null
> > > +++ b/tests/acceptance/pvh.py
> > > @@ -0,0 +1,48 @@
> > > +# Copyright (c) 2019 Red Hat, Inc.
> > > +#
> > > +# Author:
> > > +# Wainer dos Santos Moschetta <address@hidden>
> > > +#
> > > +# This work is licensed under the terms of the GNU GPL, version 2 or
> > > +# later. See the COPYING file in the top-level directory.
> > > +
> > > +"""
> > > +x86/HVM direct boot acceptance tests.
> > > +"""
> > > +
> > > +from avocado.utils.kernel import KernelBuild
> > > +from avocado_qemu import Test
> > > +from avocado_qemu import wait_for_console_pattern
> > > +
> > > +
> > > +class Pvh(Test):
> > > + """
> > > + Test suite for x86/HVM direct boot feature.
> > > +
> > > + :avocado: tags=slow,arch=x86_64,machine=q35
This should be:
:avocado: tags=slow,arch:x86_64,machine:q35
That is, the separator of key/val is ':', because the equal sign is
used to separate the docstring directive type (here it's "tags") from
their content. `avocado list -V` should show you the tag keys with
all their values inside a parenthesis. That is, for the following
docstring directive:
:avocado: tags=slow,arch:x86_64,machine:q35,machine:pc
You'd get:
slow,arch(x86_64),machine(q35,pc)
> > > + """
> > > + def test_boot_vmlinux(self):
> > > + """
> > > + Boot uncompressed kernel image.
> > > + """
> > > + # QEMU can boot a vmlinux image for kernel >= 4.21 built
> > > + # with CONFIG_PVH=y
> > > + kernel_version = '5.4.1'
> > > + kbuild = KernelBuild(kernel_version, work_dir=self.workdir)
> > > + try:
> > > + kbuild.download()
> > > + kbuild.uncompress()
> > > + kbuild.configure(targets=['defconfig', 'kvmconfig'],
> > > + extra_configs=['CONFIG_PVH=y'])
> > I'm aware of the reason why this uses APIs not fulfilled by
> > tests/requirements.txt, but, for the general public reviewing/testing
> > code with extra requirements, it's a good idea to bump the
> > requirements to a version that fulfills the requirement, and comment
> > out clearly on the temporary nature of the change (marking the patch).
>
> Good idea, thanks for the tip.
>
> >
> > For instance, for this requirement, we could have:
> >
> > diff --git a/tests/requirements.txt b/tests/requirements.txt
> > index a2a587223a..5498d67bc1 100644
> > --- a/tests/requirements.txt
> > +++ b/tests/requirements.txt
> > @@ -1,4 +1,5 @@
> > # Add Python module requirements, one per line, to be installed
> > # in the tests/venv Python virtual environment. For more info,
> > # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
> > -avocado-framework==72.0
> > +# [REMOVE ME] use post 73.0 Avocado containing the new kernel build APIs
> > +-e
> > git+https://github.com/avocado-framework/avocado@d6fb24edcf847f09c312b55df3c674c64c79793e#egg=avocado_framework
> >
> > This will not only help people to test it, but should also make
> > it work transparently on CI.
>
> True. It could had helped me to check the missing packages on Travis to
> build the kernel. I'm ashamed to tell how I did it. :)
>
Don't be, because you did check it. :)
> >
> > > + kbuild.build()
> > As stated in my response to the cover letter, I think we need to move
> > this elsewhere. The *very* minimum is to have this in a setUp()
> > method, but we should strongly consider other solutions.
>
> On the proposed implementation the kernel is built only once and only for
> this test case. If I move the code to setUp() it will attempt to build the
> vmlinux for every case even when not needed (suppose I add a 'boot not
> CONFIG_PVH vmlinux to check it properly handle error' case which uses
> distro's kernel). Unless I put a guard like 'do not build if already
> present' which IMHO is weird. In other words, IMHO setUp() should hold only
> code that is share-able across cases.
>
I was thinking of *this* test setUp(), not avocado_qemu.Test.setUp().
Anyway, looking at the other options we talked about, I was able to
boot a vmlinux image from a "mainstream distro" kernel package[1] that
already has CONFIG_PVH enabled[2] with recent QEMU (and also tested
that I wasn't able to do so with older QEMU). Other distros also
provide a vmlinux image, but as part of the debuginfo packages and
they can be HUGE, so not recommended here.
If we go with this route, compilation would be a non-issue, and this
test would be just like the other "boot_linux_console.py" tests.
> >
> > > + except:
> > > + self.cancel("Unable to build vanilla kernel %s" %
> > > kernel_version)
> > > +
> > > + self.vm.set_machine('q35')
> > > + self.vm.set_console()
> > > + kernel_command_line = 'printk.time=0 console=ttyS0'
> > > + self.vm.add_args('-kernel', kbuild.vmlinux,
> > > + '-append', kernel_command_line)
> > And just for being thorough (and purist? idealistic? Utopian? :), if
> > we stop and think about it, the following two lines are really what
> > this test is all about. Everything else should be the test's setup.
> >
> > I'm not arguing in favor of being radical and reject anything that is
> > not perfect, but just reminding ourselves (myself very much included)
> > of this general direction.
>
> IMHO we should merge tests which are "good enough" then interactively
> improve them. At least they will run with some frequency and eventually
> catch regressions while infra bits are improved. Now, what's 'good enough'
> for an acceptance test? Perhaps a test that run consistently?
>
Right. But even though this test can be proven stable (I can't
disprove it), we also have to watch for the overall user experience.
Like I've said before, I don't think users running this test are
interested in building a kernel, but asserting a QEMU feature, and
that can be a source of "test distrust" IMO.
> >
> > Cheers,
> > - Cleber.
> >
> > > + self.vm.launch()
> > > + wait_for_console_pattern(self, 'Kernel command line: %s' %
> > > + kernel_command_line)
> > > --
> > > 2.21.0
> > >
Please let me know what you think of reusing an available kernel instead
of building one.
Cheers,
- Cleber.
[1]
https://download.opensuse.org/repositories/openSUSE:/Factory/standard/x86_64/kernel-vanilla-5.3.12-1.1.x86_64.rpm
[2]
https://kernel.opensuse.org/cgit/kernel-source/tree/config/x86_64/vanilla?h=linux-next&id=03bbea2f5521b0fe7bae800297509e9ca4c23117#n331
[3]
http://mirrors.syringanetworks.net/fedora/linux/releases/31/Everything/x86_64/debug/tree/Packages/k/
signature.asc
Description: PGP signature