diff mbox series

[v3,01/15] python: qemu: add timer parameter for qmp.accept socket

Message ID 20210414170352.29927-2-eesposit@redhat.com (mailing list archive)
State New, archived
Headers show
Series qemu_iotests: improve debugging options | expand

Commit Message

Emanuele Giuseppe Esposito April 14, 2021, 5:03 p.m. UTC
Add a new _qmp_timer field to the QEMUMachine class.
The default timer is 15 sec, as per the default in the
qmp accept() function.

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

Comments

Max Reitz April 30, 2021, 11:23 a.m. UTC | #1
On 14.04.21 19:03, Emanuele Giuseppe Esposito wrote:
> Add a new _qmp_timer field to the QEMUMachine class.
> The default timer is 15 sec, as per the default in the
> qmp accept() function.
> 
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> ---
>   python/qemu/machine.py | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>

(I would have preferred for the commit message to tell me why we want 
this (I suspect some later patch is going to use it), but oh well.)
John Snow May 13, 2021, 5:54 p.m. UTC | #2
On 4/14/21 1:03 PM, Emanuele Giuseppe Esposito wrote:
> Add a new _qmp_timer field to the QEMUMachine class.
> The default timer is 15 sec, as per the default in the
> qmp accept() function.

Fine enough for now.

What's the exact need for this change, exactly? Is it just to prevent 
any unbounded blocking waits? I assume this came up in development or 
you'd not have added it.

Reviewed-by: John Snow <jsnow@redhat.com>

> 
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> ---
>   python/qemu/machine.py | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
> index 6e44bda337..12752142c9 100644
> --- a/python/qemu/machine.py
> +++ b/python/qemu/machine.py
> @@ -110,6 +110,7 @@ def __init__(self,
>           self._binary = binary
>           self._args = list(args)
>           self._wrapper = wrapper
> +        self._qmp_timer = 15.0
>   
>           self._name = name or "qemu-%d" % os.getpid()
>           self._test_dir = test_dir
> @@ -323,7 +324,7 @@ def _pre_launch(self) -> None:
>   
>       def _post_launch(self) -> None:
>           if self._qmp_connection:
> -            self._qmp.accept()
> +            self._qmp.accept(self._qmp_timer)
>   
>       def _post_shutdown(self) -> None:
>           """
>
Emanuele Giuseppe Esposito May 14, 2021, 8:16 a.m. UTC | #3
On 13/05/2021 19:54, John Snow wrote:
> On 4/14/21 1:03 PM, Emanuele Giuseppe Esposito wrote:
>> Add a new _qmp_timer field to the QEMUMachine class.
>> The default timer is 15 sec, as per the default in the
>> qmp accept() function.
> 
> Fine enough for now.
> 
> What's the exact need for this change, exactly? Is it just to prevent 
> any unbounded blocking waits? I assume this came up in development or 
> you'd not have added it.

The default was already 15 sec, but since we now want to make the wait 
unbounded if we use gdbserver or valgrind and we do it by always passing 
the _qmp_timer to the .accept function, we need to set a default value.
Yes, it came up while testing with gdb.

> 
> Reviewed-by: John Snow <jsnow@redhat.com>
> 
>>
>> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
>> ---
>>   python/qemu/machine.py | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
>> index 6e44bda337..12752142c9 100644
>> --- a/python/qemu/machine.py
>> +++ b/python/qemu/machine.py
>> @@ -110,6 +110,7 @@ def __init__(self,
>>           self._binary = binary
>>           self._args = list(args)
>>           self._wrapper = wrapper
>> +        self._qmp_timer = 15.0
>>           self._name = name or "qemu-%d" % os.getpid()
>>           self._test_dir = test_dir
>> @@ -323,7 +324,7 @@ def _pre_launch(self) -> None:
>>       def _post_launch(self) -> None:
>>           if self._qmp_connection:
>> -            self._qmp.accept()
>> +            self._qmp.accept(self._qmp_timer)
>>       def _post_shutdown(self) -> None:
>>           """
>>
>
diff mbox series

Patch

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 6e44bda337..12752142c9 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -110,6 +110,7 @@  def __init__(self,
         self._binary = binary
         self._args = list(args)
         self._wrapper = wrapper
+        self._qmp_timer = 15.0
 
         self._name = name or "qemu-%d" % os.getpid()
         self._test_dir = test_dir
@@ -323,7 +324,7 @@  def _pre_launch(self) -> None:
 
     def _post_launch(self) -> None:
         if self._qmp_connection:
-            self._qmp.accept()
+            self._qmp.accept(self._qmp_timer)
 
     def _post_shutdown(self) -> None:
         """