| Message ID | 20210414170352.29927-5-eesposit@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qemu_iotests: improve debugging options | expand |
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'],
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. > > if -gdb is not provided but $GDB_QEMU is set, ignore the > environmental variable. > > 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") > p.add_argument('-misalign', action='store_true', > help='misalign memory allocations') > p.add_argument('--color', choices=['on', 'off', 'auto'], > @@ -112,7 +115,8 @@ if __name__ == '__main__': > env = TestEnv(imgfmt=args.imgfmt, imgproto=args.imgproto, > aiomode=args.aiomode, cachemode=args.cachemode, > imgopts=args.imgopts, misalign=args.misalign, > - debug=args.debug, valgrind=args.valgrind) > + debug=args.debug, valgrind=args.valgrind, > + gdb=args.gdb) > > testfinder = TestFinder(test_dir=env.source_iotests) > > diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py > index 90d0b62523..05d0dc0751 100644 > --- a/tests/qemu-iotests/iotests.py > +++ b/tests/qemu-iotests/iotests.py > @@ -75,6 +75,10 @@ > qemu_prog = os.environ.get('QEMU_PROG', 'qemu') > qemu_opts = os.environ.get('QEMU_OPTIONS', '').strip().split(' ') > > +qemu_gdb = [] > +if os.environ.get('GDB_QEMU'): > + qemu_gdb = ['gdbserver'] + os.environ.get('GDB_QEMU').strip().split(' ') os.environ.get('GDB_QEMU') returns an Option[str], so mypy complains about the .strip() (thus failing iotest 297). (That can be fixed for example by either using os.environ['GDB_QEMU'] here, like most other places here do, or by something like: gdb_qemu_env = os.environ.get('GDB_QEMU') if gdb_qemu_env: qemu_gdb = ['gdbserver'] + gdb_qemu_env.strip().split(' ') ) Max > + > imgfmt = os.environ.get('IMGFMT', 'raw') > imgproto = os.environ.get('IMGPROTO', 'file') > output_dir = os.environ.get('OUTPUT_DIR', '.') > diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py > index 1fbec854c1..e131ff42cb 100644 > --- a/tests/qemu-iotests/testenv.py > +++ b/tests/qemu-iotests/testenv.py > @@ -72,7 +72,8 @@ class TestEnv(ContextManager['TestEnv']): > 'QEMU_NBD_OPTIONS', 'IMGOPTS', 'IMGFMT', 'IMGPROTO', > 'AIOMODE', 'CACHEMODE', 'VALGRIND_QEMU', > 'CACHEMODE_IS_DEFAULT', 'IMGFMT_GENERIC', 'IMGOPTSSYNTAX', > - 'IMGKEYSECRET', 'QEMU_DEFAULT_MACHINE', 'MALLOC_PERTURB_'] > + 'IMGKEYSECRET', 'QEMU_DEFAULT_MACHINE', 'MALLOC_PERTURB_', > + 'GDB_QEMU'] > > def get_env(self) -> Dict[str, str]: > env = {} > @@ -163,7 +164,8 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: str, > imgopts: Optional[str] = None, > misalign: bool = False, > debug: bool = False, > - valgrind: bool = False) -> None: > + valgrind: bool = False, > + gdb: bool = False) -> None: > self.imgfmt = imgfmt > self.imgproto = imgproto > self.aiomode = aiomode > @@ -171,6 +173,11 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: str, > self.misalign = misalign > self.debug = debug > > + if gdb: > + self.gdb_qemu = os.environ.get('GDB_QEMU', 'localhost:12345') > + elif 'GDB_QEMU' in os.environ: > + del os.environ['GDB_QEMU'] > + > if valgrind: > self.valgrind_qemu = 'y' > > @@ -268,7 +275,9 @@ def print_env(self) -> None: > PLATFORM -- {platform} > TEST_DIR -- {TEST_DIR} > SOCK_DIR -- {SOCK_DIR} > -SOCKET_SCM_HELPER -- {SOCKET_SCM_HELPER}""" > +SOCKET_SCM_HELPER -- {SOCKET_SCM_HELPER} > +GDB_QEMU -- "{GDB_QEMU}" > +""" > > args = collections.defaultdict(str, self.get_env()) > >
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'], >
On 30.04.21 23:03, Emanuele Giuseppe Esposito wrote: > > > 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. That’s possible, but I think better would be to contrast it with what this patch doesn’t do, but what one could think when reading this description. I.e. to say “Add/define -gdb flag [...] to each qemu instance. This patch only adds and parses this flag, it does not yet add the implementation for it.” >> 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. All right. I thought gdb could be used as a server, too, but... Looks like it can’t. (Like, I thought, you could do something like “gdb -ex 'listen localhost:12345' $cmd”. But seems like there is no server built into gdb proper.) Max
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") p.add_argument('-misalign', action='store_true', help='misalign memory allocations') p.add_argument('--color', choices=['on', 'off', 'auto'], @@ -112,7 +115,8 @@ if __name__ == '__main__': env = TestEnv(imgfmt=args.imgfmt, imgproto=args.imgproto, aiomode=args.aiomode, cachemode=args.cachemode, imgopts=args.imgopts, misalign=args.misalign, - debug=args.debug, valgrind=args.valgrind) + debug=args.debug, valgrind=args.valgrind, + gdb=args.gdb) testfinder = TestFinder(test_dir=env.source_iotests) diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index 90d0b62523..05d0dc0751 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -75,6 +75,10 @@ qemu_prog = os.environ.get('QEMU_PROG', 'qemu') qemu_opts = os.environ.get('QEMU_OPTIONS', '').strip().split(' ') +qemu_gdb = [] +if os.environ.get('GDB_QEMU'): + qemu_gdb = ['gdbserver'] + os.environ.get('GDB_QEMU').strip().split(' ') + imgfmt = os.environ.get('IMGFMT', 'raw') imgproto = os.environ.get('IMGPROTO', 'file') output_dir = os.environ.get('OUTPUT_DIR', '.') diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py index 1fbec854c1..e131ff42cb 100644 --- a/tests/qemu-iotests/testenv.py +++ b/tests/qemu-iotests/testenv.py @@ -72,7 +72,8 @@ class TestEnv(ContextManager['TestEnv']): 'QEMU_NBD_OPTIONS', 'IMGOPTS', 'IMGFMT', 'IMGPROTO', 'AIOMODE', 'CACHEMODE', 'VALGRIND_QEMU', 'CACHEMODE_IS_DEFAULT', 'IMGFMT_GENERIC', 'IMGOPTSSYNTAX', - 'IMGKEYSECRET', 'QEMU_DEFAULT_MACHINE', 'MALLOC_PERTURB_'] + 'IMGKEYSECRET', 'QEMU_DEFAULT_MACHINE', 'MALLOC_PERTURB_', + 'GDB_QEMU'] def get_env(self) -> Dict[str, str]: env = {} @@ -163,7 +164,8 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: str, imgopts: Optional[str] = None, misalign: bool = False, debug: bool = False, - valgrind: bool = False) -> None: + valgrind: bool = False, + gdb: bool = False) -> None: self.imgfmt = imgfmt self.imgproto = imgproto self.aiomode = aiomode @@ -171,6 +173,11 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: str, self.misalign = misalign self.debug = debug + if gdb: + self.gdb_qemu = os.environ.get('GDB_QEMU', 'localhost:12345') + elif 'GDB_QEMU' in os.environ: + del os.environ['GDB_QEMU'] + if valgrind: self.valgrind_qemu = 'y' @@ -268,7 +275,9 @@ def print_env(self) -> None: PLATFORM -- {platform} TEST_DIR -- {TEST_DIR} SOCK_DIR -- {SOCK_DIR} -SOCKET_SCM_HELPER -- {SOCKET_SCM_HELPER}""" +SOCKET_SCM_HELPER -- {SOCKET_SCM_HELPER} +GDB_QEMU -- "{GDB_QEMU}" +""" args = collections.defaultdict(str, self.get_env())
Add -gdb flag and GDB_QEMU environmental variable to python tests to attach a gdbserver to each qemu instance. if -gdb is not provided but $GDB_QEMU is set, ignore the environmental variable. 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(-)