[RFC PATCH v2 00/11] qemu_iotests: improve debugging options

[RFC PATCH v2 00/11] qemu_iotests: improve debugging options

[Emanuele Giuseppe Esposito]

This series adds the option to attach gdbserver and valgrind
to the QEMU binary running in qemu_iotests.
It also allows to redirect QEMU binaries output of the python tests
to the stdout, instead of a log file.

Patches 1-6 introduce the -gdb option to both python and bash tests, 
7-10 extend the already existing -valgrind flag to work also on 
python tests, and patch 11 introduces -p to enable logging to stdout.

In particular, patches 1,2,4,8 focus on extending the QMP socket timers
when using gdb/valgrind, otherwise the python tests will fail due to
delays in the QMP responses.

This series is tested on the previous serie
"qemu-iotests: quality of life improvements"
but independent from it, so it can be applied separately.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
v2:
- add valgrind and print patches
- better splitup of patches, and clearer commit messages

Emanuele Giuseppe Esposito (11):
  python: qemu: add timer parameter for qmp.accept socket
  python: qemu: pass the wrapper field from QEMUQtestmachine to
    QEMUMachine
  qemu-iotests: add option to attach gdbserver
  qemu-iotests: delay QMP socket timers
  qemu_iotests: insert gdbserver command line as wrapper for qemu binary
  qemu-iotests: add gdbserver option to script tests too
  qemu_iotests: extend the check script to support valgrind for python
    tests
  qemu_iotests: extent QMP socket timeout when using valgrind
  qemu_iotests: allow valgrint to print/delete the generated log file
  qemu_iotests: insert valgrind command line as wrapper for qemu binary
  qemu_iotests: add option to show qemu binary logs on stdout

 python/qemu/machine.py        | 12 ++++--
 python/qemu/qtest.py          |  8 ++--
 tests/qemu-iotests/check      |  7 +++-
 tests/qemu-iotests/common.rc  |  8 +++-
 tests/qemu-iotests/iotests.py | 69 ++++++++++++++++++++++++++++++++---
 tests/qemu-iotests/testenv.py | 24 ++++++++++--
 6 files changed, 111 insertions(+), 17 deletions(-)

-- 
2.30.2

[RFC PATCH v2 01/11] python: qemu: add timer parameter for qmp.accept socket

[Emanuele Giuseppe Esposito]

Extend the _post_launch function to include the timer as
parameter instead of defaulting to 15 sec.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 python/qemu/machine.py | 4 ++--
 python/qemu/qtest.py   | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 6e44bda337..c721e07d63 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -321,9 +321,9 @@ def _pre_launch(self) -> None:
                 nickname=self._name
             )
 
-    def _post_launch(self) -> None:
+    def _post_launch(self, timer) -> None:
         if self._qmp_connection:
-            self._qmp.accept()
+            self._qmp.accept(timer)
 
     def _post_shutdown(self) -> None:
         """
diff --git a/python/qemu/qtest.py b/python/qemu/qtest.py
index 39a0cf62fe..0d01715086 100644
--- a/python/qemu/qtest.py
+++ b/python/qemu/qtest.py
@@ -138,9 +138,9 @@ def _pre_launch(self) -> None:
         super()._pre_launch()
         self._qtest = QEMUQtestProtocol(self._qtest_path, server=True)
 
-    def _post_launch(self) -> None:
+    def _post_launch(self, timer) -> None:
         assert self._qtest is not None
-        super()._post_launch()
+        super()._post_launch(timer)
         self._qtest.accept()
 
     def _post_shutdown(self) -> None:
-- 
2.30.2

Re: [RFC PATCH v2 01/11] python: qemu: add timer parameter for qmp.accept socket

[John Snow]

On 4/7/21 9:50 AM, Emanuele Giuseppe Esposito wrote:
> Extend the _post_launch function to include the timer as
> parameter instead of defaulting to 15 sec.
> 
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> ---
>   python/qemu/machine.py | 4 ++--
>   python/qemu/qtest.py   | 4 ++--
>   2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
> index 6e44bda337..c721e07d63 100644
> --- a/python/qemu/machine.py
> +++ b/python/qemu/machine.py
> @@ -321,9 +321,9 @@ def _pre_launch(self) -> None:
>                   nickname=self._name
>               )
>   
> -    def _post_launch(self) -> None:
> +    def _post_launch(self, timer) -> None:
>           if self._qmp_connection:
> -            self._qmp.accept()
> +            self._qmp.accept(timer)
>   
>       def _post_shutdown(self) -> None:
>           """
> diff --git a/python/qemu/qtest.py b/python/qemu/qtest.py
> index 39a0cf62fe..0d01715086 100644
> --- a/python/qemu/qtest.py
> +++ b/python/qemu/qtest.py
> @@ -138,9 +138,9 @@ def _pre_launch(self) -> None:
>           super()._pre_launch()
>           self._qtest = QEMUQtestProtocol(self._qtest_path, server=True)
>   
> -    def _post_launch(self) -> None:
> +    def _post_launch(self, timer) -> None:
>           assert self._qtest is not None
> -        super()._post_launch()
> +        super()._post_launch(timer)
>           self._qtest.accept()
>   
>       def _post_shutdown(self) -> None:
> 

Are you forgetting to change _launch() to provide some default value for 
what timer needs to be?

I think for the "event" callbacks here, I'd prefer configuring the 
behavior as a property instead of passing it around as a parameter.

