From: Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru>
To: Willian Rampazzo <wrampazz@redhat.com>
Cc: "Paolo Bonzini" <pbonzini@redhat.com>,
"Cleber Rosa Junior" <crosa@redhat.com>,
"Alex Bennée" <alex.bennee@linaro.org>,
qemu-devel <qemu-devel@nongnu.org>,
"Philippe Mathieu Daude" <philmd@redhat.com>
Subject: Re: [PATCH 6/6] tests/acceptance: Linux boot test for record/replay
Date: Fri, 25 Jun 2021 08:36:14 +0300 [thread overview]
Message-ID: <a5de555b-e7f5-09a5-422d-f71855a269e6@ispras.ru> (raw)
In-Reply-To: <CAKJDGDZZWKFnD-48kTOjg+8b4C1mEXSwrFs-_P6g+q=3WYBzHg@mail.gmail.com>
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)
>>>
>
next prev parent reply other threads:[~2021-06-25 5:37 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-10 11:24 [PATCH 0/6] More record/replay acceptance tests Pavel Dovgalyuk
2021-06-10 11:24 ` [PATCH 1/6] tests/acceptance: add replay kernel test for s390 Pavel Dovgalyuk
2021-06-22 19:19 ` Willian Rampazzo
2021-06-10 11:24 ` [PATCH 2/6] tests/acceptance: add replay kernel test for ppc64 Pavel Dovgalyuk
2021-06-22 19:23 ` Willian Rampazzo
2021-06-23 6:49 ` Pavel Dovgalyuk
2021-06-10 11:24 ` [PATCH 3/6] tests/acceptance: add replay kernel test for openrisc Pavel Dovgalyuk
2021-06-22 19:26 ` Willian Rampazzo
2021-06-10 11:25 ` [PATCH 4/6] tests/acceptance: add replay kernel test for nios2 Pavel Dovgalyuk
2021-06-22 19:27 ` Willian Rampazzo
2021-06-10 11:25 ` [PATCH 5/6] tests/acceptance: add replay kernel test for alpha Pavel Dovgalyuk
2021-06-22 19:44 ` Willian Rampazzo
2021-06-10 11:25 ` [PATCH 6/6] tests/acceptance: Linux boot test for record/replay Pavel Dovgalyuk
2021-06-23 18:45 ` Willian Rampazzo
2021-06-24 18:15 ` Willian Rampazzo
2021-06-25 5:36 ` Pavel Dovgalyuk [this message]
2021-06-22 6:52 ` [PATCH 0/6] More record/replay acceptance tests Pavel Dovgalyuk
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a5de555b-e7f5-09a5-422d-f71855a269e6@ispras.ru \
--to=pavel.dovgalyuk@ispras.ru \
--cc=alex.bennee@linaro.org \
--cc=crosa@redhat.com \
--cc=pbonzini@redhat.com \
--cc=philmd@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=wrampazz@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).