Re: [PATCH 1/2] python: introduce qmp-shell-wrap convenience tool

["Daniel P. Berrangé" <berrange@redhat.com>]

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 :|