Re: [PATCH v4 05/15] qemu-iotests: delay QMP socket timers - Vladimir Sementsov-Ogievskiy

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
To: Emanuele Giuseppe Esposito <eesposit@redhat.com>, qemu-block@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>,
	Eduardo Habkost <ehabkost@redhat.com>,
	qemu-devel@nongnu.org, Max Reitz <mreitz@redhat.com>,
	Cleber Rosa <crosa@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>, John Snow <jsnow@redhat.com>
Subject: Re: [PATCH v4 05/15] qemu-iotests: delay QMP socket timers
Date: Fri, 28 May 2021 20:06:43 +0300	[thread overview]
Message-ID: <5bde3128-e972-3653-308b-4faa4eb52bb6@virtuozzo.com> (raw)
In-Reply-To: <20210520075236.44723-6-eesposit@redhat.com>

20.05.2021 10:52, 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>
> ---
>   tests/qemu-iotests/iotests.py | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index d667fde6f8..cf1ca60376 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -478,11 +478,13 @@ def __init__(self, seconds, errmsg="Timeout"):
>           self.seconds = seconds
>           self.errmsg = errmsg
>       def __enter__(self):
> -        signal.signal(signal.SIGALRM, self.timeout)
> -        signal.setitimer(signal.ITIMER_REAL, self.seconds)
> +        if not qemu_gdb:
> +            signal.signal(signal.SIGALRM, self.timeout)
> +            signal.setitimer(signal.ITIMER_REAL, self.seconds)
>           return self
>       def __exit__(self, exc_type, value, traceback):
> -        signal.setitimer(signal.ITIMER_REAL, 0)
> +        if not qemu_gdb:
> +            signal.setitimer(signal.ITIMER_REAL, 0)
>           return False
>       def timeout(self, signum, frame):
>           raise Exception(self.errmsg)

So, you just make the class do nothing.. I'd prefer something like this:

@contextmanager
def NoTimeout:
    yield

if qemu_gdb:
   Timeout = NoTimeout

anyway:

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

> @@ -576,7 +578,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,
>                            test_dir=test_dir,
>                            socket_scm_helper=socket_scm_helper,
> 

Still, it's not simple to avoid any kind of timeouts. The most annoying would be timeouts in event_wait() / events_wait()

-- 
Best regards,
Vladimir