From 932ca4bbde5ed6c57a8ae0b9cabb6e0a1ca1047a Mon Sep 17 00:00:00 2001 From: John Snow Date: Tue, 6 Oct 2020 19:57:58 -0400 Subject: python/qemu: use isort to lay out imports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Borrowed from the QAPI cleanup series, use the same configuration to standardize the way we write and sort imports. Signed-off-by: John Snow Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Kevin Wolf Message-id: 20201006235817.3280413-2-jsnow@redhat.com Signed-off-by: John Snow --- python/qemu/machine.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'python/qemu/machine.py') diff --git a/python/qemu/machine.py b/python/qemu/machine.py index 82f3731fc3..bc83f399c1 100644 --- a/python/qemu/machine.py +++ b/python/qemu/machine.py @@ -20,15 +20,15 @@ which provides facilities for managing the lifetime of a QEMU VM. import errno import logging import os -import subprocess import shutil import signal +import subprocess import tempfile -from typing import Optional, Type from types import TracebackType -from . import console_socket +from typing import Optional, Type + +from . import console_socket, qmp -from . import qmp LOG = logging.getLogger(__name__) -- cgit 1.4.1 From c4e6023f05fe6dd14d9193814679490feac546a4 Mon Sep 17 00:00:00 2001 From: John Snow Date: Tue, 6 Oct 2020 19:57:59 -0400 Subject: python/machine.py: Fix monitor address typing Prior to this, it's difficult for mypy to intuit what the concrete type of the monitor address is; it has difficulty inferring the type across two variables. Create _monitor_address as a property that always returns a valid address to simplify static type analysis. To preserve our ability to clean up, use a simple boolean to indicate whether or not we should try to clean up the sock file after execution. Signed-off-by: John Snow Reviewed-by: Kevin Wolf Message-id: 20201006235817.3280413-3-jsnow@redhat.com Signed-off-by: John Snow --- python/qemu/machine.py | 40 +++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-) (limited to 'python/qemu/machine.py') diff --git a/python/qemu/machine.py b/python/qemu/machine.py index bc83f399c1..3017ec072d 100644 --- a/python/qemu/machine.py +++ b/python/qemu/machine.py @@ -28,6 +28,7 @@ from types import TracebackType from typing import Optional, Type from . import console_socket, qmp +from .qmp import SocketAddrT LOG = logging.getLogger(__name__) @@ -68,7 +69,8 @@ class QEMUMachine: """ def __init__(self, binary, args=None, wrapper=None, name=None, - test_dir="/var/tmp", monitor_address=None, + test_dir="/var/tmp", + monitor_address: Optional[SocketAddrT] = None, socket_scm_helper=None, sock_dir=None, drain_console=False, console_log=None): ''' @@ -95,8 +97,14 @@ class QEMUMachine: if sock_dir is None: sock_dir = test_dir self._name = name - self._monitor_address = monitor_address - self._vm_monitor = None + if monitor_address is not None: + self._monitor_address = monitor_address + self._remove_monitor_sockfile = False + else: + self._monitor_address = os.path.join( + sock_dir, f"{name}-monitor.sock" + ) + self._remove_monitor_sockfile = True self._qemu_log_path = None self._qemu_log_file = None self._popen = None @@ -241,15 +249,17 @@ class QEMUMachine: def _base_args(self): args = ['-display', 'none', '-vga', 'none'] + if self._qmp_set: if isinstance(self._monitor_address, tuple): - moncdev = "socket,id=mon,host=%s,port=%s" % ( - self._monitor_address[0], - self._monitor_address[1]) + moncdev = "socket,id=mon,host={},port={}".format( + *self._monitor_address + ) else: - moncdev = 'socket,id=mon,path=%s' % self._vm_monitor + moncdev = f"socket,id=mon,path={self._monitor_address}" args.extend(['-chardev', moncdev, '-mon', 'chardev=mon,mode=control']) + if self._machine is not None: args.extend(['-machine', self._machine]) for _ in range(self._console_index): @@ -274,14 +284,14 @@ class QEMUMachine: self._qemu_log_file = open(self._qemu_log_path, 'wb') if self._qmp_set: - if self._monitor_address is not None: - self._vm_monitor = self._monitor_address - else: - self._vm_monitor = os.path.join(self._sock_dir, - self._name + "-monitor.sock") - self._remove_files.append(self._vm_monitor) - self._qmp = qmp.QEMUMonitorProtocol(self._vm_monitor, server=True, - nickname=self._name) + if self._remove_monitor_sockfile: + assert isinstance(self._monitor_address, str) + self._remove_files.append(self._monitor_address) + self._qmp = qmp.QEMUMonitorProtocol( + self._monitor_address, + server=True, + nickname=self._name + ) def _post_launch(self): if self._qmp: -- cgit 1.4.1 From c5e61a6da84397fe8c2af9e381d8a619a3e32c10 Mon Sep 17 00:00:00 2001 From: John Snow Date: Tue, 6 Oct 2020 19:58:00 -0400 Subject: python/machine.py: reorder __init__ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Put the init arg handling all at the top, and mostly in order (deviating when one is dependent on another), and put what is effectively runtime state declaration at the bottom. Signed-off-by: John Snow Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Kevin Wolf Message-id: 20201006235817.3280413-4-jsnow@redhat.com Signed-off-by: John Snow --- python/qemu/machine.py | 44 +++++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 19 deletions(-) (limited to 'python/qemu/machine.py') diff --git a/python/qemu/machine.py b/python/qemu/machine.py index 3017ec072d..71fe58be05 100644 --- a/python/qemu/machine.py +++ b/python/qemu/machine.py @@ -84,42 +84,54 @@ class QEMUMachine: @param monitor_address: address for QMP monitor @param socket_scm_helper: helper program, required for send_fd_scm() @param sock_dir: where to create socket (overrides test_dir for sock) - @param console_log: (optional) path to console log file @param drain_console: (optional) True to drain console socket to buffer + @param console_log: (optional) path to console log file @note: Qemu process is not started until launch() is used. ''' + # Direct user configuration + + self._binary = binary + if args is None: args = [] + # Copy mutable input: we will be modifying our copy + self._args = list(args) + if wrapper is None: wrapper = [] - if name is None: - name = "qemu-%d" % os.getpid() - if sock_dir is None: - sock_dir = test_dir - self._name = name + self._wrapper = wrapper + + self._name = name or "qemu-%d" % os.getpid() + self._test_dir = test_dir + self._sock_dir = sock_dir or self._test_dir + self._socket_scm_helper = socket_scm_helper + if monitor_address is not None: self._monitor_address = monitor_address self._remove_monitor_sockfile = False else: self._monitor_address = os.path.join( - sock_dir, f"{name}-monitor.sock" + self._sock_dir, f"{self._name}-monitor.sock" ) self._remove_monitor_sockfile = True + + self._console_log_path = console_log + if self._console_log_path: + # In order to log the console, buffering needs to be enabled. + self._drain_console = True + else: + self._drain_console = drain_console + + # Runstate self._qemu_log_path = None self._qemu_log_file = None self._popen = None - self._binary = binary - self._args = list(args) # Force copy args in case we modify them - self._wrapper = wrapper self._events = [] self._iolog = None - self._socket_scm_helper = socket_scm_helper self._qmp_set = True # Enable QMP monitor by default. self._qmp = None self._qemu_full_args = None - self._test_dir = test_dir self._temp_dir = None - self._sock_dir = sock_dir self._launched = False self._machine = None self._console_index = 0 @@ -129,12 +141,6 @@ class QEMUMachine: self._console_socket = None self._remove_files = [] self._user_killed = False - self._console_log_path = console_log - if self._console_log_path: - # In order to log the console, buffering needs to be enabled. - self._drain_console = True - else: - self._drain_console = drain_console def __enter__(self): return self -- cgit 1.4.1 From 652809dfa665860c1ea4622c540a30fbe18dc9e7 Mon Sep 17 00:00:00 2001 From: John Snow Date: Tue, 6 Oct 2020 19:58:01 -0400 Subject: python/machine.py: Don't modify state in _base_args() Don't append to the _remove_files list during _base_args; instead do so during _launch. Rework _base_args as a @property to help facilitate this impression. This has the additional benefit of making the type of _console_address easier to analyze statically. Signed-off-by: John Snow Reviewed-by: Kevin Wolf Message-id: 20201006235817.3280413-5-jsnow@redhat.com Signed-off-by: John Snow --- python/qemu/machine.py | 17 ++++++++++------- python/qemu/qtest.py | 7 ++++--- 2 files changed, 14 insertions(+), 10 deletions(-) (limited to 'python/qemu/machine.py') diff --git a/python/qemu/machine.py b/python/qemu/machine.py index 71fe58be05..812ca7d349 100644 --- a/python/qemu/machine.py +++ b/python/qemu/machine.py @@ -25,7 +25,7 @@ import signal import subprocess import tempfile from types import TracebackType -from typing import Optional, Type +from typing import List, Optional, Type from . import console_socket, qmp from .qmp import SocketAddrT @@ -137,7 +137,9 @@ class QEMUMachine: self._console_index = 0 self._console_set = False self._console_device_type = None - self._console_address = None + self._console_address = os.path.join( + self._sock_dir, f"{self._name}-console.sock" + ) self._console_socket = None self._remove_files = [] self._user_killed = False @@ -253,7 +255,8 @@ class QEMUMachine: with open(self._qemu_log_path, "r") as iolog: self._iolog = iolog.read() - def _base_args(self): + @property + def _base_args(self) -> List[str]: args = ['-display', 'none', '-vga', 'none'] if self._qmp_set: @@ -271,9 +274,6 @@ class QEMUMachine: for _ in range(self._console_index): args.extend(['-serial', 'null']) if self._console_set: - self._console_address = os.path.join(self._sock_dir, - self._name + "-console.sock") - self._remove_files.append(self._console_address) chardev = ('socket,id=console,path=%s,server,nowait' % self._console_address) args.extend(['-chardev', chardev]) @@ -289,6 +289,9 @@ class QEMUMachine: self._qemu_log_path = os.path.join(self._temp_dir, self._name + ".log") self._qemu_log_file = open(self._qemu_log_path, 'wb') + if self._console_set: + self._remove_files.append(self._console_address) + if self._qmp_set: if self._remove_monitor_sockfile: assert isinstance(self._monitor_address, str) @@ -374,7 +377,7 @@ class QEMUMachine: devnull = open(os.path.devnull, 'rb') self._pre_launch() self._qemu_full_args = (self._wrapper + [self._binary] + - self._base_args() + self._args) + self._base_args + self._args) LOG.debug('VM launch command: %r', ' '.join(self._qemu_full_args)) self._popen = subprocess.Popen(self._qemu_full_args, stdin=devnull, diff --git a/python/qemu/qtest.py b/python/qemu/qtest.py index 7700c0b09b..7fde2565a0 100644 --- a/python/qemu/qtest.py +++ b/python/qemu/qtest.py @@ -19,7 +19,7 @@ subclass of QEMUMachine, respectively. import os import socket -from typing import Optional, TextIO +from typing import List, Optional, TextIO from .machine import QEMUMachine @@ -111,8 +111,9 @@ class QEMUQtestMachine(QEMUMachine): self._qtest = None self._qtest_path = os.path.join(sock_dir, name + "-qtest.sock") - def _base_args(self): - args = super()._base_args() + @property + def _base_args(self) -> List[str]: + args = super()._base_args args.extend(['-qtest', 'unix:path=' + self._qtest_path, '-accel', 'qtest']) return args -- cgit 1.4.1 From 1847a4a8c209379cd7f6788c0dcd5c17c2bf0907 Mon Sep 17 00:00:00 2001 From: John Snow Date: Tue, 6 Oct 2020 19:58:02 -0400 Subject: python/machine.py: Handle None events in events_wait If the timeout is 0, we can get None back. Handle this explicitly. Signed-off-by: John Snow Reviewed-by: Kevin Wolf Message-id: 20201006235817.3280413-6-jsnow@redhat.com Signed-off-by: John Snow --- python/qemu/machine.py | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) (limited to 'python/qemu/machine.py') diff --git a/python/qemu/machine.py b/python/qemu/machine.py index 812ca7d349..aebfa09e9d 100644 --- a/python/qemu/machine.py +++ b/python/qemu/machine.py @@ -28,7 +28,7 @@ from types import TracebackType from typing import List, Optional, Type from . import console_socket, qmp -from .qmp import SocketAddrT +from .qmp import QMPMessage, SocketAddrT LOG = logging.getLogger(__name__) @@ -604,13 +604,20 @@ class QEMUMachine: def events_wait(self, events, timeout=60.0): """ - events_wait waits for and returns a named event - from QMP with a timeout. + events_wait waits for and returns a single named event from QMP. + In the case of multiple qualifying events, this function returns the + first one. - events: a sequence of (name, match_criteria) tuples. - The match criteria are optional and may be None. - See event_match for details. - timeout: QEMUMonitorProtocol.pull_event timeout parameter. + :param events: A sequence of (name, match_criteria) tuples. + The match criteria are optional and may be None. + See event_match for details. + :param timeout: Optional timeout, in seconds. + See QEMUMonitorProtocol.pull_event. + + :raise QMPTimeoutError: If timeout was non-zero and no matching events + were found. + :return: A QMP event matching the filter criteria. + If timeout was 0 and no event matched, None. """ def _match(event): for name, match in events: @@ -618,6 +625,8 @@ class QEMUMachine: return True return False + event: Optional[QMPMessage] + # Search cached events for event in self._events: if _match(event): @@ -627,6 +636,10 @@ class QEMUMachine: # Poll for new events while True: event = self._qmp.pull_event(wait=timeout) + if event is None: + # NB: None is only returned when timeout is false-ish. + # Timeouts raise QMPTimeoutError instead! + break if _match(event): return event self._events.append(event) -- cgit 1.4.1 From aaa81ec6093b2d45d5a318227db82fa71680a871 Mon Sep 17 00:00:00 2001 From: John Snow Date: Tue, 6 Oct 2020 19:58:03 -0400 Subject: python/machine.py: use qmp.command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit machine.py and qmp.py both do the same thing here; refactor machine.py to use qmp.py's functionality more directly. Signed-off-by: John Snow Reviewed-by: Kevin Wolf Reviewed-by: Philippe Mathieu-Daudé Message-id: 20201006235817.3280413-7-jsnow@redhat.com Signed-off-by: John Snow --- python/qemu/machine.py | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) (limited to 'python/qemu/machine.py') diff --git a/python/qemu/machine.py b/python/qemu/machine.py index aebfa09e9d..d788e8aba8 100644 --- a/python/qemu/machine.py +++ b/python/qemu/machine.py @@ -25,7 +25,13 @@ import signal import subprocess import tempfile from types import TracebackType -from typing import List, Optional, Type +from typing import ( + Any, + Dict, + List, + Optional, + Type, +) from . import console_socket, qmp from .qmp import QMPMessage, SocketAddrT @@ -515,17 +521,23 @@ class QEMUMachine: self._qmp_set = False self._qmp = None - def qmp(self, cmd, conv_keys=True, **args): - """ - Invoke a QMP command and return the response dict - """ + @classmethod + def _qmp_args(cls, _conv_keys: bool = True, **args: Any) -> Dict[str, Any]: qmp_args = dict() for key, value in args.items(): - if conv_keys: + if _conv_keys: qmp_args[key.replace('_', '-')] = value else: qmp_args[key] = value + return qmp_args + def qmp(self, cmd: str, + conv_keys: bool = True, + **args: Any) -> QMPMessage: + """ + Invoke a QMP command and return the response dict + """ + qmp_args = self._qmp_args(conv_keys, **args) return self._qmp.cmd(cmd, args=qmp_args) def command(self, cmd, conv_keys=True, **args): @@ -534,12 +546,8 @@ class QEMUMachine: On success return the response dict. On failure raise an exception. """ - reply = self.qmp(cmd, conv_keys, **args) - if reply is None: - raise qmp.QMPError("Monitor is closed") - if "error" in reply: - raise qmp.QMPResponseError(reply) - return reply["return"] + qmp_args = self._qmp_args(conv_keys, **args) + return self._qmp.command(cmd, **qmp_args) def get_qmp_event(self, wait=False): """ -- cgit 1.4.1 From be1183e52f49cd0f9c1a855afe4c4a5455f966d1 Mon Sep 17 00:00:00 2001 From: John Snow Date: Tue, 6 Oct 2020 19:58:04 -0400 Subject: python/machine.py: Add _qmp access shim Like many other Optional[] types, it's not always a given that this object will be set. Wrap it in a type-shim that raises a meaningful error and will always return a concrete type. Signed-off-by: John Snow Reviewed-by: Kevin Wolf Message-id: 20201006235817.3280413-8-jsnow@redhat.com Signed-off-by: John Snow --- python/qemu/machine.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) (limited to 'python/qemu/machine.py') diff --git a/python/qemu/machine.py b/python/qemu/machine.py index d788e8aba8..3e9cf09fd2 100644 --- a/python/qemu/machine.py +++ b/python/qemu/machine.py @@ -135,7 +135,7 @@ class QEMUMachine: self._events = [] self._iolog = None self._qmp_set = True # Enable QMP monitor by default. - self._qmp = None + self._qmp_connection: Optional[qmp.QEMUMonitorProtocol] = None self._qemu_full_args = None self._temp_dir = None self._launched = False @@ -302,14 +302,14 @@ class QEMUMachine: if self._remove_monitor_sockfile: assert isinstance(self._monitor_address, str) self._remove_files.append(self._monitor_address) - self._qmp = qmp.QEMUMonitorProtocol( + self._qmp_connection = qmp.QEMUMonitorProtocol( self._monitor_address, server=True, nickname=self._name ) def _post_launch(self): - if self._qmp: + if self._qmp_connection: self._qmp.accept() def _post_shutdown(self): @@ -320,9 +320,9 @@ class QEMUMachine: # Comprehensive reset for the failed launch case: self._early_cleanup() - if self._qmp: + if self._qmp_connection: self._qmp.close() - self._qmp = None + self._qmp_connection = None self._load_io_log() @@ -434,7 +434,7 @@ class QEMUMachine: """ self._early_cleanup() - if self._qmp is not None: + if self._qmp_connection: if not has_quit: # Might raise ConnectionReset self._qmp.cmd('quit') @@ -515,11 +515,13 @@ class QEMUMachine: line. Default is True. @note: call this function before launch(). """ - if enabled: - self._qmp_set = True - else: - self._qmp_set = False - self._qmp = None + self._qmp_set = enabled + + @property + def _qmp(self) -> qmp.QEMUMonitorProtocol: + if self._qmp_connection is None: + raise QEMUMachineError("Attempt to access QMP with no connection") + return self._qmp_connection @classmethod def _qmp_args(cls, _conv_keys: bool = True, **args: Any) -> Dict[str, Any]: -- cgit 1.4.1 From 9223fda464690b83a2b1f1487163f50602454c2e Mon Sep 17 00:00:00 2001 From: John Snow Date: Tue, 6 Oct 2020 19:58:05 -0400 Subject: python/machine.py: fix _popen access As always, Optional[T] causes problems with unchecked access. Add a helper that asserts the pipe is present before we attempt to talk with it. Signed-off-by: John Snow Reviewed-by: Kevin Wolf Message-id: 20201006235817.3280413-9-jsnow@redhat.com Signed-off-by: John Snow --- python/qemu/machine.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) (limited to 'python/qemu/machine.py') diff --git a/python/qemu/machine.py b/python/qemu/machine.py index 3e9cf09fd2..4e762fcd52 100644 --- a/python/qemu/machine.py +++ b/python/qemu/machine.py @@ -131,7 +131,7 @@ class QEMUMachine: # Runstate self._qemu_log_path = None self._qemu_log_file = None - self._popen = None + self._popen: Optional['subprocess.Popen[bytes]'] = None self._events = [] self._iolog = None self._qmp_set = True # Enable QMP monitor by default. @@ -244,6 +244,12 @@ class QEMUMachine: """Returns true if the VM is running.""" return self._popen is not None and self._popen.poll() is None + @property + def _subp(self) -> 'subprocess.Popen[bytes]': + if self._popen is None: + raise QEMUMachineError('Subprocess pipe not present') + return self._popen + def exitcode(self): """Returns the exit code if possible, or None.""" if self._popen is None: @@ -254,7 +260,7 @@ class QEMUMachine: """Returns the PID of the running process, or None.""" if not self.is_running(): return None - return self._popen.pid + return self._subp.pid def _load_io_log(self): if self._qemu_log_path is not None: @@ -415,8 +421,8 @@ class QEMUMachine: waiting for the QEMU process to terminate. """ self._early_cleanup() - self._popen.kill() - self._popen.wait(timeout=60) + self._subp.kill() + self._subp.wait(timeout=60) def _soft_shutdown(self, timeout: Optional[int], has_quit: bool = False) -> None: @@ -440,7 +446,7 @@ class QEMUMachine: self._qmp.cmd('quit') # May raise subprocess.TimeoutExpired - self._popen.wait(timeout=timeout) + self._subp.wait(timeout=timeout) def _do_shutdown(self, timeout: Optional[int], has_quit: bool = False) -> None: -- cgit 1.4.1 From aad3f3bb6c8b63b33db30911748fe32928b7b4bd Mon Sep 17 00:00:00 2001 From: John Snow Date: Tue, 6 Oct 2020 19:58:06 -0400 Subject: python/qemu: make 'args' style arguments immutable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These arguments don't need to be mutable and aren't really used as such. Clarify their types as immutable and adjust code to match where necessary. In general, It's probably best not to accept a user-defined mutable object and store it as internal object state unless there's a strong justification for doing so. Instead, try to use generic types as input with empty tuples as the default, and coerce to list where necessary. Signed-off-by: John Snow Reviewed-by: Kevin Wolf Reviewed-by: Philippe Mathieu-Daudé Message-id: 20201006235817.3280413-10-jsnow@redhat.com Signed-off-by: John Snow --- python/qemu/machine.py | 30 +++++++++++++++++------------- python/qemu/qtest.py | 22 +++++++++++++++++----- 2 files changed, 34 insertions(+), 18 deletions(-) (limited to 'python/qemu/machine.py') diff --git a/python/qemu/machine.py b/python/qemu/machine.py index 4e762fcd52..e599cb7439 100644 --- a/python/qemu/machine.py +++ b/python/qemu/machine.py @@ -18,6 +18,7 @@ which provides facilities for managing the lifetime of a QEMU VM. # import errno +from itertools import chain import logging import os import shutil @@ -30,6 +31,8 @@ from typing import ( Dict, List, Optional, + Sequence, + Tuple, Type, ) @@ -74,8 +77,12 @@ class QEMUMachine: # vm is guaranteed to be shut down here """ - def __init__(self, binary, args=None, wrapper=None, name=None, - test_dir="/var/tmp", + def __init__(self, + binary: str, + args: Sequence[str] = (), + wrapper: Sequence[str] = (), + name: Optional[str] = None, + test_dir: str = "/var/tmp", monitor_address: Optional[SocketAddrT] = None, socket_scm_helper=None, sock_dir=None, drain_console=False, console_log=None): @@ -97,14 +104,7 @@ class QEMUMachine: # Direct user configuration self._binary = binary - - if args is None: - args = [] - # Copy mutable input: we will be modifying our copy self._args = list(args) - - if wrapper is None: - wrapper = [] self._wrapper = wrapper self._name = name or "qemu-%d" % os.getpid() @@ -136,7 +136,7 @@ class QEMUMachine: self._iolog = None self._qmp_set = True # Enable QMP monitor by default. self._qmp_connection: Optional[qmp.QEMUMonitorProtocol] = None - self._qemu_full_args = None + self._qemu_full_args: Tuple[str, ...] = () self._temp_dir = None self._launched = False self._machine = None @@ -368,7 +368,7 @@ class QEMUMachine: raise QEMUMachineError('VM already launched') self._iolog = None - self._qemu_full_args = None + self._qemu_full_args = () try: self._launch() self._launched = True @@ -388,8 +388,12 @@ class QEMUMachine: """ devnull = open(os.path.devnull, 'rb') self._pre_launch() - self._qemu_full_args = (self._wrapper + [self._binary] + - self._base_args + self._args) + self._qemu_full_args = tuple( + chain(self._wrapper, + [self._binary], + self._base_args, + self._args) + ) LOG.debug('VM launch command: %r', ' '.join(self._qemu_full_args)) self._popen = subprocess.Popen(self._qemu_full_args, stdin=devnull, diff --git a/python/qemu/qtest.py b/python/qemu/qtest.py index 7fde2565a0..675310d8df 100644 --- a/python/qemu/qtest.py +++ b/python/qemu/qtest.py @@ -19,7 +19,12 @@ subclass of QEMUMachine, respectively. import os import socket -from typing import List, Optional, TextIO +from typing import ( + List, + Optional, + Sequence, + TextIO, +) from .machine import QEMUMachine @@ -99,8 +104,13 @@ class QEMUQtestMachine(QEMUMachine): A QEMU VM, with a qtest socket available. """ - def __init__(self, binary, args=None, name=None, test_dir="/var/tmp", - socket_scm_helper=None, sock_dir=None): + def __init__(self, + binary: str, + args: Sequence[str] = (), + name: Optional[str] = None, + test_dir: str = "/var/tmp", + socket_scm_helper: Optional[str] = None, + sock_dir: Optional[str] = None): if name is None: name = "qemu-%d" % os.getpid() if sock_dir is None: @@ -114,8 +124,10 @@ class QEMUQtestMachine(QEMUMachine): @property def _base_args(self) -> List[str]: args = super()._base_args - args.extend(['-qtest', 'unix:path=' + self._qtest_path, - '-accel', 'qtest']) + args.extend([ + '-qtest', f"unix:path={self._qtest_path}", + '-accel', 'qtest' + ]) return args def _pre_launch(self): -- cgit 1.4.1 From f12a282ff4728c8b66435eaddde589db41745beb Mon Sep 17 00:00:00 2001 From: John Snow Date: Tue, 6 Oct 2020 19:58:08 -0400 Subject: python/qemu: Add mypy type annotations These should all be purely annotations with no changes in behavior at all. You need to be in the python folder, but you should be able to confirm that these annotations are correct (or at least self-consistent) by running `mypy --strict qemu`. Signed-off-by: John Snow Reviewed-by: Kevin Wolf Message-id: 20201006235817.3280413-12-jsnow@redhat.com Signed-off-by: John Snow --- python/qemu/accel.py | 8 +++-- python/qemu/machine.py | 98 ++++++++++++++++++++++++++++---------------------- python/qemu/qmp.py | 44 +++++++++++++---------- python/qemu/qtest.py | 26 ++++++++------ 4 files changed, 101 insertions(+), 75 deletions(-) (limited to 'python/qemu/machine.py') diff --git a/python/qemu/accel.py b/python/qemu/accel.py index 3ec6bdcfdb..297933df2a 100644 --- a/python/qemu/accel.py +++ b/python/qemu/accel.py @@ -17,6 +17,7 @@ accelerators. import logging import os import subprocess +from typing import List, Optional LOG = logging.getLogger(__name__) @@ -30,7 +31,7 @@ ADDITIONAL_ARCHES = { } -def list_accel(qemu_bin): +def list_accel(qemu_bin: str) -> List[str]: """ List accelerators enabled in the QEMU binary. @@ -50,7 +51,8 @@ def list_accel(qemu_bin): return [acc.strip() for acc in out.splitlines()[1:]] -def kvm_available(target_arch=None, qemu_bin=None): +def kvm_available(target_arch: Optional[str] = None, + qemu_bin: Optional[str] = None) -> bool: """ Check if KVM is available using the following heuristic: - Kernel module is present in the host; @@ -73,7 +75,7 @@ def kvm_available(target_arch=None, qemu_bin=None): return True -def tcg_available(qemu_bin): +def tcg_available(qemu_bin: str) -> bool: """ Check if TCG is available. diff --git a/python/qemu/machine.py b/python/qemu/machine.py index e599cb7439..6420f01bed 100644 --- a/python/qemu/machine.py +++ b/python/qemu/machine.py @@ -23,11 +23,13 @@ import logging import os import shutil import signal +import socket import subprocess import tempfile from types import TracebackType from typing import ( Any, + BinaryIO, Dict, List, Optional, @@ -37,7 +39,7 @@ from typing import ( ) from . import console_socket, qmp -from .qmp import QMPMessage, SocketAddrT +from .qmp import QMPMessage, QMPReturnValue, SocketAddrT LOG = logging.getLogger(__name__) @@ -67,7 +69,7 @@ class AbnormalShutdown(QEMUMachineError): class QEMUMachine: """ - A QEMU VM + A QEMU VM. Use this object as a context manager to ensure the QEMU process terminates:: @@ -84,8 +86,10 @@ class QEMUMachine: name: Optional[str] = None, test_dir: str = "/var/tmp", monitor_address: Optional[SocketAddrT] = None, - socket_scm_helper=None, sock_dir=None, - drain_console=False, console_log=None): + socket_scm_helper: Optional[str] = None, + sock_dir: Optional[str] = None, + drain_console: bool = False, + console_log: Optional[str] = None): ''' Initialize a QEMUMachine @@ -129,28 +133,28 @@ class QEMUMachine: self._drain_console = drain_console # Runstate - self._qemu_log_path = None - self._qemu_log_file = None + self._qemu_log_path: Optional[str] = None + self._qemu_log_file: Optional[BinaryIO] = None self._popen: Optional['subprocess.Popen[bytes]'] = None - self._events = [] - self._iolog = None + self._events: List[QMPMessage] = [] + self._iolog: Optional[str] = None self._qmp_set = True # Enable QMP monitor by default. self._qmp_connection: Optional[qmp.QEMUMonitorProtocol] = None self._qemu_full_args: Tuple[str, ...] = () - self._temp_dir = None + self._temp_dir: Optional[str] = None self._launched = False - self._machine = None + self._machine: Optional[str] = None self._console_index = 0 self._console_set = False - self._console_device_type = None + self._console_device_type: Optional[str] = None self._console_address = os.path.join( self._sock_dir, f"{self._name}-console.sock" ) - self._console_socket = None - self._remove_files = [] + self._console_socket: Optional[socket.socket] = None + self._remove_files: List[str] = [] self._user_killed = False - def __enter__(self): + def __enter__(self) -> 'QEMUMachine': return self def __exit__(self, @@ -159,14 +163,15 @@ class QEMUMachine: exc_tb: Optional[TracebackType]) -> None: self.shutdown() - def add_monitor_null(self): + def add_monitor_null(self) -> None: """ This can be used to add an unused monitor instance. """ self._args.append('-monitor') self._args.append('null') - def add_fd(self, fd, fdset, opaque, opts=''): + def add_fd(self, fd: int, fdset: int, + opaque: str, opts: str = '') -> 'QEMUMachine': """ Pass a file descriptor to the VM """ @@ -185,7 +190,8 @@ class QEMUMachine: self._args.append(','.join(options)) return self - def send_fd_scm(self, fd=None, file_path=None): + def send_fd_scm(self, fd: Optional[int] = None, + file_path: Optional[str] = None) -> int: """ Send an fd or file_path to socket_scm_helper. @@ -229,7 +235,7 @@ class QEMUMachine: return proc.returncode @staticmethod - def _remove_if_exists(path): + def _remove_if_exists(path: str) -> None: """ Remove file object at path if it exists """ @@ -240,7 +246,7 @@ class QEMUMachine: return raise - def is_running(self): + def is_running(self) -> bool: """Returns true if the VM is running.""" return self._popen is not None and self._popen.poll() is None @@ -250,19 +256,19 @@ class QEMUMachine: raise QEMUMachineError('Subprocess pipe not present') return self._popen - def exitcode(self): + def exitcode(self) -> Optional[int]: """Returns the exit code if possible, or None.""" if self._popen is None: return None return self._popen.poll() - def get_pid(self): + def get_pid(self) -> Optional[int]: """Returns the PID of the running process, or None.""" if not self.is_running(): return None return self._subp.pid - def _load_io_log(self): + def _load_io_log(self) -> None: if self._qemu_log_path is not None: with open(self._qemu_log_path, "r") as iolog: self._iolog = iolog.read() @@ -296,7 +302,7 @@ class QEMUMachine: args.extend(['-device', device]) return args - def _pre_launch(self): + def _pre_launch(self) -> None: self._temp_dir = tempfile.mkdtemp(dir=self._test_dir) self._qemu_log_path = os.path.join(self._temp_dir, self._name + ".log") self._qemu_log_file = open(self._qemu_log_path, 'wb') @@ -314,11 +320,11 @@ class QEMUMachine: nickname=self._name ) - def _post_launch(self): + def _post_launch(self) -> None: if self._qmp_connection: self._qmp.accept() - def _post_shutdown(self): + def _post_shutdown(self) -> None: """ Called to cleanup the VM instance after the process has exited. May also be called after a failed launch. @@ -358,7 +364,7 @@ class QEMUMachine: self._user_killed = False self._launched = False - def launch(self): + def launch(self) -> None: """ Launch the VM and make sure we cleanup and expose the command line/output in case of exception @@ -382,7 +388,7 @@ class QEMUMachine: LOG.debug('Output: %r', self._iolog) raise - def _launch(self): + def _launch(self) -> None: """ Launch the VM and establish a QMP connection """ @@ -501,7 +507,7 @@ class QEMUMachine: finally: self._post_shutdown() - def kill(self): + def kill(self) -> None: """ Terminate the VM forcefully, wait for it to exit, and perform cleanup. """ @@ -516,7 +522,7 @@ class QEMUMachine: """ self.shutdown(has_quit=True, timeout=timeout) - def set_qmp_monitor(self, enabled=True): + def set_qmp_monitor(self, enabled: bool = True) -> None: """ Set the QMP monitor. @@ -552,7 +558,9 @@ class QEMUMachine: qmp_args = self._qmp_args(conv_keys, **args) return self._qmp.cmd(cmd, args=qmp_args) - def command(self, cmd, conv_keys=True, **args): + def command(self, cmd: str, + conv_keys: bool = True, + **args: Any) -> QMPReturnValue: """ Invoke a QMP command. On success return the response dict. @@ -561,7 +569,7 @@ class QEMUMachine: qmp_args = self._qmp_args(conv_keys, **args) return self._qmp.command(cmd, **qmp_args) - def get_qmp_event(self, wait=False): + def get_qmp_event(self, wait: bool = False) -> Optional[QMPMessage]: """ Poll for one queued QMP events and return it """ @@ -569,7 +577,7 @@ class QEMUMachine: return self._events.pop(0) return self._qmp.pull_event(wait=wait) - def get_qmp_events(self, wait=False): + def get_qmp_events(self, wait: bool = False) -> List[QMPMessage]: """ Poll for queued QMP events and return a list of dicts """ @@ -580,7 +588,7 @@ class QEMUMachine: return events @staticmethod - def event_match(event, match=None): + def event_match(event: Any, match: Optional[Any]) -> bool: """ Check if an event matches optional match criteria. @@ -610,9 +618,11 @@ class QEMUMachine: return True except TypeError: # either match or event wasn't iterable (not a dict) - return match == event + return bool(match == event) - def event_wait(self, name, timeout=60.0, match=None): + def event_wait(self, name: str, + timeout: float = 60.0, + match: Optional[QMPMessage] = None) -> Optional[QMPMessage]: """ event_wait waits for and returns a named event from QMP with a timeout. @@ -622,7 +632,9 @@ class QEMUMachine: """ return self.events_wait([(name, match)], timeout) - def events_wait(self, events, timeout=60.0): + def events_wait(self, + events: Sequence[Tuple[str, Any]], + timeout: float = 60.0) -> Optional[QMPMessage]: """ events_wait waits for and returns a single named event from QMP. In the case of multiple qualifying events, this function returns the @@ -639,7 +651,7 @@ class QEMUMachine: :return: A QMP event matching the filter criteria. If timeout was 0 and no event matched, None. """ - def _match(event): + def _match(event: QMPMessage) -> bool: for name, match in events: if event['event'] == name and self.event_match(event, match): return True @@ -666,20 +678,20 @@ class QEMUMachine: return None - def get_log(self): + def get_log(self) -> Optional[str]: """ After self.shutdown or failed qemu execution, this returns the output of the qemu process. """ return self._iolog - def add_args(self, *args): + def add_args(self, *args: str) -> None: """ Adds to the list of extra arguments to be given to the QEMU binary """ self._args.extend(args) - def set_machine(self, machine_type): + def set_machine(self, machine_type: str) -> None: """ Sets the machine type @@ -688,7 +700,9 @@ class QEMUMachine: """ self._machine = machine_type - def set_console(self, device_type=None, console_index=0): + def set_console(self, + device_type: Optional[str] = None, + console_index: int = 0) -> None: """ Sets the device type for a console device @@ -719,7 +733,7 @@ class QEMUMachine: self._console_index = console_index @property - def console_socket(self): + def console_socket(self) -> socket.socket: """ Returns a socket connected to the console """ diff --git a/python/qemu/qmp.py b/python/qemu/qmp.py index ddf8347ac1..9223307ed8 100644 --- a/python/qemu/qmp.py +++ b/python/qemu/qmp.py @@ -15,6 +15,7 @@ from types import TracebackType from typing import ( Any, Dict, + List, Optional, TextIO, Tuple, @@ -90,7 +91,9 @@ class QEMUMonitorProtocol: #: Logger object for debugging messages logger = logging.getLogger('QMP') - def __init__(self, address, server=False, nickname=None): + def __init__(self, address: SocketAddrT, + server: bool = False, + nickname: Optional[str] = None): """ Create a QEMUMonitorProtocol class. @@ -102,7 +105,7 @@ class QEMUMonitorProtocol: @note No connection is established, this is done by the connect() or accept() methods """ - self.__events = [] + self.__events: List[QMPMessage] = [] self.__address = address self.__sock = self.__get_sock() self.__sockfile: Optional[TextIO] = None @@ -114,14 +117,14 @@ class QEMUMonitorProtocol: self.__sock.bind(self.__address) self.__sock.listen(1) - def __get_sock(self): + def __get_sock(self) -> socket.socket: if isinstance(self.__address, tuple): family = socket.AF_INET else: family = socket.AF_UNIX return socket.socket(family, socket.SOCK_STREAM) - def __negotiate_capabilities(self): + def __negotiate_capabilities(self) -> QMPMessage: greeting = self.__json_read() if greeting is None or "QMP" not in greeting: raise QMPConnectError @@ -131,7 +134,7 @@ class QEMUMonitorProtocol: return greeting raise QMPCapabilitiesError - def __json_read(self, only_event=False): + def __json_read(self, only_event: bool = False) -> Optional[QMPMessage]: assert self.__sockfile is not None while True: data = self.__sockfile.readline() @@ -148,7 +151,7 @@ class QEMUMonitorProtocol: continue return resp - def __get_events(self, wait=False): + def __get_events(self, wait: Union[bool, float] = False) -> None: """ Check for new events in the stream and cache them in __events. @@ -186,7 +189,7 @@ class QEMUMonitorProtocol: raise QMPConnectError("Error while reading from socket") self.__sock.settimeout(None) - def __enter__(self): + def __enter__(self) -> 'QEMUMonitorProtocol': # Implement context manager enter function. return self @@ -199,7 +202,7 @@ class QEMUMonitorProtocol: # Implement context manager exit function. self.close() - def connect(self, negotiate=True): + def connect(self, negotiate: bool = True) -> Optional[QMPMessage]: """ Connect to the QMP Monitor and perform capabilities negotiation. @@ -214,7 +217,7 @@ class QEMUMonitorProtocol: return self.__negotiate_capabilities() return None - def accept(self, timeout=15.0): + def accept(self, timeout: float = 15.0) -> QMPMessage: """ Await connection from QMP Monitor and perform capabilities negotiation. @@ -250,7 +253,9 @@ class QEMUMonitorProtocol: self.logger.debug("<<< %s", resp) return resp - def cmd(self, name, args=None, cmd_id=None): + def cmd(self, name: str, + args: Optional[Dict[str, Any]] = None, + cmd_id: Optional[Any] = None) -> QMPMessage: """ Build a QMP command and send it to the QMP Monitor. @@ -258,14 +263,14 @@ class QEMUMonitorProtocol: @param args: command arguments (dict) @param cmd_id: command id (dict, list, string or int) """ - qmp_cmd = {'execute': name} + qmp_cmd: QMPMessage = {'execute': name} if args: qmp_cmd['arguments'] = args if cmd_id: qmp_cmd['id'] = cmd_id return self.cmd_obj(qmp_cmd) - def command(self, cmd, **kwds): + def command(self, cmd: str, **kwds: Any) -> QMPReturnValue: """ Build and send a QMP command to the monitor, report errors if any """ @@ -278,7 +283,8 @@ class QEMUMonitorProtocol: ) return cast(QMPReturnValue, ret['return']) - def pull_event(self, wait=False): + def pull_event(self, + wait: Union[bool, float] = False) -> Optional[QMPMessage]: """ Pulls a single event. @@ -298,7 +304,7 @@ class QEMUMonitorProtocol: return self.__events.pop(0) return None - def get_events(self, wait=False): + def get_events(self, wait: bool = False) -> List[QMPMessage]: """ Get a list of available QMP events. @@ -315,13 +321,13 @@ class QEMUMonitorProtocol: self.__get_events(wait) return self.__events - def clear_events(self): + def clear_events(self) -> None: """ Clear current list of pending events. """ self.__events = [] - def close(self): + def close(self) -> None: """ Close the socket and socket file. """ @@ -330,7 +336,7 @@ class QEMUMonitorProtocol: if self.__sockfile: self.__sockfile.close() - def settimeout(self, timeout): + def settimeout(self, timeout: float) -> None: """ Set the socket timeout. @@ -339,7 +345,7 @@ class QEMUMonitorProtocol: """ self.__sock.settimeout(timeout) - def get_sock_fd(self): + def get_sock_fd(self) -> int: """ Get the socket file descriptor. @@ -347,7 +353,7 @@ class QEMUMonitorProtocol: """ return self.__sock.fileno() - def is_scm_available(self): + def is_scm_available(self) -> bool: """ Check if the socket allows for SCM_RIGHTS. diff --git a/python/qemu/qtest.py b/python/qemu/qtest.py index 675310d8df..39a0cf62fe 100644 --- a/python/qemu/qtest.py +++ b/python/qemu/qtest.py @@ -27,6 +27,7 @@ from typing import ( ) from .machine import QEMUMachine +from .qmp import SocketAddrT class QEMUQtestProtocol: @@ -43,7 +44,8 @@ class QEMUQtestProtocol: No conection is estabalished by __init__(), this is done by the connect() or accept() methods. """ - def __init__(self, address, server=False): + def __init__(self, address: SocketAddrT, + server: bool = False): self._address = address self._sock = self._get_sock() self._sockfile: Optional[TextIO] = None @@ -51,14 +53,14 @@ class QEMUQtestProtocol: self._sock.bind(self._address) self._sock.listen(1) - def _get_sock(self): + def _get_sock(self) -> socket.socket: if isinstance(self._address, tuple): family = socket.AF_INET else: family = socket.AF_UNIX return socket.socket(family, socket.SOCK_STREAM) - def connect(self): + def connect(self) -> None: """ Connect to the qtest socket. @@ -67,7 +69,7 @@ class QEMUQtestProtocol: self._sock.connect(self._address) self._sockfile = self._sock.makefile(mode='r') - def accept(self): + def accept(self) -> None: """ Await connection from QEMU. @@ -76,7 +78,7 @@ class QEMUQtestProtocol: self._sock, _ = self._sock.accept() self._sockfile = self._sock.makefile(mode='r') - def cmd(self, qtest_cmd): + def cmd(self, qtest_cmd: str) -> str: """ Send a qtest command on the wire. @@ -87,14 +89,16 @@ class QEMUQtestProtocol: resp = self._sockfile.readline() return resp - def close(self): - """Close this socket.""" + def close(self) -> None: + """ + Close this socket. + """ self._sock.close() if self._sockfile: self._sockfile.close() self._sockfile = None - def settimeout(self, timeout): + def settimeout(self, timeout: Optional[float]) -> None: """Set a timeout, in seconds.""" self._sock.settimeout(timeout) @@ -118,7 +122,7 @@ class QEMUQtestMachine(QEMUMachine): super().__init__(binary, args, name=name, test_dir=test_dir, socket_scm_helper=socket_scm_helper, sock_dir=sock_dir) - self._qtest = None + self._qtest: Optional[QEMUQtestProtocol] = None self._qtest_path = os.path.join(sock_dir, name + "-qtest.sock") @property @@ -130,7 +134,7 @@ class QEMUQtestMachine(QEMUMachine): ]) return args - def _pre_launch(self): + def _pre_launch(self) -> None: super()._pre_launch() self._qtest = QEMUQtestProtocol(self._qtest_path, server=True) @@ -139,7 +143,7 @@ class QEMUQtestMachine(QEMUMachine): super()._post_launch() self._qtest.accept() - def _post_shutdown(self): + def _post_shutdown(self) -> None: super()._post_shutdown() self._remove_if_exists(self._qtest_path) -- cgit 1.4.1