qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/2] tests/functional: Adapt reverse_debugging to run w/o Avocado (yet another try)
@ 2025-09-15 12:42 Thomas Huth
  2025-09-15 12:42 ` [RFC PATCH 1/2] tests/functional: Provide GDB to the functional tests Thomas Huth
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Thomas Huth @ 2025-09-15 12:42 UTC (permalink / raw)
  To: Gustavo Romero, qemu-devel, Daniel P . Berrangé
  Cc: qemu-arm, Alex Bennée

Here's yet another attempt to remove the avocado dependency from the
reverse debugging tests: I basically took Gustavo's patches to rework
tests/functional/reverse_debugging.py, but instead of calling that
through tests/guest-debug/run-test.py and adding the cumbersome code
to support additional test execution logic, I kept our normal way of
running tests via pycotap. Instead, the essential logic for running
gdb is copied from tests/guest-debug/run-test.py into the new function
reverse_debug() that then runs gdb directly with using
tests/functional/reverse_debugging.py as the script.

Marked as an RFC since this still needs some love... The aarch64 test
seems to work already (after applying the fix for the reverse debug there
first: https://patchew.org/QEMU/20250603125459.17688-1-1844144@gmail.com/ ),
but the ppc64 and x86 tests are currently still completely broken.
Also we currently log into two different folders this way, into
tests/functional/aarch64/test_reverse_debug.ReverseDebugging_AArch64.test_aarch64_virt
for the normal outer test, and into
tests/functional/aarch64/reverse_debugging.ReverseDebugging.test_reverse_debugging
for the script that is run in gdb ... it's likely ok for the aarch64
test, but the ppc64 test contains two subtests, so we need to come up
with a better solution here for the final implementation.

Gustavo Romero (2):
  tests/functional: Provide GDB to the functional tests
  tests/functional: Adapt reverse_debugging to run w/o Avocado

 configure                                     |   2 +
 meson.build                                   |   4 +
 meson_options.txt                             |   2 +
 scripts/meson-buildoptions.sh                 |   2 +
 .../functional/aarch64/test_reverse_debug.py  |  16 +-
 tests/functional/meson.build                  |   7 +
 tests/functional/ppc64/test_reverse_debug.py  |  18 +-
 tests/functional/reverse_debugging.py         | 235 +++++++++++-------
 tests/functional/x86_64/test_reverse_debug.py |  20 +-
 9 files changed, 188 insertions(+), 118 deletions(-)

-- 
2.51.0



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [RFC PATCH 1/2] tests/functional: Provide GDB to the functional tests
  2025-09-15 12:42 [RFC PATCH 0/2] tests/functional: Adapt reverse_debugging to run w/o Avocado (yet another try) Thomas Huth
@ 2025-09-15 12:42 ` Thomas Huth
  2025-09-15 16:11   ` Alex Bennée
  2025-09-15 22:02   ` Gustavo Romero
  2025-09-15 12:42 ` [RFC PATCH 2/2] tests/functional: Adapt reverse_debugging to run w/o Avocado Thomas Huth
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 15+ messages in thread
From: Thomas Huth @ 2025-09-15 12:42 UTC (permalink / raw)
  To: Gustavo Romero, qemu-devel, Daniel P . Berrangé
  Cc: qemu-arm, Alex Bennée

From: Gustavo Romero <gustavo.romero@linaro.org>

The probe of gdb is done in 'configure' and the full path is passed
to meson.build via the -Dgdb=option.

meson then can pass the location of gdb to the test via an environment
variable.

This patch is based on an earlier patch ("Support tests that require a
runner") by Gustavo Romero.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 configure                     | 2 ++
 meson.build                   | 4 ++++
 meson_options.txt             | 2 ++
 scripts/meson-buildoptions.sh | 2 ++
 tests/functional/meson.build  | 7 +++++++
 5 files changed, 17 insertions(+)

diff --git a/configure b/configure
index 274a7787642..8e2e2cd562a 100755
--- a/configure
+++ b/configure
@@ -1978,6 +1978,8 @@ if test "$skip_meson" = no; then
   test -n "${LIB_FUZZING_ENGINE+xxx}" && meson_option_add "-Dfuzzing_engine=$LIB_FUZZING_ENGINE"
   test "$plugins" = yes && meson_option_add "-Dplugins=true"
   test "$tcg" != enabled && meson_option_add "-Dtcg=$tcg"
+  test -n "$gdb_bin" && meson_option_add "-Dgdb=$gdb_bin"
+
   run_meson() {
     NINJA=$ninja $meson setup "$@" "$PWD" "$source_path"
   }
diff --git a/meson.build b/meson.build
index 3d738733566..4cbc3c8ac65 100644
--- a/meson.build
+++ b/meson.build
@@ -75,6 +75,10 @@ have_user = have_linux_user or have_bsd_user
 
 sh = find_program('sh')
 python = import('python').find_installation()
+# Meson python.get_path() on 'purelib' or 'platlib' doesn't properly return the
+# site-packages dir in pyvenv, so it is built manually.
+python_ver = python.language_version()
+python_site_packages = meson.build_root() / 'pyvenv/lib/python' + python_ver / 'site-packages'
 
 cc = meson.get_compiler('c')
 all_languages = ['c']
diff --git a/meson_options.txt b/meson_options.txt
index fff1521e580..5bb41bcbc43 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -36,6 +36,8 @@ option('trace_file', type: 'string', value: 'trace',
 option('coroutine_backend', type: 'combo',
        choices: ['ucontext', 'sigaltstack', 'windows', 'wasm', 'auto'],
        value: 'auto', description: 'coroutine backend to use')
+option('gdb', type: 'string', value: '',
+       description: 'Path to GDB')
 
 # Everything else can be set via --enable/--disable-* option
 # on the configure script command line.  After adding an option
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index 0ebe6bc52a6..f4bd21220ee 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -58,6 +58,7 @@ meson_options_help() {
   printf "%s\n" '  --enable-ubsan           enable undefined behaviour sanitizer'
   printf "%s\n" '  --firmwarepath=VALUES    search PATH for firmware files [share/qemu-'
   printf "%s\n" '                           firmware]'
+  printf "%s\n" '  --gdb=VALUE              Path to GDB'
   printf "%s\n" '  --iasl=VALUE             Path to ACPI disassembler'
   printf "%s\n" '  --includedir=VALUE       Header file directory [include]'
   printf "%s\n" '  --interp-prefix=VALUE    where to find shared libraries etc., use %M for'
@@ -323,6 +324,7 @@ _meson_option_parse() {
     --disable-fuzzing) printf "%s" -Dfuzzing=false ;;
     --enable-gcrypt) printf "%s" -Dgcrypt=enabled ;;
     --disable-gcrypt) printf "%s" -Dgcrypt=disabled ;;
+    --gdb=*) quote_sh "-Dgdb=$2" ;;
     --enable-gettext) printf "%s" -Dgettext=enabled ;;
     --disable-gettext) printf "%s" -Dgettext=disabled ;;
     --enable-gio) printf "%s" -Dgio=enabled ;;
diff --git a/tests/functional/meson.build b/tests/functional/meson.build
index 2a0c5aa1418..c822eb66309 100644
--- a/tests/functional/meson.build
+++ b/tests/functional/meson.build
@@ -77,6 +77,12 @@ foreach speed : ['quick', 'thorough']
     test_env.set('PYTHONPATH', meson.project_source_root() / 'python:' +
                                meson.current_source_dir())
 
+    # Define the GDB environment variable if gdb is available.
+    gdb = get_option('gdb')
+    if gdb != ''
+      test_env.set('QEMU_TEST_GDB', gdb)
+    endif
+
     foreach test : target_tests
       testname = '@0@-@1@'.format(target_base, test)
       if fs.exists('generic' / 'test_' + test + '.py')
@@ -121,6 +127,7 @@ foreach speed : ['quick', 'thorough']
            priority: time_out,
            suite: suites)
     endforeach
+
   endforeach
 endforeach
 
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [RFC PATCH 2/2] tests/functional: Adapt reverse_debugging to run w/o Avocado
  2025-09-15 12:42 [RFC PATCH 0/2] tests/functional: Adapt reverse_debugging to run w/o Avocado (yet another try) Thomas Huth
  2025-09-15 12:42 ` [RFC PATCH 1/2] tests/functional: Provide GDB to the functional tests Thomas Huth
