[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH 2/3] iotests: ask QEMU for supported formats
From: |
Andrey Shinkevich |
Subject: |
Re: [Qemu-block] [PATCH 2/3] iotests: ask QEMU for supported formats |
Date: |
Wed, 6 Mar 2019 18:03:54 +0000 |
On 06/03/2019 17:31, Kevin Wolf wrote:
> Am 04.03.2019 um 11:08 hat Andrey Shinkevich geschrieben:
>> Supported formats listed by 'qemu' may differ from those listed by
>> 'qemu-img' due to whitelists. Some test cases require specific formats
>> that may be used with qemu. They can be inquired directly by running
>> 'qemu -drive format=help'. The response takes whitelists into account.
>> The method supported_formats() serves for that. The method decorator
>> skip_if_unsupported() checks if all requested formats are whitelisted.
>> If not, the test case will be skipped. That has been implemented in
>> the 'check' file in the way similar to the 'test notrun' mechanism.
>>
>> Suggested-by: Roman Kagan <address@hidden>
>> Suggested-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>> Signed-off-by: Andrey Shinkevich <address@hidden>
>> ---
>> tests/qemu-iotests/check | 16 +++++++++++++-
>> tests/qemu-iotests/iotests.py | 50
>> +++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 65 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
>> index 895e1e3..b9d539c 100755
>> --- a/tests/qemu-iotests/check
>> +++ b/tests/qemu-iotests/check
>> @@ -25,6 +25,7 @@ try=0
>> n_bad=0
>> bad=""
>> notrun=""
>> +casenotrun=""
>> interrupt=true
>>
>> # by default don't output timestamps
>> @@ -664,6 +665,11 @@ END { if (NR > 0) {
>> echo "Not run:$notrun"
>> echo "Not run:$notrun" >>check.log
>> fi
>> + if [ ! -z "$casenotrun" ]
>> + then
>> + echo "Some cases not run in:$casenotrun"
>> + echo "Some cases not run in:$casenotrun" >>check.log
>> + fi
>> if [ ! -z "$n_bad" -a $n_bad != 0 ]
>> then
>> echo "Failures:$bad"
>> @@ -743,6 +749,10 @@ do
>> printf " " # prettier output with timestamps.
>> fi
>> rm -f core $seq.notrun
>> + if [ -f $seq.casenotrun ]
>> + then
>> + rm -f $seq.casenotrun
>> + fi
>
> The if is unnecessary here, 'rm -f' doesn't print an error if the file
> doesn't exist.
>
>> start=$(_wallclock)
>> $timestamp && printf %s " [$(date "+%T")]"
>> @@ -823,7 +833,11 @@ do
>> fi
>> fi
>> fi
>> -
>> + if [ -f $seq.casenotrun ]
>> + then
>> + cat $seq.casenotrun
>> + casenotrun="$casenotrun $seq"
>> + fi
>> fi
>>
>> # come here for each test, except when $showme is true
>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>> index b461f53..8fe1620 100644
>> --- a/tests/qemu-iotests/iotests.py
>> +++ b/tests/qemu-iotests/iotests.py
>> @@ -728,6 +728,56 @@ def verify_quorum():
>> if not supports_quorum():
>> notrun('quorum support missing')
>>
>> +def qemu_pipe(*args):
>> + '''Run qemu with an option to print something and exit (e.g. a help
>> option),
>> + and return its output'''
>> + args = [qemu_prog] + qemu_opts + list(args)
>> + subp = subprocess.Popen(args, stdout=subprocess.PIPE,
>> + stderr=subprocess.STDOUT)
>> + exitcode = subp.wait()
>> + if exitcode < 0:
>> + sys.stderr.write('qemu received signal %i: %s\n' % (-exitcode,
>> + ' '.join(args)))
>> + return subp.communicate()[0]
>> +
>> +def supported_formats(read_only=False):
>> + '''Set 'read_only' to True to check ro-whitelist
>> + Otherwise, rw-whitelist is checked'''
>> + format_message = qemu_pipe("-drive", "format=help")
>> + available = []
>> +
>> + if read_only:
>> + # Check read-only whitelist
>> + available = format_message.splitlines()[1].split(":")[1].split()
>> + else:
>> + # Check read-write whitelist
>> + available = format_message.splitlines()[0].split(":")[1].split()
>> +
>> + return available
>
> What do you need available for when you only assign it and then
> immediately return it without using it before? I think I would write
> it much shorter like this:
>
> line = 1 if read_only else 0
> return format_message.splitlines()[line].split(":")[1].split()
>
Thank you, Kevin, for your useful review. I will amend the series with
the v3.
Andrey
>> +def case_notrun(reason):
>> + '''Skip this test case'''
>> + # Each test in qemu-iotests has a number ("seq")
>> + seq = os.path.basename(sys.argv[0])
>> +
>> + open('%s/%s.casenotrun' % (output_dir, seq), 'ab').write(
>> + ' [case not run] ' + reason + '\n')
>
> Maybe move this next to notrun(), which is similar?
>
> Kevin
>