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

[Qemu-devel] [PATCH v4 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 and
    improve the robustness of QMP parsing
(3) Add a special transaction( ... ) syntax that lets users
    build up transactional commands using the existing qmp shell
    syntax to define each action.
(4) 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.

For convenience, this branch is available at:
https://github.com/jnsnow/qemu.git branch qmp-shell++
This version is tagged qmp-shell++-v4.

===
v++
===

 - Use the AST to allow 'true', 'false' and 'null' within QMP expressions
 - Fix a bunch of stupid junk I broke in v2, apparently.

===
v3:
===

 - Folding in hotfix from list (import ast)

===
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 | 147 +++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 116 insertions(+), 31 deletions(-)

-- 
2.1.0

[Qemu-devel] [PATCH v4 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 v4 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", "False" and "None" literals instead
of JSON's equivalent true, false, and null 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 | 63 ++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 47 insertions(+), 16 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index a9632ec..7f2c554 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
@@ -51,6 +52,19 @@ class QMPShellError(Exception):
 class QMPShellBadPort(QMPShellError):
     pass
 
+class FuzzyJSON(ast.NodeTransformer):
+    '''This extension of ast.NodeTransformer filters literal "true/false/null"
+    values in an AST and replaces them by proper "True/False/None" values that
+    Python can properly evaluate.'''
+    def visit_Name(self, node):
+        if node.id == 'true':
+            node.id = 'True'
+        if node.id == 'false':
+            node.id = 'False'
+        if node.id == 'null':
+            node.id = 'None'
+        return node
+
 # TODO: QMPShell's interface is a bit ugly (eg. _fill_completion() and
 #       _execute_cmd()). Let's design a better one.
 class QMPShell(qmp.QEMUMonitorProtocol):
@@ -88,23 +102,40 @@ 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 first as pure JSON:
+            try:
+                return json.loads(val)
+            except ValueError:
+                pass
+            # Try once again as FuzzyJSON:
+            try:
+                st = ast.parse(val, mode='eval')
+                return ast.literal_eval(FuzzyJSON().visit(st))
+            except SyntaxError:
+                pass
+            except ValueError:
+                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 = self.__parse_value(val)
+            optpath = key.split('.')
             curpath = []
             for p in optpath[:-1]:
                 curpath.append(p)
@@ -117,7 +148,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 v4 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>
---
 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 7f2c554..1df2ca7 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -73,6 +73,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):
         """
@@ -159,6 +161,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 len(cmdargs) > 1:
+                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
@@ -171,6 +203,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'
@@ -191,6 +226,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.
@@ -330,7 +370,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 v4 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.

The QMP is converted from a Python literal to JSON first,
to ensure that it is viable input to the actual QMP parser.

As a side-effect, this JSON output will helpfully show all
the necessary conversions that were performed on the input,
illustrating that "True" was transformed back into "true",
literal values are now escaped with "" instead of '', and so on.

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

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 1df2ca7..65280d2 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -195,6 +195,13 @@ class QMPShell(qmp.QEMUMonitorProtocol):
         self.__cli_expr(cmdargs[1:], qmpcmd['arguments'])
         return qmpcmd
 
+    def _print(self, qmp):
+        jsobj = json.dumps(qmp)
+        if self._pp is not None:
+            self._pp.pprint(jsobj)
+        else:
+            print str(jsobj)
+
     def _execute_cmd(self, cmdline):
         try:
             qmpcmd = self.__build_cmd(cmdline)
@@ -206,15 +213,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):
@@ -250,6 +255,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)
@@ -327,7 +335,7 @@ def die(msg):
 def fail_cmdline(option=None):
     if option:
         sys.stderr.write('ERROR: bad command-line option \'%s\'\n' % option)
-    sys.stderr.write('qemu-shell [ -p ] [ -H ] < UNIX socket path> | < TCP address:port >\n')
+    sys.stderr.write('qemu-shell [ -v ] [ -p ] [ -H ] < UNIX socket path> | < TCP address:port >\n')
     sys.exit(1)
 
 def main():
@@ -335,6 +343,7 @@ def main():
     qemu = None
     hmp = False
     pp = None
+    verbose = False
 
     try:
         for arg in sys.argv[1:]:
@@ -346,6 +355,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)
@@ -370,6 +381,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 v4 2/4] scripts: qmp-shell: Expand support for QMP expressions

[Eric Blake]

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

On 04/29/2015 01:14 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", "False" and "None" literals instead
> of JSON's equivalent true, false, and null 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 | 63 ++++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 47 insertions(+), 16 deletions(-)
> 

>  
> +class FuzzyJSON(ast.NodeTransformer):
> +    '''This extension of ast.NodeTransformer filters literal "true/false/null"
> +    values in an AST and replaces them by proper "True/False/None" values that
> +    Python can properly evaluate.'''
> +    def visit_Name(self, node):
> +        if node.id == 'true':
> +            node.id = 'True'
> +        if node.id == 'false':
> +            node.id = 'False'
> +        if node.id == 'null':
> +            node.id = 'None'
> +        return node

Cute!

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

-- 
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 v4 3/4] scripts: qmp-shell: add transaction subshell

[Eric Blake]

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

On 04/29/2015 01:14 PM, John Snow wrote:
> 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.
> 

> 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>
> ---
>  scripts/qmp/qmp-shell | 42 +++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 41 insertions(+), 1 deletion(-)
> 

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

-- 
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 v4 4/4] scripts: qmp-shell: Add verbose flag

[Eric Blake]

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

On 04/29/2015 01:14 PM, John Snow wrote:
> Add a verbose flag that shows the QMP command that was
> constructed, to allow for later copy/pasting, reference,
> debugging, etc.
> 
> The QMP is converted from a Python literal to JSON first,
> to ensure that it is viable input to the actual QMP parser.
> 
> As a side-effect, this JSON output will helpfully show all
> the necessary conversions that were performed on the input,
> illustrating that "True" was transformed back into "true",
> literal values are now escaped with "" instead of '', and so on.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  scripts/qmp/qmp-shell | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 

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

-- 
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 v4 2/4] scripts: qmp-shell: Expand support for QMP expressions

[John Snow]

On 04/29/2015 03:25 PM, Eric Blake wrote:
> On 04/29/2015 01:14 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", "False" and "None" literals instead
>> of JSON's equivalent true, false, and null 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 | 63 ++++++++++++++++++++++++++++++++++++++-------------
>>   1 file changed, 47 insertions(+), 16 deletions(-)
>>
>
>>
>> +class FuzzyJSON(ast.NodeTransformer):
>> +    '''This extension of ast.NodeTransformer filters literal "true/false/null"
>> +    values in an AST and replaces them by proper "True/False/None" values that
>> +    Python can properly evaluate.'''
>> +    def visit_Name(self, node):
>> +        if node.id == 'true':
>> +            node.id = 'True'
>> +        if node.id == 'false':
>> +            node.id = 'False'
>> +        if node.id == 'null':
>> +            node.id = 'None'
>> +        return node
>
> Cute!
>

;)

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

So the only remaining crime I am aware of is that when specifying 
key=val pairs without hoping for a favorable QMP deserialization is that 
we still accept e.g. "TrUe" and "FaLSe" and so on.

Patch 4 tries to make amends by explicitly converting objects back to 
strict JSON and printing that out for the user (if they supplied -v) so 
we can observe what conversions qmp-shell made for us to make things nice.

And I think I'm done playing with this for now. If I go any further, 
there's bound to be flex and bison files in the tree!

--js

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

[Kashyap Chamarthy]

On Wed, Apr 29, 2015 at 03:14:00PM -0400, 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 and
>     improve the robustness of QMP parsing
> (3) Add a special transaction( ... ) syntax that lets users
>     build up transactional commands using the existing qmp shell
>     syntax to define each action.
> (4) 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.
> 
> For convenience, this branch is available at:
> https://github.com/jnsnow/qemu.git branch qmp-shell++
> This version is tagged qmp-shell++-v4.
> 
> ===
> v++
> ===
> 
>  - Use the AST to allow 'true', 'false' and 'null' within QMP expressions
>  - Fix a bunch of stupid junk I broke in v2, apparently.
> 
> ===
> v3:
> ===
> 
>  - Folding in hotfix from list (import ast)
> 
> ===
> 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 | 147 +++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 116 insertions(+), 31 deletions(-)


Quick test, works as advertized. This time, I ran this series on top of
your incremental backup branch:

A positive test (sorry for the un-wrapped long lines). I already had the
target image pre-created:

$ ./qmp-shell -v ./qmp-sock
Welcome to the QMP low-level shell!
Connected to QEMU 2.2.94
(QEMU)
(QEMU) transaction(
TRANS> blockdev-snapshot-internal-sync device=drive-ide0-0-0 name=snapshot5
TRANS> block-dirty-bitmap-add node=drive-ide0-0-0 name=bitmap1
TRANS> block-dirty-bitmap-clear node=drive-ide0-0-0 name=bitmap0                                                                                                                           
TRANS> drive-backup device=drive-ide0-0-0 bitmap=bitmap1 sync=dirty-bitmap target=./incremental.0.img mode=existing format=qcow2
TRANS> )
{"execute": "transaction", "arguments": {"actions": [{"data": {"device": "drive-ide0-0-0", "name": "snapshot5"}, "type": "blockdev-snapshot-internal-sync"}, {"data": {"node": "drive-ide0-0-0", "name": "bitmap1"}, "type": "block-dirty-bitmap-add"}, {"data": {"node": "drive-ide0-0-0", "name": "bitmap0"}, "type": "block-dirty-bitmap-clear"}, {"data": {"target": "./incremental.0.img", "format": "qcow2", "sync": "dirty-bitmap", "bitmap": "bitmap1", "mode": "existing", "device": "drive-ide0-0-0"}, "type": "drive-backup"}]}}
{"return": {}}
(QEMU)

And a quick negative test: don't pre-create the target image when
running the `drive-backup` command, appropriate error is thrown.


So, FWIW:

  Tested-by: Kashyap Chamarthy <kchamart@redhat.com>

-- 
/kashyap

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

[John Snow]

Ping
(I should've CC'd Luiz to begin with ...)

Eric's given this series the once over and Kashyap has tested it, so it 
should in theory be good to go.

On 04/29/2015 03:14 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 and
>      improve the robustness of QMP parsing
> (3) Add a special transaction( ... ) syntax that lets users
>      build up transactional commands using the existing qmp shell
>      syntax to define each action.
> (4) 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.
>
> For convenience, this branch is available at:
> https://github.com/jnsnow/qemu.git branch qmp-shell++
> This version is tagged qmp-shell++-v4.
>
> ===
> v++
> ===
>
>   - Use the AST to allow 'true', 'false' and 'null' within QMP expressions
>   - Fix a bunch of stupid junk I broke in v2, apparently.
>
> ===
> v3:
> ===
>
>   - Folding in hotfix from list (import ast)
>
> ===
> 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 | 147 +++++++++++++++++++++++++++++++++++++++-----------
>   1 file changed, 116 insertions(+), 31 deletions(-)
>

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

[Luiz Capitulino]

On Mon, 04 May 2015 15:24:02 -0400
John Snow <jsnow@redhat.com> wrote:

> Ping
> (I should've CC'd Luiz to begin with ...)
> 
> Eric's given this series the once over and Kashyap has tested it, so it 
> should in theory be good to go.

I lost track of it. Is v4 a new posting?

> 
> On 04/29/2015 03:14 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 and
> >      improve the robustness of QMP parsing
> > (3) Add a special transaction( ... ) syntax that lets users
> >      build up transactional commands using the existing qmp shell
> >      syntax to define each action.
> > (4) 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.
> >
> > For convenience, this branch is available at:
> > https://github.com/jnsnow/qemu.git branch qmp-shell++
> > This version is tagged qmp-shell++-v4.
> >
> > ===
> > v++
> > ===
> >
> >   - Use the AST to allow 'true', 'false' and 'null' within QMP expressions
> >   - Fix a bunch of stupid junk I broke in v2, apparently.
> >
> > ===
> > v3:
> > ===
> >
> >   - Folding in hotfix from list (import ast)
> >
> > ===
> > 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 | 147 +++++++++++++++++++++++++++++++++++++++-----------
> >   1 file changed, 116 insertions(+), 31 deletions(-)
> >
> 

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

[John Snow]

On 05/05/2015 08:52 AM, Luiz Capitulino wrote:
> I lost track of it. Is v4 a new posting?

Yup. Latest with fixes. Eric re-reviewed it all and Kashyap (and I) 
tested it.

--js

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

[Luiz Capitulino]

On Tue, 05 May 2015 11:34:02 -0400
John Snow <jsnow@redhat.com> wrote:

> 
> 
> On 05/05/2015 08:52 AM, Luiz Capitulino wrote:
> > I lost track of it. Is v4 a new posting?
> 
> Yup. Latest with fixes. Eric re-reviewed it all and Kashyap (and I) 
> tested it.

OK, I'm queuing it for my next pass over my qmp-patches mailbox.

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

[Luiz Capitulino]

On Wed, 29 Apr 2015 15:14:00 -0400
John Snow <jsnow@redhat.com> 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 and
>     improve the robustness of QMP parsing
> (3) Add a special transaction( ... ) syntax that lets users
>     build up transactional commands using the existing qmp shell
>     syntax to define each action.
> (4) 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.
> 
> For convenience, this branch is available at:
> https://github.com/jnsnow/qemu.git branch qmp-shell++
> This version is tagged qmp-shell++-v4.

Applied to the qmp branch, thanks.

> 
> ===
> v++
> ===
> 
>  - Use the AST to allow 'true', 'false' and 'null' within QMP expressions
>  - Fix a bunch of stupid junk I broke in v2, apparently.
> 
> ===
> v3:
> ===
> 
>  - Folding in hotfix from list (import ast)
> 
> ===
> 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 | 147 +++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 116 insertions(+), 31 deletions(-)
>