qemu-devel
[Top][All Lists]
Advanced

[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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]