Re: [PATCH 2/9] tests/acceptance: add base class record/replay kernel tests

[Willian Rampazzo]

On Mon, May 25, 2020 at 8:22 AM Pavel Dovgalyuk
<address@hidden> wrote:
>
> This patch adds a base for testing kernel boot recording and replaying.
> Each test has the phase of recording and phase of replaying.
> Virtual machines just boot the kernel and do not interact with
> the network.
> Structure and image links for the tests are borrowed from 
> boot_linux_console.py
> Testing controls the message pattern at the end of the kernel
> boot for both record and replay modes. In replay mode QEMU is also
> intended to finish the execution automatically.
>
> Signed-off-by: Pavel Dovgalyuk <address@hidden>
> ---
>  MAINTAINERS                       |    1
>  tests/acceptance/replay_kernel.py |   80 
> +++++++++++++++++++++++++++++++++++++
>  2 files changed, 81 insertions(+)
>  create mode 100644 tests/acceptance/replay_kernel.py
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 47ef3139e6..e9a9ce4f66 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2497,6 +2497,7 @@ F: net/filter-replay.c
>  F: include/sysemu/replay.h
>  F: docs/replay.txt
>  F: stubs/replay.c
> +F: tests/acceptance/replay_kernel.py
>
>  IOVA Tree
>  M: Peter Xu <address@hidden>
> diff --git a/tests/acceptance/replay_kernel.py 
> b/tests/acceptance/replay_kernel.py
> new file mode 100644
> index 0000000000..3208179789
> --- /dev/null
> +++ b/tests/acceptance/replay_kernel.py
> @@ -0,0 +1,80 @@
> +# Record/replay test that boots a Linux kernel
> +#
> +# Copyright (c) 2020 ISP RAS
> +#
> +# Author:
> +#  Pavel Dovgalyuk <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 os
> +import gzip
> +
> +from avocado_qemu import Test
> +from avocado_qemu import wait_for_console_pattern
> +from avocado.utils import process
> +from avocado.utils import archive
> +
> +class ReplayKernel(Test):
> +    """
> +    Boots a Linux kernel in record mode and checks that the console
> +    is operational and the kernel command line is properly passed
> +    from QEMU to the kernel.
> +    Then replays the same scenario and verifies, that QEMU correctly
> +    terminates.
> +    """

The best to do here, IMHO, is to split the BootLinuxConsole class on
boot_linux_console.py into two classes, one with the necessary
utilities inheriting from Test and the second with the tests itself,
inheriting from the first. After that you can also inherit from the
first class in the boot_linux_console.py here and avoid code
duplication.

> +
> +    timeout = 90
> +
> +    KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 '
> +
> +    def wait_for_console_pattern(self, success_message, vm):
> +        wait_for_console_pattern(self, success_message,
> +                                 failure_message='Kernel panic - not 
> syncing',
> +                                 vm=vm)
> +
> +    def extract_from_deb(self, deb, path):
> +        """
> +        Extracts a file from a deb package into the test workdir
> +
> +        :param deb: path to the deb archive
> +        :param path: path within the deb archive of the file to be extracted
> +        :returns: path of the extracted file
> +        """
> +        cwd = os.getcwd()
> +        os.chdir(self.workdir)
> +        file_path = process.run("ar t %s" % deb).stdout_text.split()[2]
> +        process.run("ar x %s %s" % (deb, file_path))
> +        archive.extract(file_path, self.workdir)
> +        os.chdir(cwd)
> +        # Return complete path to extracted file.  Because callers to
> +        # extract_from_deb() specify 'path' with a leading slash, it is
> +        # necessary to use os.path.relpath() as otherwise os.path.join()
> +        # interprets it as an absolute path and drops the self.workdir part.
> +        return os.path.normpath(os.path.join(self.workdir,
> +                                             os.path.relpath(path, '/')))
> +
> +    def run_vm(self, kernel_path, kernel_command_line, console_pattern, 
> record, shift, args):
> +        vm = self.get_vm()
> +        vm.set_console()
> +        if record:
> +            mode = 'record'
> +        else:
> +            mode = 'replay'
> +        vm.add_args('-icount', 'shift=%s,rr=%s,rrfile=%s' %
> +                    (shift, mode, os.path.join(self.workdir, 'replay.bin')),
> +                    '-kernel', kernel_path,
> +                    '-append', kernel_command_line,
> +                    '-net', 'none',
> +                    *args)
> +        vm.launch()
> +        self.wait_for_console_pattern(console_pattern, vm)
> +        if record:
> +            vm.shutdown()
> +        else:
> +            vm.wait()
> +
> +    def run_rr(self, kernel_path, kernel_command_line, console_pattern, 
> shift=7, args=()):

Same comment from patch file 9, here you can use the default value of
args as None and handle it in the run_vm method. It is usually
recommended to use a None value for default arguments in Python
instead of an empty structure.

> +        self.run_vm(kernel_path, kernel_command_line, console_pattern, True, 
> shift, args)
> +        self.run_vm(kernel_path, kernel_command_line, console_pattern, 
> False, shift, args)
>
>