qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] python: a few improvements to qmp-shell
@ 2022-01-17 14:11 Daniel P. Berrangé
  2022-01-17 14:11 ` [PATCH 1/2] python: introduce qmp-shell-wrap convenience tool Daniel P. Berrangé
  2022-01-17 14:11 ` [PATCH 2/2] python: support recording QMP session to a file Daniel P. Berrangé
  0 siblings, 2 replies; 12+ messages in thread
From: Daniel P. Berrangé @ 2022-01-17 14:11 UTC (permalink / raw)
  To: qemu-devel
  Cc: Eduardo Habkost, Daniel P. Berrangé,
	John Snow, Markus Armbruster, Cleber Rosa

This makes the qmp-shell program a little more pleasant to use when you
are just trying to spawn a throw-away QEMU process to query some info
from.

First it introduces a 'qmp-shell-wrap' command that takes a QEMU command
line instead of QMP socket, and spawns QEMU automatically, so its life
is tied to that of the shell.

Second it adds ability to log QMP commands/responses to a file that can
be queried with 'jq' to extract information. This is good for commands
which return huge JSON docs.

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

 python/qemu/qmp/qmp_shell.py | 80 ++++++++++++++++++++++++++++++++----
 scripts/qmp/qmp-shell-wrap   | 11 +++++
 2 files changed, 84 insertions(+), 7 deletions(-)
 create mode 100755 scripts/qmp/qmp-shell-wrap

-- 
2.33.1




^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] python: introduce qmp-shell-wrap convenience tool
  2022-01-17 14:11 [PATCH 0/2] python: a few improvements to qmp-shell Daniel P. Berrangé
@ 2022-01-17 14:11 ` Daniel P. Berrangé
  2022-01-17 23:27   ` John Snow
  2022-01-17 14:11 ` [PATCH 2/2] python: support recording QMP session to a file Daniel P. Berrangé
  1 sibling, 1 reply; 12+ messages in thread
From: Daniel P. Berrangé @ 2022-01-17 14:11 UTC (permalink / raw)
  To: qemu-devel
  Cc: Eduardo Habkost, Daniel P. Berrangé,
	John Snow, Markus Armbruster, Cleber Rosa

With the current 'qmp-shell' tool developers must first spawn QEMU with
a suitable -qmp arg and then spawn qmp-shell in a separate terminal
pointing to the right socket.

With 'qmp-shell-wrap' developers can ignore QMP sockets entirely and
just pass the QEMU command and arguments they want. The program will
listen on a UNIX socket and tell QEMU to connect QMP to that.

For example, this:

 # qmp-shell-wrap -- qemu-system-x86_64 -display none

Is roughly equivalent of running:

 # qemu-system-x86_64 -display none -qmp qmp-shell-1234 &
 # qmp-shell qmp-shell-1234

Except that 'qmp-shell-wrap' switches the socket peers around so that
it is the UNIX socket server and QEMU is the socket client. This makes
QEMU reliably go away when qmp-shell-wrap exits, closing the server
socket.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 python/qemu/qmp/qmp_shell.py | 61 +++++++++++++++++++++++++++++++++---
 scripts/qmp/qmp-shell-wrap   | 11 +++++++
 2 files changed, 68 insertions(+), 4 deletions(-)
 create mode 100755 scripts/qmp/qmp-shell-wrap

diff --git a/python/qemu/qmp/qmp_shell.py b/python/qemu/qmp/qmp_shell.py
index e7d7eb18f1..12f7d28afc 100644
--- a/python/qemu/qmp/qmp_shell.py
+++ b/python/qemu/qmp/qmp_shell.py
@@ -86,6 +86,7 @@
 import os
 import re
 import readline
+from subprocess import Popen
 import sys
 from typing import (
     Iterator,
@@ -162,8 +163,10 @@ class QMPShell(qmp.QEMUMonitorProtocol):
     :param verbose: Echo outgoing QMP messages to console.
     """
     def __init__(self, address: qmp.SocketAddrT,
-                 pretty: bool = False, verbose: bool = False):
-        super().__init__(address)
+                 pretty: bool = False,
+                 verbose: bool = False,
+                 server: bool = False):
+        super().__init__(address, server=server)
         self._greeting: Optional[QMPMessage] = None
         self._completer = QMPCompleter()
         self._transmode = False
@@ -404,8 +407,10 @@ class HMPShell(QMPShell):
     :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)
+                 pretty: bool = False,
+                 verbose: bool = False,
+                 server: bool = False):
+        super().__init__(address, pretty, verbose, server)
         self._cpu_index = 0
 
     def _cmd_completion(self) -> None:
@@ -529,6 +534,54 @@ def main() -> None:
         for _ in qemu.repl():
             pass
 
+def main_wrap() -> None:
+    """
+    qmp-shell-wrap entry point: parse command line arguments and start the REPL.
+    """
+    parser = argparse.ArgumentParser()
+    parser.add_argument('-H', '--hmp', action='store_true',
+                        help='Use HMP interface')
+    parser.add_argument('-v', '--verbose', action='store_true',
+                        help='Verbose (echo commands sent and received)')
+    parser.add_argument('-p', '--pretty', action='store_true',
+                        help='Pretty-print JSON')
+
+    parser.add_argument('command', nargs=argparse.REMAINDER,
+                        help='QEMU command line to invoke')
+
+    args = parser.parse_args()
+
+    cmd = args.command
+    if len(cmd) != 0 and cmd[0] == '--':
+        cmd = cmd[1:]
+    if len(cmd) == 0:
+        cmd = "qemu-system-x86_64"
+
+    sockpath = "qmp-shell-wrap-%d" % os.getpid()
+    cmd += ["-qmp", "unix:%s" % sockpath]
+
+    shell_class = HMPShell if args.hmp else QMPShell
+
+    try:
+        address = shell_class.parse_address(sockpath)
+    except qmp.QMPBadPortError:
+        parser.error(f"Bad port number: {socketpath}")
+        return  # pycharm doesn't know error() is noreturn
+
+    with shell_class(address, args.pretty, args.verbose, True) as qemu:
+        qemuproc = Popen(cmd)
+
+        try:
+            qemu.accept()
+        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 {sockpath}: {err!s}")
+
+        for _ in qemu.repl():
+            pass
 
 if __name__ == '__main__':
     main()
diff --git a/scripts/qmp/qmp-shell-wrap b/scripts/qmp/qmp-shell-wrap
new file mode 100755
index 0000000000..9e94da114f
--- /dev/null
+++ b/scripts/qmp/qmp-shell-wrap
@@ -0,0 +1,11 @@
+#!/usr/bin/env python3
+
+import os
+import sys
+
+sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
+from qemu.qmp import qmp_shell
+
+
+if __name__ == '__main__':
+    qmp_shell.main_wrap()
-- 
2.33.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] python: support recording QMP session to a file
  2022-01-17 14:11 [PATCH 0/2] python: a few improvements to qmp-shell Daniel P. Berrangé
  2022-01-17 14:11 ` [PATCH 1/2] python: introduce qmp-shell-wrap convenience tool Daniel P. Berrangé
@ 2022-01-17 14:11 ` Daniel P. Berrangé
  2022-01-17 15:04   ` Philippe Mathieu-Daudé via
  1 sibling, 1 reply; 12+ messages in thread
