[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 7/7] qemu-iotests: add 039 qcow2 lazy refcoun

From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH v2 7/7] qemu-iotests: add 039 qcow2 lazy refcounts test
Date: Wed, 25 Jul 2012 17:41:07 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:14.0) Gecko/20120717 Thunderbird/14.0

On 07/25/2012 04:45 PM, Stefan Hajnoczi wrote:

>> Since you are assuming bash (and even if you were to assume POSIX
>> /bin/sh)...
>>> +
>>> +seq=`basename $0`
>> I prefer $() over ``.
>>> +echo "QA output created by $seq"
>>> +
>>> +here=`pwd`
>> POSIX (and therefore bash) guarantees that $PWD is sane, and faster to
>> access than $(pwd).
>>> +tmp=/tmp/$$
>> That's not very secure.  It may be worth using bash's $RANDOM, or using
>> mkstemp(1).
>> Beyond that, the series seemed reasonable to me.
> All qemu-iotests scripts do these things in the same way and I'd like
> for them to be consistent.

Good argument.

> If we make these changes they should be applied to all qemu-iotests
> scripts.  I agree with your points but also think the value in making
> the change now is small.

Indeed - what you have is technically correct, even if not the most
efficient.  Any such cleanups should, as you say, be a separate patch
globally applied to the qemu-iotests, and not this test in isolation.

> Do you want to send a patch that fixes these issues in qemu-iotests?

Up to you; or read another way, it bothered me enough to comment, but
not enough to write the patch myself, so I'm fine living with status quo
if it doesn't bother anyone else either.  Old-school techniques aren't
wrong, per se, just inefficient; and while the insecure temp file name
could be exploited, people running the testsuite tend to be on personal
platforms rather than enterprise systems, and the cost of exploiting a
testsuite is not as severe as the cost of exploiting an installed script.

Eric Blake   address@hidden    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

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