From 3bc72e3aed76e0326703db81964b13f1da075cbf Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 10 Jan 2022 18:28:45 -0500 Subject: python/aqmp: add __del__ method to legacy interface asyncio can complain *very* loudly if you forget to back out of things gracefully before the garbage collector starts destroying objects that contain live references to asyncio Tasks. The usual fix is just to remember to call aqmp.disconnect(), but for the sake of the legacy wrapper and quick, one-off scripts where a graceful shutdown is not necessarily of paramount imporance, add a courtesy cleanup that will trigger prior to seeing screenfuls of confusing asyncio tracebacks. Note that we can't *always* save you from yourself; depending on when the GC runs, you might just seriously be out of luck. The best we can do in this case is to gently remind you to clean up after yourself. (Still much better than multiple pages of incomprehensible python warnings for the crime of forgetting to put your toys away.) Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Beraldo Leal --- python/qemu/aqmp/legacy.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'python/qemu/aqmp/legacy.py') diff --git a/python/qemu/aqmp/legacy.py b/python/qemu/aqmp/legacy.py index 9e7b9fb80b..2ccb136b02 100644 --- a/python/qemu/aqmp/legacy.py +++ b/python/qemu/aqmp/legacy.py @@ -16,6 +16,8 @@ from typing import ( import qemu.qmp from qemu.qmp import QMPMessage, QMPReturnValue, SocketAddrT +from .error import AQMPError +from .protocol import Runstate from .qmp_client import QMPClient @@ -136,3 +138,19 @@ class QEMUMonitorProtocol(qemu.qmp.QEMUMonitorProtocol): def send_fd_scm(self, fd: int) -> None: self._aqmp.send_fd_scm(fd) + + def __del__(self) -> None: + if self._aqmp.runstate == Runstate.IDLE: + return + + if not self._aloop.is_running(): + self.close() + else: + # Garbage collection ran while the event loop was running. + # Nothing we can do about it now, but if we don't raise our + # own error, the user will be treated to a lot of traceback + # they might not understand. + raise AQMPError( + "QEMUMonitorProtocol.close()" + " was not called before object was garbage collected" + ) -- cgit 1.4.1 From 0e6bfd8b96e407db7b0cb5e8c14cc315a7154f53 Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 10 Jan 2022 18:28:47 -0500 Subject: python/aqmp: copy type definitions from qmp Copy the remaining type definitions from QMP into the qemu.aqmp.legacy module. Now, users that require the legacy interface don't need to import anything else but qemu.aqmp.legacy wrapper. Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Beraldo Leal --- python/qemu/aqmp/legacy.py | 22 ++++++++++++++++++++-- python/qemu/aqmp/protocol.py | 16 ++++++++++------ 2 files changed, 30 insertions(+), 8 deletions(-) (limited to 'python/qemu/aqmp/legacy.py') diff --git a/python/qemu/aqmp/legacy.py b/python/qemu/aqmp/legacy.py index 2ccb136b02..9431fe9330 100644 --- a/python/qemu/aqmp/legacy.py +++ b/python/qemu/aqmp/legacy.py @@ -6,7 +6,9 @@ This class pretends to be qemu.qmp.QEMUMonitorProtocol. import asyncio from typing import ( + Any, Awaitable, + Dict, List, Optional, TypeVar, @@ -14,13 +16,29 @@ from typing import ( ) import qemu.qmp -from qemu.qmp import QMPMessage, QMPReturnValue, SocketAddrT from .error import AQMPError -from .protocol import Runstate +from .protocol import Runstate, SocketAddrT from .qmp_client import QMPClient +#: QMPMessage is an entire QMP message of any kind. +QMPMessage = Dict[str, Any] + +#: QMPReturnValue is the 'return' value of a command. +QMPReturnValue = object + +#: QMPObject is any object in a QMP message. +QMPObject = Dict[str, object] + +# QMPMessage can be outgoing commands or incoming events/returns. +# QMPReturnValue is usually a dict/json object, but due to QAPI's +# 'returns-whitelist', it can actually be anything. +# +# {'return': {}} is a QMPMessage, +# {} is the QMPReturnValue. + + # pylint: disable=missing-docstring diff --git a/python/qemu/aqmp/protocol.py b/python/qemu/aqmp/protocol.py index c4fbe35a0e..5b4f2f0d0a 100644 --- a/python/qemu/aqmp/protocol.py +++ b/python/qemu/aqmp/protocol.py @@ -46,6 +46,10 @@ T = TypeVar('T') _U = TypeVar('_U') _TaskFN = Callable[[], Awaitable[None]] # aka ``async def func() -> None`` +InternetAddrT = Tuple[str, int] +UnixAddrT = str +SocketAddrT = Union[UnixAddrT, InternetAddrT] + class Runstate(Enum): """Protocol session runstate.""" @@ -257,7 +261,7 @@ class AsyncProtocol(Generic[T]): @upper_half @require(Runstate.IDLE) - async def accept(self, address: Union[str, Tuple[str, int]], + async def accept(self, address: SocketAddrT, ssl: Optional[SSLContext] = None) -> None: """ Accept a connection and begin processing message queues. @@ -275,7 +279,7 @@ class AsyncProtocol(Generic[T]): @upper_half @require(Runstate.IDLE) - async def connect(self, address: Union[str, Tuple[str, int]], + async def connect(self, address: SocketAddrT, ssl: Optional[SSLContext] = None) -> None: """ Connect to the server and begin processing message queues. @@ -337,7 +341,7 @@ class AsyncProtocol(Generic[T]): @upper_half async def _new_session(self, - address: Union[str, Tuple[str, int]], + address: SocketAddrT, ssl: Optional[SSLContext] = None, accept: bool = False) -> None: """ @@ -397,7 +401,7 @@ class AsyncProtocol(Generic[T]): @upper_half async def _establish_connection( self, - address: Union[str, Tuple[str, int]], + address: SocketAddrT, ssl: Optional[SSLContext] = None, accept: bool = False ) -> None: @@ -424,7 +428,7 @@ class AsyncProtocol(Generic[T]): await self._do_connect(address, ssl) @upper_half - async def _do_accept(self, address: Union[str, Tuple[str, int]], + async def _do_accept(self, address: SocketAddrT, ssl: Optional[SSLContext] = None) -> None: """ Acting as the transport server, accept a single connection. @@ -482,7 +486,7 @@ class AsyncProtocol(Generic[T]): self.logger.debug("Connection accepted.") @upper_half - async def _do_connect(self, address: Union[str, Tuple[str, int]], + async def _do_connect(self, address: SocketAddrT, ssl: Optional[SSLContext] = None) -> None: """ Acting as the transport client, initiate a connection to a server. -- cgit 1.4.1 From 6e7751dc388df6daf425db0e245d4d3a10859803 Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 10 Jan 2022 18:28:49 -0500 Subject: python/aqmp: rename AQMPError to QMPError This is in preparation for renaming qemu.aqmp to qemu.qmp. I should have done this from this from the very beginning, but it's a convenient time to make sure this churn is taken care of. Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy --- python/qemu/aqmp/__init__.py | 6 +++--- python/qemu/aqmp/error.py | 12 ++++++------ python/qemu/aqmp/events.py | 4 ++-- python/qemu/aqmp/legacy.py | 4 ++-- python/qemu/aqmp/protocol.py | 8 ++++---- python/qemu/aqmp/qmp_client.py | 8 ++++---- 6 files changed, 21 insertions(+), 21 deletions(-) (limited to 'python/qemu/aqmp/legacy.py') diff --git a/python/qemu/aqmp/__init__.py b/python/qemu/aqmp/__init__.py index 05f467c141..4c22c38079 100644 --- a/python/qemu/aqmp/__init__.py +++ b/python/qemu/aqmp/__init__.py @@ -6,7 +6,7 @@ asynchronously with QMP protocol servers, as implemented by QEMU, the QEMU Guest Agent, and the QEMU Storage Daemon. `QMPClient` provides the main functionality of this package. All errors -raised by this library derive from `AQMPError`, see `aqmp.error` for +raised by this library derive from `QMPError`, see `aqmp.error` for additional detail. See `aqmp.events` for an in-depth tutorial on managing QMP events. """ @@ -23,7 +23,7 @@ managing QMP events. import logging -from .error import AQMPError +from .error import QMPError from .events import EventListener from .message import Message from .protocol import ( @@ -48,7 +48,7 @@ __all__ = ( 'Runstate', # Exceptions, most generic to most explicit - 'AQMPError', + 'QMPError', 'StateError', 'ConnectError', 'ExecuteError', diff --git a/python/qemu/aqmp/error.py b/python/qemu/aqmp/error.py index 781f49b008..24ba4d5054 100644 --- a/python/qemu/aqmp/error.py +++ b/python/qemu/aqmp/error.py @@ -1,21 +1,21 @@ """ -AQMP Error Classes +QMP Error Classes This package seeks to provide semantic error classes that are intended to be used directly by clients when they would like to handle particular semantic failures (e.g. "failed to connect") without needing to know the enumeration of possible reasons for that failure. -AQMPError serves as the ancestor for all exceptions raised by this +QMPError serves as the ancestor for all exceptions raised by this package, and is suitable for use in handling semantic errors from this library. In most cases, individual public methods will attempt to catch and re-encapsulate various exceptions to provide a semantic error-handling interface. -.. admonition:: AQMP Exception Hierarchy Reference +.. admonition:: QMP Exception Hierarchy Reference | `Exception` - | +-- `AQMPError` + | +-- `QMPError` | +-- `ConnectError` | +-- `StateError` | +-- `ExecInterruptedError` @@ -31,11 +31,11 @@ error-handling interface. """ -class AQMPError(Exception): +class QMPError(Exception): """Abstract error class for all errors originating from this package.""" -class ProtocolError(AQMPError): +class ProtocolError(QMPError): """ Abstract error class for protocol failures. diff --git a/python/qemu/aqmp/events.py b/python/qemu/aqmp/events.py index 5f7150c78d..f3d4e2b5e8 100644 --- a/python/qemu/aqmp/events.py +++ b/python/qemu/aqmp/events.py @@ -443,7 +443,7 @@ from typing import ( Union, ) -from .error import AQMPError +from .error import QMPError from .message import Message @@ -451,7 +451,7 @@ EventNames = Union[str, Iterable[str], None] EventFilter = Callable[[Message], bool] -class ListenerError(AQMPError): +class ListenerError(QMPError): """ Generic error class for `EventListener`-related problems. """ diff --git a/python/qemu/aqmp/legacy.py b/python/qemu/aqmp/legacy.py index 9431fe9330..27df22818a 100644 --- a/python/qemu/aqmp/legacy.py +++ b/python/qemu/aqmp/legacy.py @@ -17,7 +17,7 @@ from typing import ( import qemu.qmp -from .error import AQMPError +from .error import QMPError from .protocol import Runstate, SocketAddrT from .qmp_client import QMPClient @@ -168,7 +168,7 @@ class QEMUMonitorProtocol(qemu.qmp.QEMUMonitorProtocol): # Nothing we can do about it now, but if we don't raise our # own error, the user will be treated to a lot of traceback # they might not understand. - raise AQMPError( + raise QMPError( "QEMUMonitorProtocol.close()" " was not called before object was garbage collected" ) diff --git a/python/qemu/aqmp/protocol.py b/python/qemu/aqmp/protocol.py index 5b4f2f0d0a..50e973c2f2 100644 --- a/python/qemu/aqmp/protocol.py +++ b/python/qemu/aqmp/protocol.py @@ -29,7 +29,7 @@ from typing import ( cast, ) -from .error import AQMPError +from .error import QMPError from .util import ( bottom_half, create_task, @@ -65,7 +65,7 @@ class Runstate(Enum): DISCONNECTING = 3 -class ConnectError(AQMPError): +class ConnectError(QMPError): """ Raised when the initial connection process has failed. @@ -90,7 +90,7 @@ class ConnectError(AQMPError): return f"{self.error_message}: {cause}" -class StateError(AQMPError): +class StateError(QMPError): """ An API command (connect, execute, etc) was issued at an inappropriate time. @@ -363,7 +363,7 @@ class AsyncProtocol(Generic[T]): This exception will wrap a more concrete one. In most cases, the wrapped exception will be `OSError` or `EOFError`. If a protocol-level failure occurs while establishing a new - session, the wrapped error may also be an `AQMPError`. + session, the wrapped error may also be an `QMPError`. """ assert self.runstate == Runstate.IDLE diff --git a/python/qemu/aqmp/qmp_client.py b/python/qemu/aqmp/qmp_client.py index 6a985ffe30..f1a845cc82 100644 --- a/python/qemu/aqmp/qmp_client.py +++ b/python/qemu/aqmp/qmp_client.py @@ -20,7 +20,7 @@ from typing import ( cast, ) -from .error import AQMPError, ProtocolError +from .error import ProtocolError, QMPError from .events import Events from .message import Message from .models import ErrorResponse, Greeting @@ -66,7 +66,7 @@ class NegotiationError(_WrappedProtocolError): """ -class ExecuteError(AQMPError): +class ExecuteError(QMPError): """ Exception raised by `QMPClient.execute()` on RPC failure. @@ -87,7 +87,7 @@ class ExecuteError(AQMPError): self.error_class: str = error_response.error.class_ -class ExecInterruptedError(AQMPError): +class ExecInterruptedError(QMPError): """ Exception raised by `execute()` (et al) when an RPC is interrupted. @@ -641,7 +641,7 @@ class QMPClient(AsyncProtocol[Message], Events): sock = self._writer.transport.get_extra_info('socket') if sock.family != socket.AF_UNIX: - raise AQMPError("Sending file descriptors requires a UNIX socket.") + raise QMPError("Sending file descriptors requires a UNIX socket.") if not hasattr(sock, 'sendmsg'): # We need to void the warranty sticker. -- cgit 1.4.1 From f3efd12930f34b9724e15d8fd2ff56a97b67219d Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 10 Jan 2022 18:28:53 -0500 Subject: python/qmp: switch qmp-shell to AQMP We have a replacement for async QMP, but it doesn't have feature parity yet. For now, then, port the old tool onto the new backend. Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy --- python/qemu/aqmp/legacy.py | 3 +++ python/qemu/qmp/qmp_shell.py | 31 +++++++++++++++++-------------- 2 files changed, 20 insertions(+), 14 deletions(-) (limited to 'python/qemu/aqmp/legacy.py') diff --git a/python/qemu/aqmp/legacy.py b/python/qemu/aqmp/legacy.py index 27df22818a..0890f95b16 100644 --- a/python/qemu/aqmp/legacy.py +++ b/python/qemu/aqmp/legacy.py @@ -22,6 +22,9 @@ from .protocol import Runstate, SocketAddrT from .qmp_client import QMPClient +# (Temporarily) Re-export QMPBadPortError +QMPBadPortError = qemu.qmp.QMPBadPortError + #: QMPMessage is an entire QMP message of any kind. QMPMessage = Dict[str, Any] diff --git a/python/qemu/qmp/qmp_shell.py b/python/qemu/qmp/qmp_shell.py index e7d7eb18f1..d11bf54b00 100644 --- a/python/qemu/qmp/qmp_shell.py +++ b/python/qemu/qmp/qmp_shell.py @@ -95,8 +95,13 @@ from typing import ( Sequence, ) -from qemu import qmp -from qemu.qmp import QMPMessage +from qemu.aqmp import ConnectError, QMPError, SocketAddrT +from qemu.aqmp.legacy import ( + QEMUMonitorProtocol, + QMPBadPortError, + QMPMessage, + QMPObject, +) LOG = logging.getLogger(__name__) @@ -125,7 +130,7 @@ class QMPCompleter: return None -class QMPShellError(qmp.QMPError): +class QMPShellError(QMPError): """ QMP Shell Base error class. """ @@ -153,7 +158,7 @@ class FuzzyJSON(ast.NodeTransformer): return node -class QMPShell(qmp.QEMUMonitorProtocol): +class QMPShell(QEMUMonitorProtocol): """ QMPShell provides a basic readline-based QMP shell. @@ -161,7 +166,7 @@ class QMPShell(qmp.QEMUMonitorProtocol): :param pretty: Pretty-print QMP messages. :param verbose: Echo outgoing QMP messages to console. """ - def __init__(self, address: qmp.SocketAddrT, + def __init__(self, address: SocketAddrT, pretty: bool = False, verbose: bool = False): super().__init__(address) self._greeting: Optional[QMPMessage] = None @@ -237,7 +242,7 @@ class QMPShell(qmp.QEMUMonitorProtocol): def _cli_expr(self, tokens: Sequence[str], - parent: qmp.QMPObject) -> None: + parent: QMPObject) -> None: for arg in tokens: (key, sep, val) = arg.partition('=') if sep != '=': @@ -403,7 +408,7 @@ class HMPShell(QMPShell): :param pretty: Pretty-print QMP messages. :param verbose: Echo outgoing QMP messages to console. """ - def __init__(self, address: qmp.SocketAddrT, + def __init__(self, address: SocketAddrT, pretty: bool = False, verbose: bool = False): super().__init__(address, pretty, verbose) self._cpu_index = 0 @@ -512,19 +517,17 @@ def main() -> None: try: address = shell_class.parse_address(args.qmp_server) - except qmp.QMPBadPortError: + except QMPBadPortError: parser.error(f"Bad port number: {args.qmp_server}") return # pycharm doesn't know error() is noreturn with shell_class(address, args.pretty, args.verbose) as qemu: try: qemu.connect(negotiate=not args.skip_negotiation) - except qmp.QMPConnectError: - die("Didn't get QMP greeting message") - except qmp.QMPCapabilitiesError: - die("Couldn't negotiate capabilities") - except OSError as err: - die(f"Couldn't connect to {args.qmp_server}: {err!s}") + except ConnectError as err: + if isinstance(err.exc, OSError): + die(f"Couldn't connect to {args.qmp_server}: {err!s}") + die(str(err)) for _ in qemu.repl(): pass -- cgit 1.4.1