[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] qemu-iotests: Test qcow2 image creation options
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH] qemu-iotests: Test qcow2 image creation options |
Date: |
Tue, 29 Jan 2013 17:21:40 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20120605 Thunderbird/13.0 |
Am 29.01.2013 17:12, schrieb Eric Blake:
> On 01/29/2013 03:01 AM, Kevin Wolf wrote:
>> Just create lots of images and try out each of the creation options that
>> qcow2 provides (except backing_file/fmt for now)
>>
>> I'm not totally happy with the behaviour of qemu-img in each of the
>> cases, but let's be explicit and update the test when we do change
>> things later.
>>
>> Signed-off-by: Kevin Wolf <address@hidden>
>> ---
>
>> @@ -0,0 +1,117 @@
>> +#!/bin/bash
>
> Good, because you definitely used some bash-isms (such as sizes+="more").
>
>> +# creator
>> address@hidden
>> +
>> +seq=`basename $0`
>
> Since you are already using a capable shell, why not go all the way and
> use $() instead of ``?
I would have if this wasn't only code copied from other test cases.
(Same thing below: the POSIX function is copied, the bash one is new)
> And in this case, why not:
>
> seq=${0##*/}
>
> to avoid a fork?
Because I can't read that and one fork more or less really makes no
difference. :-) (and it's copied as well)
>> +echo "QA output created by $seq"
>> +
>> +here=`pwd`
>
> Likewise, since you are using a capable shell, you can avoid a fork:
>
> here=$PWD
>
>> +tmp=/tmp/$$
>
> And since you are using a capable shell, it would be more secure to use:
>
> tmp=/tmp/$$.$RANDOM
Both of them are copied.
If you really care about any of them, you should probably submit a patch
that fixes it in all test cases - otherwise I'll again copy one of the
wrong versions next time.
Kevin