Merge remote-tracking branch 'remotes/jsnow-gitlab/tags/python-pull-r…

…equest' into staging

Python patches

A few fixes to the Python CI tests, a few fixes to the (async) QMP
library, and a set of patches that begin to shift us towards using the
new qmp lib.

# gpg: Signature made Sat 22 Jan 2022 00:07:58 GMT
# gpg:                using RSA key F9B7ABDBBCACDF95BE76CBD07DEF8106AAFC390E
# gpg: Good signature from "John Snow (John Huston) <jsnow@redhat.com>" [full]
# Primary key fingerprint: FAEB 9711 A12C F475 812F  18F2 88A9 064D 1835 61EB
#      Subkey fingerprint: F9B7 ABDB BCAC DF95 BE76  CBD0 7DEF 8106 AAFC 390E

* remotes/jsnow-gitlab/tags/python-pull-request:
  scripts/render-block-graph: switch to AQMP
  scripts/cpu-x86-uarch-abi: switch to AQMP
  scripts/cpu-x86-uarch-abi: fix CLI parsing
  python: move qmp-shell under the AQMP package
  python: move qmp utilities to python/qemu/utils
  python/qmp: switch qmp-shell to AQMP
  python/qmp: switch qom tools to AQMP
  python/qmp: switch qemu-ga-client to AQMP
  python/qemu-ga-client: don't use deprecated CLI syntax in usage comment
  python/aqmp: rename AQMPError to QMPError
  python/aqmp: add SocketAddrT to package root
  python/aqmp: copy type definitions from qmp
  python/aqmp: handle asyncio.TimeoutError on execute()
  python/aqmp: add __del__ method to legacy interface
  python/aqmp: fix docstring typo
  python: use avocado's "new" runner
  python: pin setuptools below v60.0.0

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

python/Makefile

@@ -68,6 +68,8 @@ $(QEMU_VENV_DIR) $(QEMU_VENV_DIR)/bin/activate: setup.cfg
 		echo "ACTIVATE $(QEMU_VENV_DIR)";		\
 		. $(QEMU_VENV_DIR)/bin/activate;		\
 		echo "INSTALL qemu[devel] $(QEMU_VENV_DIR)";	\
+		pip install --disable-pip-version-check		\
+			"setuptools<60.0.0" 1>/dev/null;	\
 		make develop 1>/dev/null;			\
 	)
 	@touch $(QEMU_VENV_DIR)

python/README.rst

@@ -59,7 +59,7 @@ Package installation also normally provides executable console scripts,
 so that tools like ``qmp-shell`` are always available via $PATH. To
 invoke them without installation, you can invoke e.g.:
 
-``> PYTHONPATH=~/src/qemu/python python3 -m qemu.qmp.qmp_shell``
+``> PYTHONPATH=~/src/qemu/python python3 -m qemu.aqmp.qmp_shell``
 
 The mappings between console script name and python module path can be
 found in ``setup.cfg``.

python/avocado.cfg

@@ -1,5 +1,5 @@
 [run]
-test_runner = runner
+test_runner = nrunner
 
 [simpletests]
 # Don't show stdout/stderr in the test *summary*

python/qemu/aqmp/__init__.py

@@ -6,7 +6,7 @@
 QEMU Guest Agent, and the QEMU Storage Daemon.
 
 `QMPClient` provides the main functionality of this package. All errors
-raised by this library dervive 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,10 +23,15 @@
 
 import logging
 
-from .error import AQMPError
+from .error import QMPError
 from .events import EventListener
 from .message import Message
-from .protocol import ConnectError, Runstate, StateError
+from .protocol import (
+    ConnectError,
+    Runstate,
+    SocketAddrT,
+    StateError,
+)
 from .qmp_client import ExecInterruptedError, ExecuteError, QMPClient
 
 
@@ -43,9 +48,12 @@
     'Runstate',
 
     # Exceptions, most generic to most explicit
