[Qemu-devel] [PATCH v2 0/4] scripts: qmp-shell: add transaction support

[Qemu-devel] [PATCH v2 0/4] scripts: qmp-shell: add transaction support

[John Snow]

The qmp-shell is a little rudimentary, but it can be hacked
to give us some transactional support without too much difficulty.

(1) Prep.
(2) Add support for serializing json arrays
(3) Allow users to use 'single quotes' instead of "double quotes"
(4) Add a special transaction( ... ) syntax that lets users
    build up transactional commands using the existing qmp shell
    syntax to define each action.
(5) Add a verbose flag to display generated QMP commands.

The parsing is not as robust as one would like, but this suffices
without adding a proper parser.

Design considerations:

(1) Try not to disrupt the existing design of the qmp-shell. The existing
    API is not disturbed.

(2) Pick a "magic token" such that it could not be confused for legitimate
    QMP/JSON syntax. Parentheses are used for this purpose.

===
v2:
===

 - Squash patches 2 & 3:
  - Remove wholesale replacement of single quotes, in favor of try blocks
    that attempt to parse as pure JSON, then as Python.
  - Factored out the value parser block to accomplish the above.
  - Allow both true/True and false/False for values.
 - Fix typo in patch 3 cover letter. (was patch 4.)

John Snow (4):
  scripts: qmp-shell: refactor helpers
  scripts: qmp-shell: Expand support for QMP expressions
  scripts: qmp-shell: add transaction subshell
  scripts: qmp-shell: Add verbose flag

 scripts/qmp/qmp-shell | 125 ++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 95 insertions(+), 30 deletions(-)

-- 
2.1.0

[Qemu-devel] [PATCH v2 1/4] scripts: qmp-shell: refactor helpers

[John Snow]

Refactor the qmp-shell command line processing function
into two components. This will be used to allow sub-expressions,
which will assist us in adding transactional support to qmp-shell.

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

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index e0e848b..a9632ec 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -88,16 +88,8 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         # clearing everything as it doesn't seem to matter
         readline.set_completer_delims('')
 
-    def __build_cmd(self, cmdline):
-        """
-        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()
-        qmpcmd = { 'execute': cmdargs[0], 'arguments': {} }
-        for arg in cmdargs[1:]:
+    def __cli_expr(self, tokens, parent):
+        for arg in tokens:
             opt = arg.split('=')
             try:
                 if(len(opt) > 2):
@@ -113,7 +105,6 @@ class QMPShell(qmp.QEMUMonitorProtocol):
                 else:
                     value = opt[1]
             optpath = opt[0].split('.')
-            parent = qmpcmd['arguments']
             curpath = []
             for p in optpath[:-1]:
                 curpath.append(p)
@@ -128,6 +119,17 @@ class QMPShell(qmp.QEMUMonitorProtocol):
                 else:
                     raise QMPShellError('Cannot set "%s" multiple times' % opt[0])
             parent[optpath[-1]] = value
+
+    def __build_cmd(self, cmdline):
+        """
+        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()
+        qmpcmd = { 'execute': cmdargs[0], 'arguments': {} }
+        self.__cli_expr(cmdargs[1:], qmpcmd['arguments'])
         return qmpcmd
 
     def _execute_cmd(self, cmdline):
-- 
2.1.0

[Qemu-devel] [PATCH v2 2/4] scripts: qmp-shell: Expand support for QMP expressions

[John Snow]

This includes support for [] expressions, single-quotes in
QMP expressions (which is not strictly a part of JSON), and
the ability to use "True" or "False" literals instead of
JSON's lowercased 'true' and 'false' literals.

qmp-shell currently allows you to describe values as
JSON expressions:
key={"key":{"key2":"val"}}

But it does not currently support arrays, which are needed
for serializing and deserializing transactions:
key=[{"type":"drive-backup","data":{...}}]

qmp-shell also only currently accepts doubly quoted strings
as-per JSON spec, but QMP allows single quotes.

Lastly, python allows you to utilize "True" or "False" as
boolean literals, but JSON expects "true" or "false". Expand
qmp-shell to allow the user to type either, converting to the
correct type.

As a consequence of the above, the key=val parsing is also improved
to give better error messages if a key=val token is not provided.

