[6/6] tests/acceptance: Linux boot test for record/replay

From: Pavel Dovgalyuk <Pavel.Dovgaluk@gmail.com>

This patch adds a test for record/replay, which boots Linux
image from the disk and interacts with the network.
The idea and code of this test is borrowed from boot_linux.py
This test includes only x86_64 platform. Other platforms and
machines will be added later after testing and improving
record/replay to completely support them.

Each test consists of the following phases:
 - downloading the disk image
 - recording the execution
 - replaying the execution

Replay does not validates the output, but waits until QEMU
finishes the execution. This is reasonable, because
QEMU usually hangs when replay goes wrong.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
---
 MAINTAINERS                      |    1 
 tests/acceptance/replay_linux.py |  116 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 117 insertions(+)
 create mode 100644 tests/acceptance/replay_linux.py

Hi Pavel,

On Thu, Jun 10, 2021 at 8:25 AM Pavel Dovgalyuk
<pavel.dovgalyuk@ispras.ru> wrote:
>
> From: Pavel Dovgalyuk <Pavel.Dovgaluk@gmail.com>
>
> This patch adds a test for record/replay, which boots Linux
> image from the disk and interacts with the network.
> The idea and code of this test is borrowed from boot_linux.py
> This test includes only x86_64 platform. Other platforms and
> machines will be added later after testing and improving
> record/replay to completely support them.
>
> Each test consists of the following phases:
>  - downloading the disk image
>  - recording the execution
>  - replaying the execution
>
> Replay does not validates the output, but waits until QEMU
> finishes the execution. This is reasonable, because
> QEMU usually hangs when replay goes wrong.
>

It took me some time to review this patch because I could not identify
what makes it an automated test. I mean, when I look at an automated
test I expect a pass/fail/skip output. I could not identify the
expected output of this test compared to the actual result. If I did
not miss anything, this test will always pass unless there is an
exception that, potentially, could not be related to the record/replay
mechanism.

Also, as far as I could check, you inherit from the LinuxTest class
but only use the cloudinit methods. Most of the other methods are not
used or overridden. In this case, I think it is worth splitting the
LinuxTest with a new mixin utility class to handle the cloudinit part.
If you need help with that, let me know.

> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
> ---
>  MAINTAINERS                      |    1
>  tests/acceptance/replay_linux.py |  116 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 117 insertions(+)
>  create mode 100644 tests/acceptance/replay_linux.py
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7d9cd29042..9675a1095b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2863,6 +2863,7 @@ F: include/sysemu/replay.h
>  F: docs/replay.txt
>  F: stubs/replay.c
>  F: tests/acceptance/replay_kernel.py
> +F: tests/acceptance/replay_linux.py
>  F: tests/acceptance/reverse_debugging.py
>  F: qapi/replay.json
>
> diff --git a/tests/acceptance/replay_linux.py b/tests/acceptance/replay_linux.py
> new file mode 100644
> index 0000000000..15953f9e49
> --- /dev/null
> +++ b/tests/acceptance/replay_linux.py
> @@ -0,0 +1,116 @@
> +# Record/replay test that boots a complete Linux system via a cloud image
> +#
> +# Copyright (c) 2020 ISP RAS
> +#
> +# Author:
> +#  Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
> +#
> +# 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 logging
> +import time
> +
> +from avocado import skipUnless
> +from avocado.utils import cloudinit
> +from avocado.utils import network
> +from avocado.utils import vmimage
> +from avocado.utils import datadrainer
> +from avocado.utils.path import find_command
> +from avocado_qemu import LinuxTest
> +
> +class ReplayLinux(LinuxTest):
> +    """
> +    Boots a Linux system, checking for a successful initialization
> +    """
> +
> +    timeout = 1800
> +    chksum = None
> +    hdd = 'ide-hd'
> +    cd = 'ide-cd'
> +    bus = 'ide'
> +
> +    def setUp(self):
> +        super(ReplayLinux, self).setUp()
> +        self.boot_path = self.download_boot()
> +        self.cloudinit_path = self.prepare_cloudinit()
> +
> +    def vm_add_disk(self, vm, path, id, device):
> +        bus_string = ''
> +        if self.bus:
> +            bus_string = ',bus=%s.%d' % (self.bus, id,)
> +        vm.add_args('-drive', 'file=%s,snapshot,id=disk%s,if=none' % (path, id))
> +        vm.add_args('-drive',
> +            'driver=blkreplay,id=disk%s-rr,if=none,image=disk%s' % (id, id))
> +        vm.add_args('-device',
> +            '%s,drive=disk%s-rr%s' % (device, id, bus_string))
> +
> +    def launch_and_wait(self, record, args, shift):
> +        vm = self.get_vm()
> +        vm.add_args('-smp', '1')
> +        vm.add_args('-m', '1024')
> +        vm.add_args('-object', 'filter-replay,id=replay,netdev=hub0port0')
> +        if args:
> +            vm.add_args(*args)
> +        self.vm_add_disk(vm, self.boot_path, 0, self.hdd)
> +        self.vm_add_disk(vm, self.cloudinit_path, 1, self.cd)
> +        logger = logging.getLogger('replay')
> +        if record:
> +            logger.info('recording the execution...')
> +            mode = 'record'
> +        else:
> +            logger.info('replaying the execution...')
> +            mode = 'replay'
> +        replay_path = os.path.join(self.workdir, 'replay.bin')
> +        vm.add_args('-icount', 'shift=%s,rr=%s,rrfile=%s' %
> +                    (shift, mode, replay_path))
> +
> +        start_time = time.time()
> +
> +        vm.set_console()
> +        vm.launch()
> +        console_drainer = datadrainer.LineLogger(vm.console_socket.fileno(),
> +                                    logger=self.log.getChild('console'),
> +                                    stop_check=(lambda : not vm.is_running()))
> +        console_drainer.start()
> +        if record:
> +            cloudinit.wait_for_phone_home(('0.0.0.0', self.phone_home_port),
> +                                          self.name)
> +            vm.shutdown()
> +            logger.info('finished the recording with log size %s bytes'
> +                % os.path.getsize(replay_path))
> +        else:
> +            vm.event_wait('SHUTDOWN', self.timeout)
> +            vm.shutdown(True)
> +            logger.info('successfully fihished the replay')
> +        elapsed = time.time() - start_time
> +        logger.info('elapsed time %.2f sec' % elapsed)
> +        return elapsed
> +
> +    def run_rr(self, args=None, shift=7):
> +        t1 = self.launch_and_wait(True, args, shift)
> +        t2 = self.launch_and_wait(False, args, shift)
> +        logger = logging.getLogger('replay')
> +        logger.info('replay overhead {:.2%}'.format(t2 / t1 - 1))
> +
> +@skipUnless(os.getenv('AVOCADO_TIMEOUT_EXPECTED'), 'Test might timeout')
> +class ReplayLinuxX8664(ReplayLinux):
> +    """
> +    :avocado: tags=arch:x86_64
> +    :avocado: tags=accel:tcg
> +    """
> +
> +    chksum = 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'
> +
> +    def test_pc_i440fx(self):
> +        """
> +        :avocado: tags=machine:pc
> +        """
> +        self.run_rr(shift=1)
> +
> +    def test_pc_q35(self):
> +        """
> +        :avocado: tags=machine:q35
> +        """
> +        self.run_rr(shift=3)
>

On Wed, Jun 23, 2021 at 3:45 PM Willian Rampazzo <wrampazz@redhat.com> wrote:
>
> Hi Pavel,
>
> On Thu, Jun 10, 2021 at 8:25 AM Pavel Dovgalyuk
> <pavel.dovgalyuk@ispras.ru> wrote:
> >
> > From: Pavel Dovgalyuk <Pavel.Dovgaluk@gmail.com>
> >
> > This patch adds a test for record/replay, which boots Linux
> > image from the disk and interacts with the network.
> > The idea and code of this test is borrowed from boot_linux.py
> > This test includes only x86_64 platform. Other platforms and
> > machines will be added later after testing and improving
> > record/replay to completely support them.
> >
> > Each test consists of the following phases:
> >  - downloading the disk image
> >  - recording the execution
> >  - replaying the execution
> >
> > Replay does not validates the output, but waits until QEMU
> > finishes the execution. This is reasonable, because
> > QEMU usually hangs when replay goes wrong.
> >
>
> It took me some time to review this patch because I could not identify
> what makes it an automated test. I mean, when I look at an automated
> test I expect a pass/fail/skip output. I could not identify the
> expected output of this test compared to the actual result. If I did
> not miss anything, this test will always pass unless there is an
> exception that, potentially, could not be related to the record/replay
> mechanism.

I was looking at the current record/replay test, replay_kernel.py and
I noticed you followed the same pattern in this test. Although I do
not agree much with a test that does not have a specific
objective/check, I'm fine if this has value for you.

>
> Also, as far as I could check, you inherit from the LinuxTest class
> but only use the cloudinit methods. Most of the other methods are not
> used or overridden. In this case, I think it is worth splitting the
> LinuxTest with a new mixin utility class to handle the cloudinit part.
> If you need help with that, let me know.

