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

[Vladimir Sementsov-Ogievskiy]

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>


--
Best regards,
Vladimir