From: Daniel P. Berrangé @ 2022-01-17 14:11 UTC (permalink / raw)
  To: qemu-devel
  Cc: Eduardo Habkost, Daniel P. Berrangé,
	John Snow, Markus Armbruster, Cleber Rosa

When running QMP commands with very large response payloads, it is often
not easy to spot the info you want. If we can save the response to a
file then tools like 'grep' or 'jq' can be used to extract information.

For convenience of processing, we merge the QMP command and response
dictionaries together:

  {
      "arguments": {},
      "execute": "query-kvm",
      "return": {
          "enabled": false,
          "present": true
      }
  }

Example usage

  $ ./scripts/qmp/qmp-shell-wrap -l q.log -p -- ./build/qemu-system-x86_64 -display none
  Welcome to the QMP low-level shell!
  Connected
  (QEMU) query-kvm
  {
      "return": {
          "enabled": false,
          "present": true
      }
  }
  (QEMU) query-mice
  {
      "return": [
          {
              "absolute": false,
              "current": true,
              "index": 2,
              "name": "QEMU PS/2 Mouse"
          }
      ]
  }

 $ jq --slurp '. | to_entries[] | select(.value.execute == "query-kvm") |
               .value.return.enabled' < q.log
   false

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 python/qemu/qmp/qmp_shell.py | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/python/qemu/qmp/qmp_shell.py b/python/qemu/qmp/qmp_shell.py
index 12f7d28afc..a50c3f1bc7 100644
--- a/python/qemu/qmp/qmp_shell.py
+++ b/python/qemu/qmp/qmp_shell.py
@@ -165,7 +165,8 @@ class QMPShell(qmp.QEMUMonitorProtocol):
     def __init__(self, address: qmp.SocketAddrT,
                  pretty: bool = False,
                  verbose: bool = False,
-                 server: bool = False):
+                 server: bool = False,
+                 logfile: str = None):
         super().__init__(address, server=server)
         self._greeting: Optional[QMPMessage] = None
         self._completer = QMPCompleter()
@@ -175,6 +176,10 @@ def __init__(self, address: qmp.SocketAddrT,
                                       '.qmp-shell_history')
         self.pretty = pretty
         self.verbose = verbose
+        self.logfile = None
+
+        if logfile is not None:
+            self.logfile = open(logfile, "w")
 
     def close(self) -> None:
         # Hook into context manager of parent to save shell history.
@@ -315,11 +320,11 @@ def _build_cmd(self, cmdline: str) -> Optional[QMPMessage]:
         self._cli_expr(cmdargs[1:], qmpcmd['arguments'])
         return qmpcmd
 
-    def _print(self, qmp_message: object) -> None:
+    def _print(self, qmp_message: object, fh=sys.stdout) -> None:
         jsobj = json.dumps(qmp_message,
                            indent=4 if self.pretty else None,
                            sort_keys=self.pretty)
-        print(str(jsobj))
+        print(str(jsobj), file=fh)
 
     def _execute_cmd(self, cmdline: str) -> bool:
         try:
@@ -342,6 +347,9 @@ def _execute_cmd(self, cmdline: str) -> bool:
             print('Disconnected')
             return False
         self._print(resp)
+        if self.logfile is not None:
+            cmd = {**qmpcmd, **resp}
+            self._print(cmd, fh=self.logfile)
         return True
 
     def connect(self, negotiate: bool = True) -> None:
@@ -409,8 +417,9 @@ class HMPShell(QMPShell):
     def __init__(self, address: qmp.SocketAddrT,
                  pretty: bool = False,
                  verbose: bool = False,
-                 server: bool = False):
-        super().__init__(address, pretty, verbose, server)
+                 server: bool = False,
+                 logfile: str = None):
+        super().__init__(address, pretty, verbose, server, logfile)
         self._cpu_index = 0
 
     def _cmd_completion(self) -> None:
@@ -503,6 +512,8 @@ def main() -> None:
                         help='Verbose (echo commands sent and received)')
     parser.add_argument('-p', '--pretty', action='store_true',
                         help='Pretty-print JSON')
+    parser.add_argument('-l', '--logfile',
+                        help='Save log of all QMP messages to PATH')
 
     default_server = os.environ.get('QMP_SOCKET')
     parser.add_argument('qmp_server', action='store',
@@ -521,7 +532,7 @@ def main() -> None:
         parser.error(f"Bad port number: {args.qmp_server}")
         return  # pycharm doesn't know error() is noreturn
 
-    with shell_class(address, args.pretty, args.verbose) as qemu:
+    with shell_class(address, args.pretty, args.verbose, args.logfile) as qemu:
         try:
             qemu.connect(negotiate=not args.skip_negotiation)
         except qmp.QMPConnectError:
@@ -545,6 +556,8 @@ def main_wrap() -> None:
                         help='Verbose (echo commands sent and received)')
     parser.add_argument('-p', '--pretty', action='store_true',
                         help='Pretty-print JSON')
+    parser.add_argument('-l', '--logfile',
+                        help='Save log of all QMP messages to PATH')
 
     parser.add_argument('command', nargs=argparse.REMAINDER,
                         help='QEMU command line to invoke')
@@ -568,7 +581,7 @@ def main_wrap() -> None:
         parser.error(f"Bad port number: {socketpath}")
         return  # pycharm doesn't know error() is noreturn
 
-    with shell_class(address, args.pretty, args.verbose, True) as qemu:
+    with shell_class(address, args.pretty, args.verbose, True, args.logfile) as qemu:
         qemuproc = Popen(cmd)
 
         try:
-- 
2.33.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] python: support recording QMP session to a file
  2022-01-17 14:11 ` [PATCH 2/2] python: support recording QMP session to a file Daniel P. Berrangé
@ 2022-01-17 15:04   ` Philippe Mathieu-Daudé via
  0 siblings, 0 replies; 12+ messages in thread
From: Philippe Mathieu-Daudé via @ 2022-01-17 15:04 UTC (permalink / raw)
  To: Daniel P. Berrangé, qemu-devel
  Cc: Eduardo Habkost, John Snow, Markus Armbruster, Cleber Rosa

On 1/17/22 15:11, Daniel P. Berrangé wrote:
> When running QMP commands with very large response payloads, it is often
> not easy to spot the info you want. If we can save the response to a
> file then tools like 'grep' or 'jq' can be used to extract information.

> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  python/qemu/qmp/qmp_shell.py | 27 ++++++++++++++++++++-------
>  1 file changed, 20 insertions(+), 7 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] python: introduce qmp-shell-wrap convenience tool
  2022-01-17 14:11 ` [PATCH 1/2] python: introduce qmp-shell-wrap convenience tool Daniel P. Berrangé
@ 2022-01-17 23:27   ` John Snow
  2022-01-18  5:13     ` Philippe Mathieu-Daudé via
  2022-01-18 10:06     ` Daniel P. Berrangé
  0 siblings, 2 replies; 12+ messages in thread
From: John Snow @ 2022-01-17 23:27 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: Eduardo Habkost, Cleber Rosa, qemu-devel, Markus Armbruster

