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.ben...@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