@ 2025-09-15 12:42 ` Thomas Huth
  2025-09-15 16:14   ` Alex Bennée
                     ` (2 more replies)
  2025-09-15 16:13 ` [RFC PATCH 0/2] tests/functional: Adapt reverse_debugging to run w/o Avocado (yet another try) Alex Bennée
                   ` (2 subsequent siblings)
  4 siblings, 3 replies; 15+ messages in thread
From: Thomas Huth @ 2025-09-15 12:42 UTC (permalink / raw)
  To: Gustavo Romero, qemu-devel, Daniel P . Berrangé
  Cc: qemu-arm, Alex Bennée

From: Gustavo Romero <gustavo.romero@linaro.org>

This commit removes Avocado as a dependency for running the
reverse_debugging test.

The main benefit, beyond eliminating an extra dependency, is that there
is no longer any need to handle GDB packets manually. This removes the
need for ad-hoc functions dealing with endianness and arch-specific
register numbers, making the test easier to read. The timeout variable
is also removed, since Meson now manages timeouts automatically.

The reverse_debugging test is now executed through running GDB via a
python script. The test itself is only responsible for invoking
GDB with the appropriate arguments and for passing the test script to
GDB.

reverse_debugging is kept "skipped" for aarch64, ppc64, and x86_64, so
won't run unless QEMU_TEST_FLAKY_TESTS=1 is set in the test environment,
before running 'make check-functional' or 'meson test [...]'.

Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
[thuth: Rework the test to run without tests/guest-debug/run-test.py]
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 .../functional/aarch64/test_reverse_debug.py  |  16 +-
 tests/functional/ppc64/test_reverse_debug.py  |  18 +-
 tests/functional/reverse_debugging.py         | 235 +++++++++++-------
 tests/functional/x86_64/test_reverse_debug.py |  20 +-
 4 files changed, 171 insertions(+), 118 deletions(-)

diff --git a/tests/functional/aarch64/test_reverse_debug.py b/tests/functional/aarch64/test_reverse_debug.py
index 85e35645db0..a84ddd07acb 100755
--- a/tests/functional/aarch64/test_reverse_debug.py
+++ b/tests/functional/aarch64/test_reverse_debug.py
@@ -2,7 +2,7 @@
 #
 # SPDX-License-Identifier: GPL-2.0-or-later
 #
-# Reverse debugging test
+# Reverse debugging test for aarch64
 #
 # Copyright (c) 2020 ISP RAS
 #
@@ -12,14 +12,13 @@
 # This work is licensed under the terms of the GNU GPL, version 2 or
 # later.  See the COPYING file in the top-level directory.
 
-from qemu_test import Asset, skipIfMissingImports, skipFlakyTest
-from reverse_debugging import ReverseDebugging
+from qemu_test import QemuSystemTest
+from qemu_test import Asset, skipFlakyTest
 
+from reverse_debugging import reverse_debug
 
-@skipIfMissingImports('avocado.utils')
-class ReverseDebugging_AArch64(ReverseDebugging):
 
-    REG_PC = 32
+class ReverseDebugging_AArch64(QemuSystemTest):
 
     ASSET_KERNEL = Asset(
         ('https://archives.fedoraproject.org/pub/archive/fedora/linux/'
@@ -29,9 +28,8 @@ class ReverseDebugging_AArch64(ReverseDebugging):
     def test_aarch64_virt(self):
         self.set_machine('virt')
         self.cpu = 'cortex-a53'
-        kernel_path = self.ASSET_KERNEL.fetch()
-        self.reverse_debugging(args=('-kernel', kernel_path))
+        reverse_debug(self, self.ASSET_KERNEL)
 
 
 if __name__ == '__main__':
-    ReverseDebugging.main()
+    QemuSystemTest.main()
diff --git a/tests/functional/ppc64/test_reverse_debug.py b/tests/functional/ppc64/test_reverse_debug.py
index 5931adef5a9..7da5ede06c8 100755
--- a/tests/functional/ppc64/test_reverse_debug.py
+++ b/tests/functional/ppc64/test_reverse_debug.py
@@ -2,7 +2,7 @@
 #
 # SPDX-License-Identifier: GPL-2.0-or-later
 #
-# Reverse debugging test
+# Reverse debugging test for ppc64
 #
 # Copyright (c) 2020 ISP RAS
 #
@@ -12,14 +12,12 @@
 # This work is licensed under the terms of the GNU GPL, version 2 or
 # later.  See the COPYING file in the top-level directory.
 
-from qemu_test import skipIfMissingImports, skipFlakyTest
-from reverse_debugging import ReverseDebugging
+from qemu_test import QemuSystemTest, skipFlakyTest
 
+from reverse_debugging import reverse_debug
 
-@skipIfMissingImports('avocado.utils')
-class ReverseDebugging_ppc64(ReverseDebugging):
 
-    REG_PC = 0x40
+class ReverseDebugging_ppc64(QemuSystemTest):
 
     @skipFlakyTest("https://gitlab.com/qemu-project/qemu/-/issues/1992")
     def test_ppc64_pseries(self):
@@ -27,15 +25,13 @@ def test_ppc64_pseries(self):
         # SLOF branches back to its entry point, which causes this test
         # to take the 'hit a breakpoint again' path. That's not a problem,
         # just slightly different than the other machines.
-        self.endian_is_le = False
-        self.reverse_debugging()
+        reverse_debug(self)
 
     @skipFlakyTest("https://gitlab.com/qemu-project/qemu/-/issues/1992")
     def test_ppc64_powernv(self):
         self.set_machine('powernv')
-        self.endian_is_le = False
-        self.reverse_debugging()
+        reverse_debug(self)
 
 
 if __name__ == '__main__':
-    ReverseDebugging.main()
+    QemuSystemTest.main()
diff --git a/tests/functional/reverse_debugging.py b/tests/functional/reverse_debugging.py
index f9a1d395f1d..c889247defa 100644
--- a/tests/functional/reverse_debugging.py
+++ b/tests/functional/reverse_debugging.py
@@ -1,21 +1,72 @@
-# Reverse debugging test
-#
 # SPDX-License-Identifier: GPL-2.0-or-later
 #
+# Reverse debugging test
+#
 # Copyright (c) 2020 ISP RAS
+# Copyright (c) 2025 Linaro Limited
 #
 # Author:
 #  Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
+#  Gustavo Romero <gustavo.romero@linaro.org> (Run without Avocado)
 #
 # 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 os
+import subprocess
+import sys
+
+try:
+    import gdb
+    _has_gdb = True
+except ModuleNotFoundError:
+    _has_gdb = False
 
 from qemu_test import LinuxKernelTest, get_qemu_img
 from qemu_test.ports import Ports
 
 
+def reverse_debug(test, asset_kernel=None, shift=7, args=None):
+
+    # Now launch gdb with our test and collect the result.
+    gdb_cmd = os.getenv('QEMU_TEST_GDB')
+    assert(gdb_cmd)
+
+    # Run quietly and ignore .gdbinit.
+    gdb_cmd += " -q -n -batch"
+    # Disable pagination.
+    gdb_cmd += " -ex 'set pagination off'"
+    # Disable prompts in case of crash.
+    gdb_cmd += " -ex 'set confirm off'"
+    # Finally the test script itself.
+    argv = [__file__]
+    gdb_cmd += f" -ex \"py sys.argv={argv}\""
+    gdb_cmd += " -x %s" % __file__
+
+    test.log.info("GDB CMD: %s" % gdb_cmd)
+
+    gdb_env = dict(os.environ)
+    gdb_pythonpath = gdb_env.get("PYTHONPATH", "").split(os.pathsep)
+    gdb_pythonpath.append(os.path.dirname(os.path.realpath(__file__)))
+    gdb_env["PYTHONPATH"] = os.pathsep.join(gdb_pythonpath)
+    gdb_env["QEMU_TEST_MACHINE"] = test.machine
+    if test.cpu:
+        gdb_env["QEMU_TEST_CPU"] = test.cpu
+    if asset_kernel:
+        gdb_env["QEMU_TEST_KERNEL"] = asset_kernel.fetch()
+    result = subprocess.run(gdb_cmd, shell=True, check=False,
+                            stdout=subprocess.PIPE,
+                            stderr=subprocess.STDOUT,
+                            encoding='utf8',
+                            env=gdb_env)
+    test.log.info("gdb output:\n %s" % result.stdout)
+    if result.returncode != 0:
+        test.fail(f"gdb failed with return code {result.returncode}")
+    else:
+        test.log.info("gdb run succeeded!")
+
+
 class ReverseDebugging(LinuxKernelTest):
     """
     Test GDB reverse debugging commands: reverse step and reverse continue.
@@ -28,13 +79,9 @@ class ReverseDebugging(LinuxKernelTest):
     that the execution is stopped at the last of them.
     """
 
-    timeout = 10
     STEPS = 10
-    endian_is_le = True
 
     def run_vm(self, record, shift, args, replay_path, image_path, port):
-        from avocado.utils import datadrainer
-
         logger = logging.getLogger('replay')
         vm = self.get_vm(name='record' if record else 'replay')
         vm.set_console()
@@ -52,59 +99,58 @@ def run_vm(self, record, shift, args, replay_path, image_path, port):
         if args:
             vm.add_args(*args)
         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()
+
         return vm
 
     @staticmethod
-    def get_reg_le(g, reg):
-        res = g.cmd(b'p%x' % reg)
-        num = 0
-        for i in range(len(res))[-2::-2]:
-            num = 0x100 * num + int(res[i:i + 2], 16)
-        return num
+    def gdb_connect(host, port):
+        # Set debug on connection to get the qSupport string.
+        gdb.execute("set debug remote 1")
+        r = gdb.execute(f"target remote {host}:{port}", False, True)
+        gdb.execute("set debug remote 0")
+
+        return r
 
     @staticmethod
-    def get_reg_be(g, reg):
-        res = g.cmd(b'p%x' % reg)
-        return int(res, 16)
-
-    def get_reg(self, g, reg):
-        # value may be encoded in BE or LE order
-        if self.endian_is_le:
-            return self.get_reg_le(g, reg)
-        else:
-            return self.get_reg_be(g, reg)
+    def get_pc():
+        val = gdb.parse_and_eval("$pc")
+        pc = int(val)
 
-    def get_pc(self, g):
-        return self.get_reg(g, self.REG_PC)
+        return pc
 
-    def check_pc(self, g, addr):
-        pc = self.get_pc(g)
+    def check_pc(self, addr):
+        logger = logging.getLogger('reply')
+        pc = self.get_pc()
         if pc != addr:
-            self.fail('Invalid PC (read %x instead of %x)' % (pc, addr))
+            logger.info('Invalid PC (read %x instead of %x)' % (pc, addr))
+            gdb.execute("exit 1")
 
     @staticmethod
-    def gdb_step(g):
-        g.cmd(b's', b'T05thread:01;')
+    def gdb_step():
+        gdb.execute("stepi")
 
     @staticmethod
-    def gdb_bstep(g):
-        g.cmd(b'bs', b'T05thread:01;')
+    def gdb_bstep():
+        gdb.execute("reverse-stepi")
 
     @staticmethod
     def vm_get_icount(vm):
         return vm.qmp('query-replay')['return']['icount']
 
-    def reverse_debugging(self, shift=7, args=None):
-        from avocado.utils import gdb
-        from avocado.utils import process
+    def test_reverse_debugging(self):
+
+        shift = 7
+
+        self.set_machine(os.getenv('QEMU_TEST_MACHINE'))
+        self.cpu = os.getenv('QEMU_TEST_CPU')
+        kernel_path = os.getenv('QEMU_TEST_KERNEL')
+        args = None
+        if kernel_path:
+            args = ['-kernel', kernel_path]
 
         logger = logging.getLogger('replay')
 
-        # create qcow2 for snapshots
+        # Create qcow2 for snapshots
         logger.info('creating qcow2 image for VM snapshots')
         image_path = os.path.join(self.workdir, 'disk.qcow2')
         qemu_img = get_qemu_img(self)
@@ -112,11 +158,11 @@ def reverse_debugging(self, shift=7, args=None):
             self.skipTest('Could not find "qemu-img", which is required to '
                           'create the temporary qcow2 image')
         cmd = '%s create -f qcow2 %s 128M' % (qemu_img, image_path)
-        process.run(cmd)
+        subprocess.run(cmd, shell=True)
 
         replay_path = os.path.join(self.workdir, 'replay.bin')
 
-        # record the log
+        # Record the log.
         vm = self.run_vm(True, shift, args, replay_path, image_path, -1)
         while self.vm_get_icount(vm) <= self.STEPS:
             pass
@@ -125,72 +171,91 @@ def reverse_debugging(self, shift=7, args=None):
 
         logger.info("recorded log with %s+ steps" % last_icount)
 
-        # replay and run debug commands
+        # Replay and run debug commands.
         with Ports() as ports:
             port = ports.find_free_port()
             vm = self.run_vm(False, shift, args, replay_path, image_path, port)
-        logger.info('connecting to gdbstub')
-        g = gdb.GDBRemote('127.0.0.1', port, False, False)
-        g.connect()
-        r = g.cmd(b'qSupported')
-        if b'qXfer:features:read+' in r:
-            g.cmd(b'qXfer:features:read:target.xml:0,ffb')
-        if b'ReverseStep+' not in r:
+        logger.info('Connecting to gdbstub')
+        r = self.gdb_connect('127.0.0.1', port)
+        if 'ReverseStep+' not in r:
             self.fail('Reverse step is not supported by QEMU')
-        if b'ReverseContinue+' not in r:
+        if 'ReverseContinue+' not in r:
             self.fail('Reverse continue is not supported by QEMU')
 
-        logger.info('stepping forward')
+        logger.info('Stepping forward')
         steps = []
-        # record first instruction addresses
+        # Record first instruction addresses.
         for _ in range(self.STEPS):
-            pc = self.get_pc(g)
-            logger.info('saving position %x' % pc)
+            pc = self.get_pc()
+            logger.info('Saving position %x' % pc)
             steps.append(pc)
-            self.gdb_step(g)
+            self.gdb_step()
 
-        # visit the recorded instruction in reverse order
-        logger.info('stepping backward')
+        # Visit the recorded instruction in reverse order.
+        logger.info('Stepping backward')
         for addr in steps[::-1]:
-            self.gdb_bstep(g)
-            self.check_pc(g, addr)
-            logger.info('found position %x' % addr)
+            self.gdb_bstep()
+            self.check_pc(addr)
+            logger.info('Found position %x' % addr)
 
-        # visit the recorded instruction in forward order
-        logger.info('stepping forward')
+        # Visit the recorded instruction in forward order.
+        logger.info('Stepping forward')
         for addr in steps:
-            self.check_pc(g, addr)
-            self.gdb_step(g)
-            logger.info('found position %x' % addr)
+            self.check_pc(addr)
+            self.gdb_step()
+            logger.info('Found position %x' % addr)
 
-        # set breakpoints for the instructions just stepped over
-        logger.info('setting breakpoints')
+        # Set breakpoints for the instructions just stepped over.
+        logger.info('Setting breakpoints')
         for addr in steps:
             # hardware breakpoint at addr with len=1
-            g.cmd(b'Z1,%x,1' % addr, b'OK')
+            gdb.execute(f"break *{hex(addr)}")
 
-        # this may hit a breakpoint if first instructions are executed
-        # again
-        logger.info('continuing execution')
+        # This may hit a breakpoint if first instructions are executed again.
+        logger.info('Continuing execution')
         vm.qmp('replay-break', icount=last_icount - 1)
-        # continue - will return after pausing
+        # continue - will return after pausing.
         # This could stop at the end and get a T02 return, or by
         # re-executing one of the breakpoints and get a T05 return.
-        g.cmd(b'c')
+        gdb.execute("continue")
         if self.vm_get_icount(vm) == last_icount - 1:
-            logger.info('reached the end (icount %s)' % (last_icount - 1))
+            logger.info('Reached the end (icount %s)' % (last_icount - 1))
         else:
-            logger.info('hit a breakpoint again at %x (icount %s)' %
-                        (self.get_pc(g), self.vm_get_icount(vm)))
+            logger.info('Hit a breakpoint again at %x (icount %s)' %
+                        (self.get_pc(), self.vm_get_icount(vm)))
 
-        logger.info('running reverse continue to reach %x' % steps[-1])
-        # reverse continue - will return after stopping at the breakpoint
-        g.cmd(b'bc', b'T05thread:01;')
+        logger.info('Running reverse continue to reach %x' % steps[-1])
+        # reverse continue - will return after stopping at the breakpoint.
+        gdb.execute("reverse-continue")
 
-        # assume that none of the first instructions is executed again
-        # breaking the order of the breakpoints
-        self.check_pc(g, steps[-1])
-        logger.info('successfully reached %x' % steps[-1])
+        # Assume that none of the first instructions is executed again
+        # breaking the order of the breakpoints.
+        # steps[-1] is the first saved $pc in reverse order.
+        self.check_pc(steps[-1])
+        logger.info('Successfully reached %x' % steps[-1])
 
-        logger.info('exiting gdb and qemu')
+        logger.info('Exiting GDB and QEMU...')
+        # Disconnect from the VM.
+        gdb.execute("disconnect")
+        # Guarantee VM is shutdown.
         vm.shutdown()
+        # Gently exit from GDB.
+        gdb.execute('print "test succeeded"')
+        gdb.execute("exit 0")
+
+    @staticmethod
+    def main():
+        try:
+            LinuxKernelTest.main()
+        except SystemExit:
+            # If the test is marked with @skipFlakyTest, then it will be exited
+            # via sys.exit() before we have the chance to exit from GDB gently.
+            # Because recent versions of GDB will return a failure value if this
+            # happens, we catch the SystemExit and exit from GDB gently with 77,
+            # which meson interprets correctly as a skipped test.
+            gdb.execute("exit 77")
+
+if __name__ == '__main__':
+    if not _has_gdb:
+        sys.exit("This script must be launched via tests/guest-debug/run-test.py!")
+    ReverseDebugging.main()
diff --git a/tests/functional/x86_64/test_reverse_debug.py b/tests/functional/x86_64/test_reverse_debug.py
index d713e91e144..e823f0d4953 100755
--- a/tests/functional/x86_64/test_reverse_debug.py
+++ b/tests/functional/x86_64/test_reverse_debug.py
@@ -2,7 +2,7 @@
 #
 # SPDX-License-Identifier: GPL-2.0-or-later
 #
-# Reverse debugging test
+# Reverse debugging test for x86_64
 #
 # Copyright (c) 2020 ISP RAS
 #
@@ -12,25 +12,19 @@
 # This work is licensed under the terms of the GNU GPL, version 2 or
 # later.  See the COPYING file in the top-level directory.
 
-from qemu_test import skipIfMissingImports, skipFlakyTest
-from reverse_debugging import ReverseDebugging
+from qemu_test import QemuSystemTest, skipFlakyTest
 
+from reverse_debugging import reverse_debug
 
-@skipIfMissingImports('avocado.utils')
-class ReverseDebugging_X86_64(ReverseDebugging):
 
-    REG_PC = 0x10
-    REG_CS = 0x12
-    def get_pc(self, g):
-        return self.get_reg_le(g, self.REG_PC) \
-            + self.get_reg_le(g, self.REG_CS) * 0x10
+class ReverseDebugging_X86_64(QemuSystemTest):
 
     @skipFlakyTest("https://gitlab.com/qemu-project/qemu/-/issues/2922")
     def test_x86_64_pc(self):
         self.set_machine('pc')
-        # start with BIOS only
-        self.reverse_debugging()
+        # Start with BIOS only
+        reverse_debug(self)
 
 
 if __name__ == '__main__':
-    ReverseDebugging.main()
+    QemuSystemTest.main()
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH 1/2] tests/functional: Provide GDB to the functional tests
  2025-09-15 12:42 ` [RFC PATCH 1/2] tests/functional: Provide GDB to the functional tests Thomas Huth
