Re: [PATCH 2/2] python/qemu/machine: accept QMP connection asynchronously

[Daniel P . Berrangé]

On Tue, Jun 28, 2022 at 05:49:39PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> QMP accept is currently synchronous. If qemu dies before the connection
> is established, it will wait there. Instead turn the code to do
> concurrently accept() and wait(). Returns when the first task is
> completed to determine whether a connection was established.

If the spawned QEMU process was given -daemonize, won't this code
mistakenly think the subprocess has quit ?

> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  python/qemu/machine/machine.py | 11 ++++++++++-
>  python/qemu/qmp/legacy.py      | 10 ++++++++++
>  2 files changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
> index 55c45f4b1205..5e2df7dc5055 100644
> --- a/python/qemu/machine/machine.py
> +++ b/python/qemu/machine/machine.py
> @@ -362,9 +362,18 @@ def _pre_launch(self) -> None:
>              self._args
>          ))
>  
> +    async def _async_accept(self) -> bool:
> +        accept = asyncio.create_task(self._qmp.async_accept())
> +        wait = asyncio.create_task(self._subproc.wait())
> +        done, pending = await asyncio.wait([accept, wait],
> +                                           
> return_when=asyncio.FIRST_COMPLETED)
> +        return accept in done
> +
>      def _post_launch(self) -> None:
>          if self._qmp_connection:
> -            self._qmp.accept(self._qmp_timer)
> +            accepted = self._sync(self._async_accept())
> +            if not accepted:
> +                raise QEMUMachineError('VM returned before QMP accept')
>  
>      def _close_qemu_log_file(self) -> None:
>          if self._qemu_log_file is not None:
> diff --git a/python/qemu/qmp/legacy.py b/python/qemu/qmp/legacy.py
> index 03b5574618fa..88bdbfb6e350 100644
> --- a/python/qemu/qmp/legacy.py
> +++ b/python/qemu/qmp/legacy.py
> @@ -167,6 +167,16 @@ def accept(self, timeout: Optional[float] = 15.0) -> 
> QMPMessage:
>          assert ret is not None
>          return ret
>  
> +    async def async_accept(self) -> QMPMessage:
> +        self._qmp.await_greeting = True
> +        self._qmp.negotiate = True
> +
> +        await self._qmp.accept()
> +
> +        ret = self._get_greeting()
> +        assert ret is not None
> +        return ret
> +


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|