[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH for-2.6?] qemu-iotests: iotests: fail hard if no
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [PATCH for-2.6?] qemu-iotests: iotests: fail hard if not run via "check" |
Date: |
Fri, 15 Apr 2016 00:11:20 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 |
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:
>
> - It creates files with predictable file names in a world-writable
> location (/var/tmp).
>
> - 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.
>
> Instead fail hard and tell the user we want to be run via "check".
>
> Signed-off-by: Sascha Silbe <address@hidden>
> Reviewed-by: Bo Tu <address@hidden>
> ---
> 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.
>
> 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(-)
>
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> 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'):
>
> imgfmt = os.environ.get('IMGFMT', 'raw')
> imgproto = os.environ.get('IMGPROTO', 'file')
> -test_dir = os.environ.get('TEST_DIR', '/var/tmp')
> +test_dir = os.environ.get('TEST_DIR')
> output_dir = os.environ.get('OUTPUT_DIR', '.')
> cachemode = os.environ.get('CACHEMODE')
> qemu_default_machine = os.environ.get('QEMU_DEFAULT_MACHINE')
> @@ -441,6 +441,10 @@ def verify_quorum():
> def main(supported_fmts=[], supported_oses=['linux']):
> '''Run tests'''
>
> + 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 = '-d' in sys.argv
> verbosity = 1
> verify_image_format(supported_fmts)
>
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-block] [Qemu-devel] [PATCH for-2.6?] qemu-iotests: iotests: fail hard if not run via "check", Markus Armbruster, 2016/04/18