[Qemu-devel] [PATCH for-2.11 v2 0/5] qmp-shell non-interactive mode, delete scripts/qmp/qmp

[Qemu-devel] [PATCH for-2.11 v2 0/5] qmp-shell non-interactive mode, delete scripts/qmp/qmp

[Eduardo Habkost]

This series adds the ability to run QMP commands
non-interactively to qmp-shell, and deletes scripts/qmp/qmp.

Changes v1 -> v2:
* Use optparse module instead of argparse. optparse is deprecated
  since Python 2.7, but argparse is not available on Python 2.6,
  the minimum Python version required to build QEMU.
  * Reported-by: Stefan Hajnoczi <stefanha@gmail.com>
  * Suggested-by: "Daniel P. Berrange" <berrange@redhat.com>

Eduardo Habkost (5):
  qmp-shell: Use optparse module
  qmp-shell: Pass split cmdargs to __build_cmd()
  qmp-shell: execute_cmdargs() method
  qmp-shell: Accept QMP command as argument
  Remove scripts/qmp/qmp

 scripts/qmp/qmp       | 126 --------------------------------------------------
 scripts/qmp/qmp-shell |  87 ++++++++++++++++------------------
 2 files changed, 39 insertions(+), 174 deletions(-)
 delete mode 100755 scripts/qmp/qmp

-- 
2.9.4

[Qemu-devel] [PATCH for-2.11 v2 1/5] qmp-shell: Use optparse module

[Eduardo Habkost]

It makes command-line parsing and generation of help text much
simpler.

The optparse module is deprecated since Python 2.7, but argparse
is not available in Python 2.6 (the minimum Python version
required for building QEMU).

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 -> v2:
* Use optparse module, as the minimum Python version for building
  QEMU is 2.6
  * Reported-by: Stefan Hajnoczi <stefanha@gmail.com>
  * Suggested-by: "Daniel P. Berrange" <berrange@redhat.com>
---
 scripts/qmp/qmp-shell | 63 +++++++++++++++++++--------------------------------
 1 file changed, 23 insertions(+), 40 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 860ffb2..ad72ef9 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -73,6 +73,7 @@ import sys
 import os
 import errno
 import atexit
+import optparse
 
 class QMPCompleter(list):
     def complete(self, text, state):
@@ -393,52 +394,34 @@ def die(msg):
     sys.stderr.write('ERROR: %s\n' % msg)
     sys.exit(1)
 
-def fail_cmdline(option=None):
-    if option:
-        sys.stderr.write('ERROR: bad command-line option \'%s\'\n' % option)
-    sys.stderr.write('qmp-shell [ -v ] [ -p ] [ -H ] [ -N ] < UNIX socket path> | < TCP address:port >\n')
-    sys.stderr.write('    -v     Verbose (echo command sent and received)\n')
-    sys.stderr.write('    -p     Pretty-print JSON\n')
-    sys.stderr.write('    -H     Use HMP interface\n')
-    sys.stderr.write('    -N     Skip negotiate (for qemu-ga)\n')
-    sys.exit(1)
-
 def main():
-    addr = ''
-    qemu = None
-    hmp = False
-    pretty = False
-    verbose = False
-    negotiate = True
+    parser = optparse.OptionParser(description='QMP shell utility')
+    parser.set_usage("%prog [options] <UNIX socket path> | <TCP address:port>")
+    parser.add_option('-v', action='store_true', dest='verbose',
+        help='Verbose (echo command sent and received)')
+    parser.add_option('-p', action='store_true', dest='pretty',
+        help='Pretty-print JSON')
+    parser.add_option('-H', action='store_true', dest='hmp',
+        help='Use HMP interface')
+    parser.add_option('-N', action='store_false', dest='negotiate',
+        default=True, help='Skip negotiate (for qemu-ga)')
+    opts,args = parser.parse_args()
+
+    if len(args) != 1:
+        parser.print_help(sys.stderr)
+        sys.exit(1)
+    addr = args[0]
 
     try:
-        for arg in sys.argv[1:]:
-            if arg == "-H":
-                if qemu is not None:
-                    fail_cmdline(arg)
-                hmp = True
-            elif arg == "-p":
-                pretty = True
-            elif arg == "-N":
-                negotiate = False
-            elif arg == "-v":
-                verbose = True
-            else:
-                if qemu is not None:
-                    fail_cmdline(arg)
-                if hmp:
-                    qemu = HMPShell(arg)
-                else:
-                    qemu = QMPShell(arg, pretty)
-                addr = arg
-
-        if qemu is None:
-            fail_cmdline()
+        if opts.hmp:
+            qemu = HMPShell(addr)
+        else:
+            qemu = QMPShell(addr, opts.pretty)
     except QMPShellBadPort:
         die('bad port number in command-line')
 
     try:
