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

[Thomas Huth <thuth@redhat.com>]

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é <berrange@redhat.com> 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/20250915124207.42053-1-thuth@redhat.com/

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