On Thu, Sep 11, 2025 at 08:51:08PM -0300, Gustavo Romero wrote:
> Hi Daniel,
>
> Thanks a lot for review and the suggestions.
>
> On 9/8/25 08:49, Daniel P. Berrangé wrote:
> > On Thu, Sep 04, 2025 at 03:46:35PM +0000, Gustavo Romero wrote:
> > > In this series, we leveraged the run-test.py script used in the
> > > check-tcg tests, making it a GDB runner capable of calling a test script
> > > without spawning any VMs. In this configuration, the test scripts can
> > > manage the VM and also import gdb, making the GDB Python API inside the
> > > functional test scripts.
> > >
> > > A --quiet option has been added to run-test.py so it doesn't print the
> > > command line used to execute GDB to the stdout. This ensures that users
> > > don't get confused about how to re-run the tests. One can re-run the
> > > test simply by copying and pasting the command line shown by Meson when
> > > V=1 is passed:
> > >
> > > $ make -j check-functional V=1
> > >
> > > or, alternatively, once the test run completes, the exact command found
> > > in the 'command:' field of the build/meson-logs/testlog-thorough.txt
> > > file generated by Meson. Both methods provide the correct environment
> > > variables required to run the test, such as the proper $PYTHONPATH.
> >
> > While I like the conceptual idea of just sending human GDB commands,
> > instead of working with GDB protocol packets, I really dislike the
> > effect this has on the execution / startup of the functional tests
> > via use of the custom runner for a number of reasons
> >
> > * The command line for launching the test outside of meson is very
> > complicated, so not memorable
>
> Why very complicated? It calls a simple runner instead of calling the
> test script directly, but it doesn't change the way to re-run a single
> test. One just have to pass V=1 to see make's command line and copy
> and paste the full command line to re-run the test. I mentioned
> inspecting 'testlog-thorough.txt' just for completeness.
Today we can run the individual tests directly
# ./tests/functional/x86_64/test_reverse_debug.py
TAP version 13
ok 1 test_reverse_debug.ReverseDebugging_X86_64.test_x86_64_pc
1..1
(assuming you have PYTHONPATH and QEMU_TEST_QEMU_BINARY env set)
This gives you a very easy way to interact with the test, see
its progress, understand what failed, and debug it with strace,
etc.
This change looses all that. It appears I can run it with
# ./tests/guest-debug/run-test.py --quiet --gdb gdb --test \
./tests/functional/x86_64/test_reverse_debug.py
but the output is unintelligible
TAP version 13
0x000000000000fff0 in ?? ()
0x000000000000e05b in ?? ()
0x000000000000e062 in ?? ()
0x000000000000e066 in ?? ()
0x000000000000e068 in ?? ()
0x000000000000e06a in ?? ()
0x000000000000e070 in ?? ()
0x000000000000e076 in ?? ()
0x000000000000cf30 in ?? ()
0x000000000000cf31 in ?? ()
0x000000000000cf32 in ?? ()
0x000000000000cf31 in ?? ()
0x000000000000cf30 in ?? ()
0x000000000000e076 in ?? ()
0x000000000000e070 in ?? ()
0x000000000000e06a in ?? ()
0x000000000000e068 in ?? ()
0x000000000000e066 in ?? ()
0x000000000000e062 in ?? ()
0x000000000000e05b in ?? ()
0x000000000000fff0 in ?? ()
0x000000000000e05b in ?? ()
0x000000000000e062 in ?? ()
0x000000000000e066 in ?? ()
0x000000000000e068 in ?? ()
0x000000000000e06a in ?? ()
0x000000000000e070 in ?? ()
0x000000000000e076 in ?? ()
0x000000000000cf30 in ?? ()
0x000000000000cf31 in ?? ()
0x000000000000cf32 in ?? ()
Breakpoint 1 at 0xfff0
Breakpoint 2 at 0xe05b
Breakpoint 3 at 0xe062
Breakpoint 4 at 0xe066
Breakpoint 5 at 0xe068
Breakpoint 6 at 0xe06a
Breakpoint 7 at 0xe070
Breakpoint 8 at 0xe076
Breakpoint 9 at 0xcf30
Breakpoint 10 at 0xcf31
Program received signal SIGINT, Interrupt.
0x00000000000d80dc in ?? ()
Breakpoint 1, 0x000000000000fff0 in ?? ()
[Inferior 1 (process 1) detached]
This undermines the core goals of what we aimed to achieve with
the new functional test harness.
>
> > * It makes the meson.build rules much more complicated
>
> Do we want to never augment functional tests' meson.build? Nothing
> complicated is being added. Basically, just a new variable suffixed with
> '_with_runner' which holds a tuple (test, runner) that tell the test
> to be executed, following the same logic we already have for all the other
> variables that specify the tests per arch/mode/speed.
>
> Another option would be to select a runner based on a suffix in the test
> name, for instance, 'reverse_debug_with_runner.py'.
IMHO the overall concept of using the run-test.py runner for launching
the tests is flawed and not viable. It adds too much complexity to the
use of the tests, and harms the output.
> > * Running standalone there is no TAP output available making the
> > test hard to debug on failure or timeout
>
> This is because of an unfortunate GDB Python API issue, please see my
> reply in your comment on patch 5/5. This can be solved but needs more
> investigation on GDB side.
>
>
> > I understand the need to spawn the test via gdb, in order to be able
> > to import the 'gdb' python module. Looking at what reverse_debugging.py
> > does, however, makes me question whether we actually need to directly
> > use the 'gdb' python module.
> >
> > The only APIs we use are 'gdb.execute' and 'gdb.parse_and_eval'.
> >
> > The latter is only used once as
> >
> > gdb.parse_and_eval("$pc")
> >
> > and I believe that can be changed to
> >
> > gdb.execute("printf \"0x%x\", $pc", to_string=True)
> >
> > IOW, all we need is 'gdb.execute("....", to_string=True)'
>
> Yes, I do want to directly use the 'gdb' python module directly in the
> tests. We shouldn't look at a solution only for reverse_debug.py but also
> think of any future tests that will require the GDB Python API, so I don't
> want to specialize here and reduce the API to a single method.
If any other tests needing GDB arrive int he future we can consider
them at that time.
I like the idea of the test being able to execute human gdb commands,
but I don't think the GDB provided 'gdb' module is viable to use
directly. We need to retain control over how we launch our tests
without intermediate runners present.
> > With a little extra helper proxy script, we can achieve this without
> > changing the way scripts are launched.
> >
> > The script needs to listen on a UNIX socket path. When a client
> > connects, it should read lines of data from the client and pass
> > them to 'gdb.execute(..., to_string=True)' and whatever data
> > gdb returns should be written back to the client.
> >
> > A very very crude example with no error handling would be:
> >
> > #!/usr/bin/python3
> >
> > import gdb
> > import os
> > import socket
> >
> > sock = os.environ.get("QEMU_PROXY", "/tmp/qemu.gdb.proxy")
> >
> > try:
> > os.unlink(sock)
> > except:
> > pass
> >
> > with socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) as s:
> > s.bind(sock)
> > s.listen()
> > conn, addr = s.accept()
> > fh = conn.makefile('rw')
> > with conn:
> > while True:
> > line = fh.readline()
> > if not line:
> > break
> > data = gdb.execute(line, to_string=True)
> > fh.write(data)
> > fh.flush()
> >
> >
> > In the functional test suite, we should have a helper file
> > tests/functional/qemu_test/gdb.py that provides an API for
> > launching GDB to execute this proxy script, and an API to
> > execute commands by talking over this UNIX socket path.
> >
> > With this, we will need no changes in the way we execute the
> > reverse debugging script from a test runner POV, thus avoiding
> > all the downsides of use of the run-test.py script. IOW, the
> > first 4 patches in this series go away completely. Instead we
> > need a patch to create the proxy script and a patch to create
> > the helper APIs in tests/functional/qemu_test/gdb.py, whereupon
> > the last patch can replace
> >
> > try:
> > import gdb
> > except ModuleNotFoundError:
> > from sys import exit
> > exit("This script must be launched via tests/guest-debug/run-test.py!")
> >
> > with
> >
> > from qemu_test import gdb
> >
> > and the earlier mentioned replacement of parse_and_eval()
>
> For the sake of not adding a few lines into meson.build, we are going
> to design a new ad-hoc API for the functional tests on top of the GDB
> Python API, which will communicate with the test script via a socket
> and will _still require a runner anyway_ (just now hidden under a
> module/API)? This is far more complicated than having a simple runner
> to call GDB and pass the test script.
This is not exclusively about the meson.build changes. It is about the
overall execution environment of the tests being *simple* and easy to
understand. That is the overrriding goal of how we approached design
of the new functional test harness that made it valuable to spend the
time to replace avocado. The GDB runner usage undermines the benefits
we have achieved.
> In fact, I think that if the test script had any clue in its name,
> like the "_with_runner" suffix I mentioned above, maybe Meson's could
> take care of calling GDB itself without calling any runner. Would that
> address your first comment in the bullets (and maybe the second one too,
> but not sure without trying it) and get this series accepted by you,
> since the third one, about the exitcode, is related to GDB's odd behavior?
The tests need to be runnable directly as standalone python programs,
with well formed TAP output.
Given the limitations of GDB, if we want to use its python module, then
the proxy idea I describe above is the only way forward I see.
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 :|