As this is more related to code design, I can split the cloudinit code
later and adjust your code.
In this case,

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

>
> > Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
> > ---
> >  MAINTAINERS                      |    1
> >  tests/acceptance/replay_linux.py |  116 ++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 117 insertions(+)
> >  create mode 100644 tests/acceptance/replay_linux.py
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 7d9cd29042..9675a1095b 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -2863,6 +2863,7 @@ F: include/sysemu/replay.h
> >  F: docs/replay.txt
> >  F: stubs/replay.c
> >  F: tests/acceptance/replay_kernel.py
> > +F: tests/acceptance/replay_linux.py
> >  F: tests/acceptance/reverse_debugging.py
> >  F: qapi/replay.json
> >
> > diff --git a/tests/acceptance/replay_linux.py b/tests/acceptance/replay_linux.py
> > new file mode 100644
> > index 0000000000..15953f9e49
> > --- /dev/null
> > +++ b/tests/acceptance/replay_linux.py
> > @@ -0,0 +1,116 @@
> > +# Record/replay test that boots a complete Linux system via a cloud image
> > +#
> > +# Copyright (c) 2020 ISP RAS
> > +#
> > +# Author:
> > +#  Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
> > +#
> > +# 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 logging
> > +import time
> > +
> > +from avocado import skipUnless
> > +from avocado.utils import cloudinit
> > +from avocado.utils import network
> > +from avocado.utils import vmimage
> > +from avocado.utils import datadrainer
> > +from avocado.utils.path import find_command
> > +from avocado_qemu import LinuxTest
> > +
> > +class ReplayLinux(LinuxTest):
> > +    """
> > +    Boots a Linux system, checking for a successful initialization
> > +    """
> > +
> > +    timeout = 1800
> > +    chksum = None
> > +    hdd = 'ide-hd'
> > +    cd = 'ide-cd'
> > +    bus = 'ide'
> > +
> > +    def setUp(self):
> > +        super(ReplayLinux, self).setUp()
> > +        self.boot_path = self.download_boot()
> > +        self.cloudinit_path = self.prepare_cloudinit()
> > +
> > +    def vm_add_disk(self, vm, path, id, device):
> > +        bus_string = ''
> > +        if self.bus:
> > +            bus_string = ',bus=%s.%d' % (self.bus, id,)
> > +        vm.add_args('-drive', 'file=%s,snapshot,id=disk%s,if=none' % (path, id))
> > +        vm.add_args('-drive',
> > +            'driver=blkreplay,id=disk%s-rr,if=none,image=disk%s' % (id, id))
> > +        vm.add_args('-device',
> > +            '%s,drive=disk%s-rr%s' % (device, id, bus_string))
> > +
> > +    def launch_and_wait(self, record, args, shift):
> > +        vm = self.get_vm()
> > +        vm.add_args('-smp', '1')
> > +        vm.add_args('-m', '1024')
> > +        vm.add_args('-object', 'filter-replay,id=replay,netdev=hub0port0')
> > +        if args:
> > +            vm.add_args(*args)
> > +        self.vm_add_disk(vm, self.boot_path, 0, self.hdd)
> > +        self.vm_add_disk(vm, self.cloudinit_path, 1, self.cd)
> > +        logger = logging.getLogger('replay')
> > +        if record:
> > +            logger.info('recording the execution...')
> > +            mode = 'record'
> > +        else:
> > +            logger.info('replaying the execution...')
> > +            mode = 'replay'
> > +        replay_path = os.path.join(self.workdir, 'replay.bin')
> > +        vm.add_args('-icount', 'shift=%s,rr=%s,rrfile=%s' %
> > +                    (shift, mode, replay_path))
> > +
> > +        start_time = time.time()
> > +
> > +        vm.set_console()
> > +        vm.launch()
> > +        console_drainer = datadrainer.LineLogger(vm.console_socket.fileno(),
> > +                                    logger=self.log.getChild('console'),
> > +                                    stop_check=(lambda : not vm.is_running()))
> > +        console_drainer.start()
> > +        if record:
> > +            cloudinit.wait_for_phone_home(('0.0.0.0', self.phone_home_port),
> > +                                          self.name)
> > +            vm.shutdown()
> > +            logger.info('finished the recording with log size %s bytes'
> > +                % os.path.getsize(replay_path))
> > +        else:
> > +            vm.event_wait('SHUTDOWN', self.timeout)
> > +            vm.shutdown(True)
> > +            logger.info('successfully fihished the replay')
> > +        elapsed = time.time() - start_time
> > +        logger.info('elapsed time %.2f sec' % elapsed)
> > +        return elapsed
> > +
> > +    def run_rr(self, args=None, shift=7):
> > +        t1 = self.launch_and_wait(True, args, shift)
> > +        t2 = self.launch_and_wait(False, args, shift)
> > +        logger = logging.getLogger('replay')
> > +        logger.info('replay overhead {:.2%}'.format(t2 / t1 - 1))
> > +
> > +@skipUnless(os.getenv('AVOCADO_TIMEOUT_EXPECTED'), 'Test might timeout')
> > +class ReplayLinuxX8664(ReplayLinux):
> > +    """
> > +    :avocado: tags=arch:x86_64
> > +    :avocado: tags=accel:tcg
> > +    """
> > +
> > +    chksum = 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'
> > +
> > +    def test_pc_i440fx(self):
> > +        """
> > +        :avocado: tags=machine:pc
> > +        """
> > +        self.run_rr(shift=1)
> > +
> > +    def test_pc_q35(self):
> > +        """
> > +        :avocado: tags=machine:q35
> > +        """
> > +        self.run_rr(shift=3)
> >

