[PATCH v4 0/9] tests/functional: Adapt reverse_debugging to run w/o Avocado

[PATCH v4 0/9] tests/functional: Adapt reverse_debugging to run w/o Avocado

[Gustavo Romero]

tests/functional: Adapt reverse_debugging to run w/o Avocado

The goal of this series is to remove Avocado as a dependency for running
the reverse_debugging functional test.

After several rounds of discussions about v1 and v2, and experiments
done by Daniel and Thomas (thanks for all the experiments and comments
so far), I've taken a new approach and moved away from using a runner
for GDB. The changes, I believe, are much simpler now.

This new series uses GDB's machine interface (MI) via the pygdbmi module
(thanks Manos and Peter for the inputs). pygdbmi provides a controller
to start GDB and communicate with it through MI, so there is no longer a
risk of version clashes between libpython in GDB and Python modules in
the pyvenv, as it could, in theory, happen when GDB executes the test
script via -x option.

Also, as Daniel pointed out, the overall test output is pretty bad and
currently does not allow one to easily follow the sequence of GDB
commands used in the test. I took this opportunity to improve the output
and it now prints the sequence in a format that can be copied and pasted
directly into GDB.

The TAP protocol is respected, and Meson correctly displays GDB's test
output in testlog-thorough.txt.

Because the pygdbmi "shim" is so thin, I had to write a trivial GDB
class around it to easily capture and print the payloads returned by its
write() method. The GDB class allows clean, single-line commands to be
used in the tests through method chaining, making them easier to follow,
for example:

pc = gdb.cli("print $pc").get_add()

The test is kept “skipped” for aarch64, ppc64, and x86_64, so it is
necessary to set QEMU_TEST_FLAKY_TESTS=1 in the test environment to
effectively run the test on these archs.

On aarch64, the test is flaky, but there is a fix that I’ve tested while
writing this series [0] that resolves it. On ppc64 and x86_64, the test
always fails: on ppc64, GDB gets a bogus PC, and on x86_64, the last
part of the test (reverse-continue) does not hit the last executed PC
(as it should happen) but instead jumps to the beginning of the code
(first PC in forward order).

Thus, to effectively run the reverse_debugging test on aarch64:

$ export QEMU_TEST_FLAKY_TESTS=1
$ make check-functional

or even, to run only the reverse_debug test after 'make check-functional':
$ ./pyvenv/bin/meson test --verbose --no-rebuild -t 1 --setup thorough --suite func-thorough func-aarch64-reverse_debug


Cheers,
Gustavo

v1:
https://patchew.org/QEMU/20250819143916.4138035-1-gustavo.romero@linaro.org/

v2:
https://patchew.org/QEMU/20250904154640.52687-1-gustavo.romero@linaro.org/

v3:
https://patchew.org/QEMU/20250922054351.14289-1-gustavo.romero@linaro.org/

v4:
- Installed pygdbmi only in check-functional step and using pip,
  so not from local wheels (thomas)
- Fixed GDB probe in configure by ensuring that the GDB passed to Meson
  supports all that arches in the target list
- Fixed error found by Alex when skipping the test, "name 'test' is not
  defined", by using the new decorator suggested by Daniel to skip the
  tests if QEMU_TEST_GDB env var is missing (alex, daniel)
- Moved GDB class into its own file under qemu_test, in gdb.py. (alex,
  daniel)
- Put subprocess and the drop of the data drainer changes in separate
  commits (used previous commits from Daniel for it) (alex, daniel)
- Put back /usr/bin/env python shebang into per arch test files (daniel)
- Added new method reverse_debugging_run() to separate test bootstrap
  from GDB test execution, also helping to get a better diff view
  (daniel)
- Made diffs better for reviewing by avoiding mixing text changes with
  changes in the code related to Avocado's removal (alex, daniel)


Daniel P. Berrangé (2):
  tests/functional: replace avocado process with subprocess
  tests/functional: drop datadrainer class in reverse debugging

Gustavo Romero (7):
  tests/functional: Re-activate the check-venv target
  python: Install pygdbmi in meson's venv
  tests/functional: Provide GDB to the functional tests
  tests/functional: Add GDB class
  tests/functional: Add decorator to skip test on missing env vars
  tests/functional: Adapt reverse_debugging to run w/o Avocado
  tests/functional: Adapt arches to reverse_debugging w/o Avocado

 configure                                     |  21 +++
 meson_options.txt                             |   2 +
 pythondeps.toml                               |   1 +
 scripts/meson-buildoptions.sh                 |   2 +
 tests/Makefile.include                        |   2 +-
 .../functional/aarch64/test_reverse_debug.py  |   9 +-
 tests/functional/meson.build                  |   6 +
 tests/functional/ppc64/test_reverse_debug.py  |  11 +-
 tests/functional/qemu_test/__init__.py        |   4 +-
 tests/functional/qemu_test/decorators.py      |  18 +++
 tests/functional/qemu_test/gdb.py             |  85 +++++++++++
 tests/functional/reverse_debugging.py         | 140 ++++++++----------
 tests/functional/x86_64/test_reverse_debug.py |  13 +-
 13 files changed, 216 insertions(+), 98 deletions(-)
 create mode 100644 tests/functional/qemu_test/gdb.py

-- 
2.34.1

[PATCH v4 1/9] tests/functional: Re-activate the check-venv target

[Gustavo Romero]

Add check-venv target as a dependency for the functional tests. This
causes Python modules listed in pythondeps.toml, under the testdeps
group, to be installed when 'make check-functional' is executed to
prepare and run the functional tests.

Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
Suggested-by: Thomas Huth <thuth@redhat.com>
---
 tests/Makefile.include | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 3538c0c740..d012a9b25d 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -109,7 +109,7 @@ $(FUNCTIONAL_TARGETS):
 	@$(MAKE) SPEED=thorough $(subst -functional,-func,$@)
 
 .PHONY: check-functional
-check-functional:
+check-functional: check-venv
 	@$(NINJA) precache-functional
 	@QEMU_TEST_NO_DOWNLOAD=1 $(MAKE) SPEED=thorough check-func check-func-quick
 
-- 
2.34.1

[PATCH v4 2/9] python: Install pygdbmi in meson's venv

[Gustavo Romero]

The upcoming changes in the reverse_debugging functional test to remove
Avocado as a dependency will require pygdbmi for interacting with GDB,
so install it in meson's venv (located in the build dir's pyvenv/).

Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
---
 pythondeps.toml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/pythondeps.toml b/pythondeps.toml
index 16fb2a989c..98e99e7900 100644
--- a/pythondeps.toml
+++ b/pythondeps.toml
@@ -33,3 +33,4 @@ sphinx_rtd_theme = { accepted = ">=0.5", installed = "1.2.2" }
 
 [testdeps]
 qemu.qmp = { accepted = ">=0.0.3", installed = "0.0.3" }
+pygdbmi = { accepted = ">=0.11.0.0", installed = "0.11.0.0" }
-- 
2.34.1

[PATCH v4 3/9] tests/functional: Provide GDB to the functional tests

[Gustavo Romero]

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

Because a single functional test can cover different arches, such as
aarch64, ppc64, and x86_64, only a GDB that supports all the arches in
the target list is passed to Meson for use in the functional tests. To
handle this check, a new shell function, is_target_arch_in_arch_list, is
introduced in 'configure'.

Meson then can pass the location of GDB to the test via an environment
variable: QEMU_TEST_GDB.

Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 configure                     | 21 +++++++++++++++++++++
 meson_options.txt             |  2 ++
 scripts/meson-buildoptions.sh |  2 ++
 tests/functional/meson.build  |  6 ++++++
 4 files changed, 31 insertions(+)

diff --git a/configure b/configure
index 0f7eb95586..20e05d233f 100755
--- a/configure
+++ b/configure
@@ -1142,12 +1142,31 @@ fi
 #########################################
 # gdb test
 
+# Check if all target arches are in a provided list of arches.
+is_target_arch_in_arch_list() {
+    arch_list=$1
+    for target in $target_list; do
+        arch=${target%%-*}
+        if test "${arch_list#*$arch}" = "$arch_list"; then
+            # Target arch not in arch list
+            return 1
+        fi
+    done
+    return 0
+}
+
 if test -n "$gdb_bin"; then
     gdb_version_string=$($gdb_bin --version | head -n 1)
     # Extract last field in the version string
     gdb_version=${gdb_version_string##* }
     if version_ge $gdb_version 9.1; then
         gdb_arches=$($python "$source_path/scripts/probe-gdb-support.py" $gdb_bin)
+
+	if is_target_arch_in_arch_list "$gdb_arches"; then
+            gdb_multiarch="yes"
+        else
+            gdb_multiarch=""
+	fi
     else
         gdb_bin=""
     fi
@@ -1984,6 +2003,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 "$gdb_multiarch" = yes && meson_option_add "-Dgdb=$gdb_bin"
+
   run_meson() {
     NINJA=$ninja $meson setup "$@" "$PWD" "$source_path"
   }
diff --git a/meson_options.txt b/meson_options.txt
index fff1521e58..5bb41bcbc4 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 0ebe6bc52a..f4bd21220e 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 2a0c5aa141..725630d308 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')
-- 
2.34.1

[PATCH v4 4/9] tests/functional: Add GDB class

[Gustavo Romero]

Add GDB class, which provides methods to run and capture GDB command
output. The GDB class is a wrapper around the pygdbmi module and
interacts with GDB via GDB's machine interface (MI).

Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
---
 tests/functional/qemu_test/__init__.py |  1 +
 tests/functional/qemu_test/gdb.py      | 85 ++++++++++++++++++++++++++
 2 files changed, 86 insertions(+)
 create mode 100644 tests/functional/qemu_test/gdb.py

diff --git a/tests/functional/qemu_test/__init__.py b/tests/functional/qemu_test/__init__.py
index 6e666a059f..60d19891bf 100644
--- a/tests/functional/qemu_test/__init__.py
+++ b/tests/functional/qemu_test/__init__.py
@@ -18,3 +18,4 @@
     skipIfMissingImports, skipIfOperatingSystem, skipLockedMemoryTest
 from .archive import archive_extract
 from .uncompress import uncompress
+from .gdb import GDB
diff --git a/tests/functional/qemu_test/gdb.py b/tests/functional/qemu_test/gdb.py
new file mode 100644
index 0000000000..15eb5eca18
--- /dev/null
+++ b/tests/functional/qemu_test/gdb.py
@@ -0,0 +1,85 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+#
+# A simple interface module built around pygdbmi for handling GDB commands.
+#
+# Copyright (c) 2025 Linaro Limited
+#
+# Author:
+#  Gustavo Romero <gustavo.romero@linaro.org>
+#
+# 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 re
+from pygdbmi.gdbcontroller import GdbController
+
+
+class GDB:
+    """Provides methods to run and capture GDB command output."""
+
+
+    def __init__(self, gdb_path, echo=True, suffix='# ', prompt="$ "):
+        gdb_cmd = [gdb_path, "-q", "--interpreter=mi2"]
+        self.gdbmi = GdbController(gdb_cmd)
+        self.echo = echo
+        self.suffix = suffix
+        self.prompt = prompt
+        self.response = None
+        self.cmd_output = None
+
+
+    def get_payload(self, response, kind):
+        output = []
+        for o in response:
+            # Unpack payloads of the same type.
+            _type, _, payload, *_ = o.values()
+            if _type == kind:
+                output += [payload]
+
+        # Some output lines do not end with \n but begin with it,
+        # so remove the leading \n and merge them with the next line
+        # that ends with \n.
+        lines = [line.lstrip('\n') for line in output]
+        lines = "".join(lines)
+        lines = lines.splitlines(keepends=True)
+
+        return lines
+
+
+    def cli(self, cmd, timeout=4.0):
+        self.response = self.gdbmi.write(cmd, timeout_sec=timeout)
+        self.cmd_output = self.get_payload(self.response, "console")
+        if self.echo:
+            print(self.suffix + self.prompt + cmd)
+
+            if len(self.cmd_output) > 0:
+                cmd_output = self.suffix.join(self.cmd_output)
+                print(self.suffix + cmd_output, end="")
+
+        return self
+
+
+    def get_addr(self):
+        address_pattern = r"0x[0-9A-Fa-f]+"
+        cmd_output = "".join(self.cmd_output) # Concat output lines.
+
+        match = re.search(address_pattern, cmd_output)
+
+        return int(match[0], 16) if match else None
+
+
+    def get_log(self):
+        r = self.get_payload(self.response, kind="log")
+        r = "".join(r)
+
+        return r
+
+
+    def get_console(self):
+        r = "".join(self.cmd_output)
+
+        return r
+
+
+    def exit(self):
+        self.gdbmi.exit()
-- 
2.34.1

[PATCH v4 5/9] tests/functional: replace avocado process with subprocess

[Gustavo Romero]

From: Daniel P. Berrangé <berrange@redhat.com>

The standard python subprocess.check_call method is better than
avocado.utils.process as it doesn't require stuffing all args
into a single string.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/functional/reverse_debugging.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tests/functional/reverse_debugging.py b/tests/functional/reverse_debugging.py
index f9a1d395f1..a7ff47cb90 100644
--- a/tests/functional/reverse_debugging.py
+++ b/tests/functional/reverse_debugging.py
@@ -11,6 +11,7 @@
 # later.  See the COPYING file in the top-level directory.
 import os
 import logging
+from subprocess import check_output
 
 from qemu_test import LinuxKernelTest, get_qemu_img
 from qemu_test.ports import Ports
@@ -100,7 +101,6 @@ def vm_get_icount(vm):
 
     def reverse_debugging(self, shift=7, args=None):
         from avocado.utils import gdb
-        from avocado.utils import process
 
         logger = logging.getLogger('replay')
 
@@ -111,8 +111,9 @@ def reverse_debugging(self, shift=7, args=None):
         if qemu_img is 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)
