diff mbox series

[v5,06/16] qemu-iotests: delay QMP socket timers

Message ID 20210604091723.13419-7-eesposit@redhat.com
State New
Headers show
Series qemu_iotests: improve debugging options | expand

Commit Message

Emanuele Giuseppe Esposito June 4, 2021, 9:17 a.m. UTC
Attaching gdbserver implies that the qmp socket
should wait indefinitely for an answer from QEMU.

For Timeout class, create a @contextmanager that
switches Timeout with NoTimeout (empty context manager)
so that if --gdb is set, no timeout will be triggered.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/iotests.py | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Emanuele Giuseppe Esposito June 14, 2021, 10:36 a.m. UTC | #1
On 04/06/2021 11:17, Emanuele Giuseppe Esposito wrote:
> Attaching gdbserver implies that the qmp socket
> should wait indefinitely for an answer from QEMU.
> 
> For Timeout class, create a @contextmanager that
> switches Timeout with NoTimeout (empty context manager)
> so that if --gdb is set, no timeout will be triggered.
> 
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   tests/qemu-iotests/iotests.py | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index c86f239d81..d4bfd8f1d6 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -486,6 +486,13 @@ def __exit__(self, exc_type, value, traceback):
>       def timeout(self, signum, frame):
>           raise Exception(self.errmsg)
>   
> +@contextmanager
> +def NoTimeout():
> +    yield
> +
> +if qemu_gdb:
> +    Timeout = NoTimeout
> +

@Vladimir or anyone expert enough in python:
This fix above works, but I just noticed that makes pylint (test 297) 
fail. My bad, I should have tried it before.

The reason for that is
>> +    Timeout = NoTimeout

They obviously have different types.

> +iotests.py:507: error: Cannot assign to a type
> +iotests.py:507: error: Incompatible types in assignment (expression has type "Callable[[], _GeneratorContextManager[Any]]", variable has type "Type[Timeout]")
> +Found 2 errors in 1 file (checked 1 source file)

Any ideas on how to fix this? Otherwise I will get rid of it.

Thank you,
Emanuele

>   def file_pattern(name):
>       return "{0}-{1}".format(os.getpid(), name)
>   
> @@ -575,7 +582,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,
>
Vladimir Sementsov-Ogievskiy June 15, 2021, 7:57 a.m. UTC | #2
14.06.2021 13:36, Emanuele Giuseppe Esposito wrote:
> 
> 
> On 04/06/2021 11:17, Emanuele Giuseppe Esposito wrote:
>> Attaching gdbserver implies that the qmp socket
>> should wait indefinitely for an answer from QEMU.
>>
>> For Timeout class, create a @contextmanager that
>> switches Timeout with NoTimeout (empty context manager)
>> so that if --gdb is set, no timeout will be triggered.
>>
>> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
>> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   tests/qemu-iotests/iotests.py | 9 ++++++++-
>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>> index c86f239d81..d4bfd8f1d6 100644
>> --- a/tests/qemu-iotests/iotests.py
>> +++ b/tests/qemu-iotests/iotests.py
>> @@ -486,6 +486,13 @@ def __exit__(self, exc_type, value, traceback):
>>       def timeout(self, signum, frame):
>>           raise Exception(self.errmsg)
>> +@contextmanager
>> +def NoTimeout():
>> +    yield
>> +
>> +if qemu_gdb:
>> +    Timeout = NoTimeout
>> +
> 
> @Vladimir or anyone expert enough in python:
> This fix above works, but I just noticed that makes pylint (test 297) fail. My bad, I should have tried it before.

I think, just make mypy ignore it, like:

    Timeout = NoTimeout # type: ignore


> 
> The reason for that is
>>> +    Timeout = NoTimeout
> 
> They obviously have different types.
> 
>> +iotests.py:507: error: Cannot assign to a type
>> +iotests.py:507: error: Incompatible types in assignment (expression has type "Callable[[], _GeneratorContextManager[Any]]", variable has type "Type[Timeout]")
>> +Found 2 errors in 1 file (checked 1 source file)
> 
> Any ideas on how to fix this? Otherwise I will get rid of it.
> 
> Thank you,
> Emanuele
> 
>>   def file_pattern(name):
>>       return "{0}-{1}".format(os.getpid(), name)
>> @@ -575,7 +582,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,
>>
>
Emanuele Giuseppe Esposito June 16, 2021, 7:09 a.m. UTC | #3
On 15/06/2021 09:57, Vladimir Sementsov-Ogievskiy wrote:
> 14.06.2021 13:36, Emanuele Giuseppe Esposito wrote:
>>
>>
>> On 04/06/2021 11:17, Emanuele Giuseppe Esposito wrote:
>>> Attaching gdbserver implies that the qmp socket
>>> should wait indefinitely for an answer from QEMU.
>>>
>>> For Timeout class, create a @contextmanager that
>>> switches Timeout with NoTimeout (empty context manager)
>>> so that if --gdb is set, no timeout will be triggered.
>>>
>>> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
>>> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> ---
>>>   tests/qemu-iotests/iotests.py | 9 ++++++++-
>>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tests/qemu-iotests/iotests.py 
>>> b/tests/qemu-iotests/iotests.py
>>> index c86f239d81..d4bfd8f1d6 100644
>>> --- a/tests/qemu-iotests/iotests.py
>>> +++ b/tests/qemu-iotests/iotests.py
>>> @@ -486,6 +486,13 @@ def __exit__(self, exc_type, value, traceback):
>>>       def timeout(self, signum, frame):
>>>           raise Exception(self.errmsg)
>>> +@contextmanager
>>> +def NoTimeout():
>>> +    yield
>>> +
>>> +if qemu_gdb:
>>> +    Timeout = NoTimeout
>>> +
>>
>> @Vladimir or anyone expert enough in python:
>> This fix above works, but I just noticed that makes pylint (test 297) 
>> fail. My bad, I should have tried it before.
> 
> I think, just make mypy ignore it, like:
> 
>     Timeout = NoTimeout # type: ignore
> 
> 