CAVEAT: The parser is still extremely rudimentary and does not
expect to find spaces in {} nor [] expressions. This patch does
not improve this functionality.

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

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index a9632ec..4cdcb6c 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -88,23 +88,35 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         # clearing everything as it doesn't seem to matter
         readline.set_completer_delims('')
 
+    def __parse_value(self, val):
+        try:
+            return int(val)
+        except ValueError:
+            pass
+
+        if val.lower() == 'true':
+            return True
+        if val.lower() == 'false':
+            return False
+        if val.startswith(('{', '[')):
+            try:
+                return json.loads(val)
+            except ValueError:
+                pass
+            try:
+                return ast.literal_eval(val)
+            except SyntaxError:
+                pass
+        return val
+
     def __cli_expr(self, tokens, parent):
         for arg in tokens:
-            opt = arg.split('=')
-            try:
-                if(len(opt) > 2):
-                    opt[1] = '='.join(opt[1:])
-                value = int(opt[1])
-            except ValueError:
-                if opt[1] == 'true':
-                    value = True
-                elif opt[1] == 'false':
-                    value = False
-                elif opt[1].startswith('{'):
-                    value = json.loads(opt[1])
-                else:
-                    value = opt[1]
-            optpath = opt[0].split('.')
+            (key, _, val) = arg.partition('=')
+            if not val:
+                raise QMPShellError("Expected a key=value pair, got '%s'" % arg)
+
+            value = __parse_value(val)
+            optpath = key.split('.')
             curpath = []
             for p in optpath[:-1]:
                 curpath.append(p)
@@ -117,7 +129,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
                 if type(parent[optpath[-1]]) is dict:
                     raise QMPShellError('Cannot use "%s" as both leaf and non-leaf key' % '.'.join(curpath))
                 else:
-                    raise QMPShellError('Cannot set "%s" multiple times' % opt[0])
+                    raise QMPShellError('Cannot set "%s" multiple times' % key)
             parent[optpath[-1]] = value
 
     def __build_cmd(self, cmdline):
-- 
2.1.0

[Qemu-devel] [PATCH v2 3/4] scripts: qmp-shell: add transaction subshell

[John Snow]

Add a special processing mode to craft transactions.

By entering "transaction(" the shell will enter a special
mode where each subsequent command will be saved as a transaction
instead of executed as an individual command.

The transaction can be submitted by entering ")" on a line by itself.

Examples:

Separate lines:

(QEMU) transaction(
TRANS> block-dirty-bitmap-add node=drive0 name=bitmap1
TRANS> block-dirty-bitmap-clear node=drive0 name=bitmap0
TRANS> )

With a transaction action included on the first line:

(QEMU) transaction( block-dirty-bitmap-add node=drive0 name=bitmap2
TRANS> block-dirty-bitmap-add node=drive0 name=bitmap3
TRANS> )

As a one-liner, with just one transaction action:

(QEMU) transaction( block-dirty-bitmap-add node=drive0 name=bitmap0 )

As a side-effect of this patch, blank lines are now parsed as no-ops,
regardless of which shell mode you are in.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 scripts/qmp/qmp-shell | 42 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 4cdcb6c..3fffe15 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -59,6 +59,8 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         self._greeting = None
         self._completer = None
         self._pp = pp
+        self._transmode = False
+        self._actions = list()
 
     def __get_address(self, arg):
         """
@@ -140,6 +142,36 @@ class QMPShell(qmp.QEMUMonitorProtocol):
             < command-name > [ arg-name1=arg1 ] ... [ arg-nameN=argN ]
         """
         cmdargs = cmdline.split()
+
+        # Transactional CLI entry/exit:
+        if cmdargs[0] == 'transaction(':
+            self._transmode = True
+            cmdargs.pop(0)
+        elif cmdargs[0] == ')' and self._transmode:
+            self._transmode = False
+            if cmdargs:
+                raise QMPShellError("Unexpected input after close of Transaction sub-shell")
+            qmpcmd = { 'execute': 'transaction',
+                       'arguments': { 'actions': self._actions } }
+            self._actions = list()
+            return qmpcmd
+
+        # Nothing to process?
+        if not cmdargs:
+            return None
+
+        # Parse and then cache this Transactional Action
+        if self._transmode:
+            finalize = False
+            action = { 'type': cmdargs[0], 'data': {} }
+            if cmdargs[-1] == ')':
+                cmdargs.pop(-1)
+                finalize = True
+            self.__cli_expr(cmdargs[1:], action['data'])
+            self._actions.append(action)
+            return self.__build_cmd(')') if finalize else None
+
+        # Standard command: parse and return it to be executed.
         qmpcmd = { 'execute': cmdargs[0], 'arguments': {} }
         self.__cli_expr(cmdargs[1:], qmpcmd['arguments'])
         return qmpcmd
