[PATCH 00/11] python: move /scripts/qmp/gemu-ga-client.py to qemu.qmp package

[PATCH 00/11] python: move /scripts/qmp/gemu-ga-client.py to qemu.qmp package

[John Snow]

Delint and type the qemu-ga-client and move it over into
/python/qemu/qmp/qemu_ga_client.py.

Based-on: <20210603003719.1321369-1-jsnow@redhat.com>
GitLab: https://gitlab.com/jsnow/qemu/-/commits/python-package-qga
CI: https://gitlab.com/jsnow/qemu/-/pipelines/315122004

(Weakly based on "[PATCH v3 00/19] Python: move /scripts/qmp/qom* to
/python/qemu/qmp/qom*", for the purposes of avoiding context conflicts
in /python/setup.cfg, but is trivially rebased without it.)

Add a new console entrypoint to the package under "qemu-ga-client",
keeping the old name. (This makes a script named "qemu-ga-client"
available in your $PATH when you use pip to install the qemu.qmp
package.)

Add a forwarder shim back to /scripts/qmp/qemu-ga-client.py that
forwards to the new script, keeping functionality as it was in the old
location, at least for a little while. I intend to eventually deprecate
these forwarders, but not yet. (This allows you to use "qemu-ga-client"
from the scripts directory without needing to install the qemu.qmp
package.)

Now this script is protected against regressions against the qemu.qmp
package because it's part of it, and validated regularly by GitLab CI.

John Snow (11):
  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/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 shim

 python/qemu/qmp/__init__.py       |  25 ++-
 python/qemu/qmp/qemu_ga_client.py | 323 ++++++++++++++++++++++++++++++
 python/setup.cfg                  |   1 +
 scripts/qmp/qemu-ga-client        | 297 +--------------------------
 4 files changed, 341 insertions(+), 305 deletions(-)
 create mode 100644 python/qemu/qmp/qemu_ga_client.py

-- 
2.31.1

[PATCH 01/11] 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 348d85864c..97f4047a62 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.31.1

[PATCH 02/11] 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 97f4047a62..566bddc89d 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=on,wait=off,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.31.1

[PATCH 03/11] scripts/qemu-ga-client: Fix exception handling

[John Snow]

Fixes: 50d189c

These error classes aren't available anymore. Fix the bitrot.

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 566bddc89d..7aba09f0fe 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.31.1

[PATCH 04/11] scripts/qemu-ga-client: replace deprecated optparse with argparse

[John Snow]

optparse isn't supported anymore, it's from the python2 days. Replace it
with the mostly similar argparse.

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 7aba09f0fe..8eb4015e61 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.31.1

[PATCH 05/11] 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 8eb4015e61..e81937e0ea 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
-#
+"""
+QEMU Guest Agent Client
+
+Usage:
+
+Start QEMU with:
+
+# qemu [...] -chardev socket,path=/tmp/qga.sock,server,wait=off,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
+"""
+
 # 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=on,wait=off,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.31.1

[PATCH 06/11] 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 a
dynamic RPC mechanism.

I believe it's worth preserving the effort and care that went into
making this script by updating it to work with our current
infrastructure. However, I am disabling the requirement for docstrings
in this file.

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 e81937e0ea..ece9f74fa8 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.31.1

[PATCH 07/11] 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>

---

A note for Vladimir: I'm not using 'object' here yet because it causes a
few regressions in iotests.py type checking that I'm not ready to fix
just yet, but it will eventually happen.

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 822c793c32..a6e1a7b857 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, int]
 UnixAddrT = str
@@ -297,8 +302,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.
 
@@ -313,7 +318,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.31.1

[PATCH 08/11] scripts/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 ece9f74fa8..a7d0ef8347 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.31.1

[PATCH 09/11] scripts/qemu-ga-client: move to python/qemu/qmp/qemu_ga_client.py

[John Snow]

The script itself will be unavailable for a few commits before being
restored, with no way to run it right after this commit. This helps move
git history into the new file. To prevent linter regressions, though, we
do need to immediately touch up the filename to remove dashes (to make
the module importable), and remove the executable bit.

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 a7d0ef8347..d2938ad47c 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.31.1

[PATCH 10/11] python/qemu-ga-client: add entry point

[John Snow]

Remove the shebang, and add a package-defined entry point instead. Now,
it can be accessed using 'qemu-ga-client' from the command line after
installing the package.

The next commit adds a forwarder shim that allows the running of this
script without needing to install the package again.

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 d2938ad47c..67ac0b4211
--- a/python/qemu/qmp/qemu_ga_client.py
+++ b/python/qemu/qmp/qemu_ga_client.py
@@ -1,5 +1,3 @@
-#!/usr/bin/env python3
-
 """
 QEMU Guest Agent Client
 
diff --git a/python/setup.cfg b/python/setup.cfg
index 6b6be8b03c..7f3c59d74e 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -50,6 +50,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 [fuse]
+    qemu-ga-client = qemu.qmp.qemu_ga_client:main
 
 [flake8]
 extend-ignore = E722  # Prefer pylint's bare-except checks to flake8's
-- 
2.31.1

[PATCH 11/11] scripts/qemu-ga-client: Add forwarder shim

[John Snow]

Add a little forwarder shim until we are sure that everyone is
comfortable with how to use the tools in their new packaged 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 0000000000..102fd2cad9
--- /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.31.1

Re: [PATCH 00/11] python: move /scripts/qmp/gemu-ga-client.py to qemu.qmp package

[John Snow]

On 6/4/21 11:55 AM, John Snow wrote:
> Delint and type the qemu-ga-client and move it over into
> /python/qemu/qmp/qemu_ga_client.py.
> 
> Based-on: <20210603003719.1321369-1-jsnow@redhat.com>
> GitLab: https://gitlab.com/jsnow/qemu/-/commits/python-package-qga
> CI: https://gitlab.com/jsnow/qemu/-/pipelines/315122004
> 
> (Weakly based on "[PATCH v3 00/19] Python: move /scripts/qmp/qom* to
> /python/qemu/qmp/qom*", for the purposes of avoiding context conflicts
> in /python/setup.cfg, but is trivially rebased without it.)
> 
> Add a new console entrypoint to the package under "qemu-ga-client",
> keeping the old name. (This makes a script named "qemu-ga-client"
> available in your $PATH when you use pip to install the qemu.qmp
> package.)
> 
> Add a forwarder shim back to /scripts/qmp/qemu-ga-client.py that
> forwards to the new script, keeping functionality as it was in the old
> location, at least for a little while. I intend to eventually deprecate
> these forwarders, but not yet. (This allows you to use "qemu-ga-client"
> from the scripts directory without needing to install the qemu.qmp
> package.)
> 
> Now this script is protected against regressions against the qemu.qmp
> package because it's part of it, and validated regularly by GitLab CI.
> 
> John Snow (11):
>    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/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 shim
> 
>   python/qemu/qmp/__init__.py       |  25 ++-
>   python/qemu/qmp/qemu_ga_client.py | 323 ++++++++++++++++++++++++++++++
>   python/setup.cfg                  |   1 +
>   scripts/qmp/qemu-ga-client        | 297 +--------------------------
>   4 files changed, 341 insertions(+), 305 deletions(-)
>   create mode 100644 python/qemu/qmp/qemu_ga_client.py
> 

Thanks, preliminarily staged on my python branch:

https://gitlab.com/jsnow/qemu/-/commits/python

CI (covers this series and the scripts/qmp/qom* series):
https://gitlab.com/jsnow/qemu/-/pipelines/319584565

I intend to send a PR this coming Friday after staging the qmp-shell 
cleanup series.

--js