-        qemu.connect(negotiate)
+        qemu.connect(opts.negotiate)
     except qmp.QMPConnectError:
         die('Didn\'t get QMP greeting message')
     except qmp.QMPCapabilitiesError:
@@ -447,7 +430,7 @@ def main():
         die('Could not connect to %s' % addr)
 
     qemu.show_banner()
-    qemu.set_verbosity(verbose)
+    qemu.set_verbosity(opts.verbose)
     while qemu.read_exec_command(qemu.get_prompt()):
         pass
     qemu.close()
-- 
2.9.4

[Qemu-devel] [PATCH for-2.11 v2 2/5] qmp-shell: Pass split cmdargs to __build_cmd()

[Eduardo Habkost]

This will allow us to implement a method to run a command that is
already split in a list.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: John Snow <jsnow@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 -> v2:
* (none)
---
 scripts/qmp/qmp-shell | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index ad72ef9..f93c1cf 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -212,15 +212,13 @@ class QMPShell(qmp.QEMUMonitorProtocol):
                     raise QMPShellError('Cannot set "%s" multiple times' % key)
             parent[optpath[-1]] = value
 
-    def __build_cmd(self, cmdline):
+    def __build_cmd(self, cmdargs):
         """
         Build a QMP input object from a user provided command-line in the
         following format:
 
             < command-name > [ arg-name1=arg1 ] ... [ arg-nameN=argN ]
         """
-        cmdargs = cmdline.split()
-
         # Transactional CLI entry/exit:
         if cmdargs[0] == 'transaction(':
             self._transmode = True
@@ -247,7 +245,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
                 finalize = True
             self.__cli_expr(cmdargs[1:], action['data'])
             self._actions.append(action)
-            return self.__build_cmd(')') if finalize else None
+            return self.__build_cmd([')']) if finalize else None
 
         # Standard command: parse and return it to be executed.
         qmpcmd = { 'execute': cmdargs[0], 'arguments': {} }
@@ -262,8 +260,9 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         print str(jsobj)
 
     def _execute_cmd(self, cmdline):
+        cmdargs = cmdline.split()
         try:
-            qmpcmd = self.__build_cmd(cmdline)
+            qmpcmd = self.__build_cmd(cmdargs)
         except Exception as e:
             print 'Error while parsing command line: %s' % e
             print 'command format: <command-name> ',
-- 
2.9.4

[Qemu-devel] [PATCH for-2.11 v2 3/5] qmp-shell: execute_cmdargs() method

[Eduardo Habkost]

This will allow us to execute a command that was already split in
a list.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: John Snow <jsnow@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 -> v2:
* (none)
---
 scripts/qmp/qmp-shell | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index f93c1cf..4b9a420 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -260,7 +260,9 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         print str(jsobj)
 
     def _execute_cmd(self, cmdline):
-        cmdargs = cmdline.split()
+        return self.execute_cmdargs(cmdline.split())
+
+    def execute_cmdargs(self, cmdargs):
         try:
             qmpcmd = self.__build_cmd(cmdargs)
         except Exception as e:
@@ -386,6 +388,9 @@ class HMPShell(QMPShell):
             print '%s: %s' % (resp['error']['class'], resp['error']['desc'])
         return True
 
+    def execute_cmdargs(self, cmdargs):
+        return self._execute_cmd(' '.join(cmdargs))
+
     def show_banner(self):
         QMPShell.show_banner(self, msg='Welcome to the HMP shell!')
 
-- 
2.9.4

[Qemu-devel] [PATCH for-2.11 v2 4/5] qmp-shell: Accept QMP command as argument

[Eduardo Habkost]

This is useful for testing QMP commands in scripts.

Example usage, combined with 'jq' for filtering the results:

  $ ./scripts/qmp/qmp-shell /tmp/qmp qom-list path=/ | jq -r .return[].name
  machine
  type
  chardevs
  backend
  $

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 -> v2:
* Rewritten using optparse module
---
 scripts/qmp/qmp-shell | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 4b9a420..4b7374e 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -400,7 +400,7 @@ def die(msg):
 
 def main():
     parser = optparse.OptionParser(description='QMP shell utility')
