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

[Stefan Hajnoczi]

On Wed, Jul 25, 2012 at 11:54:50AM -0600, Eric Blake wrote:
> On 07/25/2012 06:21 AM, Stefan Hajnoczi wrote:
> > This tests establishes the basic post-conditions of the qcow2 lazy
> > refcounts features:
> > 
> >   1. If the image was closed normally, it is marked clean.
> > 
> >   2. If an allocating write was performed and the image was not close
> >      normally, then it is marked dirty.
> > 
> >      a. Written data can be read back successfully.
> >      b. The image file can be repaired and will be marked clean again.
> > 
> > Signed-off-by: Stefan Hajnoczi <address@hidden>
> 
> > +++ b/tests/qemu-iotests/039
> > @@ -0,0 +1,117 @@
> > +#!/bin/bash
> 
> 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.

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.

Do you want to send a patch that fixes these issues in qemu-iotests?
The general shell scripting style used there is quite old school and
makes use of backquotes often.

Stefan