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

From: Emanuele Giuseppe Esposito <eesposit@redhat.com>
To: Max Reitz <mreitz@redhat.com>, qemu-block@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>,
	Eduardo Habkost <ehabkost@redhat.com>,
	qemu-devel@nongnu.org, Cleber Rosa <crosa@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>, John Snow <jsnow@redhat.com>
Subject: Re: [PATCH v3 04/15] qemu-iotests: add option to attach gdbserver
Date: Fri, 30 Apr 2021 23:03:30 +0200	[thread overview]
Message-ID: <4189f418-ef3f-bb94-585a-5efc95ef1257@redhat.com> (raw)
In-Reply-To: <e06a8560-641b-bf10-230c-fa99a21998c8@redhat.com>

On 30/04/2021 13:38, Max Reitz wrote:
> 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.

Maybe "define" rather than "add"? In the sense of defining the "-gdb" 
option, which is what it actually does.

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

As far as I have tried, using only gdb with ./check is very hard to use, 
because the stdout is filtered out by the script.
So invoking gdb attaches it to QEMU, but it is not possible to start 
execution (run command) or interact with it, because of the python 
script filtering. This leaves the test hanging.

gdbserver is just something that a gdb client can attach to (for 
example, in another console or even in another host) for example with 
the command
# gdb -iex "target remote localhost:12345" . This provides a nice and 
separate gdb monitor to the client.

Emanuele
> 
> (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'],
> 