-    parser.set_usage("%prog [options] <UNIX socket path> | <TCP address:port>")
+    parser.set_usage("%prog [options] <UNIX socket path> | <TCP address:port> [COMMAND [ARG=VALUE]...]")
     parser.add_option('-v', action='store_true', dest='verbose',
         help='Verbose (echo command sent and received)')
     parser.add_option('-p', action='store_true', dest='pretty',
@@ -411,10 +411,11 @@ def main():
         default=True, help='Skip negotiate (for qemu-ga)')
     opts,args = parser.parse_args()
 
-    if len(args) != 1:
+    if len(args) < 1:
         parser.print_help(sys.stderr)
         sys.exit(1)
     addr = args[0]
+    cmdargs = args[1:]
 
     try:
         if opts.hmp:
@@ -433,10 +434,13 @@ def main():
     except qemu.error:
         die('Could not connect to %s' % addr)
 
-    qemu.show_banner()
     qemu.set_verbosity(opts.verbose)
-    while qemu.read_exec_command(qemu.get_prompt()):
-        pass
+    if len(cmdargs):
+        qemu.execute_cmdargs(cmdargs)
+    else:
+        qemu.show_banner()
+        while qemu.read_exec_command(qemu.get_prompt()):
+            pass
     qemu.close()
 
 if __name__ == '__main__':
-- 
2.9.4

[Qemu-devel] [PATCH for-2.11 v2 5/5] Remove scripts/qmp/qmp

[Eduardo Habkost]

The only purpose of scripts/qmp/qmp was the ability to run QMP
commands non-interactively.  Now it is possible to run qmp-shell
non-interactively by providing a QMP command a command-line
argument, making scripts/qmp/qmp obsolete.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: John Snow <jsnow@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 -> v2:
* (none)
---
 scripts/qmp/qmp | 126 --------------------------------------------------------
 1 file changed, 126 deletions(-)
 delete mode 100755 scripts/qmp/qmp

diff --git a/scripts/qmp/qmp b/scripts/qmp/qmp
deleted file mode 100755
index 514b539..0000000
--- a/scripts/qmp/qmp
+++ /dev/null
@@ -1,126 +0,0 @@
-#!/usr/bin/python
-#
-# QMP command line tool
-#
-# Copyright IBM, Corp. 2011
-#
-# Authors:
-#  Anthony Liguori <aliguori@us.ibm.com>
-#
-# This work is licensed under the terms of the GNU GPLv2 or later.
-# See the COPYING file in the top-level directory.
-
-import sys, os
-from qmp import QEMUMonitorProtocol
-
-def print_response(rsp, prefix=[]):
-    if type(rsp) == list:
-        i = 0
-        for item in rsp:
-            if prefix == []:
-                prefix = ['item']
-            print_response(item, prefix[:-1] + ['%s[%d]' % (prefix[-1], i)])
-            i += 1
-    elif type(rsp) == dict:
-        for key in rsp.keys():
-            print_response(rsp[key], prefix + [key])
-    else:
-        if len(prefix):
-            print '%s: %s' % ('.'.join(prefix), rsp)
-        else:
-            print '%s' % (rsp)
-
-def main(args):
-    path = None
-
-    # Use QMP_PATH if it's set
-    if os.environ.has_key('QMP_PATH'):
-        path = os.environ['QMP_PATH']
-
-    while len(args):
-        arg = args[0]
-
-        if arg.startswith('--'):
-            arg = arg[2:]
-            if arg.find('=') == -1:
-                value = True
-            else:
-                arg, value = arg.split('=', 1)
-
-            if arg in ['path']:
-                if type(value) == str:
-                    path = value
-            elif arg in ['help']:
-                os.execlp('man', 'man', 'qmp')
-            else:
-                print 'Unknown argument "%s"' % arg
-
-            args = args[1:]
-        else:
-            break
-
-    if not path:
-        print "QMP path isn't set, use --path=qmp-monitor-address or set QMP_PATH"
-        return 1
-
-    if len(args):
-        command, args = args[0], args[1:]
-    else:
-        print 'No command found'
-        print 'Usage: "qmp [--path=qmp-monitor-address] qmp-cmd arguments"'
-        return 1
-
-    if command in ['help']:
-        os.execlp('man', 'man', 'qmp')
-
-    srv = QEMUMonitorProtocol(path)
-    srv.connect()
-
-    def do_command(srv, cmd, **kwds):
-        rsp = srv.cmd(cmd, kwds)
-        if rsp.has_key('error'):
-            raise Exception(rsp['error']['desc'])
-        return rsp['return']
-
-    commands = map(lambda x: x['name'], do_command(srv, 'query-commands'))
-
-    srv.close()
-
-    if command not in commands:
-        fullcmd = 'qmp-%s' % command
-        try:
-            os.environ['QMP_PATH'] = path
-            os.execvp(fullcmd, [fullcmd] + args)
-        except OSError as exc:
-            if exc.errno == 2:
-                print 'Command "%s" not found.' % (fullcmd)
-                return 1
-            raise
-        return 0
-
-    srv = QEMUMonitorProtocol(path)
-    srv.connect()
-
-    arguments = {}
-    for arg in args:
-        if not arg.startswith('--'):
-            print 'Unknown argument "%s"' % arg
-            return 1
-
-        arg = arg[2:]
-        if arg.find('=') == -1:
-            value = True
-        else:
-            arg, value = arg.split('=', 1)
-
-        if arg in ['help']:
-            os.execlp('man', 'man', 'qmp-%s' % command)
-            return 1
-
-        arguments[arg] = value
-
-    rsp = do_command(srv, command, **arguments)
-    print_response(rsp)
-
-if __name__ == '__main__':
-    sys.exit(main(sys.argv[1:]))
-- 
2.9.4

