diff mbox series

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

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

Commit Message

Emanuele Giuseppe Esposito May 20, 2021, 7:52 a.m. UTC
Alsp add a new _qmp_timer field to the QEMUMachine class.

Let's change the default socket timeout to None, so that if
a subclass needs to add a timer, it can be done by modifying
this private field.

At the same time, restore the timer to be 15 seconds in iotests.py, to
give an upper bound to qemu-iotests execution.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 python/qemu/machine.py        | 7 +++++--
 python/qemu/qtest.py          | 5 +++--
 tests/qemu-iotests/iotests.py | 3 ++-
 3 files changed, 10 insertions(+), 5 deletions(-)

Comments

Vladimir Sementsov-Ogievskiy May 26, 2021, 11:13 a.m. UTC | #1
20.05.2021 10:52, Emanuele Giuseppe Esposito wrote:
> Alsp add a new _qmp_timer field to the QEMUMachine class.
> 
> Let's change the default socket timeout to None, so that if
> a subclass needs to add a timer, it can be done by modifying
> this private field.
> 
> At the same time, restore the timer to be 15 seconds in iotests.py, to
> give an upper bound to qemu-iotests execution.

Hmm. Not to qemu-iotests execution, but only to connection to qmp monitor, isn't it?

> 
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> ---
>   python/qemu/machine.py        | 7 +++++--
>   python/qemu/qtest.py          | 5 +++--
>   tests/qemu-iotests/iotests.py | 3 ++-
>   3 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
> index 6e44bda337..df32de4377 100644
> --- a/python/qemu/machine.py
> +++ b/python/qemu/machine.py
> @@ -89,7 +89,8 @@ def __init__(self,
>                    socket_scm_helper: Optional[str] = None,
>                    sock_dir: Optional[str] = None,
>                    drain_console: bool = False,
> -                 console_log: Optional[str] = None):
> +                 console_log: Optional[str] = None,
> +                 qmp_timer: Optional[float] = None):
>           '''
>           Initialize a QEMUMachine
>   
> @@ -103,6 +104,7 @@ def __init__(self,
>           @param sock_dir: where to create socket (overrides test_dir for sock)
>           @param drain_console: (optional) True to drain console socket to buffer
>           @param console_log: (optional) path to console log file
> +        @param qmp_timer: (optional) default QMP socket timeout
>           @note: Qemu process is not started until launch() is used.
>           '''
>           # Direct user configuration
> @@ -110,6 +112,7 @@ def __init__(self,
>           self._binary = binary
>           self._args = list(args)
>           self._wrapper = wrapper
> +        self._qmp_timer = qmp_timer
>   
>           self._name = name or "qemu-%d" % os.getpid()
>           self._test_dir = test_dir
> @@ -323,7 +326,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 --git a/python/qemu/qtest.py b/python/qemu/qtest.py
> index 39a0cf62fe..afea210d9d 100644
> --- a/python/qemu/qtest.py
> +++ b/python/qemu/qtest.py
> @@ -114,14 +114,15 @@ def __init__(self,
>                    name: Optional[str] = None,
>                    test_dir: str = "/var/tmp",
>                    socket_scm_helper: Optional[str] = None,
> -                 sock_dir: Optional[str] = None):
> +                 sock_dir: Optional[str] = None,
> +                 qmp_timer: Optional[float] = None):
>           if name is None:
>               name = "qemu-%d" % os.getpid()
>           if sock_dir is None:
>               sock_dir = test_dir
>           super().__init__(binary, args, name=name, test_dir=test_dir,
>                            socket_scm_helper=socket_scm_helper,
> -                         sock_dir=sock_dir)
> +                         sock_dir=sock_dir, qmp_timer=qmp_timer)
>           self._qtest: Optional[QEMUQtestProtocol] = None
>           self._qtest_path = os.path.join(sock_dir, name + "-qtest.sock")
>   
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index ec3c69daf1..5d78de0f0b 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -571,10 +571,11 @@ class VM(qtest.QEMUQtestMachine):
>   
>       def __init__(self, path_suffix=''):
>           name = "qemu%s-%d" % (path_suffix, os.getpid())
> +        timer = 15.0
>           super().__init__(qemu_prog, qemu_opts, name=name,
>                            test_dir=test_dir,
>                            socket_scm_helper=socket_scm_helper,
> -                         sock_dir=sock_dir)
> +                         sock_dir=sock_dir, qmp_timer=timer)
>           self._num_drives = 0
>   
>       def add_object(self, opts):
> 

