* [PATCH v3 0/3] python/qemu/machine: fix potential hang in QMP accept
@ 2023-01-11 8:00 marcandre.lureau
2023-01-11 8:00 ` [PATCH v3 1/3] python/qmp/protocol: add open_with_socket() marcandre.lureau
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: marcandre.lureau @ 2023-01-11 8:00 UTC (permalink / raw)
To: qemu-devel
Cc: berrange, Beraldo Leal, John Snow, Cleber Rosa, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Hi,
As reported earlier by Richard Henderson ("virgl avocado hang" thread), avocado
tests may hang when QEMU exits before the QMP connection is established.
v3:
- after merge in https://gitlab.com/qemu-project/python-qemu-qmp
- resend as requested by John Snow
v2:
- use a socketpair() for QMP (instead of async concurrent code from v1) as
suggested by Daniel Berrange.
- should not regress (hopefully)
Marc-André Lureau (3):
python/qmp/protocol: add open_with_socket()
python/qmp/legacy: make QEMUMonitorProtocol accept a socket
python/qemu/machine: use socketpair() for QMP by default
python/qemu/machine/machine.py | 24 ++++++++++++++++--------
python/qemu/qmp/legacy.py | 18 +++++++++++++++---
python/qemu/qmp/protocol.py | 25 ++++++++++++++++++++-----
3 files changed, 51 insertions(+), 16 deletions(-)
--
2.39.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 1/3] python/qmp/protocol: add open_with_socket()
2023-01-11 8:00 [PATCH v3 0/3] python/qemu/machine: fix potential hang in QMP accept marcandre.lureau
@ 2023-01-11 8:00 ` marcandre.lureau
2023-01-11 8:01 ` [PATCH v3 2/3] python/qmp/legacy: make QEMUMonitorProtocol accept a socket marcandre.lureau
2023-01-11 8:01 ` [PATCH v3 3/3] python/qemu/machine: use socketpair() for QMP by default marcandre.lureau
2 siblings, 0 replies; 9+ messages in thread
From: marcandre.lureau @ 2023-01-11 8:00 UTC (permalink / raw)
To: qemu-devel
Cc: berrange, Beraldo Leal, John Snow, Cleber Rosa, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Instead of listening for incoming connections with a SocketAddr, add a
new method open_with_socket() that accepts an existing socket.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
python/qemu/qmp/protocol.py | 25 ++++++++++++++++++++-----
1 file changed, 20 insertions(+), 5 deletions(-)
diff --git a/python/qemu/qmp/protocol.py b/python/qemu/qmp/protocol.py
index 6ea86650ad..4710a57f91 100644
--- a/python/qemu/qmp/protocol.py
+++ b/python/qemu/qmp/protocol.py
@@ -18,6 +18,7 @@
from enum import Enum
from functools import wraps
import logging
+import socket
from ssl import SSLContext
from typing import (
Any,
@@ -296,6 +297,19 @@ async def start_server_and_accept(
await self.accept()
assert self.runstate == Runstate.RUNNING
+ @upper_half
+ @require(Runstate.IDLE)
+ async def open_with_socket(self, sock: socket.socket) -> None:
+ """
+ Start connection with given socket.
+
+ :param sock: A socket.
+
+ :raise StateError: When the `Runstate` is not `IDLE`.
+ """
+ self._reader, self._writer = await asyncio.open_connection(sock=sock)
+ self._set_state(Runstate.CONNECTING)
+
@upper_half
@require(Runstate.IDLE)
async def start_server(self, address: SocketAddrT,
@@ -343,11 +357,12 @@ async def accept(self) -> None:
protocol-level failure occurs while establishing a new
session, the wrapped error may also be an `QMPError`.
"""
- if self._accepted is None:
- raise QMPError("Cannot call accept() before start_server().")
- await self._session_guard(
- self._do_accept(),
- 'Failed to establish connection')
+ if not self._reader:
+ if self._accepted is None:
+ raise QMPError("Cannot call accept() before start_server().")
+ await self._session_guard(
+ self._do_accept(),
+ 'Failed to establish connection')
await self._session_guard(
self._establish_session(),
'Failed to establish session')
--
2.39.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 2/3] python/qmp/legacy: make QEMUMonitorProtocol accept a socket
2023-01-11 8:00 [PATCH v3 0/3] python/qemu/machine: fix potential hang in QMP accept marcandre.lureau
2023-01-11 8:00 ` [PATCH v3 1/3] python/qmp/protocol: add open_with_socket() marcandre.lureau
@ 2023-01-11 8:01 ` marcandre.lureau
2023-01-11 8:01 ` [PATCH v3 3/3] python/qemu/machine: use socketpair() for QMP by default marcandre.lureau
2 siblings, 0 replies; 9+ messages in thread
From: marcandre.lureau @ 2023-01-11 8:01 UTC (permalink / raw)
To: qemu-devel
Cc: berrange, Beraldo Leal, John Snow, Cleber Rosa, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Teach QEMUMonitorProtocol to accept an exisiting socket.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
python/qemu/qmp/legacy.py | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/python/qemu/qmp/legacy.py b/python/qemu/qmp/legacy.py
index 1951754455..8b09ee7dbb 100644
--- a/python/qemu/qmp/legacy.py
+++ b/python/qemu/qmp/legacy.py
@@ -22,6 +22,7 @@
#
import asyncio
+import socket
from types import TracebackType
from typing import (
Any,
@@ -69,22 +70,32 @@ class QEMUMonitorProtocol:
:param address: QEMU address, can be either a unix socket path (string)
or a tuple in the form ( address, port ) for a TCP
- connection
+ connection or None
+ :param sock: a socket or None
:param server: Act as the socket server. (See 'accept')
:param nickname: Optional nickname used for logging.
"""
- def __init__(self, address: SocketAddrT,
+ def __init__(self,
+ address: Optional[SocketAddrT] = None,
+ sock: Optional[socket.socket] = None,
server: bool = False,
nickname: Optional[str] = None):
+ assert address or sock
self._qmp = QMPClient(nickname)
self._aloop = asyncio.get_event_loop()
self._address = address
+ self._sock = sock
self._timeout: Optional[float] = None
if server:
- self._sync(self._qmp.start_server(self._address))
+ if sock:
+ assert self._sock is not None
+ self._sync(self._qmp.open_with_socket(self._sock))
+ else:
+ assert self._address is not None
+ self._sync(self._qmp.start_server(self._address))
_T = TypeVar('_T')
@@ -139,6 +150,7 @@ def connect(self, negotiate: bool = True) -> Optional[QMPMessage]:
:return: QMP greeting dict, or None if negotiate is false
:raise ConnectError: on connection errors
"""
+ assert self._address is not None
self._qmp.await_greeting = negotiate
self._qmp.negotiate = negotiate
--
2.39.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 3/3] python/qemu/machine: use socketpair() for QMP by default
2023-01-11 8:00 [PATCH v3 0/3] python/qemu/machine: fix potential hang in QMP accept marcandre.lureau
2023-01-11 8:00 ` [PATCH v3 1/3] python/qmp/protocol: add open_with_socket() marcandre.lureau
2023-01-11 8:01 ` [PATCH v3 2/3] python/qmp/legacy: make QEMUMonitorProtocol accept a socket marcandre.lureau
@ 2023-01-11 8:01 ` marcandre.lureau
2023-01-17 22:36 ` John Snow
2023-03-17 19:36 ` Vladimir Sementsov-Ogievskiy
2 siblings, 2 replies; 9+ messages in thread
From: marcandre.lureau @ 2023-01-11 8:01 UTC (permalink / raw)
To: qemu-devel
Cc: berrange, Beraldo Leal, John Snow, Cleber Rosa, Marc-André Lureau
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>
Reviewed-by: Daniel P. Berrangé <berrange@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 748a0d807c..5b2e499e68 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.39.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3 3/3] python/qemu/machine: use socketpair() for QMP by default
2023-01-11 8:01 ` [PATCH v3 3/3] python/qemu/machine: use socketpair() for QMP by default marcandre.lureau
@ 2023-01-17 22:36 ` John Snow
2023-01-18 8:02 ` Marc-André Lureau
2023-03-17 19:36 ` Vladimir Sementsov-Ogievskiy
1 sibling, 1 reply; 9+ messages in thread
From: John Snow @ 2023-01-17 22:36 UTC (permalink / raw)
To: marcandre.lureau; +Cc: qemu-devel, berrange, Beraldo Leal, Cleber Rosa
On Wed, Jan 11, 2023 at 3:01 AM <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>
> Reviewed-by: Daniel P. Berrangé <berrange@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 748a0d807c..5b2e499e68 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()
Needs an assert or an error-check here for _sock_pair to be non-None,
otherwise mypy will shout. Try running "make check-dev" from
qemu.git/python directory as a smoke test.
> if self._qmp_connection:
> self._qmp.accept(self._qmp_timer)
>
> --
> 2.39.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 3/3] python/qemu/machine: use socketpair() for QMP by default
2023-01-17 22:36 ` John Snow
@ 2023-01-18 8:02 ` Marc-André Lureau
2023-01-24 0:22 ` John Snow
0 siblings, 1 reply; 9+ messages in thread
From: Marc-André Lureau @ 2023-01-18 8:02 UTC (permalink / raw)
To: John Snow; +Cc: qemu-devel, berrange, Beraldo Leal, Cleber Rosa
Hi
On Wed, Jan 18, 2023 at 2:36 AM John Snow <jsnow@redhat.com> wrote:
>
> On Wed, Jan 11, 2023 at 3:01 AM <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>
> > Reviewed-by: Daniel P. Berrangé <berrange@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 748a0d807c..5b2e499e68 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()
>
> Needs an assert or an error-check here for _sock_pair to be non-None,
> otherwise mypy will shout. Try running "make check-dev" from
> qemu.git/python directory as a smoke test.
Good catch, it should be:
+ if self._sock_pair:
+ self._sock_pair[0].close()
Let me know if you want me to resend the whole series, or if you can
touch it during picking.
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 3/3] python/qemu/machine: use socketpair() for QMP by default
2023-01-18 8:02 ` Marc-André Lureau
@ 2023-01-24 0:22 ` John Snow
0 siblings, 0 replies; 9+ messages in thread
From: John Snow @ 2023-01-24 0:22 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: qemu-devel, berrange, Beraldo Leal, Cleber Rosa
On Wed, Jan 18, 2023 at 3:03 AM Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> Hi
>
> On Wed, Jan 18, 2023 at 2:36 AM John Snow <jsnow@redhat.com> wrote:
> >
> > On Wed, Jan 11, 2023 at 3:01 AM <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>
> > > Reviewed-by: Daniel P. Berrangé <berrange@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 748a0d807c..5b2e499e68 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()
> >
> > Needs an assert or an error-check here for _sock_pair to be non-None,
> > otherwise mypy will shout. Try running "make check-dev" from
> > qemu.git/python directory as a smoke test.
>
> Good catch, it should be:
>
> + if self._sock_pair:
> + self._sock_pair[0].close()
>
> Let me know if you want me to resend the whole series, or if you can
> touch it during picking.
Touching it up during picking, running tests, PR soon. Thanks,
--js
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 3/3] python/qemu/machine: use socketpair() for QMP by default
2023-01-11 8:01 ` [PATCH v3 3/3] python/qemu/machine: use socketpair() for QMP by default marcandre.lureau
2023-01-17 22:36 ` John Snow
@ 2023-03-17 19:36 ` Vladimir Sementsov-Ogievskiy
2023-03-20 10:56 ` Daniel P. Berrangé
1 sibling, 1 reply; 9+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2023-03-17 19:36 UTC (permalink / raw)
To: marcandre.lureau, qemu-devel
Cc: berrange, Beraldo Leal, John Snow, Cleber Rosa, Maksim Davydov
Hi!
By my investigation, this commit (bd4c0ef409140bd1be393407c04005ac077d4574) breaks long qmp output again.
Simple test:
$ cd python
$ cat test.py
#!/usr/bin/env python3
import sys
from qemu.machine import QEMUMachine
monitor_address = sys.argv[2] if len(sys.argv) > 2 else None
vm = QEMUMachine('../build/qemu-system-x86_64',
monitor_address=monitor_address)
vm.launch()
for x in range(int(sys.argv[1])):
vm.qmp('blockdev-add', {'driver': 'null-co', 'node-name': f'x{x}'})
vm.qmp('query-named-block-nodes')
vm.shutdown()
./test.py 1000 /tmp/sock
- works, but if use default behavior (socketpair) we get:
$ ./test.py 1000
Traceback (most recent call last):
File "/home/vsementsov/work/src/qemu/master/python/./test.py", line 14, in <module>
vm.qmp('query-named-block-nodes')
File "/home/vsementsov/work/src/qemu/master/python/qemu/machine/machine.py", line 686, in qmp
ret = self._qmp.cmd(cmd, args=qmp_args)
File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/legacy.py", line 216, in cmd
return self.cmd_obj(qmp_cmd)
File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/legacy.py", line 190, in cmd_obj
self._sync(
File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/legacy.py", line 105, in _sync
return self._aloop.run_until_complete(
File "/usr/lib/python3.10/asyncio/base_events.py", line 646, in run_until_complete
return future.result()
File "/usr/lib/python3.10/asyncio/tasks.py", line 408, in wait_for
return await fut
File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/qmp_client.py", line 547, in _raw
return await self._execute(msg, assign_id=assign_id)
File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/qmp_client.py", line 496, in _execute
return await self._reply(exec_id)
File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/qmp_client.py", line 463, in _reply
raise result
qemu.qmp.qmp_client.ExecInterruptedError: Disconnected
Exception ignored in: <function QEMUMonitorProtocol.__del__ at 0x7f8708283eb0>
Traceback (most recent call last):
File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/legacy.py", line 318, in __del__
File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/legacy.py", line 289, in close
File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/legacy.py", line 105, in _sync
File "/usr/lib/python3.10/asyncio/base_events.py", line 646, in run_until_complete
File "/usr/lib/python3.10/asyncio/tasks.py", line 408, in wait_for
File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/protocol.py", line 413, in disconnect
File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/protocol.py", line 725, in _wait_disconnect
File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/protocol.py", line 876, in _bh_loop_forever
File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/protocol.py", line 914, in _bh_recv_message
File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/protocol.py", line 1015, in _recv
File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/qmp_client.py", line 402, in _do_recv
File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/protocol.py", line 979, in _readline
File "/usr/lib/python3.10/asyncio/streams.py", line 534, in readline
ValueError: Separator is not found, and chunk exceed the limit
./test.py 100
- works well.
On 11.01.23 11:01, 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>
> Reviewed-by: Daniel P. Berrangé <berrange@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 748a0d807c..5b2e499e68 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)
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 3/3] python/qemu/machine: use socketpair() for QMP by default
2023-03-17 19:36 ` Vladimir Sementsov-Ogievskiy
@ 2023-03-20 10:56 ` Daniel P. Berrangé
0 siblings, 0 replies; 9+ messages in thread
From: Daniel P. Berrangé @ 2023-03-20 10:56 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: marcandre.lureau, qemu-devel, Beraldo Leal, John Snow,
Cleber Rosa, Maksim Davydov
On Fri, Mar 17, 2023 at 10:36:37PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Hi!
>
> By my investigation, this commit (bd4c0ef409140bd1be393407c04005ac077d4574) breaks long qmp output again.
>
> ./test.py 1000 /tmp/sock
>
> - works, but if use default behavior (socketpair) we get:
>
> $ ./test.py 1000
> Traceback (most recent call last):
snip
> File "/usr/lib/python3.10/asyncio/streams.py", line 534, in readline
> ValueError: Separator is not found, and chunk exceed the limit
After going off in the weeds I realized this message is the key bit. We
failed to pass the raised recv limit to asyncio when using a pre-opened
socket. Since the QMP reply was greater than 64kb asyncio raised an
exception. This was a pre-existing latent bug, exposed with the patch
to enable use of socketpair(). I've CC'd you on a patch to fix this.
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 :|
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-03-20 10:56 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-11 8:00 [PATCH v3 0/3] python/qemu/machine: fix potential hang in QMP accept marcandre.lureau
2023-01-11 8:00 ` [PATCH v3 1/3] python/qmp/protocol: add open_with_socket() marcandre.lureau
2023-01-11 8:01 ` [PATCH v3 2/3] python/qmp/legacy: make QEMUMonitorProtocol accept a socket marcandre.lureau
2023-01-11 8:01 ` [PATCH v3 3/3] python/qemu/machine: use socketpair() for QMP by default marcandre.lureau
2023-01-17 22:36 ` John Snow
2023-01-18 8:02 ` Marc-André Lureau
2023-01-24 0:22 ` John Snow
2023-03-17 19:36 ` Vladimir Sementsov-Ogievskiy
2023-03-20 10:56 ` Daniel P. Berrangé
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.