On Mon, Jan 17, 2022 at 9:11 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> With the current 'qmp-shell' tool developers must first spawn QEMU with
> a suitable -qmp arg and then spawn qmp-shell in a separate terminal
> pointing to the right socket.
>
> With 'qmp-shell-wrap' developers can ignore QMP sockets entirely and
> just pass the QEMU command and arguments they want. The program will
> listen on a UNIX socket and tell QEMU to connect QMP to that.
>
> For example, this:
>
>  # qmp-shell-wrap -- qemu-system-x86_64 -display none
>
> Is roughly equivalent of running:
>
>  # qemu-system-x86_64 -display none -qmp qmp-shell-1234 &
>  # qmp-shell qmp-shell-1234
>
> Except that 'qmp-shell-wrap' switches the socket peers around so that
> it is the UNIX socket server and QEMU is the socket client. This makes
> QEMU reliably go away when qmp-shell-wrap exits, closing the server
> socket.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  python/qemu/qmp/qmp_shell.py | 61 +++++++++++++++++++++++++++++++++---
>  scripts/qmp/qmp-shell-wrap   | 11 +++++++
>  2 files changed, 68 insertions(+), 4 deletions(-)
>  create mode 100755 scripts/qmp/qmp-shell-wrap
>
> diff --git a/python/qemu/qmp/qmp_shell.py b/python/qemu/qmp/qmp_shell.py
> index e7d7eb18f1..12f7d28afc 100644
> --- a/python/qemu/qmp/qmp_shell.py
> +++ b/python/qemu/qmp/qmp_shell.py
> @@ -86,6 +86,7 @@
>  import os
>  import re
>  import readline
> +from subprocess import Popen
>  import sys
>  from typing import (
>      Iterator,
> @@ -162,8 +163,10 @@ class QMPShell(qmp.QEMUMonitorProtocol):
>      :param verbose: Echo outgoing QMP messages to console.
>      """
>      def __init__(self, address: qmp.SocketAddrT,
> -                 pretty: bool = False, verbose: bool = False):
> -        super().__init__(address)
> +                 pretty: bool = False,
> +                 verbose: bool = False,
> +                 server: bool = False):
> +        super().__init__(address, server=server)
>          self._greeting: Optional[QMPMessage] = None
>          self._completer = QMPCompleter()
>          self._transmode = False
> @@ -404,8 +407,10 @@ class HMPShell(QMPShell):
>      :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)
> +                 pretty: bool = False,
> +                 verbose: bool = False,
> +                 server: bool = False):
> +        super().__init__(address, pretty, verbose, server)
>          self._cpu_index = 0
>
>      def _cmd_completion(self) -> None:
> @@ -529,6 +534,54 @@ def main() -> None:
>          for _ in qemu.repl():
>              pass
>
> +def main_wrap() -> None:
> +    """
> +    qmp-shell-wrap entry point: parse command line arguments and start the REPL.
> +    """
> +    parser = argparse.ArgumentParser()
> +    parser.add_argument('-H', '--hmp', action='store_true',
> +                        help='Use HMP interface')
> +    parser.add_argument('-v', '--verbose', action='store_true',
> +                        help='Verbose (echo commands sent and received)')
> +    parser.add_argument('-p', '--pretty', action='store_true',
> +                        help='Pretty-print JSON')
> +
> +    parser.add_argument('command', nargs=argparse.REMAINDER,
> +                        help='QEMU command line to invoke')
> +
> +    args = parser.parse_args()
> +
> +    cmd = args.command
> +    if len(cmd) != 0 and cmd[0] == '--':
> +        cmd = cmd[1:]
> +    if len(cmd) == 0:
> +        cmd = "qemu-system-x86_64"
> +
> +    sockpath = "qmp-shell-wrap-%d" % os.getpid()
> +    cmd += ["-qmp", "unix:%s" % sockpath]
> +
> +    shell_class = HMPShell if args.hmp else QMPShell
> +
> +    try:
> +        address = shell_class.parse_address(sockpath)
> +    except qmp.QMPBadPortError:
> +        parser.error(f"Bad port number: {socketpath}")
> +        return  # pycharm doesn't know error() is noreturn
> +
> +    with shell_class(address, args.pretty, args.verbose, True) as qemu:
> +        qemuproc = Popen(cmd)
> +
> +        try:
> +            qemu.accept()
> +        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 {sockpath}: {err!s}")
> +
> +        for _ in qemu.repl():
> +            pass
>
>  if __name__ == '__main__':
>      main()
> diff --git a/scripts/qmp/qmp-shell-wrap b/scripts/qmp/qmp-shell-wrap
> new file mode 100755
> index 0000000000..9e94da114f
> --- /dev/null
> +++ b/scripts/qmp/qmp-shell-wrap
> @@ -0,0 +1,11 @@
> +#!/usr/bin/env python3
> +
> +import os
> +import sys
> +
> +sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
> +from qemu.qmp import qmp_shell
> +
> +
> +if __name__ == '__main__':
> +    qmp_shell.main_wrap()
> --
> 2.33.1
>

Adds some new failures to the python linters; try "make check-dev" in
the python sub-dir.

... Though, due to a bug in avocado, this helpfully doesn't actually
show you the failure output right now ...

making this little edit should fix that, sorry for the inconvenience here.

diff --git a/python/avocado.cfg b/python/avocado.cfg
index c7722e7ecd..a460420059 100644
--- a/python/avocado.cfg
+++ b/python/avocado.cfg
@@ -1,5 +1,5 @@
 [run]
-test_runner = runner
+test_runner = nrunner



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] python: introduce qmp-shell-wrap convenience tool
  2022-01-17 23:27   ` John Snow
@ 2022-01-18  5:13     ` Philippe Mathieu-Daudé via
  2022-01-20 12:58       ` Beraldo Leal
  2022-01-18 10:06     ` Daniel P. Berrangé
  1 sibling, 1 reply; 12+ messages in thread
From: Philippe Mathieu-Daudé via @ 2022-01-18  5:13 UTC (permalink / raw)
  To: John Snow, Daniel P. Berrangé, Beraldo Leal
  Cc: Eduardo Habkost, Cleber Rosa, qemu-devel, Markus Armbruster

On 18/1/22 00:27, John Snow wrote:
> On Mon, Jan 17, 2022 at 9:11 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>>
>> With the current 'qmp-shell' tool developers must first spawn QEMU with
>> a suitable -qmp arg and then spawn qmp-shell in a separate terminal
>> pointing to the right socket.
>>
>> With 'qmp-shell-wrap' developers can ignore QMP sockets entirely and
>> just pass the QEMU command and arguments they want. The program will
>> listen on a UNIX socket and tell QEMU to connect QMP to that.
>>
>> For example, this:
>>
>>   # qmp-shell-wrap -- qemu-system-x86_64 -display none
>>
>> Is roughly equivalent of running:
>>
>>   # qemu-system-x86_64 -display none -qmp qmp-shell-1234 &
>>   # qmp-shell qmp-shell-1234
>>
>> Except that 'qmp-shell-wrap' switches the socket peers around so that
>> it is the UNIX socket server and QEMU is the socket client. This makes
>> QEMU reliably go away when qmp-shell-wrap exits, closing the server
>> socket.
>>
>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>> ---
>>   python/qemu/qmp/qmp_shell.py | 61 +++++++++++++++++++++++++++++++++---
>>   scripts/qmp/qmp-shell-wrap   | 11 +++++++
>>   2 files changed, 68 insertions(+), 4 deletions(-)
>>   create mode 100755 scripts/qmp/qmp-shell-wrap

>> diff --git a/scripts/qmp/qmp-shell-wrap b/scripts/qmp/qmp-shell-wrap
>> new file mode 100755
>> index 0000000000..9e94da114f
>> --- /dev/null
>> +++ b/scripts/qmp/qmp-shell-wrap
>> @@ -0,0 +1,11 @@
>> +#!/usr/bin/env python3
>> +
>> +import os
>> +import sys
>> +
>> +sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
>> +from qemu.qmp import qmp_shell
>> +
>> +
>> +if __name__ == '__main__':
>> +    qmp_shell.main_wrap()
>> --
>> 2.33.1
>>
> 
> Adds some new failures to the python linters; try "make check-dev" in
> the python sub-dir.
> 
> ... Though, due to a bug in avocado, this helpfully doesn't actually
> show you the failure output right now ...
> 
> making this little edit should fix that, sorry for the inconvenience here.
> 
> diff --git a/python/avocado.cfg b/python/avocado.cfg
> index c7722e7ecd..a460420059 100644
> --- a/python/avocado.cfg
> +++ b/python/avocado.cfg
> @@ -1,5 +1,5 @@
>   [run]
> -test_runner = runner
> +test_runner = nrunner

Cc'ing Beraldo, Willian once told me the nrunner switch was scheduled
for QEMU next release.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] python: introduce qmp-shell-wrap convenience tool
  2022-01-17 23:27   ` John Snow
  2022-01-18  5:13     ` Philippe Mathieu-Daudé via
@ 2022-01-18 10:06     ` Daniel P. Berrangé
  2022-01-18 18:04       ` John Snow
  1 sibling, 1 reply; 12+ messages in thread
From: Daniel P. Berrangé @ 2022-01-18 10:06 UTC (permalink / raw)
  To: John Snow; +Cc: Eduardo Habkost, Cleber Rosa, qemu-devel, Markus Armbruster

On Mon, Jan 17, 2022 at 06:27:24PM -0500, John Snow wrote:
> On Mon, Jan 17, 2022 at 9:11 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > With the current 'qmp-shell' tool developers must first spawn QEMU with
> > a suitable -qmp arg and then spawn qmp-shell in a separate terminal
> > pointing to the right socket.
> >
> > With 'qmp-shell-wrap' developers can ignore QMP sockets entirely and
> > just pass the QEMU command and arguments they want. The program will
> > listen on a UNIX socket and tell QEMU to connect QMP to that.
> >
> > For example, this:
> >
> >  # qmp-shell-wrap -- qemu-system-x86_64 -display none
> >
> > Is roughly equivalent of running:
> >
> >  # qemu-system-x86_64 -display none -qmp qmp-shell-1234 &
> >  # qmp-shell qmp-shell-1234
> >
> > Except that 'qmp-shell-wrap' switches the socket peers around so that
> > it is the UNIX socket server and QEMU is the socket client. This makes
> > QEMU reliably go away when qmp-shell-wrap exits, closing the server
> > socket.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  python/qemu/qmp/qmp_shell.py | 61 +++++++++++++++++++++++++++++++++---
> >  scripts/qmp/qmp-shell-wrap   | 11 +++++++
> >  2 files changed, 68 insertions(+), 4 deletions(-)
> >  create mode 100755 scripts/qmp/qmp-shell-wrap
> >
> > diff --git a/python/qemu/qmp/qmp_shell.py b/python/qemu/qmp/qmp_shell.py
> > index e7d7eb18f1..12f7d28afc 100644
> > --- a/python/qemu/qmp/qmp_shell.py
> > +++ b/python/qemu/qmp/qmp_shell.py
> > @@ -86,6 +86,7 @@
> >  import os
> >  import re
> >  import readline
> > +from subprocess import Popen
> >  import sys
> >  from typing import (
> >      Iterator,
> > @@ -162,8 +163,10 @@ class QMPShell(qmp.QEMUMonitorProtocol):
> >      :param verbose: Echo outgoing QMP messages to console.
> >      """
> >      def __init__(self, address: qmp.SocketAddrT,
> > -                 pretty: bool = False, verbose: bool = False):
> > -        super().__init__(address)
> > +                 pretty: bool = False,
> > +                 verbose: bool = False,
> > +                 server: bool = False):
> > +        super().__init__(address, server=server)
> >          self._greeting: Optional[QMPMessage] = None
> >          self._completer = QMPCompleter()
> >          self._transmode = False
> > @@ -404,8 +407,10 @@ class HMPShell(QMPShell):
> >      :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)
> > +                 pretty: bool = False,
> > +                 verbose: bool = False,
> > +                 server: bool = False):
> > +        super().__init__(address, pretty, verbose, server)
> >          self._cpu_index = 0
> >
> >      def _cmd_completion(self) -> None:
> > @@ -529,6 +534,54 @@ def main() -> None:
> >          for _ in qemu.repl():
> >              pass
> >
> > +def main_wrap() -> None:
> > +    """
> > +    qmp-shell-wrap entry point: parse command line arguments and start the REPL.
> > +    """
> > +    parser = argparse.ArgumentParser()
> > +    parser.add_argument('-H', '--hmp', action='store_true',
> > +                        help='Use HMP interface')
> > +    parser.add_argument('-v', '--verbose', action='store_true',
> > +                        help='Verbose (echo commands sent and received)')
> > +    parser.add_argument('-p', '--pretty', action='store_true',
> > +                        help='Pretty-print JSON')
> > +
> > +    parser.add_argument('command', nargs=argparse.REMAINDER,
> > +                        help='QEMU command line to invoke')
> > +
> > +    args = parser.parse_args()
> > +
> > +    cmd = args.command
> > +    if len(cmd) != 0 and cmd[0] == '--':
> > +        cmd = cmd[1:]
> > +    if len(cmd) == 0:
> > +        cmd = "qemu-system-x86_64"
> > +
> > +    sockpath = "qmp-shell-wrap-%d" % os.getpid()
> > +    cmd += ["-qmp", "unix:%s" % sockpath]
> > +
> > +    shell_class = HMPShell if args.hmp else QMPShell
> > +
> > +    try:
> > +        address = shell_class.parse_address(sockpath)
> > +    except qmp.QMPBadPortError:
> > +        parser.error(f"Bad port number: {socketpath}")
> > +        return  # pycharm doesn't know error() is noreturn
> > +
> > +    with shell_class(address, args.pretty, args.verbose, True) as qemu:
> > +        qemuproc = Popen(cmd)
> > +
> > +        try:
> > +            qemu.accept()
> > +        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 {sockpath}: {err!s}")
> > +
> > +        for _ in qemu.repl():
> > +            pass
> >
> >  if __name__ == '__main__':
> >      main()
> > diff --git a/scripts/qmp/qmp-shell-wrap b/scripts/qmp/qmp-shell-wrap
> > new file mode 100755
> > index 0000000000..9e94da114f
> > --- /dev/null
> > +++ b/scripts/qmp/qmp-shell-wrap
> > @@ -0,0 +1,11 @@
> > +#!/usr/bin/env python3
> > +
> > +import os
> > +import sys
> > +
> > +sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
> > +from qemu.qmp import qmp_shell
> > +
> > +
> > +if __name__ == '__main__':
> > +    qmp_shell.main_wrap()
> > --
> > 2.33.1
> >
> 
> Adds some new failures to the python linters; try "make check-dev" in
> the python sub-dir.