-    'AQMPError',
+    'QMPError',
     'StateError',
     'ConnectError',
     'ExecuteError',
     'ExecInterruptedError',
+
+    # Type aliases
+    'SocketAddrT',
 )

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 @@
 """
 
 
-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.
 

python/qemu/aqmp/events.py

@@ -443,15 +443,15 @@ def accept(self, event) -> bool:
     Union,
 )
 
-from .error import AQMPError
+from .error import QMPError
 from .message import Message
 
 
 EventNames = Union[str, Iterable[str], None]
 EventFilter = Callable[[Message], bool]
 
 
-class ListenerError(AQMPError):
+class ListenerError(QMPError):
     """
     Generic error class for `EventListener`-related problems.
     """

python/qemu/aqmp/legacy.py

@@ -6,19 +6,42 @@
 
 import asyncio
 from typing import (
+    Any,
     Awaitable,
+    Dict,
     List,
     Optional,
     TypeVar,
     Union,
 )
 
 import qemu.qmp
-from qemu.qmp import QMPMessage, QMPReturnValue, SocketAddrT
 
+from .error import QMPError
+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]
+
+#: 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
 
 
@@ -136,3 +159,19 @@ def settimeout(self, timeout: Optional[float]) -> None:
 
     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 QMPError(
+                "QEMUMonitorProtocol.close()"
+                " was not called before object was garbage collected"
+            )

python/qemu/aqmp/protocol.py

@@ -29,7 +29,7 @@
     cast,
 )
 
-from .error import AQMPError
+from .error import QMPError
 from .util import (
     bottom_half,
     create_task,
@@ -46,6 +46,10 @@
 _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."""
@@ -61,7 +65,7 @@ class Runstate(Enum):
     DISCONNECTING = 3
 
 
-class ConnectError(AQMPError):
+class ConnectError(QMPError):
     """
     Raised when the initial connection process has failed.
 
@@ -86,7 +90,7 @@ def __str__(self) -> str:
         return f"{self.error_message}: {cause}"
 
 
-class StateError(AQMPError):
+class StateError(QMPError):
     """
     An API command (connect, execute, etc) was issued at an inappropriate time.
 
@@ -257,7 +261,7 @@ async def runstate_changed(self) -> Runstate:
 
     @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 @@ async def accept(self, address: Union[str, Tuple[str, int]],
 
     @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 @@ def _set_state(self, state: Runstate) -> None:
 
     @upper_half
     async def _new_session(self,
-                           address: Union[str, Tuple[str, int]],
+                           address: SocketAddrT,
                            ssl: Optional[SSLContext] = None,
                            accept: bool = False) -> None:
         """
@@ -359,7 +363,7 @@ async def _new_session(self,
             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
 
@@ -397,7 +401,7 @@ async def _new_session(self,
     @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 @@ async def _establish_connection(
             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 @@ async def _client_connected_cb(reader: asyncio.StreamReader,
         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.

python/qemu/aqmp/qmp_client.py

@@ -20,7 +20,7 @@
     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 @@ def __init__(self, error_response: ErrorResponse,
         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.
 
@@ -435,7 +435,11 @@ async def _issue(self, msg: Message) -> Union[None, str]:
             msg_id = msg['id']
 
         self._pending[msg_id] = asyncio.Queue(maxsize=1)
-        await self._outgoing.put(msg)
+        try:
+            await self._outgoing.put(msg)
+        except:
+            del self._pending[msg_id]
+            raise
 
         return msg_id
 
@@ -452,9 +456,9 @@ async def _reply(self, msg_id: Union[str, None]) -> Message:
             was lost, or some other problem.
         """
         queue = self._pending[msg_id]
-        result = await queue.get()
 
         try:
+            result = await queue.get()
             if isinstance(result, ExecInterruptedError):
                 raise result
             return result
@@ -637,7 +641,7 @@ def send_fd_scm(self, fd: int) -> None:
         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.