Re: [PATCH v2 3/3] python/qemu/machine: use socketpair() for QMP by default

[Daniel P . Berrangé]

On Thu, Jun 30, 2022 at 04:34:19PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> When no monitor address is given, establish the QMP communication through
> a socketpair() (API is also supported on Windows since Python 3.5)
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  python/qemu/machine/machine.py | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
> index 37191f433b2d..aa1d9447352d 100644
> --- a/python/qemu/machine/machine.py
> +++ b/python/qemu/machine/machine.py
> @@ -158,17 +158,13 @@ def __init__(self,
>          self._qmp_timer = qmp_timer
>  
>          self._name = name or f"qemu-{os.getpid()}-{id(self):02x}"
> +        self._sock_pair: Optional[Tuple[socket.socket, socket.socket]] = None
>          self._temp_dir: Optional[str] = None
>          self._base_temp_dir = base_temp_dir
>          self._sock_dir = sock_dir
>          self._log_dir = log_dir
>  
> -        if monitor_address is not None:
> -            self._monitor_address = monitor_address
> -        else:
> -            self._monitor_address = os.path.join(
> -                self.sock_dir, f"{self._name}-monitor.sock"
> -            )
> +        self._monitor_address = monitor_address

Almost nothing in QEMU passes 'monitor_address' right now, so thue effect
of this will be that essentially all usage switches to the socketpair
behaviour. Should be ok, as nothing is expecting to have the ability to
leave QEMU running, and re-connect to its monitor in another process
later.

>  
>          self._console_log_path = console_log
>          if self._console_log_path:
> @@ -303,7 +299,11 @@ def _base_args(self) -> List[str]:
>          args = ['-display', 'none', '-vga', 'none']
>  
>          if self._qmp_set:
> -            if isinstance(self._monitor_address, tuple):
> +            if self._sock_pair:
> +                fd = self._sock_pair[0].fileno()
> +                os.set_inheritable(fd, True)
> +                moncdev = f"socket,id=mon,fd={fd}"
> +            elif isinstance(self._monitor_address, tuple):
>                  moncdev = "socket,id=mon,host={},port={}".format(
>                      *self._monitor_address
>                  )
> @@ -337,10 +337,17 @@ def _pre_launch(self) -> None:
>              self._remove_files.append(self._console_address)
>  
>          if self._qmp_set:
> +            monitor_address = None
> +            sock = None
> +            if self._monitor_address is None:
> +                self._sock_pair = socket.socketpair()
> +                sock = self._sock_pair[1]
>              if isinstance(self._monitor_address, str):
>                  self._remove_files.append(self._monitor_address)
> +                monitor_address = self._monitor_address
>              self._qmp_connection = QEMUMonitorProtocol(
> -                self._monitor_address,
> +                address=monitor_address,
> +                sock=sock,
>                  server=True,
>                  nickname=self._name
>              )
> @@ -360,6 +367,7 @@ def _pre_launch(self) -> None:
>          ))
>  
>      def _post_launch(self) -> None:
> +        self._sock_pair[0].close()
>          if self._qmp_connection:
>              self._qmp.accept(self._qmp_timer)
>  
> -- 
> 2.37.0.rc0
> 

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 :|