I think I am going to drop this change.
This series already has patch 2 to ignore another pylint warning, plus 
another separate patch was sent to silence a warning that pops out with 
newer versions of pylint.

So once this gets in, feel free to add a patch with this change.

Emanuele
>>
>> The reason for that is
>>>> +    Timeout = NoTimeout
>>
>> They obviously have different types.
>>
>>> +iotests.py:507: error: Cannot assign to a type
>>> +iotests.py:507: error: Incompatible types in assignment (expression 
>>> has type "Callable[[], _GeneratorContextManager[Any]]", variable has 
>>> type "Type[Timeout]")
>>> +Found 2 errors in 1 file (checked 1 source file)
>>
>> Any ideas on how to fix this? Otherwise I will get rid of it.
>>
>> Thank you,
>> Emanuele
>>
>>>   def file_pattern(name):
>>>       return "{0}-{1}".format(os.getpid(), name)
>>> @@ -575,7 +582,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,
>>>
>>
> 
>
Vladimir Sementsov-Ogievskiy June 16, 2021, 9:39 a.m. UTC | #4
16.06.2021 10:09, Emanuele Giuseppe Esposito wrote:
> 
> 
> On 15/06/2021 09:57, Vladimir Sementsov-Ogievskiy wrote:
>> 14.06.2021 13:36, Emanuele Giuseppe Esposito wrote:
>>>
>>>
>>> On 04/06/2021 11:17, Emanuele Giuseppe Esposito wrote:
>>>> Attaching gdbserver implies that the qmp socket
>>>> should wait indefinitely for an answer from QEMU.
>>>>
>>>> For Timeout class, create a @contextmanager that
>>>> switches Timeout with NoTimeout (empty context manager)
>>>> so that if --gdb is set, no timeout will be triggered.
>>>>
>>>> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
>>>> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>>> ---
>>>>   tests/qemu-iotests/iotests.py | 9 ++++++++-
>>>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>>>> index c86f239d81..d4bfd8f1d6 100644
>>>> --- a/tests/qemu-iotests/iotests.py
>>>> +++ b/tests/qemu-iotests/iotests.py
>>>> @@ -486,6 +486,13 @@ def __exit__(self, exc_type, value, traceback):
>>>>       def timeout(self, signum, frame):
>>>>           raise Exception(self.errmsg)
>>>> +@contextmanager
>>>> +def NoTimeout():
>>>> +    yield
>>>> +
>>>> +if qemu_gdb:
>>>> +    Timeout = NoTimeout
>>>> +
>>>
>>> @Vladimir or anyone expert enough in python:
>>> This fix above works, but I just noticed that makes pylint (test 297) fail. My bad, I should have tried it before.
>>
>> I think, just make mypy ignore it, like:
>>
>>     Timeout = NoTimeout # type: ignore
>>
>>
> 
> I think I am going to drop this change.
> This series already has patch 2 to ignore another pylint warning, plus another separate patch was sent to silence a warning that pops out with newer versions of pylint.

pylint complains should not be a reason for bad pattern. Shadowing the whole class is not good too, but it's at least a separate small hack, instead of silencing the whole logic of existing well-defined class internally with help of global variable.
diff mbox series

Patch

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index c86f239d81..d4bfd8f1d6 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -486,6 +486,13 @@  def __exit__(self, exc_type, value, traceback):
     def timeout(self, signum, frame):
         raise Exception(self.errmsg)
 
+@contextmanager
+def NoTimeout():
+    yield
+
+if qemu_gdb:
+    Timeout = NoTimeout
+
 def file_pattern(name):
     return "{0}-{1}".format(os.getpid(), name)
 
@@ -575,7 +582,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,