(Also, we have an awful lot of timeouts now... is it time to think about 
rewriting this using asyncio so that we can allow the callers to specify 
their own timeouts in with context blocks? Just a thought for later; we 
have an awful lot of timeouts scattered throughout machine.py, qmp.py, etc.)

--js

Re: [RFC PATCH v2 01/11] python: qemu: add timer parameter for qmp.accept socket

[Emanuele Giuseppe Esposito]

>> --- a/python/qemu/qtest.py
>> +++ b/python/qemu/qtest.py
>> @@ -138,9 +138,9 @@ def _pre_launch(self) -> None:
>>           super()._pre_launch()
>>           self._qtest = QEMUQtestProtocol(self._qtest_path, server=True)
>> -    def _post_launch(self) -> None:
>> +    def _post_launch(self, timer) -> None:
>>           assert self._qtest is not None
>> -        super()._post_launch()
>> +        super()._post_launch(timer)
>>           self._qtest.accept()
>>       def _post_shutdown(self) -> None:
>>
> 
> Are you forgetting to change _launch() to provide some default value for 
> what timer needs to be?
> 
> I think for the "event" callbacks here, I'd prefer configuring the 
> behavior as a property instead of passing it around as a parameter.

I agree, I changed it in a field of the QEMUMachine class called 
_qmp_timer that defaults to 15 seconds.

Emanuele

[RFC PATCH v2 02/11] python: qemu: pass the wrapper field from QEMUQtestmachine to QEMUMachine

