* [Qemu-devel] [PATCH v3 0/4] scripts: qmp-shell: add transaction support
@ 2015-04-23 14:34 John Snow
2015-04-23 14:34 ` [Qemu-devel] [PATCH v3 1/4] scripts: qmp-shell: refactor helpers John Snow
` (5 more replies)
0 siblings, 6 replies; 10+ messages in thread
From: John Snow @ 2015-04-23 14:34 UTC (permalink / raw)
To: qemu-devel; +Cc: John Snow, lcapitulino, kchamart
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.
===
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 | 126 ++++++++++++++++++++++++++++++++++++++------------
1 file changed, 96 insertions(+), 30 deletions(-)
--
2.1.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH v3 1/4] scripts: qmp-shell: refactor helpers
2015-04-23 14:34 [Qemu-devel] [PATCH v3 0/4] scripts: qmp-shell: add transaction support John Snow
@ 2015-04-23 14:34 ` John Snow
2015-04-23 14:34 ` [Qemu-devel] [PATCH v3 2/4] scripts: qmp-shell: Expand support for QMP expressions John Snow
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: John Snow @ 2015-04-23 14:34 UTC (permalink / raw)
To: qemu-devel; +Cc: John Snow, lcapitulino, kchamart
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH v3 2/4] scripts: qmp-shell: Expand support for QMP expressions
2015-04-23 14:34 [Qemu-devel] [PATCH v3 0/4] scripts: qmp-shell: add transaction support John Snow
2015-04-23 14:34 ` [Qemu-devel] [PATCH v3 1/4] scripts: qmp-shell: refactor helpers John Snow
@ 2015-04-23 14:34 ` John Snow
2015-04-23 14:35 ` [Qemu-devel] [PATCH v3 3/4] scripts: qmp-shell: add transaction subshell John Snow
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: John Snow @ 2015-04-23 14:34 UTC (permalink / raw)
To: qemu-devel; +Cc: John Snow, lcapitulino, kchamart
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>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
scripts/qmp/qmp-shell | 45 +++++++++++++++++++++++++++++----------------
1 file changed, 29 insertions(+), 16 deletions(-)
diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index a9632ec..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
@@ -88,23 +89,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 +130,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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH v3 3/4] scripts: qmp-shell: add transaction subshell
2015-04-23 14:34 [Qemu-devel] [PATCH v3 0/4] scripts: qmp-shell: add transaction support John Snow
2015-04-23 14:34 ` [Qemu-devel] [PATCH v3 1/4] scripts: qmp-shell: refactor helpers John Snow
2015-04-23 14:34 ` [Qemu-devel] [PATCH v3 2/4] scripts: qmp-shell: Expand support for QMP expressions John Snow
@ 2015-04-23 14:35 ` John Snow
2015-04-23 14:35 ` [Qemu-devel] [PATCH v3 4/4] scripts: qmp-shell: Add verbose flag John Snow
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: John Snow @ 2015-04-23 14:35 UTC (permalink / raw)
To: qemu-devel; +Cc: John Snow, lcapitulino, kchamart
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 e7b40eb..c9e4439 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -60,6 +60,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):
"""
@@ -141,6 +143,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
@@ -153,6 +185,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'
@@ -173,6 +208,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.
@@ -312,7 +352,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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH v3 4/4] scripts: qmp-shell: Add verbose flag
2015-04-23 14:34 [Qemu-devel] [PATCH v3 0/4] scripts: qmp-shell: add transaction support John Snow
` (2 preceding siblings ...)
2015-04-23 14:35 ` [Qemu-devel] [PATCH v3 3/4] scripts: qmp-shell: add transaction subshell John Snow
@ 2015-04-23 14:35 ` John Snow
2015-04-23 16:23 ` [Qemu-devel] [PATCH v3 0/4] scripts: qmp-shell: add transaction support John Snow
2015-04-23 16:23 ` Kashyap Chamarthy
5 siblings, 0 replies; 10+ messages in thread
From: John Snow @ 2015-04-23 14:35 UTC (permalink / raw)
To: qemu-devel; +Cc: John Snow, lcapitulino, kchamart
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 c9e4439..17babcd 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -177,6 +177,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)
@@ -188,15 +194,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):
@@ -232,6 +236,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)
@@ -317,6 +324,7 @@ def main():
qemu = None
hmp = False
pp = None
+ verbose = False
try:
for arg in sys.argv[1:]:
@@ -328,6 +336,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)
@@ -352,6 +362,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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/4] scripts: qmp-shell: add transaction support
2015-04-23 14:34 [Qemu-devel] [PATCH v3 0/4] scripts: qmp-shell: add transaction support John Snow
` (3 preceding siblings ...)
2015-04-23 14:35 ` [Qemu-devel] [PATCH v3 4/4] scripts: qmp-shell: Add verbose flag John Snow
@ 2015-04-23 16:23 ` John Snow
2015-04-23 16:23 ` Kashyap Chamarthy
5 siblings, 0 replies; 10+ messages in thread
From: John Snow @ 2015-04-23 16:23 UTC (permalink / raw)
To: qemu-devel; +Cc: kchamart, lcapitulino
On 04/23/2015 10:34 AM, 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.
>
> ===
> 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 | 126 ++++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 96 insertions(+), 30 deletions(-)
>
NACK for now, sorry for the noise, everyone. Kashyap found a bug while
testing with this.
Thanks, Kashyap!
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/4] scripts: qmp-shell: add transaction support
2015-04-23 14:34 [Qemu-devel] [PATCH v3 0/4] scripts: qmp-shell: add transaction support John Snow
` (4 preceding siblings ...)
2015-04-23 16:23 ` [Qemu-devel] [PATCH v3 0/4] scripts: qmp-shell: add transaction support John Snow
@ 2015-04-23 16:23 ` Kashyap Chamarthy
2015-04-28 16:17 ` Kashyap Chamarthy
5 siblings, 1 reply; 10+ messages in thread
From: Kashyap Chamarthy @ 2015-04-23 16:23 UTC (permalink / raw)
To: John Snow; +Cc: qemu-devel, lcapitulino
On Thu, Apr 23, 2015 at 10:34:57AM -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
> (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.
>
> ===
> v3:
> ===
>
> - Folding in hotfix from list (import ast)
Seems like a regression from your v2.
It fails here, even for a non-transaction command, with your patch series
applied:
(QEMU) blockdev-snapshot-internal-sync device=drive-ide0-0-0 name=snapshot0
Error while parsing command line: global name '_QMPShell__parse_value' is not defined
command format: <command-name> [arg-name1=arg1] ... [arg-nameN=argN]
Earlier the day, with v2, I was able to test it correctly:
(QEMU) transaction(
TRANS> blockdev-snapshot-sync device=drive-ide0-0-0 snapshot-file=./ext-snap1.qcow2 format=qcow2
TRANS> blockdev-snapshot-internal-sync device=drive-ide0-0-0 name=snapshot0
TRANS> )
{u'return': {}}
(QEMU)
--
/kashyap
>
> ===
> 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 | 126 ++++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 96 insertions(+), 30 deletions(-)
>
> --
> 2.1.0
>
--
/kashyap
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/4] scripts: qmp-shell: add transaction support
2015-04-23 16:23 ` Kashyap Chamarthy
@ 2015-04-28 16:17 ` Kashyap Chamarthy
2015-04-28 16:19 ` John Snow
2015-04-29 15:20 ` Kashyap Chamarthy
0 siblings, 2 replies; 10+ messages in thread
From: Kashyap Chamarthy @ 2015-04-28 16:17 UTC (permalink / raw)
To: John Snow; +Cc: qemu-devel, lcapitulino
On Thu, Apr 23, 2015 at 06:23:31PM +0200, Kashyap Chamarthy wrote:
> On Thu, Apr 23, 2015 at 10:34:57AM -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
> > (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.
> >
> > ===
> > v3:
> > ===
> >
> > - Folding in hotfix from list (import ast)
>
> Seems like a regression from your v2.
>
> It fails here, even for a non-transaction command, with your patch series
> applied:
>
> (QEMU) blockdev-snapshot-internal-sync device=drive-ide0-0-0 name=snapshot0
> Error while parsing command line: global name '_QMPShell__parse_value' is not defined
> command format: <command-name> [arg-name1=arg1] ... [arg-nameN=argN]
I now tested again with your qmp-shell-plus branch:
$ git describe
v2.3.0-rc4-4-g994af97
$ git log --oneline | head -4
994af97 scripts: qmp-shell: Add verbose flag
1009369 scripts: qmp-shell: add transaction subshell
0ae65ff scripts: qmp-shell: Expand support for QMP expressions
5f367d9 scripts: qmp-shell: refactor helpers
Result:
- The non-transaction commands work just fine; so that regression is
fixed in your qmp-shell-plus branch.
- The transaction command (same commands as tested previously,
retained it below) still fails.
Just wanted to let you know here.
--
/kashyap
> Earlier the day, with v2, I was able to test it correctly:
>
> (QEMU) transaction(
> TRANS> blockdev-snapshot-sync device=drive-ide0-0-0 snapshot-file=./ext-snap1.qcow2 format=qcow2
> TRANS> blockdev-snapshot-internal-sync device=drive-ide0-0-0 name=snapshot0
> TRANS> )
> {u'return': {}}
> (QEMU)
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/4] scripts: qmp-shell: add transaction support
2015-04-28 16:17 ` Kashyap Chamarthy
@ 2015-04-28 16:19 ` John Snow
2015-04-29 15:20 ` Kashyap Chamarthy
1 sibling, 0 replies; 10+ messages in thread
From: John Snow @ 2015-04-28 16:19 UTC (permalink / raw)
To: Kashyap Chamarthy; +Cc: qemu-devel, lcapitulino
On 04/28/2015 12:17 PM, Kashyap Chamarthy wrote:
> On Thu, Apr 23, 2015 at 06:23:31PM +0200, Kashyap Chamarthy wrote:
>> On Thu, Apr 23, 2015 at 10:34:57AM -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
>>> (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.
>>>
>>> ===
>>> v3:
>>> ===
>>>
>>> - Folding in hotfix from list (import ast)
>>
>> Seems like a regression from your v2.
>>
>> It fails here, even for a non-transaction command, with your patch series
>> applied:
>>
>> (QEMU) blockdev-snapshot-internal-sync device=drive-ide0-0-0 name=snapshot0
>> Error while parsing command line: global name '_QMPShell__parse_value' is not defined
>> command format: <command-name> [arg-name1=arg1] ... [arg-nameN=argN]
>
> I now tested again with your qmp-shell-plus branch:
>
> $ git describe
> v2.3.0-rc4-4-g994af97
>
> $ git log --oneline | head -4
> 994af97 scripts: qmp-shell: Add verbose flag
> 1009369 scripts: qmp-shell: add transaction subshell
> 0ae65ff scripts: qmp-shell: Expand support for QMP expressions
> 5f367d9 scripts: qmp-shell: refactor helpers
>
> Result:
>
> - The non-transaction commands work just fine; so that regression is
> fixed in your qmp-shell-plus branch.
> - The transaction command (same commands as tested previously,
> retained it below) still fails.
>
> Just wanted to let you know here.
>
Yeah, I NACKed the version sent to list because I need to review it more
after some hasty changes last week. I apparently made some cleanup edits
that I thought didn't change anything, but very clearly they do :)
Might as well take the time to try to improve it a little bit more while
I'm at it.
--js
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/4] scripts: qmp-shell: add transaction support
2015-04-28 16:17 ` Kashyap Chamarthy
2015-04-28 16:19 ` John Snow
@ 2015-04-29 15:20 ` Kashyap Chamarthy
1 sibling, 0 replies; 10+ messages in thread
From: Kashyap Chamarthy @ 2015-04-29 15:20 UTC (permalink / raw)
To: John Snow; +Cc: qemu-devel, lcapitulino
On Tue, Apr 28, 2015 at 06:17:32PM +0200, Kashyap Chamarthy wrote:
[. . .]
> > Seems like a regression from your v2.
> >
> > It fails here, even for a non-transaction command, with your patch series
> > applied:
> >
> > (QEMU) blockdev-snapshot-internal-sync device=drive-ide0-0-0 name=snapshot0
> > Error while parsing command line: global name '_QMPShell__parse_value' is not defined
> > command format: <command-name> [arg-name1=arg1] ... [arg-nameN=argN]
>
> I now tested again with your qmp-shell-plus branch:
>
> $ git describe
> v2.3.0-rc4-4-g994af97
>
> $ git log --oneline | head -4
> 994af97 scripts: qmp-shell: Add verbose flag
> 1009369 scripts: qmp-shell: add transaction subshell
> 0ae65ff scripts: qmp-shell: Expand support for QMP expressions
> 5f367d9 scripts: qmp-shell: refactor helpers
>
> Result:
>
> - The non-transaction commands work just fine; so that regression is
> fixed in your qmp-shell-plus branch.
> - The transaction command (same commands as tested previously,
> retained it below) still fails.
Tested with your newer branch[1], this time, good news -- from my
minimal testing, no regressions found while running transactional/
non-transactional commands.
As a transactional subshell test, I ran a three-combination command like
below:
(QEMU) transaction(
TRANS> blockdev-snapshot-sync device=drive-ide0-0-0 snapshot-file=./ext-snap2.qcow2 format=qcow2
TRANS> blockdev-snapshot-internal-sync device=drive-ide0-0-0 name=snapshot1
TRANS> drive-backup device=drive-ide0-0-0 sync=full target=./backup-copy.qcow2 mode=absolute-paths format=qcow2
TRANS> )
{"return": {}}
(QEMU)
When you submit a new version to the list, FWIW, you can carry my
'Tested-by'.
[1] https://github.com/jnsnow/qemu/tree/qmp-shell%2B%2B
--
/kashyap
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-04-29 15:20 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-23 14:34 [Qemu-devel] [PATCH v3 0/4] scripts: qmp-shell: add transaction support John Snow
2015-04-23 14:34 ` [Qemu-devel] [PATCH v3 1/4] scripts: qmp-shell: refactor helpers John Snow
2015-04-23 14:34 ` [Qemu-devel] [PATCH v3 2/4] scripts: qmp-shell: Expand support for QMP expressions John Snow
2015-04-23 14:35 ` [Qemu-devel] [PATCH v3 3/4] scripts: qmp-shell: add transaction subshell John Snow
2015-04-23 14:35 ` [Qemu-devel] [PATCH v3 4/4] scripts: qmp-shell: Add verbose flag John Snow
2015-04-23 16:23 ` [Qemu-devel] [PATCH v3 0/4] scripts: qmp-shell: add transaction support John Snow
2015-04-23 16:23 ` Kashyap Chamarthy
2015-04-28 16:17 ` Kashyap Chamarthy
2015-04-28 16:19 ` John Snow
2015-04-29 15:20 ` Kashyap Chamarthy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.