@ 2025-09-15 16:11   ` Alex Bennée
  2025-09-15 22:02   ` Gustavo Romero
  1 sibling, 0 replies; 15+ messages in thread
From: Alex Bennée @ 2025-09-15 16:11 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Gustavo Romero, qemu-devel, Daniel P . Berrangé, qemu-arm

Thomas Huth <thuth@redhat.com> writes:

> From: Gustavo Romero <gustavo.romero@linaro.org>
>
> The probe of gdb is done in 'configure' and the full path is passed
> to meson.build via the -Dgdb=option.
>
> meson then can pass the location of gdb to the test via an environment
> variable.
>
> This patch is based on an earlier patch ("Support tests that require a
> runner") by Gustavo Romero.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
<snip>
>      foreach test : target_tests
>        testname = '@0@-@1@'.format(target_base, test)
>        if fs.exists('generic' / 'test_' + test + '.py')
> @@ -121,6 +127,7 @@ foreach speed : ['quick', 'thorough']
>             priority: time_out,
>             suite: suites)
>      endforeach
> +

spare newline?

>    endforeach
>  endforeach

Otherwise:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH 0/2] tests/functional: Adapt reverse_debugging to run w/o Avocado (yet another try)
  2025-09-15 12:42 [RFC PATCH 0/2] tests/functional: Adapt reverse_debugging to run w/o Avocado (yet another try) Thomas Huth
  2025-09-15 12:42 ` [RFC PATCH 1/2] tests/functional: Provide GDB to the functional tests Thomas Huth
  2025-09-15 12:42 ` [RFC PATCH 2/2] tests/functional: Adapt reverse_debugging to run w/o Avocado Thomas Huth
@ 2025-09-15 16:13 ` Alex Bennée
  2025-09-15 16:18   ` Thomas Huth
  2025-09-15 22:02 ` Gustavo Romero
  2025-09-16  9:15 ` Daniel P. Berrangé
  4 siblings, 1 reply; 15+ messages in thread
From: Alex Bennée @ 2025-09-15 16:13 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Gustavo Romero, qemu-devel, Daniel P . Berrangé, qemu-arm

Thomas Huth <thuth@redhat.com> writes:

> Here's yet another attempt to remove the avocado dependency from the
> reverse debugging tests: I basically took Gustavo's patches to rework
> tests/functional/reverse_debugging.py, but instead of calling that
> through tests/guest-debug/run-test.py and adding the cumbersome code
> to support additional test execution logic, I kept our normal way of
> running tests via pycotap.

Hmm I was getting:

  2025-09-15 17:10:50,798 - INFO: GDB CMD: /home/alex/src/tools/binutils-gdb.git/builds/all/install/bin/gdb -q -n -batch -ex 'set pagination off' -ex 'set confirm off' -ex "py sys.argv=['/home/alex/lsrc/qemu.git/tests/functional/reverse_debugging.py']" -x /home/alex/lsrc/qemu.git/tests/functional/reverse_debugging.py
  2025-09-15 17:10:50,803 - DEBUG: Using cached asset /home/alex/.cache/qemu/download/7e1430b81c26bdd0da025eeb8fbd77b5dc961da4364af26e771bd39f379cbbf7 for https://archives.fedoraproject.org/pub/archive/fedora/linux/releases/29/Everything/aarch64/os/images/pxeboot/vmlinuz
  2025-09-15 17:10:50,891 - INFO: gdb output:
   Python Exception <class 'ModuleNotFoundError'>: No module named 'pycotap'
  Error occurred in Python: No module named 'pycotap'

which asserted. Should gdb be seeing the pyenv PYTHONPATH or should we
add a check for python3-pycotap in configure?

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH 2/2] tests/functional: Adapt reverse_debugging to run w/o Avocado
  2025-09-15 12:42 ` [RFC PATCH 2/2] tests/functional: Adapt reverse_debugging to run w/o Avocado Thomas Huth
@ 2025-09-15 16:14   ` Alex Bennée
  2025-09-15 22:02   ` Gustavo Romero
  2025-09-16  9:22   ` Daniel P. Berrangé
  2 siblings, 0 replies; 15+ messages in thread
From: Alex Bennée @ 2025-09-15 16:14 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Gustavo Romero, qemu-devel, Daniel P . Berrangé, qemu-arm

Thomas Huth <thuth@redhat.com> writes:

> From: Gustavo Romero <gustavo.romero@linaro.org>
>
> This commit removes Avocado as a dependency for running the
> reverse_debugging test.
>
> The main benefit, beyond eliminating an extra dependency, is that there
> is no longer any need to handle GDB packets manually. This removes the
> need for ad-hoc functions dealing with endianness and arch-specific
> register numbers, making the test easier to read. The timeout variable
> is also removed, since Meson now manages timeouts automatically.
>
> The reverse_debugging test is now executed through running GDB via a
> python script. The test itself is only responsible for invoking
> GDB with the appropriate arguments and for passing the test script to
> GDB.
>
> reverse_debugging is kept "skipped" for aarch64, ppc64, and x86_64, so
> won't run unless QEMU_TEST_FLAKY_TESTS=1 is set in the test environment,
> before running 'make check-functional' or 'meson test [...]'.
>
> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> [thuth: Rework the test to run without tests/guest-debug/run-test.py]
> Signed-off-by: Thomas Huth <thuth@redhat.com>

I certainly prefer this approach to importing the python modules. Caveat
to proper splitting but you can at least have:

