From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57253) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YlJue-0007nH-3W for qemu-devel@nongnu.org; Thu, 23 Apr 2015 12:23:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YlJuZ-0003SH-1t for qemu-devel@nongnu.org; Thu, 23 Apr 2015 12:23:40 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47941) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YlJuY-0003S5-TR for qemu-devel@nongnu.org; Thu, 23 Apr 2015 12:23:34 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 99F8B47 for ; Thu, 23 Apr 2015 16:23:34 +0000 (UTC) Date: Thu, 23 Apr 2015 18:23:31 +0200 From: Kashyap Chamarthy Message-ID: <20150423162331.GE10067@tesla.redhat.com> References: <1429799701-27089-1-git-send-email-jsnow@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1429799701-27089-1-git-send-email-jsnow@redhat.com> Subject: Re: [Qemu-devel] [PATCH v3 0/4] scripts: qmp-shell: add transaction support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow Cc: qemu-devel@nongnu.org, lcapitulino@redhat.com 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: [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