Re: [Qemu-block] [PATCH v5 01/10] tests/perf: Test qemu-img convert from raw to encrypted qcow2

[Max Reitz]

On 30.04.19 10:53, Vladimir Sementsov-Ogievskiy wrote:
> 29.04.2019 1:55, Max Reitz wrote:
>> On 02.04.19 17:37, Vladimir Sementsov-Ogievskiy wrote:
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>>> ---
>>>   tests/perf/block/qcow2/convert-to-encrypted | 48 +++++++++++++++++++++
>>>   1 file changed, 48 insertions(+)
>>>   create mode 100755 tests/perf/block/qcow2/convert-to-encrypted
>>
>> Thanks for the test case, but I don’t know whether this is the right way
>> to include it.
>>
>> A concrete problem is that it doesn’t work with out-of-tree builds (I
>> only do out-of-tree builds).  I wonder whether it would be possible and
>> make sense (I have no idea) to add a subdirectory "perf" to the iotests
>> and reuse its infrastructure?  Those tests wouldn’t run by default.
>>
> 
> Honestly, I don't really like existing iotests infrastructure, bound to check
> script, which I don't like too (and any other large script in bash, sorry :(..

Hm, OK.  It would need some modifications, because it’d need to accept
non-numeric test names, and for perf tests, it probably shouldn’t
compare against a reference output.  But I don’t like bash either, and
that doesn’t sound impossible to me.

> What do you mean? You have env variables QEMU_IMG, etc, and want them to be
> accepted by script?

That would work, although it’d be cumbersome.  As for check, you just
run it from the build tree, so it auto-detects the binaries.

> I'd prefer to commit something simple and separate, to be able to build up
> infrastructure around it gradually..

Well, I’d prefer something that works.  I’d also very much prefer
something that is not separate, because that’s just adding complexity
for no good reason.  I don’t see how new infrastructure that works can
be simple.

There are Avocado tests, maybe you prefer using that.

> Finally, I want a simple way to run
> a set of perf tests on a set of git commits and get an html and ascii
> tables of performance comparison between these commits.

That doesn’t sound very simple to me, implementation-wise.  It doesn’t
sound overly complicated either, it sounds useful – but throwing out
check now because you say you just need something simple, and then
intending to make it not-so-simple later makes me raise my eyebrows.

I mean, feel free to rewrite the check script in Python, but having two
separate test infrastructures feels like a bit of waste to me.

(And you don’t have to add these new features like comparison of
performance results between commits to the check script, I think.  For
instance, you can write a wrapper script in e.g. Python or whatever that
just calls check to run the test and then processes the test result
itself.  I don’t want to force you to write more bash code, nobody wants
that, I just think that check works fine as a test launcher.)

Max

>>> diff --git a/tests/perf/block/qcow2/convert-to-encrypted 
>>> b/tests/perf/block/qcow2/convert-to-encrypted
>>> new file mode 100755
>>> index 0000000000..7a6b7b1cab
>>> --- /dev/null
>>> +++ b/tests/perf/block/qcow2/convert-to-encrypted
>>> @@ -0,0 +1,48 @@
>>> +#!/bin/bash
>>> +#
>>> +# Test qemu-img convert from raw to encrypted qcow2
>>> +#
>>> +# Copyright (c) 2019 Virtuozzo International GmbH. All rights reserved.
>>> +#
>>> +# This program is free software; you can redistribute it and/or modify
>>> +# it under the terms of the GNU General Public License as published by
>>> +# the Free Software Foundation; either version 2 of the License, or
>>> +# (at your option) any later version.
>>> +#
>>> +# This program is distributed in the hope that it will be useful,
>>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> +# GNU General Public License for more details.
>>> +#
>>> +# You should have received a copy of the GNU General Public License
>>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>> +#
>>> +
>>> +if [ "$#" -lt 2 ]; then
>>> +    echo "Usage: $0 SOURCE_FILE DESTINATION_FILE [additional qemu-img 
>>> convert parameters]"
>>> +    exit 1
>>> +fi
>>> +
>>> +ROOT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )/../../../.." >/dev/null 
>>> 2>&1 && pwd )"
>>> +QEMU_IMG="$ROOT_DIR/qemu-img"
>>> +QEMU_IO="$ROOT_DIR/qemu-io"
>>> +
>>> +size=1G
>>> +
>>> +src="$1"
>>> +shift
>>> +
>>> +dst="$1"
>>> +shift
>>> +
>>> +(
>>> +# create source
>>> +$QEMU_IMG create -f raw "$src" $size
>>> +$QEMU_IO -f raw -c "write -P 0xa 0 $size" "$src"
>>> +
>>> +# create target
>>> +$QEMU_IMG create -f qcow2 --object secret,id=sec0,data=test -o 
>>> encrypt.format=luks,encrypt.key-secret=sec0 "$dst" $size
>>> +) > /dev/null
>>> +
>>> +# test with additional parameters left in $@
>>> +/usr/bin/time -f %e $QEMU_IMG convert "$@" -f raw --object 
>>> secret,id=sec0,data=test --target-image-opts -n "$src" 
>>> "driver=qcow2,file.filename=$dst,encrypt.key-secret=sec0"
>>>
>>
>>
> 
>

signature.asc
Description: OpenPGP digital signature