Re: [Qemu-devel] [PATCH v6 08/11] block: add support for --image-opts in block I/O tests

[Daniel P. Berrange]

On Mon, Mar 21, 2016 at 02:08:16PM -0600, Eric Blake wrote:
> On 03/21/2016 08:11 AM, Daniel P. Berrange wrote:
> > Currently all block tests use the traditional syntax for images
> > just specifying a filename. To support the LUKS driver without
> > resorting to JSON, the tests need to be able to use the new
> > --image-opts argument to qemu-img and qemu-io.
> > 
> > This introduces a new env variable IMGOPTSSYNTAX. If this is
> 
> Would IMG_OPTS_SYNTAX be any more legible?

[snip]

> > @@ -53,6 +53,7 @@ export QEMU_IO_OPTIONS=""
> >  export CACHEMODE_IS_DEFAULT=true
> >  export QEMU_OPTIONS="-nodefaults"
> >  export VALGRIND_QEMU=
> > +export IMGOPTSSYNTAX=false
> 
> Particularly since we use _ between words in other variables above.

It isn't visible from the diff context but just above these
quoted lines we have IMGFMT, IMGPROTO and IMGOPTS, though we
then also have the inconssitent IMGFMT_GENERIC :-)


> >  _check_test_img()
> >  {
> > -    $QEMU_IMG check "$@" -f $IMGFMT "$TEST_IMG" 2>&1 | _filter_testdir | \
> > +    (
> > +        if [ "$IMGOPTSSYNTAX" = "true" ]; then
> > +            $QEMU_IMG check --image-opts "$@" "$TEST_IMG" 2>&1
> > +        else
> > +            $QEMU_IMG check "$@" -f $IMGFMT "$TEST_IMG" 2>&1
> > +        fi
> > +    ) | _filter_testdir | \
> 
> Would '{ if ... fi; } |' be any better than a subshell?

FWIW this is the style used elsewhere in the I/O tests


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|