Re: [Qemu-devel] [PATCH for-2.11 v2 4/5] qmp-shell: Accept QMP command as argument

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 613 bytes --]

On Tue, Aug 08, 2017 at 05:39:34PM -0300, Eduardo Habkost wrote:
> This is useful for testing QMP commands in scripts.
> 
> Example usage, combined with 'jq' for filtering the results:
> 
>   $ ./scripts/qmp/qmp-shell /tmp/qmp qom-list path=/ | jq -r .return[].name
>   machine
>   type
>   chardevs
>   backend
>   $
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Changes v1 -> v2:
> * Rewritten using optparse module
> ---
>  scripts/qmp/qmp-shell | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH for-2.11 v2 1/5] qmp-shell: Use optparse module

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 803 bytes --]

On Tue, Aug 08, 2017 at 05:39:31PM -0300, Eduardo Habkost wrote:
> It makes command-line parsing and generation of help text much
> simpler.
> 
> The optparse module is deprecated since Python 2.7, but argparse
> is not available in Python 2.6 (the minimum Python version
> required for building QEMU).
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Changes v1 -> v2:
> * Use optparse module, as the minimum Python version for building
>   QEMU is 2.6
>   * Reported-by: Stefan Hajnoczi <stefanha@gmail.com>
>   * Suggested-by: "Daniel P. Berrange" <berrange@redhat.com>
> ---
>  scripts/qmp/qmp-shell | 63 +++++++++++++++++++--------------------------------
>  1 file changed, 23 insertions(+), 40 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH for-2.11 v2 1/5] qmp-shell: Use optparse module

[Markus Armbruster]

Eduardo Habkost <ehabkost@redhat.com> writes:

> It makes command-line parsing and generation of help text much
> simpler.

There's really no excuse for parsing command line arguments by hand in
Python.

> The optparse module is deprecated since Python 2.7, but argparse
> is not available in Python 2.6 (the minimum Python version
> required for building QEMU).

We have a few uses of argparse in the tree.  Are they okay?

We also use getopt in places.  Perhaps we should pick one way to parse
command lines and stick to it.

>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Re: [Qemu-devel] [PATCH for-2.11 v2 1/5] qmp-shell: Use optparse module

[Daniel P. Berrange]

On Tue, Aug 15, 2017 at 11:47:28AM +0200, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > It makes command-line parsing and generation of help text much
> > simpler.
> 
> There's really no excuse for parsing command line arguments by hand in
> Python.
> 
> > The optparse module is deprecated since Python 2.7, but argparse
> > is not available in Python 2.6 (the minimum Python version
> > required for building QEMU).
> 
> We have a few uses of argparse in the tree.  Are they okay?
> 
> We also use getopt in places.  Perhaps we should pick one way to parse
> command lines and stick to it.

I just came up against this problem with keycodemapdb which I'm adding as
a submodule to qemu.  I used argparse there because it is the modern
recommended API for python >= 2.7.   I examined possibilty of using
optparse instead, but it lacks key features - in particular the idea
of 'subcommands' is entirely missing from optparse.

The 'argparse' module is part of python core, but is *also* available
as a standalone module that is implemented in terms of optparse.

So, I would suggest that we just copy the 'argparse' module into the
QEMU 'scripts/thirdparty' directory.

Then in any files which need argparse, you can do this

  try:
      import argparse
  except:
      import os, sys
      sys.path.append(os.path.join(os.path.dirname(__file__), "thirdparty"))
      import argparse

so that it tries to use the argparse provided by python, and falls back
to pulling in the one in our scripts/thirdparty directory.

When we finally bump our min python to 2.7, we can simply drop this
compat code for the import statement.  This avoids need for us to
re-write code to use a deprecated API, with a worse feature set.

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