+        out = check_output([qemu_img, 'create', '-f', 'qcow2', image_path, '128M'],
+                           encoding='utf8')
+        logger.info("qemu-img: %s" % out)
 
         replay_path = os.path.join(self.workdir, 'replay.bin')
 
-- 
2.34.1

[PATCH v4 6/9] tests/functional: drop datadrainer class in reverse debugging

[Gustavo Romero]

From: Daniel P. Berrangé <berrange@redhat.com>

The reverse debugging test uses the avocado datadrainer class to
create a background thread that reads from the console socket and
dumps it via python logger.

Most tests log console output as a side effect of doing calls
to match strings, but this test never tries to match anything.

This isn't critical, so just drop the functionality.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/functional/reverse_debugging.py | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/tests/functional/reverse_debugging.py b/tests/functional/reverse_debugging.py
index a7ff47cb90..7fd8c7607f 100644
--- a/tests/functional/reverse_debugging.py
+++ b/tests/functional/reverse_debugging.py
@@ -34,8 +34,6 @@ class ReverseDebugging(LinuxKernelTest):
     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()
@@ -53,10 +51,6 @@ 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
-- 
2.34.1

[PATCH v4 7/9] tests/functional: Add decorator to skip test on missing env vars

[Gustavo Romero]

Add a decorator to skip tests on missing env variable(s). Multiple
variable names can be provided and if one or more of them are not set in
the test environment the test is skipped and the missing vars are
printed out.

Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
---
 tests/functional/qemu_test/__init__.py   |  3 ++-
 tests/functional/qemu_test/decorators.py | 18 ++++++++++++++++++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/tests/functional/qemu_test/__init__.py b/tests/functional/qemu_test/__init__.py
index 60d19891bf..320193591b 100644
--- a/tests/functional/qemu_test/__init__.py
+++ b/tests/functional/qemu_test/__init__.py
@@ -15,7 +15,8 @@
 from .linuxkernel import LinuxKernelTest
 from .decorators import skipIfMissingCommands, skipIfNotMachine, \
     skipFlakyTest, skipUntrustedTest, skipBigDataTest, skipSlowTest, \
-    skipIfMissingImports, skipIfOperatingSystem, skipLockedMemoryTest
+    skipIfMissingImports, skipIfOperatingSystem, skipLockedMemoryTest, \
+    skipIfMissingEnv
 from .archive import archive_extract
 from .uncompress import uncompress
 from .gdb import GDB
