Gustavo Romero <gustavo.rom...@linaro.org> writes:
> Hi Alex,
>
> On 8/25/25 14:26, Gustavo Romero wrote:
>> Hi Alex!
>> On 8/25/25 14:01, Alex Bennée wrote:
>>> Gustavo Romero <gustavo.rom...@linaro.org> writes:
>>>
>>>> This commit makes QEMU optional in run-test.py, allowing it to be used
>>>> as a GDB runner, i.e., to call GDB and pass a test script to it without
>>>> launching QEMU. In this configuration, it is the test script’s duty to
>>>> configure and run the VMs that GDB connects to.
>>>>
>>>> The --binary option continues to be required when --qemu is passed.
>>>> sys.argv now includes the full path to the test script in addition to
>>>> the script’s arguments, which allows unittest introspection to work
>>>> properly in case it is used in the test script.
>>>>
>>>> Signed-off-by: Gustavo Romero <gustavo.rom...@linaro.org>
>>>> ---
>>>> tests/guest-debug/run-test.py | 81 +++++++++++++++++++----------------
>>>> 1 file changed, 45 insertions(+), 36 deletions(-)
>>>>
>>>> diff --git a/tests/guest-debug/run-test.py b/tests/guest-debug/run-test.py
>>>> index 75e9c92e03..7fa17aedca 100755
>>>> --- a/tests/guest-debug/run-test.py
>>>> +++ b/tests/guest-debug/run-test.py
>>>> @@ -22,10 +22,10 @@
>>>> def get_args():
>>>> parser = argparse.ArgumentParser(description="A gdbstub test runner")
>>>> parser.add_argument("--qemu", help="Qemu binary for test",
>>>> - required=True)
>>>> + required=False)
>>>> parser.add_argument("--qargs", help="Qemu arguments for test")
>>>> parser.add_argument("--binary", help="Binary to debug",
>>>> - required=True)
>>>> + required=False)
>>>
>>> parser.add_argument("--binary", help="Binary to debug",
>>> required='--qemu' in sys.argv)
>>>
>>>> parser.add_argument("--test", help="GDB test script")
>>>> parser.add_argument('test_args', nargs='*',
>>>> help="Additional args for GDB test script. "
>>>> @@ -53,7 +53,7 @@ def log(output, msg):
>>>> if __name__ == '__main__':
>>>> args = get_args()
>>>> - # Search for a gdb we can use
>>>> + # Search for a gdb we can use.
>>>> if not args.gdb:
>>>> args.gdb = shutil.which("gdb-multiarch")
>>>> if not args.gdb:
>>>> @@ -73,41 +73,49 @@ def log(output, msg):
>>>> socket_dir = TemporaryDirectory("qemu-gdbstub")
>>>> socket_name = os.path.join(socket_dir.name, "gdbstub.socket")
>>>> - # Launch QEMU with binary
>>>> - if "system" in args.qemu:
>>>> - if args.no_suspend:
>>>> - suspend = ''
>>>> - else:
>>>> - suspend = ' -S'
>>>> - cmd = f'{args.qemu} {args.qargs} {args.binary}' \
>>>> - f'{suspend} -gdb unix:path={socket_name},server=on'
>>>> - else:
>>>> - if args.no_suspend:
>>>> - suspend = ',suspend=n'
>>>> - else:
>>>> - suspend = ''
>>>> - cmd = f'{args.qemu} {args.qargs} -g {socket_name}{suspend}' \
>>>> - f' {args.binary}'
>>>> -
>>>> - log(output, "QEMU CMD: %s" % (cmd))
>>>> - inferior = subprocess.Popen(shlex.split(cmd))
>>>> + if args.qemu and not args.binary:
>>>> + print("QEMU needs a binary to run, but no binary provided")
>>>> + exit(-1)
>>>
>>> then we can avoid this.
>> Sure, thanks for the suggestion. I'm fixing it in v2.
>>
>>>> - # Now launch gdb with our test and collect the result
>>>> - gdb_cmd = "%s %s" % (args.gdb, args.binary)
>>>> + if args.qemu:
>>>> + # Launch QEMU with binary.
>>>> + if "system" in args.qemu:
>>>> + if args.no_suspend:
>>>> + suspend = ''
>>>> + else:
>>>> + suspend = ' -S'
>>>> + cmd = f'{args.qemu} {args.qargs} {args.binary}' \
>>>> + f'{suspend} -gdb unix:path={socket_name},server=on'
>>>> + else:
>>>> + if args.no_suspend:
>>>> + suspend = ',suspend=n'
>>>> + else:
>>>> + suspend = ''
>>>> + cmd = f'{args.qemu} {args.qargs} -g {socket_name}{suspend}' \
>>>> + f' {args.binary}'
>>>> +
>>>> + log(output, "QEMU CMD: %s" % (cmd))
>>>> + inferior = subprocess.Popen(shlex.split(cmd))
>>>> +
>>>> + # Now launch gdb with our test and collect the result.
>>>> + gdb_cmd = args.gdb
>>>> + if args.binary:
>>>> + gdb_cmd += " %s" % (args.binary)
>>>> if args.gdb_args:
>>>> gdb_cmd += " %s" % (args.gdb_args)
>>>> - # run quietly and ignore .gdbinit
>>>> + # Run quietly and ignore .gdbinit.
>>>> gdb_cmd += " -q -n -batch"
>>>> - # disable pagination
>>>> + # Disable pagination.
>>>> gdb_cmd += " -ex 'set pagination off'"
>>>> - # disable prompts in case of crash
>>>> + # Disable prompts in case of crash.
>>>> gdb_cmd += " -ex 'set confirm off'"
>>>
>>> The re-formatting makes the diffs very noisy. If you want to clean up
>>> the captilization of stuff do that in another commit.
>> OK.
>>
>>>> - # connect to remote
>>>> - gdb_cmd += " -ex 'target remote %s'" % (socket_name)
>>>> - # finally the test script itself
>>>> + # Connect automatically to remote only if QEMU is launched.
>>>> + if args.qemu:
>>>> + gdb_cmd += " -ex 'target remote %s'" % (socket_name)
>>>> + # Finally the test script itself.
>>>> if args.test:
>>>> - if args.test_args:
>>>> - gdb_cmd += f" -ex \"py sys.argv={args.test_args}\""
>>>> + argv = [args.test] + args.test_args
>>>> + gdb_cmd += f" -ex \"py sys.argv={argv}\""
>>>> gdb_cmd += " -x %s" % (args.test)
>>>
>>> I can see this echoes from:
>>>
>>> env QEMU_TEST_FLAKY_TESTS=1 ./pyvenv/bin/meson test --suite thorough
>>> func-aarch64-aarch64_reverse_debug --verbose
>>>
>>> Shows:
>>>
>>> GDB CMD: /usr/bin/gdb-multiarch -q -n -batch -ex 'set pagination
>>> off' -ex 'set confirm off' -ex "py
>>> sys.argv=['/home/alex/lsrc/qemu.git/tests/functional/test_aarch64_reverse_debug.py']"
>>> -x
>>> /home/alex/lsrc/qemu.git/tests/functional/test_aarch64_reverse_debug.py
>>>
>>> But trying to piece that together on my the command line:
>>>
>>> env
>>> PYTHONPATH=/home/alex/lsrc/qemu.git/python:/home/alex/lsrc/qemu.git/tests/functional
>>> /usr/bin/gdb-multiarch -q -n -batch -ex 'set pagination off' -ex
>>> 'set confirm off' -ex "py
>>> sys.argv=['/home/alex/lsrc/qemu.git/tests/functional/test_aarch64_reverse_debug.py']"
>>> -x
>>> /home/alex/lsrc/qemu.git/tests/functional/test_aarch64_reverse_debug.py
>>> Python Exception <class 'ModuleNotFoundError'>: No module named 'pycotap'
>>> Error occurred in Python: No module named 'pycotap'
>>>
>>> What am I missing?
>> meson.build in tests/functional is properly setting PYTHONPATH in
>> the env
>> when meson runs this command. libpython called from gdb binary will inspect
>> PYTHONPATH later. It's meson that knows where site-packages from the pyvenv
>> is located, so that's why meson is setting PYTHONPATH and that's why it works
>> when run by meson.
>> We should never need to put pieces together to run QEMU tests, I
>> really
>> hate it (see my previous reply to Thomas about why using Avocado GDB for
>> test like this is not a good idea IMO).
>
> ah, and I can think of some way to avoid having to put command pieces
> together to re-run the test after we agree in the other more fundamental
> aspects of this series, e.g., ok to kept it in tests/functional, ok to
> use GDB Python API, etc.
Hmm could meson devenv help?
>
> Cheers,
> Gustavo
>
>> That said, try to add the site-packages from your pyvenv set in your
>> build dir to PYTHONPATH:
>> gromero@gromero0:/mnt/git/qemu_/build$ ls -l
>> ./pyvenv/lib/python3.10/site-packages
>> total 16
>> drwxrwxr-x 3 gromero gromero 4096 Aug 25 12:47 meson-1.8.1.dist-info
>> drwxrwxr-x 17 gromero gromero 4096 Aug 25 12:47 mesonbuild
>> drwxrwxr-x 3 gromero gromero 4096 Aug 25 12:47 pycotap
>> drwxrwxr-x 2 gromero gromero 4096 Aug 25 12:47 pycotap-1.3.1.dist-info
>> This is where the pycotap and other potential modules reside.
>> Cheers,
>> Gustavo
>>
>>>> @@ -129,10 +137,11 @@ def log(output, msg):
>>>> log(output, "GDB crashed? (%d, %d) SKIPPING" % (result, result -
>>>> 128))
>>>> exit(0)
>>>> - try:
>>>> - inferior.wait(2)
>>>> - except subprocess.TimeoutExpired:
>>>> - log(output, "GDB never connected? Killed guest")
>>>> - inferior.kill()
>>>> + if args.qemu:
>>>> + try:
>>>> + inferior.wait(2)
>>>> + except subprocess.TimeoutExpired:
>>>> + log(output, "GDB never connected? Killed guest")
>>>> + inferior.kill()
>>>> exit(result)
>>>
>>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
