The old QMP library would actually bind to the server address during
__init__(). The new library delays this to the accept() call, because
binding occurs inside of the call to start_[unix_]server(), which is an
async method -- so it cannot happen during __init__ anymore.
Python 3.7+ adds the ability to create the server (and thus the bind()
call) and begin the active listening in separate steps, but we don't
have that functionality in 3.6, our current minimum.
Therefore ... Add a temporary workaround that allows the synchronous
version of the client to bind the socket in advance, guaranteeing that
there will be a UNIX socket in the filesystem ready for the QEMU client
to connect to without a race condition.
(Yes, it's ugly; fixing it more nicely will unfortunately have to wait
until I can stipulate Python 3.7+ as our minimum version. Python 3.6 is
EOL as of the beginning of this year, but I haven't checked if all of
our supported build platforms have a properly modern Python available
yet.)
Signed-off-by: John Snow <jsnow@redhat.com>
---
python/qemu/aqmp/legacy.py | 3 +++
python/qemu/aqmp/protocol.py | 41 +++++++++++++++++++++++++++++++++---
2 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/python/qemu/aqmp/legacy.py b/python/qemu/aqmp/legacy.py
index 9e7b9fb80b..cf7634ee95 100644
--- a/python/qemu/aqmp/legacy.py
+++ b/python/qemu/aqmp/legacy.py
@@ -33,6 +33,9 @@ def __init__(self, address: SocketAddrT,
self._address = address
self._timeout: Optional[float] = None
+ if server:
+ self._aqmp._bind_hack(address) # pylint: disable=protected-access
+
_T = TypeVar('_T')
def _sync(
diff --git a/python/qemu/aqmp/protocol.py b/python/qemu/aqmp/protocol.py
index c4fbe35a0e..eb740a5452 100644
--- a/python/qemu/aqmp/protocol.py
+++ b/python/qemu/aqmp/protocol.py
@@ -15,6 +15,7 @@
from enum import Enum
from functools import wraps
import logging
+import socket
from ssl import SSLContext
from typing import (
Any,
@@ -234,6 +235,9 @@ def __init__(self, name: Optional[str] = None) -> None:
self._runstate = Runstate.IDLE
self._runstate_changed: Optional[asyncio.Event] = None
+ # Workaround for bind()
+ self._sock: Optional[socket.socket] = None
+
def __repr__(self) -> str:
cls_name = type(self).__name__
tokens = []
@@ -423,6 +427,34 @@ async def _establish_connection(
else:
await self._do_connect(address, ssl)
+ def _bind_hack(self, address: Union[str, Tuple[str, int]]) -> None:
+ """
+ Used to create a socket in advance of accept().
+
+ This is a workaround to ensure that we can guarantee timing of
+ precisely when a socket exists to avoid a connection attempt
+ bouncing off of nothing.
+
+ Python 3.7+ adds a feature to separate the server creation and
+ listening phases instead, and should be used instead of this
+ hack.
+ """
+ if isinstance(address, tuple):
+ family = socket.AF_INET
+ else:
+ family = socket.AF_UNIX
+
+ sock = socket.socket(family, socket.SOCK_STREAM)
+ sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+
+ try:
+ sock.bind(address)
+ except:
+ sock.close()
+ raise
+
+ self._sock = sock
+
@upper_half
async def _do_accept(self, address: Union[str, Tuple[str, int]],
ssl: Optional[SSLContext] = None) -> None:
@@ -460,24 +492,27 @@ async def _client_connected_cb(reader: asyncio.StreamReader,
if isinstance(address, tuple):
coro = asyncio.start_server(
_client_connected_cb,
- host=address[0],
- port=address[1],
+ host=None if self._sock else address[0],
+ port=None if self._sock else address[1],
ssl=ssl,
backlog=1,
limit=self._limit,
+ sock=self._sock,
)
else:
coro = asyncio.start_unix_server(
_client_connected_cb,
- path=address,
+ path=None if self._sock else address,
ssl=ssl,
backlog=1,
limit=self._limit,
+ sock=self._sock,
)
server = await coro # Starts listening
await connected.wait() # Waits for the callback to fire (and finish)
assert server is None
+ self._sock = None
self.logger.debug("Connection accepted.")
--
2.31.1
On Wed, Jan 19, 2022 at 02:39:16PM -0500, John Snow wrote: > The old QMP library would actually bind to the server address during > __init__(). The new library delays this to the accept() call, because > binding occurs inside of the call to start_[unix_]server(), which is an > async method -- so it cannot happen during __init__ anymore. > > Python 3.7+ adds the ability to create the server (and thus the bind() > call) and begin the active listening in separate steps, but we don't > have that functionality in 3.6, our current minimum. > > Therefore ... Add a temporary workaround that allows the synchronous > version of the client to bind the socket in advance, guaranteeing that > there will be a UNIX socket in the filesystem ready for the QEMU client > to connect to without a race condition. > > (Yes, it's ugly; fixing it more nicely will unfortunately have to wait > until I can stipulate Python 3.7+ as our minimum version. Python 3.6 is > EOL as of the beginning of this year, but I haven't checked if all of > our supported build platforms have a properly modern Python available > yet.) RHEL-8 system python will remain 3.6 for the life of RHEL-8. While you can bring in newer python versions in parallel, IMHO it is highly desirable to remain compatible with the system python as that's the one you can guarantee users actually have available by default. 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 :|
On Thu, Jan 20, 2022, 4:13 AM Daniel P. Berrangé <berrange@redhat.com> wrote: > On Wed, Jan 19, 2022 at 02:39:16PM -0500, John Snow wrote: > > The old QMP library would actually bind to the server address during > > __init__(). The new library delays this to the accept() call, because > > binding occurs inside of the call to start_[unix_]server(), which is an > > async method -- so it cannot happen during __init__ anymore. > > > > Python 3.7+ adds the ability to create the server (and thus the bind() > > call) and begin the active listening in separate steps, but we don't > > have that functionality in 3.6, our current minimum. > > > > Therefore ... Add a temporary workaround that allows the synchronous > > version of the client to bind the socket in advance, guaranteeing that > > there will be a UNIX socket in the filesystem ready for the QEMU client > > to connect to without a race condition. > > > > (Yes, it's ugly; fixing it more nicely will unfortunately have to wait > > until I can stipulate Python 3.7+ as our minimum version. Python 3.6 is > > EOL as of the beginning of this year, but I haven't checked if all of > > our supported build platforms have a properly modern Python available > > yet.) > > RHEL-8 system python will remain 3.6 for the life of RHEL-8. > > While you can bring in newer python versions in parallel, > IMHO it is highly desirable to remain compatible with the > system python as that's the one you can guarantee users > actually have available by default. > I agree, but over time my hand will be forced. Libraries are beginning to drop support for Python 3.6 upstream, and it's only a matter of time before it becomes implausible to support an EOL python version. I actually go out of my way to ensure compatibility with the very oldest versions I possibly can - *extremely* out of my way - but there's only so much I can reasonably do. Supporting 3.6 and 3.11 simultaneously may prove challenging. Either way, I'm not bumping the version here in this series. I'm just stating that this hack is kind of the best I can (quickly and easily) do until 3.7. (3.7 adds start_server=False to start_unix_server which allows the separation of steps without needing to muck around with the socket object.) > > > 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 : >
© 2016 - 2026 Red Hat, Inc.