diff --git a/tests/functional/qemu_test/decorators.py b/tests/functional/qemu_test/decorators.py
index c0d1567b14..b239295804 100644
--- a/tests/functional/qemu_test/decorators.py
+++ b/tests/functional/qemu_test/decorators.py
@@ -11,6 +11,24 @@
 from .cmd import which
 
 '''
+Decorator to skip execution of a test if the provided
+environment variables are not set.
+Example:
+
+  @skipIfMissingEnv("QEMU_ENV_VAR0", "QEMU_ENV_VAR1")
+'''
+def skipIfMissingEnv(*vars_):
+    missing_vars = []
+    for var in vars_:
+        if os.getenv(var) == None:
+            missing_vars.append(var)
+
+    has_vars = True if len(missing_vars) == 0 else False
+
+    return skipUnless(has_vars, f"Missing env var(s): {', '.join(missing_vars)}")
+
+'''
+
 Decorator to skip execution of a test if the list
 of command binaries is not available in $PATH.
 Example:
-- 
2.34.1

[PATCH v4 8/9] tests/functional: Adapt reverse_debugging to run w/o Avocado

[Gustavo Romero]

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.

reverse_debugging now uses the pygdbmi module to interact with GDB, if
it's available in the test environment, otherwise the test is skipped.
GDB is detect via the QEMU_TEST_GDB env. variable.

This commit also significantly improves the output for the test and
now prints all the GDB commands used in sequence. It also adds
some clarifications to existing comments, for example, clarifying that
once the replay-break is reached, a SIGINT is captured in 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>
---
 tests/functional/reverse_debugging.py | 127 +++++++++++++-------------
 1 file changed, 61 insertions(+), 66 deletions(-)

diff --git a/tests/functional/reverse_debugging.py b/tests/functional/reverse_debugging.py
index 7fd8c7607f..4e4200dd48 100644
--- a/tests/functional/reverse_debugging.py
+++ b/tests/functional/reverse_debugging.py
@@ -1,19 +1,23 @@
-# 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
+from pygdbmi.constants import GdbTimeoutError
 from subprocess import check_output
 
-from qemu_test import LinuxKernelTest, get_qemu_img
+from qemu_test import LinuxKernelTest, get_qemu_img, GDB, skipIfMissingEnv
 from qemu_test.ports import Ports
 
 
@@ -29,9 +33,7 @@ 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):
         logger = logging.getLogger('replay')
@@ -53,49 +55,11 @@ def run_vm(self, record, shift, args, replay_path, image_path, port):
         vm.launch()
         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
-
-    @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(self, g):
-        return self.get_reg(g, self.REG_PC)
-
-    def check_pc(self, g, addr):
-        pc = self.get_pc(g)
-        if pc != addr:
-            self.fail('Invalid PC (read %x instead of %x)' % (pc, addr))
-
-    @staticmethod
-    def gdb_step(g):
-        g.cmd(b's', b'T05thread:01;')
-
-    @staticmethod
-    def gdb_bstep(g):
-        g.cmd(b'bs', b'T05thread:01;')
-
     @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
-
         logger = logging.getLogger('replay')
 
         # create qcow2 for snapshots
@@ -124,68 +88,99 @@ def reverse_debugging(self, shift=7, args=None):
         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:
+
+        try:
+            logger.info('Connecting to gdbstub...')
+            self.reverse_debugging_run(vm, port, last_icount)
+            logger.info('Test passed.')
+        except GdbTimeoutError:
+            self.fail("Connection to gdbstub timeouted...")
+
+    @skipIfMissingEnv("QEMU_TEST_GDB")
+    def reverse_debugging_run(self, vm, port, last_icount):
+        logger = logging.getLogger('replay')
+
+        gdb_cmd = os.getenv('QEMU_TEST_GDB')
+        gdb = GDB(gdb_cmd)
+
+        gdb.cli("set debug remote 1")
+
+        c = gdb.cli(f"target remote localhost:{port}").get_console()
+        if not f"Remote debugging using localhost:{port}" in c:
+            self.fail("Could not connect to gdbstub!")
+
+        # Remote debug messages are in 'log' payloads.
+        r = gdb.get_log()
+        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')
 
+        gdb.cli("set debug remote 0")
+
         logger.info('stepping forward')
         steps = []
         # record first instruction addresses
         for _ in range(self.STEPS):
-            pc = self.get_pc(g)
+            pc = gdb.cli("print $pc").get_addr()
             logger.info('saving position %x' % pc)
             steps.append(pc)
-            self.gdb_step(g)
+            gdb.cli("stepi")
 
         # 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)
+            gdb.cli("reverse-stepi")
+            pc = gdb.cli("print $pc").get_addr()
+            if pc != addr:
+                logger.info('Invalid PC (read %x instead of %x)' % (pc, addr))
+                self.fail('Reverse stepping failed!')
 
         # 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)
+            pc = gdb.cli("print $pc").get_addr()
+            if pc != addr:
+                logger.info('Invalid PC (read %x instead of %x)' % (pc, addr))
+                self.fail('Forward stepping failed!')
+            gdb.cli("stepi")
 
         # 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.cli(f"break *{hex(addr)}")
 
         # 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
-        # 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')
+        # This can stop at the end of the replay-break and gdb gets a SIGINT,
+        # or by re-executing one of the breakpoints and gdb stops at a
+        # breakpoint.
+        gdb.cli("continue")
+
+        pc = gdb.cli("print $pc").get_addr()
         if self.vm_get_icount(vm) == 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)))
+                        (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;')
+        gdb.cli("reverse-continue")
 
         # assume that none of the first instructions is executed again
         # breaking the order of the breakpoints
-        self.check_pc(g, steps[-1])
+        pc = gdb.cli("print $pc").get_addr()
+        if pc != steps[-1]:
+            self.fail("'reverse-continue' did not hit the first PC in reverse order!")
+
         logger.info('successfully reached %x' % steps[-1])
 
         logger.info('exiting gdb and qemu')
+        gdb.exit()
         vm.shutdown()
-- 
2.34.1

[PATCH v4 9/9] tests/functional: Adapt arches to reverse_debugging w/o Avocado

[Gustavo Romero]

reverse_debugging no longer depends on Avocado, so remove the import
checks for Avocado, the per-arch endianness tweaks, and the per-arch
register settings. All of these are now handled in the ReverseDebugging
class, automatically.

Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
---
 tests/functional/aarch64/test_reverse_debug.py |  9 ++++-----
 tests/functional/ppc64/test_reverse_debug.py   | 11 ++++-------
 tests/functional/x86_64/test_reverse_debug.py  | 13 ++++---------
 3 files changed, 12 insertions(+), 21 deletions(-)

diff --git a/tests/functional/aarch64/test_reverse_debug.py b/tests/functional/aarch64/test_reverse_debug.py
index 8bc91ccfde..36985a4a1d 100755
--- a/tests/functional/aarch64/test_reverse_debug.py
+++ b/tests/functional/aarch64/test_reverse_debug.py
@@ -2,25 +2,24 @@
 #
 # SPDX-License-Identifier: GPL-2.0-or-later
 #
-# Reverse debugging test
+# Reverse debugging test for aarch64
 #
 # 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.
 
-from qemu_test import Asset, skipIfMissingImports, skipFlakyTest
+from qemu_test import Asset, skipFlakyTest
 from reverse_debugging import ReverseDebugging
 
 
-@skipIfMissingImports('avocado.utils')
 class ReverseDebugging_AArch64(ReverseDebugging):
 
-    REG_PC = 32
-
     ASSET_KERNEL = Asset(
         ('https://archives.fedoraproject.org/pub/archive/fedora/linux/'
          'releases/29/Everything/aarch64/os/images/pxeboot/vmlinuz'),
diff --git a/tests/functional/ppc64/test_reverse_debug.py b/tests/functional/ppc64/test_reverse_debug.py
index 5931adef5a..b32a186a9a 100755
--- a/tests/functional/ppc64/test_reverse_debug.py
+++ b/tests/functional/ppc64/test_reverse_debug.py
@@ -2,38 +2,35 @@
 #
 # SPDX-License-Identifier: GPL-2.0-or-later
 #
-# Reverse debugging test
+# Reverse debugging test for ppc64
 #
 # 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.
 
-from qemu_test import skipIfMissingImports, skipFlakyTest
+from qemu_test import skipFlakyTest
 from reverse_debugging import ReverseDebugging
 
 
-@skipIfMissingImports('avocado.utils')
 class ReverseDebugging_ppc64(ReverseDebugging):
 
-    REG_PC = 0x40
-
     @skipFlakyTest("https://gitlab.com/qemu-project/qemu/-/issues/1992")
     def test_ppc64_pseries(self):
         self.set_machine('pseries')
         # 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()
 
     @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()
 
 
diff --git a/tests/functional/x86_64/test_reverse_debug.py b/tests/functional/x86_64/test_reverse_debug.py
index d713e91e14..63d08bbada 100755
--- a/tests/functional/x86_64/test_reverse_debug.py
+++ b/tests/functional/x86_64/test_reverse_debug.py
@@ -2,29 +2,24 @@
 #
 # SPDX-License-Identifier: GPL-2.0-or-later
 #
-# Reverse debugging test
+# Reverse debugging test for x86_64
 #
 # 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.
 
-from qemu_test import skipIfMissingImports, skipFlakyTest
+from qemu_test import skipFlakyTest
 from reverse_debugging import ReverseDebugging
 
 
-@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
-
     @skipFlakyTest("https://gitlab.com/qemu-project/qemu/-/issues/2922")
     def test_x86_64_pc(self):
         self.set_machine('pc')
-- 
2.34.1

Re: [PATCH v4 1/9] tests/functional: Re-activate the check-venv target

[Thomas Huth]

On 26/09/2025 07.15, Gustavo Romero wrote:
> Add check-venv target as a dependency for the functional tests. This
> causes Python modules listed in pythondeps.toml, under the testdeps
> group, to be installed when 'make check-functional' is executed to
> prepare and run the functional tests.
> 
> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> Suggested-by: Thomas Huth <thuth@redhat.com>
> ---
>   tests/Makefile.include | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 3538c0c740..d012a9b25d 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -109,7 +109,7 @@ $(FUNCTIONAL_TARGETS):
>   	@$(MAKE) SPEED=thorough $(subst -functional,-func,$@)
>   
>   .PHONY: check-functional
> -check-functional:
> +check-functional: check-venv
>   	@$(NINJA) precache-functional
>   	@QEMU_TEST_NO_DOWNLOAD=1 $(MAKE) SPEED=thorough check-func check-func-quick

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v4 2/9] python: Install pygdbmi in meson's venv

[Thomas Huth]

On 26/09/2025 07.15, Gustavo Romero wrote:
> The upcoming changes in the reverse_debugging functional test to remove
> Avocado as a dependency will require pygdbmi for interacting with GDB,
> so install it in meson's venv (located in the build dir's pyvenv/).
> 
> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> ---
>   pythondeps.toml | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/pythondeps.toml b/pythondeps.toml
> index 16fb2a989c..98e99e7900 100644
> --- a/pythondeps.toml
> +++ b/pythondeps.toml
> @@ -33,3 +33,4 @@ sphinx_rtd_theme = { accepted = ">=0.5", installed = "1.2.2" }
>   
>   [testdeps]
>   qemu.qmp = { accepted = ">=0.0.3", installed = "0.0.3" }
> +pygdbmi = { accepted = ">=0.11.0.0", installed = "0.11.0.0" }

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v4 0/9] tests/functional: Adapt reverse_debugging to run w/o Avocado

[Philippe Mathieu-Daudé]

Hi Gustavo,

On 26/9/25 07:15, Gustavo Romero wrote:
> tests/functional: Adapt reverse_debugging to run w/o Avocado
> 
> The goal of this series is to remove Avocado as a dependency for running
> the reverse_debugging functional test.


> Daniel P. Berrangé (2):
>    tests/functional: replace avocado process with subprocess
>    tests/functional: drop datadrainer class in reverse debugging
> 
> Gustavo Romero (7):
>    tests/functional: Re-activate the check-venv target
>    python: Install pygdbmi in meson's venv
>    tests/functional: Provide GDB to the functional tests
>    tests/functional: Add GDB class
>    tests/functional: Add decorator to skip test on missing env vars
>    tests/functional: Adapt reverse_debugging to run w/o Avocado
>    tests/functional: Adapt arches to reverse_debugging w/o Avocado

Out of curiosity, do you plan to post the final patch removing Avocado
use / dependency?

Regards,

Phil.

Re: [PATCH v4 4/9] tests/functional: Add GDB class

[Thomas Huth]

On 26/09/2025 07.15, Gustavo Romero wrote:
> Add GDB class, which provides methods to run and capture GDB command
> output. The GDB class is a wrapper around the pygdbmi module and
> interacts with GDB via GDB's machine interface (MI).
> 
> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> ---
...
> +    def cli(self, cmd, timeout=4.0):

Please consider that tests might run on heavily overloaded CI runners ... my 
gut feeling, mixed with experience from the past: Rather use timeouts in the 
ballpark of 10 seconds or more, even for stuff that normally finishes within 
one second.

Apart from that:
Acked-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v4 5/9] tests/functional: replace avocado process with subprocess

[Thomas Huth]

On 26/09/2025 07.15, Gustavo Romero wrote:
> From: Daniel P. Berrangé <berrange@redhat.com>
> 
> The standard python subprocess.check_call method is better than
> avocado.utils.process as it doesn't require stuffing all args
> into a single string.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   tests/functional/reverse_debugging.py | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/functional/reverse_debugging.py b/tests/functional/reverse_debugging.py
> index f9a1d395f1..a7ff47cb90 100644
> --- a/tests/functional/reverse_debugging.py
> +++ b/tests/functional/reverse_debugging.py
> @@ -11,6 +11,7 @@
>   # later.  See the COPYING file in the top-level directory.
>   import os
>   import logging
> +from subprocess import check_output
>   
>   from qemu_test import LinuxKernelTest, get_qemu_img
>   from qemu_test.ports import Ports
> @@ -100,7 +101,6 @@ def vm_get_icount(vm):
>   
>       def reverse_debugging(self, shift=7, args=None):
>           from avocado.utils import gdb
> -        from avocado.utils import process
>   
>           logger = logging.getLogger('replay')
>   
> @@ -111,8 +111,9 @@ def reverse_debugging(self, shift=7, args=None):
>           if qemu_img is 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)
> +        out = check_output([qemu_img, 'create', '-f', 'qcow2', image_path, '128M'],
> +                           encoding='utf8')
> +        logger.info("qemu-img: %s" % out)
>   
>           replay_path = os.path.join(self.workdir, 'replay.bin')

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v4 6/9] tests/functional: drop datadrainer class in reverse debugging

[Thomas Huth]

On 26/09/2025 07.15, Gustavo Romero wrote:
> From: Daniel P. Berrangé <berrange@redhat.com>
> 
> The reverse debugging test uses the avocado datadrainer class to
> create a background thread that reads from the console socket and
> dumps it via python logger.
> 
> Most tests log console output as a side effect of doing calls
> to match strings, but this test never tries to match anything.
> 
> This isn't critical, so just drop the functionality.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v4 7/9] tests/functional: Add decorator to skip test on missing env vars

[Thomas Huth]

On 26/09/2025 07.15, Gustavo Romero wrote:
> Add a decorator to skip tests on missing env variable(s). Multiple
> variable names can be provided and if one or more of them are not set in
> the test environment the test is skipped and the missing vars are
> printed out.
> 
> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> ---
>   tests/functional/qemu_test/__init__.py   |  3 ++-
>   tests/functional/qemu_test/decorators.py | 18 ++++++++++++++++++
>   2 files changed, 20 insertions(+), 1 deletion(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v4 1/9] tests/functional: Re-activate the check-venv target

[Thomas Huth]

On 26/09/2025 07.15, Gustavo Romero wrote:
> Add check-venv target as a dependency for the functional tests. This
> causes Python modules listed in pythondeps.toml, under the testdeps
> group, to be installed when 'make check-functional' is executed to
> prepare and run the functional tests.
> 
> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> Suggested-by: Thomas Huth <thuth@redhat.com>
> ---
>   tests/Makefile.include | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 3538c0c740..d012a9b25d 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -109,7 +109,7 @@ $(FUNCTIONAL_TARGETS):
>   	@$(MAKE) SPEED=thorough $(subst -functional,-func,$@)
>   
>   .PHONY: check-functional
> -check-functional:
> +check-functional: check-venv

I just noticed that there's still a problem: If you run "make 
check-functional-aarch64" immediately after configuring + compiling QEMU in 
a fresh folder for the first time, the functional tests fail with:

ModuleNotFoundError: No module named 'pygdbmi'

We either need to add dependencies to the check-functional-<arch> targets, 
too, or we have to make sure that tests still get properly skipped in the 
case that pygdbmi has not been installed into the venv yet.

  Thomas

Re: [PATCH v4 1/9] tests/functional: Re-activate the check-venv target

[Daniel P. Berrangé]

On Fri, Sep 26, 2025 at 10:34:01AM +0200, Thomas Huth wrote:
> On 26/09/2025 07.15, Gustavo Romero wrote:
> > Add check-venv target as a dependency for the functional tests. This
> > causes Python modules listed in pythondeps.toml, under the testdeps
> > group, to be installed when 'make check-functional' is executed to
> > prepare and run the functional tests.
> > 
> > Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> > Suggested-by: Thomas Huth <thuth@redhat.com>
> > ---
> >   tests/Makefile.include | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > index 3538c0c740..d012a9b25d 100644
> > --- a/tests/Makefile.include
> > +++ b/tests/Makefile.include
> > @@ -109,7 +109,7 @@ $(FUNCTIONAL_TARGETS):
> >   	@$(MAKE) SPEED=thorough $(subst -functional,-func,$@)
> >   .PHONY: check-functional
> > -check-functional:
> > +check-functional: check-venv
> 
> I just noticed that there's still a problem: If you run "make
> check-functional-aarch64" immediately after configuring + compiling QEMU in
> a fresh folder for the first time, the functional tests fail with:
> 
> ModuleNotFoundError: No module named 'pygdbmi'
> 
> We either need to add dependencies to the check-functional-<arch> targets,
> too, or we have to make sure that tests still get properly skipped in the
> case that pygdbmi has not been installed into the venv yet.

We already have a decorator for skipping tests when modules are missing,
so we should add usage of that.

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 :|

Re: [PATCH v4 1/9] tests/functional: Re-activate the check-venv target

[Thomas Huth]

On 26/09/2025 10.37, Daniel P. Berrangé wrote:
> On Fri, Sep 26, 2025 at 10:34:01AM +0200, Thomas Huth wrote:
>> On 26/09/2025 07.15, Gustavo Romero wrote:
>>> Add check-venv target as a dependency for the functional tests. This
>>> causes Python modules listed in pythondeps.toml, under the testdeps
>>> group, to be installed when 'make check-functional' is executed to
>>> prepare and run the functional tests.
>>>
>>> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
>>> Suggested-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>    tests/Makefile.include | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>>> index 3538c0c740..d012a9b25d 100644
>>> --- a/tests/Makefile.include
>>> +++ b/tests/Makefile.include
>>> @@ -109,7 +109,7 @@ $(FUNCTIONAL_TARGETS):
>>>    	@$(MAKE) SPEED=thorough $(subst -functional,-func,$@)
>>>    .PHONY: check-functional
>>> -check-functional:
>>> +check-functional: check-venv
>>
>> I just noticed that there's still a problem: If you run "make
>> check-functional-aarch64" immediately after configuring + compiling QEMU in
>> a fresh folder for the first time, the functional tests fail with:
>>
>> ModuleNotFoundError: No module named 'pygdbmi'
>>
>> We either need to add dependencies to the check-functional-<arch> targets,
>> too, or we have to make sure that tests still get properly skipped in the
>> case that pygdbmi has not been installed into the venv yet.
> 
> We already have a decorator for skipping tests when modules are missing,
> so we should add usage of that.

Ack ... and the "from .gdb import GDB" in qemu_test/__init__.py likely also 
has to go away, to avoid that each and every test tries to pull in the gdb code.

  Thomas

Re: [PATCH v4 8/9] tests/functional: Adapt reverse_debugging to run w/o Avocado

[Thomas Huth]

On 26/09/2025 07.15, Gustavo Romero wrote:
> 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.
> 
> reverse_debugging now uses the pygdbmi module to interact with GDB, if
> it's available in the test environment, otherwise the test is skipped.
> GDB is detect via the QEMU_TEST_GDB env. variable.
> 
> This commit also significantly improves the output for the test and
> now prints all the GDB commands used in sequence. It also adds
> some clarifications to existing comments, for example, clarifying that
> once the replay-break is reached, a SIGINT is captured in 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>
> ---
...
> @@ -53,49 +55,11 @@ def run_vm(self, record, shift, args, replay_path, image_path, port):
>           vm.launch()
>           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
> -
> -    @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(self, g):
> -        return self.get_reg(g, self.REG_PC)
> -
> -    def check_pc(self, g, addr):
> -        pc = self.get_pc(g)
> -        if pc != addr:
> -            self.fail('Invalid PC (read %x instead of %x)' % (pc, addr))

I think it would make sense to keep wrapper functions get_pc() and 
check_pc(), since that functionality is still used in a bunch of places 
(e.g. "gdb.cli("print $pc").get_addr()" is used a couple of times).

> @@ -124,68 +88,99 @@ def reverse_debugging(self, shift=7, args=None):
>           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:
> +
> +        try:
> +            logger.info('Connecting to gdbstub...')
> +            self.reverse_debugging_run(vm, port, last_icount)
> +            logger.info('Test passed.')
> +        except GdbTimeoutError:
> +            self.fail("Connection to gdbstub timeouted...")

I'm not a native English speaker, but I think "timeout" is not a verb. So 
maybe better: "Timeout while connecting to gdbstub" or something similar?

> +    @skipIfMissingEnv("QEMU_TEST_GDB")

Somehow this SKIP is always triggered on my system, even if I correctly 
pointed "configure" to the right GDB with the "--gdb" option ... not sure 
why this happens, but I'll try to find out...

  Thomas

Re: [PATCH v4 1/9] tests/functional: Re-activate the check-venv target

[Daniel P. Berrangé]

On Fri, Sep 26, 2025 at 10:42:22AM +0200, Thomas Huth wrote:
> On 26/09/2025 10.37, Daniel P. Berrangé wrote:
> > On Fri, Sep 26, 2025 at 10:34:01AM +0200, Thomas Huth wrote:
> > > On 26/09/2025 07.15, Gustavo Romero wrote:
> > > > Add check-venv target as a dependency for the functional tests. This
> > > > causes Python modules listed in pythondeps.toml, under the testdeps
> > > > group, to be installed when 'make check-functional' is executed to
> > > > prepare and run the functional tests.
> > > > 
> > > > Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> > > > Suggested-by: Thomas Huth <thuth@redhat.com>
> > > > ---
> > > >    tests/Makefile.include | 2 +-
> > > >    1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > > > index 3538c0c740..d012a9b25d 100644
> > > > --- a/tests/Makefile.include
> > > > +++ b/tests/Makefile.include
> > > > @@ -109,7 +109,7 @@ $(FUNCTIONAL_TARGETS):
> > > >    	@$(MAKE) SPEED=thorough $(subst -functional,-func,$@)
> > > >    .PHONY: check-functional
> > > > -check-functional:
> > > > +check-functional: check-venv
> > > 
> > > I just noticed that there's still a problem: If you run "make
> > > check-functional-aarch64" immediately after configuring + compiling QEMU in
> > > a fresh folder for the first time, the functional tests fail with:
> > > 
> > > ModuleNotFoundError: No module named 'pygdbmi'
> > > 
> > > We either need to add dependencies to the check-functional-<arch> targets,
> > > too, or we have to make sure that tests still get properly skipped in the
> > > case that pygdbmi has not been installed into the venv yet.
> > 
> > We already have a decorator for skipping tests when modules are missing,
> > so we should add usage of that.
> 
> Ack ... and the "from .gdb import GDB" in qemu_test/__init__.py likely also
> has to go away, to avoid that each and every test tries to pull in the gdb
> code.

Or alternatively the gdb module can move the gdbmi import so that it is
only referenced in method scope, so it becomes relevant only when
executed.


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 :|

Re: [PATCH v4 9/9] tests/functional: Adapt arches to reverse_debugging w/o Avocado

[Thomas Huth]

On 26/09/2025 07.15, Gustavo Romero wrote:
> reverse_debugging no longer depends on Avocado, so remove the import
> checks for Avocado, the per-arch endianness tweaks, and the per-arch
> register settings. All of these are now handled in the ReverseDebugging
> class, automatically.
> 
> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> ---
>   tests/functional/aarch64/test_reverse_debug.py |  9 ++++-----
>   tests/functional/ppc64/test_reverse_debug.py   | 11 ++++-------
>   tests/functional/x86_64/test_reverse_debug.py  | 13 ++++---------
>   3 files changed, 12 insertions(+), 21 deletions(-)
> 
> diff --git a/tests/functional/aarch64/test_reverse_debug.py b/tests/functional/aarch64/test_reverse_debug.py
> index 8bc91ccfde..36985a4a1d 100755
> --- a/tests/functional/aarch64/test_reverse_debug.py
> +++ b/tests/functional/aarch64/test_reverse_debug.py
> @@ -2,25 +2,24 @@
>   #
>   # SPDX-License-Identifier: GPL-2.0-or-later
>   #
> -# Reverse debugging test
> +# Reverse debugging test for aarch64
>   #
>   # 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.
>   
> -from qemu_test import Asset, skipIfMissingImports, skipFlakyTest
> +from qemu_test import Asset, skipFlakyTest
>   from reverse_debugging import ReverseDebugging
>   
>   
> -@skipIfMissingImports('avocado.utils')
>   class ReverseDebugging_AArch64(ReverseDebugging):

Maybe use @skipIfMissingImports('pygdbmi') now?

Apart from that:
Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v4 0/9] tests/functional: Adapt reverse_debugging to run w/o Avocado

[Thomas Huth]

On 26/09/2025 08.49, Philippe Mathieu-Daudé wrote:
> Hi Gustavo,
> 
> On 26/9/25 07:15, Gustavo Romero wrote:
>> tests/functional: Adapt reverse_debugging to run w/o Avocado
>>
>> The goal of this series is to remove Avocado as a dependency for running
>> the reverse_debugging functional test.
> 
> 
>> Daniel P. Berrangé (2):
>>    tests/functional: replace avocado process with subprocess
>>    tests/functional: drop datadrainer class in reverse debugging
>>
>> Gustavo Romero (7):
>>    tests/functional: Re-activate the check-venv target
>>    python: Install pygdbmi in meson's venv
>>    tests/functional: Provide GDB to the functional tests
>>    tests/functional: Add GDB class
>>    tests/functional: Add decorator to skip test on missing env vars
>>    tests/functional: Adapt reverse_debugging to run w/o Avocado
>>    tests/functional: Adapt arches to reverse_debugging w/o Avocado
> 
> Out of curiosity, do you plan to post the final patch removing Avocado
> use / dependency?

Which other uses of Avocado are you thinking about? AFAIK, this test here is 
the last one that used Avocado.

  Thomas

Re: [PATCH v4 0/9] tests/functional: Adapt reverse_debugging to run w/o Avocado

[Philippe Mathieu-Daudé]

On 26/9/25 11:14, Thomas Huth wrote:
> On 26/09/2025 08.49, Philippe Mathieu-Daudé wrote:
>> Hi Gustavo,
>>
>> On 26/9/25 07:15, Gustavo Romero wrote:
>>> tests/functional: Adapt reverse_debugging to run w/o Avocado
>>>
>>> The goal of this series is to remove Avocado as a dependency for running
>>> the reverse_debugging functional test.
>>
>>
>>> Daniel P. Berrangé (2):
>>>    tests/functional: replace avocado process with subprocess
>>>    tests/functional: drop datadrainer class in reverse debugging
>>>
>>> Gustavo Romero (7):
>>>    tests/functional: Re-activate the check-venv target
>>>    python: Install pygdbmi in meson's venv
>>>    tests/functional: Provide GDB to the functional tests
>>>    tests/functional: Add GDB class
>>>    tests/functional: Add decorator to skip test on missing env vars
>>>    tests/functional: Adapt reverse_debugging to run w/o Avocado
>>>    tests/functional: Adapt arches to reverse_debugging w/o Avocado
>>
>> Out of curiosity, do you plan to post the final patch removing Avocado
>> use / dependency?
> 
> Which other uses of Avocado are you thinking about? AFAIK, this test 
> here is the last one that used Avocado.

Maybe I was not clear. After these tests conversion, I don't see any
more use of avocado, so we can remove its dependency on QEMU, right?
Basically, in a final patch I'd remove anything related to:

   python/setup.cfg:37:    avocado-framework >= 90.0
   python/tests/minreqs.txt:35:avocado-framework==90.0

Re: [PATCH v4 0/9] tests/functional: Adapt reverse_debugging to run w/o Avocado

[Daniel P. Berrangé]

On Fri, Sep 26, 2025 at 11:32:58AM +0200, Philippe Mathieu-Daudé wrote:
> On 26/9/25 11:14, Thomas Huth wrote:
> > On 26/09/2025 08.49, Philippe Mathieu-Daudé wrote:
> > > Hi Gustavo,
> > > 
> > > On 26/9/25 07:15, Gustavo Romero wrote:
> > > > tests/functional: Adapt reverse_debugging to run w/o Avocado
> > > > 
> > > > The goal of this series is to remove Avocado as a dependency for running
> > > > the reverse_debugging functional test.
> > > 
> > > 
> > > > Daniel P. Berrangé (2):
> > > >    tests/functional: replace avocado process with subprocess
> > > >    tests/functional: drop datadrainer class in reverse debugging
> > > > 
> > > > Gustavo Romero (7):
> > > >    tests/functional: Re-activate the check-venv target
> > > >    python: Install pygdbmi in meson's venv
> > > >    tests/functional: Provide GDB to the functional tests
> > > >    tests/functional: Add GDB class
> > > >    tests/functional: Add decorator to skip test on missing env vars
> > > >    tests/functional: Adapt reverse_debugging to run w/o Avocado
> > > >    tests/functional: Adapt arches to reverse_debugging w/o Avocado
> > > 
> > > Out of curiosity, do you plan to post the final patch removing Avocado
> > > use / dependency?
> > 
> > Which other uses of Avocado are you thinking about? AFAIK, this test
> > here is the last one that used Avocado.
> 
> Maybe I was not clear. After these tests conversion, I don't see any
> more use of avocado, so we can remove its dependency on QEMU, right?
> Basically, in a final patch I'd remove anything related to:
> 
>   python/setup.cfg:37:    avocado-framework >= 90.0
>   python/tests/minreqs.txt:35:avocado-framework==90.0

The python code CI jobs all rely on avocado. In the python-qemu-qmp
repo John has a patch that drops avocado. That can be pulled over
into QEMU, but it is likely John will propose removing the python/
directory from QEMU instead and using a wheel.

Either way, this patch series doesn't need to touch python/ dir,
as work on that is in-progress separately.

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 :|

Re: [PATCH v4 0/9] tests/functional: Adapt reverse_debugging to run w/o Avocado

[Thomas Huth]

On 26/09/2025 11.32, Philippe Mathieu-Daudé wrote:
> On 26/9/25 11:14, Thomas Huth wrote:
>> On 26/09/2025 08.49, Philippe Mathieu-Daudé wrote:
>>> Hi Gustavo,
>>>
>>> On 26/9/25 07:15, Gustavo Romero wrote:
>>>> tests/functional: Adapt reverse_debugging to run w/o Avocado
>>>>
>>>> The goal of this series is to remove Avocado as a dependency for running
>>>> the reverse_debugging functional test.
>>>
>>>
>>>> Daniel P. Berrangé (2):
>>>>    tests/functional: replace avocado process with subprocess
>>>>    tests/functional: drop datadrainer class in reverse debugging
>>>>
>>>> Gustavo Romero (7):
>>>>    tests/functional: Re-activate the check-venv target
>>>>    python: Install pygdbmi in meson's venv
>>>>    tests/functional: Provide GDB to the functional tests
>>>>    tests/functional: Add GDB class
>>>>    tests/functional: Add decorator to skip test on missing env vars
>>>>    tests/functional: Adapt reverse_debugging to run w/o Avocado
>>>>    tests/functional: Adapt arches to reverse_debugging w/o Avocado
>>>
>>> Out of curiosity, do you plan to post the final patch removing Avocado
>>> use / dependency?
>>
>> Which other uses of Avocado are you thinking about? AFAIK, this test here 
>> is the last one that used Avocado.
> 
> Maybe I was not clear. After these tests conversion, I don't see any
> more use of avocado, so we can remove its dependency on QEMU, right?
> Basically, in a final patch I'd remove anything related to:
> 
>    python/setup.cfg:37:    avocado-framework >= 90.0
>    python/tests/minreqs.txt:35:avocado-framework==90.0

I think that's related to the qemu.qmp stuff coming from 
https://gitlab.com/qemu-project/python-qemu-qmp ... so maybe better sync 
with John Snow whether it's OK to remove that?

  Thomas

Re: [PATCH v4 3/9] tests/functional: Provide GDB to the functional tests

[Thomas Huth]

On 26/09/2025 07.15, Gustavo Romero wrote:
> The probe of GDB is done in 'configure' and the full path is passed to
> meson.build via the -Dgdb=option.
> 
> Because a single functional test can cover different arches, such as
> aarch64, ppc64, and x86_64, only a GDB that supports all the arches in
> the target list is passed to Meson for use in the functional tests. To
> handle this check, a new shell function, is_target_arch_in_arch_list, is
> introduced in 'configure'.
> 
> Meson then can pass the location of GDB to the test via an environment
> variable: QEMU_TEST_GDB.
> 
> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   configure                     | 21 +++++++++++++++++++++
>   meson_options.txt             |  2 ++
>   scripts/meson-buildoptions.sh |  2 ++
>   tests/functional/meson.build  |  6 ++++++
>   4 files changed, 31 insertions(+)
> 
> diff --git a/configure b/configure
> index 0f7eb95586..20e05d233f 100755
> --- a/configure
> +++ b/configure
> @@ -1142,12 +1142,31 @@ fi
>   #########################################
>   # gdb test
>   
> +# Check if all target arches are in a provided list of arches.
> +is_target_arch_in_arch_list() {
> +    arch_list=$1
> +    for target in $target_list; do
> +        arch=${target%%-*}
> +        if test "${arch_list#*$arch}" = "$arch_list"; then
> +            # Target arch not in arch list
> +            return 1
> +        fi
> +    done
> +    return 0
> +}
> +
>   if test -n "$gdb_bin"; then
>       gdb_version_string=$($gdb_bin --version | head -n 1)
>       # Extract last field in the version string
>       gdb_version=${gdb_version_string##* }
>       if version_ge $gdb_version 9.1; then
>           gdb_arches=$($python "$source_path/scripts/probe-gdb-support.py" $gdb_bin)
> +
> +	if is_target_arch_in_arch_list "$gdb_arches"; then

No TABs, please!

> +            gdb_multiarch="yes"
> +        else
> +            gdb_multiarch=""
> +	fi

This unfortunately does not work with the GDB from Fedora - it only supports 
"arch64_be arm riscv64 riscv32 ppc i386 s390x ppc64 aarch64 ppc64le x86_64", 
but if you configured a target like "alpha-softmmu", this breaks.
(BTW, does the gdb-multiarch from Debian/Ubuntu really also support exotic 
QEMU targets like tricore?)

I think it would be better to drop this hunk, and rather check in the spot 
where we use GDB if the required target is really there (i.e. in the 
functional test that uses it).

  Thomas

Re: [PATCH v4 1/9] tests/functional: Re-activate the check-venv target

[Gustavo Romero]

Hi Thomas!

On 9/26/25 05:34, Thomas Huth wrote:
> On 26/09/2025 07.15, Gustavo Romero wrote:
>> Add check-venv target as a dependency for the functional tests. This
>> causes Python modules listed in pythondeps.toml, under the testdeps
>> group, to be installed when 'make check-functional' is executed to
>> prepare and run the functional tests.
>>
>> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
>> Suggested-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   tests/Makefile.include | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>> index 3538c0c740..d012a9b25d 100644
>> --- a/tests/Makefile.include
>> +++ b/tests/Makefile.include
>> @@ -109,7 +109,7 @@ $(FUNCTIONAL_TARGETS):
>>       @$(MAKE) SPEED=thorough $(subst -functional,-func,$@)
>>   .PHONY: check-functional
>> -check-functional:
>> +check-functional: check-venv
> 
> I just noticed that there's still a problem: If you run "make check-functional-aarch64" immediately after configuring + compiling QEMU in a fresh folder for the first time, the functional tests fail with:
> 
> ModuleNotFoundError: No module named 'pygdbmi'
> 
> We either need to add dependencies to the check-functional-<arch> targets, too, or we have to make sure that tests still get properly skipped in the case that pygdbmi has not been installed into the venv yet.

Isn't it inconsistent that check-functional runs the test and
check-functional-<arch> doesn't? I think it's a good idea to
skip if the module is not available, yeah, I'll add it in v6,
but would it be ok to add check-venv to the check-functional-<arch>
targets too? That solution feels a tad cumbersome to me to make
them consistent but really I don't have any better idea...


Cheers,
Gustavo

Re: [PATCH v4 1/9] tests/functional: Re-activate the check-venv target

[Gustavo Romero]

Hi Daniel,

On 9/26/25 05:50, Daniel P. Berrangé wrote:
> On Fri, Sep 26, 2025 at 10:42:22AM +0200, Thomas Huth wrote:
>> On 26/09/2025 10.37, Daniel P. Berrangé wrote:
>>> On Fri, Sep 26, 2025 at 10:34:01AM +0200, Thomas Huth wrote:
>>>> On 26/09/2025 07.15, Gustavo Romero wrote:
>>>>> Add check-venv target as a dependency for the functional tests. This
>>>>> causes Python modules listed in pythondeps.toml, under the testdeps
>>>>> group, to be installed when 'make check-functional' is executed to
>>>>> prepare and run the functional tests.
>>>>>
>>>>> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
>>>>> Suggested-by: Thomas Huth <thuth@redhat.com>
>>>>> ---
>>>>>     tests/Makefile.include | 2 +-
>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>>>>> index 3538c0c740..d012a9b25d 100644
>>>>> --- a/tests/Makefile.include
>>>>> +++ b/tests/Makefile.include
>>>>> @@ -109,7 +109,7 @@ $(FUNCTIONAL_TARGETS):
>>>>>     	@$(MAKE) SPEED=thorough $(subst -functional,-func,$@)
>>>>>     .PHONY: check-functional
>>>>> -check-functional:
>>>>> +check-functional: check-venv
>>>>
>>>> I just noticed that there's still a problem: If you run "make
>>>> check-functional-aarch64" immediately after configuring + compiling QEMU in
>>>> a fresh folder for the first time, the functional tests fail with:
>>>>
>>>> ModuleNotFoundError: No module named 'pygdbmi'
>>>>
>>>> We either need to add dependencies to the check-functional-<arch> targets,
>>>> too, or we have to make sure that tests still get properly skipped in the
>>>> case that pygdbmi has not been installed into the venv yet.
>>>
>>> We already have a decorator for skipping tests when modules are missing,
>>> so we should add usage of that.
>>
>> Ack ... and the "from .gdb import GDB" in qemu_test/__init__.py likely also
>> has to go away, to avoid that each and every test tries to pull in the gdb
>> code.
> 
> Or alternatively the gdb module can move the gdbmi import so that it is
> only referenced in method scope, so it becomes relevant only when
> executed.

I can´t follow what you meant here. Do mind expanding on it a bit?


Cheers,
Gustavo

Re: [PATCH v4 1/9] tests/functional: Re-activate the check-venv target

[Daniel P. Berrangé]

On Fri, Sep 26, 2025 at 12:44:58PM -0300, Gustavo Romero wrote:
> Hi Daniel,
> 
> On 9/26/25 05:50, Daniel P. Berrangé wrote:
> > On Fri, Sep 26, 2025 at 10:42:22AM +0200, Thomas Huth wrote:
> > > On 26/09/2025 10.37, Daniel P. Berrangé wrote:
> > > > On Fri, Sep 26, 2025 at 10:34:01AM +0200, Thomas Huth wrote:
> > > > > On 26/09/2025 07.15, Gustavo Romero wrote:
> > > > > > Add check-venv target as a dependency for the functional tests. This
> > > > > > causes Python modules listed in pythondeps.toml, under the testdeps
> > > > > > group, to be installed when 'make check-functional' is executed to
> > > > > > prepare and run the functional tests.
> > > > > > 
> > > > > > Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> > > > > > Suggested-by: Thomas Huth <thuth@redhat.com>
> > > > > > ---
> > > > > >     tests/Makefile.include | 2 +-
> > > > > >     1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > 
> > > > > > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > > > > > index 3538c0c740..d012a9b25d 100644
> > > > > > --- a/tests/Makefile.include
> > > > > > +++ b/tests/Makefile.include
> > > > > > @@ -109,7 +109,7 @@ $(FUNCTIONAL_TARGETS):
> > > > > >     	@$(MAKE) SPEED=thorough $(subst -functional,-func,$@)
> > > > > >     .PHONY: check-functional
> > > > > > -check-functional:
> > > > > > +check-functional: check-venv
> > > > > 
> > > > > I just noticed that there's still a problem: If you run "make
> > > > > check-functional-aarch64" immediately after configuring + compiling QEMU in
> > > > > a fresh folder for the first time, the functional tests fail with:
> > > > > 
> > > > > ModuleNotFoundError: No module named 'pygdbmi'
> > > > > 
> > > > > We either need to add dependencies to the check-functional-<arch> targets,
> > > > > too, or we have to make sure that tests still get properly skipped in the
> > > > > case that pygdbmi has not been installed into the venv yet.
> > > > 
> > > > We already have a decorator for skipping tests when modules are missing,
> > > > so we should add usage of that.
> > > 
> > > Ack ... and the "from .gdb import GDB" in qemu_test/__init__.py likely also
> > > has to go away, to avoid that each and every test tries to pull in the gdb
> > > code.
> > 
> > Or alternatively the gdb module can move the gdbmi import so that it is
> > only referenced in method scope, so it becomes relevant only when
> > executed.
> 
> I can´t follow what you meant here. Do mind expanding on it a bit?

The code currently does:

  from pygdbmi.gdbcontroller import GdbController

  class GDB:
      def __init__(self, gdb_path, echo=True, suffix='# ', prompt="$ "):
          gdb_cmd = [gdb_path, "-q", "--interpreter=mi2"]
          self.gdbmi = GdbController(gdb_cmd)
          self.echo = echo

but it could instead do

  class GDB:
      def __init__(self, gdb_path, echo=True, suffix='# ', prompt="$ "):
          gdb_cmd = [gdb_path, "-q", "--interpreter=mi2"]
          from pygdbmi.gdbcontroller import GdbController
          self.gdbmi = GdbController(gdb_cmd)
          self.echo = echo


so pygdbmi is only required if the GDB classs is instantiated
by a test, not when 'gdb.py' is imported

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 :|

Re: [PATCH v4 8/9] tests/functional: Adapt reverse_debugging to run w/o Avocado

[Gustavo Romero]

Hi Thomas,

On 9/26/25 05:44, Thomas Huth wrote:
> On 26/09/2025 07.15, Gustavo Romero wrote:
>> 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.
>>
>> reverse_debugging now uses the pygdbmi module to interact with GDB, if
>> it's available in the test environment, otherwise the test is skipped.
>> GDB is detect via the QEMU_TEST_GDB env. variable.
>>
>> This commit also significantly improves the output for the test and
>> now prints all the GDB commands used in sequence. It also adds
>> some clarifications to existing comments, for example, clarifying that
>> once the replay-break is reached, a SIGINT is captured in 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>
>> ---
> ...
>> @@ -53,49 +55,11 @@ def run_vm(self, record, shift, args, replay_path, image_path, port):
>>           vm.launch()
>>           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
>> -
>> -    @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(self, g):
>> -        return self.get_reg(g, self.REG_PC)
>> -
>> -    def check_pc(self, g, addr):
>> -        pc = self.get_pc(g)
>> -        if pc != addr:
>> -            self.fail('Invalid PC (read %x instead of %x)' % (pc, addr))
> 
> I think it would make sense to keep wrapper functions get_pc() and check_pc(), since that functionality is still used in a bunch of places (e.g. "gdb.cli("print $pc").get_addr()" is used a couple of times).

Yeah, I considered that, but I really think that using the
wrapper functions doesn't add to this test (as in the original test).
First because I like reading the test code and easily map it to
its output and, second, the original test that used check_pc() was
actually failing with this "Invalid PC ... " message in all the cases,
which is not informative. In my version, because I check PC in place,
I also fail with a specific message for each case, like "Forward stepping failed¨,
"Reverse stepping failed", "reverse-continue", etc.

If you don't mind I'd like to keep the test this way.


>> @@ -124,68 +88,99 @@ def reverse_debugging(self, shift=7, args=None):
>>           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:
>> +
>> +        try:
>> +            logger.info('Connecting to gdbstub...')
>> +            self.reverse_debugging_run(vm, port, last_icount)
>> +            logger.info('Test passed.')
>> +        except GdbTimeoutError:
>> +            self.fail("Connection to gdbstub timeouted...")
> 
> I'm not a native English speaker, but I think "timeout" is not a verb. So maybe better: "Timeout while connecting to gdbstub" or something similar?
> 
>> +    @skipIfMissingEnv("QEMU_TEST_GDB")
> 
> Somehow this SKIP is always triggered on my system, even if I correctly pointed "configure" to the right GDB with the "--gdb" option ... not sure why this happens, but I'll try to find out...

hmm maybe you could play with the env. vars by running:

$ ./pyvenv/bin/meson test  --verbose --no-rebuild -t 1 --setup thorough  --suite func-thorough  func-aarch64-reverse_debug

which has "--verbose" so you can see and use the "raw" command containing the env. variables, e.g.:

gromero@gromero0:/mnt/git/qemu_/build$ G_TEST_SLOW=1 QEMU_TEST_QEMU_IMG=/mnt/git/qemu_/build/qemu-img ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 MESON_TEST_ITERATION=1 RUST_BACKTRACE=1 MALLOC_PERTURB_=186 SPEED=thorough LD_LIBRARY_PATH=/mnt/git/qemu_/build/contrib/plugins:/mnt/git/qemu_/build/tests/tcg/plugins MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 QEMU_TEST_QEMU_BINARY=/mnt/git/qemu_/build/qemu-system-aarch64 QEMU_BUILD_ROOT=/mnt/git/qemu_/build UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 PYTHONPATH=/mnt/git/qemu_/python:/mnt/git/qemu_/tests/functional /mnt/git/qemu_/build/pyvenv/bin/python3 /mnt/git/qemu_/tests/functional/aarch64/test_reverse_debug.py
TAP version 13
ok 1 test_reverse_debug.ReverseDebugging_AArch64.test_aarch64_virt # SKIP Missing env var(s): QEMU_TEST_GDB
1..1
gromero@gromero0:/mnt/git/qemu_/build$ echo $QEMU_TEST_GDB

gromero@gromero0:/mnt/git/qemu_/build$ which gdb-multiarch
/usr/bin/gdb-multiarch
gromero@gromero0:/mnt/git/qemu_/build$ export QEMU_TEST_GDB=/usr/bin/gdb-multiarch
gromero@gromero0:/mnt/git/qemu_/build$ G_TEST_SLOW=1 QEMU_TEST_QEMU_IMG=/mnt/git/qemu_/build/qemu-img ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 MESON_TEST_ITERATION=1 RUST_BACKTRACE=1 MALLOC_PERTURB_=186 SPEED=thorough LD_LIBRARY_PATH=/mnt/git/qemu_/build/contrib/plugins:/mnt/git/qemu_/build/tests/tcg/plugins MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 QEMU_TEST_QEMU_BINARY=/mnt/git/qemu_/build/qemu-system-aarch64 QEMU_BUILD_ROOT=/mnt/git/qemu_/build UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 PYTHONPATH=/mnt/git/qemu_/python:/mnt/git/qemu_/tests/functional /mnt/git/qemu_/build/pyvenv/bin/python3 /mnt/git/qemu_/tests/functional/aarch64/test_reverse_debug.py
TAP version 13
# $ set debug remote 1
# $ target remote localhost:64512
# Remote debugging using localhost:64512
# 0x0000000040000000 in ?? ()
# $ set debug remote 0
# $ print $pc
# $1 = (void (*)()) 0x40000000
# $ stepi
# 0x0000000040000004 in ?? ()
# $ print $pc
# $2 = (void (*)()) 0x40000004
# $ stepi
# 0x0000000040000008 in ?? ()
[...]


Let me know what you find :) Thanks!


Cheers,
Gustavo

Re: [PATCH v4 1/9] tests/functional: Re-activate the check-venv target

[Gustavo Romero]

Hi Daniel,

On 9/26/25 12:47, Daniel P. Berrangé wrote:
> On Fri, Sep 26, 2025 at 12:44:58PM -0300, Gustavo Romero wrote:
>> Hi Daniel,
>>
>> On 9/26/25 05:50, Daniel P. Berrangé wrote:
>>> On Fri, Sep 26, 2025 at 10:42:22AM +0200, Thomas Huth wrote:
>>>> On 26/09/2025 10.37, Daniel P. Berrangé wrote:
>>>>> On Fri, Sep 26, 2025 at 10:34:01AM +0200, Thomas Huth wrote:
>>>>>> On 26/09/2025 07.15, Gustavo Romero wrote:
>>>>>>> Add check-venv target as a dependency for the functional tests. This
>>>>>>> causes Python modules listed in pythondeps.toml, under the testdeps
>>>>>>> group, to be installed when 'make check-functional' is executed to
>>>>>>> prepare and run the functional tests.
>>>>>>>
>>>>>>> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
>>>>>>> Suggested-by: Thomas Huth <thuth@redhat.com>
>>>>>>> ---
>>>>>>>      tests/Makefile.include | 2 +-
>>>>>>>      1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>>>>>>> index 3538c0c740..d012a9b25d 100644
>>>>>>> --- a/tests/Makefile.include
>>>>>>> +++ b/tests/Makefile.include
>>>>>>> @@ -109,7 +109,7 @@ $(FUNCTIONAL_TARGETS):
>>>>>>>      	@$(MAKE) SPEED=thorough $(subst -functional,-func,$@)
>>>>>>>      .PHONY: check-functional
>>>>>>> -check-functional:
>>>>>>> +check-functional: check-venv
>>>>>>
>>>>>> I just noticed that there's still a problem: If you run "make
>>>>>> check-functional-aarch64" immediately after configuring + compiling QEMU in
>>>>>> a fresh folder for the first time, the functional tests fail with:
>>>>>>
>>>>>> ModuleNotFoundError: No module named 'pygdbmi'
>>>>>>
>>>>>> We either need to add dependencies to the check-functional-<arch> targets,
>>>>>> too, or we have to make sure that tests still get properly skipped in the
>>>>>> case that pygdbmi has not been installed into the venv yet.
>>>>>
>>>>> We already have a decorator for skipping tests when modules are missing,
>>>>> so we should add usage of that.
>>>>
>>>> Ack ... and the "from .gdb import GDB" in qemu_test/__init__.py likely also
>>>> has to go away, to avoid that each and every test tries to pull in the gdb
>>>> code.
>>>
>>> Or alternatively the gdb module can move the gdbmi import so that it is
>>> only referenced in method scope, so it becomes relevant only when
>>> executed.
>>
>> I can´t follow what you meant here. Do mind expanding on it a bit?
> 
> The code currently does:
> 
>    from pygdbmi.gdbcontroller import GdbController
> 
>    class GDB:
>        def __init__(self, gdb_path, echo=True, suffix='# ', prompt="$ "):
>            gdb_cmd = [gdb_path, "-q", "--interpreter=mi2"]
>            self.gdbmi = GdbController(gdb_cmd)
>            self.echo = echo
> 
> but it could instead do
> 
>    class GDB:
>        def __init__(self, gdb_path, echo=True, suffix='# ', prompt="$ "):
>            gdb_cmd = [gdb_path, "-q", "--interpreter=mi2"]
>            from pygdbmi.gdbcontroller import GdbController
>            self.gdbmi = GdbController(gdb_cmd)
>            self.echo = echo
> 
> 
> so pygdbmi is only required if the GDB classs is instantiated
> by a test, not when 'gdb.py' is imported

ah, got it. Thanks for clarification ;)


Cheers,
Gustavo

Re: [PATCH v4 3/9] tests/functional: Provide GDB to the functional tests

[Gustavo Romero]

Hi Thomas,

On 9/26/25 07:03, Thomas Huth wrote:
> On 26/09/2025 07.15, Gustavo Romero wrote:
>> The probe of GDB is done in 'configure' and the full path is passed to
>> meson.build via the -Dgdb=option.
>>
>> Because a single functional test can cover different arches, such as
>> aarch64, ppc64, and x86_64, only a GDB that supports all the arches in
>> the target list is passed to Meson for use in the functional tests. To
>> handle this check, a new shell function, is_target_arch_in_arch_list, is
>> introduced in 'configure'.
>>
>> Meson then can pass the location of GDB to the test via an environment
>> variable: QEMU_TEST_GDB.
>>
>> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   configure                     | 21 +++++++++++++++++++++
>>   meson_options.txt             |  2 ++
>>   scripts/meson-buildoptions.sh |  2 ++
>>   tests/functional/meson.build  |  6 ++++++
>>   4 files changed, 31 insertions(+)
>>
>> diff --git a/configure b/configure
>> index 0f7eb95586..20e05d233f 100755
>> --- a/configure
>> +++ b/configure
>> @@ -1142,12 +1142,31 @@ fi
>>   #########################################
>>   # gdb test
>> +# Check if all target arches are in a provided list of arches.
>> +is_target_arch_in_arch_list() {
>> +    arch_list=$1
>> +    for target in $target_list; do
>> +        arch=${target%%-*}
>> +        if test "${arch_list#*$arch}" = "$arch_list"; then
>> +            # Target arch not in arch list
>> +            return 1
>> +        fi
>> +    done
>> +    return 0
>> +}
>> +
>>   if test -n "$gdb_bin"; then
>>       gdb_version_string=$($gdb_bin --version | head -n 1)
>>       # Extract last field in the version string
>>       gdb_version=${gdb_version_string##* }
>>       if version_ge $gdb_version 9.1; then
>>           gdb_arches=$($python "$source_path/scripts/probe-gdb-support.py" $gdb_bin)
>> +
>> +    if is_target_arch_in_arch_list "$gdb_arches"; then
> 
> No TABs, please!
> 
>> +            gdb_multiarch="yes"
>> +        else
>> +            gdb_multiarch=""
>> +    fi
> 
> This unfortunately does not work with the GDB from Fedora - it only supports "arch64_be arm riscv64 riscv32 ppc i386 s390x ppc64 aarch64 ppc64le x86_64", but if you configured a target like "alpha-softmmu", this breaks.

argh! ok


> (BTW, does the gdb-multiarch from Debian/Ubuntu really also support exotic QEMU targets like tricore?)

No, I've checked GDB upstream and I can't see any trace of tricore.
And I just saw that Alex left a comment in scripts/probe-gdb-support.py
saying "# no tricore in upstream gdb", so nope, it seems that it still holds.


> I think it would be better to drop this hunk, and rather check in the spot where we use GDB if the required target is really there (i.e. in the functional test that uses it).

OK. I'm also not a big fan of doing it in bash. How do you suggest
to do it? Directly in the code, via a skipIf decorator, or something else?


Cheers,
Gustavo

Re: [PATCH v4 3/9] tests/functional: Provide GDB to the functional tests

[Gustavo Romero]

Hi Thomas,

On 9/26/25 15:08, Gustavo Romero wrote:
> Hi Thomas,
> 
> On 9/26/25 07:03, Thomas Huth wrote:
>> On 26/09/2025 07.15, Gustavo Romero wrote:
>>> The probe of GDB is done in 'configure' and the full path is passed to
>>> meson.build via the -Dgdb=option.
>>>
>>> Because a single functional test can cover different arches, such as
>>> aarch64, ppc64, and x86_64, only a GDB that supports all the arches in
>>> the target list is passed to Meson for use in the functional tests. To
>>> handle this check, a new shell function, is_target_arch_in_arch_list, is
>>> introduced in 'configure'.
>>>
>>> Meson then can pass the location of GDB to the test via an environment
>>> variable: QEMU_TEST_GDB.
>>>
>>> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>   configure                     | 21 +++++++++++++++++++++
>>>   meson_options.txt             |  2 ++
>>>   scripts/meson-buildoptions.sh |  2 ++
>>>   tests/functional/meson.build  |  6 ++++++
>>>   4 files changed, 31 insertions(+)
>>>
>>> diff --git a/configure b/configure
>>> index 0f7eb95586..20e05d233f 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -1142,12 +1142,31 @@ fi
>>>   #########################################
>>>   # gdb test
>>> +# Check if all target arches are in a provided list of arches.
>>> +is_target_arch_in_arch_list() {
>>> +    arch_list=$1
>>> +    for target in $target_list; do
>>> +        arch=${target%%-*}
>>> +        if test "${arch_list#*$arch}" = "$arch_list"; then
>>> +            # Target arch not in arch list
>>> +            return 1
>>> +        fi
>>> +    done
>>> +    return 0
>>> +}
>>> +
>>>   if test -n "$gdb_bin"; then
>>>       gdb_version_string=$($gdb_bin --version | head -n 1)
>>>       # Extract last field in the version string
>>>       gdb_version=${gdb_version_string##* }
>>>       if version_ge $gdb_version 9.1; then
>>>           gdb_arches=$($python "$source_path/scripts/probe-gdb-support.py" $gdb_bin)
>>> +
>>> +    if is_target_arch_in_arch_list "$gdb_arches"; then
>>
>> No TABs, please!
>>
>>> +            gdb_multiarch="yes"
>>> +        else
>>> +            gdb_multiarch=""
>>> +    fi
>>
>> This unfortunately does not work with the GDB from Fedora - it only supports "arch64_be arm riscv64 riscv32 ppc i386 s390x ppc64 aarch64 ppc64le x86_64", but if you configured a target like "alpha-softmmu", this breaks.
> 
> argh! ok
> 
> 
>> (BTW, does the gdb-multiarch from Debian/Ubuntu really also support exotic QEMU targets like tricore?)
> 
> No, I've checked GDB upstream and I can't see any trace of tricore.
> And I just saw that Alex left a comment in scripts/probe-gdb-support.py
> saying "# no tricore in upstream gdb", so nope, it seems that it still holds.
> 
> 
>> I think it would be better to drop this hunk, and rather check in the spot where we use GDB if the required target is really there (i.e. in the functional test that uses it).
> 
> OK. I'm also not a big fan of doing it in bash. How do you suggest
> to do it? Directly in the code, via a skipIf decorator, or something else?

$gdb_arches, obtained using scripts/probe-gdb-support.py in configure,
could be passed to meson and meson sets it in the test env, as we're
doing for $gdb_bin and QEMU_TEST_GDB env var. wdyt?


Cheers,
Gustavo

Re: [PATCH v4 8/9] tests/functional: Adapt reverse_debugging to run w/o Avocado

[Thomas Huth]

On 26/09/2025 18.00, Gustavo Romero wrote:
> Hi Thomas,
> 
> On 9/26/25 05:44, Thomas Huth wrote:
>> On 26/09/2025 07.15, Gustavo Romero wrote:
>>> 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.
>>>
>>> reverse_debugging now uses the pygdbmi module to interact with GDB, if
>>> it's available in the test environment, otherwise the test is skipped.
>>> GDB is detect via the QEMU_TEST_GDB env. variable.
>>>
>>> This commit also significantly improves the output for the test and
>>> now prints all the GDB commands used in sequence. It also adds
>>> some clarifications to existing comments, for example, clarifying that
>>> once the replay-break is reached, a SIGINT is captured in 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>
>>> ---
>> ...
>>> @@ -53,49 +55,11 @@ def run_vm(self, record, shift, args, replay_path, 
>>> image_path, port):
>>>           vm.launch()
>>>           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
>>> -
>>> -    @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(self, g):
>>> -        return self.get_reg(g, self.REG_PC)
>>> -
>>> -    def check_pc(self, g, addr):
>>> -        pc = self.get_pc(g)
>>> -        if pc != addr:
>>> -            self.fail('Invalid PC (read %x instead of %x)' % (pc, addr))
>>
>> I think it would make sense to keep wrapper functions get_pc() and 
>> check_pc(), since that functionality is still used in a bunch of places 
>> (e.g. "gdb.cli("print $pc").get_addr()" is used a couple of times).
> 
> Yeah, I considered that, but I really think that using the
> wrapper functions doesn't add to this test (as in the original test).
> First because I like reading the test code and easily map it to
> its output and, second, the original test that used check_pc() was
> actually failing with this "Invalid PC ... " message in all the cases,
> which is not informative. In my version, because I check PC in place,
> I also fail with a specific message for each case, like "Forward stepping 
> failed¨,
> "Reverse stepping failed", "reverse-continue", etc.
> 
> If you don't mind I'd like to keep the test this way.

Ok, fair point. But you could at least consider keep the wrapper for 
get_pc(), I think.

>>> +    @skipIfMissingEnv("QEMU_TEST_GDB")
>>
>> Somehow this SKIP is always triggered on my system, even if I correctly 
>> pointed "configure" to the right GDB with the "--gdb" option ... not sure 
>> why this happens, but I'll try to find out...

It was the additional logic in patch 3 (see my other mail) that caused the 
gdb detection to fail and thus the environment variable was not set.

  Thomas

Re: [PATCH v4 1/9] tests/functional: Re-activate the check-venv target

[Thomas Huth]

On 26/09/2025 17.43, Gustavo Romero wrote:
> Hi Thomas!
> 
> On 9/26/25 05:34, Thomas Huth wrote:
>> On 26/09/2025 07.15, Gustavo Romero wrote:
>>> Add check-venv target as a dependency for the functional tests. This
>>> causes Python modules listed in pythondeps.toml, under the testdeps
>>> group, to be installed when 'make check-functional' is executed to
>>> prepare and run the functional tests.
>>>
>>> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
>>> Suggested-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>   tests/Makefile.include | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>>> index 3538c0c740..d012a9b25d 100644
>>> --- a/tests/Makefile.include
>>> +++ b/tests/Makefile.include
>>> @@ -109,7 +109,7 @@ $(FUNCTIONAL_TARGETS):
>>>       @$(MAKE) SPEED=thorough $(subst -functional,-func,$@)
>>>   .PHONY: check-functional
>>> -check-functional:
>>> +check-functional: check-venv
>>
>> I just noticed that there's still a problem: If you run "make check- 
>> functional-aarch64" immediately after configuring + compiling QEMU in a 
>> fresh folder for the first time, the functional tests fail with:
>>
>> ModuleNotFoundError: No module named 'pygdbmi'
>>
>> We either need to add dependencies to the check-functional-<arch> targets, 
>> too, or we have to make sure that tests still get properly skipped in the 
>> case that pygdbmi has not been installed into the venv yet.
> 
> Isn't it inconsistent that check-functional runs the test and
> check-functional-<arch> doesn't? I think it's a good idea to
> skip if the module is not available, yeah, I'll add it in v6,
> but would it be ok to add check-venv to the check-functional-<arch>
> targets too?

I think so... so please try to change this patch to add the "check-venv" 
dependency to the "$(FUNCTIONAL_TARGETS):" line instead.

  Thomas

Re: [PATCH v4 1/9] tests/functional: Re-activate the check-venv target

[Thomas Huth]

On 29/09/2025 08.26, Thomas Huth wrote:
> On 26/09/2025 17.43, Gustavo Romero wrote:
>> Hi Thomas!
>>
>> On 9/26/25 05:34, Thomas Huth wrote:
>>> On 26/09/2025 07.15, Gustavo Romero wrote:
>>>> Add check-venv target as a dependency for the functional tests. This
>>>> causes Python modules listed in pythondeps.toml, under the testdeps
>>>> group, to be installed when 'make check-functional' is executed to
>>>> prepare and run the functional tests.
>>>>
>>>> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
>>>> Suggested-by: Thomas Huth <thuth@redhat.com>
>>>> ---
>>>>   tests/Makefile.include | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>>>> index 3538c0c740..d012a9b25d 100644
>>>> --- a/tests/Makefile.include
>>>> +++ b/tests/Makefile.include
>>>> @@ -109,7 +109,7 @@ $(FUNCTIONAL_TARGETS):
>>>>       @$(MAKE) SPEED=thorough $(subst -functional,-func,$@)
>>>>   .PHONY: check-functional
>>>> -check-functional:
>>>> +check-functional: check-venv
>>>
>>> I just noticed that there's still a problem: If you run "make check- 
>>> functional-aarch64" immediately after configuring + compiling QEMU in a 
>>> fresh folder for the first time, the functional tests fail with:
>>>
>>> ModuleNotFoundError: No module named 'pygdbmi'
>>>
>>> We either need to add dependencies to the check-functional-<arch> 
>>> targets, too, or we have to make sure that tests still get properly 
>>> skipped in the case that pygdbmi has not been installed into the venv yet.
>>
>> Isn't it inconsistent that check-functional runs the test and
>> check-functional-<arch> doesn't? I think it's a good idea to
>> skip if the module is not available, yeah, I'll add it in v6,
>> but would it be ok to add check-venv to the check-functional-<arch>
>> targets too?
> 
> I think so... so please try to change this patch to add the "check-venv" 
> dependency to the "$(FUNCTIONAL_TARGETS):" line instead.

Looking at the code twice, I think you need it in addition, not "instead" 
(since there is no direct dependency from check-functional to the 
check-functional-<ARCH> targets).

  Thomas

Re: [PATCH v4 3/9] tests/functional: Provide GDB to the functional tests

[Thomas Huth]

On 26/09/2025 20.15, Gustavo Romero wrote:
> Hi Thomas,
> 
> On 9/26/25 15:08, Gustavo Romero wrote:
>> Hi Thomas,
>>
>> On 9/26/25 07:03, Thomas Huth wrote:
>>> On 26/09/2025 07.15, Gustavo Romero wrote:
>>>> The probe of GDB is done in 'configure' and the full path is passed to
>>>> meson.build via the -Dgdb=option.
>>>>
>>>> Because a single functional test can cover different arches, such as
>>>> aarch64, ppc64, and x86_64, only a GDB that supports all the arches in
>>>> the target list is passed to Meson for use in the functional tests. To
>>>> handle this check, a new shell function, is_target_arch_in_arch_list, is
>>>> introduced in 'configure'.
>>>>
>>>> Meson then can pass the location of GDB to the test via an environment
>>>> variable: QEMU_TEST_GDB.
>>>>
>>>> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>> ---
>>>>   configure                     | 21 +++++++++++++++++++++
>>>>   meson_options.txt             |  2 ++
>>>>   scripts/meson-buildoptions.sh |  2 ++
>>>>   tests/functional/meson.build  |  6 ++++++
>>>>   4 files changed, 31 insertions(+)
>>>>
>>>> diff --git a/configure b/configure
>>>> index 0f7eb95586..20e05d233f 100755
>>>> --- a/configure
>>>> +++ b/configure
>>>> @@ -1142,12 +1142,31 @@ fi
>>>>   #########################################
>>>>   # gdb test
>>>> +# Check if all target arches are in a provided list of arches.
>>>> +is_target_arch_in_arch_list() {
>>>> +    arch_list=$1
>>>> +    for target in $target_list; do
>>>> +        arch=${target%%-*}
>>>> +        if test "${arch_list#*$arch}" = "$arch_list"; then
>>>> +            # Target arch not in arch list
>>>> +            return 1
>>>> +        fi
>>>> +    done
>>>> +    return 0
>>>> +}
>>>> +
>>>>   if test -n "$gdb_bin"; then
>>>>       gdb_version_string=$($gdb_bin --version | head -n 1)
>>>>       # Extract last field in the version string
>>>>       gdb_version=${gdb_version_string##* }
>>>>       if version_ge $gdb_version 9.1; then
>>>>           gdb_arches=$($python "$source_path/scripts/probe-gdb- 
>>>> support.py" $gdb_bin)
>>>> +
>>>> +    if is_target_arch_in_arch_list "$gdb_arches"; then
>>>
>>> No TABs, please!
>>>
>>>> +            gdb_multiarch="yes"
>>>> +        else
>>>> +            gdb_multiarch=""
>>>> +    fi
>>>
>>> This unfortunately does not work with the GDB from Fedora - it only 
>>> supports "arch64_be arm riscv64 riscv32 ppc i386 s390x ppc64 aarch64 
>>> ppc64le x86_64", but if you configured a target like "alpha-softmmu", 
>>> this breaks.
>>
>> argh! ok
>>
>>
>>> (BTW, does the gdb-multiarch from Debian/Ubuntu really also support 
>>> exotic QEMU targets like tricore?)
>>
>> No, I've checked GDB upstream and I can't see any trace of tricore.
>> And I just saw that Alex left a comment in scripts/probe-gdb-support.py
>> saying "# no tricore in upstream gdb", so nope, it seems that it still holds.
>>
>>
>>> I think it would be better to drop this hunk, and rather check in the 
>>> spot where we use GDB if the required target is really there (i.e. in the 
>>> functional test that uses it).
>>
>> OK. I'm also not a big fan of doing it in bash. How do you suggest
>> to do it? Directly in the code, via a skipIf decorator, or something else?
> 
> $gdb_arches, obtained using scripts/probe-gdb-support.py in configure,
> could be passed to meson and meson sets it in the test env, as we're
> doing for $gdb_bin and QEMU_TEST_GDB env var. wdyt?

That might be a possibility, though it's getting a little bit clunky if you 
want to run the test manually, without the meson test runner.

Maybe it would be nicer to just start gdb in the test and catch the error 
(and skip the test in that case) in the python test code if it fails to set 
the target architecture there?

  Thomas

Re: [PATCH v4 3/9] tests/functional: Provide GDB to the functional tests

[Daniel P. Berrangé]

On Mon, Sep 29, 2025 at 08:34:03AM +0200, Thomas Huth wrote:
> On 26/09/2025 20.15, Gustavo Romero wrote:
> > Hi Thomas,
> > 
> > On 9/26/25 15:08, Gustavo Romero wrote:
> > > Hi Thomas,
> > > 
> > > On 9/26/25 07:03, Thomas Huth wrote:
> > > > On 26/09/2025 07.15, Gustavo Romero wrote:
> > > > > The probe of GDB is done in 'configure' and the full path is passed to
> > > > > meson.build via the -Dgdb=option.
> > > > > 
> > > > > Because a single functional test can cover different arches, such as
> > > > > aarch64, ppc64, and x86_64, only a GDB that supports all the arches in
> > > > > the target list is passed to Meson for use in the functional tests. To
> > > > > handle this check, a new shell function, is_target_arch_in_arch_list, is
> > > > > introduced in 'configure'.
> > > > > 
> > > > > Meson then can pass the location of GDB to the test via an environment
> > > > > variable: QEMU_TEST_GDB.
> > > > > 
> > > > > Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> > > > > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > > > > ---
> > > > >   configure                     | 21 +++++++++++++++++++++
> > > > >   meson_options.txt             |  2 ++
> > > > >   scripts/meson-buildoptions.sh |  2 ++
> > > > >   tests/functional/meson.build  |  6 ++++++
> > > > >   4 files changed, 31 insertions(+)
> > > > > 
> > > > > diff --git a/configure b/configure
> > > > > index 0f7eb95586..20e05d233f 100755
> > > > > --- a/configure
> > > > > +++ b/configure
> > > > > @@ -1142,12 +1142,31 @@ fi
> > > > >   #########################################
> > > > >   # gdb test
> > > > > +# Check if all target arches are in a provided list of arches.
> > > > > +is_target_arch_in_arch_list() {
> > > > > +    arch_list=$1
> > > > > +    for target in $target_list; do
> > > > > +        arch=${target%%-*}
> > > > > +        if test "${arch_list#*$arch}" = "$arch_list"; then
> > > > > +            # Target arch not in arch list
> > > > > +            return 1
> > > > > +        fi
> > > > > +    done
> > > > > +    return 0
> > > > > +}
> > > > > +
> > > > >   if test -n "$gdb_bin"; then
> > > > >       gdb_version_string=$($gdb_bin --version | head -n 1)
> > > > >       # Extract last field in the version string
> > > > >       gdb_version=${gdb_version_string##* }
> > > > >       if version_ge $gdb_version 9.1; then
> > > > >           gdb_arches=$($python
> > > > > "$source_path/scripts/probe-gdb- support.py" $gdb_bin)
> > > > > +
> > > > > +    if is_target_arch_in_arch_list "$gdb_arches"; then
> > > > 
> > > > No TABs, please!
> > > > 
> > > > > +            gdb_multiarch="yes"
> > > > > +        else
> > > > > +            gdb_multiarch=""
> > > > > +    fi
> > > > 
> > > > This unfortunately does not work with the GDB from Fedora - it
> > > > only supports "arch64_be arm riscv64 riscv32 ppc i386 s390x
> > > > ppc64 aarch64 ppc64le x86_64", but if you configured a target
> > > > like "alpha-softmmu", this breaks.
> > > 
> > > argh! ok
> > > 
> > > 
> > > > (BTW, does the gdb-multiarch from Debian/Ubuntu really also
> > > > support exotic QEMU targets like tricore?)
> > > 
> > > No, I've checked GDB upstream and I can't see any trace of tricore.
> > > And I just saw that Alex left a comment in scripts/probe-gdb-support.py
> > > saying "# no tricore in upstream gdb", so nope, it seems that it still holds.
> > > 
> > > 
> > > > I think it would be better to drop this hunk, and rather check
> > > > in the spot where we use GDB if the required target is really
> > > > there (i.e. in the functional test that uses it).
> > > 
> > > OK. I'm also not a big fan of doing it in bash. How do you suggest
> > > to do it? Directly in the code, via a skipIf decorator, or something else?
> > 
> > $gdb_arches, obtained using scripts/probe-gdb-support.py in configure,
> > could be passed to meson and meson sets it in the test env, as we're
> > doing for $gdb_bin and QEMU_TEST_GDB env var. wdyt?
> 
> That might be a possibility, though it's getting a little bit clunky if you
> want to run the test manually, without the meson test runner.
> 
> Maybe it would be nicer to just start gdb in the test and catch the error
> (and skip the test in that case) in the python test code if it fails to set
> the target architecture there?

If you run 'set architecture' with no arguments, it'll return a long list
of supported targets we can match on.


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 :|