I'd call it qmp_accept_timeout to be precise. Anyway:

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
John Snow June 2, 2021, 11:23 p.m. UTC | #2
On 5/20/21 3:52 AM, Emanuele Giuseppe Esposito wrote:
> Alsp add a new _qmp_timer field to the QEMUMachine class.
> 
> Let's change the default socket timeout to None, so that if
> a subclass needs to add a timer, it can be done by modifying
> this private field.
> 
> At the same time, restore the timer to be 15 seconds in iotests.py, to
> give an upper bound to qemu-iotests execution.
> 
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>

Hi Emanuele: I'm sorry, but with the recent Python PR this no longer 
applies to origin/master -- the python files got shuffled around a bit 
when I added the new CI tests.

May I please ask you to rebase? You don't have to re-spin just yet, just 
pointing me to the rebase would help me out.

(Also: if you push to gitlab, you can take advantage of the new Python 
CI tests!)

Apologies,
--js
Emanuele Giuseppe Esposito June 3, 2021, 8:06 a.m. UTC | #3
On 03/06/2021 01:23, John Snow wrote:
> On 5/20/21 3:52 AM, Emanuele Giuseppe Esposito wrote:
>> Alsp add a new _qmp_timer field to the QEMUMachine class.
>>
>> Let's change the default socket timeout to None, so that if
>> a subclass needs to add a timer, it can be done by modifying
>> this private field.
>>
>> At the same time, restore the timer to be 15 seconds in iotests.py, to
>> give an upper bound to qemu-iotests execution.
>>
>> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> 
> Hi Emanuele: I'm sorry, but with the recent Python PR this no longer 
> applies to origin/master -- the python files got shuffled around a bit 
> when I added the new CI tests.
> 
> May I please ask you to rebase? You don't have to re-spin just yet, just 
> pointing me to the rebase would help me out.

Hi John, no problem. I rebased here:
https://gitlab.com/eesposit/qemu/-/commits/qemu_iotests_io

Let me know if I need to do anything else.
I will re-spin later today.

Thank you,
Emanuele
John Snow June 3, 2021, 7:02 p.m. UTC | #4
On 6/3/21 4:06 AM, Emanuele Giuseppe Esposito wrote:
> 
> 
> On 03/06/2021 01:23, John Snow wrote:
>> On 5/20/21 3:52 AM, Emanuele Giuseppe Esposito wrote:
>>> Alsp add a new _qmp_timer field to the QEMUMachine class.
>>>
>>> Let's change the default socket timeout to None, so that if
>>> a subclass needs to add a timer, it can be done by modifying
>>> this private field.
>>>
>>> At the same time, restore the timer to be 15 seconds in iotests.py, to
>>> give an upper bound to qemu-iotests execution.
>>>
>>> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
>>
>> Hi Emanuele: I'm sorry, but with the recent Python PR this no longer 
>> applies to origin/master -- the python files got shuffled around a bit 
>> when I added the new CI tests.
>>
>> May I please ask you to rebase? You don't have to re-spin just yet, 
>> just pointing me to the rebase would help me out.
> 
> Hi John, no problem. I rebased here:
> https://gitlab.com/eesposit/qemu/-/commits/qemu_iotests_io
> 
> Let me know if I need to do anything else.
> I will re-spin later today.
> 
> Thank you,
> Emanuele
> 

Hi Emanuele:

https://gitlab.com/jsnow/qemu/-/commits/review

I added in a pylint ignore for you and these patches are clean now.

