From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33273) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ykvwr-0001ug-Pp for qemu-devel@nongnu.org; Wed, 22 Apr 2015 10:48:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ykvwq-0003E9-MB for qemu-devel@nongnu.org; Wed, 22 Apr 2015 10:48:21 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54263) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ykvwq-0003Cw-Au for qemu-devel@nongnu.org; Wed, 22 Apr 2015 10:48:20 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t3MEmITv020821 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Wed, 22 Apr 2015 10:48:18 -0400 Message-ID: <5537B4B1.9060408@redhat.com> Date: Wed, 22 Apr 2015 08:48:17 -0600 From: Eric Blake MIME-Version: 1.0 References: <1429668155-1606-1-git-send-email-jsnow@redhat.com> <1429668155-1606-5-git-send-email-jsnow@redhat.com> In-Reply-To: <1429668155-1606-5-git-send-email-jsnow@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="lAOu2DPVaSvS69TIqoA9HCAhvALeVx6XE" Subject: Re: [Qemu-devel] [PATCH 4/5] scripts: qmp-shell: add transaction subshell List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow , qemu-devel@nongnu.org Cc: kchamart@redhat.com, lcapitulino@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --lAOu2DPVaSvS69TIqoA9HCAhvALeVx6XE Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 04/21/2015 08:02 PM, John Snow wrote: > Add a special processing mode to craft transactions. >=20 > 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. >=20 > The transaction can be submitted by entering ")" on a line by itself. >=20 > Examples: >=20 > Separate lines: >=20 > (QEMU) transaction( > TRANS> block-dirty-bitmap-add node=3Ddrive0 name=3Dbitmap1 > TRANS> block-dirty-bitmap-clear node=3Ddrive0 name=3Dbitmap0 > TRANS> ) >=20 > With a transaction action included on the first line: >=20 > (QEMU) transaction( block-dirty-bitmap-add node=3Ddrive0 name=3Dbitmap2= > TRANS> block-dirty-bitmap-add node=3Ddrive0 name=3Dbitmap3 > TRANS> ) >=20 > As a one-liner, with just one transation action: s/transation/transaction/ >=20 > (QEMU) transaction( block-dirty-bitmap-add node=3Ddrive0 name=3Dbitmap0= ) >=20 > Signed-off-by: John Snow > --- > scripts/qmp/qmp-shell | 43 ++++++++++++++++++++++++++++++++++++++++++-= > 1 file changed, 42 insertions(+), 1 deletion(-) Nice. And very similar to the code reuse of how I added transaction support in libvirt (reusing the guts for constructing each action as a sibling of 'type', then stringing those together to an arguments array as a sibling of 'execute'). Minor question: > + > + # Nothing to process? > + if not cmdargs: > + return None > + This returns None if we have a blank line, whether or not we are in transaction mode... > + # Parse and then cache this Transactional Action > + if self._transmode: > + finalize =3D False > + action =3D { 'type': cmdargs[0], 'data': {} } > + if cmdargs[-1] =3D=3D ')': > + cmdargs.pop(-1) > + finalize =3D True > + self.__cli_expr(cmdargs[1:], action['data']) > + self._actions.append(action) > + return self.__build_cmd(')') if finalize else None and this returns None if we have a non-blank line but remain in transaction mode... > @@ -142,6 +175,9 @@ class QMPShell(qmp.QEMUMonitorProtocol): > print 'command format: ', > print '[arg-name1=3Darg1] ... [arg-nameN=3DargN]' > return True > + # For transaction mode, we may have just cached the action: > + if qmpcmd is None: > + return True =2E..do we care if the user is entering blank lines even when not in transaction mode? That is, should this check be tightened to if qmpcmd is None and self._transmode: On the other hand, I don't know what the previous behavior was for blank lines (if an error message was printed or not), and I also think it's just fine to be silent on a blank line (and save error messages for non-blank lines that we can't parse). So I don't think it needs changing, so much as me trying to figure out what's going on. Therefore, I'm fine with: Reviewed-by: Eric Blake --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --lAOu2DPVaSvS69TIqoA9HCAhvALeVx6XE Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJVN7SxAAoJEKeha0olJ0NqRvIIAJgnFRwnoftVF+imeKFgLvaN 89JXWPmAWh11icqU0QkkhSagR6gyZ7l8XJ58+EQV+qbSLiq+QV+KmwKjOiffViTV WFmvRw+xyUkXUoHmBcapEFDePCfP78Bv+i4r9sIPDiANNqr4IORmzED+5gvczr94 laaiSfhedJsDFYrSYv2M5wvj3aXR+CFvxwdGe3sGw+ZawZRnMGsU+VkFp+OcqAm1 sW7ugIGiuZ4HbcY1SPEs3oD6sNz0i38Yj1hCIIKmg4tog+J+LXjwZeyCyjvxLEHO nr1KyIN+M2e02OFEPt9xK+/NWMTnD8CEcqZ38Eu4SrrjWGIWm6DeDRH7GCKggmU= =1cHZ -----END PGP SIGNATURE----- --lAOu2DPVaSvS69TIqoA9HCAhvALeVx6XE--