Re: [PATCH v3 04/15] qemu-iotests: add option to attach gdbserver

[Max Reitz <mreitz@redhat.com>]

On 14.04.21 19:03, Emanuele Giuseppe Esposito wrote:
> Add -gdb flag and GDB_QEMU environmental variable
> to python tests to attach a gdbserver to each qemu instance.

Well, this patch doesn’t do this, but OK.

Out of interest: Why gdbserver and not “just” gdb?  On Fedora, those are 
separate packages, so I don’t have gdbserver installed, that’s why I’m 
asking.

(I’ve also never used gdbserver before.  From what I can tell, it’s 
basically just a limited version of gdb so it only serves as a server.)

> if -gdb is not provided but $GDB_QEMU is set, ignore the
> environmental variable.

s/environmental/environment/

> 
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> ---
>   tests/qemu-iotests/check      |  6 +++++-
>   tests/qemu-iotests/iotests.py |  4 ++++
>   tests/qemu-iotests/testenv.py | 15 ++++++++++++---
>   3 files changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index d1c87ceaf1..6186495eee 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -33,6 +33,9 @@ def make_argparser() -> argparse.ArgumentParser:
>                      help='pretty print output for make check')
>   
>       p.add_argument('-d', dest='debug', action='store_true', help='debug')
> +    p.add_argument('-gdb', action='store_true',
> +                   help="start gdbserver with $GDB_QEMU options. \
> +                         Default is localhost:12345")

That makes it sound like this were the default for the `-gdb` option. 
Since `-gdb` is just a switch, it doesn’t have a default (apart from 
being off by default).

So I’d rephrase this at least to “The default options are 
'localhost:12345'”.  Or maybe “start gdbserver with $GDB_QEMU options 
('localhost:12345' if $GDB_QEMU is empty)”.

Also, $GDB_QEMU as a name is a bit strange, because it does not specify 
which gdb to use; it just gives the options to use for gdb. 
$GDB_QEMU_OPTIONS would be more in line with the naming of the rest of 
the environment variables (or just $GDB_OPTIONS).

Max

>       p.add_argument('-misalign', action='store_true',
>                      help='misalign memory allocations')
>       p.add_argument('--color', choices=['on', 'off', 'auto'],