On 24.06.2021 21:15, Willian Rampazzo wrote:
> On Wed, Jun 23, 2021 at 3:45 PM Willian Rampazzo <wrampazz@redhat.com> wrote:
>>
>> Hi Pavel,
>>
>> On Thu, Jun 10, 2021 at 8:25 AM Pavel Dovgalyuk
>> <pavel.dovgalyuk@ispras.ru> wrote:
>>>
>>> From: Pavel Dovgalyuk <Pavel.Dovgaluk@gmail.com>
>>>
>>> This patch adds a test for record/replay, which boots Linux
>>> image from the disk and interacts with the network.
>>> The idea and code of this test is borrowed from boot_linux.py
>>> This test includes only x86_64 platform. Other platforms and
>>> machines will be added later after testing and improving
>>> record/replay to completely support them.
>>>
>>> Each test consists of the following phases:
>>>   - downloading the disk image
>>>   - recording the execution
>>>   - replaying the execution
>>>
>>> Replay does not validates the output, but waits until QEMU
>>> finishes the execution. This is reasonable, because
>>> QEMU usually hangs when replay goes wrong.
>>>
>>
>> It took me some time to review this patch because I could not identify
>> what makes it an automated test. I mean, when I look at an automated
>> test I expect a pass/fail/skip output. I could not identify the
>> expected output of this test compared to the actual result. If I did
>> not miss anything, this test will always pass unless there is an
>> exception that, potentially, could not be related to the record/replay
>> mechanism.
> 
> I was looking at the current record/replay test, replay_kernel.py and
> I noticed you followed the same pattern in this test. Although I do
> not agree much with a test that does not have a specific
> objective/check, I'm fine if this has value for you.

That's right. Test is targeted to checking that replay is successful.
There are no other execution results to check.

> 
>>
>> Also, as far as I could check, you inherit from the LinuxTest class
>> but only use the cloudinit methods. Most of the other methods are not
>> used or overridden. In this case, I think it is worth splitting the
>> LinuxTest with a new mixin utility class to handle the cloudinit part.
>> If you need help with that, let me know.
> 
> As this is more related to code design, I can split the cloudinit code
> later and adjust your code.
> In this case,
> 
> Reviewed-by: Willian Rampazzo <willianr@redhat.com>

Thanks.

