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

Emanuele Giuseppe Esposito posted 11 patches 4 years, 10 months ago
Maintainers: Kevin Wolf <kwolf@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>, Max Reitz <mreitz@redhat.com>, Cleber Rosa <crosa@redhat.com>, John Snow <jsnow@redhat.com>
There is a newer version of this series
[RFC PATCH v2 01/11] python: qemu: add timer parameter for qmp.accept socket
Posted by Emanuele Giuseppe Esposito 4 years, 10 months ago
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
Posted by John Snow 4 years, 10 months ago
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
Posted by Emanuele Giuseppe Esposito 4 years, 10 months ago
>> --- 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