It would be nice to just have this integrated into 'make check' so we
don't need to remember to run a special command.

> ... Though, due to a bug in avocado, this helpfully doesn't actually
> show you the failure output right now ...

Urgh.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] python: introduce qmp-shell-wrap convenience tool
  2022-01-18 10:06     ` Daniel P. Berrangé
@ 2022-01-18 18:04       ` John Snow
  2022-01-20 13:33         ` Philippe Mathieu-Daudé via
  0 siblings, 1 reply; 12+ messages in thread
From: John Snow @ 2022-01-18 18:04 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: Eduardo Habkost, Beraldo Leal, Cleber Rosa, qemu-devel,
	Markus Armbruster

On Tue, Jan 18, 2022 at 5:06 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Mon, Jan 17, 2022 at 06:27:24PM -0500, John Snow wrote:
> > On Mon, Jan 17, 2022 at 9:11 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
> > >
> > > With the current 'qmp-shell' tool developers must first spawn QEMU with
> > > a suitable -qmp arg and then spawn qmp-shell in a separate terminal
> > > pointing to the right socket.
> > >
> > > With 'qmp-shell-wrap' developers can ignore QMP sockets entirely and
> > > just pass the QEMU command and arguments they want. The program will
> > > listen on a UNIX socket and tell QEMU to connect QMP to that.
> > >
> > > For example, this:
> > >
> > >  # qmp-shell-wrap -- qemu-system-x86_64 -display none
> > >
> > > Is roughly equivalent of running:
> > >
> > >  # qemu-system-x86_64 -display none -qmp qmp-shell-1234 &
> > >  # qmp-shell qmp-shell-1234
> > >
> > > Except that 'qmp-shell-wrap' switches the socket peers around so that
> > > it is the UNIX socket server and QEMU is the socket client. This makes
> > > QEMU reliably go away when qmp-shell-wrap exits, closing the server
> > > socket.
> > >
> > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > > ---
> > >  python/qemu/qmp/qmp_shell.py | 61 +++++++++++++++++++++++++++++++++---
> > >  scripts/qmp/qmp-shell-wrap   | 11 +++++++
> > >  2 files changed, 68 insertions(+), 4 deletions(-)
> > >  create mode 100755 scripts/qmp/qmp-shell-wrap
> > >
> > > diff --git a/python/qemu/qmp/qmp_shell.py b/python/qemu/qmp/qmp_shell.py
> > > index e7d7eb18f1..12f7d28afc 100644
> > > --- a/python/qemu/qmp/qmp_shell.py
> > > +++ b/python/qemu/qmp/qmp_shell.py
> > > @@ -86,6 +86,7 @@
> > >  import os
> > >  import re
> > >  import readline
> > > +from subprocess import Popen
> > >  import sys
> > >  from typing import (
> > >      Iterator,
> > > @@ -162,8 +163,10 @@ class QMPShell(qmp.QEMUMonitorProtocol):
> > >      :param verbose: Echo outgoing QMP messages to console.
> > >      """
> > >      def __init__(self, address: qmp.SocketAddrT,
> > > -                 pretty: bool = False, verbose: bool = False):
> > > -        super().__init__(address)
> > > +                 pretty: bool = False,
> > > +                 verbose: bool = False,
> > > +                 server: bool = False):
> > > +        super().__init__(address, server=server)
> > >          self._greeting: Optional[QMPMessage] = None
> > >          self._completer = QMPCompleter()
> > >          self._transmode = False
> > > @@ -404,8 +407,10 @@ class HMPShell(QMPShell):
> > >      :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)
> > > +                 pretty: bool = False,
> > > +                 verbose: bool = False,
> > > +                 server: bool = False):
> > > +        super().__init__(address, pretty, verbose, server)
> > >          self._cpu_index = 0
> > >
> > >      def _cmd_completion(self) -> None:
> > > @@ -529,6 +534,54 @@ def main() -> None:
> > >          for _ in qemu.repl():
> > >              pass
> > >
> > > +def main_wrap() -> None:
> > > +    """
> > > +    qmp-shell-wrap entry point: parse command line arguments and start the REPL.
> > > +    """
> > > +    parser = argparse.ArgumentParser()
> > > +    parser.add_argument('-H', '--hmp', action='store_true',
> > > +                        help='Use HMP interface')
> > > +    parser.add_argument('-v', '--verbose', action='store_true',
> > > +                        help='Verbose (echo commands sent and received)')
> > > +    parser.add_argument('-p', '--pretty', action='store_true',
> > > +                        help='Pretty-print JSON')
> > > +
> > > +    parser.add_argument('command', nargs=argparse.REMAINDER,
> > > +                        help='QEMU command line to invoke')
> > > +
> > > +    args = parser.parse_args()
> > > +
> > > +    cmd = args.command
> > > +    if len(cmd) != 0 and cmd[0] == '--':
> > > +        cmd = cmd[1:]
> > > +    if len(cmd) == 0:
> > > +        cmd = "qemu-system-x86_64"
> > > +
> > > +    sockpath = "qmp-shell-wrap-%d" % os.getpid()
> > > +    cmd += ["-qmp", "unix:%s" % sockpath]
> > > +
> > > +    shell_class = HMPShell if args.hmp else QMPShell
> > > +
> > > +    try:
> > > +        address = shell_class.parse_address(sockpath)
> > > +    except qmp.QMPBadPortError:
> > > +        parser.error(f"Bad port number: {socketpath}")
> > > +        return  # pycharm doesn't know error() is noreturn
> > > +
> > > +    with shell_class(address, args.pretty, args.verbose, True) as qemu:
> > > +        qemuproc = Popen(cmd)
> > > +
> > > +        try:
> > > +            qemu.accept()
> > > +        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 {sockpath}: {err!s}")
> > > +
> > > +        for _ in qemu.repl():
> > > +            pass
> > >
> > >  if __name__ == '__main__':
> > >      main()
> > > diff --git a/scripts/qmp/qmp-shell-wrap b/scripts/qmp/qmp-shell-wrap
> > > new file mode 100755
> > > index 0000000000..9e94da114f
> > > --- /dev/null
> > > +++ b/scripts/qmp/qmp-shell-wrap
> > > @@ -0,0 +1,11 @@
> > > +#!/usr/bin/env python3
> > > +
> > > +import os
> > > +import sys
> > > +
> > > +sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
> > > +from qemu.qmp import qmp_shell
> > > +
> > > +
> > > +if __name__ == '__main__':
> > > +    qmp_shell.main_wrap()
> > > --
> > > 2.33.1
> > >
> >
> > Adds some new failures to the python linters; try "make check-dev" in
> > the python sub-dir.
>
> It would be nice to just have this integrated into 'make check' so we
> don't need to remember to run a special command.

The CI will run it, but 'make check' doesn't. To add it to make check,
I need to figure out how to insert a venv-building step into 'make
check' such that the venv gets deposited into the build dir instead of
the source dir.
I think I may also need yet another set of package dependencies that
pin on precise dependencies for testing purposes to prevent random
regressions during 'make check' when nobody has touched the Python
code.

Overall, I felt like maybe it was more hassle than it was worth if I
can just nudge people touching the python to run a 'make check-dev'
every so often.

Patches welcome, etc. My overall strategy with the python tests so far has been:

(1) Keep python tests fully separate from the QEMU build system to
allow them to be split out into new repositories easily.
(2) Use the pipenv test to lock the very oldest dependencies the code
and tests support, using the very oldest python we support (3.6) This
test is used as the gating test in GitLab CI, as it is very repeatable
and the GitLab CI setup ensures I can always have the exact Python
packages it requires available.
(3) Use the tox test to test against a wide variety of Python
interpreters (3.6 through 3.10 inclusive) using the very latest python
packages to detect regressions on cutting-edge environments
(4) Use the widest possible range of versions for dependent packages
in setup.cfg such that QEMU packages are unlikely to cause versioning
conflicts in environments that decide to integrate our code.

Overall, I test on 3.6 through 3.10, and against the "oldest" and
"newest" dependencies. It's a good, wide matrix.

However, It's #4 there that runs me into trouble with tests that are
guaranteed to pass -- the linters update all the time and cause new
problems. I use pipenv to lock to specific versions, but that tool
wants to run against Python 3.6 *explicitly*, so it isn't suitable for
a generic purpose 'make check' because not everyone will have a Python
3.6 interpreter available. I need something kind of halfway between,
where I can lock against specific versions but not against the Python
interpreter version, and that's what could be used for a decent 'make
check' test.

Of course, I don't want to maintain like 10 versions of a dependent
packages list, either.

(I really, really wish pip had an --use-oldest flag for dependency
resolution, but it doesn't.)

>
> > ... Though, due to a bug in avocado, this helpfully doesn't actually
> > show you the failure output right now ...
>
> Urgh.
>

It regressed during winter vacation. I sent a small patch to enable
the new runner [1], and I have been discussing with Beraldo how to
re-enable the Coverage.py tests under the new Avocado.

[1] https://lists.gnu.org/archive/html/qemu-devel/2022-01/msg03614.html



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] python: introduce qmp-shell-wrap convenience tool
  2022-01-18  5:13     ` Philippe Mathieu-Daudé via