--js
Emanuele Giuseppe Esposito June 4, 2021, 8:48 a.m. UTC | #5
On 03/06/2021 21:02, John Snow wrote:
> On 6/3/21 4:06 AM, Emanuele Giuseppe Esposito wrote:
>>
>>
>> On 03/06/2021 01:23, John Snow wrote:
>>> On 5/20/21 3:52 AM, Emanuele Giuseppe Esposito wrote:
>>>> Alsp add a new _qmp_timer field to the QEMUMachine class.
>>>>
>>>> Let's change the default socket timeout to None, so that if
>>>> a subclass needs to add a timer, it can be done by modifying
>>>> this private field.
>>>>
>>>> At the same time, restore the timer to be 15 seconds in iotests.py, to
>>>> give an upper bound to qemu-iotests execution.
>>>>
>>>> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
>>>
>>> Hi Emanuele: I'm sorry, but with the recent Python PR this no longer 
>>> applies to origin/master -- the python files got shuffled around a 
>>> bit when I added the new CI tests.
>>>
>>> May I please ask you to rebase? You don't have to re-spin just yet, 
>>> just pointing me to the rebase would help me out.
>>
>> Hi John, no problem. I rebased here:
>> https://gitlab.com/eesposit/qemu/-/commits/qemu_iotests_io
>>
>> Let me know if I need to do anything else.
>> I will re-spin later today.
>>
>> Thank you,
>> Emanuele
>>
> 
> Hi Emanuele:
> 
> https://gitlab.com/jsnow/qemu/-/commits/review
> 
> I added in a pylint ignore for you and these patches are clean now.

Thank you! I will add it to my series.

Emanuele
diff mbox series

Patch

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 6e44bda337..df32de4377 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -89,7 +89,8 @@  def __init__(self,
                  socket_scm_helper: Optional[str] = None,
                  sock_dir: Optional[str] = None,
                  drain_console: bool = False,
-                 console_log: Optional[str] = None):
+                 console_log: Optional[str] = None,
+                 qmp_timer: Optional[float] = None):
         '''
         Initialize a QEMUMachine
 
@@ -103,6 +104,7 @@  def __init__(self,
         @param sock_dir: where to create socket (overrides test_dir for sock)
         @param drain_console: (optional) True to drain console socket to buffer
         @param console_log: (optional) path to console log file
+        @param qmp_timer: (optional) default QMP socket timeout
         @note: Qemu process is not started until launch() is used.
         '''
         # Direct user configuration
@@ -110,6 +112,7 @@  def __init__(self,
         self._binary = binary
         self._args = list(args)
         self._wrapper = wrapper
+        self._qmp_timer = qmp_timer
 
         self._name = name or "qemu-%d" % os.getpid()
         self._test_dir = test_dir
@@ -323,7 +326,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 --git a/python/qemu/qtest.py b/python/qemu/qtest.py
index 39a0cf62fe..afea210d9d 100644
--- a/python/qemu/qtest.py
+++ b/python/qemu/qtest.py
@@ -114,14 +114,15 @@  def __init__(self,
                  name: Optional[str] = None,
                  test_dir: str = "/var/tmp",
                  socket_scm_helper: Optional[str] = None,
-                 sock_dir: Optional[str] = None):
+                 sock_dir: Optional[str] = None,
+                 qmp_timer: Optional[float] = None):
         if name is None:
             name = "qemu-%d" % os.getpid()
         if sock_dir is None:
             sock_dir = test_dir
         super().__init__(binary, args, name=name, test_dir=test_dir,
                          socket_scm_helper=socket_scm_helper,
-                         sock_dir=sock_dir)
+                         sock_dir=sock_dir, qmp_timer=qmp_timer)
         self._qtest: Optional[QEMUQtestProtocol] = None
         self._qtest_path = os.path.join(sock_dir, name + "-qtest.sock")
 
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index ec3c69daf1..5d78de0f0b 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -571,10 +571,11 @@  class VM(qtest.QEMUQtestMachine):
 
     def __init__(self, path_suffix=''):
         name = "qemu%s-%d" % (path_suffix, os.getpid())
+        timer = 15.0
         super().__init__(qemu_prog, qemu_opts, name=name,
                          test_dir=test_dir,
                          socket_scm_helper=socket_scm_helper,
-                         sock_dir=sock_dir)
+                         sock_dir=sock_dir, qmp_timer=timer)
         self._num_drives = 0
 
     def add_object(self, opts):