@@ -152,6 +184,9 @@ class QMPShell(qmp.QEMUMonitorProtocol):
             print 'command format: <command-name> ',
             print '[arg-name1=arg1] ... [arg-nameN=argN]'
             return True
+        # For transaction mode, we may have just cached the action:
+        if qmpcmd is None:
+            return True
         resp = self.cmd_obj(qmpcmd)
         if resp is None:
             print 'Disconnected'
@@ -172,6 +207,11 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         version = self._greeting['QMP']['version']['qemu']
         print 'Connected to QEMU %d.%d.%d\n' % (version['major'],version['minor'],version['micro'])
 
+    def get_prompt(self):
+        if self._transmode:
+            return "TRANS> "
+        return "(QEMU) "
+
     def read_exec_command(self, prompt):
         """
         Read and execute a command.
@@ -311,7 +351,7 @@ def main():
         die('Could not connect to %s' % addr)
 
     qemu.show_banner()
-    while qemu.read_exec_command('(QEMU) '):
+    while qemu.read_exec_command(qemu.get_prompt()):
         pass
     qemu.close()
 
-- 
2.1.0

[Qemu-devel] [PATCH v2 4/4] scripts: qmp-shell: Add verbose flag

[John Snow]

Add a verbose flag that shows the QMP command that was
constructed, to allow for later copy/pasting, reference,
debugging, etc.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 scripts/qmp/qmp-shell | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 3fffe15..213f515 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -176,6 +176,12 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         self.__cli_expr(cmdargs[1:], qmpcmd['arguments'])
         return qmpcmd
 
+    def _print(self, qmp):
+        if self._pp is not None:
+            self._pp.pprint(qmp)
+        else:
+            print qmp
+
     def _execute_cmd(self, cmdline):
         try:
             qmpcmd = self.__build_cmd(cmdline)
@@ -187,15 +193,13 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         # For transaction mode, we may have just cached the action:
         if qmpcmd is None:
             return True
+        if self._verbose:
+            self._print(qmpcmd)
         resp = self.cmd_obj(qmpcmd)
         if resp is None:
             print 'Disconnected'
             return False
-
-        if self._pp is not None:
-            self._pp.pprint(resp)
-        else:
-            print resp
+        self._print(resp)
         return True
 
     def connect(self):
@@ -231,6 +235,9 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         else:
             return self._execute_cmd(cmdline)
 
+    def set_verbosity(self, verbose):
+        self._verbose = verbose
+
 class HMPShell(QMPShell):
     def __init__(self, address):
         QMPShell.__init__(self, address)
@@ -316,6 +323,7 @@ def main():
     qemu = None
     hmp = False
     pp = None
+    verbose = False
 
     try:
         for arg in sys.argv[1:]:
@@ -327,6 +335,8 @@ def main():
                 if pp is not None:
                     fail_cmdline(arg)
                 pp = pprint.PrettyPrinter(indent=4)
+            elif arg == "-v":
+                verbose = True
             else:
                 if qemu is not None:
                     fail_cmdline(arg)
@@ -351,6 +361,7 @@ def main():
         die('Could not connect to %s' % addr)
 
     qemu.show_banner()
+    qemu.set_verbosity(verbose)
     while qemu.read_exec_command(qemu.get_prompt()):
         pass
     qemu.close()
-- 
2.1.0

Re: [Qemu-devel] [PATCH v2 2/4] scripts: qmp-shell: Expand support for QMP expressions

[John Snow]

On 04/22/2015 12:21 PM, John Snow wrote:
> This includes support for [] expressions, single-quotes in
> QMP expressions (which is not strictly a part of JSON), and
> the ability to use "True" or "False" literals instead of
> JSON's lowercased 'true' and 'false' literals.
>
> qmp-shell currently allows you to describe values as
> JSON expressions:
> key={"key":{"key2":"val"}}
>
> But it does not currently support arrays, which are needed
> for serializing and deserializing transactions:
> key=[{"type":"drive-backup","data":{...}}]
>
> qmp-shell also only currently accepts doubly quoted strings
> as-per JSON spec, but QMP allows single quotes.
>
> Lastly, python allows you to utilize "True" or "False" as
> boolean literals, but JSON expects "true" or "false". Expand
> qmp-shell to allow the user to type either, converting to the
> correct type.
>
> As a consequence of the above, the key=val parsing is also improved
> to give better error messages if a key=val token is not provided.
>
> CAVEAT: The parser is still extremely rudimentary and does not
> expect to find spaces in {} nor [] expressions. This patch does
> not improve this functionality.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   scripts/qmp/qmp-shell | 44 ++++++++++++++++++++++++++++----------------
>   1 file changed, 28 insertions(+), 16 deletions(-)
>

Left a line dangling in my local tree by accident:

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 4cdcb6c..e7b40eb 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -32,6 +32,7 @@

  import qmp
  import json
+import ast
  import readline
  import sys
  import pprint


> diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
> index a9632ec..4cdcb6c 100755
> --- a/scripts/qmp/qmp-shell
> +++ b/scripts/qmp/qmp-shell
> @@ -88,23 +88,35 @@ class QMPShell(qmp.QEMUMonitorProtocol):
>           # clearing everything as it doesn't seem to matter
>           readline.set_completer_delims('')
>
> +    def __parse_value(self, val):
> +        try:
> +            return int(val)
> +        except ValueError:
> +            pass
> +
> +        if val.lower() == 'true':
> +            return True
> +        if val.lower() == 'false':
> +            return False
> +        if val.startswith(('{', '[')):
> +            try:
> +                return json.loads(val)
> +            except ValueError:
> +                pass
> +            try:
> +                return ast.literal_eval(val)
> +            except SyntaxError:
> +                pass
> +        return val
> +
>       def __cli_expr(self, tokens, parent):
>           for arg in tokens:
> -            opt = arg.split('=')
> -            try:
> -                if(len(opt) > 2):
> -                    opt[1] = '='.join(opt[1:])
> -                value = int(opt[1])
> -            except ValueError:
> -                if opt[1] == 'true':
> -                    value = True
> -                elif opt[1] == 'false':
> -                    value = False
> -                elif opt[1].startswith('{'):
> -                    value = json.loads(opt[1])
> -                else:
> -                    value = opt[1]
> -            optpath = opt[0].split('.')
> +            (key, _, val) = arg.partition('=')
> +            if not val:
> +                raise QMPShellError("Expected a key=value pair, got '%s'" % arg)
> +
> +            value = __parse_value(val)
> +            optpath = key.split('.')
>               curpath = []
>               for p in optpath[:-1]:
>                   curpath.append(p)
> @@ -117,7 +129,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
>                   if type(parent[optpath[-1]]) is dict:
>                       raise QMPShellError('Cannot use "%s" as both leaf and non-leaf key' % '.'.join(curpath))
>                   else:
> -                    raise QMPShellError('Cannot set "%s" multiple times' % opt[0])
> +                    raise QMPShellError('Cannot set "%s" multiple times' % key)
>               parent[optpath[-1]] = value
>
>       def __build_cmd(self, cmdline):
>

Re: [Qemu-devel] [PATCH v2 2/4] scripts: qmp-shell: Expand support for QMP expressions

[Eric Blake]

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

On 04/22/2015 10:21 AM, John Snow wrote:
> This includes support for [] expressions, single-quotes in
> QMP expressions (which is not strictly a part of JSON), and
> the ability to use "True" or "False" literals instead of
> JSON's lowercased 'true' and 'false' literals.
> 
> qmp-shell currently allows you to describe values as
> JSON expressions:
> key={"key":{"key2":"val"}}
> 
> But it does not currently support arrays, which are needed
> for serializing and deserializing transactions:
> key=[{"type":"drive-backup","data":{...}}]
> 
> qmp-shell also only currently accepts doubly quoted strings
> as-per JSON spec, but QMP allows single quotes.
> 
> Lastly, python allows you to utilize "True" or "False" as
> boolean literals, but JSON expects "true" or "false". Expand
> qmp-shell to allow the user to type either, converting to the
> correct type.

Well, when easy to do.  See below for more discussion...

> 
> As a consequence of the above, the key=val parsing is also improved
> to give better error messages if a key=val token is not provided.
> 
> CAVEAT: The parser is still extremely rudimentary and does not
> expect to find spaces in {} nor [] expressions. This patch does
> not improve this functionality.

If someone cares about this, they can fix it later.  As I said in the v1
review, qmp-shell is mostly for debugging and testsuites, and we can
live with the caveats for now.

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

As far as I can tell, this makes qmp-shell strictly more useful, so I
have no qualms giving:

Reviewed-by: Eric Blake <eblake@redhat.com>

> 
> diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
> index a9632ec..4cdcb6c 100755
> --- a/scripts/qmp/qmp-shell
> +++ b/scripts/qmp/qmp-shell
> @@ -88,23 +88,35 @@ class QMPShell(qmp.QEMUMonitorProtocol):
>          # clearing everything as it doesn't seem to matter
>          readline.set_completer_delims('')
>  
> +    def __parse_value(self, val):
> +        try:
> +            return int(val)
> +        except ValueError:
> +            pass
> +
> +        if val.lower() == 'true':
> +            return True
> +        if val.lower() == 'false':
> +            return False

...as promised earlier,

This allows param=tRuE (neither JSON nor Python), but that's a minor
price to pay to get both param=true (param=JSON style) and param=True
(param=Python style) to work.  Meanwhile...

> +        if val.startswith(('{', '[')):
> +            try:
> +                return json.loads(val)

This statement accepts param=JSON style, as in:
 param={ "foo":true }
but it rejects (as invalid JSON):
 param={ 'foo':true }
 param={ "foo":True }
 param={ 'foo':True }

> +            except ValueError:
> +                pass
> +            try:
> +                return ast.literal_eval(val)

And this statement accepts param=Python style, as in:
 param={ "foo":True }
 param={ 'foo':True }
but it rejects (as invalid Python):
 param={ "foo":true }
 param={ 'foo':true }

Of those four combinations, QMP accepts:
 param={ "foo":true }
 param={ 'foo':true }
while rejecting:
 param={ "foo":True }
 param={ 'foo':True }

So among the four combinations of single/double quoting vs upper/lower
casing of boolean literals, qmp-shell now accepts three variants, but we
have a case that QMP accepts but qmp-shell rejects (single quotes mixed
with lower-case true).

Not the end of the world (again, if someone wants all four cases to work
in qmp-shell and/or QMP proper, they can provide a patch later to
further enhance things).  But maybe worth expanding that CAVEAT in the
commit message to acknowledge the issues.

At any rate, doesn't affect my R-b given above.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH v2 2/4] scripts: qmp-shell: Expand support for QMP expressions

[Eric Blake]

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

On 04/22/2015 11:18 AM, Eric Blake wrote:
> On 04/22/2015 10:21 AM, John Snow wrote:
>> This includes support for [] expressions, single-quotes in
>> QMP expressions (which is not strictly a part of JSON), and
>> the ability to use "True" or "False" literals instead of
>> JSON's lowercased 'true' and 'false' literals.
>>

>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>  scripts/qmp/qmp-shell | 44 ++++++++++++++++++++++++++++----------------
>>  1 file changed, 28 insertions(+), 16 deletions(-)
> 
> As far as I can tell, this makes qmp-shell strictly more useful, so I
> have no qualms giving:
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>


When combined with your one-line touchup, of course.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH v2 2/4] scripts: qmp-shell: Expand support for QMP expressions

[John Snow]

On 04/22/2015 01:18 PM, Eric Blake wrote:
> On 04/22/2015 10:21 AM, John Snow wrote:
>> This includes support for [] expressions, single-quotes in
>> QMP expressions (which is not strictly a part of JSON), and
>> the ability to use "True" or "False" literals instead of
>> JSON's lowercased 'true' and 'false' literals.
>>
>> qmp-shell currently allows you to describe values as
>> JSON expressions:
>> key={"key":{"key2":"val"}}
>>
>> But it does not currently support arrays, which are needed
>> for serializing and deserializing transactions:
>> key=[{"type":"drive-backup","data":{...}}]
>>
>> qmp-shell also only currently accepts doubly quoted strings
>> as-per JSON spec, but QMP allows single quotes.
>>
>> Lastly, python allows you to utilize "True" or "False" as
>> boolean literals, but JSON expects "true" or "false". Expand
>> qmp-shell to allow the user to type either, converting to the
>> correct type.
>
> Well, when easy to do.  See below for more discussion...
>
>>
>> As a consequence of the above, the key=val parsing is also improved
>> to give better error messages if a key=val token is not provided.
>>
>> CAVEAT: The parser is still extremely rudimentary and does not
>> expect to find spaces in {} nor [] expressions. This patch does
>> not improve this functionality.
>
> If someone cares about this, they can fix it later.  As I said in the v1
> review, qmp-shell is mostly for debugging and testsuites, and we can
> live with the caveats for now.
>
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>   scripts/qmp/qmp-shell | 44 ++++++++++++++++++++++++++++----------------
>>   1 file changed, 28 insertions(+), 16 deletions(-)
>
> As far as I can tell, this makes qmp-shell strictly more useful, so I
> have no qualms giving:
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
>>
>> diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
>> index a9632ec..4cdcb6c 100755
>> --- a/scripts/qmp/qmp-shell
>> +++ b/scripts/qmp/qmp-shell
>> @@ -88,23 +88,35 @@ class QMPShell(qmp.QEMUMonitorProtocol):
>>           # clearing everything as it doesn't seem to matter
>>           readline.set_completer_delims('')
>>
>> +    def __parse_value(self, val):
>> +        try:
>> +            return int(val)
>> +        except ValueError:
>> +            pass
>> +
>> +        if val.lower() == 'true':
>> +            return True
>> +        if val.lower() == 'false':
>> +            return False
>
> ...as promised earlier,
>
> This allows param=tRuE (neither JSON nor Python), but that's a minor
> price to pay to get both param=true (param=JSON style) and param=True
> (param=Python style) to work.  Meanwhile...
>
>> +        if val.startswith(('{', '[')):
>> +            try:
>> +                return json.loads(val)
>
> This statement accepts param=JSON style, as in:
>   param={ "foo":true }
> but it rejects (as invalid JSON):
>   param={ 'foo':true }
>   param={ "foo":True }
>   param={ 'foo':True }
>
>> +            except ValueError:
>> +                pass
>> +            try:
>> +                return ast.literal_eval(val)
>
> And this statement accepts param=Python style, as in:
>   param={ "foo":True }
>   param={ 'foo':True }
> but it rejects (as invalid Python):
>   param={ "foo":true }
>   param={ 'foo':true }
>
> Of those four combinations, QMP accepts:
>   param={ "foo":true }
>   param={ 'foo':true }
> while rejecting:
>   param={ "foo":True }
>   param={ 'foo':True }
>
> So among the four combinations of single/double quoting vs upper/lower
> casing of boolean literals, qmp-shell now accepts three variants, but we
> have a case that QMP accepts but qmp-shell rejects (single quotes mixed
> with lower-case true).
>
> Not the end of the world (again, if someone wants all four cases to work
> in qmp-shell and/or QMP proper, they can provide a patch later to
> further enhance things).  But maybe worth expanding that CAVEAT in the
> commit message to acknowledge the issues.
>
> At any rate, doesn't affect my R-b given above.
>

Yes, unfortunately the object literals still must comply with some sort 
of standard, neither of which is truly QMP: They must either be Python 
or JSON.

key=val pairs can/will accept true/TRUE/tRuE etc as well, which I think 
is probably acceptable since that falls within the realm of "qmp-shell 
syntax" which doesn't necessarily have to maintain similarities to 
JSON/QMP/etc. I could probably go back and adjust it to just lowercase 
the first letter, but this is probably only ever going to be a typo 
(which we can unambiguously resolve) and the generated QMP (if -v is 
present) will of course appear correctly formatted.

GIQO: Garbage in, QMP out? :)

I will author an additional patch as an add-on a bit later that 
documents all the quirks to help improve the usability of this little tool.

Probably I'll also update the QMP banner to display a quick help message.

Thank you,
--John

Re: [Qemu-devel] [PATCH v2 0/4] scripts: qmp-shell: add transaction support

[John Snow]

On 04/22/2015 12:21 PM, John Snow wrote:
> The qmp-shell is a little rudimentary, but it can be hacked
> to give us some transactional support without too much difficulty.
>
> (1) Prep.
> (2) Add support for serializing json arrays
> (3) Allow users to use 'single quotes' instead of "double quotes"
> (4) Add a special transaction( ... ) syntax that lets users
>      build up transactional commands using the existing qmp shell
>      syntax to define each action.
> (5) Add a verbose flag to display generated QMP commands.
>
> The parsing is not as robust as one would like, but this suffices
> without adding a proper parser.
>
> Design considerations:
>
> (1) Try not to disrupt the existing design of the qmp-shell. The existing
>      API is not disturbed.
>
> (2) Pick a "magic token" such that it could not be confused for legitimate
>      QMP/JSON syntax. Parentheses are used for this purpose.
>
> ===
> v2:
> ===
>
>   - Squash patches 2 & 3:
>    - Remove wholesale replacement of single quotes, in favor of try blocks
>      that attempt to parse as pure JSON, then as Python.
>    - Factored out the value parser block to accomplish the above.
>    - Allow both true/True and false/False for values.
>   - Fix typo in patch 3 cover letter. (was patch 4.)
>
> John Snow (4):
>    scripts: qmp-shell: refactor helpers
>    scripts: qmp-shell: Expand support for QMP expressions
>    scripts: qmp-shell: add transaction subshell
>    scripts: qmp-shell: Add verbose flag
>
>   scripts/qmp/qmp-shell | 125 ++++++++++++++++++++++++++++++++++++++------------
>   1 file changed, 95 insertions(+), 30 deletions(-)
>

Version "2+" with the hotfix added, with R-B's added:
https://github.com/jnsnow/qemu/commits/qmp-shell-plus

To whomever would pull this into their tree: Let me know if you prefer I 
send a "v3" instead.

Thanks,
--John

Re: [Qemu-devel] [PATCH v2 0/4] scripts: qmp-shell: add transaction support

[Luiz Capitulino]

On Wed, 22 Apr 2015 15:27:17 -0400
John Snow <jsnow@redhat.com> wrote:

> 
> 
> On 04/22/2015 12:21 PM, John Snow wrote:
> > The qmp-shell is a little rudimentary, but it can be hacked
> > to give us some transactional support without too much difficulty.
> >
> > (1) Prep.
> > (2) Add support for serializing json arrays
> > (3) Allow users to use 'single quotes' instead of "double quotes"
> > (4) Add a special transaction( ... ) syntax that lets users
> >      build up transactional commands using the existing qmp shell
> >      syntax to define each action.
> > (5) Add a verbose flag to display generated QMP commands.
> >
> > The parsing is not as robust as one would like, but this suffices
> > without adding a proper parser.
> >
> > Design considerations:
> >
> > (1) Try not to disrupt the existing design of the qmp-shell. The existing
> >      API is not disturbed.
> >
> > (2) Pick a "magic token" such that it could not be confused for legitimate
> >      QMP/JSON syntax. Parentheses are used for this purpose.
> >
> > ===
> > v2:
> > ===
> >
> >   - Squash patches 2 & 3:
> >    - Remove wholesale replacement of single quotes, in favor of try blocks
> >      that attempt to parse as pure JSON, then as Python.
> >    - Factored out the value parser block to accomplish the above.
> >    - Allow both true/True and false/False for values.
> >   - Fix typo in patch 3 cover letter. (was patch 4.)
> >
> > John Snow (4):
> >    scripts: qmp-shell: refactor helpers
> >    scripts: qmp-shell: Expand support for QMP expressions
> >    scripts: qmp-shell: add transaction subshell
> >    scripts: qmp-shell: Add verbose flag
> >
> >   scripts/qmp/qmp-shell | 125 ++++++++++++++++++++++++++++++++++++++------------
> >   1 file changed, 95 insertions(+), 30 deletions(-)
> >
> 
> Version "2+" with the hotfix added, with R-B's added:
> https://github.com/jnsnow/qemu/commits/qmp-shell-plus
> 
> To whomever would pull this into their tree: Let me know if you prefer I 
> send a "v3" instead.

Please, re-send.