Tested-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  .../functional/aarch64/test_reverse_debug.py  |  16 +-
>  tests/functional/ppc64/test_reverse_debug.py  |  18 +-
>  tests/functional/reverse_debugging.py         | 235 +++++++++++-------
>  tests/functional/x86_64/test_reverse_debug.py |  20 +-
>  4 files changed, 171 insertions(+), 118 deletions(-)
>
> diff --git a/tests/functional/aarch64/test_reverse_debug.py b/tests/functional/aarch64/test_reverse_debug.py
> index 85e35645db0..a84ddd07acb 100755
> --- a/tests/functional/aarch64/test_reverse_debug.py
> +++ b/tests/functional/aarch64/test_reverse_debug.py
> @@ -2,7 +2,7 @@
>  #
>  # SPDX-License-Identifier: GPL-2.0-or-later
>  #
> -# Reverse debugging test
> +# Reverse debugging test for aarch64
>  #
>  # Copyright (c) 2020 ISP RAS
>  #
> @@ -12,14 +12,13 @@
>  # This work is licensed under the terms of the GNU GPL, version 2 or
>  # later.  See the COPYING file in the top-level directory.
>  
> -from qemu_test import Asset, skipIfMissingImports, skipFlakyTest
> -from reverse_debugging import ReverseDebugging
> +from qemu_test import QemuSystemTest
> +from qemu_test import Asset, skipFlakyTest
>  
> +from reverse_debugging import reverse_debug
>  
> -@skipIfMissingImports('avocado.utils')
> -class ReverseDebugging_AArch64(ReverseDebugging):
>  
> -    REG_PC = 32
> +class ReverseDebugging_AArch64(QemuSystemTest):
>  
>      ASSET_KERNEL = Asset(
>          ('https://archives.fedoraproject.org/pub/archive/fedora/linux/'
> @@ -29,9 +28,8 @@ class ReverseDebugging_AArch64(ReverseDebugging):
>      def test_aarch64_virt(self):
>          self.set_machine('virt')
>          self.cpu = 'cortex-a53'
> -        kernel_path = self.ASSET_KERNEL.fetch()
> -        self.reverse_debugging(args=('-kernel', kernel_path))
> +        reverse_debug(self, self.ASSET_KERNEL)
>  
>  
>  if __name__ == '__main__':
> -    ReverseDebugging.main()
> +    QemuSystemTest.main()
> diff --git a/tests/functional/ppc64/test_reverse_debug.py b/tests/functional/ppc64/test_reverse_debug.py
> index 5931adef5a9..7da5ede06c8 100755
> --- a/tests/functional/ppc64/test_reverse_debug.py
> +++ b/tests/functional/ppc64/test_reverse_debug.py
> @@ -2,7 +2,7 @@
>  #
>  # SPDX-License-Identifier: GPL-2.0-or-later
>  #
> -# Reverse debugging test
> +# Reverse debugging test for ppc64
>  #
>  # Copyright (c) 2020 ISP RAS
>  #
> @@ -12,14 +12,12 @@
>  # This work is licensed under the terms of the GNU GPL, version 2 or
>  # later.  See the COPYING file in the top-level directory.
>  
> -from qemu_test import skipIfMissingImports, skipFlakyTest
> -from reverse_debugging import ReverseDebugging
> +from qemu_test import QemuSystemTest, skipFlakyTest
>  
> +from reverse_debugging import reverse_debug
>  
> -@skipIfMissingImports('avocado.utils')
> -class ReverseDebugging_ppc64(ReverseDebugging):
>  
> -    REG_PC = 0x40
> +class ReverseDebugging_ppc64(QemuSystemTest):
>  
>      @skipFlakyTest("https://gitlab.com/qemu-project/qemu/-/issues/1992")
>      def test_ppc64_pseries(self):
> @@ -27,15 +25,13 @@ def test_ppc64_pseries(self):
>          # SLOF branches back to its entry point, which causes this test
>          # to take the 'hit a breakpoint again' path. That's not a problem,
>          # just slightly different than the other machines.
> -        self.endian_is_le = False
> -        self.reverse_debugging()
> +        reverse_debug(self)
>  
>      @skipFlakyTest("https://gitlab.com/qemu-project/qemu/-/issues/1992")
>      def test_ppc64_powernv(self):
>          self.set_machine('powernv')
> -        self.endian_is_le = False
> -        self.reverse_debugging()
> +        reverse_debug(self)
>  
>  
>  if __name__ == '__main__':
> -    ReverseDebugging.main()
> +    QemuSystemTest.main()
> diff --git a/tests/functional/reverse_debugging.py b/tests/functional/reverse_debugging.py
> index f9a1d395f1d..c889247defa 100644
> --- a/tests/functional/reverse_debugging.py
> +++ b/tests/functional/reverse_debugging.py
> @@ -1,21 +1,72 @@
> -# Reverse debugging test
> -#
>  # SPDX-License-Identifier: GPL-2.0-or-later
>  #
> +# Reverse debugging test
> +#
>  # Copyright (c) 2020 ISP RAS
> +# Copyright (c) 2025 Linaro Limited
>  #
>  # Author:
>  #  Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
> +#  Gustavo Romero <gustavo.romero@linaro.org> (Run without Avocado)
>  #
>  # 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 os
> +import subprocess
> +import sys
> +
> +try:
> +    import gdb
> +    _has_gdb = True
> +except ModuleNotFoundError:
> +    _has_gdb = False
>  
>  from qemu_test import LinuxKernelTest, get_qemu_img
>  from qemu_test.ports import Ports
>  
>  
> +def reverse_debug(test, asset_kernel=None, shift=7, args=None):
> +
> +    # Now launch gdb with our test and collect the result.
> +    gdb_cmd = os.getenv('QEMU_TEST_GDB')
> +    assert(gdb_cmd)
> +
> +    # Run quietly and ignore .gdbinit.
> +    gdb_cmd += " -q -n -batch"
> +    # Disable pagination.
> +    gdb_cmd += " -ex 'set pagination off'"
> +    # Disable prompts in case of crash.
> +    gdb_cmd += " -ex 'set confirm off'"
> +    # Finally the test script itself.
> +    argv = [__file__]
> +    gdb_cmd += f" -ex \"py sys.argv={argv}\""
> +    gdb_cmd += " -x %s" % __file__
> +
> +    test.log.info("GDB CMD: %s" % gdb_cmd)
> +
> +    gdb_env = dict(os.environ)
> +    gdb_pythonpath = gdb_env.get("PYTHONPATH", "").split(os.pathsep)
> +    gdb_pythonpath.append(os.path.dirname(os.path.realpath(__file__)))
> +    gdb_env["PYTHONPATH"] = os.pathsep.join(gdb_pythonpath)
> +    gdb_env["QEMU_TEST_MACHINE"] = test.machine
> +    if test.cpu:
> +        gdb_env["QEMU_TEST_CPU"] = test.cpu
> +    if asset_kernel:
> +        gdb_env["QEMU_TEST_KERNEL"] = asset_kernel.fetch()
> +    result = subprocess.run(gdb_cmd, shell=True, check=False,
> +                            stdout=subprocess.PIPE,
> +                            stderr=subprocess.STDOUT,
> +                            encoding='utf8',
> +                            env=gdb_env)
> +    test.log.info("gdb output:\n %s" % result.stdout)
> +    if result.returncode != 0:
> +        test.fail(f"gdb failed with return code {result.returncode}")
> +    else:
> +        test.log.info("gdb run succeeded!")
> +
> +
>  class ReverseDebugging(LinuxKernelTest):
>      """
>      Test GDB reverse debugging commands: reverse step and reverse continue.
> @@ -28,13 +79,9 @@ class ReverseDebugging(LinuxKernelTest):
>      that the execution is stopped at the last of them.
>      """
>  
> -    timeout = 10
>      STEPS = 10
> -    endian_is_le = True
>  
>      def run_vm(self, record, shift, args, replay_path, image_path, port):
> -        from avocado.utils import datadrainer
> -
>          logger = logging.getLogger('replay')
>          vm = self.get_vm(name='record' if record else 'replay')
>          vm.set_console()
> @@ -52,59 +99,58 @@ def run_vm(self, record, shift, args, replay_path, image_path, port):
>          if args:
>              vm.add_args(*args)
>          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()
> +
>          return vm
>  
>      @staticmethod
> -    def get_reg_le(g, reg):
> -        res = g.cmd(b'p%x' % reg)
> -        num = 0
> -        for i in range(len(res))[-2::-2]:
> -            num = 0x100 * num + int(res[i:i + 2], 16)
> -        return num
> +    def gdb_connect(host, port):
> +        # Set debug on connection to get the qSupport string.
> +        gdb.execute("set debug remote 1")
> +        r = gdb.execute(f"target remote {host}:{port}", False, True)
> +        gdb.execute("set debug remote 0")
> +
> +        return r
>  
>      @staticmethod
> -    def get_reg_be(g, reg):
> -        res = g.cmd(b'p%x' % reg)
> -        return int(res, 16)
> -
> -    def get_reg(self, g, reg):
> -        # value may be encoded in BE or LE order
> -        if self.endian_is_le:
> -            return self.get_reg_le(g, reg)
> -        else:
> -            return self.get_reg_be(g, reg)
> +    def get_pc():
> +        val = gdb.parse_and_eval("$pc")
> +        pc = int(val)
>  
> -    def get_pc(self, g):
> -        return self.get_reg(g, self.REG_PC)
> +        return pc
>  
> -    def check_pc(self, g, addr):
> -        pc = self.get_pc(g)
> +    def check_pc(self, addr):
> +        logger = logging.getLogger('reply')
> +        pc = self.get_pc()
>          if pc != addr:
> -            self.fail('Invalid PC (read %x instead of %x)' % (pc, addr))
> +            logger.info('Invalid PC (read %x instead of %x)' % (pc, addr))
> +            gdb.execute("exit 1")
>  
>      @staticmethod
> -    def gdb_step(g):
> -        g.cmd(b's', b'T05thread:01;')
> +    def gdb_step():
> +        gdb.execute("stepi")
>  
>      @staticmethod
> -    def gdb_bstep(g):
> -        g.cmd(b'bs', b'T05thread:01;')
> +    def gdb_bstep():
> +        gdb.execute("reverse-stepi")
>  
>      @staticmethod
>      def vm_get_icount(vm):
>          return vm.qmp('query-replay')['return']['icount']
>  
> -    def reverse_debugging(self, shift=7, args=None):
> -        from avocado.utils import gdb
> -        from avocado.utils import process
> +    def test_reverse_debugging(self):
> +
> +        shift = 7
> +
> +        self.set_machine(os.getenv('QEMU_TEST_MACHINE'))
> +        self.cpu = os.getenv('QEMU_TEST_CPU')
> +        kernel_path = os.getenv('QEMU_TEST_KERNEL')
> +        args = None
> +        if kernel_path:
> +            args = ['-kernel', kernel_path]
>  
>          logger = logging.getLogger('replay')
>  
> -        # create qcow2 for snapshots
> +        # Create qcow2 for snapshots
>          logger.info('creating qcow2 image for VM snapshots')
>          image_path = os.path.join(self.workdir, 'disk.qcow2')
>          qemu_img = get_qemu_img(self)
> @@ -112,11 +158,11 @@ def reverse_debugging(self, shift=7, args=None):
>              self.skipTest('Could not find "qemu-img", which is required to '
>                            'create the temporary qcow2 image')
>          cmd = '%s create -f qcow2 %s 128M' % (qemu_img, image_path)
> -        process.run(cmd)
> +        subprocess.run(cmd, shell=True)
>  
>          replay_path = os.path.join(self.workdir, 'replay.bin')
>  
> -        # record the log
> +        # Record the log.
>          vm = self.run_vm(True, shift, args, replay_path, image_path, -1)
>          while self.vm_get_icount(vm) <= self.STEPS:
>              pass
> @@ -125,72 +171,91 @@ def reverse_debugging(self, shift=7, args=None):
>  
>          logger.info("recorded log with %s+ steps" % last_icount)
>  
> -        # replay and run debug commands
> +        # Replay and run debug commands.
>          with Ports() as ports:
>              port = ports.find_free_port()
>              vm = self.run_vm(False, shift, args, replay_path, image_path, port)
> -        logger.info('connecting to gdbstub')
> -        g = gdb.GDBRemote('127.0.0.1', port, False, False)
> -        g.connect()
> -        r = g.cmd(b'qSupported')
> -        if b'qXfer:features:read+' in r:
> -            g.cmd(b'qXfer:features:read:target.xml:0,ffb')
> -        if b'ReverseStep+' not in r:
> +        logger.info('Connecting to gdbstub')
> +        r = self.gdb_connect('127.0.0.1', port)
> +        if 'ReverseStep+' not in r:
>              self.fail('Reverse step is not supported by QEMU')
> -        if b'ReverseContinue+' not in r:
> +        if 'ReverseContinue+' not in r:
>              self.fail('Reverse continue is not supported by QEMU')
>  
> -        logger.info('stepping forward')
> +        logger.info('Stepping forward')
>          steps = []
> -        # record first instruction addresses
> +        # Record first instruction addresses.
>          for _ in range(self.STEPS):
> -            pc = self.get_pc(g)
> -            logger.info('saving position %x' % pc)
> +            pc = self.get_pc()
> +            logger.info('Saving position %x' % pc)
>              steps.append(pc)
> -            self.gdb_step(g)
> +            self.gdb_step()
>  
> -        # visit the recorded instruction in reverse order
> -        logger.info('stepping backward')
> +        # Visit the recorded instruction in reverse order.
> +        logger.info('Stepping backward')
>          for addr in steps[::-1]:
> -            self.gdb_bstep(g)
> -            self.check_pc(g, addr)
> -            logger.info('found position %x' % addr)
> +            self.gdb_bstep()
> +            self.check_pc(addr)
> +            logger.info('Found position %x' % addr)
>  
> -        # visit the recorded instruction in forward order
> -        logger.info('stepping forward')
> +        # Visit the recorded instruction in forward order.
> +        logger.info('Stepping forward')
>          for addr in steps:
> -            self.check_pc(g, addr)
> -            self.gdb_step(g)
> -            logger.info('found position %x' % addr)
> +            self.check_pc(addr)
> +            self.gdb_step()
> +            logger.info('Found position %x' % addr)
>  
> -        # set breakpoints for the instructions just stepped over
> -        logger.info('setting breakpoints')
> +        # Set breakpoints for the instructions just stepped over.
> +        logger.info('Setting breakpoints')
>          for addr in steps:
>              # hardware breakpoint at addr with len=1
> -            g.cmd(b'Z1,%x,1' % addr, b'OK')
> +            gdb.execute(f"break *{hex(addr)}")
>  
> -        # this may hit a breakpoint if first instructions are executed
> -        # again
> -        logger.info('continuing execution')
> +        # This may hit a breakpoint if first instructions are executed again.
> +        logger.info('Continuing execution')
>          vm.qmp('replay-break', icount=last_icount - 1)
> -        # continue - will return after pausing
> +        # continue - will return after pausing.
>          # This could stop at the end and get a T02 return, or by
>          # re-executing one of the breakpoints and get a T05 return.
> -        g.cmd(b'c')
> +        gdb.execute("continue")
>          if self.vm_get_icount(vm) == last_icount - 1:
> -            logger.info('reached the end (icount %s)' % (last_icount - 1))
> +            logger.info('Reached the end (icount %s)' % (last_icount - 1))
>          else:
> -            logger.info('hit a breakpoint again at %x (icount %s)' %
> -                        (self.get_pc(g), self.vm_get_icount(vm)))
> +            logger.info('Hit a breakpoint again at %x (icount %s)' %
> +                        (self.get_pc(), self.vm_get_icount(vm)))
>  
> -        logger.info('running reverse continue to reach %x' % steps[-1])
> -        # reverse continue - will return after stopping at the breakpoint
> -        g.cmd(b'bc', b'T05thread:01;')
> +        logger.info('Running reverse continue to reach %x' % steps[-1])
> +        # reverse continue - will return after stopping at the breakpoint.
> +        gdb.execute("reverse-continue")
>  
> -        # assume that none of the first instructions is executed again
> -        # breaking the order of the breakpoints
> -        self.check_pc(g, steps[-1])
> -        logger.info('successfully reached %x' % steps[-1])
> +        # Assume that none of the first instructions is executed again
> +        # breaking the order of the breakpoints.
> +        # steps[-1] is the first saved $pc in reverse order.
> +        self.check_pc(steps[-1])
> +        logger.info('Successfully reached %x' % steps[-1])
>  
> -        logger.info('exiting gdb and qemu')
> +        logger.info('Exiting GDB and QEMU...')
> +        # Disconnect from the VM.
> +        gdb.execute("disconnect")
> +        # Guarantee VM is shutdown.
>          vm.shutdown()
> +        # Gently exit from GDB.
> +        gdb.execute('print "test succeeded"')
> +        gdb.execute("exit 0")
> +
> +    @staticmethod
> +    def main():
> +        try:
> +            LinuxKernelTest.main()
> +        except SystemExit:
> +            # If the test is marked with @skipFlakyTest, then it will be exited
> +            # via sys.exit() before we have the chance to exit from GDB gently.
> +            # Because recent versions of GDB will return a failure value if this
> +            # happens, we catch the SystemExit and exit from GDB gently with 77,
> +            # which meson interprets correctly as a skipped test.
> +            gdb.execute("exit 77")
> +
> +if __name__ == '__main__':
> +    if not _has_gdb:
> +        sys.exit("This script must be launched via tests/guest-debug/run-test.py!")
> +    ReverseDebugging.main()
> diff --git a/tests/functional/x86_64/test_reverse_debug.py b/tests/functional/x86_64/test_reverse_debug.py
> index d713e91e144..e823f0d4953 100755
> --- a/tests/functional/x86_64/test_reverse_debug.py
> +++ b/tests/functional/x86_64/test_reverse_debug.py
> @@ -2,7 +2,7 @@
>  #
>  # SPDX-License-Identifier: GPL-2.0-or-later
>  #
> -# Reverse debugging test
> +# Reverse debugging test for x86_64
>  #
>  # Copyright (c) 2020 ISP RAS
>  #
> @@ -12,25 +12,19 @@
>  # This work is licensed under the terms of the GNU GPL, version 2 or
>  # later.  See the COPYING file in the top-level directory.
>  
> -from qemu_test import skipIfMissingImports, skipFlakyTest
> -from reverse_debugging import ReverseDebugging
> +from qemu_test import QemuSystemTest, skipFlakyTest
>  
> +from reverse_debugging import reverse_debug
>  
> -@skipIfMissingImports('avocado.utils')
> -class ReverseDebugging_X86_64(ReverseDebugging):
>  
> -    REG_PC = 0x10
> -    REG_CS = 0x12
> -    def get_pc(self, g):
> -        return self.get_reg_le(g, self.REG_PC) \
> -            + self.get_reg_le(g, self.REG_CS) * 0x10
> +class ReverseDebugging_X86_64(QemuSystemTest):
>  
>      @skipFlakyTest("https://gitlab.com/qemu-project/qemu/-/issues/2922")
>      def test_x86_64_pc(self):
>          self.set_machine('pc')
> -        # start with BIOS only
> -        self.reverse_debugging()
> +        # Start with BIOS only
> +        reverse_debug(self)
>  
>  
>  if __name__ == '__main__':
> -    ReverseDebugging.main()
> +    QemuSystemTest.main()

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH 0/2] tests/functional: Adapt reverse_debugging to run w/o Avocado (yet another try)
  2025-09-15 16:13 ` [RFC PATCH 0/2] tests/functional: Adapt reverse_debugging to run w/o Avocado (yet another try) Alex Bennée
@ 2025-09-15 16:18   ` Thomas Huth
  2025-09-15 18:27     ` Alex Bennée
  2025-09-15 22:03     ` Gustavo Romero
  0 siblings, 2 replies; 15+ messages in thread
From: Thomas Huth @ 2025-09-15 16:18 UTC (permalink / raw)
  To: Alex Bennée
  Cc: Gustavo Romero, qemu-devel, Daniel P.Berrangé, qemu-arm

On 15/09/2025 18.13, Alex Bennée wrote:
> Thomas Huth <thuth@redhat.com> writes:
> 
>> Here's yet another attempt to remove the avocado dependency from the
>> reverse debugging tests: I basically took Gustavo's patches to rework
>> tests/functional/reverse_debugging.py, but instead of calling that
>> through tests/guest-debug/run-test.py and adding the cumbersome code
>> to support additional test execution logic, I kept our normal way of
>> running tests via pycotap.
> 
> Hmm I was getting:
> 
>    2025-09-15 17:10:50,798 - INFO: GDB CMD: /home/alex/src/tools/binutils-gdb.git/builds/all/install/bin/gdb -q -n -batch -ex 'set pagination off' -ex 'set confirm off' -ex "py sys.argv=['/home/alex/lsrc/qemu.git/tests/functional/reverse_debugging.py']" -x /home/alex/lsrc/qemu.git/tests/functional/reverse_debugging.py
>    2025-09-15 17:10:50,803 - DEBUG: Using cached asset /home/alex/.cache/qemu/download/7e1430b81c26bdd0da025eeb8fbd77b5dc961da4364af26e771bd39f379cbbf7 for https://archives.fedoraproject.org/pub/archive/fedora/linux/releases/29/Everything/aarch64/os/images/pxeboot/vmlinuz
>    2025-09-15 17:10:50,891 - INFO: gdb output:
>     Python Exception <class 'ModuleNotFoundError'>: No module named 'pycotap'
>    Error occurred in Python: No module named 'pycotap'

Ah, sorry, I have it installed pycotap system-wide, too, so I did not notice 
it... I'll fix it in the next version if we decide to proceed with this 
approach instead of using one of the others.

  Thomas



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH 0/2] tests/functional: Adapt reverse_debugging to run w/o Avocado (yet another try)
  2025-09-15 16:18   ` Thomas Huth
@ 2025-09-15 18:27     ` Alex Bennée
  2025-09-15 22:03     ` Gustavo Romero
  1 sibling, 0 replies; 15+ messages in thread
From: Alex Bennée @ 2025-09-15 18:27 UTC (permalink / raw)
  To: Thomas Huth; +Cc: Gustavo Romero, qemu-devel, Daniel P.Berrangé, qemu-arm

Thomas Huth <thuth@redhat.com> writes:

> On 15/09/2025 18.13, Alex Bennée wrote:
>> Thomas Huth <thuth@redhat.com> writes:
>> 
>>> Here's yet another attempt to remove the avocado dependency from the
>>> reverse debugging tests: I basically took Gustavo's patches to rework
>>> tests/functional/reverse_debugging.py, but instead of calling that
>>> through tests/guest-debug/run-test.py and adding the cumbersome code
>>> to support additional test execution logic, I kept our normal way of
>>> running tests via pycotap.
>> Hmm I was getting:
>>    2025-09-15 17:10:50,798 - INFO: GDB CMD:
>> /home/alex/src/tools/binutils-gdb.git/builds/all/install/bin/gdb -q
>> -n -batch -ex 'set pagination off' -ex 'set confirm off' -ex "py
>> sys.argv=['/home/alex/lsrc/qemu.git/tests/functional/reverse_debugging.py']"
>> -x /home/alex/lsrc/qemu.git/tests/functional/reverse_debugging.py
>>    2025-09-15 17:10:50,803 - DEBUG: Using cached asset /home/alex/.cache/qemu/download/7e1430b81c26bdd0da025eeb8fbd77b5dc961da4364af26e771bd39f379cbbf7 for https://archives.fedoraproject.org/pub/archive/fedora/linux/releases/29/Everything/aarch64/os/images/pxeboot/vmlinuz
>>    2025-09-15 17:10:50,891 - INFO: gdb output:
>>     Python Exception <class 'ModuleNotFoundError'>: No module named 'pycotap'
>>    Error occurred in Python: No module named 'pycotap'
>
> Ah, sorry, I have it installed pycotap system-wide, too, so I did not
> notice it... I'll fix it in the next version if we decide to proceed
> with this approach instead of using one of the others.

FWIW I prefer this approach.

>
>  Thomas

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH 0/2] tests/functional: Adapt reverse_debugging to run w/o Avocado (yet another try)
  2025-09-15 12:42 [RFC PATCH 0/2] tests/functional: Adapt reverse_debugging to run w/o Avocado (yet another try) Thomas Huth
                   ` (2 preceding siblings ...)
  2025-09-15 16:13 ` [RFC PATCH 0/2] tests/functional: Adapt reverse_debugging to run w/o Avocado (yet another try) Alex Bennée
@ 2025-09-15 22:02 ` Gustavo Romero
  2025-09-16  9:15 ` Daniel P. Berrangé
  4 siblings, 0 replies; 15+ messages in thread
From: Gustavo Romero @ 2025-09-15 22:02 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel, Daniel P . Berrangé
  Cc: qemu-arm, Alex Bennée

Hi Thomas,

On 9/15/25 09:42, Thomas Huth wrote:
> Here's yet another attempt to remove the avocado dependency from the
> reverse debugging tests: I basically took Gustavo's patches to rework
> tests/functional/reverse_debugging.py, but instead of calling that
> through tests/guest-debug/run-test.py and adding the cumbersome code
> to support additional test execution logic, I kept our normal way of
> running tests via pycotap. Instead, the essential logic for running
> gdb is copied from tests/guest-debug/run-test.py into the new function
> reverse_debug() that then runs gdb directly with using
> tests/functional/reverse_debugging.py as the script.


Thanks a lot for this series. It's neat that we don't need to touch
meson.build, TAP works, and no additional module is used.

Thumbs up from my side :)

I just have some minor comments about it, inline in the patches.


> Marked as an RFC since this still needs some love... The aarch64 test
> seems to work already (after applying the fix for the reverse debug there
> first: https://patchew.org/QEMU/20250603125459.17688-1-1844144@gmail.com/ ),
> but the ppc64 and x86 tests are currently still completely broken.
> Also we currently log into two different folders this way, into
> tests/functional/aarch64/test_reverse_debug.ReverseDebugging_AArch64.test_aarch64_virt
> for the normal outer test, and into
> tests/functional/aarch64/reverse_debugging.ReverseDebugging.test_reverse_debugging
> for the script that is run in gdb ... it's likely ok for the aarch64
> test, but the ppc64 test contains two subtests, so we need to come up
> with a better solution here for the final implementation.

Although I don't know how to fix this last bit, I think besides the
log files nothing really changes for any arch in comparison with the
current version with Avocado in the tree.


Cheers,
Gustavo

> Gustavo Romero (2):
>    tests/functional: Provide GDB to the functional tests
>    tests/functional: Adapt reverse_debugging to run w/o Avocado
> 
>   configure                                     |   2 +
>   meson.build                                   |   4 +
>   meson_options.txt                             |   2 +
>   scripts/meson-buildoptions.sh                 |   2 +
>   .../functional/aarch64/test_reverse_debug.py  |  16 +-
>   tests/functional/meson.build                  |   7 +
>   tests/functional/ppc64/test_reverse_debug.py  |  18 +-
>   tests/functional/reverse_debugging.py         | 235 +++++++++++-------
>   tests/functional/x86_64/test_reverse_debug.py |  20 +-
>   9 files changed, 188 insertions(+), 118 deletions(-)
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH 1/2] tests/functional: Provide GDB to the functional tests
  2025-09-15 12:42 ` [RFC PATCH 1/2] tests/functional: Provide GDB to the functional tests Thomas Huth
  2025-09-15 16:11   ` Alex Bennée
@ 2025-09-15 22:02   ` Gustavo Romero
  2025-09-16  9:20     ` Daniel P. Berrangé
  1 sibling, 1 reply; 15+ messages in thread
From: Gustavo Romero @ 2025-09-15 22:02 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel, Daniel P . Berrangé
  Cc: qemu-arm, Alex Bennée

Hi Thomas,

On 9/15/25 09:42, Thomas Huth wrote:
> From: Gustavo Romero <gustavo.romero@linaro.org>
> 
> The probe of gdb is done in 'configure' and the full path is passed
> to meson.build via the -Dgdb=option.
> 
> meson then can pass the location of gdb to the test via an environment
> variable.
> 
> This patch is based on an earlier patch ("Support tests that require a
> runner") by Gustavo Romero.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   configure                     | 2 ++
>   meson.build                   | 4 ++++
>   meson_options.txt             | 2 ++
>   scripts/meson-buildoptions.sh | 2 ++
>   tests/functional/meson.build  | 7 +++++++
>   5 files changed, 17 insertions(+)
> 
> diff --git a/configure b/configure
> index 274a7787642..8e2e2cd562a 100755
> --- a/configure
> +++ b/configure
> @@ -1978,6 +1978,8 @@ if test "$skip_meson" = no; then
>     test -n "${LIB_FUZZING_ENGINE+xxx}" && meson_option_add "-Dfuzzing_engine=$LIB_FUZZING_ENGINE"
>     test "$plugins" = yes && meson_option_add "-Dplugins=true"
>     test "$tcg" != enabled && meson_option_add "-Dtcg=$tcg"
> +  test -n "$gdb_bin" && meson_option_add "-Dgdb=$gdb_bin"
> +
>     run_meson() {
>       NINJA=$ninja $meson setup "$@" "$PWD" "$source_path"
>     }
> diff --git a/meson.build b/meson.build
> index 3d738733566..4cbc3c8ac65 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -75,6 +75,10 @@ have_user = have_linux_user or have_bsd_user
>   
>   sh = find_program('sh')
>   python = import('python').find_installation()
> +# Meson python.get_path() on 'purelib' or 'platlib' doesn't properly return the
> +# site-packages dir in pyvenv, so it is built manually.
> +python_ver = python.language_version()
> +python_site_packages = meson.build_root() / 'pyvenv/lib/python' + python_ver / 'site-packages'
>   
>   cc = meson.get_compiler('c')
>   all_languages = ['c']
> diff --git a/meson_options.txt b/meson_options.txt
> index fff1521e580..5bb41bcbc43 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -36,6 +36,8 @@ option('trace_file', type: 'string', value: 'trace',
>   option('coroutine_backend', type: 'combo',
>          choices: ['ucontext', 'sigaltstack', 'windows', 'wasm', 'auto'],
>          value: 'auto', description: 'coroutine backend to use')
> +option('gdb', type: 'string', value: '',
> +       description: 'Path to GDB')
>   
>   # Everything else can be set via --enable/--disable-* option
>   # on the configure script command line.  After adding an option
> diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
> index 0ebe6bc52a6..f4bd21220ee 100644
> --- a/scripts/meson-buildoptions.sh
> +++ b/scripts/meson-buildoptions.sh
> @@ -58,6 +58,7 @@ meson_options_help() {
>     printf "%s\n" '  --enable-ubsan           enable undefined behaviour sanitizer'
>     printf "%s\n" '  --firmwarepath=VALUES    search PATH for firmware files [share/qemu-'
>     printf "%s\n" '                           firmware]'
> +  printf "%s\n" '  --gdb=VALUE              Path to GDB'
>     printf "%s\n" '  --iasl=VALUE             Path to ACPI disassembler'
>     printf "%s\n" '  --includedir=VALUE       Header file directory [include]'
>     printf "%s\n" '  --interp-prefix=VALUE    where to find shared libraries etc., use %M for'
> @@ -323,6 +324,7 @@ _meson_option_parse() {
>       --disable-fuzzing) printf "%s" -Dfuzzing=false ;;
>       --enable-gcrypt) printf "%s" -Dgcrypt=enabled ;;
>       --disable-gcrypt) printf "%s" -Dgcrypt=disabled ;;
> +    --gdb=*) quote_sh "-Dgdb=$2" ;;
>       --enable-gettext) printf "%s" -Dgettext=enabled ;;
>       --disable-gettext) printf "%s" -Dgettext=disabled ;;
>       --enable-gio) printf "%s" -Dgio=enabled ;;
> diff --git a/tests/functional/meson.build b/tests/functional/meson.build
> index 2a0c5aa1418..c822eb66309 100644
> --- a/tests/functional/meson.build
> +++ b/tests/functional/meson.build
> @@ -77,6 +77,12 @@ foreach speed : ['quick', 'thorough']
>       test_env.set('PYTHONPATH', meson.project_source_root() / 'python:' +
>                                  meson.current_source_dir())

It's necessary to add the Python modules from pyvenv to the PYTHONPATH, otherwise
when libpython from GDB looks for pycotap it cannot find it. We already have
it in python_site_packages in Meson (introduced with this series) so adding
python_site_packages to test_env.set() above, like:

diff --git a/tests/functional/meson.build b/tests/functional/meson.build
index c822eb6630..ce992adb02 100644
--- a/tests/functional/meson.build
+++ b/tests/functional/meson.build
@@ -74,8 +74,9 @@ foreach speed : ['quick', 'thorough']
      endif
      test_env.set('QEMU_TEST_QEMU_BINARY', test_emulator.full_path())
      test_env.set('QEMU_BUILD_ROOT', meson.project_build_root())
-    test_env.set('PYTHONPATH', meson.project_source_root() / 'python:' +
-                               meson.current_source_dir())
+    test_env.set('PYTHONPATH', meson.project_source_root() / 'python' +
+                               ':' + meson.current_source_dir() +
+                               ':' + python_site_packages)
  
      # Define the GDB environment variable if gdb is available.
      gdb = get_option('gdb')


fixes the error:

    Python Exception <class 'ModuleNotFoundError'>: No module named 'pycotap'
   Error occurred in Python: No module named 'pycotap'


>   
> +    # Define the GDB environment variable if gdb is available.
> +    gdb = get_option('gdb')
> +    if gdb != ''
> +      test_env.set('QEMU_TEST_GDB', gdb)
> +    endif
> +
>       foreach test : target_tests
>         testname = '@0@-@1@'.format(target_base, test)
>         if fs.exists('generic' / 'test_' + test + '.py')
> @@ -121,6 +127,7 @@ foreach speed : ['quick', 'thorough']
>              priority: time_out,
>              suite: suites)
>       endforeach
> +

As Alex pointed out, I think there is a stray newline here too.


Cheers,
Gustavo

>     endforeach
>   endforeach
>   



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH 2/2] tests/functional: Adapt reverse_debugging to run w/o Avocado
  2025-09-15 12:42 ` [RFC PATCH 2/2] tests/functional: Adapt reverse_debugging to run w/o Avocado Thomas Huth
  2025-09-15 16:14   ` Alex Bennée
@ 2025-09-15 22:02   ` Gustavo Romero
  2025-09-16  9:22   ` Daniel P. Berrangé
  2 siblings, 0 replies; 15+ messages in thread
From: Gustavo Romero @ 2025-09-15 22:02 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel, Daniel P . Berrangé
  Cc: qemu-arm, Alex Bennée

Hi Thomas,

On 9/15/25 09:42, Thomas Huth wrote:
> From: Gustavo Romero <gustavo.romero@linaro.org>
> 
> This commit removes Avocado as a dependency for running the
> reverse_debugging test.
> 
> The main benefit, beyond eliminating an extra dependency, is that there
> is no longer any need to handle GDB packets manually. This removes the
> need for ad-hoc functions dealing with endianness and arch-specific
> register numbers, making the test easier to read. The timeout variable
> is also removed, since Meson now manages timeouts automatically.
> 
> The reverse_debugging test is now executed through running GDB via a
> python script. The test itself is only responsible for invoking
> GDB with the appropriate arguments and for passing the test script to
> GDB.
> 
> reverse_debugging is kept "skipped" for aarch64, ppc64, and x86_64, so
> won't run unless QEMU_TEST_FLAKY_TESTS=1 is set in the test environment,
> before running 'make check-functional' or 'meson test [...]'.
> 
> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> [thuth: Rework the test to run without tests/guest-debug/run-test.py]
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   .../functional/aarch64/test_reverse_debug.py  |  16 +-
>   tests/functional/ppc64/test_reverse_debug.py  |  18 +-
>   tests/functional/reverse_debugging.py         | 235 +++++++++++-------
>   tests/functional/x86_64/test_reverse_debug.py |  20 +-
>   4 files changed, 171 insertions(+), 118 deletions(-)
> 
> diff --git a/tests/functional/aarch64/test_reverse_debug.py b/tests/functional/aarch64/test_reverse_debug.py
> index 85e35645db0..a84ddd07acb 100755
> --- a/tests/functional/aarch64/test_reverse_debug.py
> +++ b/tests/functional/aarch64/test_reverse_debug.py
> @@ -2,7 +2,7 @@
>   #
>   # SPDX-License-Identifier: GPL-2.0-or-later
>   #
> -# Reverse debugging test
> +# Reverse debugging test for aarch64
>   #
>   # Copyright (c) 2020 ISP RAS
>   #
> @@ -12,14 +12,13 @@
>   # This work is licensed under the terms of the GNU GPL, version 2 or
>   # later.  See the COPYING file in the top-level directory.
>   
> -from qemu_test import Asset, skipIfMissingImports, skipFlakyTest
> -from reverse_debugging import ReverseDebugging
> +from qemu_test import QemuSystemTest
> +from qemu_test import Asset, skipFlakyTest
>   
> +from reverse_debugging import reverse_debug
>   
> -@skipIfMissingImports('avocado.utils')
> -class ReverseDebugging_AArch64(ReverseDebugging):
>   
> -    REG_PC = 32
> +class ReverseDebugging_AArch64(QemuSystemTest):
>   
>       ASSET_KERNEL = Asset(
>           ('https://archives.fedoraproject.org/pub/archive/fedora/linux/'
> @@ -29,9 +28,8 @@ class ReverseDebugging_AArch64(ReverseDebugging):
>       def test_aarch64_virt(self):
>           self.set_machine('virt')
>           self.cpu = 'cortex-a53'
> -        kernel_path = self.ASSET_KERNEL.fetch()
> -        self.reverse_debugging(args=('-kernel', kernel_path))
> +        reverse_debug(self, self.ASSET_KERNEL)
>   
>   
>   if __name__ == '__main__':
> -    ReverseDebugging.main()
> +    QemuSystemTest.main()
> diff --git a/tests/functional/ppc64/test_reverse_debug.py b/tests/functional/ppc64/test_reverse_debug.py
> index 5931adef5a9..7da5ede06c8 100755
> --- a/tests/functional/ppc64/test_reverse_debug.py
> +++ b/tests/functional/ppc64/test_reverse_debug.py
> @@ -2,7 +2,7 @@
>   #
>   # SPDX-License-Identifier: GPL-2.0-or-later
>   #
> -# Reverse debugging test
> +# Reverse debugging test for ppc64
>   #
>   # Copyright (c) 2020 ISP RAS
>   #
> @@ -12,14 +12,12 @@
>   # This work is licensed under the terms of the GNU GPL, version 2 or
>   # later.  See the COPYING file in the top-level directory.
>   
> -from qemu_test import skipIfMissingImports, skipFlakyTest
> -from reverse_debugging import ReverseDebugging
> +from qemu_test import QemuSystemTest, skipFlakyTest
>   
> +from reverse_debugging import reverse_debug
>   
> -@skipIfMissingImports('avocado.utils')
> -class ReverseDebugging_ppc64(ReverseDebugging):
>   
> -    REG_PC = 0x40
> +class ReverseDebugging_ppc64(QemuSystemTest):
>   
>       @skipFlakyTest("https://gitlab.com/qemu-project/qemu/-/issues/1992")
>       def test_ppc64_pseries(self):
> @@ -27,15 +25,13 @@ def test_ppc64_pseries(self):
>           # SLOF branches back to its entry point, which causes this test
>           # to take the 'hit a breakpoint again' path. That's not a problem,
>           # just slightly different than the other machines.
> -        self.endian_is_le = False
> -        self.reverse_debugging()
> +        reverse_debug(self)
>   
>       @skipFlakyTest("https://gitlab.com/qemu-project/qemu/-/issues/1992")
>       def test_ppc64_powernv(self):
>           self.set_machine('powernv')
> -        self.endian_is_le = False
> -        self.reverse_debugging()
> +        reverse_debug(self)
>   
>   
>   if __name__ == '__main__':
> -    ReverseDebugging.main()
> +    QemuSystemTest.main()
> diff --git a/tests/functional/reverse_debugging.py b/tests/functional/reverse_debugging.py
> index f9a1d395f1d..c889247defa 100644
> --- a/tests/functional/reverse_debugging.py
> +++ b/tests/functional/reverse_debugging.py
> @@ -1,21 +1,72 @@
> -# Reverse debugging test
> -#
>   # SPDX-License-Identifier: GPL-2.0-or-later
>   #
> +# Reverse debugging test
> +#
>   # Copyright (c) 2020 ISP RAS
> +# Copyright (c) 2025 Linaro Limited
>   #
>   # Author:
>   #  Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
> +#  Gustavo Romero <gustavo.romero@linaro.org> (Run without Avocado)
>   #
>   # 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 os
> +import subprocess
> +import sys
> +
> +try:
> +    import gdb
> +    _has_gdb = True
> +except ModuleNotFoundError:
> +    _has_gdb = False

nit: maybe instead of '_has_gdb' name it '_in_gdb' (for running inside gdb),
since if we have or not GDB in the system is part of our GDB detection in
configure, QEMU_TEST_GDB, etc?



>   from qemu_test import LinuxKernelTest, get_qemu_img
>   from qemu_test.ports import Ports
>   
>   
> +def reverse_debug(test, asset_kernel=None, shift=7, args=None):
> +
> +    # Now launch gdb with our test and collect the result.
> +    gdb_cmd = os.getenv('QEMU_TEST_GDB')
> +    assert(gdb_cmd)

I think we should skip the test if no GDB is available instead of failing it?

So how about something like:

@@ -31,7 +31,8 @@ def reverse_debug(test, asset_kernel=None, shift=7, args=None):
  
      # Now launch gdb with our test and collect the result.
      gdb_cmd = os.getenv('QEMU_TEST_GDB')
-    assert(gdb_cmd)
+    if not gdb_cmd:
+        test.skipTest("Test skipped because there is no GDB available!")


> +    # Run quietly and ignore .gdbinit.
> +    gdb_cmd += " -q -n -batch"
> +    # Disable pagination.
> +    gdb_cmd += " -ex 'set pagination off'"
> +    # Disable prompts in case of crash.
> +    gdb_cmd += " -ex 'set confirm off'"
> +    # Finally the test script itself.
> +    argv = [__file__]
> +    gdb_cmd += f" -ex \"py sys.argv={argv}\""
> +    gdb_cmd += " -x %s" % __file__
> +
> +    test.log.info("GDB CMD: %s" % gdb_cmd)
> +
> +    gdb_env = dict(os.environ)
> +    gdb_pythonpath = gdb_env.get("PYTHONPATH", "").split(os.pathsep)
> +    gdb_pythonpath.append(os.path.dirname(os.path.realpath(__file__)))
> +    gdb_env["PYTHONPATH"] = os.pathsep.join(gdb_pythonpath)
> +    gdb_env["QEMU_TEST_MACHINE"] = test.machine
> +    if test.cpu:
> +        gdb_env["QEMU_TEST_CPU"] = test.cpu
> +    if asset_kernel:
> +        gdb_env["QEMU_TEST_KERNEL"] = asset_kernel.fetch()
> +    result = subprocess.run(gdb_cmd, shell=True, check=False,
> +                            stdout=subprocess.PIPE,
> +                            stderr=subprocess.STDOUT,
> +                            encoding='utf8',
> +                            env=gdb_env)
> +    test.log.info("gdb output:\n %s" % result.stdout)
> +    if result.returncode != 0:
> +        test.fail(f"gdb failed with return code {result.returncode}")
> +    else:
> +        test.log.info("gdb run succeeded!")
> +
> +
>   class ReverseDebugging(LinuxKernelTest):
>       """
>       Test GDB reverse debugging commands: reverse step and reverse continue.
> @@ -28,13 +79,9 @@ class ReverseDebugging(LinuxKernelTest):
>       that the execution is stopped at the last of them.
>       """
>   
> -    timeout = 10
>       STEPS = 10
> -    endian_is_le = True
>   
>       def run_vm(self, record, shift, args, replay_path, image_path, port):
> -        from avocado.utils import datadrainer
> -
>           logger = logging.getLogger('replay')
>           vm = self.get_vm(name='record' if record else 'replay')
>           vm.set_console()
> @@ -52,59 +99,58 @@ def run_vm(self, record, shift, args, replay_path, image_path, port):
>           if args:
>               vm.add_args(*args)
>           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()
> +
>           return vm
>   
>       @staticmethod
> -    def get_reg_le(g, reg):
> -        res = g.cmd(b'p%x' % reg)
> -        num = 0
> -        for i in range(len(res))[-2::-2]:
> -            num = 0x100 * num + int(res[i:i + 2], 16)
> -        return num
> +    def gdb_connect(host, port):
> +        # Set debug on connection to get the qSupport string.
> +        gdb.execute("set debug remote 1")
> +        r = gdb.execute(f"target remote {host}:{port}", False, True)
> +        gdb.execute("set debug remote 0")
> +
> +        return r
>   
>       @staticmethod
> -    def get_reg_be(g, reg):
> -        res = g.cmd(b'p%x' % reg)
> -        return int(res, 16)
> -
> -    def get_reg(self, g, reg):
> -        # value may be encoded in BE or LE order
> -        if self.endian_is_le:
> -            return self.get_reg_le(g, reg)
> -        else:
> -            return self.get_reg_be(g, reg)
> +    def get_pc():
> +        val = gdb.parse_and_eval("$pc")
> +        pc = int(val)
>   
> -    def get_pc(self, g):
> -        return self.get_reg(g, self.REG_PC)
> +        return pc
>   
> -    def check_pc(self, g, addr):
> -        pc = self.get_pc(g)
> +    def check_pc(self, addr):
> +        logger = logging.getLogger('reply')
> +        pc = self.get_pc()
>           if pc != addr:
> -            self.fail('Invalid PC (read %x instead of %x)' % (pc, addr))
> +            logger.info('Invalid PC (read %x instead of %x)' % (pc, addr))
> +            gdb.execute("exit 1")
>   
>       @staticmethod
> -    def gdb_step(g):
> -        g.cmd(b's', b'T05thread:01;')
> +    def gdb_step():
> +        gdb.execute("stepi")
>   
>       @staticmethod
> -    def gdb_bstep(g):
> -        g.cmd(b'bs', b'T05thread:01;')
> +    def gdb_bstep():
> +        gdb.execute("reverse-stepi")
>   
>       @staticmethod
>       def vm_get_icount(vm):
>           return vm.qmp('query-replay')['return']['icount']
>   
> -    def reverse_debugging(self, shift=7, args=None):
> -        from avocado.utils import gdb
> -        from avocado.utils import process
> +    def test_reverse_debugging(self):
> +
> +        shift = 7
> +
> +        self.set_machine(os.getenv('QEMU_TEST_MACHINE'))
> +        self.cpu = os.getenv('QEMU_TEST_CPU')
> +        kernel_path = os.getenv('QEMU_TEST_KERNEL')
> +        args = None
> +        if kernel_path:
> +            args = ['-kernel', kernel_path]
>   
>           logger = logging.getLogger('replay')
>   
> -        # create qcow2 for snapshots
> +        # Create qcow2 for snapshots
>           logger.info('creating qcow2 image for VM snapshots')
>           image_path = os.path.join(self.workdir, 'disk.qcow2')
>           qemu_img = get_qemu_img(self)
> @@ -112,11 +158,11 @@ def reverse_debugging(self, shift=7, args=None):
>               self.skipTest('Could not find "qemu-img", which is required to '
>                             'create the temporary qcow2 image')
>           cmd = '%s create -f qcow2 %s 128M' % (qemu_img, image_path)
> -        process.run(cmd)
> +        subprocess.run(cmd, shell=True)
>   
>           replay_path = os.path.join(self.workdir, 'replay.bin')
>   
> -        # record the log
> +        # Record the log.
>           vm = self.run_vm(True, shift, args, replay_path, image_path, -1)
>           while self.vm_get_icount(vm) <= self.STEPS:
>               pass
> @@ -125,72 +171,91 @@ def reverse_debugging(self, shift=7, args=None):
>   
>           logger.info("recorded log with %s+ steps" % last_icount)
>   
> -        # replay and run debug commands
> +        # Replay and run debug commands.
>           with Ports() as ports:
>               port = ports.find_free_port()
>               vm = self.run_vm(False, shift, args, replay_path, image_path, port)
> -        logger.info('connecting to gdbstub')
> -        g = gdb.GDBRemote('127.0.0.1', port, False, False)
> -        g.connect()
> -        r = g.cmd(b'qSupported')
> -        if b'qXfer:features:read+' in r:
> -            g.cmd(b'qXfer:features:read:target.xml:0,ffb')
> -        if b'ReverseStep+' not in r:
> +        logger.info('Connecting to gdbstub')
> +        r = self.gdb_connect('127.0.0.1', port)
> +        if 'ReverseStep+' not in r:
>               self.fail('Reverse step is not supported by QEMU')
> -        if b'ReverseContinue+' not in r:
> +        if 'ReverseContinue+' not in r:
>               self.fail('Reverse continue is not supported by QEMU')
>   
> -        logger.info('stepping forward')
> +        logger.info('Stepping forward')
>           steps = []
> -        # record first instruction addresses
> +        # Record first instruction addresses.
>           for _ in range(self.STEPS):
> -            pc = self.get_pc(g)
> -            logger.info('saving position %x' % pc)
> +            pc = self.get_pc()
> +            logger.info('Saving position %x' % pc)
>               steps.append(pc)
> -            self.gdb_step(g)
> +            self.gdb_step()
>   
> -        # visit the recorded instruction in reverse order
> -        logger.info('stepping backward')
> +        # Visit the recorded instruction in reverse order.
> +        logger.info('Stepping backward')
>           for addr in steps[::-1]:
> -            self.gdb_bstep(g)
> -            self.check_pc(g, addr)
> -            logger.info('found position %x' % addr)
> +            self.gdb_bstep()
> +            self.check_pc(addr)
> +            logger.info('Found position %x' % addr)
>   
> -        # visit the recorded instruction in forward order
> -        logger.info('stepping forward')
> +        # Visit the recorded instruction in forward order.
> +        logger.info('Stepping forward')
>           for addr in steps:
> -            self.check_pc(g, addr)
> -            self.gdb_step(g)
> -            logger.info('found position %x' % addr)
> +            self.check_pc(addr)
> +            self.gdb_step()
> +            logger.info('Found position %x' % addr)
>   
> -        # set breakpoints for the instructions just stepped over
> -        logger.info('setting breakpoints')
> +        # Set breakpoints for the instructions just stepped over.
> +        logger.info('Setting breakpoints')
>           for addr in steps:
>               # hardware breakpoint at addr with len=1
> -            g.cmd(b'Z1,%x,1' % addr, b'OK')
> +            gdb.execute(f"break *{hex(addr)}")
>   
> -        # this may hit a breakpoint if first instructions are executed
> -        # again
> -        logger.info('continuing execution')
> +        # This may hit a breakpoint if first instructions are executed again.
> +        logger.info('Continuing execution')
>           vm.qmp('replay-break', icount=last_icount - 1)
> -        # continue - will return after pausing
> +        # continue - will return after pausing.
>           # This could stop at the end and get a T02 return, or by
>           # re-executing one of the breakpoints and get a T05 return.
> -        g.cmd(b'c')
> +        gdb.execute("continue")
>           if self.vm_get_icount(vm) == last_icount - 1:
> -            logger.info('reached the end (icount %s)' % (last_icount - 1))
> +            logger.info('Reached the end (icount %s)' % (last_icount - 1))
>           else:
> -            logger.info('hit a breakpoint again at %x (icount %s)' %
> -                        (self.get_pc(g), self.vm_get_icount(vm)))
> +            logger.info('Hit a breakpoint again at %x (icount %s)' %
> +                        (self.get_pc(), self.vm_get_icount(vm)))
>   
> -        logger.info('running reverse continue to reach %x' % steps[-1])
> -        # reverse continue - will return after stopping at the breakpoint
> -        g.cmd(b'bc', b'T05thread:01;')
> +        logger.info('Running reverse continue to reach %x' % steps[-1])
> +        # reverse continue - will return after stopping at the breakpoint.
> +        gdb.execute("reverse-continue")
>   
> -        # assume that none of the first instructions is executed again
> -        # breaking the order of the breakpoints
> -        self.check_pc(g, steps[-1])
> -        logger.info('successfully reached %x' % steps[-1])
> +        # Assume that none of the first instructions is executed again
> +        # breaking the order of the breakpoints.
> +        # steps[-1] is the first saved $pc in reverse order.
> +        self.check_pc(steps[-1])
> +        logger.info('Successfully reached %x' % steps[-1])
>   
> -        logger.info('exiting gdb and qemu')
> +        logger.info('Exiting GDB and QEMU...')
> +        # Disconnect from the VM.
> +        gdb.execute("disconnect")
> +        # Guarantee VM is shutdown.
>           vm.shutdown()
> +        # Gently exit from GDB.
> +        gdb.execute('print "test succeeded"')
> +        gdb.execute("exit 0")
> +
> +    @staticmethod
> +    def main():
> +        try:
> +            LinuxKernelTest.main()
> +        except SystemExit:
> +            # If the test is marked with @skipFlakyTest, then it will be exited
> +            # via sys.exit() before we have the chance to exit from GDB gently.
> +            # Because recent versions of GDB will return a failure value if this
> +            # happens, we catch the SystemExit and exit from GDB gently with 77,
> +            # which meson interprets correctly as a skipped test.
> +            gdb.execute("exit 77")

Now that we're not passing the exitcode directly to Meson we can remove this overriden
main() method completely.



> +if __name__ == '__main__':
> +    if not _has_gdb:
> +        sys.exit("This script must be launched via tests/guest-debug/run-test.py!")

Since we don't rely on any run-test.py code anymore, how about:

  if __name__ == '__main__':
      if not _has_gdb:
-        sys.exit("This script must be launched via tests/guest-debug/run-test.py!")
+        sys.exit("This script must be run with GDB using the -x option.")


Cheers,
Gustavo

> +    ReverseDebugging.main()
> diff --git a/tests/functional/x86_64/test_reverse_debug.py b/tests/functional/x86_64/test_reverse_debug.py
> index d713e91e144..e823f0d4953 100755
> --- a/tests/functional/x86_64/test_reverse_debug.py
> +++ b/tests/functional/x86_64/test_reverse_debug.py
> @@ -2,7 +2,7 @@
>   #
>   # SPDX-License-Identifier: GPL-2.0-or-later
>   #
> -# Reverse debugging test
> +# Reverse debugging test for x86_64
>   #
>   # Copyright (c) 2020 ISP RAS
>   #
> @@ -12,25 +12,19 @@
>   # This work is licensed under the terms of the GNU GPL, version 2 or
>   # later.  See the COPYING file in the top-level directory.
>   
> -from qemu_test import skipIfMissingImports, skipFlakyTest
> -from reverse_debugging import ReverseDebugging
> +from qemu_test import QemuSystemTest, skipFlakyTest
>   
> +from reverse_debugging import reverse_debug
>   
> -@skipIfMissingImports('avocado.utils')
> -class ReverseDebugging_X86_64(ReverseDebugging):
>   
> -    REG_PC = 0x10
> -    REG_CS = 0x12
> -    def get_pc(self, g):
> -        return self.get_reg_le(g, self.REG_PC) \
> -            + self.get_reg_le(g, self.REG_CS) * 0x10
> +class ReverseDebugging_X86_64(QemuSystemTest):
>   
>       @skipFlakyTest("https://gitlab.com/qemu-project/qemu/-/issues/2922")
>       def test_x86_64_pc(self):
>           self.set_machine('pc')
> -        # start with BIOS only
> -        self.reverse_debugging()
> +        # Start with BIOS only
> +        reverse_debug(self)
>   
>   
>   if __name__ == '__main__':
> -    ReverseDebugging.main()
> +    QemuSystemTest.main()



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH 0/2] tests/functional: Adapt reverse_debugging to run w/o Avocado (yet another try)
  2025-09-15 16:18   ` Thomas Huth
  2025-09-15 18:27     ` Alex Bennée
@ 2025-09-15 22:03     ` Gustavo Romero
  1 sibling, 0 replies; 15+ messages in thread