Re: [Qemu-devel] [PATCH for-2.11 v2 1/5] qmp-shell: Use optparse module

[Daniel P. Berrange]

On Tue, Aug 08, 2017 at 05:39:31PM -0300, Eduardo Habkost wrote:
> It makes command-line parsing and generation of help text much
> simpler.
> 
> The optparse module is deprecated since Python 2.7, but argparse
> is not available in Python 2.6 (the minimum Python version
> required for building QEMU).
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Changes v1 -> v2:
> * Use optparse module, as the minimum Python version for building
>   QEMU is 2.6
>   * Reported-by: Stefan Hajnoczi <stefanha@gmail.com>
>   * Suggested-by: "Daniel P. Berrange" <berrange@redhat.com>

I no longer suggest this approach :-)

I'd prefer if we just bundled a copy of 'argparse.py' in the
local scripts/thirdparty directory, and add that to the
python import path, if the system python lacks argparse.

eg if we had $QEMU-GIT/scripts/thirdparty/argparse.py you can
use:

try:
    import argparse
except:
    import os, sys
    sys.path.append(os.path.join(os.path.dirname(__file__), "../thirdparty"))
    import argparse

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

Re: [Qemu-devel] [PATCH for-2.11 v2 4/5] qmp-shell: Accept QMP command as argument

[Markus Armbruster]

Eduardo Habkost <ehabkost@redhat.com> writes:

Suggest to insert here:

  If additional arguments QMP-COMMAND ARG=VAL... are given, run just
  that QMP command instead of the REPL.

Question: is this limited to simple arguments?  If no, how would I write
an object argument?  For instance, how would I do

    { "execute": "blockdev-add", "arguments": { "node-name": "foo", "driver": "nbd", "server": { "type": "inet", "host": "localhost", "port": "12345" } } }

?

> This is useful for testing QMP commands in scripts.
>
> Example usage, combined with 'jq' for filtering the results:
>
>   $ ./scripts/qmp/qmp-shell /tmp/qmp qom-list path=/ | jq -r .return[].name
>   machine
>   type
>   chardevs
>   backend

What's jq?

>   $

Let's drop this line.

>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Changes v1 -> v2:
> * Rewritten using optparse module
> ---
>  scripts/qmp/qmp-shell | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
> index 4b9a420..4b7374e 100755
> --- a/scripts/qmp/qmp-shell
> +++ b/scripts/qmp/qmp-shell
> @@ -400,7 +400,7 @@ def die(msg):
>  
>  def main():
>      parser = optparse.OptionParser(description='QMP shell utility')
> -    parser.set_usage("%prog [options] <UNIX socket path> | <TCP address:port>")
> +    parser.set_usage("%prog [options] <UNIX socket path> | <TCP address:port> [COMMAND [ARG=VALUE]...]")
>      parser.add_option('-v', action='store_true', dest='verbose',
>          help='Verbose (echo command sent and received)')
>      parser.add_option('-p', action='store_true', dest='pretty',
> @@ -411,10 +411,11 @@ def main():
>          default=True, help='Skip negotiate (for qemu-ga)')
>      opts,args = parser.parse_args()
>  
> -    if len(args) != 1:
> +    if len(args) < 1:
>          parser.print_help(sys.stderr)
>          sys.exit(1)
>      addr = args[0]
> +    cmdargs = args[1:]
>  
>      try:
>          if opts.hmp:
> @@ -433,10 +434,13 @@ def main():
>      except qemu.error:
>          die('Could not connect to %s' % addr)
>  
> -    qemu.show_banner()
>      qemu.set_verbosity(opts.verbose)
> -    while qemu.read_exec_command(qemu.get_prompt()):
> -        pass
> +    if len(cmdargs):

Superfluous len().

> +        qemu.execute_cmdargs(cmdargs)
> +    else:
> +        qemu.show_banner()
> +        while qemu.read_exec_command(qemu.get_prompt()):
> +            pass
>      qemu.close()
>  
>  if __name__ == '__main__':

Re: [Qemu-devel] [PATCH for-2.11 v2 1/5] qmp-shell: Use optparse module

[Eduardo Habkost]

