Re: [PATCH 3/4] python/machine: use socketpair() for console connections

2023-07-20 Thread Daniel P . Berrangé
On Thu, Jul 20, 2023 at 09:04:47AM -0400, John Snow wrote:
> Create a socketpair for the console output. This should help eliminate
> race conditions around console text early in the boot process that might
> otherwise have been dropped on the floor before being able to connect to
> QEMU under "server,nowait".
> 
> Signed-off-by: John Snow 
> ---
>  python/qemu/machine/machine.py | 17 ++---
>  1 file changed, 14 insertions(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrangé 

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




[PATCH 3/4] python/machine: use socketpair() for console connections

2023-07-20 Thread John Snow
Create a socketpair for the console output. This should help eliminate
race conditions around console text early in the boot process that might
otherwise have been dropped on the floor before being able to connect to
QEMU under "server,nowait".

Signed-off-by: John Snow 
---
 python/qemu/machine/machine.py | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index 8be0f684fe..ef9b2dc02e 100644
--- a/python/qemu/machine/machine.py
+++ b/python/qemu/machine/machine.py
@@ -159,6 +159,8 @@ def __init__(self,
 
 self._name = name or f"{id(self):x}"
 self._sock_pair: Optional[Tuple[socket.socket, socket.socket]] = None
+self._cons_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
@@ -315,8 +317,9 @@ def _base_args(self) -> List[str]:
 for _ in range(self._console_index):
 args.extend(['-serial', 'null'])
 if self._console_set:
-chardev = ('socket,id=console,path=%s,server=on,wait=off' %
-   self._console_address)
+assert self._cons_sock_pair is not None
+fd = self._cons_sock_pair[0].fileno()
+chardev = f"socket,id=console,fd={fd}"
 args.extend(['-chardev', chardev])
 if self._console_device_type is None:
 args.extend(['-serial', 'chardev:console'])
@@ -351,6 +354,10 @@ def _pre_launch(self) -> None:
 nickname=self._name
 )
 
+if self._console_set:
+self._cons_sock_pair = socket.socketpair()
+os.set_inheritable(self._cons_sock_pair[0].fileno(), True)
+
 # NOTE: Make sure any opened resources are *definitely* freed in
 # _post_shutdown()!
 # pylint: disable=consider-using-with
@@ -873,8 +880,12 @@ def console_socket(self) -> socket.socket:
 Returns a socket connected to the console
 """
 if self._console_socket is None:
+if not self._console_set:
+raise QEMUMachineError(
+"Attempt to access console socket with no connection")
+assert self._cons_sock_pair is not None
 self._console_socket = console_socket.ConsoleSocket(
-self._console_address,
+self._cons_sock_pair[1].fileno(),
 file=self._console_log_path,
 drain=self._drain_console)
 return self._console_socket
-- 
2.41.0