From: Gustavo Romero @ 2025-09-15 22:03 UTC (permalink / raw)
  To: Thomas Huth, Alex Bennée
  Cc: qemu-devel, Daniel P.Berrangé, qemu-arm

Hi Thomas,

On 9/15/25 13:18, Thomas Huth wrote:
> On 15/09/2025 18.13, Alex Bennée wrote:
>> Thomas Huth <thuth@redhat.com> writes:
>>
>>> Here's yet another attempt to remove the avocado dependency from the
>>> reverse debugging tests: I basically took Gustavo's patches to rework
>>> tests/functional/reverse_debugging.py, but instead of calling that
>>> through tests/guest-debug/run-test.py and adding the cumbersome code
>>> to support additional test execution logic, I kept our normal way of
>>> running tests via pycotap.
>>
>> Hmm I was getting:
>>
>>    2025-09-15 17:10:50,798 - INFO: GDB CMD: /home/alex/src/tools/binutils-gdb.git/builds/all/install/bin/gdb -q -n -batch -ex 'set pagination off' -ex 'set confirm off' -ex "py sys.argv=['/home/alex/lsrc/qemu.git/tests/functional/reverse_debugging.py']" -x /home/alex/lsrc/qemu.git/tests/functional/reverse_debugging.py
>>    2025-09-15 17:10:50,803 - DEBUG: Using cached asset /home/alex/.cache/qemu/download/7e1430b81c26bdd0da025eeb8fbd77b5dc961da4364af26e771bd39f379cbbf7 for https://archives.fedoraproject.org/pub/archive/fedora/linux/releases/29/Everything/aarch64/os/images/pxeboot/vmlinuz
>>    2025-09-15 17:10:50,891 - INFO: gdb output:
>>     Python Exception <class 'ModuleNotFoundError'>: No module named 'pycotap'
>>    Error occurred in Python: No module named 'pycotap'
> 
> Ah, sorry, I have it installed pycotap system-wide, too, so I did not notice it... I'll fix it in the next version if we decide to proceed with this approach instead of using one of the others.