On Tue, Aug 15, 2017 at 10:56:07AM +0100, Daniel P. Berrange wrote:
> On Tue, Aug 15, 2017 at 11:47:28AM +0200, Markus Armbruster wrote:
> > Eduardo Habkost <ehabkost@redhat.com> writes:
> > 
> > > It makes command-line parsing and generation of help text much
> > > simpler.
> > 
> > There's really no excuse for parsing command line arguments by hand in
> > Python.
> > 
> > > The optparse module is deprecated since Python 2.7, but argparse
> > > is not available in Python 2.6 (the minimum Python version
> > > required for building QEMU).
> > 
> > We have a few uses of argparse in the tree.  Are they okay?
> > 
> > We also use getopt in places.  Perhaps we should pick one way to parse
> > command lines and stick to it.
> 
> I just came up against this problem with keycodemapdb which I'm adding as
> a submodule to qemu.  I used argparse there because it is the modern
> recommended API for python >= 2.7.   I examined possibilty of using
> optparse instead, but it lacks key features - in particular the idea
> of 'subcommands' is entirely missing from optparse.
> 
> The 'argparse' module is part of python core, but is *also* available
> as a standalone module that is implemented in terms of optparse.
> 
> So, I would suggest that we just copy the 'argparse' module into the
> QEMU 'scripts/thirdparty' directory.
> 
> Then in any files which need argparse, you can do this
> 
>   try:
>       import argparse
>   except:
>       import os, sys
>       sys.path.append(os.path.join(os.path.dirname(__file__), "thirdparty"))
>       import argparse

What about:

  try:
      import argparse
  except:
      from thirdparty import argparse

(I think we could move all our Python modules [qemu.py, qmp.py]
to ./scripts/qemu/, so this would become "qemu.thirdparty").

> 
> so that it tries to use the argparse provided by python, and falls back
> to pulling in the one in our scripts/thirdparty directory.
> 
> When we finally bump our min python to 2.7, we can simply drop this
> compat code for the import statement.  This avoids need for us to
> re-write code to use a deprecated API, with a worse feature set.

Sounds good to me.

-- 
Eduardo

Re: [Qemu-devel] [PATCH for-2.11 v2 4/5] qmp-shell: Accept QMP command as argument

[Eduardo Habkost]

On Tue, Aug 15, 2017 at 12:03:53PM +0200, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> Suggest to insert here:
> 
>   If additional arguments QMP-COMMAND ARG=VAL... are given, run just
>   that QMP command instead of the REPL.
> 
> Question: is this limited to simple arguments?  If no, how would I write
> an object argument?  For instance, how would I do
> 
>     { "execute": "blockdev-add", "arguments": { "node-name": "foo", "driver": "nbd", "server": { "type": "inet", "host": "localhost", "port": "12345" } } }
> 
> ?

Exactly the same way you would write it when running qmp-shell in
interactive mode.  e.g.:

  $ ./scripts/qmp/qmp-shell /tmp/qmp blockdev-add driver=qcow2 node-name=node-E 'file={"driver":"file","filename":"/path/to/file.qcow2"}'


> 
> > This is useful for testing QMP commands in scripts.
> >
> > Example usage, combined with 'jq' for filtering the results:
> >
> >   $ ./scripts/qmp/qmp-shell /tmp/qmp qom-list path=/ | jq -r .return[].name
> >   machine
> >   type
> >   chardevs
> >   backend
> 
> What's jq?

https://stedolan.github.io/jq/

"like sed for JSON data"

> 
> >   $
> 
> Let's drop this line.

Will do it.

> 
> >
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> > Changes v1 -> v2:
> > * Rewritten using optparse module
> > ---
> >  scripts/qmp/qmp-shell | 14 +++++++++-----
> >  1 file changed, 9 insertions(+), 5 deletions(-)
> >
> > diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
> > index 4b9a420..4b7374e 100755
> > --- a/scripts/qmp/qmp-shell
> > +++ b/scripts/qmp/qmp-shell
> > @@ -400,7 +400,7 @@ def die(msg):
> >  
> >  def main():
> >      parser = optparse.OptionParser(description='QMP shell utility')
> > -    parser.set_usage("%prog [options] <UNIX socket path> | <TCP address:port>")
> > +    parser.set_usage("%prog [options] <UNIX socket path> | <TCP address:port> [COMMAND [ARG=VALUE]...]")
> >      parser.add_option('-v', action='store_true', dest='verbose',
> >          help='Verbose (echo command sent and received)')
> >      parser.add_option('-p', action='store_true', dest='pretty',
> > @@ -411,10 +411,11 @@ def main():
> >          default=True, help='Skip negotiate (for qemu-ga)')
> >      opts,args = parser.parse_args()
> >  
> > -    if len(args) != 1:
> > +    if len(args) < 1:
> >          parser.print_help(sys.stderr)
> >          sys.exit(1)
> >      addr = args[0]
> > +    cmdargs = args[1:]
> >  
> >      try:
> >          if opts.hmp:
> > @@ -433,10 +434,13 @@ def main():
> >      except qemu.error:
> >          die('Could not connect to %s' % addr)
> >  
> > -    qemu.show_banner()
> >      qemu.set_verbosity(opts.verbose)
> > -    while qemu.read_exec_command(qemu.get_prompt()):
> > -        pass
> > +    if len(cmdargs):
> 
> Superfluous len().

