From 439125293cc9cfb684eb4db23db04199f5f435a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Fri, 28 Jan 2022 16:11:56 +0000 Subject: [PATCH 1/6] python: introduce qmp-shell-wrap convenience tool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With the current 'qmp-shell' tool developers must first spawn QEMU with a suitable -qmp arg and then spawn qmp-shell in a separate terminal pointing to the right socket. With 'qmp-shell-wrap' developers can ignore QMP sockets entirely and just pass the QEMU command and arguments they want. The program will listen on a UNIX socket and tell QEMU to connect QMP to that. For example, this: # qmp-shell-wrap -- qemu-system-x86_64 -display none Is roughly equivalent of running: # qemu-system-x86_64 -display none -qmp qmp-shell-1234 & # qmp-shell qmp-shell-1234 Except that 'qmp-shell-wrap' switches the socket peers around so that it is the UNIX socket server and QEMU is the socket client. This makes QEMU reliably go away when qmp-shell-wrap exits, closing the server socket. Signed-off-by: Daniel P. Berrangé Message-id: 20220128161157.36261-2-berrange@redhat.com [Edited for rebase. --js] Signed-off-by: John Snow --- python/qemu/aqmp/qmp_shell.py | 65 ++++++++++++++++++++++++++++++++--- python/setup.cfg | 1 + scripts/qmp/qmp-shell-wrap | 11 ++++++ 3 files changed, 73 insertions(+), 4 deletions(-) create mode 100755 scripts/qmp/qmp-shell-wrap diff --git a/python/qemu/aqmp/qmp_shell.py b/python/qemu/aqmp/qmp_shell.py index d11bf54b00..c60df787fc 100644 --- a/python/qemu/aqmp/qmp_shell.py +++ b/python/qemu/aqmp/qmp_shell.py @@ -86,6 +86,7 @@ import logging import os import re import readline +from subprocess import Popen import sys from typing import ( Iterator, @@ -167,8 +168,10 @@ class QMPShell(QEMUMonitorProtocol): :param verbose: Echo outgoing QMP messages to console. """ def __init__(self, address: SocketAddrT, - pretty: bool = False, verbose: bool = False): - super().__init__(address) + pretty: bool = False, + verbose: bool = False, + server: bool = False): + super().__init__(address, server=server) self._greeting: Optional[QMPMessage] = None self._completer = QMPCompleter() self._transmode = False @@ -409,8 +412,10 @@ class HMPShell(QMPShell): :param verbose: Echo outgoing QMP messages to console. """ def __init__(self, address: SocketAddrT, - pretty: bool = False, verbose: bool = False): - super().__init__(address, pretty, verbose) + pretty: bool = False, + verbose: bool = False, + server: bool = False): + super().__init__(address, pretty, verbose, server) self._cpu_index = 0 def _cmd_completion(self) -> None: @@ -533,5 +538,57 @@ def main() -> None: pass +def main_wrap() -> None: + """ + qmp-shell-wrap entry point: parse command line arguments and + start the REPL. + """ + parser = argparse.ArgumentParser() + parser.add_argument('-H', '--hmp', action='store_true', + help='Use HMP interface') + parser.add_argument('-v', '--verbose', action='store_true', + help='Verbose (echo commands sent and received)') + parser.add_argument('-p', '--pretty', action='store_true', + help='Pretty-print JSON') + + parser.add_argument('command', nargs=argparse.REMAINDER, + help='QEMU command line to invoke') + + args = parser.parse_args() + + cmd = args.command + if len(cmd) != 0 and cmd[0] == '--': + cmd = cmd[1:] + if len(cmd) == 0: + cmd = ["qemu-system-x86_64"] + + sockpath = "qmp-shell-wrap-%d" % os.getpid() + cmd += ["-qmp", "unix:%s" % sockpath] + + shell_class = HMPShell if args.hmp else QMPShell + + try: + address = shell_class.parse_address(sockpath) + except QMPBadPortError: + parser.error(f"Bad port number: {sockpath}") + return # pycharm doesn't know error() is noreturn + + try: + with shell_class(address, args.pretty, args.verbose, True) as qemu: + with Popen(cmd): + + try: + qemu.accept() + 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 + finally: + os.unlink(sockpath) + + if __name__ == '__main__': main() diff --git a/python/setup.cfg b/python/setup.cfg index 18aea2bab3..0959603238 100644 --- a/python/setup.cfg +++ b/python/setup.cfg @@ -68,6 +68,7 @@ console_scripts = qom-fuse = qemu.utils.qom_fuse:QOMFuse.entry_point [fuse] qemu-ga-client = qemu.utils.qemu_ga_client:main qmp-shell = qemu.aqmp.qmp_shell:main + qmp-shell-wrap = qemu.aqmp.qmp_shell:main_wrap aqmp-tui = qemu.aqmp.aqmp_tui:main [tui] [flake8] diff --git a/scripts/qmp/qmp-shell-wrap b/scripts/qmp/qmp-shell-wrap new file mode 100755 index 0000000000..9e94da114f --- /dev/null +++ b/scripts/qmp/qmp-shell-wrap @@ -0,0 +1,11 @@ +#!/usr/bin/env python3 + +import os +import sys + +sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python')) +from qemu.qmp import qmp_shell + + +if __name__ == '__main__': + qmp_shell.main_wrap() From 5c66d7d8de9a00460199669d26cd83fba135bee5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Fri, 28 Jan 2022 16:11:57 +0000 Subject: [PATCH 2/6] python: support recording QMP session to a file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When running QMP commands with very large response payloads, it is often not easy to spot the info you want. If we can save the response to a file then tools like 'grep' or 'jq' can be used to extract information. For convenience of processing, we merge the QMP command and response dictionaries together: { "arguments": {}, "execute": "query-kvm", "return": { "enabled": false, "present": true } } Example usage $ ./scripts/qmp/qmp-shell-wrap -l q.log -p -- ./build/qemu-system-x86_64 -display none Welcome to the QMP low-level shell! Connected (QEMU) query-kvm { "return": { "enabled": false, "present": true } } (QEMU) query-mice { "return": [ { "absolute": false, "current": true, "index": 2, "name": "QEMU PS/2 Mouse" } ] } $ jq --slurp '. | to_entries[] | select(.value.execute == "query-kvm") | .value.return.enabled' < q.log false Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Daniel P. Berrangé Message-id: 20220128161157.36261-3-berrange@redhat.com Signed-off-by: John Snow --- python/qemu/aqmp/qmp_shell.py | 29 ++++++++++++++++++++++------- python/setup.cfg | 3 +++ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/python/qemu/aqmp/qmp_shell.py b/python/qemu/aqmp/qmp_shell.py index c60df787fc..35691494d0 100644 --- a/python/qemu/aqmp/qmp_shell.py +++ b/python/qemu/aqmp/qmp_shell.py @@ -89,6 +89,7 @@ import readline from subprocess import Popen import sys from typing import ( + IO, Iterator, List, NoReturn, @@ -170,7 +171,8 @@ class QMPShell(QEMUMonitorProtocol): def __init__(self, address: SocketAddrT, pretty: bool = False, verbose: bool = False, - server: bool = False): + server: bool = False, + logfile: Optional[str] = None): super().__init__(address, server=server) self._greeting: Optional[QMPMessage] = None self._completer = QMPCompleter() @@ -180,6 +182,10 @@ class QMPShell(QEMUMonitorProtocol): '.qmp-shell_history') self.pretty = pretty self.verbose = verbose + self.logfile = None + + if logfile is not None: + self.logfile = open(logfile, "w", encoding='utf-8') def close(self) -> None: # Hook into context manager of parent to save shell history. @@ -320,11 +326,11 @@ class QMPShell(QEMUMonitorProtocol): self._cli_expr(cmdargs[1:], qmpcmd['arguments']) return qmpcmd - def _print(self, qmp_message: object) -> None: + def _print(self, qmp_message: object, fh: IO[str] = sys.stdout) -> None: jsobj = json.dumps(qmp_message, indent=4 if self.pretty else None, sort_keys=self.pretty) - print(str(jsobj)) + print(str(jsobj), file=fh) def _execute_cmd(self, cmdline: str) -> bool: try: @@ -347,6 +353,9 @@ class QMPShell(QEMUMonitorProtocol): print('Disconnected') return False self._print(resp) + if self.logfile is not None: + cmd = {**qmpcmd, **resp} + self._print(cmd, fh=self.logfile) return True def connect(self, negotiate: bool = True) -> None: @@ -414,8 +423,9 @@ class HMPShell(QMPShell): def __init__(self, address: SocketAddrT, pretty: bool = False, verbose: bool = False, - server: bool = False): - super().__init__(address, pretty, verbose, server) + server: bool = False, + logfile: Optional[str] = None): + super().__init__(address, pretty, verbose, server, logfile) self._cpu_index = 0 def _cmd_completion(self) -> None: @@ -508,6 +518,8 @@ def main() -> None: help='Verbose (echo commands sent and received)') parser.add_argument('-p', '--pretty', action='store_true', help='Pretty-print JSON') + parser.add_argument('-l', '--logfile', + help='Save log of all QMP messages to PATH') default_server = os.environ.get('QMP_SOCKET') parser.add_argument('qmp_server', action='store', @@ -526,7 +538,7 @@ def main() -> None: 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: + with shell_class(address, args.pretty, args.verbose, args.logfile) as qemu: try: qemu.connect(negotiate=not args.skip_negotiation) except ConnectError as err: @@ -550,6 +562,8 @@ def main_wrap() -> None: help='Verbose (echo commands sent and received)') parser.add_argument('-p', '--pretty', action='store_true', help='Pretty-print JSON') + parser.add_argument('-l', '--logfile', + help='Save log of all QMP messages to PATH') parser.add_argument('command', nargs=argparse.REMAINDER, help='QEMU command line to invoke') @@ -574,7 +588,8 @@ def main_wrap() -> None: return # pycharm doesn't know error() is noreturn try: - with shell_class(address, args.pretty, args.verbose, True) as qemu: + with shell_class(address, args.pretty, args.verbose, + True, args.logfile) as qemu: with Popen(cmd): try: diff --git a/python/setup.cfg b/python/setup.cfg index 0959603238..9821db9880 100644 --- a/python/setup.cfg +++ b/python/setup.cfg @@ -114,7 +114,10 @@ ignore_missing_imports = True # no Warning level messages displayed, use "--disable=all --enable=classes # --disable=W". disable=consider-using-f-string, + consider-using-with, + too-many-arguments, too-many-function-args, # mypy handles this with less false positives. + too-many-instance-attributes, no-member, # mypy also handles this better. [pylint.basic] From 2ddaeb7b090ecec0debbb73bdfb18a0d9080f56d Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 7 Feb 2022 16:30:39 -0500 Subject: [PATCH 3/6] Python: discourage direct setup.py install MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When invoking setup.py directly, the default behavior for 'install' is to run the bdist_egg installation hook, which is ... actually deprecated by setuptools. It doesn't seem to work quite right anymore. By contrast, 'pip install' will invoke the bdist_wheel hook instead. This leads to differences in behavior for the two approaches. I advocate using pip in the documentation in this directory, but the 'setup.py' which has been used for quite a long time in the Python world may deceptively appear to work at first glance. Add an error message that will save a bit of time and frustration that points the user towards using the supported installation invocation. Reported-by: Daniel P. Berrangé Signed-off-by: John Snow Reviewed-by: Beraldo Leal Message-id: 20220207213039.2278569-1-jsnow@redhat.com Signed-off-by: John Snow --- python/setup.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/python/setup.py b/python/setup.py index 2014f81b75..c5bc45919a 100755 --- a/python/setup.py +++ b/python/setup.py @@ -5,9 +5,26 @@ Copyright (c) 2020-2021 John Snow for Red Hat, Inc. """ import setuptools +from setuptools.command import bdist_egg +import sys import pkg_resources +class bdist_egg_guard(bdist_egg.bdist_egg): + """ + Protect against bdist_egg from being executed + + This prevents calling 'setup.py install' directly, as the 'install' + CLI option will invoke the deprecated bdist_egg hook. "pip install" + calls the more modern bdist_wheel hook, which is what we want. + """ + def run(self): + sys.exit( + 'Installation directly via setup.py is not supported.\n' + 'Please use `pip install .` instead.' + ) + + def main(): """ QEMU tooling installer @@ -16,7 +33,7 @@ def main(): # https://medium.com/@daveshawley/safely-using-setup-cfg-for-metadata-1babbe54c108 pkg_resources.require('setuptools>=39.2') - setuptools.setup() + setuptools.setup(cmdclass={'bdist_egg': bdist_egg_guard}) if __name__ == '__main__': From 762c280d5f3c17a239204a73855d8778f6dc2113 Mon Sep 17 00:00:00 2001 From: John Snow Date: Fri, 4 Feb 2022 17:18:03 -0500 Subject: [PATCH 4/6] Python: add setuptools v60.0 workaround Setuptools v60 and later include a bundled version of distutils, a deprecated standard library scheduled for removal in future versions of Python. Setuptools v60 is only possible to install for Python 3.7 and later. Python has a distutils.sysconfig.get_python_lib() function that returns '/usr/lib/pythonX.Y' on posix systems. RPM-based systems actually use '/usr/lib64/pythonX.Y' instead, so Fedora patches stdlib distutils for Python 3.7 and Python 3.8 to return the correct value. Python 3.9 and later introduce a sys.platlibdir property, which returns the correct value on RPM-based systems. The change to a distutils package not provided by Fedora on Python 3.7 and 3.8 causes a regression in distutils.sysconfig.get_python_lib() that ultimately causes false positives to be emitted by pylint, because it can no longer find the system source libraries. Many Python tools are fairly aggressive about updating setuptools packages, and so even though this package is a fair bit newer than Python 3.7/3.8, it's not entirely unreasonable for a given user to have such a modern package with a fairly old Python interpreter. Updates to Python 3.7 and Python 3.8 are being produced for Fedora which will fix the problem on up-to-date systems. Until then, we can force the loading of platform-provided distutils when running the pylint test. This is the least-invasive yet most comprehensive fix. References: https://github.com/pypa/setuptools/pull/2896 https://github.com/PyCQA/pylint/issues/5704 https://github.com/pypa/distutils/issues/110 Signed-off-by: John Snow Message-id: 20220204221804.2047468-2-jsnow@redhat.com Signed-off-by: John Snow --- python/tests/iotests-pylint.sh | 3 ++- python/tests/pylint.sh | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/python/tests/iotests-pylint.sh b/python/tests/iotests-pylint.sh index 4cae03424b..33c5ae900a 100755 --- a/python/tests/iotests-pylint.sh +++ b/python/tests/iotests-pylint.sh @@ -1,4 +1,5 @@ #!/bin/sh -e cd ../tests/qemu-iotests/ -python3 -m linters --pylint +# See commit message for environment variable explainer. +SETUPTOOLS_USE_DISTUTILS=stdlib python3 -m linters --pylint diff --git a/python/tests/pylint.sh b/python/tests/pylint.sh index 4b10b34db7..03d64705a1 100755 --- a/python/tests/pylint.sh +++ b/python/tests/pylint.sh @@ -1,2 +1,3 @@ #!/bin/sh -e -python3 -m pylint qemu/ +# See commit message for environment variable explainer. +SETUPTOOLS_USE_DISTUTILS=stdlib python3 -m pylint qemu/ From 43a1119ef132ce23624046babbd08d9d0708dc46 Mon Sep 17 00:00:00 2001 From: John Snow Date: Fri, 4 Feb 2022 17:18:04 -0500 Subject: [PATCH 5/6] Revert "python: pin setuptools below v60.0.0" This reverts commit 1e4d8b31be35e54b6429fea54f5ecaa0083f91e7. Signed-off-by: John Snow Message-id: 20220204221804.2047468-3-jsnow@redhat.com Signed-off-by: John Snow --- python/Makefile | 2 -- python/setup.cfg | 1 - 2 files changed, 3 deletions(-) diff --git a/python/Makefile b/python/Makefile index 949c472624..3334311362 100644 --- a/python/Makefile +++ b/python/Makefile @@ -68,8 +68,6 @@ $(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) diff --git a/python/setup.cfg b/python/setup.cfg index 9821db9880..241f243e8b 100644 --- a/python/setup.cfg +++ b/python/setup.cfg @@ -167,7 +167,6 @@ deps = .[devel] .[fuse] # Workaround to trigger tox venv rebuild .[tui] # Workaround to trigger tox venv rebuild - setuptools < 60 # Workaround, please see commit msg. commands = make check From 89d38c74f4b69a93696392b55a9fee573055d78b Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 7 Feb 2022 19:05:25 -0500 Subject: [PATCH 6/6] MAINTAINERS: python - remove ehabkost and add bleal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eduardo Habkost has left Red Hat and has other daily responsibilities to attend to. In order to stop spamming him on every series, remove him as "Reviewer" for the python/ library dir and add Beraldo Leal instead. For the "python scripts" stanza (which is separate due to level of support), replace Eduardo as maintainer with myself. (Thanks for all of your hard work, Eduardo!) Signed-off-by: John Snow Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Beraldo Leal Acked-by: Eduardo Habkost Message-id: 20220208000525.2601011-1-jsnow@redhat.com Signed-off-by: John Snow --- MAINTAINERS | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index c3b500345c..62bc185d10 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2747,13 +2747,13 @@ F: backends/cryptodev*.c Python library M: John Snow M: Cleber Rosa -R: Eduardo Habkost +R: Beraldo Leal S: Maintained F: python/ T: git https://gitlab.com/jsnow/qemu.git python Python scripts -M: Eduardo Habkost +M: John Snow M: Cleber Rosa S: Odd Fixes F: scripts/*.py