Re: [Qemu-devel] [PATCH v4 5/5] Acceptance tests: add Linux kernel boot and console checking test

[Philippe Mathieu-Daudé]

Hi Cleber,

On 05/30/2018 03:41 PM, Cleber Rosa wrote:
> This test boots a Linux kernel, and checks that the given command
> line was effective in two ways:
> 
>  * It makes the kernel use the set "console device" as a console
>  * The kernel records the command line as expected in the console
> 
> Given that way too many error conditions may occur, and detecting the
> kernel boot progress status may not be trivial, this test relies on a
> timeout to handle unexpected situations.  Also, it's *not* tagged as a
> quick test for obvious reasons.
> 
> It may be useful, while interactively running/debugging this test, or
> tests similar to this one, to show some of the logging channels.
> Example:
> 
>  $ avocado --show=QMP,console run boot_linux_console.py
> 
> Signed-off-by: Cleber Rosa <address@hidden>
> ---
>  tests/acceptance/boot_linux_console.py | 47 ++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
>  create mode 100644 tests/acceptance/boot_linux_console.py
> 
> diff --git a/tests/acceptance/boot_linux_console.py 
> b/tests/acceptance/boot_linux_console.py
> new file mode 100644
> index 0000000000..98324f7591
> --- /dev/null
> +++ b/tests/acceptance/boot_linux_console.py
> @@ -0,0 +1,47 @@
> +# Functional test that boots a Linux kernel and checks the console
> +#
> +# Copyright (c) 2018 Red Hat, Inc.
> +#
> +# Author:
> +#  Cleber Rosa <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.
> +
> +import logging
> +
> +from avocado_qemu import Test
> +
> +
> +class BootLinuxConsole(Test):
> +    """
> +    Boots a x86_64 Linux kernel and checks that the console is operational
> +    and the kernel command line is properly passed from QEMU to the kernel
> +
> +    :avocado: enable
> +    :avocado: tags=x86_64

Can you move tags=x86_64 to the test() method?

> +    """
> +
> +    timeout = 60
> +
> +    def test(self):


Can we rename this test_fedora28_x86_64_pc()?

> +        kernel_url = ('https://mirrors.kernel.org/fedora/releases/28/'
> +                      'Everything/x86_64/os/images/pxeboot/vmlinuz')
> +        kernel_hash = '238e083e114c48200f80d889f7e32eeb2793e02a'
> +        kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)

Not related to this patch, but while trying to extend your test, I
noticed fetch_asset() use the url filename as cache filename, so all
further tests (other arch) keep overwriting the same file and no caching
is done.
This is in particular why I used the hashed url as the filename in my
first dirty attempt.
https://lists.gnu.org/archive/html/qemu-devel/2018-04/msg03082.html

> +
> +        self.vm.set_machine('pc')
> +        self.vm.set_console()
> +        kernel_command_line = 'console=ttyS0'
> +        self.vm.add_args('-kernel', kernel_path,
> +                         '-append', kernel_command_line)
> +        self.vm.launch()
> +        console = self.vm.console_socket.makefile()
> +        console_logger = logging.getLogger('console')
> +        while True:
> +            msg = console.readline()
> +            console_logger.debug(msg.strip())
> +            if 'Kernel command line: %s' % kernel_command_line in msg:
> +                break
> +            if 'Kernel panic - not syncing' in msg:
> +                self.fail("Kernel panic reached")
> 

Thanks for cleaning/fixing this test =)

Moving the :avocado: tag:
Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
Tested-by: Philippe Mathieu-Daudé <address@hidden>

Regards,

Phil.