Will fix it in the next version.

> 
> > +        qemu.execute_cmdargs(cmdargs)
> > +    else:
> > +        qemu.show_banner()
> > +        while qemu.read_exec_command(qemu.get_prompt()):
> > +            pass
> >      qemu.close()
> >  
> >  if __name__ == '__main__':

-- 
Eduardo

Re: [Qemu-devel] [PATCH for-2.11 v2 4/5] qmp-shell: Accept QMP command as argument

[Markus Armbruster]

Eduardo Habkost <ehabkost@redhat.com> writes:

> On Tue, Aug 15, 2017 at 12:03:53PM +0200, Markus Armbruster wrote:
>> Eduardo Habkost <ehabkost@redhat.com> writes:
>> 
>> Suggest to insert here:
>> 
>>   If additional arguments QMP-COMMAND ARG=VAL... are given, run just
>>   that QMP command instead of the REPL.
>> 
>> Question: is this limited to simple arguments?  If no, how would I write
>> an object argument?  For instance, how would I do
>> 
>>     { "execute": "blockdev-add", "arguments": { "node-name": "foo", "driver": "nbd", "server": { "type": "inet", "host": "localhost", "port": "12345" } } }
>> 
>> ?
>
> Exactly the same way you would write it when running qmp-shell in
> interactive mode.  e.g.:
>
>   $ ./scripts/qmp/qmp-shell /tmp/qmp blockdev-add driver=qcow2 node-name=node-E 'file={"driver":"file","filename":"/path/to/file.qcow2"}'

I see.

The QEMU command line uses dotted key syntax instead.

To be honest, the less qmp-shell is used, the happier I am.  Would you
like to serve as its sub-maintainer?

>> > This is useful for testing QMP commands in scripts.
>> >
>> > Example usage, combined with 'jq' for filtering the results:
>> >
>> >   $ ./scripts/qmp/qmp-shell /tmp/qmp qom-list path=/ | jq -r .return[].name
>> >   machine
>> >   type
>> >   chardevs
>> >   backend
>> 
>> What's jq?
>
> https://stedolan.github.io/jq/
>
> "like sed for JSON data"

A fine addition to your commit message.

>> 
>> >   $
>> 
>> Let's drop this line.
>
> Will do it.
>
>> 
>> >
>> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
[...]

Re: [Qemu-devel] [PATCH for-2.11 v2 1/5] qmp-shell: Use optparse module

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 2415 bytes --]

On Tue, Aug 15, 2017 at 05:06:46PM -0300, Eduardo Habkost wrote:
> On Tue, Aug 15, 2017 at 10:56:07AM +0100, Daniel P. Berrange wrote:
> > On Tue, Aug 15, 2017 at 11:47:28AM +0200, Markus Armbruster wrote:
> > > Eduardo Habkost <ehabkost@redhat.com> writes:
> > > 
> > > > It makes command-line parsing and generation of help text much
> > > > simpler.
> > > 
> > > There's really no excuse for parsing command line arguments by hand in
> > > Python.
> > > 
> > > > The optparse module is deprecated since Python 2.7, but argparse
> > > > is not available in Python 2.6 (the minimum Python version
> > > > required for building QEMU).
> > > 
> > > We have a few uses of argparse in the tree.  Are they okay?
> > > 
> > > We also use getopt in places.  Perhaps we should pick one way to parse
> > > command lines and stick to it.
> > 
> > I just came up against this problem with keycodemapdb which I'm adding as
> > a submodule to qemu.  I used argparse there because it is the modern
> > recommended API for python >= 2.7.   I examined possibilty of using
> > optparse instead, but it lacks key features - in particular the idea
> > of 'subcommands' is entirely missing from optparse.
> > 
> > The 'argparse' module is part of python core, but is *also* available
> > as a standalone module that is implemented in terms of optparse.
> > 
> > So, I would suggest that we just copy the 'argparse' module into the
> > QEMU 'scripts/thirdparty' directory.
> > 
> > Then in any files which need argparse, you can do this
> > 
> >   try:
> >       import argparse
> >   except:
> >       import os, sys
> >       sys.path.append(os.path.join(os.path.dirname(__file__), "thirdparty"))
> >       import argparse
> 
> What about:
> 
>   try:
>       import argparse
>   except:
>       from thirdparty import argparse
> 
> (I think we could move all our Python modules [qemu.py, qmp.py]
> to ./scripts/qemu/, so this would become "qemu.thirdparty").
> 
> > 
> > so that it tries to use the argparse provided by python, and falls back
> > to pulling in the one in our scripts/thirdparty directory.
> > 
> > When we finally bump our min python to 2.7, we can simply drop this
> > compat code for the import statement.  This avoids need for us to
> > re-write code to use a deprecated API, with a worse feature set.
> 
> Sounds good to me.

