From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47486) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aqpU1-00064y-9W for qemu-devel@nongnu.org; Thu, 14 Apr 2016 18:11:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aqpU0-0004MQ-4h for qemu-devel@nongnu.org; Thu, 14 Apr 2016 18:11:29 -0400 References: <1460633543-7366-1-git-send-email-silbe@linux.vnet.ibm.com> From: Max Reitz Message-ID: <57101588.3080204@redhat.com> Date: Fri, 15 Apr 2016 00:11:20 +0200 MIME-Version: 1.0 In-Reply-To: <1460633543-7366-1-git-send-email-silbe@linux.vnet.ibm.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="bjvAKgCIGOqFCL54dKNQatWflK6qdNC3C" Subject: Re: [Qemu-devel] [PATCH for-2.6?] qemu-iotests: iotests: fail hard if not run via "check" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sascha Silbe , qemu-devel@nongnu.org, qemu-block@nongnu.org Cc: Kevin Wolf This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --bjvAKgCIGOqFCL54dKNQatWflK6qdNC3C Content-Type: multipart/mixed; boundary="a6WU2e0Xrek8C7Q6FUUImiG9o18bbfGcN" From: Max Reitz To: Sascha Silbe , qemu-devel@nongnu.org, qemu-block@nongnu.org Cc: Kevin Wolf Message-ID: <57101588.3080204@redhat.com> Subject: Re: [PATCH for-2.6?] qemu-iotests: iotests: fail hard if not run via "check" References: <1460633543-7366-1-git-send-email-silbe@linux.vnet.ibm.com> In-Reply-To: <1460633543-7366-1-git-send-email-silbe@linux.vnet.ibm.com> --a6WU2e0Xrek8C7Q6FUUImiG9o18bbfGcN Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable On 14.04.2016 13:32, Sascha Silbe wrote: > Running an iotests-based Python test directly might appear to work, > but may fail in subtle ways and is insecure: >=20 > - It creates files with predictable file names in a world-writable > location (/var/tmp). >=20 > - Tests expect the environment to be set up by check. E.g. 041 and 055 > may take the wrong code paths if QEMU_DEFAULT_MACHINE is not > set. This can lead to false negatives. >=20 > Instead fail hard and tell the user we want to be run via "check". >=20 > Signed-off-by: Sascha Silbe > Reviewed-by: Bo Tu > --- > It's possible to fix iotests.py to work even outside of check, but > that requires reimplementing parts of what check currently does. I'd > prefer not to do that this late in the cycle. >=20 > It would be nice to have this in 2.6, just in case someone even tries > running a Python-based test directly instead of using ./check like for > the shell-based tests. But it's not crucial. > --- > tests/qemu-iotests/iotests.py | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) >=20 > diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests= =2Epy > index 0c0b533..8c7138f 100644 > --- a/tests/qemu-iotests/iotests.py > +++ b/tests/qemu-iotests/iotests.py > @@ -46,7 +46,7 @@ if os.environ.get('QEMU_OPTIONS'): > =20 > imgfmt =3D os.environ.get('IMGFMT', 'raw') > imgproto =3D os.environ.get('IMGPROTO', 'file') > -test_dir =3D os.environ.get('TEST_DIR', '/var/tmp') > +test_dir =3D os.environ.get('TEST_DIR') > output_dir =3D os.environ.get('OUTPUT_DIR', '.') > cachemode =3D os.environ.get('CACHEMODE') > qemu_default_machine =3D os.environ.get('QEMU_DEFAULT_MACHINE') > @@ -441,6 +441,10 @@ def verify_quorum(): > def main(supported_fmts=3D[], supported_oses=3D['linux']): > '''Run tests''' > =20 > + if test_dir is None or qemu_default_machine is None: I think checking test_dir would have been sufficient; this makes it look like it might want to be a complete list of variables that have to be set, but then "cachemode" is missing. > + sys.stderr.write('Please run this test via ./check\n') Or not ./, for people who have separate build and source trees, you don't want to invoke check in the source tree. ;-) I'm not sure whether I'd want a v2 just because of this. It's bike-shedding, but now that I think about it I guess I think I do. So would you be so kind as to replace the "./check" by "the check script"? :-) (I don't particularly care about whether you change the condition used in the if statement, though.) Max > + sys.exit(os.EX_USAGE) > + > debug =3D '-d' in sys.argv > verbosity =3D 1 > verify_image_format(supported_fmts) >=20 --a6WU2e0Xrek8C7Q6FUUImiG9o18bbfGcN-- --bjvAKgCIGOqFCL54dKNQatWflK6qdNC3C 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 iQEcBAEBCAAGBQJXEBWIAAoJEDuxQgLoOKyt+uUH/2HB2e3mYd4HmVMbGFr6BdmC 4WfL4m4K6RukJvQkFk1t/5GxVAsYEB76WGvEy6pZOL5oXpdbytd1Zb+tuZ6LGePu Z5gR9k4MQ8x+7GHOWb8YlJPn7TVVNLgv7hjStmJATtd4mIqHI5PuvoXDBh6gExQv eksyAOI/9FyeCXlRzBSPpXinecIN7YI8QjNkavC+luZoiHrHTu6N5R+6pl+Oh3Rw 6YeAjMYMjuA9PM6CyWb9HD71I6F5E8XVSgZ4SsYEImy1yaNlYM3hJttLj89FRLwY NDuCM40i2seiNZ83El/QieqLpkMSlvGWEGA+wFqJWku6E2OFYW3N/8u89YFU7Uc= =Kqu2 -----END PGP SIGNATURE----- --bjvAKgCIGOqFCL54dKNQatWflK6qdNC3C--