[{"id":1767338,"web_url":"http://patchwork.ozlabs.org/comment/1767338/","msgid":"<7d63b366-37a4-bfbe-790b-9a1ceb0d2893@redhat.com>","list_archive_url":null,"date":"2017-09-12T21:11:42","subject":"Re: [Qemu-devel] [PATCH 04/10] qemu-iotests: cleanup and fix search\n\tfor programs","submitter":{"id":6591,"url":"http://patchwork.ozlabs.org/api/people/6591/","name":"Eric Blake","email":"eblake@redhat.com"},"content":"On 09/12/2017 09:44 AM, Paolo Bonzini wrote:\n> Instead of ./check failing when a binary is missing, we try each test\n> case now and each one fails with tons of test case diffs. Also, all the\n> variables were initialized by \"check\" prior to \"common\" being sourced,\n> and then (uselessly) checked for emptiness again in \"check\".\n> \n> Centralize the search for programs in \"common\" (which will soon be\n> one with \"check\"), including the \"realpath\" invocation which can be done\n> just once in \"check\" rather than in the tests.\n> \n> For qnio_server, move the detection to \"common\", simplifying\n> set_prog_path to stop handling the unused second argument, and\n> embedding the \"realpath\" pass.\n> \n> Signed-off-by: Paolo Bonzini \n> ---\n> tests/qemu-iotests/check | 41 ---------------------\n> tests/qemu-iotests/common | 77 +++++++++++++++++++++++++++++++++++++---\n> tests/qemu-iotests/common.config | 61 +------------------------------\n> 3 files changed, 73 insertions(+), 106 deletions(-)\n> \n\n> +++ b/tests/qemu-iotests/common\n> @@ -37,6 +37,17 @@ _full_platform_details()\n> echo \"$os/$platform $host $kernel\"\n> }\n> \n> +# $1 = prog to look for\n> +set_prog_path()\n> +{\n> + p=`command -v $1 2> /dev/null`\n\nAre we sure that $1 will always respond to -v sanely?\n\n> + if [ -n \"$p\" -a -x \"$p\" ]; then\n\n[ ... -a ... ] should never be used; there are cases where it is\ninherently ambiguous (even if it happens to work here, because we know\nour shell is bash and because $p is not likely to contain text that\nthrows off the parser). Please spell that either:\n\nif [ -n \"$p\" ] && [ -x \"$p\" ]; then\nif [[ -n $p && -x $p ]]; then\n\n(that is, I don't mind if you rely on bash's [[]] in which case you can\nuse less typing, but if you stick to [], then you should use code that\ncan be pasted to other shells)\n\n> +then\n> + if [ -x \"$build_iotests/qemu\" ]; then\n> + export QEMU_PROG=\"$build_iotests/qemu\"\n> + elif [ -x \"$build_root/$arch-softmmu/qemu-system-$arch\" ]; then\n> + export QEMU_PROG=\"$build_root/$arch-softmmu/qemu-system-$arch\"\n> + else\n> + pushd \"$build_root\" > /dev/null\n\nShouldn't you check for failure to change directories?\n\n> +++ b/tests/qemu-iotests/common.config\n> @@ -22,6 +22,7 @@ export LANG=C\n> PATH=\".:$PATH\"\n> \n> HOSTOS=`uname -s`\n> +arch=`uname -m`\n\nAs long as we're touching this, should we prefer $() instead of ``?\n\n> \n> export PWD=`pwd`\n> \n> @@ -30,28 +31,6 @@ export _QEMU_HANDLE=0\n> # make sure we have a standard umask\n> umask 022\n> \n> -# $1 = prog to look for, $2* = default pathnames if not found in $PATH\n> -set_prog_path()\n> -{\n> - p=`command -v $1 2> /dev/null`\n> - if [ -n \"$p\" -a -x \"$p\" ]; then\n\nUrrgh - your use of [ -a ] was pre-existing, and you just moved it.\nStill worth fixing, though (either here or in a separate cleanup patch).","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx09.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx09.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=eblake@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xsHYf55Yvz9t2Z\n\tfor ;\n\tWed, 13 Sep 2017 07:12:22 +1000 (AEST)","from localhost ([::1]:38634 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1drsTk-000858-Op\n\tfor incoming@patchwork.ozlabs.org; Tue, 12 Sep 2017 17:12:20 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:56303)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1drsTH-000832-Br\n\tfor qemu-devel@nongnu.org; Tue, 12 Sep 2017 17:11:52 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1drsTF-0003mK-Tq\n\tfor qemu-devel@nongnu.org; Tue, 12 Sep 2017 17:11:51 -0400","from mx1.redhat.com ([209.132.183.28]:52518)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1drsTB-0003hW-5M; Tue, 12 Sep 2017 17:11:45 -0400","from smtp.corp.redhat.com\n\t(int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 0893A4ACA5;\n\tTue, 12 Sep 2017 21:11:44 +0000 (UTC)","from [10.10.120.44] (ovpn-120-44.rdu2.redhat.com [10.10.120.44])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 3C69461989;\n\tTue, 12 Sep 2017 21:11:43 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 0893A4ACA5","To":"Paolo Bonzini , qemu-devel@nongnu.org","References":"<20170912144459.11359-1-pbonzini@redhat.com>\n\t<20170912144459.11359-5-pbonzini@redhat.com>","From":"Eric Blake ","Openpgp":"url=http://people.redhat.com/eblake/eblake.gpg","Organization":"Red Hat, Inc.","Message-ID":"<7d63b366-37a4-bfbe-790b-9a1ceb0d2893@redhat.com>","Date":"Tue, 12 Sep 2017 16:11:42 -0500","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170912144459.11359-5-pbonzini@redhat.com>","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"Mcm8lEoXGD9SN7ThmrEPf0A2bkLUlkgi7\"","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.12","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.38]);\n\tTue, 12 Sep 2017 21:11:44 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","X-Content-Filtered-By":"Mailman/MimeDel 2.1.21","Subject":"Re: [Qemu-devel] [PATCH 04/10] qemu-iotests: cleanup and fix search\n\tfor programs","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"kwolf@redhat.com, qemu-block@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1767348,"web_url":"http://patchwork.ozlabs.org/comment/1767348/","msgid":"<1767007484.5449466.1505251616671.JavaMail.zimbra@redhat.com>","list_archive_url":null,"date":"2017-09-12T21:26:56","subject":"Re: [Qemu-devel] [PATCH 04/10] qemu-iotests: cleanup and fix search\n\tfor programs","submitter":{"id":2701,"url":"http://patchwork.ozlabs.org/api/people/2701/","name":"Paolo Bonzini","email":"pbonzini@redhat.com"},"content":"> > +then\n> > + if [ -x \"$build_iotests/qemu\" ]; then\n> > + export QEMU_PROG=\"$build_iotests/qemu\"\n> > + elif [ -x \"$build_root/$arch-softmmu/qemu-system-$arch\" ]; then\n> > + export QEMU_PROG=\"$build_root/$arch-softmmu/qemu-system-$arch\"\n> > + else\n> > + pushd \"$build_root\" > /dev/null\n> \n> Shouldn't you check for failure to change directories?\n> \n> > export PWD=`pwd`\n> > \n> > @@ -30,28 +31,6 @@ export _QEMU_HANDLE=0\n> > # make sure we have a standard umask\n> > umask 022\n> > \n> > -# $1 = prog to look for, $2* = default pathnames if not found in $PATH\n> > -set_prog_path()\n> > -{\n> > - p=`command -v $1 2> /dev/null`\n> > - if [ -n \"$p\" -a -x \"$p\" ]; then\n> \n> Urrgh - your use of [ -a ] was pre-existing, and you just moved it.\n\nAnd same for pushd...\n\nPaolo","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx07.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx07.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=pbonzini@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xsHw71FkNz9t2W\n\tfor ;\n\tWed, 13 Sep 2017 07:28:23 +1000 (AEST)","from localhost ([::1]:38688 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1drsjF-0008Mu-6A\n\tfor incoming@patchwork.ozlabs.org; Tue, 12 Sep 2017 17:28:21 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:36661)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1drsi0-0007i0-Aw\n\tfor qemu-devel@nongnu.org; Tue, 12 Sep 2017 17:27:05 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1drshz-0008KF-GQ\n\tfor qemu-devel@nongnu.org; Tue, 12 Sep 2017 17:27:04 -0400","from mx1.redhat.com ([209.132.183.28]:60380)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1drshu-0008Ec-0s; Tue, 12 Sep 2017 17:26:58 -0400","from smtp.corp.redhat.com\n\t(int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 21E0BC04BD2C;\n\tTue, 12 Sep 2017 21:26:57 +0000 (UTC)","from colo-mx.corp.redhat.com\n\t(colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 0BF535D6A6;\n\tTue, 12 Sep 2017 21:26:57 +0000 (UTC)","from zmail18.collab.prod.int.phx2.redhat.com\n\t(zmail18.collab.prod.int.phx2.redhat.com [10.5.83.21])\n\tby colo-mx.corp.redhat.com (Postfix) with ESMTP id 02AB93FC71;\n\tTue, 12 Sep 2017 21:26:57 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 21E0BC04BD2C","Date":"Tue, 12 Sep 2017 17:26:56 -0400 (EDT)","From":"Paolo Bonzini ","To":"Eric Blake ","Message-ID":"<1767007484.5449466.1505251616671.JavaMail.zimbra@redhat.com>","In-Reply-To":"<7d63b366-37a4-bfbe-790b-9a1ceb0d2893@redhat.com>","References":"<20170912144459.11359-1-pbonzini@redhat.com>\n\t<20170912144459.11359-5-pbonzini@redhat.com>\n\t<7d63b366-37a4-bfbe-790b-9a1ceb0d2893@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"7bit","X-Originating-IP":"[78.12.246.117, 10.4.196.25, 10.4.195.5]","Thread-Topic":"qemu-iotests: cleanup and fix search for programs","Thread-Index":"ZznNnfR7guxzzYS/HemtRN9MDX05mw==","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.15","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.31]);\n\tTue, 12 Sep 2017 21:26:57 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH 04/10] qemu-iotests: cleanup and fix search\n\tfor programs","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"kwolf@redhat.com, qemu-devel@nongnu.org, qemu-block@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1767374,"web_url":"http://patchwork.ozlabs.org/comment/1767374/","msgid":"<732c98f0-a771-ef6f-e674-a11366653162@redhat.com>","list_archive_url":null,"date":"2017-09-12T21:34:55","subject":"Re: [Qemu-devel] [PATCH 04/10] qemu-iotests: cleanup and fix search\n\tfor programs","submitter":{"id":6591,"url":"http://patchwork.ozlabs.org/api/people/6591/","name":"Eric Blake","email":"eblake@redhat.com"},"content":"On 09/12/2017 04:26 PM, Paolo Bonzini wrote:\n> \n> \n>>> +then\n>>> + if [ -x \"$build_iotests/qemu\" ]; then\n>>> + export QEMU_PROG=\"$build_iotests/qemu\"\n>>> + elif [ -x \"$build_root/$arch-softmmu/qemu-system-$arch\" ]; then\n>>> + export QEMU_PROG=\"$build_root/$arch-softmmu/qemu-system-$arch\"\n>>> + else\n>>> + pushd \"$build_root\" > /dev/null\n>>\n>> Shouldn't you check for failure to change directories?\n>> \n>>> export PWD=`pwd`\n>>> \n>>> @@ -30,28 +31,6 @@ export _QEMU_HANDLE=0\n>>> # make sure we have a standard umask\n>>> umask 022\n>>> \n>>> -# $1 = prog to look for, $2* = default pathnames if not found in $PATH\n>>> -set_prog_path()\n>>> -{\n>>> - p=`command -v $1 2> /dev/null`\n>>> - if [ -n \"$p\" -a -x \"$p\" ]; then\n>>\n>> Urrgh - your use of [ -a ] was pre-existing, and you just moved it.\n> \n> And same for pushd...\n\nTrue - since this patch is just code motion, we can do the cleanups in a\ndifferent patch, so you can have:\nReviewed-by: Eric Blake ","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx03.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx03.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=eblake@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xsJ6r5tWbz9t39\n\tfor ;\n\tWed, 13 Sep 2017 07:37:39 +1000 (AEST)","from localhost ([::1]:38739 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1drssC-0006pz-3D\n\tfor incoming@patchwork.ozlabs.org; Tue, 12 Sep 2017 17:37:36 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:40363)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1drspl-0004ui-4w\n\tfor qemu-devel@nongnu.org; Tue, 12 Sep 2017 17:35:07 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1drspk-0005OP-5x\n\tfor qemu-devel@nongnu.org; Tue, 12 Sep 2017 17:35:05 -0400","from mx1.redhat.com ([209.132.183.28]:34642)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1drspe-0005HN-6J; Tue, 12 Sep 2017 17:34:58 -0400","from smtp.corp.redhat.com\n\t(int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 3D98980F7A;\n\tTue, 12 Sep 2017 21:34:57 +0000 (UTC)","from [10.10.120.44] (ovpn-120-44.rdu2.redhat.com [10.10.120.44])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 54CD85D6A9;\n\tTue, 12 Sep 2017 21:34:56 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 3D98980F7A","To":"Paolo Bonzini ","References":"<20170912144459.11359-1-pbonzini@redhat.com>\n\t<20170912144459.11359-5-pbonzini@redhat.com>\n\t<7d63b366-37a4-bfbe-790b-9a1ceb0d2893@redhat.com>\n\t<1767007484.5449466.1505251616671.JavaMail.zimbra@redhat.com>","From":"Eric Blake ","Openpgp":"url=http://people.redhat.com/eblake/eblake.gpg","Organization":"Red Hat, Inc.","Message-ID":"<732c98f0-a771-ef6f-e674-a11366653162@redhat.com>","Date":"Tue, 12 Sep 2017 16:34:55 -0500","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<1767007484.5449466.1505251616671.JavaMail.zimbra@redhat.com>","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"KKkSQiAhkoV5KTGhTl84ogu1KNxDTxI3f\"","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.15","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.27]);\n\tTue, 12 Sep 2017 21:34:57 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","X-Content-Filtered-By":"Mailman/MimeDel 2.1.21","Subject":"Re: [Qemu-devel] [PATCH 04/10] qemu-iotests: cleanup and fix search\n\tfor programs","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"kwolf@redhat.com, qemu-devel@nongnu.org, qemu-block@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]