[Emanuele Giuseppe Esposito]

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 python/qemu/machine.py | 2 +-
 python/qemu/qtest.py   | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index c721e07d63..18d32ebe45 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -109,7 +109,7 @@ def __init__(self,
 
         self._binary = binary
         self._args = list(args)
-        self._wrapper = wrapper
+        self._wrapper = list(wrapper)
 
         self._name = name or "qemu-%d" % os.getpid()
         self._test_dir = test_dir
diff --git a/python/qemu/qtest.py b/python/qemu/qtest.py
index 0d01715086..4c90daf430 100644
--- a/python/qemu/qtest.py
+++ b/python/qemu/qtest.py
@@ -111,6 +111,7 @@ class QEMUQtestMachine(QEMUMachine):
     def __init__(self,
                  binary: str,
                  args: Sequence[str] = (),
+                 wrapper: Sequence[str] = (),
                  name: Optional[str] = None,
                  test_dir: str = "/var/tmp",
                  socket_scm_helper: Optional[str] = None,
@@ -119,7 +120,8 @@ def __init__(self,
             name = "qemu-%d" % os.getpid()
         if sock_dir is None:
             sock_dir = test_dir
-        super().__init__(binary, args, name=name, test_dir=test_dir,
+        super().__init__(binary, args, wrapper=wrapper, name=name,
+                         test_dir=test_dir,
                          socket_scm_helper=socket_scm_helper,
                          sock_dir=sock_dir)
         self._qtest: Optional[QEMUQtestProtocol] = None
-- 
2.30.2

Re: [RFC PATCH v2 02/11] python: qemu: pass the wrapper field from QEMUQtestmachine to QEMUMachine

[John Snow]

On 4/7/21 9:50 AM, Emanuele Giuseppe Esposito wrote:
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> ---
>   python/qemu/machine.py | 2 +-
>   python/qemu/qtest.py   | 4 +++-
>   2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
> index c721e07d63..18d32ebe45 100644
> --- a/python/qemu/machine.py
> +++ b/python/qemu/machine.py
> @@ -109,7 +109,7 @@ def __init__(self,
>   
>           self._binary = binary
>           self._args = list(args)
> -        self._wrapper = wrapper
> +        self._wrapper = list(wrapper)
>  

Unrelated change?

(I'm assuming you want to copy the user's input to explicitly avoid 
sharing state. Commit message blurb for this would be good.)

>           self._name = name or "qemu-%d" % os.getpid()
>           self._test_dir = test_dir
> diff --git a/python/qemu/qtest.py b/python/qemu/qtest.py
> index 0d01715086..4c90daf430 100644
> --- a/python/qemu/qtest.py
> +++ b/python/qemu/qtest.py
> @@ -111,6 +111,7 @@ class QEMUQtestMachine(QEMUMachine):
>       def __init__(self,
>                    binary: str,
>                    args: Sequence[str] = (),
> +                 wrapper: Sequence[str] = (),
>                    name: Optional[str] = None,
>                    test_dir: str = "/var/tmp",
>                    socket_scm_helper: Optional[str] = None,
> @@ -119,7 +120,8 @@ def __init__(self,
>               name = "qemu-%d" % os.getpid()
>           if sock_dir is None:
>               sock_dir = test_dir
> -        super().__init__(binary, args, name=name, test_dir=test_dir,
> +        super().__init__(binary, args, wrapper=wrapper, name=name,
> +                         test_dir=test_dir,
>                            socket_scm_helper=socket_scm_helper,
>                            sock_dir=sock_dir)
>           self._qtest: Optional[QEMUQtestProtocol] = None
> 

ACK

Re: [RFC PATCH v2 02/11] python: qemu: pass the wrapper field from QEMUQtestmachine to QEMUMachine

[Emanuele Giuseppe Esposito]

>> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
>> index c721e07d63..18d32ebe45 100644
>> --- a/python/qemu/machine.py
>> +++ b/python/qemu/machine.py
>> @@ -109,7 +109,7 @@ def __init__(self,
>>           self._binary = binary
>>           self._args = list(args)
>> -        self._wrapper = wrapper
>> +        self._wrapper = list(wrapper)
>>
> 
> Unrelated change?
> 
> (I'm assuming you want to copy the user's input to explicitly avoid 
> sharing state. Commit message blurb for this would be good.)

Yes, unrelated change. I do not see the benefit of copying the user 
state. I will drop it.

>>
> 
> ACK

(Apologies for the ignorance, is this an Acked-by?)

Emanuele

Re: [RFC PATCH v2 02/11] python: qemu: pass the wrapper field from QEMUQtestmachine to QEMUMachine

[John Snow]

On 4/9/21 12:07 PM, Emanuele Giuseppe Esposito wrote:
> 
>>> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
>>> index c721e07d63..18d32ebe45 100644
>>> --- a/python/qemu/machine.py
>>> +++ b/python/qemu/machine.py
>>> @@ -109,7 +109,7 @@ def __init__(self,
>>>           self._binary = binary
>>>           self._args = list(args)
>>> -        self._wrapper = wrapper
>>> +        self._wrapper = list(wrapper)
>>>
>>
>> Unrelated change?
>>
>> (I'm assuming you want to copy the user's input to explicitly avoid 
>> sharing state. Commit message blurb for this would be good.)
> 
> Yes, unrelated change. I do not see the benefit of copying the user 
> state. I will drop it.
> 
>>>
>>
>> ACK
> 
> (Apologies for the ignorance, is this an Acked-by?)
> 
> Emanuele
> 

Ah, yes. I just mean to say: "Minor style stuff, but looks good to me 
and I'll [likely] approve the non-RFC versions of these."

(Pending tests passing via flake8/pylint/mypy. Sorry those aren't 
formalized in the tree yet.)

--js

[RFC PATCH v2 03/11] qemu-iotests: add option to attach gdbserver

[Emanuele Giuseppe Esposito]

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 | 18 +++++++++++++++---
 3 files changed, 24 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(' ')
+
 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..669eb6b925 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,14 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: str,
         self.misalign = misalign
         self.debug = debug
 
+        self.gdb_qemu = os.getenv('GDB_QEMU')
+
+        if gdb and not self.gdb_qemu:
+            self.gdb_qemu = 'localhost:12345'
+        elif self.gdb_qemu and not gdb:
+            del self.gdb_qemu
+            del os.environ['GDB_QEMU']
+
         if valgrind:
             self.valgrind_qemu = 'y'
 
@@ -268,7 +278,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())
 
-- 
2.30.2

Re: [RFC PATCH v2 03/11] qemu-iotests: add option to attach gdbserver

[Paolo Bonzini]

On 07/04/21 15:50, Emanuele Giuseppe Esposito wrote:
> +        self.gdb_qemu = os.getenv('GDB_QEMU')
> +
> +        if gdb and not self.gdb_qemu:
> +            self.gdb_qemu = 'localhost:12345'
> +        elif self.gdb_qemu and not gdb:
> +            del self.gdb_qemu
> +            del os.environ['GDB_QEMU']

Alternatively:

     if gdb:
         self.gdb_qemu = os.environ.get('GDB_QEMU', 'localhost:12345')
     elif 'GDB_QEMU' in os.environ:
         del os.environ['GDB_QEMU']

> +GDB_QEMU      -- "{GDB_QEMU}"

Perhaps only include this if gdbserver is actually in use?  (Or not at 
all, since gdbserver anyway prints the port).

Paolo

Re: [RFC PATCH v2 03/11] qemu-iotests: add option to attach gdbserver

[Emanuele Giuseppe Esposito]

On 08/04/2021 17:40, Paolo Bonzini wrote:
> On 07/04/21 15:50, Emanuele Giuseppe Esposito wrote:
>> +        self.gdb_qemu = os.getenv('GDB_QEMU')
>> +
>> +        if gdb and not self.gdb_qemu:
>> +            self.gdb_qemu = 'localhost:12345'
>> +        elif self.gdb_qemu and not gdb:
>> +            del self.gdb_qemu
>> +            del os.environ['GDB_QEMU']
> 
> Alternatively:
> 
>      if gdb:
>          self.gdb_qemu = os.environ.get('GDB_QEMU', 'localhost:12345')
>      elif 'GDB_QEMU' in os.environ:
>          del os.environ['GDB_QEMU']

makes sense, thanks.
> 
>> +GDB_QEMU      -- "{GDB_QEMU}"
> 
> Perhaps only include this if gdbserver is actually in use?  (Or not at 
> all, since gdbserver anyway prints the port).

You forgot that by default all logs go to a log file :) so unless you 
find the corresponding log, it is not easy to find the GDB port.

Thank you,
Emanuele

> 
> Paolo
> 

[RFC PATCH v2 04/11] qemu-iotests: delay QMP socket timers

[Emanuele Giuseppe Esposito]

Attaching a gdbserver implies that the qmp socket
should wait indefinitely for an answer from QEMU.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 python/qemu/machine.py        |  4 +++-
 tests/qemu-iotests/iotests.py | 21 +++++++++++++++++----
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 18d32ebe45..284b73385f 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -408,7 +408,9 @@ def _launch(self) -> None:
                                        stderr=subprocess.STDOUT,
                                        shell=False,
                                        close_fds=False)
-        self._post_launch()
+
+        timer = None if 'gdbserver' in self._wrapper else 15.0
+        self._post_launch(timer)
 
     def _early_cleanup(self) -> None:
         """
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 05d0dc0751..17f07710db 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -686,7 +686,10 @@ def qmp_to_opts(self, obj):
 
     def get_qmp_events_filtered(self, wait=60.0):
         result = []
-        for ev in self.get_qmp_events(wait=wait):
+        qmp_wait = wait
+        if qemu_gdb:
+            qmp_wait = 0.0
+        for ev in self.get_qmp_events(wait=qmp_wait):
             result.append(filter_qmp_event(ev))
         return result
 
@@ -987,13 +990,17 @@ def cancel_and_wait(self, drive='drive0', force=False,
         result = self.vm.qmp('block-job-cancel', device=drive, force=force)
         self.assert_qmp(result, 'return', {})
 
+        qmp_wait = wait
+        if qemu_gdb:
+            qmp_wait = 0.0
+
         if resume:
             self.vm.resume_drive(drive)
 
         cancelled = False
         result = None
         while not cancelled:
-            for event in self.vm.get_qmp_events(wait=wait):
+            for event in self.vm.get_qmp_events(wait=qmp_wait):
                 if event['event'] == 'BLOCK_JOB_COMPLETED' or \
                    event['event'] == 'BLOCK_JOB_CANCELLED':
                     self.assert_qmp(event, 'data/device', drive)
@@ -1009,8 +1016,11 @@ def cancel_and_wait(self, drive='drive0', force=False,
     def wait_until_completed(self, drive='drive0', check_offset=True,
                              wait=60.0, error=None):
         '''Wait for a block job to finish, returning the event'''
+        qmp_wait = wait
+        if qemu_gdb:
+            qmp_wait = 0.0
         while True:
-            for event in self.vm.get_qmp_events(wait=wait):
+            for event in self.vm.get_qmp_events(wait=qmp_wait):
                 if event['event'] == 'BLOCK_JOB_COMPLETED':
                     self.assert_qmp(event, 'data/device', drive)
                     if error is None:
@@ -1054,7 +1064,10 @@ def complete_and_wait(self, drive='drive0', wait_ready=True,
         self.assertTrue(event['data']['type'] in ['mirror', 'commit'])
 
     def pause_wait(self, job_id='job0'):
-        with Timeout(3, "Timeout waiting for job to pause"):
+        def_timeout = 3
+        if qemu_gdb:
+            def_timeout = 3000
+        with Timeout(def_timeout, "Timeout waiting for job to pause"):
             while True:
                 result = self.vm.qmp('query-block-jobs')
                 found = False
-- 
2.30.2

Re: [RFC PATCH v2 04/11] qemu-iotests: delay QMP socket timers

[Paolo Bonzini]

On 07/04/21 15:50, Emanuele Giuseppe Esposito wrote:
>       def get_qmp_events_filtered(self, wait=60.0):
>           result = []
> -        for ev in self.get_qmp_events(wait=wait):
> +        qmp_wait = wait
> +        if qemu_gdb:
> +            qmp_wait = 0.0
> +        for ev in self.get_qmp_events(wait=qmp_wait):
>               result.append(filter_qmp_event(ev))
>           return result

Should this be handled in get_qmp_events instead, since you're basically 
changing all the callers?

Paolo

Re: [RFC PATCH v2 04/11] qemu-iotests: delay QMP socket timers

[Emanuele Giuseppe Esposito]

On 08/04/2021 17:40, Paolo Bonzini wrote:
> On 07/04/21 15:50, Emanuele Giuseppe Esposito wrote:
>>       def get_qmp_events_filtered(self, wait=60.0):
>>           result = []
>> -        for ev in self.get_qmp_events(wait=wait):
>> +        qmp_wait = wait
>> +        if qemu_gdb:
>> +            qmp_wait = 0.0
>> +        for ev in self.get_qmp_events(wait=qmp_wait):
>>               result.append(filter_qmp_event(ev))
>>           return result
> 
> Should this be handled in get_qmp_events instead, since you're basically 
> changing all the callers?

get_qmp_events is in python/machine.py, which as I understand might be 
used also by some other scripts, so I want to keep the changes there to 
the minimum. Also, machine.py has no access to qemu_gdb or 
qemu_valgrind, so passing a boolean or something to delay the timer 
would still require to add a similar check in all sections.

Or do you have a cleaner way to do this?

Emanuele

Re: [RFC PATCH v2 04/11] qemu-iotests: delay QMP socket timers

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 1124 bytes --]

Il gio 8 apr 2021, 18:06 Emanuele Giuseppe Esposito <eesposit@redhat.com>
ha scritto:

>
>
> On 08/04/2021 17:40, Paolo Bonzini wrote:
> > On 07/04/21 15:50, Emanuele Giuseppe Esposito wrote:
> >>       def get_qmp_events_filtered(self, wait=60.0):
> >>           result = []
> >> -        for ev in self.get_qmp_events(wait=wait):
> >> +        qmp_wait = wait
> >> +        if qemu_gdb:
> >> +            qmp_wait = 0.0
> >> +        for ev in self.get_qmp_events(wait=qmp_wait):
> >>               result.append(filter_qmp_event(ev))
> >>           return result
> >
> > Should this be handled in get_qmp_events instead, since you're basically
> > changing all the callers?
>
> get_qmp_events is in python/machine.py, which as I understand might be
> used also by some other scripts, so I want to keep the changes there to
> the minimum. Also, machine.py has no access to qemu_gdb or
> qemu_valgrind, so passing a boolean or something to delay the timer
> would still require to add a similar check in all sections.
>
> Or do you have a cleaner way to do this?
>

Maybe a subclass IotestsMachine?

Paolo


> Emanuele
>
>

[-- Attachment #2: Type: text/html, Size: 1922 bytes --]

Re: [RFC PATCH v2 04/11] qemu-iotests: delay QMP socket timers

[Emanuele Giuseppe Esposito]

On 08/04/2021 21:03, Paolo Bonzini wrote:
> 
> 
> Il gio 8 apr 2021, 18:06 Emanuele Giuseppe Esposito <eesposit@redhat.com 
> <mailto:eesposit@redhat.com>> ha scritto:
> 
> 
> 
>     On 08/04/2021 17:40, Paolo Bonzini wrote:
>      > On 07/04/21 15:50, Emanuele Giuseppe Esposito wrote:
>      >>       def get_qmp_events_filtered(self, wait=60.0):
>      >>           result = []
>      >> -        for ev in self.get_qmp_events(wait=wait):
>      >> +        qmp_wait = wait
>      >> +        if qemu_gdb:
>      >> +            qmp_wait = 0.0
>      >> +        for ev in self.get_qmp_events(wait=qmp_wait):
>      >>               result.append(filter_qmp_event(ev))
>      >>           return result
>      >
>      > Should this be handled in get_qmp_events instead, since you're
>     basically
>      > changing all the callers?
> 
>     get_qmp_events is in python/machine.py, which as I understand might be
>     used also by some other scripts, so I want to keep the changes there to
>     the minimum. Also, machine.py has no access to qemu_gdb or
>     qemu_valgrind, so passing a boolean or something to delay the timer
>     would still require to add a similar check in all sections.
> 
>     Or do you have a cleaner way to do this?
> 
> 
> Maybe a subclass IotestsMachine?
> 

I actually figured that I could override get_qmp_events and put the 
check there. Something like (simplified):

class VM(qtest.QEMUQtestMachine):

	...

	def get_qmp_events(self, wait)
		if qemu_gdb or qemu_valgrind:
			wait = 0.0
		return super().get_qmp_events(wait)

Emanuele

[RFC PATCH v2 05/11] qemu_iotests: insert gdbserver command line as wrapper for qemu binary

[Emanuele Giuseppe Esposito]

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 tests/qemu-iotests/iotests.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 17f07710db..8f6bb20af5 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -579,7 +579,8 @@ class VM(qtest.QEMUQtestMachine):
 
     def __init__(self, path_suffix=''):
         name = "qemu%s-%d" % (path_suffix, os.getpid())
-        super().__init__(qemu_prog, qemu_opts, name=name,
+        super().__init__(qemu_prog, qemu_opts, wrapper=qemu_gdb,
+                         name=name,
                          test_dir=test_dir,
                          socket_scm_helper=socket_scm_helper,
                          sock_dir=sock_dir)
-- 
2.30.2

[RFC PATCH v2 06/11] qemu-iotests: add gdbserver option to script tests too

[Emanuele Giuseppe Esposito]

The only limitation here is that running a script with gdbserver
will make the test output mismatch with the expected
results, making the test fail.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 tests/qemu-iotests/common.rc | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 65cdba5723..53a3310fee 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -166,8 +166,14 @@ _qemu_wrapper()
         if [ -n "${QEMU_NEED_PID}" ]; then
             echo $BASHPID > "${QEMU_TEST_DIR}/qemu-${_QEMU_HANDLE}.pid"
         fi
+
+        GDB="${QEMU_PROG}"
+        if [ ! -z ${GDB_QEMU} ]; then
+            GDB="gdbserver ${GDB_QEMU} ${GDB}"
+        fi
+
         VALGRIND_QEMU="${VALGRIND_QEMU_VM}" _qemu_proc_exec "${VALGRIND_LOGFILE}" \
-            "$QEMU_PROG" $QEMU_OPTIONS "$@"
+           $GDB $QEMU_OPTIONS "$@"
     )
     RETVAL=$?
     _qemu_proc_valgrind_log "${VALGRIND_LOGFILE}" $RETVAL
-- 
2.30.2

[RFC PATCH v2 07/11] qemu_iotests: extend the check script to support valgrind for python tests

[Emanuele Giuseppe Esposito]

Currently, the check script only parses the option and sets the
VALGRIND_QEMU environmental variable to "y".
Add another iotests python variable that prepares the command line,
identical to the one provided in the test scripts.

Because the python script does not know in advance the valgrind
PID to assign to the log file name, use the "%p" flag in valgrind
log file name that automatically puts the process PID at runtime.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 tests/qemu-iotests/iotests.py | 11 +++++++++++
 tests/qemu-iotests/testenv.py |  1 +
 2 files changed, 12 insertions(+)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 8f6bb20af5..7c28f0cb74 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -96,6 +96,17 @@
     sys.stderr.write('Please run this test via the "check" script\n')
     sys.exit(os.EX_USAGE)
 
+qemu_valgrind = []
+if os.environ.get('VALGRIND_QEMU') == "y" and \
+    os.environ.get('NO_VALGRIND') != "y":
+    valgrind_logfile = "--log-file=" + test_dir.strip()
+    # %p allows to put the valgrind process PID, since
+    # we don't know it a priori (subprocess.Peopen is
+    # not yet invoked)
+    valgrind_logfile += "/%p.valgrind"
+
+    qemu_valgrind = ['valgrind', valgrind_logfile, '--error-exitcode=99']
+
 socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', 'socket_scm_helper')
 
 luks_default_secret_object = 'secret,id=keysec0,data=' + \
diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index 669eb6b925..86798bf752 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -280,6 +280,7 @@ def print_env(self) -> None:
 SOCK_DIR      -- {SOCK_DIR}
 SOCKET_SCM_HELPER -- {SOCKET_SCM_HELPER}
 GDB_QEMU      -- "{GDB_QEMU}"
+VALGRIND_QEMU     -- "{VALGRIND_QEMU}"
 """
 
         args = collections.defaultdict(str, self.get_env())
-- 
2.30.2

[RFC PATCH v2 08/11] qemu_iotests: extent QMP socket timeout when using valgrind

[Emanuele Giuseppe Esposito]

As with gdbserver, valgrind delays the test execution, so
the default QMP socket timeout expires too soon.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 python/qemu/machine.py        | 4 +++-
 tests/qemu-iotests/iotests.py | 8 ++++----
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 284b73385f..4b6eb39856 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -409,7 +409,9 @@ def _launch(self) -> None:
                                        shell=False,
                                        close_fds=False)
 
-        timer = None if 'gdbserver' in self._wrapper else 15.0
+        delay_timer = 'gdbserver' in self._wrapper
+        delay_timer |= 'valgrind' in self._wrapper
+        timer = None if delay_timer else 15.0
         self._post_launch(timer)
 
     def _early_cleanup(self) -> None:
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 7c28f0cb74..56733954b2 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -699,7 +699,7 @@ def qmp_to_opts(self, obj):
     def get_qmp_events_filtered(self, wait=60.0):
         result = []
         qmp_wait = wait
-        if qemu_gdb:
+        if qemu_gdb or qemu_valgrind:
             qmp_wait = 0.0
         for ev in self.get_qmp_events(wait=qmp_wait):
             result.append(filter_qmp_event(ev))
@@ -1003,7 +1003,7 @@ def cancel_and_wait(self, drive='drive0', force=False,
         self.assert_qmp(result, 'return', {})
 
         qmp_wait = wait
-        if qemu_gdb:
+        if qemu_gdb or qemu_valgrind:
             qmp_wait = 0.0
 
         if resume:
@@ -1029,7 +1029,7 @@ def wait_until_completed(self, drive='drive0', check_offset=True,
                              wait=60.0, error=None):
         '''Wait for a block job to finish, returning the event'''
         qmp_wait = wait
-        if qemu_gdb:
+        if qemu_gdb or qemu_valgrind:
             qmp_wait = 0.0
         while True:
             for event in self.vm.get_qmp_events(wait=qmp_wait):
@@ -1077,7 +1077,7 @@ def complete_and_wait(self, drive='drive0', wait_ready=True,
 
     def pause_wait(self, job_id='job0'):
         def_timeout = 3
-        if qemu_gdb:
+        if qemu_gdb or qemu_valgrind:
             def_timeout = 3000
         with Timeout(def_timeout, "Timeout waiting for job to pause"):
             while True:
-- 
2.30.2

[RFC PATCH v2 09/11] qemu_iotests: allow valgrint to print/delete the generated log file

[Emanuele Giuseppe Esposito]

When using valgrind on the test scripts, it generates a log file
in $TEST_DIR that is either print (if valgrind finds problems) or
otherwise deleted. Provide the same exact behavior when using
-valgrind on the python tests.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 tests/qemu-iotests/iotests.py | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 56733954b2..b6166b6f7b 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -597,6 +597,26 @@ def __init__(self, path_suffix=''):
                          sock_dir=sock_dir)
         self._num_drives = 0
 
+    def subprocess_check_valgrind(self, valgrind) -> None:
+
+        if not valgrind:
+            return
+
+        valgrind_filename =  test_dir + "/" + str(self._popen.pid) + ".valgrind"
+
+        if self.exitcode() == 99:
+            with open(valgrind_filename) as f:
+                content = f.readlines()
+            for line in content:
+                print(line, end ="")
+            print("")
+        else:
+            os.remove(valgrind_filename)
+
+    def _post_shutdown(self) -> None:
+        super()._post_shutdown()
+        self.subprocess_check_valgrind(qemu_valgrind)
+
     def add_object(self, opts):
         self._args.append('-object')
         self._args.append(opts)
-- 
2.30.2

[RFC PATCH v2 10/11] qemu_iotests: insert valgrind command line as wrapper for qemu binary

[Emanuele Giuseppe Esposito]

The priority will be given to gdb command line, meaning if -gdb
and -valgrind parameters are given, only gdb will be wrapped around
the qemu binary.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 tests/qemu-iotests/iotests.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index b6166b6f7b..b23bfdfdff 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -590,7 +590,8 @@ class VM(qtest.QEMUQtestMachine):
 
     def __init__(self, path_suffix=''):
         name = "qemu%s-%d" % (path_suffix, os.getpid())
-        super().__init__(qemu_prog, qemu_opts, wrapper=qemu_gdb,
+        wrapper = qemu_gdb if qemu_gdb else qemu_valgrind
+        super().__init__(qemu_prog, qemu_opts, wrapper=wrapper,
                          name=name,
                          test_dir=test_dir,
                          socket_scm_helper=socket_scm_helper,
-- 
2.30.2

[RFC PATCH v2 11/11] qemu_iotests: add option to show qemu binary logs on stdout

[Emanuele Giuseppe Esposito]

Using the flag -p, allow the qemu binary to print to stdout.
This is helpful especially for print-debugging.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 tests/qemu-iotests/check      | 3 ++-
 tests/qemu-iotests/iotests.py | 9 +++++++++
 tests/qemu-iotests/testenv.py | 9 +++++++--
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 6186495eee..6c469c14ff 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -33,6 +33,7 @@ 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('-p', dest='print', action='store_true', help='enable prints')
     p.add_argument('-gdb', action='store_true',
                    help="start gdbserver with $GDB_QEMU options. \
                          Default is localhost:12345")
@@ -116,7 +117,7 @@ if __name__ == '__main__':
                   aiomode=args.aiomode, cachemode=args.cachemode,
                   imgopts=args.imgopts, misalign=args.misalign,
                   debug=args.debug, valgrind=args.valgrind,
-                  gdb=args.gdb)
+                  gdb=args.gdb, qprint=args.print)
 
     testfinder = TestFinder(test_dir=env.source_iotests)
 
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index b23bfdfdff..bb29074b63 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -79,6 +79,8 @@
 if os.environ.get('GDB_QEMU'):
     qemu_gdb = ['gdbserver'] + os.environ.get('GDB_QEMU').strip().split(' ')
 
+qemu_print = os.environ.get('PRINT_QEMU', False)
+
 imgfmt = os.environ.get('IMGFMT', 'raw')
 imgproto = os.environ.get('IMGPROTO', 'file')
 output_dir = os.environ.get('OUTPUT_DIR', '.')
@@ -618,6 +620,13 @@ def _post_shutdown(self) -> None:
         super()._post_shutdown()
         self.subprocess_check_valgrind(qemu_valgrind)
 
+    def _pre_launch(self) -> None:
+        super()._pre_launch()
+        if qemu_print and self._qemu_log_file != None:
+            # set QEMU binary output to stdout
+            self._qemu_log_file.close()
+            self._qemu_log_file = None
+
     def add_object(self, opts):
         self._args.append('-object')
         self._args.append(opts)
diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index 86798bf752..9192d7154b 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -73,7 +73,7 @@ class TestEnv(ContextManager['TestEnv']):
                      'AIOMODE', 'CACHEMODE', 'VALGRIND_QEMU',
                      'CACHEMODE_IS_DEFAULT', 'IMGFMT_GENERIC', 'IMGOPTSSYNTAX',
                      'IMGKEYSECRET', 'QEMU_DEFAULT_MACHINE', 'MALLOC_PERTURB_',
-                     'GDB_QEMU']
+                     'GDB_QEMU', 'PRINT_QEMU']
 
     def get_env(self) -> Dict[str, str]:
         env = {}
@@ -165,7 +165,8 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: str,
                  misalign: bool = False,
                  debug: bool = False,
                  valgrind: bool = False,
-                 gdb: bool = False) -> None:
+                 gdb: bool = False,
+                 qprint: bool = False) -> None:
         self.imgfmt = imgfmt
         self.imgproto = imgproto
         self.aiomode = aiomode
@@ -173,6 +174,9 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: str,
         self.misalign = misalign
         self.debug = debug
 
+        if qprint:
+            self.print_qemu = 'y'
+
         self.gdb_qemu = os.getenv('GDB_QEMU')
 
         if gdb and not self.gdb_qemu:
@@ -281,6 +285,7 @@ def print_env(self) -> None:
 SOCKET_SCM_HELPER -- {SOCKET_SCM_HELPER}
 GDB_QEMU      -- "{GDB_QEMU}"
 VALGRIND_QEMU     -- "{VALGRIND_QEMU}"
+PRINT_QEMU    --  "{PRINT_QEMU}"
 """
 
         args = collections.defaultdict(str, self.get_env())
-- 
2.30.2

Re: [RFC PATCH v2 00/11] qemu_iotests: improve debugging options

[Markus Armbruster]

Emanuele Giuseppe Esposito <eesposit@redhat.com> writes:

> This series adds the option to attach gdbserver and valgrind
> to the QEMU binary running in qemu_iotests.
> It also allows to redirect QEMU binaries output of the python tests
> to the stdout, instead of a log file.
>
> Patches 1-6 introduce the -gdb option to both python and bash tests, 
> 7-10 extend the already existing -valgrind flag to work also on 
> python tests, and patch 11 introduces -p to enable logging to stdout.
>
> In particular, patches 1,2,4,8 focus on extending the QMP socket timers
> when using gdb/valgrind, otherwise the python tests will fail due to
> delays in the QMP responses.
>
> This series is tested on the previous serie
> "qemu-iotests: quality of life improvements"
> but independent from it, so it can be applied separately.
>
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>

How discoverable are these goodies for developers with only superficial
knowledge of iotests?

Re: [RFC PATCH v2 00/11] qemu_iotests: improve debugging options

[Emanuele Giuseppe Esposito]

On 08/04/2021 10:26, Markus Armbruster wrote:
> Emanuele Giuseppe Esposito <eesposit@redhat.com> writes:
> 
>> This series adds the option to attach gdbserver and valgrind
>> to the QEMU binary running in qemu_iotests.
>> It also allows to redirect QEMU binaries output of the python tests
>> to the stdout, instead of a log file.
>>
>> Patches 1-6 introduce the -gdb option to both python and bash tests,
>> 7-10 extend the already existing -valgrind flag to work also on
>> python tests, and patch 11 introduces -p to enable logging to stdout.
>>
>> In particular, patches 1,2,4,8 focus on extending the QMP socket timers
>> when using gdb/valgrind, otherwise the python tests will fail due to
>> delays in the QMP responses.
>>
>> This series is tested on the previous serie
>> "qemu-iotests: quality of life improvements"
>> but independent from it, so it can be applied separately.
>>
>> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> 
> How discoverable are these goodies for developers with only superficial
> knowledge of iotests?
> 

Not really sure what you mean, but

./check --help now shows:

> -p         enable prints
> -gdb       start gdbserver with $GDB_QEMU options. Default is localhost:12345

Which I guess should be clear enough? Btw two-three weeks ago I didn't 
know anything about these tests either.

I agree I can make -p more clear, saying "enable qemu binary prints to 
stdout", and move -valgrind to the "optional arguments" instead of 
"bash-only"

Emanuele

Re: [RFC PATCH v2 00/11] qemu_iotests: improve debugging options

[Markus Armbruster]

Emanuele Giuseppe Esposito <eesposit@redhat.com> writes:

> On 08/04/2021 10:26, Markus Armbruster wrote:
>> Emanuele Giuseppe Esposito <eesposit@redhat.com> writes:
>> 
>>> This series adds the option to attach gdbserver and valgrind
>>> to the QEMU binary running in qemu_iotests.
>>> It also allows to redirect QEMU binaries output of the python tests
>>> to the stdout, instead of a log file.
>>>
>>> Patches 1-6 introduce the -gdb option to both python and bash tests,
>>> 7-10 extend the already existing -valgrind flag to work also on
>>> python tests, and patch 11 introduces -p to enable logging to stdout.
>>>
>>> In particular, patches 1,2,4,8 focus on extending the QMP socket timers
>>> when using gdb/valgrind, otherwise the python tests will fail due to
>>> delays in the QMP responses.
>>>
>>> This series is tested on the previous serie
>>> "qemu-iotests: quality of life improvements"
>>> but independent from it, so it can be applied separately.
>>>
>>> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
>> 
>> How discoverable are these goodies for developers with only superficial
>> knowledge of iotests?
>> 
>
> Not really sure what you mean, but
>
> ./check --help now shows:
>
>> -p         enable prints
>> -gdb       start gdbserver with $GDB_QEMU options. Default is localhost:12345
>
> Which I guess should be clear enough? Btw two-three weeks ago I didn't 
> know anything about these tests either.
>
> I agree I can make -p more clear, saying "enable qemu binary prints to 
> stdout", and move -valgrind to the "optional arguments" instead of 
> "bash-only"

Yes, please (this is not a demand).

docs/devel/testing.rst section "QEMU iotests" is the place to explain
things in more detail than --help, if that's useful.  Right now it
simply points to check -h.

Re: [RFC PATCH v2 00/11] qemu_iotests: improve debugging options

[Emanuele Giuseppe Esposito]

On 08/04/2021 14:39, Markus Armbruster wrote:
> Emanuele Giuseppe Esposito <eesposit@redhat.com> writes:
> 
>> On 08/04/2021 10:26, Markus Armbruster wrote:
>>> Emanuele Giuseppe Esposito <eesposit@redhat.com> writes:
>>>
>>>> This series adds the option to attach gdbserver and valgrind
>>>> to the QEMU binary running in qemu_iotests.
>>>> It also allows to redirect QEMU binaries output of the python tests
>>>> to the stdout, instead of a log file.
>>>>
>>>> Patches 1-6 introduce the -gdb option to both python and bash tests,
>>>> 7-10 extend the already existing -valgrind flag to work also on
>>>> python tests, and patch 11 introduces -p to enable logging to stdout.
>>>>
>>>> In particular, patches 1,2,4,8 focus on extending the QMP socket timers
>>>> when using gdb/valgrind, otherwise the python tests will fail due to
>>>> delays in the QMP responses.
>>>>
>>>> This series is tested on the previous serie
>>>> "qemu-iotests: quality of life improvements"
>>>> but independent from it, so it can be applied separately.
>>>>
>>>> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
>>>
>>> How discoverable are these goodies for developers with only superficial
>>> knowledge of iotests?
>>>
>>
>> Not really sure what you mean, but
>>
>> ./check --help now shows:
>>
>>> -p         enable prints
>>> -gdb       start gdbserver with $GDB_QEMU options. Default is localhost:12345
>>
>> Which I guess should be clear enough? Btw two-three weeks ago I didn't
>> know anything about these tests either.
>>
>> I agree I can make -p more clear, saying "enable qemu binary prints to
>> stdout", and move -valgrind to the "optional arguments" instead of
>> "bash-only"
> 
> Yes, please (this is not a demand).
> 
> docs/devel/testing.rst section "QEMU iotests" is the place to explain
> things in more detail than --help, if that's useful.  Right now it
> simply points to check -h.
> 

Ok, I will add a new section in testing.rst explaining the new flags.

Thank you,
Emanuele