Re: [PATCH 1/4] tests/guest-debug: Make QEMU optional in run-test.py

["Alex Bennée" <alex.bennee@linaro.org>]

Gustavo Romero <gustavo.romero@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.romero@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.

>  
> -    # 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.

> -    # 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?

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