Max Reitz <mre...@redhat.com> writes:
> On 04.02.21 14:23, Alex Bennée wrote: >> >> Cleber Rosa <cr...@redhat.com> writes: >> >>> If the vmlinuz variable is set to anything that evaluates to True, >>> then the respective arguments should be set. If the variable contains >>> an empty string, than it will evaluate to False, and the extra >>> arguments will not be set. >>> >>> This keeps the same logic, but improves readability a bit. >>> >>> Signed-off-by: Cleber Rosa <cr...@redhat.com> >>> --- >>> tests/acceptance/virtiofs_submounts.py | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/tests/acceptance/virtiofs_submounts.py >>> b/tests/acceptance/virtiofs_submounts.py >>> index f1b49f03bb..f25a386a19 100644 >>> --- a/tests/acceptance/virtiofs_submounts.py >>> +++ b/tests/acceptance/virtiofs_submounts.py >>> @@ -241,7 +241,7 @@ class VirtiofsSubmountsTest(BootLinux): >>> >>> super(VirtiofsSubmountsTest, self).setUp(pubkey) >>> >>> - if len(vmlinuz) > 0: >>> + if vmlinuz: >>> self.vm.add_args('-kernel', vmlinuz, >>> '-append', 'console=ttyS0 root=/dev/sda1') >> >> And this is were I gave up because I can't see how to run the test: >> >> ./tests/venv/bin/avocado run ./tests/acceptance/virtiofs_submounts.py >> JOB ID : b3d9bfcfcd603189a471bec7d770fca3b50a81ee >> JOB LOG : >> /home/alex/avocado/job-results/job-2021-02-04T13.21-b3d9bfc/job.log >> (1/5) >> ./tests/acceptance/virtiofs_submounts.py:VirtiofsSubmountsTest.test_pre_virtiofsd_set_up: >> CANCEL: vmlinuz parameter not set; you must point it to a Linux kernel >> binary to test (to run this test with the on-image kernel, set it to an >> empty string) (0.00 s) >> (2/5) >> ./tests/acceptance/virtiofs_submounts.py:VirtiofsSubmountsTest.test_pre_launch_set_up: >> CANCEL: vmlinuz parameter not set; you must point it to a Linux kernel >> binary to test (to run this test with the on-image kernel, set it to an >> empty string) (0.00 s) >> (3/5) >> ./tests/acceptance/virtiofs_submounts.py:VirtiofsSubmountsTest.test_post_launch_set_up: >> CANCEL: vmlinuz parameter not set; you must point it to a Linux kernel >> binary >> to test (to run this test with the on-image kernel, set it to an empty >> string) (0.00 s) >> (4/5) >> ./tests/acceptance/virtiofs_submounts.py:VirtiofsSubmountsTest.test_post_mount_set_up: >> CANCEL: vmlinuz parameter not set; you must point it to a Linux kernel >> binary to test (to run this test with the on-image kernel, set it to an >> empty string) (0.00 s) >> (5/5) >> ./tests/acceptance/virtiofs_submounts.py:VirtiofsSubmountsTest.test_two_runs: >> CANCEL: vmlinuz parameter not set; you must point it to a Linux kernel >> binary to test (to run this test with the on-image kernel, set it to an >> empty string) (0.00 s) >> RESULTS : PASS 0 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | >> CANCEL 5 >> JOB TIME : 0.56 s >> >> Given the test seems to make assumptions about an environment being >> setup for it I think we need some documentation somewhere about what >> those pre-requisites are. > > I find the cancel message pretty clear, but then again it was me who > wrote it... > > Either you point the vmlinuz parameter to some Linux kernel you built > yourself, or you set it to the empty string to just use the kernel > that’s part of the image. Setting Avocado parameters is done via -p, > i.e. “-p key=value”. So in this case > “-p vmlinuz=/path/to/linux/build/arch/x86/boot/bzImage”, or > “-p vmlinuz=''”. > > Ideally, vmlinuz='' would be the default, but there’s a problem with > that: I submitted this test along with the patches that added the > required feature to the Linux kernel, so at that point that feature was > not part of Linux. That’s why you generally have to point it to a Linux > kernel binary you built yourself that has this feature (5.10 does). This is where it deviates from the rest of the check-acceptance tests. Generally I don't have to worry about finding the right image myself. At the least it would be worth pointing to a part of our docs on how to build such an image. > Using vmlinuz='' you can test it with the kernel that is part of the > cloud image. Once that kernel is sufficiently new (i.e., >=5.10), we > can make that the default. Good. > > Max -- Alex Bennée