See my comment in patch 1/2 about adding python_site_packages to
PYTHONPATH to fix it. I think there is no harm to the other tests
by adding it to the test_env, so I don't think it's worth taking
care of it only in reverse_debugging.py.


Cheers,
Gustavo


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH 0/2] tests/functional: Adapt reverse_debugging to run w/o Avocado (yet another try)
  2025-09-15 12:42 [RFC PATCH 0/2] tests/functional: Adapt reverse_debugging to run w/o Avocado (yet another try) Thomas Huth
                   ` (3 preceding siblings ...)
  2025-09-15 22:02 ` Gustavo Romero
@ 2025-09-16  9:15 ` Daniel P. Berrangé
  4 siblings, 0 replies; 15+ messages in thread
From: Daniel P. Berrangé @ 2025-09-16  9:15 UTC (permalink / raw)
  To: Thomas Huth; +Cc: Gustavo Romero, qemu-devel, qemu-arm, Alex Bennée

On Mon, Sep 15, 2025 at 02:42:05PM +0200, Thomas Huth wrote:
> Here's yet another attempt to remove the avocado dependency from the
> reverse debugging tests: I basically took Gustavo's patches to rework
> tests/functional/reverse_debugging.py, but instead of calling that
> through tests/guest-debug/run-test.py and adding the cumbersome code
> to support additional test execution logic, I kept our normal way of
> running tests via pycotap. Instead, the essential logic for running
> gdb is copied from tests/guest-debug/run-test.py into the new function
> reverse_debug() that then runs gdb directly with using
> tests/functional/reverse_debugging.py as the script.

Something I've not previously realized is that when run via GDB, we are
not honouring the Python version we chose to use with QEMU. GDB is not
actually running the python interpreter binary, instead it has linked to
the libpython.so and runs python code in-process to GDB. Thus the version
of python being used is out of our control, it is whatever the distro
chose to link GDB to. When I look back at how we've handled our min
python, vs what the distros use as system python, I think the constraint
is highly undesirable.

> Marked as an RFC since this still needs some love... The aarch64 test
> seems to work already (after applying the fix for the reverse debug there
> first: https://patchew.org/QEMU/20250603125459.17688-1-1844144@gmail.com/ ),
> but the ppc64 and x86 tests are currently still completely broken.
> Also we currently log into two different folders this way, into
> tests/functional/aarch64/test_reverse_debug.ReverseDebugging_AArch64.test_aarch64_virt
> for the normal outer test, and into
> tests/functional/aarch64/reverse_debugging.ReverseDebugging.test_reverse_debugging
> for the script that is run in gdb ... it's likely ok for the aarch64
> test, but the ppc64 test contains two subtests, so we need to come up
> with a better solution here for the final implementation.

Right, this is one of the things I rather dislike with this, as it is
making debugging much more painful.

In one base.log you just get stdout from gdb with no context of what
commands are run, and in the other base.log you get the logs from the
sub-process. Well at least you would if we fixed the tests to use
self.log, instead of creating a new logger category.

Even with that fixed though, it is very difficult to correlate GDB
output in one log, with GDB commands invoke in the other log, in
order to understand why it is failing in the x86/ppc tests.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH 1/2] tests/functional: Provide GDB to the functional tests
  2025-09-15 22:02   ` Gustavo Romero