@ 2022-01-20 12:58       ` Beraldo Leal
  0 siblings, 0 replies; 12+ messages in thread
From: Beraldo Leal @ 2022-01-20 12:58 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Eduardo Habkost, Daniel P. Berrangé,
	qemu-devel, Markus Armbruster, Cleber Rosa, John Snow

On Tue, Jan 18, 2022 at 06:13:48AM +0100, Philippe Mathieu-Daudé wrote:
> On 18/1/22 00:27, John Snow wrote:
> > On Mon, Jan 17, 2022 at 9:11 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > 
> > > With the current 'qmp-shell' tool developers must first spawn QEMU with
> > > a suitable -qmp arg and then spawn qmp-shell in a separate terminal
> > > pointing to the right socket.
> > > 
> > > With 'qmp-shell-wrap' developers can ignore QMP sockets entirely and
> > > just pass the QEMU command and arguments they want. The program will
> > > listen on a UNIX socket and tell QEMU to connect QMP to that.
> > > 
> > > For example, this:
> > > 
> > >   # qmp-shell-wrap -- qemu-system-x86_64 -display none
> > > 
> > > Is roughly equivalent of running:
> > > 
> > >   # qemu-system-x86_64 -display none -qmp qmp-shell-1234 &
> > >   # qmp-shell qmp-shell-1234
> > > 
> > > Except that 'qmp-shell-wrap' switches the socket peers around so that
> > > it is the UNIX socket server and QEMU is the socket client. This makes
> > > QEMU reliably go away when qmp-shell-wrap exits, closing the server
> > > socket.
> > > 
> > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > > ---
> > >   python/qemu/qmp/qmp_shell.py | 61 +++++++++++++++++++++++++++++++++---
> > >   scripts/qmp/qmp-shell-wrap   | 11 +++++++
> > >   2 files changed, 68 insertions(+), 4 deletions(-)
> > >   create mode 100755 scripts/qmp/qmp-shell-wrap
> 
> > > diff --git a/scripts/qmp/qmp-shell-wrap b/scripts/qmp/qmp-shell-wrap
> > > new file mode 100755
> > > index 0000000000..9e94da114f
> > > --- /dev/null
> > > +++ b/scripts/qmp/qmp-shell-wrap
> > > @@ -0,0 +1,11 @@
> > > +#!/usr/bin/env python3
> > > +
> > > +import os
> > > +import sys
> > > +
> > > +sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
> > > +from qemu.qmp import qmp_shell
> > > +
> > > +
> > > +if __name__ == '__main__':
> > > +    qmp_shell.main_wrap()
> > > --
> > > 2.33.1
> > > 
> > 
> > Adds some new failures to the python linters; try "make check-dev" in
> > the python sub-dir.
> > 
> > ... Though, due to a bug in avocado, this helpfully doesn't actually
> > show you the failure output right now ...
> > 
> > making this little edit should fix that, sorry for the inconvenience here.
> > 
> > diff --git a/python/avocado.cfg b/python/avocado.cfg
> > index c7722e7ecd..a460420059 100644
> > --- a/python/avocado.cfg
> > +++ b/python/avocado.cfg
> > @@ -1,5 +1,5 @@
> >   [run]
> > -test_runner = runner
> > +test_runner = nrunner
> 
> Cc'ing Beraldo, Willian once told me the nrunner switch was scheduled
> for QEMU next release.

Thanks Philippe, for the Cc.

We solved a few issues on the nrunner side recently and implemented a
set of changes regarding the logging logic aimed to improve the
debugging process.

Currently, our legacy runner ('runner') is in "dismantle mode," and its
usage with the latest versions of Avocado is discouraged.

Because of the nrunner's new architecture and how Coverage works [1], we
spotted an issue when using nrunner + Coverage [2]. We are also
investigating a possible alternative [3] in parallel.

[1] - https://coverage.readthedocs.io/en/stable/subprocess.html
[2] - https://github.com/avocado-framework/avocado/issues/4765
[3] - https://gitlab.com/beraldoleal/qemu/-/commit/c7aeb34505aab4357421890575cc9ddd7e83d04f

--
Beraldo



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] python: introduce qmp-shell-wrap convenience tool
  2022-01-18 18:04       ` John Snow
