[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v1 6/6] tests/acceptance: pick a random gdb port for reverse
From: |
Cleber Rosa |
Subject: |
Re: [PATCH v1 6/6] tests/acceptance: pick a random gdb port for reverse debugging |
Date: |
Thu, 22 Oct 2020 11:46:58 -0400 |
On Wed, Oct 21, 2020 at 05:31:36PM +0100, Alex Bennée wrote:
> Currently the test randomly fails if you are using a shared machine
> due to contention on the well known port 1234. We can ameliorate this
> a bit by picking a random non-ephemeral port although it doesn't
> totally avoid the problem. While we could use a totally unique socket
> address for debugging it's impossible to probe for gdb support of the
> feature which makes this a sub-optimal but less fiddly option.
>
Hi Alex,
This is already a clear improvement, so consider my points as suggestions.
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> tests/acceptance/reverse_debugging.py | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tests/acceptance/reverse_debugging.py
> b/tests/acceptance/reverse_debugging.py
> index b72fdf6cdc..f2e8245471 100644
> --- a/tests/acceptance/reverse_debugging.py
> +++ b/tests/acceptance/reverse_debugging.py
> @@ -16,6 +16,7 @@ from avocado.utils import gdb
> from avocado.utils import process
> from avocado.utils.path import find_command
> from boot_linux_console import LinuxKernelTest
> +from random import randrange
Avocado ships with a "avocado.utils.network.ports.find_free_port" utility:
https://avocado-framework.readthedocs.io/en/81.0/api/utils/avocado.utils.network.html#avocado.utils.network.ports.find_free_port
Which *minimizes* the possibility of a clash by checking if the port
is available. I think it's worth to consider using it.
>
> class ReverseDebugging(LinuxKernelTest):
> """
> @@ -43,7 +44,8 @@ class ReverseDebugging(LinuxKernelTest):
> else:
> logger.info('replaying the execution...')
> mode = 'replay'
> - vm.add_args('-s', '-S')
> + self.port = randrange(2048, 49152)
> + vm.add_args('-gdb', 'tcp::%d' % (self.port), '-S')
It's a good idea to try to avoid setting instance attributes outside
of __init__(). In this specific case, I'd just add a "port" parameter
to run_vm().
> vm.add_args('-icount', 'shift=%s,rr=%s,rrfile=%s,rrsnapshot=init' %
> (shift, mode, replay_path),
> '-net', 'none')
> @@ -122,7 +124,7 @@ class ReverseDebugging(LinuxKernelTest):
> # replay and run debug commands
> vm = self.run_vm(False, shift, args, replay_path, image_path)
> logger.info('connecting to gdbstub')
> - g = gdb.GDBRemote('127.0.0.1', 1234, False, False)
> + g = gdb.GDBRemote('127.0.0.1', self.port, False, False)
> g.connect()
> r = g.cmd(b'qSupported')
> if b'qXfer:features:read+' in r:
> --
> 2.20.1
>
So, the overall diff of my suggestions look like:
---
diff --git a/tests/acceptance/reverse_debugging.py
b/tests/acceptance/reverse_debugging.py
index f2e8245471..3d91dfaa8f 100644
--- a/tests/acceptance/reverse_debugging.py
+++ b/tests/acceptance/reverse_debugging.py
@@ -14,9 +14,10 @@ from avocado import skipIf
from avocado_qemu import BUILD_DIR
from avocado.utils import gdb
from avocado.utils import process
+from avocado.utils.network.ports import find_free_port
from avocado.utils.path import find_command
from boot_linux_console import LinuxKernelTest
-from random import randrange
+
class ReverseDebugging(LinuxKernelTest):
"""
@@ -34,7 +35,7 @@ class ReverseDebugging(LinuxKernelTest):
STEPS = 10
endian_is_le = True
- def run_vm(self, record, shift, args, replay_path, image_path):
+ def run_vm(self, record, shift, args, replay_path, image_path, port):
logger = logging.getLogger('replay')
vm = self.get_vm()
vm.set_console()
@@ -44,8 +45,7 @@ class ReverseDebugging(LinuxKernelTest):
else:
logger.info('replaying the execution...')
mode = 'replay'
- self.port = randrange(2048, 49152)
- vm.add_args('-gdb', 'tcp::%d' % (self.port), '-S')
+ vm.add_args('-gdb', 'tcp::%d' % port, '-S')
vm.add_args('-icount', 'shift=%s,rr=%s,rrfile=%s,rrsnapshot=init' %
(shift, mode, replay_path),
'-net', 'none')
@@ -111,9 +111,10 @@ class ReverseDebugging(LinuxKernelTest):
process.run(cmd)
replay_path = os.path.join(self.workdir, 'replay.bin')
+ port = find_free_port()
# record the log
- vm = self.run_vm(True, shift, args, replay_path, image_path)
+ vm = self.run_vm(True, shift, args, replay_path, image_path, port)
while self.vm_get_icount(vm) <= self.STEPS:
pass
last_icount = self.vm_get_icount(vm)
@@ -122,9 +123,9 @@ class ReverseDebugging(LinuxKernelTest):
logger.info("recorded log with %s+ steps" % last_icount)
# replay and run debug commands
- vm = self.run_vm(False, shift, args, replay_path, image_path)
+ vm = self.run_vm(False, shift, args, replay_path, image_path, port)
logger.info('connecting to gdbstub')
- g = gdb.GDBRemote('127.0.0.1', self.port, False, False)
+ g = gdb.GDBRemote('127.0.0.1', port, False, False)
g.connect()
r = g.cmd(b'qSupported')
if b'qXfer:features:read+' in r:
---
Regards,
- Cleber.
signature.asc
Description: PGP signature
- [PATCH v1 0/6] testing/next (gitdm, acceptance, docker, gitlab), Alex Bennée, 2020/10/21
- [PATCH v1 1/6] Adding ani's email as an individual contributor, Alex Bennée, 2020/10/21
- [PATCH v1 2/6] contrib/gitdm: Add more individual contributors, Alex Bennée, 2020/10/21
- [PATCH v1 5/6] scripts: fix error from checkpatch.pl when no commits are found, Alex Bennée, 2020/10/21
- [PATCH v1 6/6] tests/acceptance: pick a random gdb port for reverse debugging, Alex Bennée, 2020/10/21
- Re: [PATCH v1 6/6] tests/acceptance: pick a random gdb port for reverse debugging, Pavel Dovgalyuk, 2020/10/22
- Re: [PATCH v1 6/6] tests/acceptance: pick a random gdb port for reverse debugging,
Cleber Rosa <=
- [PATCH v1 4/6] gitlab: skip checkpatch.pl checks if no commit delta on branch, Alex Bennée, 2020/10/21
- [PATCH v1 3/6] tests/docker/dockerfiles/centos: Use SDL2 instead of SDL1, Alex Bennée, 2020/10/21