Re: [Qemu-devel] [PATCH 1/7] iotests: Allow out-of-tree run

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2254 bytes --]

On 05/15/2014 04:26 PM, Max Reitz wrote:
> As out-of-tree builds are preferred for qemu, running the qemu-iotests
> in that out-of-tree build should be supported as well. To do so, a
> symbolic link has to be created pointing to the check script in the
> source directory. That script will check whether it has been run through
> a symlink, and if so, will assume it is run in the build tree. All
> output and temporary operations performed by iotests are then redirected
> here and, unless specified otherwise by the user, QEMU_PROG etc. will be
> set to paths appropriate for the build tree.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  tests/qemu-iotests/check         | 78 ++++++++++++++++++++++++++++++++++------

>  
> +if [ -L "$0" ]
> +then
> +    # called from the build tree
> +    source_iotests="$(cd "$(dirname "$(readlink "$0")")"; pwd)"

This is potentially dangerous. If readlink or dirname fails, you can
invoke cd "" (which on bash is stupidly a no-op instead of an error),
and end up calling pwd in the wrong directory.  But in the common case
it works, so I'm not sure it's worth bending over backwards to make it
more robust.

> +        if [ -n "$arch" -a -x "$build_root/$arch-softmmu/qemu-system-$arch" ]

-a and -o are NOT portable inside []; POSIX strongly discourages their
use because they can cause ambiguous parses:

Is [ ! '(' -o ')' ] true or false? Depends on whether it was parsed as {
! '(' } -o ')' (false -o true => true) or as ! { '(' -o ')' } (! (true
-o true) => false)

But this is bash, so you could do:

if [[ $arch && -x $build_root/$arch-softmmu/qemu-system-$arch ]]

for less typing, and no risk of [] ambiguity.


> +++ b/tests/qemu-iotests/common.rc
> @@ -318,9 +318,9 @@ _do()
>          status=1; exit
>      fi
>  
> -    (eval "echo '---' \"$_cmd\"") >>$here/$seq.full
> +    (eval "echo '---' \"$_cmd\"") >>"$OUTPUT_DIR/$seq.full"
>      (eval "$_cmd") >$tmp._out 2>&1; ret=$?

Pre-existing, but we're using 'eval'?  That's probably a security risk
if $_cmd can contain user-controlled text, such as an odd directory name.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]