Sounds good.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH for-2.11 v2 4/5] qmp-shell: Accept QMP command as argument

[Eduardo Habkost]

On Wed, Aug 16, 2017 at 08:25:41AM +0200, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > On Tue, Aug 15, 2017 at 12:03:53PM +0200, Markus Armbruster wrote:
> >> Eduardo Habkost <ehabkost@redhat.com> writes:
> >> 
> >> Suggest to insert here:
> >> 
> >>   If additional arguments QMP-COMMAND ARG=VAL... are given, run just
> >>   that QMP command instead of the REPL.
> >> 
> >> Question: is this limited to simple arguments?  If no, how would I write
> >> an object argument?  For instance, how would I do
> >> 
> >>     { "execute": "blockdev-add", "arguments": { "node-name": "foo", "driver": "nbd", "server": { "type": "inet", "host": "localhost", "port": "12345" } } }
> >> 
> >> ?
> >
> > Exactly the same way you would write it when running qmp-shell in
> > interactive mode.  e.g.:
> >
> >   $ ./scripts/qmp/qmp-shell /tmp/qmp blockdev-add driver=qcow2 node-name=node-E 'file={"driver":"file","filename":"/path/to/file.qcow2"}'
> 
> I see.
> 
> The QEMU command line uses dotted key syntax instead.

You mean -blockdev, or is there another way to send QMP commands
to QEMU that I'm not aware of?

> 
> To be honest, the less qmp-shell is used, the happier I am.

What people would use instead of it?

>                                                              Would you
> like to serve as its sub-maintainer?

I'd be glad to.

-- 
Eduardo

Re: [Qemu-devel] [PATCH for-2.11 v2 4/5] qmp-shell: Accept QMP command as argument

[Markus Armbruster]

Eduardo Habkost <ehabkost@redhat.com> writes:

> On Wed, Aug 16, 2017 at 08:25:41AM +0200, Markus Armbruster wrote:
>> Eduardo Habkost <ehabkost@redhat.com> writes:
>> 
>> > On Tue, Aug 15, 2017 at 12:03:53PM +0200, Markus Armbruster wrote:
>> >> Eduardo Habkost <ehabkost@redhat.com> writes:
>> >> 
>> >> Suggest to insert here:
>> >> 
>> >>   If additional arguments QMP-COMMAND ARG=VAL... are given, run just
>> >>   that QMP command instead of the REPL.
>> >> 
>> >> Question: is this limited to simple arguments?  If no, how would I write
>> >> an object argument?  For instance, how would I do
>> >> 
>> >>     { "execute": "blockdev-add", "arguments": { "node-name": "foo", "driver": "nbd", "server": { "type": "inet", "host": "localhost", "port": "12345" } } }
>> >> 
>> >> ?
>> >
>> > Exactly the same way you would write it when running qmp-shell in
>> > interactive mode.  e.g.:
>> >
>> >   $ ./scripts/qmp/qmp-shell /tmp/qmp blockdev-add driver=qcow2 node-name=node-E 'file={"driver":"file","filename":"/path/to/file.qcow2"}'
>> 
>> I see.
>> 
>> The QEMU command line uses dotted key syntax instead.
>
> You mean -blockdev, or is there another way to send QMP commands
> to QEMU that I'm not aware of?

Dotted keys were invented for the block layer's -drive.  I adopted them
for -blockdev after considerable discussion.  The -blockdev code is all
new, and it even has documentation and tests.

>> To be honest, the less qmp-shell is used, the happier I am.
>
> What people would use instead of it?

The syntactic sugar qmp-shell provides over plain QMP doesn't simplify
its use all that much, and it can get in the way.

For my interactive QMP needs, I use

    $ socat UNIX:/wherever/qmp-socket READLINE,history=$HOME/.qmp_history,prompt='QMP> '

In programs, I send or receive either plain text or JSON objects,
depending on the program's needs.  Libqtest supports the latter.

>>                                                              Would you
>> like to serve as its sub-maintainer?
>
> I'd be glad to.

Great!  Please post a suitable MAINTAINERS stanza.