From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47632) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZsbpY-00054G-1z for qemu-devel@nongnu.org; Sat, 31 Oct 2015 15:28:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZsbpW-0008Bv-Qp for qemu-devel@nongnu.org; Sat, 31 Oct 2015 15:28:47 -0400 References: <6ace4d33d6a0cd429e851abf38713dfaf32a6136.1445600995.git.berto@igalia.com> From: Max Reitz Message-ID: <56351663.7000908@redhat.com> Date: Sat, 31 Oct 2015 20:28:35 +0100 MIME-Version: 1.0 In-Reply-To: <6ace4d33d6a0cd429e851abf38713dfaf32a6136.1445600995.git.berto@igalia.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="NXIVUhAnvLsK0HifcQrOii2LOKjj3AUK0" Subject: Re: [Qemu-devel] [PATCH v3 4/4] iotests: Add tests for the x-blockdev-del command List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alberto Garcia , qemu-devel@nongnu.org Cc: Kevin Wolf , Stefan Hajnoczi , qemu-block@nongnu.org, Markus Armbruster This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --NXIVUhAnvLsK0HifcQrOii2LOKjj3AUK0 Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable On 23.10.2015 14:03, Alberto Garcia wrote: > Signed-off-by: Alberto Garcia > --- > tests/qemu-iotests/139 | 408 +++++++++++++++++++++++++++++++++++++= ++++++++ > tests/qemu-iotests/139.out | 5 + > tests/qemu-iotests/group | 1 + > 3 files changed, 414 insertions(+) > create mode 100644 tests/qemu-iotests/139 > create mode 100644 tests/qemu-iotests/139.out >=20 > diff --git a/tests/qemu-iotests/139 b/tests/qemu-iotests/139 > new file mode 100644 > index 0000000..f38f7e4 > --- /dev/null > +++ b/tests/qemu-iotests/139 > @@ -0,0 +1,408 @@ [...] > + > + # Add a BlockDriverState that has base_img as its backing file Not quite; while new_img does have base_img as its backing file, new_img's format BDS explicitly does not. > + def addBlockDriverStateOverlay(self, node): > + self.checkBlockDriverState(node, False) > + iotests.qemu_img('create', '-f', iotests.imgfmt, > + '-b', base_img, new_img, '1M') > + opts =3D {'driver': iotests.imgfmt, > + 'node-name': node, > + 'backing': '', > + 'file': {'driver': 'file', > + 'filename': new_img}} > + result =3D self.vm.qmp('blockdev-add', conv_keys =3D False, op= tions =3D opts) > + self.assert_qmp(result, 'return', {}) > + self.checkBlockDriverState(node) [...] > + # Add a BlkDebug node > + def addBlkDebug(self, debug, node): > + self.checkBlockDriverState(node, False) > + self.checkBlockDriverState(debug, False) > + image =3D {'driver': iotests.imgfmt, > + 'node-name': node, > + 'file': {'driver': 'file', > + 'filename': base_img}} > + opts =3D {'driver': 'blkdebug', > + 'node-name': debug, > + 'image': image} Normally, blkdebug is supposed to sit between the format and the protocol BDS. (You can either make this a test of "raw" instead of "blkdebug", amend the blkdebug placement, or simply ignore how it's "normally supposed to be".) > + result =3D self.vm.qmp('blockdev-add', conv_keys =3D False, op= tions =3D opts) > + self.assert_qmp(result, 'return', {}) > + self.checkBlockDriverState(node) > + self.checkBlockDriverState(debug) > + > + # Add a BlkVerify node > + def addBlkVerify(self, blkverify, test, raw): > + self.checkBlockDriverState(test, False) > + self.checkBlockDriverState(raw, False) > + self.checkBlockDriverState(blkverify, False) > + iotests.qemu_img('create', '-f', iotests.imgfmt, new_img, '1M'= ) > + node_0 =3D {'driver': iotests.imgfmt, > + 'node-name': test, > + 'file': {'driver': 'file', > + 'filename': base_img}} > + node_1 =3D {'driver': iotests.imgfmt, > + 'node-name': raw, > + 'file': {'driver': 'file', > + 'filename': new_img}} And, well, this is normally supposed to be a protocol BDS. The use case of blkverify is not to be a dumbed-down quorum, but to test the block driver used for the test node. (Here, your choices are either to drop this test, or again to ignore how it's normally supposed to be.) > + opts =3D {'driver': 'blkverify', > + 'node-name': blkverify, > + 'test': node_0, > + 'raw': node_1} > + result =3D self.vm.qmp('blockdev-add', conv_keys =3D False, op= tions =3D opts) > + self.assert_qmp(result, 'return', {}) > + self.checkBlockDriverState(test) > + self.checkBlockDriverState(raw) > + self.checkBlockDriverState(blkverify) Assuming you choose the "ignore" option for both of the above, and you do something about the *Overlay comment or I simply accept it as "not that wrong, and it's just a comment in a test": Reviewed-by: Max Reitz --NXIVUhAnvLsK0HifcQrOii2LOKjj3AUK0 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 iQEcBAEBCAAGBQJWNRZjAAoJEDuxQgLoOKyt3+cH/juUoduHUzls8tVvb13g42SQ yLAOALQsKQajuDozLiHSxgmI+ukHhl6eZ/qhG6LLtZWKLBFEwxwI25LaJ0ptDOkb KO7gh0n1bnzf7wwhD24nczvM4LLSZ5tkaGlWAPTvM36WpY48dbmwmkBwBdee0P1g v2gNbhAman4+/JoBb+vIMfXGhvhjJntag4NHMobbKdXXWwpn2//q7Qa5yrV+UrwB tDdU6LV6tLvdCFAl2RfF71uLxmakE2c6+nrZU2rJNUaLd22rOiLPx3DhxWqNPQRW mwHLfbj4+He/wuaiYpc9/gBMFiA9RmNdYSq47DYaY/RwHViPmOS5UH9oDqRjcy8= =wkGf -----END PGP SIGNATURE----- --NXIVUhAnvLsK0HifcQrOii2LOKjj3AUK0--