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

[John Snow]

On 6/21/21 6:33 AM, Emanuele Giuseppe Esposito wrote:
> Also 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 the QMP monitor test command execution.
>
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

LGTM, thank you for your patience while I re-settled the Python 
subcomponent.

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

and, for patches 1-3; I am happy with these being staged by a block 
subsystem maintainer:

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

--js

> ---
>   python/qemu/machine/machine.py | 7 +++++--
>   python/qemu/machine/qtest.py   | 5 +++--
>   tests/qemu-iotests/iotests.py  | 3 ++-
>   3 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
> index b62435528e..fdf2fc0e9c 100644
> --- a/python/qemu/machine/machine.py
> +++ b/python/qemu/machine/machine.py
> @@ -95,7 +95,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
>   
> @@ -109,6 +110,7 @@ def __init__(self,
>           @param sock_dir: where to create socket (defaults to base_temp_dir)
>           @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
> @@ -116,6 +118,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._base_temp_dir = base_temp_dir
> @@ -333,7 +336,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/machine/qtest.py b/python/qemu/machine/qtest.py
> index 93700684d1..33a86a9d69 100644
> --- a/python/qemu/machine/qtest.py
> +++ b/python/qemu/machine/qtest.py
> @@ -115,14 +115,15 @@ def __init__(self,
>                    name: Optional[str] = None,
>                    base_temp_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 = base_temp_dir
>           super().__init__(binary, args, name=name, base_temp_dir=base_temp_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 89663dac06..6b0db4ce54 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -570,10 +570,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,
>                            base_temp_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):