| Message ID | 20220630123419.1019367-4-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | python/qemu/machine: fix potential hang in QMP accept | expand |
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
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)