@ 2022-01-20 13:33         ` Philippe Mathieu-Daudé via
  2022-01-20 13:40           ` Daniel P. Berrangé
  0 siblings, 1 reply; 12+ messages in thread
From: Philippe Mathieu-Daudé via @ 2022-01-20 13:33 UTC (permalink / raw)
  To: John Snow, Daniel P. Berrangé
  Cc: Eduardo Habkost, Beraldo Leal, Cleber Rosa, qemu-devel,
	Markus Armbruster

On 18/1/22 19:04, John Snow wrote:
> On Tue, Jan 18, 2022 at 5:06 AM Daniel P. Berrangé <berrange@redhat.com> wrote:

>> It would be nice to just have this integrated into 'make check' so we
>> don't need to remember to run a special command.
> 
> The CI will run it, but 'make check' doesn't. To add it to make check,
> I need to figure out how to insert a venv-building step into 'make
> check' such that the venv gets deposited into the build dir instead of
> the source dir.
> I think I may also need yet another set of package dependencies that
> pin on precise dependencies for testing purposes to prevent random
> regressions during 'make check' when nobody has touched the Python
> code.
> 
> Overall, I felt like maybe it was more hassle than it was worth if I
> can just nudge people touching the python to run a 'make check-dev'
> every so often.
> 
> Patches welcome, etc. My overall strategy with the python tests so far has been:
> 
> (1) Keep python tests fully separate from the QEMU build system to
> allow them to be split out into new repositories easily.
> (2) Use the pipenv test to lock the very oldest dependencies the code
> and tests support, using the very oldest python we support (3.6) This
> test is used as the gating test in GitLab CI, as it is very repeatable
> and the GitLab CI setup ensures I can always have the exact Python
> packages it requires available.
> (3) Use the tox test to test against a wide variety of Python
> interpreters (3.6 through 3.10 inclusive) using the very latest python
> packages to detect regressions on cutting-edge environments
> (4) Use the widest possible range of versions for dependent packages
> in setup.cfg such that QEMU packages are unlikely to cause versioning
> conflicts in environments that decide to integrate our code.
> 
> Overall, I test on 3.6 through 3.10, and against the "oldest" and
> "newest" dependencies. It's a good, wide matrix.
> 
> However, It's #4 there that runs me into trouble with tests that are
> guaranteed to pass -- the linters update all the time and cause new
> problems. I use pipenv to lock to specific versions, but that tool
> wants to run against Python 3.6 *explicitly*, so it isn't suitable for
> a generic purpose 'make check' because not everyone will have a Python
> 3.6 interpreter available. I need something kind of halfway between,
> where I can lock against specific versions but not against the Python
> interpreter version, and that's what could be used for a decent 'make
> check' test.
> 
> Of course, I don't want to maintain like 10 versions of a dependent
> packages list, either.
> 
> (I really, really wish pip had an --use-oldest flag for dependency
> resolution, but it doesn't.)

Could we simply use a virtualenv for all QEMU testing tasks (packages
consumed by QEMU tests), and only deal with installed Python packages
for regular non-testing QEMU uses (things exposed via pyqemu that we
want stable)?


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] python: introduce qmp-shell-wrap convenience tool
  2022-01-20 13:33         ` Philippe Mathieu-Daudé via
@ 2022-01-20 13:40           ` Daniel P. Berrangé
  2022-01-20 21:34             ` John Snow
  0 siblings, 1 reply; 12+ messages in thread
From: Daniel P. Berrangé @ 2022-01-20 13:40 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Eduardo Habkost, Beraldo Leal, Markus Armbruster, qemu-devel,
	Cleber Rosa, John Snow

On Thu, Jan 20, 2022 at 02:33:46PM +0100, Philippe Mathieu-Daudé wrote:
> On 18/1/22 19:04, John Snow wrote:
> > On Tue, Jan 18, 2022 at 5:06 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
> 
> > > It would be nice to just have this integrated into 'make check' so we
> > > don't need to remember to run a special command.
> > 
> > The CI will run it, but 'make check' doesn't. To add it to make check,
> > I need to figure out how to insert a venv-building step into 'make
> > check' such that the venv gets deposited into the build dir instead of
> > the source dir.
> > I think I may also need yet another set of package dependencies that
> > pin on precise dependencies for testing purposes to prevent random
> > regressions during 'make check' when nobody has touched the Python
> > code.
> > 
> > Overall, I felt like maybe it was more hassle than it was worth if I
> > can just nudge people touching the python to run a 'make check-dev'
> > every so often.
> > 
> > Patches welcome, etc. My overall strategy with the python tests so far has been:
> > 
> > (1) Keep python tests fully separate from the QEMU build system to
> > allow them to be split out into new repositories easily.
> > (2) Use the pipenv test to lock the very oldest dependencies the code
> > and tests support, using the very oldest python we support (3.6) This
> > test is used as the gating test in GitLab CI, as it is very repeatable
> > and the GitLab CI setup ensures I can always have the exact Python
> > packages it requires available.
> > (3) Use the tox test to test against a wide variety of Python
> > interpreters (3.6 through 3.10 inclusive) using the very latest python
> > packages to detect regressions on cutting-edge environments
> > (4) Use the widest possible range of versions for dependent packages
> > in setup.cfg such that QEMU packages are unlikely to cause versioning
> > conflicts in environments that decide to integrate our code.
> > 
> > Overall, I test on 3.6 through 3.10, and against the "oldest" and
> > "newest" dependencies. It's a good, wide matrix.
> > 
> > However, It's #4 there that runs me into trouble with tests that are
> > guaranteed to pass -- the linters update all the time and cause new
> > problems. I use pipenv to lock to specific versions, but that tool
> > wants to run against Python 3.6 *explicitly*, so it isn't suitable for
> > a generic purpose 'make check' because not everyone will have a Python
> > 3.6 interpreter available. I need something kind of halfway between,
> > where I can lock against specific versions but not against the Python
> > interpreter version, and that's what could be used for a decent 'make
> > check' test.
> > 
> > Of course, I don't want to maintain like 10 versions of a dependent
> > packages list, either.
> > 
> > (I really, really wish pip had an --use-oldest flag for dependency
> > resolution, but it doesn't.)
> 
> Could we simply use a virtualenv for all QEMU testing tasks (packages
> consumed by QEMU tests), and only deal with installed Python packages
> for regular non-testing QEMU uses (things exposed via pyqemu that we
> want stable)?

I don't especially like the idea of using virtualenv. It is a good thing
that we're testing on the distro python packages, because that is the
scenario that our contributors/developers will actually use these
tools in. We're got a well defined set of target platforms that QEMU
aims for that we need to cover in testing. If we also want to do tests
against general python using a virtualenv in CI pipelines thats fine,
but doesn't replace the need to testing against our explicit platform
targets, its just additive.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] python: introduce qmp-shell-wrap convenience tool
  2022-01-20 13:40           ` Daniel P. Berrangé
