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

marcandre.lureau@redhat.com posted 3 patches 3 years, 7 months ago
Maintainers: John Snow <jsnow@redhat.com>, Cleber Rosa <crosa@redhat.com>, Beraldo Leal <bleal@redhat.com>
There is a newer version of this series
[PATCH v2 3/3] python/qemu/machine: use socketpair() for QMP by default
Posted by marcandre.lureau@redhat.com 3 years, 7 months ago
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
 
         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


Re: [PATCH v2 3/3] python/qemu/machine: use socketpair() for QMP by default
Posted by Daniel P. Berrangé 3 years, 6 months ago
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 :|