Re: [PATCH v6 06/16] qemu-iotests: delay QMP socket timers

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

21.06.2021 13:33, Emanuele Giuseppe Esposito wrote:
> Attaching gdbserver implies that the qmp socket
> should wait indefinitely for an answer from QEMU.
> 
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Please, let this patch go without my r-b. I don't like it, I explained my thoughts, and you even used my suggested way in previous version.

What I don't like: good component - Timeout class, which is currently independent of outer code and can be simply moved to another module becomes dependent on global variable, which doesn't relate to the class itself. Neither I like logic of the dependency which just make the whole class do nothing by skipping any action internally. So, IMHO Timeout class becomes worse.

I'm not a maintainer here anyway, so my r-b isn't necessary :)

> ---
>   tests/qemu-iotests/iotests.py | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index c86f239d81..e176a84620 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -477,10 +477,14 @@ def __init__(self, seconds, errmsg="Timeout"):
>           self.seconds = seconds
>           self.errmsg = errmsg
>       def __enter__(self):
> +        if qemu_gdb:
> +            return self
>           signal.signal(signal.SIGALRM, self.timeout)
>           signal.setitimer(signal.ITIMER_REAL, self.seconds)
>           return self
>       def __exit__(self, exc_type, value, traceback):
> +        if qemu_gdb:
> +            return False
>           signal.setitimer(signal.ITIMER_REAL, 0)
>           return False
>       def timeout(self, signum, frame):
> @@ -575,7 +579,7 @@ class VM(qtest.QEMUQtestMachine):
>   
>       def __init__(self, path_suffix=''):
>           name = "qemu%s-%d" % (path_suffix, os.getpid())
> -        timer = 15.0
> +        timer = 15.0 if not qemu_gdb else None
>           super().__init__(qemu_prog, qemu_opts, name=name,
>                            base_temp_dir=test_dir,
>                            socket_scm_helper=socket_scm_helper,
> 


-- 
Best regards,
Vladimir