@ 2025-09-16  9:20     ` Daniel P. Berrangé
  0 siblings, 0 replies; 15+ messages in thread
From: Daniel P. Berrangé @ 2025-09-16  9:20 UTC (permalink / raw)
  To: Gustavo Romero; +Cc: Thomas Huth, qemu-devel, qemu-arm, Alex Bennée

On Mon, Sep 15, 2025 at 07:02:24PM -0300, Gustavo Romero wrote:
> Hi Thomas,
> 
> On 9/15/25 09:42, Thomas Huth wrote:
> > From: Gustavo Romero <gustavo.romero@linaro.org>
> > 
> > The probe of gdb is done in 'configure' and the full path is passed
> > to meson.build via the -Dgdb=option.
> > 
> > meson then can pass the location of gdb to the test via an environment
> > variable.
> > 
> > This patch is based on an earlier patch ("Support tests that require a
> > runner") by Gustavo Romero.
> > 
> > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > ---
> >   configure                     | 2 ++
> >   meson.build                   | 4 ++++
> >   meson_options.txt             | 2 ++
> >   scripts/meson-buildoptions.sh | 2 ++
> >   tests/functional/meson.build  | 7 +++++++
> >   5 files changed, 17 insertions(+)
> > 
> > diff --git a/configure b/configure
> > index 274a7787642..8e2e2cd562a 100755
> > --- a/configure
> > +++ b/configure
> > @@ -1978,6 +1978,8 @@ if test "$skip_meson" = no; then
> >     test -n "${LIB_FUZZING_ENGINE+xxx}" && meson_option_add "-Dfuzzing_engine=$LIB_FUZZING_ENGINE"
> >     test "$plugins" = yes && meson_option_add "-Dplugins=true"
> >     test "$tcg" != enabled && meson_option_add "-Dtcg=$tcg"
> > +  test -n "$gdb_bin" && meson_option_add "-Dgdb=$gdb_bin"
> > +
> >     run_meson() {
> >       NINJA=$ninja $meson setup "$@" "$PWD" "$source_path"
> >     }
> > diff --git a/meson.build b/meson.build
> > index 3d738733566..4cbc3c8ac65 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -75,6 +75,10 @@ have_user = have_linux_user or have_bsd_user
> >   sh = find_program('sh')
> >   python = import('python').find_installation()
> > +# Meson python.get_path() on 'purelib' or 'platlib' doesn't properly return the
> > +# site-packages dir in pyvenv, so it is built manually.
> > +python_ver = python.language_version()
> > +python_site_packages = meson.build_root() / 'pyvenv/lib/python' + python_ver / 'site-packages'
> >   cc = meson.get_compiler('c')
> >   all_languages = ['c']
> > diff --git a/meson_options.txt b/meson_options.txt
> > index fff1521e580..5bb41bcbc43 100644
> > --- a/meson_options.txt
> > +++ b/meson_options.txt
> > @@ -36,6 +36,8 @@ option('trace_file', type: 'string', value: 'trace',
> >   option('coroutine_backend', type: 'combo',
> >          choices: ['ucontext', 'sigaltstack', 'windows', 'wasm', 'auto'],
> >          value: 'auto', description: 'coroutine backend to use')
> > +option('gdb', type: 'string', value: '',
> > +       description: 'Path to GDB')
> >   # Everything else can be set via --enable/--disable-* option
> >   # on the configure script command line.  After adding an option
> > diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
> > index 0ebe6bc52a6..f4bd21220ee 100644
> > --- a/scripts/meson-buildoptions.sh
> > +++ b/scripts/meson-buildoptions.sh
> > @@ -58,6 +58,7 @@ meson_options_help() {
> >     printf "%s\n" '  --enable-ubsan           enable undefined behaviour sanitizer'
> >     printf "%s\n" '  --firmwarepath=VALUES    search PATH for firmware files [share/qemu-'
> >     printf "%s\n" '                           firmware]'
> > +  printf "%s\n" '  --gdb=VALUE              Path to GDB'
> >     printf "%s\n" '  --iasl=VALUE             Path to ACPI disassembler'
> >     printf "%s\n" '  --includedir=VALUE       Header file directory [include]'
> >     printf "%s\n" '  --interp-prefix=VALUE    where to find shared libraries etc., use %M for'
> > @@ -323,6 +324,7 @@ _meson_option_parse() {
> >       --disable-fuzzing) printf "%s" -Dfuzzing=false ;;
> >       --enable-gcrypt) printf "%s" -Dgcrypt=enabled ;;
> >       --disable-gcrypt) printf "%s" -Dgcrypt=disabled ;;
> > +    --gdb=*) quote_sh "-Dgdb=$2" ;;
> >       --enable-gettext) printf "%s" -Dgettext=enabled ;;
> >       --disable-gettext) printf "%s" -Dgettext=disabled ;;
> >       --enable-gio) printf "%s" -Dgio=enabled ;;
> > diff --git a/tests/functional/meson.build b/tests/functional/meson.build
> > index 2a0c5aa1418..c822eb66309 100644
> > --- a/tests/functional/meson.build
> > +++ b/tests/functional/meson.build
> > @@ -77,6 +77,12 @@ foreach speed : ['quick', 'thorough']
> >       test_env.set('PYTHONPATH', meson.project_source_root() / 'python:' +
> >                                  meson.current_source_dir())
> 
> It's necessary to add the Python modules from pyvenv to the PYTHONPATH, otherwise
> when libpython from GDB looks for pycotap it cannot find it. We already have
> it in python_site_packages in Meson (introduced with this series) so adding
> python_site_packages to test_env.set() above, like:

This dovetails to the point I made on the cover letter.

The pyvenv is populated wrt the python version we selected for QEMU.
This cannot be assumed to be the same as the version that GDB was
built against. Pointing GDB to a venv for a different python version
is not a good idea.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH 2/2] tests/functional: Adapt reverse_debugging to run w/o Avocado
  2025-09-15 12:42 ` [RFC PATCH 2/2] tests/functional: Adapt reverse_debugging to run w/o Avocado Thomas Huth
  2025-09-15 16:14   ` Alex Bennée
  2025-09-15 22:02   ` Gustavo Romero
