[PULL 0/6] Python patches

[PULL 0/6] Python patches

[John Snow]

The following changes since commit 31e3caf21b6cdf54d11f3744b8b341f07a30b5d7:

  Merge remote-tracking branch 'remotes/lvivier-gitlab/tags/trivial-branch-for-7.0-pull-request' into staging (2022-02-22 20:17:09 +0000)

are available in the Git repository at:

  https://gitlab.com/jsnow/qemu.git tags/python-pull-request

for you to fetch changes up to 89d38c74f4b69a93696392b55a9fee573055d78b:

  MAINTAINERS: python - remove ehabkost and add bleal (2022-02-23 17:07:26 -0500)

----------------------------------------------------------------
Python patches

New functionality in qmp-shell from Dan, and some packaging fixes.

----------------------------------------------------------------

Daniel P. Berrangé (2):
  python: introduce qmp-shell-wrap convenience tool
  python: support recording QMP session to a file

John Snow (4):
  Python: discourage direct setup.py install
  Python: add setuptools v60.0 workaround
  Revert "python: pin setuptools below v60.0.0"
  MAINTAINERS: python - remove ehabkost and add bleal

 MAINTAINERS                    |  4 +-
 python/Makefile                |  2 -
 python/qemu/aqmp/qmp_shell.py  | 86 +++++++++++++++++++++++++++++++---
 python/setup.cfg               |  5 +-
 python/setup.py                | 19 +++++++-
 python/tests/iotests-pylint.sh |  3 +-
 python/tests/pylint.sh         |  3 +-
 scripts/qmp/qmp-shell-wrap     | 11 +++++
 8 files changed, 118 insertions(+), 15 deletions(-)
 create mode 100755 scripts/qmp/qmp-shell-wrap

-- 
2.34.1

[PULL 1/6] python: introduce qmp-shell-wrap convenience tool

[John Snow]

From: Daniel P. Berrangé <berrange@redhat.com>

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é <berrange@redhat.com>
Message-id: 20220128161157.36261-2-berrange@redhat.com
[Edited for rebase. --js]
Signed-off-by: John Snow <jsnow@redhat.com>
---
 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 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()
-- 
2.34.1

[PULL 2/6] python: support recording QMP session to a file

[John Snow]

From: Daniel P. Berrangé <berrange@redhat.com>

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é <f4bug@amsat.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-id: 20220128161157.36261-3-berrange@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 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 @@
 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 @@ def __init__(self, address: SocketAddrT,
                                       '.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 @@ def _build_cmd(self, cmdline: str) -> Optional[QMPMessage]:
         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 @@ def _execute_cmd(self, cmdline: str) -> bool:
             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]
-- 
2.34.1

[PULL 3/6] Python: discourage direct setup.py install

[John Snow]

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é <berrange@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Beraldo Leal <bleal@redhat.com>
Message-id: 20220207213039.2278569-1-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 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 @@
 """
 
 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__':
-- 
2.34.1

[PULL 4/6] Python: add setuptools v60.0 workaround

[John Snow]

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 <jsnow@redhat.com>
Message-id: 20220204221804.2047468-2-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 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/
-- 
2.34.1

[PULL 5/6] Revert "python: pin setuptools below v60.0.0"

[John Snow]

This reverts commit 1e4d8b31be35e54b6429fea54f5ecaa0083f91e7.

Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 20220204221804.2047468-3-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 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
 
-- 
2.34.1

[PULL 6/6] MAINTAINERS: python - remove ehabkost and add bleal

[John Snow]

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 <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Beraldo Leal <bleal@redhat.com>
Acked-by: Eduardo Habkost <eduardo@habkost.net>
Message-id: 20220208000525.2601011-1-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 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 <jsnow@redhat.com>
 M: Cleber Rosa <crosa@redhat.com>
-R: Eduardo Habkost <eduardo@habkost.net>
+R: Beraldo Leal <bleal@redhat.com>
 S: Maintained
 F: python/
 T: git https://gitlab.com/jsnow/qemu.git python
 
 Python scripts
-M: Eduardo Habkost <eduardo@habkost.net>
+M: John Snow <jsnow@redhat.com>
 M: Cleber Rosa <crosa@redhat.com>
 S: Odd Fixes
 F: scripts/*.py
-- 
2.34.1

Re: [PULL 0/6] Python patches

[Peter Maydell]

On Wed, 23 Feb 2022 at 22:09, John Snow <jsnow@redhat.com> wrote:
>
> The following changes since commit 31e3caf21b6cdf54d11f3744b8b341f07a30b5d7:
>
>   Merge remote-tracking branch 'remotes/lvivier-gitlab/tags/trivial-branch-for-7.0-pull-request' into staging (2022-02-22 20:17:09 +0000)
>
> are available in the Git repository at:
>
>   https://gitlab.com/jsnow/qemu.git tags/python-pull-request
>
> for you to fetch changes up to 89d38c74f4b69a93696392b55a9fee573055d78b:
>
>   MAINTAINERS: python - remove ehabkost and add bleal (2022-02-23 17:07:26 -0500)
>
> ----------------------------------------------------------------
> Python patches
>
> New functionality in qmp-shell from Dan, and some packaging fixes.
>
> ----------------------------------------------------------------


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/7.0
for any user-visible changes.

-- PMM