[PATCH v2 00/72] python: move scripts/qmp to python/qemu/qmp

[PATCH v2 00/72] python: move scripts/qmp to python/qemu/qmp

[John Snow]

Based-on: <20201020193555.1493936-1-jsnow@redhat.com>
          [PATCH v3 00/15] python: create installable package

Hi! This series looks big, but don't panic. Like many python refactoring
series, most of the individual patches are very tiny, isolated changes.

This series depends on the details of the installable package series,
so don't sweat too hard reviewing the particulars just yet, or at all.
I did want to showcase the process of moving scripts INTO the python
library, though, so here it is.

The purpose of this series is to move ./scripts/qmp/* to
./python/qemu/qmp/*.py.  To do so, we need to ensure that these files
pass the various linters and code quality standards we have instituted
for the python library. Doing so allows us to check these tools with the
same type checking utilities that we use to enforce integrity in the
base library.

This way, changes to the library will cause integration tests to fail,
and we can ensure the continued stability of these scripts.

Nicest of all, by installing this python package to your working
environment, you will now have access to 'qom-set', 'qom-get',
'qmp-shell', etc no matter what your working directory is, and it will
Just Work.

Let's run down the list:

- qom-set, qom-get, qom-tree, and qom-list are merged into a 'qom' tool.
- qom-[set|get|tree|list] are still available under those names, too.
- qemu-ga-client is lightly refurbished and is also now available.
- qmp-shell is lightly polished and will be available.

Patches 1-3: Introduce 'qom' tool, deprecate qom-[set|get|tree|list]
Patches 4-16: port qom-fuse to qemu.qmp.qom tool
Patches 17-27: port qemu-ga-client to qemu.qmp package
Patches 28-72: port qmp-shell to qemu.qmp package

Reviewer notes:

- I just rewrote qom-xxx entirely, though it is based on the original
  scripts. Doing it brick by brick was too slow and awkward.

- I added symlinks (not in git) to the old scripts in the new location
  to run the linters against the files that haven't been moved yet, and
  once they're almost done (except for import problems), I move them.

- After moving scripts, I add a forwarder to the new location from the
  old location. The forwarders can be deleted eventually.

- some scripts disappear for a commit or two before being
  re-established. This helps preserve git-blame history where it
  happens; it was the best I could do.

John Snow (72):
  python/qmp: Add qom script rewrites
  python/qmp: add qom script entry points
  scripts/qmp: redirect qom-xxx scripts to python/qemu/qmp/
  scripts/qom-fuse: apply isort rules
  scripts/qom-fuse: apply flake8 rules
  python: Add 'fh' to known-good variable names
  scripts/qom-fuse: Apply pylint rules
  scripts/qom-fuse: Add docstrings
  scripts/qom-fuse: Convert to QOMCommand
  scripts/qom-fuse: use QOMCommand.qom_list()
  scripts/qom-fuse: ensure QOMFuse.read always returns bytes
  scripts/qom-fuse: add static type hints
  scripts/qom-fuse: move to python/qemu/qmp/qom_fuse.py
  scripts/qom-fuse: add redirection shim to python/qemu/qmp/qom-fuse.py
  python: add fuse command to 'qom' tools
  python: add optional fuse dependency
  scripts/qemu-ga-client: apply isort rules
  scripts/qemu-ga-client: apply (most) flake8 rules
  scripts/qemu-ga-client: Fix exception handling
  scripts/qemu-ga-client: replace deprecated optparse with argparse
  scripts/qemu-ga-client: add module docstring
  scripts/qemu-ga-client: apply (most) pylint rules
  python/qmp: Correct type of QMPReturnValue
  scripts/qmp/qemu-ga-client: add mypy type hints
  scripts/qemu-ga-client: move to python/qemu/qmp/qemu_ga_client.py
  python/qemu-ga-client: add entry point
  scripts/qemu-ga-client: Add forwarder stub
  scripts/qmp-shell: apply isort rules
  scripts/qmp-shell: Apply flake8 rules
  scripts/qmp-shell: fix show_banner signature
  scripts/qmp-shell: fix exception handling
  scripts/qmp-shell: fix connect method signature
  scripts/qmp-shell: remove shadowed variable from _print()
  scripts/qmp-shell: use @classmethod where appropriate
  scripts/qmp-shell: Use python3-style super()
  scripts/qmp-shell: declare verbose in __init__
  scripts/qmp-shell: use triple-double-quote docstring style
  scripts/qmp-shell: ignore visit_Name name
  scripts/qmp-shell: make QMPCompleter returns explicit
  scripts/qmp-shell: rename one and two-letter variables
  scripts/qmp-shell: fix shell history exception handling
  scripts/qmp-shell: explicitly chain exception context
  scripts/qmp-shell: remove if-raise-else patterns
  scripts/qmp-shell: use isinstance() instead of type()
  scripts/qmp-shell: use argparse
  python/qmp: Fix type of SocketAddrT
  python/qmp: add parse_address classmethod
  scripts/qmp-shell: Add pretty attribute to HMP shell
  scripts/qmp-shell: Make verbose a public attribute
  scripts/qmp-shell: move get_prompt() to prompt property
  scripts/qmp-shell: remove prompt argument from read_exec_command
  scripts/qmp-shell: move the REPL functionality into QMPShell
  scripts/qmp-shell: Fix "FuzzyJSON" parser
  scripts/qmp-shell: refactor QMPCompleter
  scripts/qmp-shell: initialize completer early
  python/qmp: add QMPObject type alias
  scripts/qmp-shell: add mypy types
  scripts/qmp-shell: Accept SocketAddrT instead of string
  scripts/qmp-shell: unprivatize 'pretty' property
  python/qmp: return generic type from context manager
  scripts/qmp-shell: Use context manager instead of atexit
  scripts/qmp-shell: use logging to show warnings
  scripts/qmp-shell: remove TODO
  scripts/qmp-shell: Fix empty-transaction invocation
  scripts/qmp-shell: Remove too-broad-exception
  scripts/qmp-shell: convert usage comment to docstring
  scripts/qmp-shell: remove double-underscores
  scripts/qmp-shell: make QMPShellError inherit QMPError
  scripts/qmp-shell: add docstrings
  scripts/qmp-shell: move to python/qemu/qmp/qmp_shell.py
  python: add qmp-shell entry point
  scripts/qmp-shell: add redirection shim

 python/Pipfile.lock               |  13 +-
 python/qemu/qmp/__init__.py       |  61 +++-
 python/qemu/qmp/qemu_ga_client.py | 323 ++++++++++++++++++
 python/qemu/qmp/qmp_shell.py      | 535 ++++++++++++++++++++++++++++++
 python/qemu/qmp/qom.py            | 217 ++++++++++++
 python/qemu/qmp/qom_common.py     | 153 +++++++++
 python/qemu/qmp/qom_fuse.py       | 207 ++++++++++++
 python/setup.cfg                  |  30 +-
 scripts/qmp/qemu-ga-client        | 297 +----------------
 scripts/qmp/qmp-shell             | 454 +------------------------
 scripts/qmp/qom-fuse              | 144 +-------
 scripts/qmp/qom-get               |  66 +---
 scripts/qmp/qom-list              |  63 +---
 scripts/qmp/qom-set               |  63 +---
 scripts/qmp/qom-tree              |  74 +----
 15 files changed, 1546 insertions(+), 1154 deletions(-)
 create mode 100644 python/qemu/qmp/qemu_ga_client.py
 create mode 100644 python/qemu/qmp/qmp_shell.py
 create mode 100644 python/qemu/qmp/qom.py
 create mode 100644 python/qemu/qmp/qom_common.py
 create mode 100644 python/qemu/qmp/qom_fuse.py

-- 
2.26.2

[PATCH v2 01/72] python/qmp: Add qom script rewrites

[John Snow]

Inspired by qom-set, qom-get, qom-tree and qom-list; combine all four of
those scripts into one script.

A later addition of qom-fuse necessitates that some common features are
split out and shared between them.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/qemu/qmp/qom.py        | 207 ++++++++++++++++++++++++++++++++++
 python/qemu/qmp/qom_common.py | 153 +++++++++++++++++++++++++
 2 files changed, 360 insertions(+)
 create mode 100644 python/qemu/qmp/qom.py
 create mode 100644 python/qemu/qmp/qom_common.py

diff --git a/python/qemu/qmp/qom.py b/python/qemu/qmp/qom.py
new file mode 100644
index 000000000000..912d1809e60d
--- /dev/null
+++ b/python/qemu/qmp/qom.py
@@ -0,0 +1,207 @@
+"""
+QEMU Object Model testing tools.
+
+usage: qom.py [-h] {set,get,list,tree} ...
+
+Query and manipulate QOM data
+
+optional arguments:
+  -h, --help           show this help message and exit
+
+QOM commands:
+  {set,get,list,tree,fuse}
+    set                Set a QOM property value
+    get                Get a QOM property value
+    list               List QOM properties at a given path
+    tree               Show QOM tree from a given path
+"""
+##
+# Copyright John Snow 2020, for Red Hat, Inc.
+# Copyright IBM, Corp. 2011
+#
+# Authors:
+#  John Snow <jsnow@redhat.com>
+#  Anthony Liguori <aliguori@amazon.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or later.
+# See the COPYING file in the top-level directory.
+#
+# Based on ./scripts/qmp/qom-[set|get|tree|list]
+##
+
+import argparse
+import sys
+
+from . import QMPResponseError
+from .qom_common import QOMCommand
+
+
+class QOMSet(QOMCommand):
+    """QOM Command - Set a property to a given value."""
+    name = 'set'
+    help = 'Set a QOM property value'
+
+    @classmethod
+    def configure_parser(cls, parser: argparse.ArgumentParser) -> None:
+        super().configure_parser(parser)
+        cls.add_path_prop_arg(parser)
+        parser.add_argument(
+            'value',
+            metavar='<value>',
+            action='store',
+            help='new QOM property value'
+        )
+
+    def __init__(self, args: argparse.Namespace):
+        super().__init__(args)
+        self.path, self.prop = args.path_prop.rsplit('.', 1)
+        self.value = args.value
+
+    def run(self) -> int:
+        rsp = self.qmp.command(
+            'qom-set',
+            path=self.path,
+            property=self.prop,
+            value=self.value
+        )
+        print(rsp)
+        return 0
+
+
+class QOMGet(QOMCommand):
+    """QOM Command - Get a property's current value."""
+    name = 'get'
+    help = 'Get a QOM property value'
+
+    @classmethod
+    def configure_parser(cls, parser: argparse.ArgumentParser) -> None:
+        super().configure_parser(parser)
+        cls.add_path_prop_arg(parser)
+
+    def __init__(self, args: argparse.Namespace):
+        super().__init__(args)
+        try:
+            tmp = args.path_prop.rsplit('.', 1)
+        except ValueError as err:
+            raise ValueError('Invalid format for <path>.<property>') from err
+        self.path = tmp[0]
+        self.prop = tmp[1]
+
+    def run(self) -> int:
+        rsp = self.qmp.command(
+            'qom-get',
+            path=self.path,
+            property=self.prop
+        )
+        if isinstance(rsp, dict):
+            for key, value in rsp.items():
+                print(f"{key}: {value}")
+        else:
+            print(rsp)
+        return 0
+
+
+class QOMList(QOMCommand):
+    """QOM Command - List the properties at a given path."""
+    name = 'list'
+    help = 'List QOM properties at a given path'
+
+    @classmethod
+    def configure_parser(cls, parser: argparse.ArgumentParser) -> None:
+        super().configure_parser(parser)
+        parser.add_argument(
+            'path',
+            metavar='<path>',
+            action='store',
+            help='QOM path',
+        )
+
+    def __init__(self, args: argparse.Namespace):
+        super().__init__(args)
+        self.path = args.path
+
+    def run(self) -> int:
+        rsp = self.qom_list(self.path)
+        for item in rsp:
+            if item.child:
+                print(f"{item.name}/")
+            elif item.link:
+                print(f"@{item.name}/")
+            else:
+                print(item.name)
+        return 0
+
+
+class QOMTree(QOMCommand):
+    """QOM Command - Show the full tree below a given path."""
+    name = 'tree'
+    help = 'Show QOM tree from a given path'
+
+    @classmethod
+    def configure_parser(cls, parser: argparse.ArgumentParser) -> None:
+        super().configure_parser(parser)
+        parser.add_argument(
+            'path',
+            metavar='<path>',
+            action='store',
+            help='QOM path',
+            nargs='?',
+            default='/'
+        )
+
+    def __init__(self, args: argparse.Namespace):
+        super().__init__(args)
+        self.path = args.path
+
+    def _list_node(self, path: str) -> None:
+        print(path)
+        items = self.qom_list(path)
+        for item in items:
+            if item.child:
+                continue
+            try:
+                rsp = self.qmp.command('qom-get', path=path,
+                                       property=item.name)
+                print(f"  {item.name}: {rsp} ({item.type})")
+            except QMPResponseError as err:
+                print(f"  {item.name}: <EXCEPTION: {err!s}> ({item.type})")
+        print('')
+        for item in items:
+            if not item.child:
+                continue
+            if path == '/':
+                path = ''
+            self._list_node(f"{path}/{item.name}")
+
+    def run(self) -> int:
+        self._list_node(self.path)
+        return 0
+
+
+def main() -> int:
+    """QOM script main entry point."""
+    parser = argparse.ArgumentParser(
+        description='Query and manipulate QOM data'
+    )
+    subparsers = parser.add_subparsers(
+        title='QOM commands',
+        dest='command'
+    )
+
+    for command in QOMCommand.__subclasses__():
+        command.register(subparsers)
+
+    args = parser.parse_args()
+
+    if args.command is None:
+        parser.error('Command not specified.')
+        return 1
+
+    assert issubclass(args.cmd_class, QOMCommand)
+    cmd = args.cmd_class(args)
+    assert isinstance(cmd, QOMCommand)
+    return cmd.run()
+
+
+if __name__ == '__main__':
+    sys.exit(main())
diff --git a/python/qemu/qmp/qom_common.py b/python/qemu/qmp/qom_common.py
new file mode 100644
index 000000000000..3890b247ae68
--- /dev/null
+++ b/python/qemu/qmp/qom_common.py
@@ -0,0 +1,153 @@
+"""
+QOM Command abstractions.
+"""
+##
+# Copyright John Snow 2020, for Red Hat, Inc.
+# Copyright IBM, Corp. 2011
+#
+# Authors:
+#  John Snow <jsnow@redhat.com>
+#  Anthony Liguori <aliguori@amazon.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or later.
+# See the COPYING file in the top-level directory.
+#
+# Based on ./scripts/qmp/qom-[set|get|tree|list]
+##
+
+import argparse
+import os
+from typing import (
+    Any,
+    Dict,
+    List,
+    Optional,
+)
+
+from . import QEMUMonitorProtocol
+
+
+Subparsers = argparse._SubParsersAction  # pylint: disable=protected-access
+
+
+class ObjectPropertyInfo:
+    """
+    Represents the return type from e.g. qom-list.
+    """
+    def __init__(self, name: str, type_: str,
+                 description: Optional[str] = None,
+                 default_value: Optional[object] = None):
+        self.name = name
+        self.type = type_
+        self.description = description
+        self.default_value = default_value
+
+    @classmethod
+    def make(cls, value: Dict[str, Any]) -> 'ObjectPropertyInfo':
+        """
+        Build an ObjectPropertyInfo from a Dict with an unknown shape.
+        """
+        assert value.keys() >= {'name', 'type'}
+        assert value.keys() <= {'name', 'type', 'description', 'default-value'}
+        return cls(value['name'], value['type'],
+                   value.get('description'),
+                   value.get('default-value'))
+
+    @property
+    def child(self) -> bool:
+        """Is this property a child property?"""
+        return self.type.startswith('child<')
+
+    @property
+    def link(self) -> bool:
+        """Is this property a link property?"""
+        return self.type.startswith('link<')
+
+
+class QOMCommand:
+    """
+    Represents a QOM sub-command.
+
+    :param args: Parsed arguments, as returned from parser.parse_args.
+    """
+    name: str
+    help: str
+
+    def __init__(self, args: argparse.Namespace):
+        if args.socket is None:
+            raise Exception("No QMP socket path or address given")
+        self.qmp = QEMUMonitorProtocol(args.socket)
+        self.qmp.connect()
+
+    @classmethod
+    def register(cls, subparsers: Subparsers) -> None:
+        """
+        Register this command with the argument parser.
+
+        :param subparsers: argparse subparsers object, from "add_subparsers".
+        """
+        subparser = subparsers.add_parser(cls.name, help=cls.help,
+                                          description=cls.help)
+        cls.configure_parser(subparser)
+
+    @classmethod
+    def configure_parser(cls, parser: argparse.ArgumentParser) -> None:
+        """
+        Configure a parser with this command's arguments.
+
+        :param parser: argparse parser or subparser object.
+        """
+        default_path = os.environ.get('QMP_SOCKET')
+        parser.add_argument(
+            '--socket', '-s',
+            dest='socket',
+            action='store',
+            help='QMP socket path or address (addr:port).'
+            ' May also be set via QMP_SOCKET environment variable.',
+            default=default_path
+        )
+        parser.set_defaults(cmd_class=cls)
+
+    @classmethod
+    def add_path_prop_arg(cls, parser: argparse.ArgumentParser) -> None:
+        """
+        Add the <path>.<proptery> positional argument to this command.
+
+        :param parser: The parser to add the argument to.
+        """
+        parser.add_argument(
+            'path_prop',
+            metavar='<path>.<property>',
+            action='store',
+            help="QOM path and property, separated by a period '.'"
+        )
+
+    def run(self) -> int:
+        """
+        Run this command.
+
+        :return: 0 on success, 1 otherwise.
+        """
+        raise NotImplementedError
+
+    def qom_list(self, path: str) -> List[ObjectPropertyInfo]:
+        """
+        :return: a strongly typed list from the 'qom-list' command.
+        """
+        rsp = self.qmp.command('qom-list', path=path)
+        # qom-list returns List[ObjectPropertyInfo]
+        assert isinstance(rsp, list)
+        return [ObjectPropertyInfo.make(x) for x in rsp]
+
+    @classmethod
+    def entry_point(cls) -> int:
+        """
+        Build this command's parser, parse arguments, and run the command.
+
+        :return: `run`'s return code.
+        """
+        parser = argparse.ArgumentParser(description=cls.help)
+        cls.configure_parser(parser)
+        args = parser.parse_args()
+        cmd = cls(args)
+        return cmd.run()
-- 
2.26.2

[PATCH v2 02/72] python/qmp: add qom script entry points

[John Snow]

Add the 'qom', 'qom-set', 'qom-get', 'qom-list', and 'qom-tree' scripts
to the qemu.qmp package. When you install this package, these scripts
will become available on your command line.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 python/setup.cfg | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/python/setup.cfg b/python/setup.cfg
index 2c12d9ab89b4..c8b7215996e9 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -28,6 +28,13 @@ devel =
     pylint >= 2.6.0
     pytest >= 3.0.0
 
+[options.entry_points]
+console_scripts =
+    qom = qemu.qmp.qom:main
+    qom-set = qemu.qmp.qom:QOMSet.entry_point
+    qom-get = qemu.qmp.qom:QOMGet.entry_point
+    qom-list = qemu.qmp.qom:QOMList.entry_point
+    qom-tree = qemu.qmp.qom:QOMTree.entry_point
 
 [flake8]
 extend-ignore = E722  # Prefer pylint's bare-except checks to flake8's
-- 
2.26.2

[PATCH v2 03/72] scripts/qmp: redirect qom-xxx scripts to python/qemu/qmp/

[John Snow]

Redirect to the new qom scripts. These forwarders can be deleted
eventually when there has been more time for the dust on the Python
packaging to settle and people understand how to find these commands.

Note: You can run these by setting $PYTHONPATH in your shell and then
running "python3 -m qemu.qmp.qom", or you can install the qemu namespace
package and use the "qom" or "qom-set" scripts.

I've written how to install the package elsewhere, but for the sake of
git-blame, cd to ./python, and then do:

- pip3 install [--user] [-e] .

--user will install to your local user install (will not work inside of
  a venv), omitting this flag installs to your system-wide packages
  (outside of a venv) or to your current virtual environment (inside the
  venv).

  When installing to a venv or to your system-wide packages, "qom"
  should be in your $PATH already. If you do a user install, you may
  need to add ~/.local/bin to your $PATH if you haven't already.

-e installs in editable mode: the installed package is effectively just
 a symlink to this folder; so changes to your git working tree are
 reflected in the installed package.

Alternatively to the above, If you have `pipenv` installed (`pip3
install --user pipenv`), you may also invoke 'pipenv shell' to enter a
pipenv-managed virtual environment (as a shell process that you may
leave with ctrt+d) that has 'qom' already in $PATH.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 scripts/qmp/qom-get  | 66 +++------------------------------------
 scripts/qmp/qom-list | 63 +++----------------------------------
 scripts/qmp/qom-set  | 63 +++----------------------------------
 scripts/qmp/qom-tree | 74 +++-----------------------------------------
 4 files changed, 16 insertions(+), 250 deletions(-)

diff --git a/scripts/qmp/qom-get b/scripts/qmp/qom-get
index 666df718320c..e4f3e0c01381 100755
--- a/scripts/qmp/qom-get
+++ b/scripts/qmp/qom-get
@@ -1,69 +1,11 @@
 #!/usr/bin/env python3
-##
-# QEMU Object Model test tools
-#
-# Copyright IBM, Corp. 2011
-#
-# Authors:
-#  Anthony Liguori   <aliguori@us.ibm.com>
-#
-# This work is licensed under the terms of the GNU GPL, version 2 or later.  See
-# the COPYING file in the top-level directory.
-##
 
-import sys
 import os
+import sys
 
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
-from qemu.qmp import QEMUMonitorProtocol
+from qemu.qmp.qom import QOMGet
 
-cmd, args = sys.argv[0], sys.argv[1:]
-socket_path = None
-path = None
-prop = None
 
-def usage():
-    return '''environment variables:
-    QMP_SOCKET=<path | addr:port>
-usage:
-    %s [-h] [-s <QMP socket path | addr:port>] <path>.<property>
-''' % cmd
-
-def usage_error(error_msg = "unspecified error"):
-    sys.stderr.write('%s\nERROR: %s\n' % (usage(), error_msg))
-    exit(1)
-
-if len(args) > 0:
-    if args[0] == "-h":
-        print(usage())
-        exit(0);
-    elif args[0] == "-s":
-        try:
-            socket_path = args[1]
-        except:
-            usage_error("missing argument: QMP socket path or address");
-        args = args[2:]
-
-if not socket_path:
-    if 'QMP_SOCKET' in os.environ:
-        socket_path = os.environ['QMP_SOCKET']
-    else:
-        usage_error("no QMP socket path or address given");
-
-if len(args) > 0:
-    try:
-        path, prop = args[0].rsplit('.', 1)
-    except:
-        usage_error("invalid format for path/property/value")
-else:
-    usage_error("not enough arguments")
-
-srv = QEMUMonitorProtocol(socket_path)
-srv.connect()
-
-rsp = srv.command('qom-get', path=path, property=prop)
-if type(rsp) == dict:
-    for i in rsp.keys():
-        print('%s: %s' % (i, rsp[i]))
-else:
-    print(rsp)
+if __name__ == '__main__':
+    sys.exit(QOMGet.entry_point())
diff --git a/scripts/qmp/qom-list b/scripts/qmp/qom-list
index 5074fd939f4a..7a071a54e1e7 100755
--- a/scripts/qmp/qom-list
+++ b/scripts/qmp/qom-list
@@ -1,66 +1,11 @@
 #!/usr/bin/env python3
-##
-# QEMU Object Model test tools
-#
-# Copyright IBM, Corp. 2011
-#
-# Authors:
-#  Anthony Liguori   <aliguori@us.ibm.com>
-#
-# This work is licensed under the terms of the GNU GPL, version 2 or later.  See
-# the COPYING file in the top-level directory.
-##
 
-import sys
 import os
+import sys
 
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
-from qemu.qmp import QEMUMonitorProtocol
+from qemu.qmp.qom import QOMList
 
-cmd, args = sys.argv[0], sys.argv[1:]
-socket_path = None
-path = None
-prop = None
 
-def usage():
-    return '''environment variables:
-    QMP_SOCKET=<path | addr:port>
-usage:
-    %s [-h] [-s <QMP socket path | addr:port>] [<path>]
-''' % cmd
-
-def usage_error(error_msg = "unspecified error"):
-    sys.stderr.write('%s\nERROR: %s\n' % (usage(), error_msg))
-    exit(1)
-
-if len(args) > 0:
-    if args[0] == "-h":
-        print(usage())
-        exit(0);
-    elif args[0] == "-s":
-        try:
-            socket_path = args[1]
-        except:
-            usage_error("missing argument: QMP socket path or address");
-        args = args[2:]
-
-if not socket_path:
-    if 'QMP_SOCKET' in os.environ:
-        socket_path = os.environ['QMP_SOCKET']
-    else:
-        usage_error("no QMP socket path or address given");
-
-srv = QEMUMonitorProtocol(socket_path)
-srv.connect()
-
-if len(args) == 0:
-    print('/')
-    sys.exit(0)
-
-for item in srv.command('qom-list', path=args[0]):
-    if item['type'].startswith('child<'):
-        print('%s/' % item['name'])
-    elif item['type'].startswith('link<'):
-        print('@%s/' % item['name'])
-    else:
-        print('%s' % item['name'])
+if __name__ == '__main__':
+    sys.exit(QOMList.entry_point())
diff --git a/scripts/qmp/qom-set b/scripts/qmp/qom-set
index 240a78187f9b..9ca9e2ba106b 100755
--- a/scripts/qmp/qom-set
+++ b/scripts/qmp/qom-set
@@ -1,66 +1,11 @@
 #!/usr/bin/env python3
-##
-# QEMU Object Model test tools
-#
-# Copyright IBM, Corp. 2011
-#
-# Authors:
-#  Anthony Liguori   <aliguori@us.ibm.com>
-#
-# This work is licensed under the terms of the GNU GPL, version 2 or later.  See
-# the COPYING file in the top-level directory.
-##
 
-import sys
 import os
+import sys
 
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
-from qemu.qmp import QEMUMonitorProtocol
+from qemu.qmp.qom import QOMSet
 
-cmd, args = sys.argv[0], sys.argv[1:]
-socket_path = None
-path = None
-prop = None
-value = None
 
-def usage():
-    return '''environment variables:
-    QMP_SOCKET=<path | addr:port>
-usage:
-    %s [-h] [-s <QMP socket path | addr:port>] <path>.<property> <value>
-''' % cmd
-
-def usage_error(error_msg = "unspecified error"):
-    sys.stderr.write('%s\nERROR: %s\n' % (usage(), error_msg))
-    exit(1)
-
-if len(args) > 0:
-    if args[0] == "-h":
-        print(usage())
-        exit(0);
-    elif args[0] == "-s":
-        try:
-            socket_path = args[1]
-        except:
-            usage_error("missing argument: QMP socket path or address");
-        args = args[2:]
-
-if not socket_path:
-    if 'QMP_SOCKET' in os.environ:
-        socket_path = os.environ['QMP_SOCKET']
-    else:
-        usage_error("no QMP socket path or address given");
-
-if len(args) > 1:
-    try:
-        path, prop = args[0].rsplit('.', 1)
-    except:
-        usage_error("invalid format for path/property/value")
-    value = args[1]
-else:
-    usage_error("not enough arguments")
-
-srv = QEMUMonitorProtocol(socket_path)
-srv.connect()
-
-print(srv.command('qom-set', path=path, property=prop, value=value))
+if __name__ == '__main__':
+    sys.exit(QOMSet.entry_point())
diff --git a/scripts/qmp/qom-tree b/scripts/qmp/qom-tree
index 25b0781323cc..7d0ccca3a4dd 100755
--- a/scripts/qmp/qom-tree
+++ b/scripts/qmp/qom-tree
@@ -1,77 +1,11 @@
 #!/usr/bin/env python3
-##
-# QEMU Object Model test tools
-#
-# Copyright IBM, Corp. 2011
-# Copyright (c) 2013 SUSE LINUX Products GmbH
-#
-# Authors:
-#  Anthony Liguori   <aliguori@amazon.com>
-#  Andreas Faerber   <afaerber@suse.de>
-#
-# This work is licensed under the terms of the GNU GPL, version 2 or later.  See
-# the COPYING file in the top-level directory.
-##
 
-import sys
 import os
+import sys
 
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
-from qemu.qmp import QEMUMonitorProtocol
+from qemu.qmp.qom import QOMTree
 
-cmd, args = sys.argv[0], sys.argv[1:]
-socket_path = None
-path = None
-prop = None
 
-def usage():
-    return '''environment variables:
-    QMP_SOCKET=<path | addr:port>
-usage:
-    %s [-h] [-s <QMP socket path | addr:port>] [<path>]
-''' % cmd
-
-def usage_error(error_msg = "unspecified error"):
-    sys.stderr.write('%s\nERROR: %s\n' % (usage(), error_msg))
-    exit(1)
-
-if len(args) > 0:
-    if args[0] == "-h":
-        print(usage())
-        exit(0);
-    elif args[0] == "-s":
-        try:
-            socket_path = args[1]
-        except:
-            usage_error("missing argument: QMP socket path or address");
-        args = args[2:]
-
-if not socket_path:
-    if 'QMP_SOCKET' in os.environ:
-        socket_path = os.environ['QMP_SOCKET']
-    else:
-        usage_error("no QMP socket path or address given");
-
-srv = QEMUMonitorProtocol(socket_path)
-srv.connect()
-
-def list_node(path):
-    print('%s' % path)
-    items = srv.command('qom-list', path=path)
-    for item in items:
-        if not item['type'].startswith('child<'):
-            try:
-                print('  %s: %s (%s)' % (item['name'], srv.command('qom-get', path=path, property=item['name']), item['type']))
-            except:
-                print('  %s: <EXCEPTION> (%s)' % (item['name'], item['type']))
-    print('')
-    for item in items:
-        if item['type'].startswith('child<'):
-            list_node((path if (path != '/') else '')  + '/' + item['name'])
-
-if len(args) == 0:
-    path = '/'
-else:
-    path = args[0]
-
-list_node(path)
+if __name__ == '__main__':
+    sys.exit(QOMTree.entry_point())
-- 
2.26.2

[PATCH v2 04/72] scripts/qom-fuse: apply isort rules

[John Snow]

Hint: you can use symlinks to create qom_fuse.py in python/qemu/qmp/ and
point to scripts/qom-fuse to apply the standard linting rules to this
script.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 scripts/qmp/qom-fuse | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/scripts/qmp/qom-fuse b/scripts/qmp/qom-fuse
index 7c7cff8edfba..62deb9adb12d 100755
--- a/scripts/qmp/qom-fuse
+++ b/scripts/qmp/qom-fuse
@@ -13,14 +13,20 @@
 # the COPYING file in the top-level directory.
 ##
 
-import fuse, stat
-from fuse import FUSE, FuseOSError, Operations
-import os, posix, sys
 from errno import *
+import os
+import posix
+import stat
+import sys
+
+import fuse
+from fuse import FUSE, FuseOSError, Operations
+
 
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
 from qemu.qmp import QEMUMonitorProtocol
 
+
 fuse.fuse_python_api = (0, 2)
 
 class QOMFS(Operations):
-- 
2.26.2

[PATCH v2 05/72] scripts/qom-fuse: apply flake8 rules

[John Snow]

flake8 still has one warning because of the sys.path hack, but that will
be going away by the end of this patch series.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qom-fuse | 81 +++++++++++++++++++++++---------------------
 1 file changed, 43 insertions(+), 38 deletions(-)

diff --git a/scripts/qmp/qom-fuse b/scripts/qmp/qom-fuse
index 62deb9adb12d..ca30e928679e 100755
--- a/scripts/qmp/qom-fuse
+++ b/scripts/qmp/qom-fuse
@@ -9,13 +9,12 @@
 #  Anthony Liguori   <aliguori@us.ibm.com>
 #  Markus Armbruster <armbru@redhat.com>
 #
-# This work is licensed under the terms of the GNU GPL, version 2 or later.  See
-# the COPYING file in the top-level directory.
+# This work is licensed under the terms of the GNU GPL, version 2 or later.
+# See the COPYING file in the top-level directory.
 ##
 
-from errno import *
+from errno import ENOENT, EPERM
 import os
-import posix
 import stat
 import sys
 
@@ -29,6 +28,7 @@ from qemu.qmp import QEMUMonitorProtocol
 
 fuse.fuse_python_api = (0, 2)
 
+
 class QOMFS(Operations):
     def __init__(self, qmp):
         self.qmp = qmp
@@ -45,7 +45,7 @@ class QOMFS(Operations):
 
     def is_object(self, path):
         try:
-            items = self.qmp.command('qom-list', path=path)
+            self.qmp.command('qom-list', path=path)
             return True
         except:
             return False
@@ -85,7 +85,7 @@ class QOMFS(Operations):
             path = '/'
         try:
             data = self.qmp.command('qom-get', path=path, property=prop)
-            data += '\n' # make values shell friendly
+            data += '\n'  # make values shell friendly
         except:
             raise FuseOSError(EPERM)
 
@@ -104,38 +104,44 @@ class QOMFS(Operations):
 
     def getattr(self, path, fh=None):
         if self.is_link(path):
-            value = { 'st_mode': 0o755 | stat.S_IFLNK,
-                      'st_ino': self.get_ino(path),
-                      'st_dev': 0,
-                      'st_nlink': 2,
-                      'st_uid': 1000,
-                      'st_gid': 1000,
-                      'st_size': 4096,
-                      'st_atime': 0,
-                      'st_mtime': 0,
-                      'st_ctime': 0 }
+            value = {
+                'st_mode': 0o755 | stat.S_IFLNK,
+                'st_ino': self.get_ino(path),
+                'st_dev': 0,
+                'st_nlink': 2,
+                'st_uid': 1000,
+                'st_gid': 1000,
+                'st_size': 4096,
+                'st_atime': 0,
+                'st_mtime': 0,
+                'st_ctime': 0
+            }
         elif self.is_object(path):
-            value = { 'st_mode': 0o755 | stat.S_IFDIR,
-                      'st_ino': self.get_ino(path),
-                      'st_dev': 0,
-                      'st_nlink': 2,
-                      'st_uid': 1000,
-                      'st_gid': 1000,
-                      'st_size': 4096,
-                      'st_atime': 0,
-                      'st_mtime': 0,
-                      'st_ctime': 0 }
+            value = {
+                'st_mode': 0o755 | stat.S_IFDIR,
+                'st_ino': self.get_ino(path),
+                'st_dev': 0,
+                'st_nlink': 2,
+                'st_uid': 1000,
+                'st_gid': 1000,
+                'st_size': 4096,
+                'st_atime': 0,
+                'st_mtime': 0,
+                'st_ctime': 0
+            }
         elif self.is_property(path):
-            value = { 'st_mode': 0o644 | stat.S_IFREG,
-                      'st_ino': self.get_ino(path),
-                      'st_dev': 0,
-                      'st_nlink': 1,
-                      'st_uid': 1000,
-                      'st_gid': 1000,
-                      'st_size': 4096,
-                      'st_atime': 0,
-                      'st_mtime': 0,
-                      'st_ctime': 0 }
+            value = {
+                'st_mode': 0o644 | stat.S_IFREG,
+                'st_ino': self.get_ino(path),
+                'st_dev': 0,
+                'st_nlink': 1,
+                'st_uid': 1000,
+                'st_gid': 1000,
+                'st_size': 4096,
+                'st_atime': 0,
+                'st_mtime': 0,
+                'st_ctime': 0
+            }
         else:
             raise FuseOSError(ENOENT)
         return value
@@ -146,8 +152,7 @@ class QOMFS(Operations):
         for item in self.qmp.command('qom-list', path=path):
             yield str(item['name'])
 
+
 if __name__ == '__main__':
-    import os
-
     fuse = FUSE(QOMFS(QEMUMonitorProtocol(os.environ['QMP_SOCKET'])),
                 sys.argv[1], foreground=True)
-- 
2.26.2

[PATCH v2 06/72] python: Add 'fh' to known-good variable names

[John Snow]

fd and fh are fine: we often use these for "file descriptor" or "file
handle" accordingly. It is rarely the case that you need to enforce a
more semantically meaningful name beyond "This is the file we are using
right now."

While we're here: add comments for all of the non-standard pylint
names. (And the underscore.)

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/setup.cfg | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/python/setup.cfg b/python/setup.cfg
index c8b7215996e9..c963d05527aa 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -68,9 +68,10 @@ good-names=i,
            k,
            ex,
            Run,
-           _,
-           fd,
-           c,
+           _,   # By convention: Unused variable
+           fh,  # fh = open(...)
+           fd,  # fd = os.open(...)
+           c,   # for c in string: ...
 
 [pylint.similarities]
 # Ignore imports when computing similarities.
-- 
2.26.2

[PATCH v2 07/72] scripts/qom-fuse: Apply pylint rules

[John Snow]

- Catch specific exceptions from QMP
- Reraise errors with explicit context
- method parameters should match parent's names

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 scripts/qmp/qom-fuse | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/scripts/qmp/qom-fuse b/scripts/qmp/qom-fuse
index ca30e928679e..805e99c8ecde 100755
--- a/scripts/qmp/qom-fuse
+++ b/scripts/qmp/qom-fuse
@@ -23,7 +23,7 @@ from fuse import FUSE, FuseOSError, Operations
 
 
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
-from qemu.qmp import QEMUMonitorProtocol
+from qemu.qmp import QEMUMonitorProtocol, QMPResponseError
 
 
 fuse.fuse_python_api = (0, 2)
@@ -47,7 +47,7 @@ class QOMFS(Operations):
         try:
             self.qmp.command('qom-list', path=path)
             return True
-        except:
+        except QMPResponseError:
             return False
 
     def is_property(self, path):
@@ -59,7 +59,7 @@ class QOMFS(Operations):
                 if item['name'] == prop:
                     return True
             return False
-        except:
+        except QMPResponseError:
             return False
 
     def is_link(self, path):
@@ -73,10 +73,10 @@ class QOMFS(Operations):
                         return True
                     return False
             return False
-        except:
+        except QMPResponseError:
             return False
 
-    def read(self, path, length, offset, fh):
+    def read(self, path, size, offset, fh):
         if not self.is_property(path):
             return -ENOENT
 
@@ -86,13 +86,13 @@ class QOMFS(Operations):
         try:
             data = self.qmp.command('qom-get', path=path, property=prop)
             data += '\n'  # make values shell friendly
-        except:
-            raise FuseOSError(EPERM)
+        except QMPResponseError as err:
+            raise FuseOSError(EPERM) from err
 
         if offset > len(data):
             return ''
 
-        return bytes(data[offset:][:length], encoding='utf-8')
+        return bytes(data[offset:][:size], encoding='utf-8')
 
     def readlink(self, path):
         if not self.is_link(path):
-- 
2.26.2

[PATCH v2 08/72] scripts/qom-fuse: Add docstrings

[John Snow]

The methods inherited from fuse don't need docstrings; that's up to
fusepy to handle.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qom-fuse | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/scripts/qmp/qom-fuse b/scripts/qmp/qom-fuse
index 805e99c8ecde..5b80da9df66b 100755
--- a/scripts/qmp/qom-fuse
+++ b/scripts/qmp/qom-fuse
@@ -1,7 +1,20 @@
 #!/usr/bin/env python3
+"""
+QEMU Object Model FUSE filesystem tool
+
+This script offers a simple FUSE filesystem within which the QOM tree
+may be browsed, queried and edited using traditional shell tooling.
+
+This script requires the 'fusepy' python package;
+you may install it by using ``pip3 install --user fusepy``.
+
+ENV:
+    QMP_SOCKET: Path to the QMP server socket
+
+Usage:
+    qom-fuse /mount/to/here
+"""
 ##
-# QEMU Object Model test tools
-#
 # Copyright IBM, Corp. 2012
 # Copyright (C) 2020 Red Hat, Inc.
 #
@@ -30,6 +43,7 @@ fuse.fuse_python_api = (0, 2)
 
 
 class QOMFS(Operations):
+    """QOMFS implements fuse.Operations to provide a QOM filesystem."""
     def __init__(self, qmp):
         self.qmp = qmp
         self.qmp.connect()
@@ -37,6 +51,7 @@ class QOMFS(Operations):
         self.ino_count = 1
 
     def get_ino(self, path):
+        """Get an inode number for a given QOM path."""
         if path in self.ino_map:
             return self.ino_map[path]
         self.ino_map[path] = self.ino_count
@@ -44,6 +59,7 @@ class QOMFS(Operations):
         return self.ino_map[path]
 
     def is_object(self, path):
+        """Is the given QOM path an object?"""
         try:
             self.qmp.command('qom-list', path=path)
             return True
@@ -51,6 +67,7 @@ class QOMFS(Operations):
             return False
 
     def is_property(self, path):
+        """Is the given QOM path a property?"""
         path, prop = path.rsplit('/', 1)
         if path == '':
             path = '/'
@@ -63,6 +80,7 @@ class QOMFS(Operations):
             return False
 
     def is_link(self, path):
+        """Is the given QOM path a link?"""
         path, prop = path.rsplit('/', 1)
         if path == '':
             path = '/'
-- 
2.26.2

[PATCH v2 09/72] scripts/qom-fuse: Convert to QOMCommand

[John Snow]

Move qom-fuse onto the QOMCommand base established in
python/qemu/qmp/qom_common.py. The interface doesn't change
incompatibly, "qom-fuse mountpoint" still works as an invocation, and
QMP_SOCKET is still used as the environment variable.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qom-fuse | 59 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 46 insertions(+), 13 deletions(-)

diff --git a/scripts/qmp/qom-fuse b/scripts/qmp/qom-fuse
index 5b80da9df66b..f9bf85f38217 100755
--- a/scripts/qmp/qom-fuse
+++ b/scripts/qmp/qom-fuse
@@ -8,11 +8,19 @@ may be browsed, queried and edited using traditional shell tooling.
 This script requires the 'fusepy' python package;
 you may install it by using ``pip3 install --user fusepy``.
 
-ENV:
-    QMP_SOCKET: Path to the QMP server socket
 
-Usage:
-    qom-fuse /mount/to/here
+usage: qom-fuse [-h] [--socket SOCKET] <mount>
+
+Mount a QOM tree as a FUSE filesystem
+
+positional arguments:
+  <mount>               Mount point
+
+optional arguments:
+  -h, --help            show this help message and exit
+  --socket SOCKET, -s SOCKET
+                        QMP socket path or address (addr:port). May also be
+                        set via QMP_SOCKET environment variable.
 """
 ##
 # Copyright IBM, Corp. 2012
@@ -26,30 +34,56 @@ Usage:
 # See the COPYING file in the top-level directory.
 ##
 
+import argparse
 from errno import ENOENT, EPERM
 import os
 import stat
 import sys
+from typing import Dict
 
 import fuse
 from fuse import FUSE, FuseOSError, Operations
 
 
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
-from qemu.qmp import QEMUMonitorProtocol, QMPResponseError
+from qemu.qmp import QMPResponseError
+from qemu.qmp.qom_common import QOMCommand
 
 
 fuse.fuse_python_api = (0, 2)
 
 
-class QOMFS(Operations):
-    """QOMFS implements fuse.Operations to provide a QOM filesystem."""
-    def __init__(self, qmp):
-        self.qmp = qmp
-        self.qmp.connect()
-        self.ino_map = {}
+class QOMFuse(QOMCommand, Operations):
+    """
+    QOMFuse implements both fuse.Operations and QOMCommand.
+
+    Operations implements the FS, and QOMCommand implements the CLI command.
+    """
+    name = 'fuse'
+    help = 'Mount a QOM tree as a FUSE filesystem'
+    fuse: FUSE
+
+    @classmethod
+    def configure_parser(cls, parser: argparse.ArgumentParser) -> None:
+        super().configure_parser(parser)
+        parser.add_argument(
+            'mount',
+            metavar='<mount>',
+            action='store',
+            help="Mount point",
+        )
+
+    def __init__(self, args: argparse.Namespace):
+        super().__init__(args)
+        self.mount = args.mount
+        self.ino_map: Dict[str, int] = {}
         self.ino_count = 1
 
+    def run(self) -> int:
+        print(f"Mounting QOMFS to '{self.mount}'", file=sys.stderr)
+        self.fuse = FUSE(self, self.mount, foreground=True)
+        return 0
+
     def get_ino(self, path):
         """Get an inode number for a given QOM path."""
         if path in self.ino_map:
@@ -172,5 +206,4 @@ class QOMFS(Operations):
 
 
 if __name__ == '__main__':
-    fuse = FUSE(QOMFS(QEMUMonitorProtocol(os.environ['QMP_SOCKET'])),
-                sys.argv[1], foreground=True)
+    sys.exit(QOMFuse.entry_point())
-- 
2.26.2

[PATCH v2 10/72] scripts/qom-fuse: use QOMCommand.qom_list()

[John Snow]

the qom_list method provides a type-safe object that's easier to type
check, so switch to using it.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qom-fuse | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/scripts/qmp/qom-fuse b/scripts/qmp/qom-fuse
index f9bf85f38217..b1030273efaa 100755
--- a/scripts/qmp/qom-fuse
+++ b/scripts/qmp/qom-fuse
@@ -95,7 +95,7 @@ class QOMFuse(QOMCommand, Operations):
     def is_object(self, path):
         """Is the given QOM path an object?"""
         try:
-            self.qmp.command('qom-list', path=path)
+            self.qom_list(path)
             return True
         except QMPResponseError:
             return False
@@ -106,8 +106,8 @@ class QOMFuse(QOMCommand, Operations):
         if path == '':
             path = '/'
         try:
-            for item in self.qmp.command('qom-list', path=path):
-                if item['name'] == prop:
+            for item in self.qom_list(path):
+                if item.name == prop:
                     return True
             return False
         except QMPResponseError:
@@ -119,11 +119,9 @@ class QOMFuse(QOMCommand, Operations):
         if path == '':
             path = '/'
         try:
-            for item in self.qmp.command('qom-list', path=path):
-                if item['name'] == prop:
-                    if item['type'].startswith('link<'):
-                        return True
-                    return False
+            for item in self.qom_list(path):
+                if item.name == prop and item.link:
+                    return True
             return False
         except QMPResponseError:
             return False
@@ -201,8 +199,8 @@ class QOMFuse(QOMCommand, Operations):
     def readdir(self, path, fh):
         yield '.'
         yield '..'
-        for item in self.qmp.command('qom-list', path=path):
-            yield str(item['name'])
+        for item in self.qom_list(path):
+            yield item.name
 
 
 if __name__ == '__main__':
-- 
2.26.2

[PATCH v2 11/72] scripts/qom-fuse: ensure QOMFuse.read always returns bytes

[John Snow]

- Use FuseOSError to signal ENOENT instead of returning it
- Wrap qom-get in str(), as we don't always know its type
- The empty return should be b'', not ''.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 scripts/qmp/qom-fuse | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/scripts/qmp/qom-fuse b/scripts/qmp/qom-fuse
index b1030273efaa..b120b93391ba 100755
--- a/scripts/qmp/qom-fuse
+++ b/scripts/qmp/qom-fuse
@@ -128,19 +128,19 @@ class QOMFuse(QOMCommand, Operations):
 
     def read(self, path, size, offset, fh):
         if not self.is_property(path):
-            return -ENOENT
+            raise FuseOSError(ENOENT)
 
         path, prop = path.rsplit('/', 1)
         if path == '':
             path = '/'
         try:
-            data = self.qmp.command('qom-get', path=path, property=prop)
+            data = str(self.qmp.command('qom-get', path=path, property=prop))
             data += '\n'  # make values shell friendly
         except QMPResponseError as err:
             raise FuseOSError(EPERM) from err
 
         if offset > len(data):
-            return ''
+            return b''
 
         return bytes(data[offset:][:size], encoding='utf-8')
 
-- 
2.26.2

[PATCH v2 12/72] scripts/qom-fuse: add static type hints

[John Snow]

Because fusepy does not have type hints, add some targeted warning
suppressions.

NOTE: Prior to this change, running 'mypy qemu' from the python
directory worked OK, but only coincidentally. Going forward, you will
need to run 'mypy -p qemu' instead. These invocation forms will be
codified in a CI test soon.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/setup.cfg     |  8 ++++++++
 scripts/qmp/qom-fuse | 26 +++++++++++++++++---------
 2 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/python/setup.cfg b/python/setup.cfg
index c963d05527aa..831555552f16 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -47,6 +47,14 @@ python_version = 3.6
 warn_unused_configs = True
 namespace_packages = True
 
+[mypy-qemu.qmp.qom_fuse]
+# fusepy has no type stubs:
+allow_subclassing_any = True
+
+[mypy-fuse]
+# fusepy has no type stubs:
+ignore_missing_imports = True
+
 [pylint.messages control]
 # Disable the message, report, category or checker with the given id(s). You
 # can either give multiple identifiers separated by comma (,) or put this
diff --git a/scripts/qmp/qom-fuse b/scripts/qmp/qom-fuse
index b120b93391ba..22be36634967 100755
--- a/scripts/qmp/qom-fuse
+++ b/scripts/qmp/qom-fuse
@@ -39,7 +39,14 @@ from errno import ENOENT, EPERM
 import os
 import stat
 import sys
-from typing import Dict
+from typing import (
+    IO,
+    Dict,
+    Iterator,
+    Mapping,
+    Optional,
+    Union,
+)
 
 import fuse
 from fuse import FUSE, FuseOSError, Operations
@@ -84,7 +91,7 @@ class QOMFuse(QOMCommand, Operations):
         self.fuse = FUSE(self, self.mount, foreground=True)
         return 0
 
-    def get_ino(self, path):
+    def get_ino(self, path: str) -> int:
         """Get an inode number for a given QOM path."""
         if path in self.ino_map:
             return self.ino_map[path]
@@ -92,7 +99,7 @@ class QOMFuse(QOMCommand, Operations):
         self.ino_count += 1
         return self.ino_map[path]
 
-    def is_object(self, path):
+    def is_object(self, path: str) -> bool:
         """Is the given QOM path an object?"""
         try:
             self.qom_list(path)
@@ -100,7 +107,7 @@ class QOMFuse(QOMCommand, Operations):
         except QMPResponseError:
             return False
 
-    def is_property(self, path):
+    def is_property(self, path: str) -> bool:
         """Is the given QOM path a property?"""
         path, prop = path.rsplit('/', 1)
         if path == '':
@@ -113,7 +120,7 @@ class QOMFuse(QOMCommand, Operations):
         except QMPResponseError:
             return False
 
-    def is_link(self, path):
+    def is_link(self, path: str) -> bool:
         """Is the given QOM path a link?"""
         path, prop = path.rsplit('/', 1)
         if path == '':
@@ -126,7 +133,7 @@ class QOMFuse(QOMCommand, Operations):
         except QMPResponseError:
             return False
 
-    def read(self, path, size, offset, fh):
+    def read(self, path: str, size: int, offset: int, fh: IO[bytes]) -> bytes:
         if not self.is_property(path):
             raise FuseOSError(ENOENT)
 
@@ -144,7 +151,7 @@ class QOMFuse(QOMCommand, Operations):
 
         return bytes(data[offset:][:size], encoding='utf-8')
 
-    def readlink(self, path):
+    def readlink(self, path: str) -> Union[bool, str]:
         if not self.is_link(path):
             return False
         path, prop = path.rsplit('/', 1)
@@ -152,7 +159,8 @@ class QOMFuse(QOMCommand, Operations):
         return prefix + str(self.qmp.command('qom-get', path=path,
                                              property=prop))
 
-    def getattr(self, path, fh=None):
+    def getattr(self, path: str,
+                fh: Optional[IO[bytes]] = None) -> Mapping[str, object]:
         if self.is_link(path):
             value = {
                 'st_mode': 0o755 | stat.S_IFLNK,
@@ -196,7 +204,7 @@ class QOMFuse(QOMCommand, Operations):
             raise FuseOSError(ENOENT)
         return value
 
-    def readdir(self, path, fh):
+    def readdir(self, path: str, fh: IO[bytes]) -> Iterator[str]:
         yield '.'
         yield '..'
         for item in self.qom_list(path):
-- 
2.26.2

[PATCH v2 13/72] scripts/qom-fuse: move to python/qemu/qmp/qom_fuse.py

[John Snow]

Move qom-fuse over to the python package now that it passes the
linter. Update the import paradigms so that it passes.

Immediately add 'fusepy' to the development requirements and re-lock the
pipenv so that the test suite continues to pass.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/Pipfile.lock                                 | 13 ++++++++++---
 scripts/qmp/qom-fuse => python/qemu/qmp/qom_fuse.py | 12 ++----------
 python/setup.cfg                                    |  1 +
 3 files changed, 13 insertions(+), 13 deletions(-)
 rename scripts/qmp/qom-fuse => python/qemu/qmp/qom_fuse.py (95%)
 mode change 100755 => 100644

diff --git a/python/Pipfile.lock b/python/Pipfile.lock
index 0352a628ef6f..d1e56cbcafc6 100644
--- a/python/Pipfile.lock
+++ b/python/Pipfile.lock
@@ -46,6 +46,12 @@
             "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3'",
             "version": "==3.8.4"
         },
+        "fusepy": {
+            "hashes": [
+                "sha256:72ff783ec2f43de3ab394e3f7457605bf04c8cf288a2f4068b4cde141d4ee6bd"
+            ],
+            "version": "==3.0.1"
+        },
         "importlib-metadata": {
             "hashes": [
                 "sha256:77a540690e24b0305878c37ffd421785a6f7e53c8b5720d211b211de8d0e95da",
@@ -208,10 +214,11 @@
         },
         "toml": {
             "hashes": [
-                "sha256:926b612be1e5ce0634a2ca03470f95169cf16f939018233a670519cb4ac58b0f",
-                "sha256:bda89d5935c2eac546d648028b9901107a595863cb36bae0c73ac804a9b4ce88"
+                "sha256:806143ae5bfb6a3c6e736a764057db0e6a0e05e338b5630894a5f779cabb4f9b",
+                "sha256:b3bda1d108d5dd99f4a20d24d9c348e91c4db7ab1b749200bded2f839ccbe68f"
             ],
-            "version": "==0.10.1"
+            "markers": "python_version >= '2.6' and python_version not in '3.0, 3.1, 3.2'",
+            "version": "==0.10.2"
         },
         "typed-ast": {
             "hashes": [
diff --git a/scripts/qmp/qom-fuse b/python/qemu/qmp/qom_fuse.py
old mode 100755
new mode 100644
similarity index 95%
rename from scripts/qmp/qom-fuse
rename to python/qemu/qmp/qom_fuse.py
index 22be36634967..b0adcc1f8e62
--- a/scripts/qmp/qom-fuse
+++ b/python/qemu/qmp/qom_fuse.py
@@ -1,4 +1,3 @@
-#!/usr/bin/env python3
 """
 QEMU Object Model FUSE filesystem tool
 
@@ -36,7 +35,6 @@
 
 import argparse
 from errno import ENOENT, EPERM
-import os
 import stat
 import sys
 from typing import (
@@ -51,10 +49,8 @@
 import fuse
 from fuse import FUSE, FuseOSError, Operations
 
-
-sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
-from qemu.qmp import QMPResponseError
-from qemu.qmp.qom_common import QOMCommand
+from . import QMPResponseError
+from .qom_common import QOMCommand
 
 
 fuse.fuse_python_api = (0, 2)
@@ -209,7 +205,3 @@ def readdir(self, path: str, fh: IO[bytes]) -> Iterator[str]:
         yield '..'
         for item in self.qom_list(path):
             yield item.name
-
-
-if __name__ == '__main__':
-    sys.exit(QOMFuse.entry_point())
diff --git a/python/setup.cfg b/python/setup.cfg
index 831555552f16..a9a76784d45a 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -27,6 +27,7 @@ devel =
     mypy >= 0.770
     pylint >= 2.6.0
     pytest >= 3.0.0
+    fusepy >= 2.0.4
 
 [options.entry_points]
 console_scripts =
-- 
2.26.2

[PATCH v2 14/72] scripts/qom-fuse: add redirection shim to python/qemu/qmp/qom-fuse.py

[John Snow]

By leaving the script absent for a commit, git-blame travels to the new
file instead of staying on the shim.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 scripts/qmp/qom-fuse | 11 +++++++++++
 1 file changed, 11 insertions(+)
 create mode 100755 scripts/qmp/qom-fuse

diff --git a/scripts/qmp/qom-fuse b/scripts/qmp/qom-fuse
new file mode 100755
index 000000000000..a58c8ef9793b
--- /dev/null
+++ b/scripts/qmp/qom-fuse
@@ -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.qom_fuse import QOMFuse
+
+
+if __name__ == '__main__':
+    sys.exit(QOMFuse.entry_point())
-- 
2.26.2

[PATCH v2 15/72] python: add fuse command to 'qom' tools

[John Snow]

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/qemu/qmp/qom.py | 12 +++++++++++-
 python/setup.cfg       |  1 +
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/python/qemu/qmp/qom.py b/python/qemu/qmp/qom.py
index 912d1809e60d..52e06a949ab3 100644
--- a/python/qemu/qmp/qom.py
+++ b/python/qemu/qmp/qom.py
@@ -1,7 +1,7 @@
 """
 QEMU Object Model testing tools.
 
-usage: qom.py [-h] {set,get,list,tree} ...
+usage: qom.py [-h] {set,get,list,tree,fuse} ...
 
 Query and manipulate QOM data
 
@@ -14,6 +14,7 @@
     get                Get a QOM property value
     list               List QOM properties at a given path
     tree               Show QOM tree from a given path
+    fuse               Mount a QOM tree as a FUSE filesystem
 """
 ##
 # Copyright John Snow 2020, for Red Hat, Inc.
@@ -36,6 +37,15 @@
 from .qom_common import QOMCommand
 
 
+try:
+    from .qom_fuse import QOMFuse
+except ImportError:
+    # either fusepy isn't installed, or qom_fuse itself is broken.
+    pass
+else:
+    assert issubclass(QOMFuse, QOMCommand)
+
+
 class QOMSet(QOMCommand):
     """QOM Command - Set a property to a given value."""
     name = 'set'
diff --git a/python/setup.cfg b/python/setup.cfg
index a9a76784d45a..908e53527ddc 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -36,6 +36,7 @@ console_scripts =
     qom-get = qemu.qmp.qom:QOMGet.entry_point
     qom-list = qemu.qmp.qom:QOMList.entry_point
     qom-tree = qemu.qmp.qom:QOMTree.entry_point
+    qom-fuse = qemu.qmp.qom_fuse:QOMFuse.entry_point
 
 [flake8]
 extend-ignore = E722  # Prefer pylint's bare-except checks to flake8's
-- 
2.26.2

[PATCH v2 16/72] python: add optional fuse dependency

[John Snow]

This can be opted into by installing "qemu[fuse]" instead of just
"qemu". The package ought to work perfectly well with or without it.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/setup.cfg | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/python/setup.cfg b/python/setup.cfg
index 908e53527ddc..4e1c638d72d5 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -29,6 +29,10 @@ devel =
     pytest >= 3.0.0
     fusepy >= 2.0.4
 
+# Provides qom-fuse functionality
+fuse =
+    fusepy >= 2.0.4
+
 [options.entry_points]
 console_scripts =
     qom = qemu.qmp.qom:main
-- 
2.26.2

[PATCH v2 17/72] scripts/qemu-ga-client: apply isort rules

[John Snow]

Hint:
> ln -s scripts/qmp/qemu-ga-client python/qemu/qmp/qemu_ga_client.py
> cd python
> isort qemu

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qemu-ga-client | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/scripts/qmp/qemu-ga-client b/scripts/qmp/qemu-ga-client
index ce122984a9ca..ae71b30dfa43 100755
--- a/scripts/qmp/qemu-ga-client
+++ b/scripts/qmp/qemu-ga-client
@@ -36,10 +36,11 @@
 # See also: https://wiki.qemu.org/Features/QAPI/GuestAgent
 #
 
-import os
-import sys
 import base64
+import os
 import random
+import sys
+
 
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
 from qemu import qmp
@@ -279,9 +280,9 @@ def main(address, cmd, args):
 
 
 if __name__ == '__main__':
-    import sys
-    import os
     import optparse
+    import os
+    import sys
 
     address = os.environ['QGA_CLIENT_ADDRESS'] if 'QGA_CLIENT_ADDRESS' in os.environ else None
 
-- 
2.26.2

[PATCH v2 18/72] scripts/qemu-ga-client: apply (most) flake8 rules

[John Snow]

- Line length should be < 80
- You shouldn't perform unscoped imports except at the top of the module

Notably, the sys.path hack creates problems with the import rule. This
will be fixed later.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qemu-ga-client | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/scripts/qmp/qemu-ga-client b/scripts/qmp/qemu-ga-client
index ae71b30dfa43..a12e0e9b2a6e 100755
--- a/scripts/qmp/qemu-ga-client
+++ b/scripts/qmp/qemu-ga-client
@@ -12,7 +12,8 @@
 # Start QEMU with:
 #
 # # qemu [...] -chardev socket,path=/tmp/qga.sock,server,nowait,id=qga0 \
-#   -device virtio-serial -device virtserialport,chardev=qga0,name=org.qemu.guest_agent.0
+#   -device virtio-serial \
+#   -device virtserialport,chardev=qga0,name=org.qemu.guest_agent.0
 #
 # Run the script:
 #
@@ -37,6 +38,7 @@
 #
 
 import base64
+import optparse
 import os
 import random
 import sys
@@ -94,9 +96,11 @@ class QemuGuestAgentClient:
         msgs = []
         msgs.append('version: ' + info['version'])
         msgs.append('supported_commands:')
-        enabled = [c['name'] for c in info['supported_commands'] if c['enabled']]
+        enabled = [c['name'] for c in info['supported_commands']
+                   if c['enabled']]
         msgs.append('\tenabled: ' + ', '.join(enabled))
-        disabled = [c['name'] for c in info['supported_commands'] if not c['enabled']]
+        disabled = [c['name'] for c in info['supported_commands']
+                    if not c['enabled']]
         msgs.append('\tdisabled: ' + ', '.join(disabled))
 
         return '\n'.join(msgs)
@@ -119,11 +123,11 @@ class QemuGuestAgentClient:
                     if ipaddr['ip-address-type'] == 'ipv4':
                         addr = ipaddr['ip-address']
                         mask = self.__gen_ipv4_netmask(int(ipaddr['prefix']))
-                        msgs.append("\tinet %s  netmask %s" % (addr, mask))
+                        msgs.append(f"\tinet {addr}  netmask {mask}")
                     elif ipaddr['ip-address-type'] == 'ipv6':
                         addr = ipaddr['ip-address']
                         prefix = ipaddr['prefix']
-                        msgs.append("\tinet6 %s  prefixlen %s" % (addr, prefix))
+                        msgs.append(f"\tinet6 {addr}  prefixlen {prefix}")
             if nif['hardware-address'] != '00:00:00:00:00:00':
                 msgs.append("\tether " + nif['hardware-address'])
 
@@ -237,6 +241,8 @@ def _cmd_suspend(client, args):
 
 def _cmd_shutdown(client, args):
     client.shutdown()
+
+
 _cmd_powerdown = _cmd_shutdown
 
 
@@ -280,17 +286,15 @@ def main(address, cmd, args):
 
 
 if __name__ == '__main__':
-    import optparse
-    import os
-    import sys
+    address = os.environ.get('QGA_CLIENT_ADDRESS')
 
-    address = os.environ['QGA_CLIENT_ADDRESS'] if 'QGA_CLIENT_ADDRESS' in os.environ else None
-
-    usage = "%prog [--address=<unix_path>|<ipv4_address>] <command> [args...]\n"
+    usage = ("%prog [--address=<unix_path>|<ipv4_address>]"
+             " <command> [args...]\n")
     usage += '<command>: ' + ', '.join(commands)
     parser = optparse.OptionParser(usage=usage)
     parser.add_option('--address', action='store', type='string',
-                      default=address, help='Specify a ip:port pair or a unix socket path')
+                      default=address,
+                      help='Specify a ip:port pair or a unix socket path')
     options, args = parser.parse_args()
 
     address = options.address
-- 
2.26.2

[PATCH v2 19/72] scripts/qemu-ga-client: Fix exception handling

[John Snow]

Fixes: 50d189c
Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qemu-ga-client | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/scripts/qmp/qemu-ga-client b/scripts/qmp/qemu-ga-client
index a12e0e9b2a6e..c4fbb6165d54 100755
--- a/scripts/qmp/qemu-ga-client
+++ b/scripts/qmp/qemu-ga-client
@@ -56,8 +56,6 @@ class QemuGuestAgent(qmp.QEMUMonitorProtocol):
 
 
 class QemuGuestAgentClient:
-    error = QemuGuestAgent.error
-
     def __init__(self, address):
         self.qga = QemuGuestAgent(address)
         self.qga.connect(negotiate=False)
@@ -137,7 +135,7 @@ class QemuGuestAgentClient:
         self.qga.settimeout(timeout)
         try:
             self.qga.ping()
-        except self.qga.timeout:
+        except TimeoutError:
             return False
         return True
 
@@ -269,11 +267,11 @@ def main(address, cmd, args):
 
     try:
         client = QemuGuestAgentClient(address)
-    except QemuGuestAgent.error as e:
+    except OSError as err:
         import errno
 
-        print(e)
-        if e.errno == errno.ECONNREFUSED:
+        print(err)
+        if err.errno == errno.ECONNREFUSED:
             print('Hint: qemu is not running?')
         sys.exit(1)
 
-- 
2.26.2

[PATCH v2 20/72] scripts/qemu-ga-client: replace deprecated optparse with argparse

[John Snow]

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qemu-ga-client | 32 +++++++++++++++-----------------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/scripts/qmp/qemu-ga-client b/scripts/qmp/qemu-ga-client
index c4fbb6165d54..73d262e8beb4 100755
--- a/scripts/qmp/qemu-ga-client
+++ b/scripts/qmp/qemu-ga-client
@@ -37,8 +37,8 @@
 # See also: https://wiki.qemu.org/Features/QAPI/GuestAgent
 #
 
+import argparse
 import base64
-import optparse
 import os
 import random
 import sys
@@ -255,7 +255,7 @@ def _cmd_reboot(client, args):
 commands = [m.replace('_cmd_', '') for m in dir() if '_cmd_' in m]
 
 
-def main(address, cmd, args):
+def send_command(address, cmd, args):
     if not os.path.exists(address):
         print('%s not found' % address)
         sys.exit(1)
@@ -283,25 +283,23 @@ def main(address, cmd, args):
     globals()['_cmd_' + cmd](client, args)
 
 
-if __name__ == '__main__':
+def main():
     address = os.environ.get('QGA_CLIENT_ADDRESS')
 
-    usage = ("%prog [--address=<unix_path>|<ipv4_address>]"
-             " <command> [args...]\n")
-    usage += '<command>: ' + ', '.join(commands)
-    parser = optparse.OptionParser(usage=usage)
-    parser.add_option('--address', action='store', type='string',
-                      default=address,
-                      help='Specify a ip:port pair or a unix socket path')
-    options, args = parser.parse_args()
+    parser = argparse.ArgumentParser()
+    parser.add_argument('--address', action='store',
+                        default=address,
+                        help='Specify a ip:port pair or a unix socket path')
+    parser.add_argument('command', choices=commands)
+    parser.add_argument('args', nargs='*')
 
-    address = options.address
-    if address is None:
+    args = parser.parse_args()
+    if args.address is None:
         parser.error('address is not specified')
         sys.exit(1)
 
-    if len(args) == 0:
-        parser.error('Less argument')
-        sys.exit(1)
+    send_command(args.address, args.command, args.args)
 
-    main(address, args[0], args[1:])
+
+if __name__ == '__main__':
+    main()
-- 
2.26.2

[PATCH v2 21/72] scripts/qemu-ga-client: add module docstring

[John Snow]

Turn that nice usage comment into a docstring.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qemu-ga-client | 65 +++++++++++++++++++-------------------
 1 file changed, 33 insertions(+), 32 deletions(-)

diff --git a/scripts/qmp/qemu-ga-client b/scripts/qmp/qemu-ga-client
index 73d262e8beb4..1061d2eb1437 100755
--- a/scripts/qmp/qemu-ga-client
+++ b/scripts/qmp/qemu-ga-client
@@ -1,41 +1,42 @@
 #!/usr/bin/env python3
 
-# QEMU Guest Agent Client
-#
 # Copyright (C) 2012 Ryota Ozaki <ozaki.ryota@gmail.com>
 #
 # This work is licensed under the terms of the GNU GPL, version 2.  See
 # the COPYING file in the top-level directory.
-#
-# Usage:
-#
-# Start QEMU with:
-#
-# # qemu [...] -chardev socket,path=/tmp/qga.sock,server,nowait,id=qga0 \
-#   -device virtio-serial \
-#   -device virtserialport,chardev=qga0,name=org.qemu.guest_agent.0
-#
-# Run the script:
-#
-# $ qemu-ga-client --address=/tmp/qga.sock <command> [args...]
-#
-# or
-#
-# $ export QGA_CLIENT_ADDRESS=/tmp/qga.sock
-# $ qemu-ga-client <command> [args...]
-#
-# For example:
-#
-# $ qemu-ga-client cat /etc/resolv.conf
-# # Generated by NetworkManager
-# nameserver 10.0.2.3
-# $ qemu-ga-client fsfreeze status
-# thawed
-# $ qemu-ga-client fsfreeze freeze
-# 2 filesystems frozen
-#
-# See also: https://wiki.qemu.org/Features/QAPI/GuestAgent
-#
+
+"""
+QEMU Guest Agent Client
+
+Usage:
+
+Start QEMU with:
+
+# qemu [...] -chardev socket,path=/tmp/qga.sock,server,nowait,id=qga0 \
+  -device virtio-serial \
+  -device virtserialport,chardev=qga0,name=org.qemu.guest_agent.0
+
+Run the script:
+
+$ qemu-ga-client --address=/tmp/qga.sock <command> [args...]
+
+or
+
+$ export QGA_CLIENT_ADDRESS=/tmp/qga.sock
+$ qemu-ga-client <command> [args...]
+
+For example:
+
+$ qemu-ga-client cat /etc/resolv.conf
+# Generated by NetworkManager
+nameserver 10.0.2.3
+$ qemu-ga-client fsfreeze status
+thawed
+$ qemu-ga-client fsfreeze freeze
+2 filesystems frozen
+
+See also: https://wiki.qemu.org/Features/QAPI/GuestAgent
+"""
 
 import argparse
 import base64
-- 
2.26.2

[PATCH v2 22/72] scripts/qemu-ga-client: apply (most) pylint rules

[John Snow]

I'm only doing a very quick best-effort to preserve this script, to help
keep it from breaking further. I think there are pending ideas swirling
on the right way to implement better SDKs and better clients, and this
script might be a handy reference for those discussions. It presents
some interesting design problems, like static type safety when using an
RPC-like mechanism.

So; disable docstrings for now. If you would like to help improve this
script, please add docstrings alongside any refactors or rejuvenations
you might apply at that time.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qemu-ga-client | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/scripts/qmp/qemu-ga-client b/scripts/qmp/qemu-ga-client
index 1061d2eb1437..59b91c78d884 100755
--- a/scripts/qmp/qemu-ga-client
+++ b/scripts/qmp/qemu-ga-client
@@ -40,6 +40,7 @@ See also: https://wiki.qemu.org/Features/QAPI/GuestAgent
 
 import argparse
 import base64
+import errno
 import os
 import random
 import sys
@@ -49,6 +50,13 @@ sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
 from qemu import qmp
 
 
+# This script has not seen many patches or careful attention in quite
+# some time. If you would like to improve it, please review the design
+# carefully and add docstrings at that point in time. Until then:
+
+# pylint: disable=missing-docstring
+
+
 class QemuGuestAgent(qmp.QEMUMonitorProtocol):
     def __getattr__(self, name):
         def wrapper(**kwds):
@@ -104,7 +112,8 @@ class QemuGuestAgentClient:
 
         return '\n'.join(msgs)
 
-    def __gen_ipv4_netmask(self, prefixlen):
+    @classmethod
+    def __gen_ipv4_netmask(cls, prefixlen):
         mask = int('1' * prefixlen + '0' * (32 - prefixlen), 2)
         return '.'.join([str(mask >> 24),
                          str((mask >> 16) & 0xff),
@@ -207,10 +216,12 @@ def _cmd_fstrim(client, args):
 
 
 def _cmd_ifconfig(client, args):
+    assert not args
     print(client.ifconfig())
 
 
 def _cmd_info(client, args):
+    assert not args
     print(client.info())
 
 
@@ -239,6 +250,7 @@ def _cmd_suspend(client, args):
 
 
 def _cmd_shutdown(client, args):
+    assert not args
     client.shutdown()
 
 
@@ -246,10 +258,12 @@ _cmd_powerdown = _cmd_shutdown
 
 
 def _cmd_halt(client, args):
+    assert not args
     client.shutdown('halt')
 
 
 def _cmd_reboot(client, args):
+    assert not args
     client.shutdown('reboot')
 
 
@@ -269,8 +283,6 @@ def send_command(address, cmd, args):
     try:
         client = QemuGuestAgentClient(address)
     except OSError as err:
-        import errno
-
         print(err)
         if err.errno == errno.ECONNREFUSED:
             print('Hint: qemu is not running?')
-- 
2.26.2

[PATCH v2 23/72] python/qmp: Correct type of QMPReturnValue

[John Snow]

It's only a Dict[str, Any] most of the time. It's not actually
guaranteed to be anything in particular. Fix this type to be
more accurate to the reality we live in.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/qemu/qmp/__init__.py | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/python/qemu/qmp/__init__.py b/python/qemu/qmp/__init__.py
index 9606248a3d2e..7355a558796f 100644
--- a/python/qemu/qmp/__init__.py
+++ b/python/qemu/qmp/__init__.py
@@ -35,14 +35,19 @@
 )
 
 
-# QMPMessage is a QMP Message of any kind.
-# e.g. {'yee': 'haw'}
-#
-# QMPReturnValue is the inner value of return values only.
-# {'return': {}} is the QMPMessage,
-# {} is the QMPReturnValue.
+#: QMPMessage is an entire QMP message of any kind.
 QMPMessage = Dict[str, Any]
-QMPReturnValue = Dict[str, Any]
+
+#: QMPReturnValue is the 'return' value of a command.
+QMPReturnValue = 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.
+
 
 InternetAddrT = Tuple[str, str]
 UnixAddrT = str
@@ -271,8 +276,8 @@ def cmd_obj(self, qmp_cmd: QMPMessage) -> QMPMessage:
         return resp
 
     def cmd(self, name: str,
-            args: Optional[Dict[str, Any]] = None,
-            cmd_id: Optional[Any] = None) -> QMPMessage:
+            args: Optional[Dict[str, object]] = None,
+            cmd_id: Optional[object] = None) -> QMPMessage:
         """
         Build a QMP command and send it to the QMP Monitor.
 
@@ -287,7 +292,7 @@ def cmd(self, name: str,
             qmp_cmd['id'] = cmd_id
         return self.cmd_obj(qmp_cmd)
 
-    def command(self, cmd: str, **kwds: Any) -> QMPReturnValue:
+    def command(self, cmd: str, **kwds: object) -> QMPReturnValue:
         """
         Build and send a QMP command to the monitor, report errors if any
         """
-- 
2.26.2

[PATCH v2 24/72] scripts/qmp/qemu-ga-client: add mypy type hints

[John Snow]

This script is in slightly rough shape, but it still works. A lot of
care went into its initial development. In good faith, I'm updating it
to the latest Python coding standards. If there is in interest in this
script, though, I'll be asking for a contributor to take care of it
further.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qemu-ga-client | 89 +++++++++++++++++++++-----------------
 1 file changed, 49 insertions(+), 40 deletions(-)

diff --git a/scripts/qmp/qemu-ga-client b/scripts/qmp/qemu-ga-client
index 59b91c78d884..3e617e7e7abe 100755
--- a/scripts/qmp/qemu-ga-client
+++ b/scripts/qmp/qemu-ga-client
@@ -44,10 +44,18 @@ import errno
 import os
 import random
 import sys
+from typing import (
+    Any,
+    Callable,
+    Dict,
+    Optional,
+    Sequence,
+)
 
 
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
 from qemu import qmp
+from qemu.qmp import SocketAddrT
 
 
 # This script has not seen many patches or careful attention in quite
@@ -58,18 +66,18 @@ from qemu import qmp
 
 
 class QemuGuestAgent(qmp.QEMUMonitorProtocol):
-    def __getattr__(self, name):
-        def wrapper(**kwds):
+    def __getattr__(self, name: str) -> Callable[..., Any]:
+        def wrapper(**kwds: object) -> object:
             return self.command('guest-' + name.replace('_', '-'), **kwds)
         return wrapper
 
 
 class QemuGuestAgentClient:
-    def __init__(self, address):
+    def __init__(self, address: SocketAddrT):
         self.qga = QemuGuestAgent(address)
         self.qga.connect(negotiate=False)
 
-    def sync(self, timeout=3):
+    def sync(self, timeout: Optional[float] = 3) -> None:
         # Avoid being blocked forever
         if not self.ping(timeout):
             raise EnvironmentError('Agent seems not alive')
@@ -79,9 +87,9 @@ class QemuGuestAgentClient:
             if isinstance(ret, int) and int(ret) == uid:
                 break
 
-    def __file_read_all(self, handle):
+    def __file_read_all(self, handle: int) -> bytes:
         eof = False
-        data = ''
+        data = b''
         while not eof:
             ret = self.qga.file_read(handle=handle, count=1024)
             _data = base64.b64decode(ret['buf-b64'])
@@ -89,7 +97,7 @@ class QemuGuestAgentClient:
             eof = ret['eof']
         return data
 
-    def read(self, path):
+    def read(self, path: str) -> bytes:
         handle = self.qga.file_open(path=path)
         try:
             data = self.__file_read_all(handle)
@@ -97,7 +105,7 @@ class QemuGuestAgentClient:
             self.qga.file_close(handle=handle)
         return data
 
-    def info(self):
+    def info(self) -> str:
         info = self.qga.info()
 
         msgs = []
@@ -113,14 +121,14 @@ class QemuGuestAgentClient:
         return '\n'.join(msgs)
 
     @classmethod
-    def __gen_ipv4_netmask(cls, prefixlen):
+    def __gen_ipv4_netmask(cls, prefixlen: int) -> str:
         mask = int('1' * prefixlen + '0' * (32 - prefixlen), 2)
         return '.'.join([str(mask >> 24),
                          str((mask >> 16) & 0xff),
                          str((mask >> 8) & 0xff),
                          str(mask & 0xff)])
 
-    def ifconfig(self):
+    def ifconfig(self) -> str:
         nifs = self.qga.network_get_interfaces()
 
         msgs = []
@@ -141,7 +149,7 @@ class QemuGuestAgentClient:
 
         return '\n'.join(msgs)
 
-    def ping(self, timeout):
+    def ping(self, timeout: Optional[float]) -> bool:
         self.qga.settimeout(timeout)
         try:
             self.qga.ping()
@@ -149,37 +157,40 @@ class QemuGuestAgentClient:
             return False
         return True
 
-    def fsfreeze(self, cmd):
+    def fsfreeze(self, cmd: str) -> object:
         if cmd not in ['status', 'freeze', 'thaw']:
             raise Exception('Invalid command: ' + cmd)
-
+        # Can be int (freeze, thaw) or GuestFsfreezeStatus (status)
         return getattr(self.qga, 'fsfreeze' + '_' + cmd)()
 
-    def fstrim(self, minimum=0):
-        return getattr(self.qga, 'fstrim')(minimum=minimum)
+    def fstrim(self, minimum: int) -> Dict[str, object]:
+        # returns GuestFilesystemTrimResponse
+        ret = getattr(self.qga, 'fstrim')(minimum=minimum)
+        assert isinstance(ret, dict)
+        return ret
 
-    def suspend(self, mode):
+    def suspend(self, mode: str) -> None:
         if mode not in ['disk', 'ram', 'hybrid']:
             raise Exception('Invalid mode: ' + mode)
 
         try:
             getattr(self.qga, 'suspend' + '_' + mode)()
             # On error exception will raise
-        except self.qga.timeout:
+        except TimeoutError:
             # On success command will timed out
             return
 
-    def shutdown(self, mode='powerdown'):
+    def shutdown(self, mode: str = 'powerdown') -> None:
         if mode not in ['powerdown', 'halt', 'reboot']:
             raise Exception('Invalid mode: ' + mode)
 
         try:
             self.qga.shutdown(mode=mode)
-        except self.qga.timeout:
-            return
+        except TimeoutError:
+            pass
 
 
-def _cmd_cat(client, args):
+def _cmd_cat(client: QemuGuestAgentClient, args: Sequence[str]) -> None:
     if len(args) != 1:
         print('Invalid argument')
         print('Usage: cat <file>')
@@ -187,7 +198,7 @@ def _cmd_cat(client, args):
     print(client.read(args[0]))
 
 
-def _cmd_fsfreeze(client, args):
+def _cmd_fsfreeze(client: QemuGuestAgentClient, args: Sequence[str]) -> None:
     usage = 'Usage: fsfreeze status|freeze|thaw'
     if len(args) != 1:
         print('Invalid argument')
@@ -201,13 +212,14 @@ def _cmd_fsfreeze(client, args):
     ret = client.fsfreeze(cmd)
     if cmd == 'status':
         print(ret)
-    elif cmd == 'freeze':
-        print("%d filesystems frozen" % ret)
-    else:
-        print("%d filesystems thawed" % ret)
+        return
 
+    assert isinstance(ret, int)
+    verb = 'frozen' if cmd == 'freeze' else 'thawed'
+    print(f"{ret:d} filesystems {verb}")
 
-def _cmd_fstrim(client, args):
+
+def _cmd_fstrim(client: QemuGuestAgentClient, args: Sequence[str]) -> None:
     if len(args) == 0:
         minimum = 0
     else:
@@ -215,28 +227,25 @@ def _cmd_fstrim(client, args):
     print(client.fstrim(minimum))
 
 
-def _cmd_ifconfig(client, args):
+def _cmd_ifconfig(client: QemuGuestAgentClient, args: Sequence[str]) -> None:
     assert not args
     print(client.ifconfig())
 
 
-def _cmd_info(client, args):
+def _cmd_info(client: QemuGuestAgentClient, args: Sequence[str]) -> None:
     assert not args
     print(client.info())
 
 
-def _cmd_ping(client, args):
-    if len(args) == 0:
-        timeout = 3
-    else:
-        timeout = float(args[0])
+def _cmd_ping(client: QemuGuestAgentClient, args: Sequence[str]) -> None:
+    timeout = 3.0 if len(args) == 0 else float(args[0])
     alive = client.ping(timeout)
     if not alive:
         print("Not responded in %s sec" % args[0])
         sys.exit(1)
 
 
-def _cmd_suspend(client, args):
+def _cmd_suspend(client: QemuGuestAgentClient, args: Sequence[str]) -> None:
     usage = 'Usage: suspend disk|ram|hybrid'
     if len(args) != 1:
         print('Less argument')
@@ -249,7 +258,7 @@ def _cmd_suspend(client, args):
     client.suspend(args[0])
 
 
-def _cmd_shutdown(client, args):
+def _cmd_shutdown(client: QemuGuestAgentClient, args: Sequence[str]) -> None:
     assert not args
     client.shutdown()
 
@@ -257,12 +266,12 @@ def _cmd_shutdown(client, args):
 _cmd_powerdown = _cmd_shutdown
 
 
-def _cmd_halt(client, args):
+def _cmd_halt(client: QemuGuestAgentClient, args: Sequence[str]) -> None:
     assert not args
     client.shutdown('halt')
 
 
-def _cmd_reboot(client, args):
+def _cmd_reboot(client: QemuGuestAgentClient, args: Sequence[str]) -> None:
     assert not args
     client.shutdown('reboot')
 
@@ -270,7 +279,7 @@ def _cmd_reboot(client, args):
 commands = [m.replace('_cmd_', '') for m in dir() if '_cmd_' in m]
 
 
-def send_command(address, cmd, args):
+def send_command(address: str, cmd: str, args: Sequence[str]) -> None:
     if not os.path.exists(address):
         print('%s not found' % address)
         sys.exit(1)
@@ -296,7 +305,7 @@ def send_command(address, cmd, args):
     globals()['_cmd_' + cmd](client, args)
 
 
-def main():
+def main() -> None:
     address = os.environ.get('QGA_CLIENT_ADDRESS')
 
     parser = argparse.ArgumentParser()
-- 
2.26.2

[PATCH v2 25/72] scripts/qemu-ga-client: move to python/qemu/qmp/qemu_ga_client.py

[John Snow]

The script will be unavailable for a few commits before being
restored. This helps move git history into the new file. To prevent
linter regressions, though, we do need to immediately touch up the
filename, remove the executable bit, and change the import paradigm.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qemu-ga-client => python/qemu/qmp/qemu_ga_client.py | 2 --
 1 file changed, 2 deletions(-)
 rename scripts/qmp/qemu-ga-client => python/qemu/qmp/qemu_ga_client.py (99%)

diff --git a/scripts/qmp/qemu-ga-client b/python/qemu/qmp/qemu_ga_client.py
similarity index 99%
rename from scripts/qmp/qemu-ga-client
rename to python/qemu/qmp/qemu_ga_client.py
index 3e617e7e7abe..43abc023c63c 100755
--- a/scripts/qmp/qemu-ga-client
+++ b/python/qemu/qmp/qemu_ga_client.py
@@ -52,8 +52,6 @@
     Sequence,
 )
 
-
-sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
 from qemu import qmp
 from qemu.qmp import SocketAddrT
 
-- 
2.26.2

[PATCH v2 26/72] python/qemu-ga-client: add entry point

[John Snow]

Remove the shebang, and add a package-defined entry point instead.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/qemu/qmp/qemu_ga_client.py | 2 --
 python/setup.cfg                  | 1 +
 2 files changed, 1 insertion(+), 2 deletions(-)
 mode change 100755 => 100644 python/qemu/qmp/qemu_ga_client.py

diff --git a/python/qemu/qmp/qemu_ga_client.py b/python/qemu/qmp/qemu_ga_client.py
old mode 100755
new mode 100644
index 43abc023c63c..6915e906374b
--- a/python/qemu/qmp/qemu_ga_client.py
+++ b/python/qemu/qmp/qemu_ga_client.py
@@ -1,5 +1,3 @@
-#!/usr/bin/env python3
-
 # Copyright (C) 2012 Ryota Ozaki <ozaki.ryota@gmail.com>
 #
 # This work is licensed under the terms of the GNU GPL, version 2.  See
diff --git a/python/setup.cfg b/python/setup.cfg
index 4e1c638d72d5..f2f54bcaefe8 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -41,6 +41,7 @@ console_scripts =
     qom-list = qemu.qmp.qom:QOMList.entry_point
     qom-tree = qemu.qmp.qom:QOMTree.entry_point
     qom-fuse = qemu.qmp.qom_fuse:QOMFuse.entry_point
+    qemu-ga-client = qemu.qmp.qemu_ga_client:main
 
 [flake8]
 extend-ignore = E722  # Prefer pylint's bare-except checks to flake8's
-- 
2.26.2

[PATCH v2 27/72] scripts/qemu-ga-client: Add forwarder stub

[John Snow]

Mimicking other recently moved tools: add a little forwarder stub until
we are sure that everyone is comfortable understanding how to use the
tools in their new location.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qemu-ga-client | 11 +++++++++++
 1 file changed, 11 insertions(+)
 create mode 100755 scripts/qmp/qemu-ga-client

diff --git a/scripts/qmp/qemu-ga-client b/scripts/qmp/qemu-ga-client
new file mode 100755
index 000000000000..102fd2cad93f
--- /dev/null
+++ b/scripts/qmp/qemu-ga-client
@@ -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 qemu_ga_client
+
+
+if __name__ == '__main__':
+    sys.exit(qemu_ga_client.main())
-- 
2.26.2

[PATCH v2 28/72] scripts/qmp-shell: apply isort rules

[John Snow]

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index b4d06096abd6..7ed45b8195b1 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -65,18 +65,20 @@
 # which will echo back the properly formatted JSON-compliant QMP that is being
 # sent to QEMU, which is useful for debugging and documentation generation.
 
-import json
 import ast
-import readline
-import sys
-import os
-import errno
 import atexit
+import errno
+import json
+import os
 import re
+import readline
+import sys
+
 
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
 from qemu import qmp
 
+
 class QMPCompleter(list):
     def complete(self, text, state):
         for cmd in self:
-- 
2.26.2

[PATCH v2 29/72] scripts/qmp-shell: Apply flake8 rules

[John Snow]

A lot of fiddling around to get us below 80 columns.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 68 +++++++++++++++++++++++++++++--------------
 1 file changed, 46 insertions(+), 22 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 7ed45b8195b1..20c420ba4251 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -88,12 +88,15 @@ class QMPCompleter(list):
                 else:
                     state -= 1
 
+
 class QMPShellError(Exception):
     pass
 
+
 class QMPShellBadPort(QMPShellError):
     pass
 
+
 class FuzzyJSON(ast.NodeTransformer):
     '''This extension of ast.NodeTransformer filters literal "true/false/null"
     values in an AST and replaces them by proper "True/False/None" values that
@@ -107,6 +110,7 @@ class FuzzyJSON(ast.NodeTransformer):
             node.id = 'None'
         return node
 
+
 # TODO: QMPShell's interface is a bit ugly (eg. _fill_completion() and
 #       _execute_cmd()). Let's design a better one.
 class QMPShell(qmp.QEMUMonitorProtocol):
@@ -131,7 +135,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
                 port = int(addr[1])
             except ValueError:
                 raise QMPShellBadPort
-            return ( addr[0], port )
+            return addr[0], port
         # socket path
         return arg
 
@@ -148,8 +152,8 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         readline.set_history_length(1024)
         readline.set_completer(self._completer.complete)
         readline.parse_and_bind("tab: complete")
-        # XXX: default delimiters conflict with some command names (eg. query-),
-        # clearing everything as it doesn't seem to matter
+        # NB: default delimiters conflict with some command names
+        # (eg. query-), clearing everything as it doesn't seem to matter
         readline.set_completer_delims('')
         try:
             readline.read_history_file(self._histfile)
@@ -197,7 +201,9 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         for arg in tokens:
             (key, sep, val) = arg.partition('=')
             if sep != '=':
-                raise QMPShellError("Expected a key=value pair, got '%s'" % arg)
+                raise QMPShellError(
+                    f"Expected a key=value pair, got '{arg!s}'"
+                )
 
             value = self.__parse_value(val)
             optpath = key.split('.')
@@ -206,14 +212,16 @@ class QMPShell(qmp.QEMUMonitorProtocol):
                 curpath.append(p)
                 d = parent.get(p, {})
                 if type(d) is not dict:
-                    raise QMPShellError('Cannot use "%s" as both leaf and non-leaf key' % '.'.join(curpath))
+                    msg = 'Cannot use "{:s}" as both leaf and non-leaf key'
+                    raise QMPShellError(msg.format('.'.join(curpath)))
                 parent[p] = d
                 parent = d
             if optpath[-1] in parent:
                 if type(parent[optpath[-1]]) is dict:
-                    raise QMPShellError('Cannot use "%s" as both leaf and non-leaf key' % '.'.join(curpath))
+                    msg = 'Cannot use "{:s}" as both leaf and non-leaf key'
+                    raise QMPShellError(msg.format('.'.join(curpath)))
                 else:
-                    raise QMPShellError('Cannot set "%s" multiple times' % key)
+                    raise QMPShellError(f'Cannot set "{key}" multiple times')
             parent[optpath[-1]] = value
 
     def __build_cmd(self, cmdline):
@@ -223,7 +231,8 @@ class QMPShell(qmp.QEMUMonitorProtocol):
 
             < command-name > [ arg-name1=arg1 ] ... [ arg-nameN=argN ]
         """
-        cmdargs = re.findall(r'''(?:[^\s"']|"(?:\\.|[^"])*"|'(?:\\.|[^'])*')+''', cmdline)
+        argument_regex = r'''(?:[^\s"']|"(?:\\.|[^"])*"|'(?:\\.|[^'])*')+'''
+        cmdargs = re.findall(argument_regex, cmdline)
 
         # Transactional CLI entry/exit:
         if cmdargs[0] == 'transaction(':
@@ -232,9 +241,12 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         elif cmdargs[0] == ')' and self._transmode:
             self._transmode = False
             if len(cmdargs) > 1:
-                raise QMPShellError("Unexpected input after close of Transaction sub-shell")
-            qmpcmd = { 'execute': 'transaction',
-                       'arguments': { 'actions': self._actions } }
+                msg = 'Unexpected input after close of Transaction sub-shell'
+                raise QMPShellError(msg)
+            qmpcmd = {
+                'execute': 'transaction',
+                'arguments': {'actions': self._actions}
+            }
             self._actions = list()
             return qmpcmd
 
@@ -245,7 +257,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         # Parse and then cache this Transactional Action
         if self._transmode:
             finalize = False
-            action = { 'type': cmdargs[0], 'data': {} }
+            action = {'type': cmdargs[0], 'data': {}}
             if cmdargs[-1] == ')':
                 cmdargs.pop(-1)
                 finalize = True
@@ -254,7 +266,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
             return self.__build_cmd(')') if finalize else None
 
         # Standard command: parse and return it to be executed.
-        qmpcmd = { 'execute': cmdargs[0], 'arguments': {} }
+        qmpcmd = {'execute': cmdargs[0], 'arguments': {}}
         self.__cli_expr(cmdargs[1:], qmpcmd['arguments'])
         return qmpcmd
 
@@ -295,7 +307,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
             print('Connected')
             return
         version = self._greeting['QMP']['version']['qemu']
-        print('Connected to QEMU %d.%d.%d\n' % (version['major'],version['minor'],version['micro']))
+        print("Connected to QEMU {major}.{minor}.{micro}\n".format(**version))
 
     def get_prompt(self):
         if self._transmode:
@@ -324,6 +336,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
     def set_verbosity(self, verbose):
         self._verbose = verbose
 
+
 class HMPShell(QMPShell):
     def __init__(self, address):
         QMPShell.__init__(self, address)
@@ -332,7 +345,7 @@ class HMPShell(QMPShell):
     def __cmd_completion(self):
         for cmd in self.__cmd_passthrough('help')['return'].split('\r\n'):
             if cmd and cmd[0] != '[' and cmd[0] != '\t':
-                name = cmd.split()[0] # drop help text
+                name = cmd.split()[0]  # drop help text
                 if name == 'info':
                     continue
                 if name.find('|') != -1:
@@ -344,7 +357,7 @@ class HMPShell(QMPShell):
                     else:
                         name = opt[0]
                 self._completer.append(name)
-                self._completer.append('help ' + name) # help completion
+                self._completer.append('help ' + name)  # help completion
 
     def __info_completion(self):
         for cmd in self.__cmd_passthrough('info')['return'].split('\r\n'):
@@ -360,17 +373,21 @@ class HMPShell(QMPShell):
         self.__info_completion()
         self.__other_completion()
 
-    def __cmd_passthrough(self, cmdline, cpu_index = 0):
-        return self.cmd_obj({ 'execute': 'human-monitor-command', 'arguments':
-                              { 'command-line': cmdline,
-                                'cpu-index': cpu_index } })
+    def __cmd_passthrough(self, cmdline, cpu_index=0):
+        return self.cmd_obj({
+            'execute': 'human-monitor-command',
+            'arguments': {
+                'command-line': cmdline,
+                'cpu-index': cpu_index
+            }
+        })
 
     def _execute_cmd(self, cmdline):
         if cmdline.split()[0] == "cpu":
             # trap the cpu command, it requires special setting
             try:
                 idx = int(cmdline.split()[1])
-                if not 'return' in self.__cmd_passthrough('info version', idx):
+                if 'return' not in self.__cmd_passthrough('info version', idx):
                     print('bad CPU index')
                     return True
                 self.__cpu_index = idx
@@ -394,20 +411,26 @@ class HMPShell(QMPShell):
     def show_banner(self):
         QMPShell.show_banner(self, msg='Welcome to the HMP shell!')
 
+
 def die(msg):
     sys.stderr.write('ERROR: %s\n' % msg)
     sys.exit(1)
 
+
 def fail_cmdline(option=None):
     if option:
         sys.stderr.write('ERROR: bad command-line option \'%s\'\n' % option)
-    sys.stderr.write('qmp-shell [ -v ] [ -p ] [ -H ] [ -N ] < UNIX socket path> | < TCP address:port >\n')
+    sys.stderr.write(
+        'qmp-shell [ -v ] [ -p ] [ -H ] [ -N ] '
+        '< UNIX socket path> | < TCP address:port >\n'
+    )
     sys.stderr.write('    -v     Verbose (echo command sent and received)\n')
     sys.stderr.write('    -p     Pretty-print JSON\n')
     sys.stderr.write('    -H     Use HMP interface\n')
     sys.stderr.write('    -N     Skip negotiate (for qemu-ga)\n')
     sys.exit(1)
 
+
 def main():
     addr = ''
     qemu = None
@@ -457,5 +480,6 @@ def main():
         pass
     qemu.close()
 
+
 if __name__ == '__main__':
     main()
-- 
2.26.2

[PATCH v2 30/72] scripts/qmp-shell: fix show_banner signature

[John Snow]

The signatures need to match.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 20c420ba4251..e2ad979aad98 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -408,8 +408,8 @@ class HMPShell(QMPShell):
             print('%s: %s' % (resp['error']['class'], resp['error']['desc']))
         return True
 
-    def show_banner(self):
-        QMPShell.show_banner(self, msg='Welcome to the HMP shell!')
+    def show_banner(self, msg='Welcome to the HMP shell!'):
+        QMPShell.show_banner(self, msg)
 
 
 def die(msg):
-- 
2.26.2

[PATCH v2 31/72] scripts/qmp-shell: fix exception handling

[John Snow]

Fixes: 50d189c
Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index e2ad979aad98..330f8b32f9c2 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -471,7 +471,7 @@ def main():
         die('Didn\'t get QMP greeting message')
     except qmp.QMPCapabilitiesError:
         die('Could not negotiate capabilities')
-    except qemu.error:
+    except OSError:
         die('Could not connect to %s' % addr)
 
     qemu.show_banner()
-- 
2.26.2

[PATCH v2 32/72] scripts/qmp-shell: fix connect method signature

[John Snow]

It needs to match the parent's. (The negotiate parameter must be optional.)

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 330f8b32f9c2..27632ed5fee7 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -297,7 +297,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         self._print(resp)
         return True
 
-    def connect(self, negotiate):
+    def connect(self, negotiate: bool = True):
         self._greeting = super(QMPShell, self).connect(negotiate)
         self.__completer_setup()
 
-- 
2.26.2

[PATCH v2 33/72] scripts/qmp-shell: remove shadowed variable from _print()

[John Snow]

Don't use 'qmp' here, which shadows the qmp module.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 27632ed5fee7..6767a01eaaed 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -270,11 +270,11 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         self.__cli_expr(cmdargs[1:], qmpcmd['arguments'])
         return qmpcmd
 
-    def _print(self, qmp):
+    def _print(self, qmp_message):
         indent = None
         if self._pretty:
             indent = 4
-        jsobj = json.dumps(qmp, indent=indent, sort_keys=self._pretty)
+        jsobj = json.dumps(qmp_message, indent=indent, sort_keys=self._pretty)
         print(str(jsobj))
 
     def _execute_cmd(self, cmdline):
-- 
2.26.2

[PATCH v2 34/72] scripts/qmp-shell: use @classmethod where appropriate

[John Snow]

Methods with no self-use should belong to the class.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 6767a01eaaed..69359ec8ad7f 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -101,7 +101,8 @@ class FuzzyJSON(ast.NodeTransformer):
     '''This extension of ast.NodeTransformer filters literal "true/false/null"
     values in an AST and replaces them by proper "True/False/None" values that
     Python can properly evaluate.'''
-    def visit_Name(self, node):
+    @classmethod
+    def visit_Name(cls, node):
         if node.id == 'true':
             node.id = 'True'
         if node.id == 'false':
@@ -124,7 +125,8 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         self._histfile = os.path.join(os.path.expanduser('~'),
                                       '.qmp-shell_history')
 
-    def __get_address(self, arg):
+    @classmethod
+    def __get_address(cls, arg):
         """
         Figure out if the argument is in the port:host form, if it's not it's
         probably a file path.
@@ -171,7 +173,8 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         except Exception as e:
             print("Failed to save history file '%s'; %s" % (self._histfile, e))
 
-    def __parse_value(self, val):
+    @classmethod
+    def __parse_value(cls, val):
         try:
             return int(val)
         except ValueError:
-- 
2.26.2

[PATCH v2 35/72] scripts/qmp-shell: Use python3-style super()

[John Snow]

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 69359ec8ad7f..2e9ae5d59bd9 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -116,7 +116,7 @@ class FuzzyJSON(ast.NodeTransformer):
 #       _execute_cmd()). Let's design a better one.
 class QMPShell(qmp.QEMUMonitorProtocol):
     def __init__(self, address, pretty=False):
-        super(QMPShell, self).__init__(self.__get_address(address))
+        super().__init__(self.__get_address(address))
         self._greeting = None
         self._completer = None
         self._pretty = pretty
@@ -301,7 +301,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         return True
 
     def connect(self, negotiate: bool = True):
-        self._greeting = super(QMPShell, self).connect(negotiate)
+        self._greeting = super().connect(negotiate)
         self.__completer_setup()
 
     def show_banner(self, msg='Welcome to the QMP low-level shell!'):
@@ -342,7 +342,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
 
 class HMPShell(QMPShell):
     def __init__(self, address):
-        QMPShell.__init__(self, address)
+        super().__init__(address)
         self.__cpu_index = 0
 
     def __cmd_completion(self):
-- 
2.26.2

[PATCH v2 36/72] scripts/qmp-shell: declare verbose in __init__

[John Snow]

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 2e9ae5d59bd9..7233c4e00b89 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -124,6 +124,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         self._actions = list()
         self._histfile = os.path.join(os.path.expanduser('~'),
                                       '.qmp-shell_history')
+        self._verbose = False
 
     @classmethod
     def __get_address(cls, arg):
-- 
2.26.2

[PATCH v2 37/72] scripts/qmp-shell: use triple-double-quote docstring style

[John Snow]

(2014 me had never written python before.)

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 7233c4e00b89..73b2c099f0f0 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -98,9 +98,12 @@ class QMPShellBadPort(QMPShellError):
 
 
 class FuzzyJSON(ast.NodeTransformer):
-    '''This extension of ast.NodeTransformer filters literal "true/false/null"
+    """
+    This extension of ast.NodeTransformer filters literal "true/false/null"
     values in an AST and replaces them by proper "True/False/None" values that
-    Python can properly evaluate.'''
+    Python can properly evaluate.
+    """
+
     @classmethod
     def visit_Name(cls, node):
         if node.id == 'true':
-- 
2.26.2

[PATCH v2 38/72] scripts/qmp-shell: ignore visit_Name name

[John Snow]

Not something I control, sorry, pylint.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 73b2c099f0f0..0f03c6c89575 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -105,7 +105,7 @@ class FuzzyJSON(ast.NodeTransformer):
     """
 
     @classmethod
-    def visit_Name(cls, node):
+    def visit_Name(cls, node):  # pylint: disable=invalid-name
         if node.id == 'true':
             node.id = 'True'
         if node.id == 'false':
-- 
2.26.2

[PATCH v2 39/72] scripts/qmp-shell: make QMPCompleter returns explicit

[John Snow]

This function returns None when it doesn't find a match; do that
explicitly.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 0f03c6c89575..e4163daf438d 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -83,10 +83,10 @@ class QMPCompleter(list):
     def complete(self, text, state):
         for cmd in self:
             if cmd.startswith(text):
-                if not state:
+                if state == 0:
                     return cmd
-                else:
-                    state -= 1
+                state -= 1
+        return None
 
 
 class QMPShellError(Exception):
-- 
2.26.2

[PATCH v2 40/72] scripts/qmp-shell: rename one and two-letter variables

[John Snow]

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index e4163daf438d..2fd677e3dabd 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -196,8 +196,8 @@ class QMPShell(qmp.QEMUMonitorProtocol):
                 pass
             # Try once again as FuzzyJSON:
             try:
-                st = ast.parse(val, mode='eval')
-                return ast.literal_eval(FuzzyJSON().visit(st))
+                tree = ast.parse(val, mode='eval')
+                return ast.literal_eval(FuzzyJSON().visit(tree))
             except SyntaxError:
                 pass
             except ValueError:
@@ -215,14 +215,14 @@ class QMPShell(qmp.QEMUMonitorProtocol):
             value = self.__parse_value(val)
             optpath = key.split('.')
             curpath = []
-            for p in optpath[:-1]:
-                curpath.append(p)
-                d = parent.get(p, {})
-                if type(d) is not dict:
+            for path in optpath[:-1]:
+                curpath.append(path)
+                obj = parent.get(path, {})
+                if type(obj) is not dict:
                     msg = 'Cannot use "{:s}" as both leaf and non-leaf key'
                     raise QMPShellError(msg.format('.'.join(curpath)))
-                parent[p] = d
-                parent = d
+                parent[path] = obj
+                parent = obj
             if optpath[-1] in parent:
                 if type(parent[optpath[-1]]) is dict:
                     msg = 'Cannot use "{:s}" as both leaf and non-leaf key'
@@ -287,8 +287,8 @@ class QMPShell(qmp.QEMUMonitorProtocol):
     def _execute_cmd(self, cmdline):
         try:
             qmpcmd = self.__build_cmd(cmdline)
-        except Exception as e:
-            print('Error while parsing command line: %s' % e)
+        except Exception as err:
+            print('Error while parsing command line: %s' % err)
             print('command format: <command-name> ', end=' ')
             print('[arg-name1=arg1] ... [arg-nameN=argN]')
             return True
@@ -333,8 +333,8 @@ class QMPShell(qmp.QEMUMonitorProtocol):
             print()
             return False
         if cmdline == '':
-            for ev in self.get_events():
-                print(ev)
+            for event in self.get_events():
+                print(event)
             self.clear_events()
             return True
         else:
-- 
2.26.2

[PATCH v2 41/72] scripts/qmp-shell: fix shell history exception handling

[John Snow]

We want to remove exceptions that are too broad here; we only want to
catch IOErrors that get raised as a direct result of the open call.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 2fd677e3dabd..6ad9384c1804 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -67,7 +67,6 @@
 
 import ast
 import atexit
-import errno
 import json
 import os
 import re
@@ -163,19 +162,17 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         readline.set_completer_delims('')
         try:
             readline.read_history_file(self._histfile)
-        except Exception as e:
-            if isinstance(e, IOError) and e.errno == errno.ENOENT:
-                # File not found. No problem.
-                pass
-            else:
-                print("Failed to read history '%s'; %s" % (self._histfile, e))
+        except FileNotFoundError:
+            pass
+        except IOError as err:
+            print(f"Failed to read history '{self._histfile}': {err!s}")
         atexit.register(self.__save_history)
 
     def __save_history(self):
         try:
             readline.write_history_file(self._histfile)
-        except Exception as e:
-            print("Failed to save history file '%s'; %s" % (self._histfile, e))
+        except IOError as err:
+            print(f"Failed to save history file '{self._histfile}': {err!s}")
 
     @classmethod
     def __parse_value(cls, val):
-- 
2.26.2

[PATCH v2 42/72] scripts/qmp-shell: explicitly chain exception context

[John Snow]

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 6ad9384c1804..b3ab735397d4 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -138,8 +138,8 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         if len(addr) == 2:
             try:
                 port = int(addr[1])
-            except ValueError:
-                raise QMPShellBadPort
+            except ValueError as err:
+                raise QMPShellBadPort from err
             return addr[0], port
         # socket path
         return arg
-- 
2.26.2

[PATCH v2 43/72] scripts/qmp-shell: remove if-raise-else patterns

[John Snow]

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index b3ab735397d4..47863a604f51 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -224,8 +224,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
                 if type(parent[optpath[-1]]) is dict:
                     msg = 'Cannot use "{:s}" as both leaf and non-leaf key'
                     raise QMPShellError(msg.format('.'.join(curpath)))
-                else:
-                    raise QMPShellError(f'Cannot set "{key}" multiple times')
+                raise QMPShellError(f'Cannot set "{key}" multiple times')
             parent[optpath[-1]] = value
 
     def __build_cmd(self, cmdline):
@@ -329,13 +328,14 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         except EOFError:
             print()
             return False
+
         if cmdline == '':
             for event in self.get_events():
                 print(event)
             self.clear_events()
             return True
-        else:
-            return self._execute_cmd(cmdline)
+
+        return self._execute_cmd(cmdline)
 
     def set_verbosity(self, verbose):
         self._verbose = verbose
-- 
2.26.2

[PATCH v2 44/72] scripts/qmp-shell: use isinstance() instead of type()

[John Snow]

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 47863a604f51..0cd3ed7d43c7 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -215,13 +215,13 @@ class QMPShell(qmp.QEMUMonitorProtocol):
             for path in optpath[:-1]:
                 curpath.append(path)
                 obj = parent.get(path, {})
-                if type(obj) is not dict:
+                if not isinstance(obj, dict):
                     msg = 'Cannot use "{:s}" as both leaf and non-leaf key'
                     raise QMPShellError(msg.format('.'.join(curpath)))
                 parent[path] = obj
                 parent = obj
             if optpath[-1] in parent:
-                if type(parent[optpath[-1]]) is dict:
+                if isinstance(parent[optpath[-1]], dict):
                     msg = 'Cannot use "{:s}" as both leaf and non-leaf key'
                     raise QMPShellError(msg.format('.'.join(curpath)))
                 raise QMPShellError(f'Cannot set "{key}" multiple times')
-- 
2.26.2

[PATCH v2 45/72] scripts/qmp-shell: use argparse

[John Snow]

Use argparse instead of an open-coded CLI parser, for consistency with
everything else.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 82 +++++++++++++++++--------------------------
 1 file changed, 32 insertions(+), 50 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 0cd3ed7d43c7..d5496aeac0bd 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -64,7 +64,7 @@
 # Use the -v and -p options to activate the verbose and pretty-print options,
 # which will echo back the properly formatted JSON-compliant QMP that is being
 # sent to QEMU, which is useful for debugging and documentation generation.
-
+import argparse
 import ast
 import atexit
 import json
@@ -421,65 +421,47 @@ def die(msg):
     sys.exit(1)
 
 
-def fail_cmdline(option=None):
-    if option:
-        sys.stderr.write('ERROR: bad command-line option \'%s\'\n' % option)
-    sys.stderr.write(
-        'qmp-shell [ -v ] [ -p ] [ -H ] [ -N ] '
-        '< UNIX socket path> | < TCP address:port >\n'
-    )
-    sys.stderr.write('    -v     Verbose (echo command sent and received)\n')
-    sys.stderr.write('    -p     Pretty-print JSON\n')
-    sys.stderr.write('    -H     Use HMP interface\n')
-    sys.stderr.write('    -N     Skip negotiate (for qemu-ga)\n')
-    sys.exit(1)
-
-
 def main():
-    addr = ''
-    qemu = None
-    hmp = False
-    pretty = False
-    verbose = False
-    negotiate = True
+    parser = argparse.ArgumentParser()
+    parser.add_argument('-H', '--hmp', action='store_true',
+                        help='Use HMP interface')
+    parser.add_argument('-N', '--skip-negotiation', action='store_true',
+                        help='Skip negotiate (for qemu-ga)')
+    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')
 
+    default_server = os.environ.get('QMP_SOCKET')
+    parser.add_argument('qmp_server', action='store',
+                        default=default_server,
+                        help='< UNIX socket path | TCP address:port >')
+
+    args = parser.parse_args()
+    if args.qmp_server is None:
+        parser.error("QMP socket or TCP address must be specified")
+
+    qemu: QMPShell
     try:
-        for arg in sys.argv[1:]:
-            if arg == "-H":
-                if qemu is not None:
-                    fail_cmdline(arg)
-                hmp = True
-            elif arg == "-p":
-                pretty = True
-            elif arg == "-N":
-                negotiate = False
-            elif arg == "-v":
-                verbose = True
-            else:
-                if qemu is not None:
-                    fail_cmdline(arg)
-                if hmp:
-                    qemu = HMPShell(arg)
-                else:
-                    qemu = QMPShell(arg, pretty)
-                addr = arg
-
-        if qemu is None:
-            fail_cmdline()
+        if args.hmp:
+            qemu = HMPShell(args.qmp_server)
+        else:
+            qemu = QMPShell(args.qmp_server, args.pretty)
     except QMPShellBadPort:
-        die('bad port number in command-line')
+        parser.error(f"Bad port number: {args.qmp_server}")
+        return  # pycharm doesn't know error() is noreturn
 
     try:
-        qemu.connect(negotiate)
+        qemu.connect(negotiate=not args.skip_negotiation)
     except qmp.QMPConnectError:
-        die('Didn\'t get QMP greeting message')
+        die("Didn't get QMP greeting message")
     except qmp.QMPCapabilitiesError:
-        die('Could not negotiate capabilities')
-    except OSError:
-        die('Could not connect to %s' % addr)
+        die("Couldn't negotiate capabilities")
+    except OSError as err:
+        die(f"Couldn't connect to {args.qmp_server}: {err!s}")
 
     qemu.show_banner()
-    qemu.set_verbosity(verbose)
+    qemu.set_verbosity(args.verbose)
     while qemu.read_exec_command(qemu.get_prompt()):
         pass
     qemu.close()
-- 
2.26.2

[PATCH v2 46/72] python/qmp: Fix type of SocketAddrT

[John Snow]

This is an integer, not a string. We didn't catch this before because we
have not been type checking any of the clients that use this, and it was
self-consistent enough.

Mypy was not introspecting into the socket() interface to realize we
were passing a bad type. Fixed now.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/qemu/qmp/__init__.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python/qemu/qmp/__init__.py b/python/qemu/qmp/__init__.py
index 7355a558796f..dd211e3f6511 100644
--- a/python/qemu/qmp/__init__.py
+++ b/python/qemu/qmp/__init__.py
@@ -49,7 +49,7 @@
 # {} is the QMPReturnValue.
 
 
-InternetAddrT = Tuple[str, str]
+InternetAddrT = Tuple[str, int]
 UnixAddrT = str
 SocketAddrT = Union[InternetAddrT, UnixAddrT]
 
-- 
2.26.2

[PATCH v2 47/72] python/qmp: add parse_address classmethod

[John Snow]

This takes the place of qmp-shell's __get_address function.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/qemu/qmp/__init__.py | 26 ++++++++++++++++++++++++++
 scripts/qmp/qmp-shell       | 27 ++-------------------------
 2 files changed, 28 insertions(+), 25 deletions(-)

diff --git a/python/qemu/qmp/__init__.py b/python/qemu/qmp/__init__.py
index dd211e3f6511..a6e1a7b85775 100644
--- a/python/qemu/qmp/__init__.py
+++ b/python/qemu/qmp/__init__.py
@@ -97,6 +97,12 @@ def __init__(self, reply: QMPMessage):
         self.reply = reply
 
 
+class QMPBadPortError(QMPError):
+    """
+    Unable to parse socket address: Port was non-numerical.
+    """
+
+
 class QEMUMonitorProtocol:
     """
     Provide an API to connect to QEMU via QEMU Monitor Protocol (QMP) and then
@@ -224,6 +230,26 @@ def __exit__(self,
         # Implement context manager exit function.
         self.close()
 
+    @classmethod
+    def parse_address(cls, address: str) -> SocketAddrT:
+        """
+        Parse a string into a QMP address.
+
+        Figure out if the argument is in the port:host form.
+        If it's not, it's probably a file path.
+        """
+        components = address.split(':')
+        if len(components) == 2:
+            try:
+                port = int(components[1])
+            except ValueError:
+                msg = f"Bad port: '{components[1]}' in '{address}'."
+                raise QMPBadPortError(msg) from None
+            return (components[0], port)
+
+        # Treat as filepath.
+        return address
+
     def connect(self, negotiate: bool = True) -> Optional[QMPMessage]:
         """
         Connect to the QMP Monitor and perform capabilities negotiation.
diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index d5496aeac0bd..5a72b9d39502 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -92,10 +92,6 @@ class QMPShellError(Exception):
     pass
 
 
-class QMPShellBadPort(QMPShellError):
-    pass
-
-
 class FuzzyJSON(ast.NodeTransformer):
     """
     This extension of ast.NodeTransformer filters literal "true/false/null"
@@ -118,7 +114,7 @@ class FuzzyJSON(ast.NodeTransformer):
 #       _execute_cmd()). Let's design a better one.
 class QMPShell(qmp.QEMUMonitorProtocol):
     def __init__(self, address, pretty=False):
-        super().__init__(self.__get_address(address))
+        super().__init__(self.parse_address(address))
         self._greeting = None
         self._completer = None
         self._pretty = pretty
@@ -128,22 +124,6 @@ class QMPShell(qmp.QEMUMonitorProtocol):
                                       '.qmp-shell_history')
         self._verbose = False
 
-    @classmethod
-    def __get_address(cls, arg):
-        """
-        Figure out if the argument is in the port:host form, if it's not it's
-        probably a file path.
-        """
-        addr = arg.split(':')
-        if len(addr) == 2:
-            try:
-                port = int(addr[1])
-            except ValueError as err:
-                raise QMPShellBadPort from err
-            return addr[0], port
-        # socket path
-        return arg
-
     def _fill_completion(self):
         cmds = self.cmd('query-commands')
         if 'error' in cmds:
@@ -337,9 +317,6 @@ class QMPShell(qmp.QEMUMonitorProtocol):
 
         return self._execute_cmd(cmdline)
 
-    def set_verbosity(self, verbose):
-        self._verbose = verbose
-
 
 class HMPShell(QMPShell):
     def __init__(self, address):
@@ -447,7 +424,7 @@ def main():
             qemu = HMPShell(args.qmp_server)
         else:
             qemu = QMPShell(args.qmp_server, args.pretty)
-    except QMPShellBadPort:
+    except qmp.QMPBadPortError:
         parser.error(f"Bad port number: {args.qmp_server}")
         return  # pycharm doesn't know error() is noreturn
 
-- 
2.26.2

[PATCH v2 48/72] scripts/qmp-shell: Add pretty attribute to HMP shell

[John Snow]

It's less useful, but it makes the initialization methods LSP
consistent.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 5a72b9d39502..ba27e9801c68 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -319,8 +319,8 @@ class QMPShell(qmp.QEMUMonitorProtocol):
 
 
 class HMPShell(QMPShell):
-    def __init__(self, address):
-        super().__init__(address)
+    def __init__(self, address, pretty=False):
+        super().__init__(address, pretty)
         self.__cpu_index = 0
 
     def __cmd_completion(self):
@@ -418,12 +418,9 @@ def main():
     if args.qmp_server is None:
         parser.error("QMP socket or TCP address must be specified")
 
-    qemu: QMPShell
+    shell_class = HMPShell if args.hmp else QMPShell
     try:
-        if args.hmp:
-            qemu = HMPShell(args.qmp_server)
-        else:
-            qemu = QMPShell(args.qmp_server, args.pretty)
+        qemu = shell_class(args.qmp_server, args.pretty)
     except qmp.QMPBadPortError:
         parser.error(f"Bad port number: {args.qmp_server}")
         return  # pycharm doesn't know error() is noreturn
-- 
2.26.2

[PATCH v2 49/72] scripts/qmp-shell: Make verbose a public attribute

[John Snow]

No real reason to hide this behind an underscore; make it part of the
initializer and make it a regular RW attribute.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index ba27e9801c68..cfcefb95f9da 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -113,7 +113,7 @@ class FuzzyJSON(ast.NodeTransformer):
 # TODO: QMPShell's interface is a bit ugly (eg. _fill_completion() and
 #       _execute_cmd()). Let's design a better one.
 class QMPShell(qmp.QEMUMonitorProtocol):
-    def __init__(self, address, pretty=False):
+    def __init__(self, address, pretty=False, verbose=False):
         super().__init__(self.parse_address(address))
         self._greeting = None
         self._completer = None
@@ -122,7 +122,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         self._actions = list()
         self._histfile = os.path.join(os.path.expanduser('~'),
                                       '.qmp-shell_history')
-        self._verbose = False
+        self.verbose = verbose
 
     def _fill_completion(self):
         cmds = self.cmd('query-commands')
@@ -271,7 +271,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         # For transaction mode, we may have just cached the action:
         if qmpcmd is None:
             return True
-        if self._verbose:
+        if self.verbose:
             self._print(qmpcmd)
         resp = self.cmd_obj(qmpcmd)
         if resp is None:
@@ -319,8 +319,8 @@ class QMPShell(qmp.QEMUMonitorProtocol):
 
 
 class HMPShell(QMPShell):
-    def __init__(self, address, pretty=False):
-        super().__init__(address, pretty)
+    def __init__(self, address, pretty=False, verbose=False):
+        super().__init__(address, pretty, verbose)
         self.__cpu_index = 0
 
     def __cmd_completion(self):
@@ -420,7 +420,7 @@ def main():
 
     shell_class = HMPShell if args.hmp else QMPShell
     try:
-        qemu = shell_class(args.qmp_server, args.pretty)
+        qemu = shell_class(args.qmp_server, args.pretty, args.verbose)
     except qmp.QMPBadPortError:
         parser.error(f"Bad port number: {args.qmp_server}")
         return  # pycharm doesn't know error() is noreturn
@@ -435,7 +435,6 @@ def main():
         die(f"Couldn't connect to {args.qmp_server}: {err!s}")
 
     qemu.show_banner()
-    qemu.set_verbosity(args.verbose)
     while qemu.read_exec_command(qemu.get_prompt()):
         pass
     qemu.close()
-- 
2.26.2

[PATCH v2 50/72] scripts/qmp-shell: move get_prompt() to prompt property

[John Snow]

Small tidying; treat "prompt" like an immutable property instead of
function/method/routine.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index cfcefb95f9da..3b86ef7d8844 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -292,10 +292,11 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         version = self._greeting['QMP']['version']['qemu']
         print("Connected to QEMU {major}.{minor}.{micro}\n".format(**version))
 
-    def get_prompt(self):
+    @property
+    def prompt(self):
         if self._transmode:
-            return "TRANS> "
-        return "(QEMU) "
+            return 'TRANS> '
+        return '(QEMU) '
 
     def read_exec_command(self, prompt):
         """
@@ -435,7 +436,7 @@ def main():
         die(f"Couldn't connect to {args.qmp_server}: {err!s}")
 
     qemu.show_banner()
-    while qemu.read_exec_command(qemu.get_prompt()):
+    while qemu.read_exec_command(qemu.prompt):
         pass
     qemu.close()
 
-- 
2.26.2

[PATCH v2 51/72] scripts/qmp-shell: remove prompt argument from read_exec_command

[John Snow]

It's only ever used by one caller, we can just absorb that logic.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 3b86ef7d8844..31269859c4c4 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -298,14 +298,14 @@ class QMPShell(qmp.QEMUMonitorProtocol):
             return 'TRANS> '
         return '(QEMU) '
 
-    def read_exec_command(self, prompt):
+    def read_exec_command(self):
         """
         Read and execute a command.
 
         @return True if execution was ok, return False if disconnected.
         """
         try:
-            cmdline = input(prompt)
+            cmdline = input(self.prompt)
         except EOFError:
             print()
             return False
@@ -436,7 +436,7 @@ def main():
         die(f"Couldn't connect to {args.qmp_server}: {err!s}")
 
     qemu.show_banner()
-    while qemu.read_exec_command(qemu.prompt):
+    while qemu.read_exec_command():
         pass
     qemu.close()
 
-- 
2.26.2

[PATCH v2 52/72] scripts/qmp-shell: move the REPL functionality into QMPShell

[John Snow]

Instead of doing this in main, move it into the class.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 31269859c4c4..aa148517a864 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -318,6 +318,12 @@ class QMPShell(qmp.QEMUMonitorProtocol):
 
         return self._execute_cmd(cmdline)
 
+    def repl(self):
+        self.show_banner()
+        while self.read_exec_command():
+            yield
+        self.close()
+
 
 class HMPShell(QMPShell):
     def __init__(self, address, pretty=False, verbose=False):
@@ -435,10 +441,8 @@ def main():
     except OSError as err:
         die(f"Couldn't connect to {args.qmp_server}: {err!s}")
 
-    qemu.show_banner()
-    while qemu.read_exec_command():
+    for _ in qemu.repl():
         pass
-    qemu.close()
 
 
 if __name__ == '__main__':
-- 
2.26.2

[PATCH v2 53/72] scripts/qmp-shell: Fix "FuzzyJSON" parser

[John Snow]

I'm not sure when this regressed (Or maybe if it was ever working right
to begin with?), but the Python AST requires you to change "Names" to
"Constants" in order to truly convert `false` to `False`.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index aa148517a864..847d34890f97 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -95,18 +95,19 @@ class QMPShellError(Exception):
 class FuzzyJSON(ast.NodeTransformer):
     """
     This extension of ast.NodeTransformer filters literal "true/false/null"
-    values in an AST and replaces them by proper "True/False/None" values that
-    Python can properly evaluate.
+    values in a Python AST and replaces them by proper "True/False/None" values
+    that Python can properly evaluate.
     """
 
     @classmethod
-    def visit_Name(cls, node):  # pylint: disable=invalid-name
+    def visit_Name(cls,  # pylint: disable=invalid-name
+                   node: ast.Name) -> ast.AST:
         if node.id == 'true':
-            node.id = 'True'
+            return ast.Constant(value=True)
         if node.id == 'false':
-            node.id = 'False'
+            return ast.Constant(value=False)
         if node.id == 'null':
-            node.id = 'None'
+            return ast.Constant(value=None)
         return node
 
 
@@ -174,10 +175,9 @@ class QMPShell(qmp.QEMUMonitorProtocol):
             # Try once again as FuzzyJSON:
             try:
                 tree = ast.parse(val, mode='eval')
-                return ast.literal_eval(FuzzyJSON().visit(tree))
-            except SyntaxError:
-                pass
-            except ValueError:
+                transformed = FuzzyJSON().visit(tree)
+                return ast.literal_eval(transformed)
+            except (SyntaxError, ValueError):
                 pass
         return val
 
-- 
2.26.2

[PATCH v2 54/72] scripts/qmp-shell: refactor QMPCompleter

[John Snow]

list is a generic type, but we expect to use strings directly. We could
subclass list[str], but pylint does not presently understand that
invocation.

Change this class to envelop a list instead of *being* a list, for
simpler typing.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 847d34890f97..73694035b203 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -78,9 +78,17 @@ sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
 from qemu import qmp
 
 
-class QMPCompleter(list):
-    def complete(self, text, state):
-        for cmd in self:
+class QMPCompleter:
+    # NB: Python 3.9+ will probably allow us to subclass list[str] directly,
+    # but pylint as of today does not know that List[str] is simply 'list'.
+    def __init__(self) -> None:
+        self._matches: List[str] = []
+
+    def append(self, value: str) -> None:
+        return self._matches.append(value)
+
+    def complete(self, text: str, state: int) -> Optional[str]:
+        for cmd in self._matches:
             if cmd.startswith(text):
                 if state == 0:
                     return cmd
-- 
2.26.2

[PATCH v2 55/72] scripts/qmp-shell: initialize completer early

[John Snow]

Add an empty completer as a more type-safe placeholder instead of
'None'.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 73694035b203..670361322c51 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -125,7 +125,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
     def __init__(self, address, pretty=False, verbose=False):
         super().__init__(self.parse_address(address))
         self._greeting = None
-        self._completer = None
+        self._completer = QMPCompleter()
         self._pretty = pretty
         self._transmode = False
         self._actions = list()
-- 
2.26.2

[PATCH v2 56/72] python/qmp: add QMPObject type alias

[John Snow]

This is meant to represent any generic object seen in a QMPMessage, not
just the root object itself.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/qemu/qmp/__init__.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/python/qemu/qmp/__init__.py b/python/qemu/qmp/__init__.py
index a6e1a7b85775..ba0d2281d678 100644
--- a/python/qemu/qmp/__init__.py
+++ b/python/qemu/qmp/__init__.py
@@ -41,6 +41,9 @@
 #: 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.
-- 
2.26.2

[PATCH v2 57/72] scripts/qmp-shell: add mypy types

[John Snow]

As per my usual, this patch is annotations only. Any changes with side
effects are done elsewhere.

Note: pylint does not understand the subscripts for Collection in Python 3.6,
so use the stronger Sequence type as a workaround.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 67 ++++++++++++++++++++++++++-----------------
 1 file changed, 41 insertions(+), 26 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 670361322c51..2d0e85b5f723 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -72,10 +72,18 @@ import os
 import re
 import readline
 import sys
+from typing import (
+    Iterator,
+    List,
+    NoReturn,
+    Optional,
+    Sequence,
+)
 
 
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
 from qemu import qmp
+from qemu.qmp import QMPMessage
 
 
 class QMPCompleter:
@@ -122,25 +130,26 @@ class FuzzyJSON(ast.NodeTransformer):
 # TODO: QMPShell's interface is a bit ugly (eg. _fill_completion() and
 #       _execute_cmd()). Let's design a better one.
 class QMPShell(qmp.QEMUMonitorProtocol):
-    def __init__(self, address, pretty=False, verbose=False):
+    def __init__(self, address: str, pretty: bool = False,
+                 verbose: bool = False):
         super().__init__(self.parse_address(address))
-        self._greeting = None
+        self._greeting: Optional[QMPMessage] = None
         self._completer = QMPCompleter()
         self._pretty = pretty
         self._transmode = False
-        self._actions = list()
+        self._actions: List[QMPMessage] = []
         self._histfile = os.path.join(os.path.expanduser('~'),
                                       '.qmp-shell_history')
         self.verbose = verbose
 
-    def _fill_completion(self):
+    def _fill_completion(self) -> None:
         cmds = self.cmd('query-commands')
         if 'error' in cmds:
             return
         for cmd in cmds['return']:
             self._completer.append(cmd['name'])
 
-    def __completer_setup(self):
+    def __completer_setup(self) -> None:
         self._completer = QMPCompleter()
         self._fill_completion()
         readline.set_history_length(1024)
@@ -157,14 +166,14 @@ class QMPShell(qmp.QEMUMonitorProtocol):
             print(f"Failed to read history '{self._histfile}': {err!s}")
         atexit.register(self.__save_history)
 
-    def __save_history(self):
+    def __save_history(self) -> None:
         try:
             readline.write_history_file(self._histfile)
         except IOError as err:
             print(f"Failed to save history file '{self._histfile}': {err!s}")
 
     @classmethod
-    def __parse_value(cls, val):
+    def __parse_value(cls, val: str) -> object:
         try:
             return int(val)
         except ValueError:
@@ -189,7 +198,9 @@ class QMPShell(qmp.QEMUMonitorProtocol):
                 pass
         return val
 
-    def __cli_expr(self, tokens, parent):
+    def __cli_expr(self,
+                   tokens: Sequence[str],
+                   parent: qmp.QMPObject) -> None:
         for arg in tokens:
             (key, sep, val) = arg.partition('=')
             if sep != '=':
@@ -215,7 +226,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
                 raise QMPShellError(f'Cannot set "{key}" multiple times')
             parent[optpath[-1]] = value
 
-    def __build_cmd(self, cmdline):
+    def __build_cmd(self, cmdline: str) -> Optional[QMPMessage]:
         """
         Build a QMP input object from a user provided command-line in the
         following format:
@@ -224,6 +235,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         """
         argument_regex = r'''(?:[^\s"']|"(?:\\.|[^"])*"|'(?:\\.|[^'])*')+'''
         cmdargs = re.findall(argument_regex, cmdline)
+        qmpcmd: QMPMessage
 
         # Transactional CLI entry/exit:
         if cmdargs[0] == 'transaction(':
@@ -261,14 +273,14 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         self.__cli_expr(cmdargs[1:], qmpcmd['arguments'])
         return qmpcmd
 
-    def _print(self, qmp_message):
+    def _print(self, qmp_message: object) -> None:
         indent = None
         if self._pretty:
             indent = 4
         jsobj = json.dumps(qmp_message, indent=indent, sort_keys=self._pretty)
         print(str(jsobj))
 
-    def _execute_cmd(self, cmdline):
+    def _execute_cmd(self, cmdline: str) -> bool:
         try:
             qmpcmd = self.__build_cmd(cmdline)
         except Exception as err:
@@ -288,11 +300,12 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         self._print(resp)
         return True
 
-    def connect(self, negotiate: bool = True):
+    def connect(self, negotiate: bool = True) -> None:
         self._greeting = super().connect(negotiate)
         self.__completer_setup()
 
-    def show_banner(self, msg='Welcome to the QMP low-level shell!'):
+    def show_banner(self,
+                    msg: str = 'Welcome to the QMP low-level shell!') -> None:
         print(msg)
         if not self._greeting:
             print('Connected')
@@ -301,12 +314,12 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         print("Connected to QEMU {major}.{minor}.{micro}\n".format(**version))
 
     @property
-    def prompt(self):
+    def prompt(self) -> str:
         if self._transmode:
             return 'TRANS> '
         return '(QEMU) '
 
-    def read_exec_command(self):
+    def read_exec_command(self) -> bool:
         """
         Read and execute a command.
 
@@ -326,7 +339,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
 
         return self._execute_cmd(cmdline)
 
-    def repl(self):
+    def repl(self) -> Iterator[None]:
         self.show_banner()
         while self.read_exec_command():
             yield
@@ -334,11 +347,12 @@ class QMPShell(qmp.QEMUMonitorProtocol):
 
 
 class HMPShell(QMPShell):
-    def __init__(self, address, pretty=False, verbose=False):
+    def __init__(self, address: str,
+                 pretty: bool = False, verbose: bool = False):
         super().__init__(address, pretty, verbose)
         self.__cpu_index = 0
 
-    def __cmd_completion(self):
+    def __cmd_completion(self) -> None:
         for cmd in self.__cmd_passthrough('help')['return'].split('\r\n'):
             if cmd and cmd[0] != '[' and cmd[0] != '\t':
                 name = cmd.split()[0]  # drop help text
@@ -355,21 +369,22 @@ class HMPShell(QMPShell):
                 self._completer.append(name)
                 self._completer.append('help ' + name)  # help completion
 
-    def __info_completion(self):
+    def __info_completion(self) -> None:
         for cmd in self.__cmd_passthrough('info')['return'].split('\r\n'):
             if cmd:
                 self._completer.append('info ' + cmd.split()[1])
 
-    def __other_completion(self):
+    def __other_completion(self) -> None:
         # special cases
         self._completer.append('help info')
 
-    def _fill_completion(self):
+    def _fill_completion(self) -> None:
         self.__cmd_completion()
         self.__info_completion()
         self.__other_completion()
 
-    def __cmd_passthrough(self, cmdline, cpu_index=0):
+    def __cmd_passthrough(self, cmdline: str,
+                          cpu_index: int = 0) -> QMPMessage:
         return self.cmd_obj({
             'execute': 'human-monitor-command',
             'arguments': {
@@ -378,7 +393,7 @@ class HMPShell(QMPShell):
             }
         })
 
-    def _execute_cmd(self, cmdline):
+    def _execute_cmd(self, cmdline: str) -> bool:
         if cmdline.split()[0] == "cpu":
             # trap the cpu command, it requires special setting
             try:
@@ -404,16 +419,16 @@ class HMPShell(QMPShell):
             print('%s: %s' % (resp['error']['class'], resp['error']['desc']))
         return True
 
-    def show_banner(self, msg='Welcome to the HMP shell!'):
+    def show_banner(self, msg: str = 'Welcome to the HMP shell!') -> None:
         QMPShell.show_banner(self, msg)
 
 
-def die(msg):
+def die(msg: str) -> NoReturn:
     sys.stderr.write('ERROR: %s\n' % msg)
     sys.exit(1)
 
 
-def main():
+def main() -> None:
     parser = argparse.ArgumentParser()
     parser.add_argument('-H', '--hmp', action='store_true',
                         help='Use HMP interface')
-- 
2.26.2

[PATCH v2 58/72] scripts/qmp-shell: Accept SocketAddrT instead of string

[John Snow]

Don't extend QEMUMonitorProtocol but change the argument types; move the
string parsing just outside of the class.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 2d0e85b5f723..b465c7f9e2d6 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -130,9 +130,9 @@ class FuzzyJSON(ast.NodeTransformer):
 # TODO: QMPShell's interface is a bit ugly (eg. _fill_completion() and
 #       _execute_cmd()). Let's design a better one.
 class QMPShell(qmp.QEMUMonitorProtocol):
-    def __init__(self, address: str, pretty: bool = False,
-                 verbose: bool = False):
-        super().__init__(self.parse_address(address))
+    def __init__(self, address: qmp.SocketAddrT,
+                 pretty: bool = False, verbose: bool = False):
+        super().__init__(address)
         self._greeting: Optional[QMPMessage] = None
         self._completer = QMPCompleter()
         self._pretty = pretty
@@ -347,7 +347,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
 
 
 class HMPShell(QMPShell):
-    def __init__(self, address: str,
+    def __init__(self, address: qmp.SocketAddrT,
                  pretty: bool = False, verbose: bool = False):
         super().__init__(address, pretty, verbose)
         self.__cpu_index = 0
@@ -450,11 +450,13 @@ def main() -> None:
 
     shell_class = HMPShell if args.hmp else QMPShell
     try:
-        qemu = shell_class(args.qmp_server, args.pretty, args.verbose)
+        address = shell_class.parse_address(args.qmp_server)
     except qmp.QMPBadPortError:
         parser.error(f"Bad port number: {args.qmp_server}")
         return  # pycharm doesn't know error() is noreturn
 
+    qemu = shell_class(address, args.pretty, args.verbose)
+
     try:
         qemu.connect(negotiate=not args.skip_negotiation)
     except qmp.QMPConnectError:
-- 
2.26.2

[PATCH v2 59/72] scripts/qmp-shell: unprivatize 'pretty' property

[John Snow]

Similar to verbose, there's no reason this needs to be hidden.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index b465c7f9e2d6..f14fe211cca4 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -135,11 +135,11 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         super().__init__(address)
         self._greeting: Optional[QMPMessage] = None
         self._completer = QMPCompleter()
-        self._pretty = pretty
         self._transmode = False
         self._actions: List[QMPMessage] = []
         self._histfile = os.path.join(os.path.expanduser('~'),
                                       '.qmp-shell_history')
+        self.pretty = pretty
         self.verbose = verbose
 
     def _fill_completion(self) -> None:
@@ -274,10 +274,9 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         return qmpcmd
 
     def _print(self, qmp_message: object) -> None:
-        indent = None
-        if self._pretty:
-            indent = 4
-        jsobj = json.dumps(qmp_message, indent=indent, sort_keys=self._pretty)
+        jsobj = json.dumps(qmp_message,
+                           indent=4 if self.pretty else None,
+                           sort_keys=self.pretty)
         print(str(jsobj))
 
     def _execute_cmd(self, cmdline: str) -> bool:
-- 
2.26.2

[PATCH v2 60/72] python/qmp: return generic type from context manager

[John Snow]

__enter__ can be invoked from a subclass, so it needs a more specific
type.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/qemu/qmp/__init__.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/python/qemu/qmp/__init__.py b/python/qemu/qmp/__init__.py
index ba0d2281d678..376954cb6d27 100644
--- a/python/qemu/qmp/__init__.py
+++ b/python/qemu/qmp/__init__.py
@@ -30,6 +30,7 @@
     TextIO,
     Tuple,
     Type,
+    TypeVar,
     Union,
     cast,
 )
@@ -220,7 +221,9 @@ def __get_events(self, wait: Union[bool, float] = False) -> None:
             if ret is None:
                 raise QMPConnectError("Error while reading from socket")
 
-    def __enter__(self) -> 'QEMUMonitorProtocol':
+    T = TypeVar('T')
+
+    def __enter__(self: T) -> T:
         # Implement context manager enter function.
         return self
 
-- 
2.26.2

[PATCH v2 61/72] scripts/qmp-shell: Use context manager instead of atexit

[John Snow]

We can invoke the shell history writing when we leave the QMPShell scope
instead of relying on atexit. Doing so may be preferable to avoid global
state being registered from within a class instead of from the
application logic directly.

Use QMP's context manager to hook this history saving at close time,
which gets invoked when we leave the context block.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index f14fe211cca4..ec028d662e8e 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -66,7 +66,6 @@
 # sent to QEMU, which is useful for debugging and documentation generation.
 import argparse
 import ast
-import atexit
 import json
 import os
 import re
@@ -142,6 +141,11 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         self.pretty = pretty
         self.verbose = verbose
 
+    def close(self) -> None:
+        # Hook into context manager of parent to save shell history.
+        self._save_history()
+        super().close()
+
     def _fill_completion(self) -> None:
         cmds = self.cmd('query-commands')
         if 'error' in cmds:
@@ -164,9 +168,8 @@ class QMPShell(qmp.QEMUMonitorProtocol):
             pass
         except IOError as err:
             print(f"Failed to read history '{self._histfile}': {err!s}")
-        atexit.register(self.__save_history)
 
-    def __save_history(self) -> None:
+    def _save_history(self) -> None:
         try:
             readline.write_history_file(self._histfile)
         except IOError as err:
@@ -448,25 +451,25 @@ def main() -> None:
         parser.error("QMP socket or TCP address must be specified")
 
     shell_class = HMPShell if args.hmp else QMPShell
+
     try:
         address = shell_class.parse_address(args.qmp_server)
     except qmp.QMPBadPortError:
         parser.error(f"Bad port number: {args.qmp_server}")
         return  # pycharm doesn't know error() is noreturn
 
-    qemu = shell_class(address, args.pretty, args.verbose)
+    with shell_class(address, args.pretty, args.verbose) as qemu:
+        try:
+            qemu.connect(negotiate=not args.skip_negotiation)
+        except qmp.QMPConnectError:
+            die("Didn't get QMP greeting message")
+        except qmp.QMPCapabilitiesError:
+            die("Couldn't negotiate capabilities")
+        except OSError as err:
+            die(f"Couldn't connect to {args.qmp_server}: {err!s}")
 
-    try:
-        qemu.connect(negotiate=not args.skip_negotiation)
-    except qmp.QMPConnectError:
-        die("Didn't get QMP greeting message")
-    except qmp.QMPCapabilitiesError:
-        die("Couldn't negotiate capabilities")
-    except OSError as err:
-        die(f"Couldn't connect to {args.qmp_server}: {err!s}")
-
-    for _ in qemu.repl():
-        pass
+        for _ in qemu.repl():
+            pass
 
 
 if __name__ == '__main__':
-- 
2.26.2

[PATCH v2 62/72] scripts/qmp-shell: use logging to show warnings

[John Snow]

Perfect candidate is non-fatal shell history messages.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index ec028d662e8e..0199a13a3428 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -67,6 +67,7 @@
 import argparse
 import ast
 import json
+import logging
 import os
 import re
 import readline
@@ -85,6 +86,9 @@ from qemu import qmp
 from qemu.qmp import QMPMessage
 
 
+LOG = logging.getLogger(__name__)
+
+
 class QMPCompleter:
     # NB: Python 3.9+ will probably allow us to subclass list[str] directly,
     # but pylint as of today does not know that List[str] is simply 'list'.
@@ -167,13 +171,15 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         except FileNotFoundError:
             pass
         except IOError as err:
-            print(f"Failed to read history '{self._histfile}': {err!s}")
+            msg = f"Failed to read history '{self._histfile}': {err!s}"
+            LOG.warning(msg)
 
     def _save_history(self) -> None:
         try:
             readline.write_history_file(self._histfile)
         except IOError as err:
-            print(f"Failed to save history file '{self._histfile}': {err!s}")
+            msg = f"Failed to save history file '{self._histfile}': {err!s}"
+            LOG.warning(msg)
 
     @classmethod
     def __parse_value(cls, val: str) -> object:
-- 
2.26.2

[PATCH v2 63/72] scripts/qmp-shell: remove TODO

[John Snow]

We still want to revamp qmp-shell again, but there's much more to the
idea than the comment now intuits. Remove it.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 2 --
 1 file changed, 2 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 0199a13a3428..3c32b576a37d 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -130,8 +130,6 @@ class FuzzyJSON(ast.NodeTransformer):
         return node
 
 
-# TODO: QMPShell's interface is a bit ugly (eg. _fill_completion() and
-#       _execute_cmd()). Let's design a better one.
 class QMPShell(qmp.QEMUMonitorProtocol):
     def __init__(self, address: qmp.SocketAddrT,
                  pretty: bool = False, verbose: bool = False):
-- 
2.26.2

[PATCH v2 64/72] scripts/qmp-shell: Fix empty-transaction invocation

[John Snow]

calling "transaction( )" is pointless, but valid. Rework the parser to
allow this kind of invocation. This helps clean up exception handling
later by removing accidental breakages of the parser that aren't
explicitly forbidden.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 3c32b576a37d..78e4eae00754 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -244,11 +244,14 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         cmdargs = re.findall(argument_regex, cmdline)
         qmpcmd: QMPMessage
 
-        # Transactional CLI entry/exit:
-        if cmdargs[0] == 'transaction(':
+        # Transactional CLI entry:
+        if cmdargs and cmdargs[0] == 'transaction(':
             self._transmode = True
+            self._actions = []
             cmdargs.pop(0)
-        elif cmdargs[0] == ')' and self._transmode:
+
+        # Transactional CLI exit:
+        if cmdargs and cmdargs[0] == ')' and self._transmode:
             self._transmode = False
             if len(cmdargs) > 1:
                 msg = 'Unexpected input after close of Transaction sub-shell'
@@ -257,15 +260,14 @@ class QMPShell(qmp.QEMUMonitorProtocol):
                 'execute': 'transaction',
                 'arguments': {'actions': self._actions}
             }
-            self._actions = list()
             return qmpcmd
 
-        # Nothing to process?
+        # No args, or no args remaining
         if not cmdargs:
             return None
 
-        # Parse and then cache this Transactional Action
         if self._transmode:
+            # Parse and cache this Transactional Action
             finalize = False
             action = {'type': cmdargs[0], 'data': {}}
             if cmdargs[-1] == ')':
-- 
2.26.2

[PATCH v2 65/72] scripts/qmp-shell: Remove too-broad-exception

[John Snow]

We are only anticipating QMPShellErrors here, for syntax we weren't able
to understand. Other errors, if any, should be allowed to percolate
upwards.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 78e4eae00754..8d5845ab4815 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -291,10 +291,13 @@ class QMPShell(qmp.QEMUMonitorProtocol):
     def _execute_cmd(self, cmdline: str) -> bool:
         try:
             qmpcmd = self.__build_cmd(cmdline)
-        except Exception as err:
-            print('Error while parsing command line: %s' % err)
-            print('command format: <command-name> ', end=' ')
-            print('[arg-name1=arg1] ... [arg-nameN=argN]')
+        except QMPShellError as err:
+            print(
+                f"Error while parsing command line: {err!s}\n"
+                "command format: <command-name> "
+                "[arg-name1=arg1] ... [arg-nameN=argN",
+                file=sys.stderr
+            )
             return True
         # For transaction mode, we may have just cached the action:
         if qmpcmd is None:
-- 
2.26.2

[PATCH v2 66/72] scripts/qmp-shell: convert usage comment to docstring

[John Snow]

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 128 ++++++++++++++++++++++++------------------
 1 file changed, 72 insertions(+), 56 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 8d5845ab4815..82fe16cff820 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -1,7 +1,5 @@
 #!/usr/bin/env python3
 #
-# Low-level QEMU shell on top of QMP.
-#
 # Copyright (C) 2009, 2010 Red Hat Inc.
 #
 # Authors:
@@ -10,60 +8,78 @@
 # This work is licensed under the terms of the GNU GPL, version 2.  See
 # the COPYING file in the top-level directory.
 #
-# Usage:
-#
-# Start QEMU with:
-#
-# # qemu [...] -qmp unix:./qmp-sock,server
-#
-# Run the shell:
-#
-# $ qmp-shell ./qmp-sock
-#
-# Commands have the following format:
-#
-#    < command-name > [ arg-name1=arg1 ] ... [ arg-nameN=argN ]
-#
-# For example:
-#
-# (QEMU) device_add driver=e1000 id=net1
-# {u'return': {}}
-# (QEMU)
-#
-# key=value pairs also support Python or JSON object literal subset notations,
-# without spaces. Dictionaries/objects {} are supported as are arrays [].
-#
-#    example-command arg-name1={'key':'value','obj'={'prop':"value"}}
-#
-# Both JSON and Python formatting should work, including both styles of
-# string literal quotes. Both paradigms of literal values should work,
-# including null/true/false for JSON and None/True/False for Python.
-#
-#
-# Transactions have the following multi-line format:
-#
-#    transaction(
-#    action-name1 [ arg-name1=arg1 ] ... [arg-nameN=argN ]
-#    ...
-#    action-nameN [ arg-name1=arg1 ] ... [arg-nameN=argN ]
-#    )
-#
-# One line transactions are also supported:
-#
-#    transaction( action-name1 ... )
-#
-# For example:
-#
-#     (QEMU) transaction(
-#     TRANS> block-dirty-bitmap-add node=drive0 name=bitmap1
-#     TRANS> block-dirty-bitmap-clear node=drive0 name=bitmap0
-#     TRANS> )
-#     {"return": {}}
-#     (QEMU)
-#
-# Use the -v and -p options to activate the verbose and pretty-print options,
-# which will echo back the properly formatted JSON-compliant QMP that is being
-# sent to QEMU, which is useful for debugging and documentation generation.
+
+"""
+Low-level QEMU shell on top of QMP.
+
+usage: qmp-shell [-h] [-H] [-N] [-v] [-p] qmp_server
+
+positional arguments:
+  qmp_server            < UNIX socket path | TCP address:port >
+
+optional arguments:
+  -h, --help            show this help message and exit
+  -H, --hmp             Use HMP interface
+  -N, --skip-negotiation
+                        Skip negotiate (for qemu-ga)
+  -v, --verbose         Verbose (echo commands sent and received)
+  -p, --pretty          Pretty-print JSON
+
+
+Start QEMU with:
+
+# qemu [...] -qmp unix:./qmp-sock,server
+
+Run the shell:
+
+$ qmp-shell ./qmp-sock
+
+Commands have the following format:
+
+   < command-name > [ arg-name1=arg1 ] ... [ arg-nameN=argN ]
+
+For example:
+
+(QEMU) device_add driver=e1000 id=net1
+{'return': {}}
+(QEMU)
+
+key=value pairs also support Python or JSON object literal subset notations,
+without spaces. Dictionaries/objects {} are supported as are arrays [].
+
+   example-command arg-name1={'key':'value','obj'={'prop':"value"}}
+
+Both JSON and Python formatting should work, including both styles of
+string literal quotes. Both paradigms of literal values should work,
+including null/true/false for JSON and None/True/False for Python.
+
+
+Transactions have the following multi-line format:
+
+   transaction(
+   action-name1 [ arg-name1=arg1 ] ... [arg-nameN=argN ]
+   ...
+   action-nameN [ arg-name1=arg1 ] ... [arg-nameN=argN ]
+   )
+
+One line transactions are also supported:
+
+   transaction( action-name1 ... )
+
+For example:
+
+    (QEMU) transaction(
+    TRANS> block-dirty-bitmap-add node=drive0 name=bitmap1
+    TRANS> block-dirty-bitmap-clear node=drive0 name=bitmap0
+    TRANS> )
+    {"return": {}}
+    (QEMU)
+
+Use the -v and -p options to activate the verbose and pretty-print options,
+which will echo back the properly formatted JSON-compliant QMP that is being
+sent to QEMU, which is useful for debugging and documentation generation.
+"""
+
 import argparse
 import ast
 import json
-- 
2.26.2

[PATCH v2 67/72] scripts/qmp-shell: remove double-underscores

[John Snow]

They're not needed; single underscore is enough to express intent that
these methods are "internal". double underscore is used as a weak name
mangling, but that isn't beneficial for us here.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 52 +++++++++++++++++++++----------------------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 82fe16cff820..40ff9e0a82bd 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -171,7 +171,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         for cmd in cmds['return']:
             self._completer.append(cmd['name'])
 
-    def __completer_setup(self) -> None:
+    def _completer_setup(self) -> None:
         self._completer = QMPCompleter()
         self._fill_completion()
         readline.set_history_length(1024)
@@ -196,7 +196,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
             LOG.warning(msg)
 
     @classmethod
-    def __parse_value(cls, val: str) -> object:
+    def _parse_value(cls, val: str) -> object:
         try:
             return int(val)
         except ValueError:
@@ -221,9 +221,9 @@ class QMPShell(qmp.QEMUMonitorProtocol):
                 pass
         return val
 
-    def __cli_expr(self,
-                   tokens: Sequence[str],
-                   parent: qmp.QMPObject) -> None:
+    def _cli_expr(self,
+                  tokens: Sequence[str],
+                  parent: qmp.QMPObject) -> None:
         for arg in tokens:
             (key, sep, val) = arg.partition('=')
             if sep != '=':
@@ -231,7 +231,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
                     f"Expected a key=value pair, got '{arg!s}'"
                 )
 
-            value = self.__parse_value(val)
+            value = self._parse_value(val)
             optpath = key.split('.')
             curpath = []
             for path in optpath[:-1]:
@@ -249,7 +249,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
                 raise QMPShellError(f'Cannot set "{key}" multiple times')
             parent[optpath[-1]] = value
 
-    def __build_cmd(self, cmdline: str) -> Optional[QMPMessage]:
+    def _build_cmd(self, cmdline: str) -> Optional[QMPMessage]:
         """
         Build a QMP input object from a user provided command-line in the
         following format:
@@ -289,13 +289,13 @@ class QMPShell(qmp.QEMUMonitorProtocol):
             if cmdargs[-1] == ')':
                 cmdargs.pop(-1)
                 finalize = True
-            self.__cli_expr(cmdargs[1:], action['data'])
+            self._cli_expr(cmdargs[1:], action['data'])
             self._actions.append(action)
-            return self.__build_cmd(')') if finalize else None
+            return self._build_cmd(')') if finalize else None
 
         # Standard command: parse and return it to be executed.
         qmpcmd = {'execute': cmdargs[0], 'arguments': {}}
-        self.__cli_expr(cmdargs[1:], qmpcmd['arguments'])
+        self._cli_expr(cmdargs[1:], qmpcmd['arguments'])
         return qmpcmd
 
     def _print(self, qmp_message: object) -> None:
@@ -306,7 +306,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
 
     def _execute_cmd(self, cmdline: str) -> bool:
         try:
-            qmpcmd = self.__build_cmd(cmdline)
+            qmpcmd = self._build_cmd(cmdline)
         except QMPShellError as err:
             print(
                 f"Error while parsing command line: {err!s}\n"
@@ -329,7 +329,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
 
     def connect(self, negotiate: bool = True) -> None:
         self._greeting = super().connect(negotiate)
-        self.__completer_setup()
+        self._completer_setup()
 
     def show_banner(self,
                     msg: str = 'Welcome to the QMP low-level shell!') -> None:
@@ -377,10 +377,10 @@ class HMPShell(QMPShell):
     def __init__(self, address: qmp.SocketAddrT,
                  pretty: bool = False, verbose: bool = False):
         super().__init__(address, pretty, verbose)
-        self.__cpu_index = 0
+        self._cpu_index = 0
 
-    def __cmd_completion(self) -> None:
-        for cmd in self.__cmd_passthrough('help')['return'].split('\r\n'):
+    def _cmd_completion(self) -> None:
+        for cmd in self._cmd_passthrough('help')['return'].split('\r\n'):
             if cmd and cmd[0] != '[' and cmd[0] != '\t':
                 name = cmd.split()[0]  # drop help text
                 if name == 'info':
@@ -396,22 +396,22 @@ class HMPShell(QMPShell):
                 self._completer.append(name)
                 self._completer.append('help ' + name)  # help completion
 
-    def __info_completion(self) -> None:
-        for cmd in self.__cmd_passthrough('info')['return'].split('\r\n'):
+    def _info_completion(self) -> None:
+        for cmd in self._cmd_passthrough('info')['return'].split('\r\n'):
             if cmd:
                 self._completer.append('info ' + cmd.split()[1])
 
-    def __other_completion(self) -> None:
+    def _other_completion(self) -> None:
         # special cases
         self._completer.append('help info')
 
     def _fill_completion(self) -> None:
-        self.__cmd_completion()
-        self.__info_completion()
-        self.__other_completion()
+        self._cmd_completion()
+        self._info_completion()
+        self._other_completion()
 
-    def __cmd_passthrough(self, cmdline: str,
-                          cpu_index: int = 0) -> QMPMessage:
+    def _cmd_passthrough(self, cmdline: str,
+                         cpu_index: int = 0) -> QMPMessage:
         return self.cmd_obj({
             'execute': 'human-monitor-command',
             'arguments': {
@@ -425,14 +425,14 @@ class HMPShell(QMPShell):
             # trap the cpu command, it requires special setting
             try:
                 idx = int(cmdline.split()[1])
-                if 'return' not in self.__cmd_passthrough('info version', idx):
+                if 'return' not in self._cmd_passthrough('info version', idx):
                     print('bad CPU index')
                     return True
-                self.__cpu_index = idx
+                self._cpu_index = idx
             except ValueError:
                 print('cpu command takes an integer argument')
                 return True
-        resp = self.__cmd_passthrough(cmdline, self.__cpu_index)
+        resp = self._cmd_passthrough(cmdline, self._cpu_index)
         if resp is None:
             print('Disconnected')
             return False
-- 
2.26.2

[PATCH v2 68/72] scripts/qmp-shell: make QMPShellError inherit QMPError

[John Snow]

This way, all exceptions from the qemu.qmp namespace all derive from
QMPError.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 40ff9e0a82bd..1a8a4ba18ab4 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -123,7 +123,7 @@ class QMPCompleter:
         return None
 
 
-class QMPShellError(Exception):
+class QMPShellError(qmp.QMPError):
     pass
 
 
-- 
2.26.2

[PATCH v2 69/72] scripts/qmp-shell: add docstrings

[John Snow]

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 39 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 1a8a4ba18ab4..15aedb80c2af 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -106,15 +106,20 @@ LOG = logging.getLogger(__name__)
 
 
 class QMPCompleter:
+    """
+    QMPCompleter provides a readline library tab-complete behavior.
+    """
     # NB: Python 3.9+ will probably allow us to subclass list[str] directly,
     # but pylint as of today does not know that List[str] is simply 'list'.
     def __init__(self) -> None:
         self._matches: List[str] = []
 
     def append(self, value: str) -> None:
+        """Append a new valid completion to the list of possibilities."""
         return self._matches.append(value)
 
     def complete(self, text: str, state: int) -> Optional[str]:
+        """readline.set_completer() callback implementation."""
         for cmd in self._matches:
             if cmd.startswith(text):
                 if state == 0:
@@ -124,7 +129,9 @@ class QMPCompleter:
 
 
 class QMPShellError(qmp.QMPError):
-    pass
+    """
+    QMP Shell Base error class.
+    """
 
 
 class FuzzyJSON(ast.NodeTransformer):
@@ -137,6 +144,9 @@ class FuzzyJSON(ast.NodeTransformer):
     @classmethod
     def visit_Name(cls,  # pylint: disable=invalid-name
                    node: ast.Name) -> ast.AST:
+        """
+        Transform Name nodes with certain values into Constant (keyword) nodes.
+        """
         if node.id == 'true':
             return ast.Constant(value=True)
         if node.id == 'false':
@@ -147,6 +157,13 @@ class FuzzyJSON(ast.NodeTransformer):
 
 
 class QMPShell(qmp.QEMUMonitorProtocol):
+    """
+    QMPShell provides a basic readline-based QMP shell.
+
+    :param address: Address of the QMP server.
+    :param pretty: Pretty-print QMP messages.
+    :param verbose: Echo outgoing QMP messages to console.
+    """
     def __init__(self, address: qmp.SocketAddrT,
                  pretty: bool = False, verbose: bool = False):
         super().__init__(address)
@@ -333,6 +350,9 @@ class QMPShell(qmp.QEMUMonitorProtocol):
 
     def show_banner(self,
                     msg: str = 'Welcome to the QMP low-level shell!') -> None:
+        """
+        Print to stdio a greeting, and the QEMU version if available.
+        """
         print(msg)
         if not self._greeting:
             print('Connected')
@@ -342,6 +362,9 @@ class QMPShell(qmp.QEMUMonitorProtocol):
 
     @property
     def prompt(self) -> str:
+        """
+        Return the current shell prompt, including a trailing space.
+        """
         if self._transmode:
             return 'TRANS> '
         return '(QEMU) '
@@ -367,6 +390,9 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         return self._execute_cmd(cmdline)
 
     def repl(self) -> Iterator[None]:
+        """
+        Return an iterator that implements the REPL.
+        """
         self.show_banner()
         while self.read_exec_command():
             yield
@@ -374,6 +400,13 @@ class QMPShell(qmp.QEMUMonitorProtocol):
 
 
 class HMPShell(QMPShell):
+    """
+    HMPShell provides a basic readline-based HMP shell, tunnelled via QMP.
+
+    :param address: Address of the QMP server.
+    :param pretty: Pretty-print QMP messages.
+    :param verbose: Echo outgoing QMP messages to console.
+    """
     def __init__(self, address: qmp.SocketAddrT,
                  pretty: bool = False, verbose: bool = False):
         super().__init__(address, pretty, verbose)
@@ -451,11 +484,15 @@ class HMPShell(QMPShell):
 
 
 def die(msg: str) -> NoReturn:
+    """Write an error to stderr, then exit with a return code of 1."""
     sys.stderr.write('ERROR: %s\n' % msg)
     sys.exit(1)
 
 
 def main() -> None:
+    """
+    qmp-shell 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')
-- 
2.26.2

[PATCH v2 70/72] scripts/qmp-shell: move to python/qemu/qmp/qmp_shell.py

[John Snow]

The script will be unavailable for a commit or two, which will help
preserve development history attached to the new file. A forwarder will
be added shortly afterwards.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell => python/qemu/qmp/qmp_shell.py | 3 ---
 1 file changed, 3 deletions(-)
 rename scripts/qmp/qmp-shell => python/qemu/qmp/qmp_shell.py (99%)
 mode change 100755 => 100644

diff --git a/scripts/qmp/qmp-shell b/python/qemu/qmp/qmp_shell.py
old mode 100755
new mode 100644
similarity index 99%
rename from scripts/qmp/qmp-shell
rename to python/qemu/qmp/qmp_shell.py
index 15aedb80c2af..337acfce2d26
--- a/scripts/qmp/qmp-shell
+++ b/python/qemu/qmp/qmp_shell.py
@@ -1,4 +1,3 @@
-#!/usr/bin/env python3
 #
 # Copyright (C) 2009, 2010 Red Hat Inc.
 #
@@ -96,8 +95,6 @@
     Sequence,
 )
 
-
-sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
 from qemu import qmp
 from qemu.qmp import QMPMessage
 
-- 
2.26.2

[PATCH v2 71/72] python: add qmp-shell entry point

[John Snow]

now 'qmp-shell' should be available from the command line when
installing the python package.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/setup.cfg | 1 +
 1 file changed, 1 insertion(+)

diff --git a/python/setup.cfg b/python/setup.cfg
index f2f54bcaefe8..230fd870ea91 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -42,6 +42,7 @@ console_scripts =
     qom-tree = qemu.qmp.qom:QOMTree.entry_point
     qom-fuse = qemu.qmp.qom_fuse:QOMFuse.entry_point
     qemu-ga-client = qemu.qmp.qemu_ga_client:main
+    qmp-shell = qemu.qmp.qmp_shell:main
 
 [flake8]
 extend-ignore = E722  # Prefer pylint's bare-except checks to flake8's
-- 
2.26.2

[PATCH v2 72/72] scripts/qmp-shell: add redirection shim

[John Snow]

qmp-shell has a new home, add a redirect for a little while as the dust
settles.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qmp/qmp-shell | 11 +++++++++++
 1 file changed, 11 insertions(+)
 create mode 100755 scripts/qmp/qmp-shell

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
new file mode 100755
index 000000000000..4a20f97db708
--- /dev/null
+++ b/scripts/qmp/qmp-shell
@@ -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()
-- 
2.26.2