@ 2022-01-20 21:34             ` John Snow
  0 siblings, 0 replies; 12+ messages in thread
From: John Snow @ 2022-01-20 21:34 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: Eduardo Habkost, Beraldo Leal, Markus Armbruster, qemu-devel,
	Cleber Rosa, Philippe Mathieu-Daudé

On Thu, Jan 20, 2022 at 8:40 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Thu, Jan 20, 2022 at 02:33:46PM +0100, Philippe Mathieu-Daudé wrote:
> > On 18/1/22 19:04, John Snow wrote:
> > > On Tue, Jan 18, 2022 at 5:06 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > > > It would be nice to just have this integrated into 'make check' so we
> > > > don't need to remember to run a special command.
> > >
> > > The CI will run it, but 'make check' doesn't. To add it to make check,
> > > I need to figure out how to insert a venv-building step into 'make
> > > check' such that the venv gets deposited into the build dir instead of
> > > the source dir.
> > > I think I may also need yet another set of package dependencies that
> > > pin on precise dependencies for testing purposes to prevent random
> > > regressions during 'make check' when nobody has touched the Python
> > > code.
> > >
> > > Overall, I felt like maybe it was more hassle than it was worth if I
> > > can just nudge people touching the python to run a 'make check-dev'
> > > every so often.
> > >
> > > Patches welcome, etc. My overall strategy with the python tests so far has been:
> > >
> > > (1) Keep python tests fully separate from the QEMU build system to
> > > allow them to be split out into new repositories easily.
> > > (2) Use the pipenv test to lock the very oldest dependencies the code
> > > and tests support, using the very oldest python we support (3.6) This
> > > test is used as the gating test in GitLab CI, as it is very repeatable
> > > and the GitLab CI setup ensures I can always have the exact Python
> > > packages it requires available.
> > > (3) Use the tox test to test against a wide variety of Python
> > > interpreters (3.6 through 3.10 inclusive) using the very latest python
> > > packages to detect regressions on cutting-edge environments
> > > (4) Use the widest possible range of versions for dependent packages
> > > in setup.cfg such that QEMU packages are unlikely to cause versioning
> > > conflicts in environments that decide to integrate our code.
> > >
> > > Overall, I test on 3.6 through 3.10, and against the "oldest" and
> > > "newest" dependencies. It's a good, wide matrix.
> > >
> > > However, It's #4 there that runs me into trouble with tests that are
> > > guaranteed to pass -- the linters update all the time and cause new
> > > problems. I use pipenv to lock to specific versions, but that tool
> > > wants to run against Python 3.6 *explicitly*, so it isn't suitable for
> > > a generic purpose 'make check' because not everyone will have a Python
> > > 3.6 interpreter available. I need something kind of halfway between,
> > > where I can lock against specific versions but not against the Python
> > > interpreter version, and that's what could be used for a decent 'make
> > > check' test.
> > >
> > > Of course, I don't want to maintain like 10 versions of a dependent
> > > packages list, either.
> > >
> > > (I really, really wish pip had an --use-oldest flag for dependency
> > > resolution, but it doesn't.)
> >
> > Could we simply use a virtualenv for all QEMU testing tasks (packages
> > consumed by QEMU tests), and only deal with installed Python packages
> > for regular non-testing QEMU uses (things exposed via pyqemu that we
> > want stable)?

There's no python packages we need except those that are for testing.
Everything we need for the build is self-contained. "Python packages
regular non-testing QEMU uses" is an empty set. We do ship *one*
self-contained Python script, qemu-trace-stap. That lives in
scripts/qemu-trace-stap and we don't need to mess with it right now.

Now, in the process of setting up the python/ directory, Cleber and I
had discussed how to handle virtual environments for that directory --
as it has its own requirements, its own goals, etc. We decided at the
time it would be best not to mix the dependencies of the test system
with the dependencies of the burgeoning QEMU library. Let me elaborate
on why we believe that to be the right decision:

In summary, the Python in use in the QEMU tree can be found in these categories:

- Used during the build itself, Not shipped with QEMU. Zero external
dependencies.
- Used for testing, Not shipped with QEMU. Avocado tests use a scant
few external dependencies.
- Not used for building nor testing, Shipped with QEMU. (This is to my
knowledge a single script with no dependencies.)

Now, there are two kinds of tests that involves python code:

- Tests that are used for testing QEMU that incidentally use or are
written in Python
- Tests that are used for testing the Python code itself

The first category of test there makes sense to be tied to a QEMU
build. That's what Avocado tests are, and they use their own virtual
environment for the purpose.
The second category of test does not make sense to be tied to a QEMU
build, because it does not test QEMU. This is what the python/ CI
tests behave like.

The different in purpose and motivation between those two categories
of tests is why where are presently two different virtual environment
regimes in the tree, and it's very likely that making any attempt to
combine them is a very bad idea.
For example, /python/qemu/aqmp/ is on its way to being forked out as
its own standalone library. This library needs its own set of
requirements for being installed, tested, etc. tests/avocado/, on the
other hand, is an application that will consume the QMP library, and
it will likely have its own requirements.

Put another way: mixing the requirements of library and application
together simply because they're in the same git tree at the moment is
not good.

>
> I don't especially like the idea of using virtualenv. It is a good thing
> that we're testing on the distro python packages, because that is the
> scenario that our contributors/developers will actually use these
> tools in. We're got a well defined set of target platforms that QEMU
> aims for that we need to cover in testing. If we also want to do tests
> against general python using a virtualenv in CI pipelines thats fine,
> but doesn't replace the need to testing against our explicit platform
> targets, its just additive.

> I don't especially like the idea of using virtualenv.

Well, we already are using them in multiple places.
tests/requirements.txt is used to instantiate a per-build VENV that is
used to run the Avocado tests, and the python/ tests use several
different ones in order to achieve a wide testing matrix for testing
itself.

> It is a good thing that we're testing on the distro python packages

I mean, virtual environments don't download new copies of Python. No
matter what, we're testing against "distro python". All the virtual
environment does is allow us to pull very specific python package
dependencies. For example, the avocado tests mandate that we use
version 88.1 of avocado-framework. I assume most distributions don't
even *have* avocado bundled in their repo. For sure, absolutely nobody
will have packaged "qemu.qmp" by the time I need to start using it.
Python testing packages (mypy, pylint, avocado, et al) move so fast
that trying to maintain compatibility with every last one that ships
as part of a distro repository is impossible (I've tried).

Most of all, though, I don't see the benefit in trying to maintain
compatibility with a random ancient version of 'pylint' that someone
might have as part of their platform. I see no reason why downloading
a more modern pylint into a cached folder somewhere hurts our ability
to test QEMU against our supported build platforms ... especially if
said package-grab indeed works on all of those build platforms.

> because that is the scenario that our contributors/developers will actually use these tools in

Not if the virtual environment is part of the test invocation
script... like it already is. Then everyone is using a very tightly
controlled version of the testing environment with the exact correct
dependencies. Mind you, I am drawing a distinction between build
scripts, test scripts, and tools. The argument I see you making here
is something that I believe applies to libraries and tools. That's
indeed exactly why the testing regime for python/ is completely
separate and fairly different from the QEMU build process, and covers
a wide spectrum of dependent versions precisely to guarantee maximum
compatibility.

> We're got a well defined set of target platforms that QEMU aims for that we need to cover in testing.

And we do! If we use a VENV to pull in some pure python code, who
cares? The point is that QEMU is getting built and tested on those
platforms. It matters extremely little if the test environment uses
tighter bounds for thinks like a QMP library. This is barely different
from initializing git submodules at the start of a build. I really
feel like you are being artificially harsh here.

> If we also want to do tests against general python using a virtualenv in CI pipelines thats fine,
> but doesn't replace the need to testing against our explicit platform targets, its just additive.

I don't understand this paragraph, unless you believe that virtual
environments also replace the Python interpreter. Any use of a venv to
run python tests is still gonna run those tests on these target
platforms you wanna make sure work. I don't understand your
apprehension.

--js



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-01-21  0:36 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-17 14:11 [PATCH 0/2] python: a few improvements to qmp-shell Daniel P. Berrangé
2022-01-17 14:11 ` [PATCH 1/2] python: introduce qmp-shell-wrap convenience tool Daniel P. Berrangé
2022-01-17 23:27   ` John Snow
2022-01-18  5:13     ` Philippe Mathieu-Daudé via
2022-01-20 12:58       ` Beraldo Leal
2022-01-18 10:06     ` Daniel P. Berrangé
2022-01-18 18:04       ` John Snow
2022-01-20 13:33         ` Philippe Mathieu-Daudé via
2022-01-20 13:40           ` Daniel P. Berrangé
2022-01-20 21:34             ` John Snow
2022-01-17 14:11 ` [PATCH 2/2] python: support recording QMP session to a file Daniel P. Berrangé
2022-01-17 15:04   ` Philippe Mathieu-Daudé via

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).