From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60341) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YnkQX-0006Q6-Hj for qemu-devel@nongnu.org; Thu, 30 Apr 2015 05:06:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YnkQR-00030w-F8 for qemu-devel@nongnu.org; Thu, 30 Apr 2015 05:06:37 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60808) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YnkQR-00030b-7b for qemu-devel@nongnu.org; Thu, 30 Apr 2015 05:06:31 -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 (8.14.4/8.14.4) with ESMTP id t3U96TTE027672 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Thu, 30 Apr 2015 05:06:29 -0400 Date: Thu, 30 Apr 2015 11:06:26 +0200 From: Kashyap Chamarthy Message-ID: <20150430090626.GE11726@tesla.redhat.com> References: <1430334844-7015-1-git-send-email-jsnow@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1430334844-7015-1-git-send-email-jsnow@redhat.com> Subject: Re: [Qemu-devel] [PATCH v4 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 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 -- /kashyap