> 
>>
>>> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>>> ---
>>>   MAINTAINERS                      |    1
>>>   tests/acceptance/replay_linux.py |  116 ++++++++++++++++++++++++++++++++++++++
>>>   2 files changed, 117 insertions(+)
>>>   create mode 100644 tests/acceptance/replay_linux.py
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 7d9cd29042..9675a1095b 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -2863,6 +2863,7 @@ F: include/sysemu/replay.h
>>>   F: docs/replay.txt
>>>   F: stubs/replay.c
>>>   F: tests/acceptance/replay_kernel.py
>>> +F: tests/acceptance/replay_linux.py
>>>   F: tests/acceptance/reverse_debugging.py
>>>   F: qapi/replay.json
>>>
>>> diff --git a/tests/acceptance/replay_linux.py b/tests/acceptance/replay_linux.py
>>> new file mode 100644
>>> index 0000000000..15953f9e49
>>> --- /dev/null
>>> +++ b/tests/acceptance/replay_linux.py
>>> @@ -0,0 +1,116 @@
>>> +# Record/replay test that boots a complete Linux system via a cloud image
>>> +#
>>> +# Copyright (c) 2020 ISP RAS
>>> +#
>>> +# Author:
>>> +#  Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>>> +#
>>> +# 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 logging
>>> +import time
>>> +
>>> +from avocado import skipUnless
>>> +from avocado.utils import cloudinit
>>> +from avocado.utils import network
>>> +from avocado.utils import vmimage
>>> +from avocado.utils import datadrainer
>>> +from avocado.utils.path import find_command
>>> +from avocado_qemu import LinuxTest
>>> +
>>> +class ReplayLinux(LinuxTest):
>>> +    """
>>> +    Boots a Linux system, checking for a successful initialization
>>> +    """
>>> +
>>> +    timeout = 1800
>>> +    chksum = None
>>> +    hdd = 'ide-hd'
>>> +    cd = 'ide-cd'
>>> +    bus = 'ide'
>>> +
>>> +    def setUp(self):
>>> +        super(ReplayLinux, self).setUp()
>>> +        self.boot_path = self.download_boot()
>>> +        self.cloudinit_path = self.prepare_cloudinit()
>>> +
>>> +    def vm_add_disk(self, vm, path, id, device):
>>> +        bus_string = ''
>>> +        if self.bus:
>>> +            bus_string = ',bus=%s.%d' % (self.bus, id,)
>>> +        vm.add_args('-drive', 'file=%s,snapshot,id=disk%s,if=none' % (path, id))
>>> +        vm.add_args('-drive',
>>> +            'driver=blkreplay,id=disk%s-rr,if=none,image=disk%s' % (id, id))
>>> +        vm.add_args('-device',
>>> +            '%s,drive=disk%s-rr%s' % (device, id, bus_string))
>>> +
>>> +    def launch_and_wait(self, record, args, shift):
>>> +        vm = self.get_vm()
>>> +        vm.add_args('-smp', '1')
>>> +        vm.add_args('-m', '1024')
>>> +        vm.add_args('-object', 'filter-replay,id=replay,netdev=hub0port0')
>>> +        if args:
>>> +            vm.add_args(*args)
>>> +        self.vm_add_disk(vm, self.boot_path, 0, self.hdd)
>>> +        self.vm_add_disk(vm, self.cloudinit_path, 1, self.cd)
>>> +        logger = logging.getLogger('replay')
>>> +        if record:
>>> +            logger.info('recording the execution...')
>>> +            mode = 'record'
>>> +        else:
>>> +            logger.info('replaying the execution...')
>>> +            mode = 'replay'
>>> +        replay_path = os.path.join(self.workdir, 'replay.bin')
>>> +        vm.add_args('-icount', 'shift=%s,rr=%s,rrfile=%s' %
>>> +                    (shift, mode, replay_path))
>>> +
>>> +        start_time = time.time()
>>> +
>>> +        vm.set_console()
>>> +        vm.launch()
>>> +        console_drainer = datadrainer.LineLogger(vm.console_socket.fileno(),
>>> +                                    logger=self.log.getChild('console'),
>>> +                                    stop_check=(lambda : not vm.is_running()))
>>> +        console_drainer.start()
>>> +        if record:
>>> +            cloudinit.wait_for_phone_home(('0.0.0.0', self.phone_home_port),
>>> +                                          self.name)
>>> +            vm.shutdown()
>>> +            logger.info('finished the recording with log size %s bytes'
>>> +                % os.path.getsize(replay_path))
>>> +        else:
>>> +            vm.event_wait('SHUTDOWN', self.timeout)
>>> +            vm.shutdown(True)
>>> +            logger.info('successfully fihished the replay')
>>> +        elapsed = time.time() - start_time
>>> +        logger.info('elapsed time %.2f sec' % elapsed)
>>> +        return elapsed
>>> +
>>> +    def run_rr(self, args=None, shift=7):
>>> +        t1 = self.launch_and_wait(True, args, shift)
>>> +        t2 = self.launch_and_wait(False, args, shift)
>>> +        logger = logging.getLogger('replay')
>>> +        logger.info('replay overhead {:.2%}'.format(t2 / t1 - 1))
>>> +
>>> +@skipUnless(os.getenv('AVOCADO_TIMEOUT_EXPECTED'), 'Test might timeout')
>>> +class ReplayLinuxX8664(ReplayLinux):
>>> +    """
>>> +    :avocado: tags=arch:x86_64
>>> +    :avocado: tags=accel:tcg
>>> +    """
>>> +
>>> +    chksum = 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'
>>> +
>>> +    def test_pc_i440fx(self):
>>> +        """
>>> +        :avocado: tags=machine:pc
>>> +        """
>>> +        self.run_rr(shift=1)
>>> +
>>> +    def test_pc_q35(self):
>>> +        """
>>> +        :avocado: tags=machine:q35
>>> +        """
>>> +        self.run_rr(shift=3)
>>>
>