From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40331) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WlJMV-0007iS-TB for qemu-devel@nongnu.org; Fri, 16 May 2014 10:43:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WlJMP-0004y7-Oq for qemu-devel@nongnu.org; Fri, 16 May 2014 10:43:51 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48757) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WlJMP-0004xr-HM for qemu-devel@nongnu.org; Fri, 16 May 2014 10:43:45 -0400 Message-ID: <5376241A.3050904@redhat.com> Date: Fri, 16 May 2014 16:43:38 +0200 From: Max Reitz MIME-Version: 1.0 References: <1400192774-606-1-git-send-email-mreitz@redhat.com> <1400192774-606-2-git-send-email-mreitz@redhat.com> <53754526.5090407@redhat.com> In-Reply-To: <53754526.5090407@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 1/7] iotests: Allow out-of-tree run List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , qemu-devel@nongnu.org Cc: Kevin Wolf , Peter Maydell , Markus Armbruster , Stefan Hajnoczi On 16.05.2014 00:52, Eric Blake wrote: > 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 throu= gh >> a symlink, and if so, will assume it is run in the build tree. All >> output and temporary operations performed by iotests are then redirect= ed >> 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 >> --- >> tests/qemu-iotests/check | 78 ++++++++++++++++++++++++++++++= ++++------ >> =20 >> +if [ -L "$0" ] >> +then >> + # called from the build tree >> + source_iotests=3D"$(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. I guess using something like source_iotests=3D"$(dirname "$(readlink "$0")")"; if [ -z=20 "$source_iotests" ]; then; /* abort */; fi; source_iotests=3D"$(cd=20 "$dirname"; pwd)" should work better, then? >> + 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: Hm, I remembered you telling me something about -a before and looked in=20 test's manpage but couldn't find anything bad. Well, it's the GNU manpage= =E2=80=A6 > Is [ ! '(' -o ')' ] true or false? Depends on whether it was parsed as = { > ! '(' } -o ')' (false -o true =3D> true) or as ! { '(' -o ')' } (! (tru= e > -o true) =3D> 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. If you're telling me I'm free to use bashisms, I'll believe you. :-) >> +++ b/tests/qemu-iotests/common.rc >> @@ -318,9 +318,9 @@ _do() >> status=3D1; exit >> fi >> =20 >> - (eval "echo '---' \"$_cmd\"") >>$here/$seq.full >> + (eval "echo '---' \"$_cmd\"") >>"$OUTPUT_DIR/$seq.full" >> (eval "$_cmd") >$tmp._out 2>&1; ret=3D$? > 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 nam= e. Actually, I don't even see that function ("_do") being used anywhere in=20 the iotests. We could probably drop it. But the eval should be of no harm, as it is the intention of this=20 function "_do" to execute the function given as a parameter. If anyone=20 should make sure, this eval does not execute user-controlled text, it is=20 the code using _do, I think. Max