On 12/09/2025 18.27, Daniel P. Berrangé wrote:
On Fri, Sep 12, 2025 at 05:04:40PM +0100, Alex Bennée wrote:
Daniel P. Berrangé <[email protected]> writes:

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)

and the old version of Avocado...

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

<snip>


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.

We already have a whole chunk of gdb tests under check-tcg. Maybe it
would be easier just to re-write the tests to use the check-tcg system
tests rather than jumping through hoops to fit in with the
check-functional requirements.

Well if 'easy' is our goal, then we can just copy the gdbmi.py
file from avocado into our test suite.

But maintaining that stuff in the QEMU repository is also kind of ugly.

I took another stab at the problem:

 https://lore.kernel.org/qemu-devel/[email protected]/

It's basically Gustavo's patches, but I removed all the stuff around tests/guest-debug/run-test.py and the cumbersome code for running a test through gdb via meson.build.

The test start in normal pycotap mode, then call a new function called reverse_debug() which then takes care of calling gdb with the right arguments (i.e. this logic has been copied from the run-test.py script).

It seems to work fine with the aarch64 test already, but details and other architectures still need some more love.

WDYT? Is it worth to pursue that approach?

 Thomas


Reply via email to