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 <waine...@redhat.com> > > > --- > > > 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 <waine...@redhat.com> > > > +# > > > +# 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