@ 2025-09-16  9:22   ` Daniel P. Berrangé
  2 siblings, 0 replies; 15+ messages in thread
From: Daniel P. Berrangé @ 2025-09-16  9:22 UTC (permalink / raw)
  To: Thomas Huth; +Cc: Gustavo Romero, qemu-devel, qemu-arm, Alex Bennée

On Mon, Sep 15, 2025 at 02:42:07PM +0200, Thomas Huth wrote:
> From: Gustavo Romero <gustavo.romero@linaro.org>
> 
> This commit removes Avocado as a dependency for running the
> reverse_debugging test.
> 
> The main benefit, beyond eliminating an extra dependency, is that there
> is no longer any need to handle GDB packets manually. This removes the
> need for ad-hoc functions dealing with endianness and arch-specific
> register numbers, making the test easier to read. The timeout variable
> is also removed, since Meson now manages timeouts automatically.
> 
> The reverse_debugging test is now executed through running GDB via a
> python script. The test itself is only responsible for invoking
> GDB with the appropriate arguments and for passing the test script to
> GDB.
> 
> reverse_debugging is kept "skipped" for aarch64, ppc64, and x86_64, so
> won't run unless QEMU_TEST_FLAKY_TESTS=1 is set in the test environment,
> before running 'make check-functional' or 'meson test [...]'.
> 
> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> [thuth: Rework the test to run without tests/guest-debug/run-test.py]
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  .../functional/aarch64/test_reverse_debug.py  |  16 +-
>  tests/functional/ppc64/test_reverse_debug.py  |  18 +-
>  tests/functional/reverse_debugging.py         | 235 +++++++++++-------
>  tests/functional/x86_64/test_reverse_debug.py |  20 +-
>  4 files changed, 171 insertions(+), 118 deletions(-)
> 

snip

>  
> -        # assume that none of the first instructions is executed again
> -        # breaking the order of the breakpoints
> -        self.check_pc(g, steps[-1])
> -        logger.info('successfully reached %x' % steps[-1])
> +        # Assume that none of the first instructions is executed again
> +        # breaking the order of the breakpoints.
> +        # steps[-1] is the first saved $pc in reverse order.
> +        self.check_pc(steps[-1])
> +        logger.info('Successfully reached %x' % steps[-1])
>  
> -        logger.info('exiting gdb and qemu')
> +        logger.info('Exiting GDB and QEMU...')
> +        # Disconnect from the VM.
> +        gdb.execute("disconnect")
> +        # Guarantee VM is shutdown.
>          vm.shutdown()
> +        # Gently exit from GDB.
> +        gdb.execute('print "test succeeded"')
> +        gdb.execute("exit 0")

This causes immediate terminatino of the python program which prevents
any of the tearDown cleanup logic from running.

> +
> +    @staticmethod
> +    def main():
> +        try:
> +            LinuxKernelTest.main()
> +        except SystemExit:
> +            # If the test is marked with @skipFlakyTest, then it will be exited
> +            # via sys.exit() before we have the chance to exit from GDB gently.
> +            # Because recent versions of GDB will return a failure value if this
> +            # happens, we catch the SystemExit and exit from GDB gently with 77,
> +            # which meson interprets correctly as a skipped test.
> +            gdb.execute("exit 77")
> +
> +if __name__ == '__main__':
> +    if not _has_gdb:
> +        sys.exit("This script must be launched via tests/guest-debug/run-test.py!")

This is incorrect.

> +    ReverseDebugging.main()


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2025-09-16  9:23 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-15 12:42 [RFC PATCH 0/2] tests/functional: Adapt reverse_debugging to run w/o Avocado (yet another try) Thomas Huth
2025-09-15 12:42 ` [RFC PATCH 1/2] tests/functional: Provide GDB to the functional tests Thomas Huth
2025-09-15 16:11   ` Alex Bennée
2025-09-15 22:02   ` Gustavo Romero
2025-09-16  9:20     ` Daniel P. Berrangé
2025-09-15 12:42 ` [RFC PATCH 2/2] tests/functional: Adapt reverse_debugging to run w/o Avocado Thomas Huth
2025-09-15 16:14   ` Alex Bennée
2025-09-15 22:02   ` Gustavo Romero
2025-09-16  9:22   ` Daniel P. Berrangé
2025-09-15 16:13 ` [RFC PATCH 0/2] tests/functional: Adapt reverse_debugging to run w/o Avocado (yet another try) Alex Bennée
2025-09-15 16:18   ` Thomas Huth
2025-09-15 18:27     ` Alex Bennée
2025-09-15 22:03     ` Gustavo Romero
2025-09-15 22:02 ` Gustavo Romero
2025-09-16  9:15 ` Daniel P. Berrangé

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).