From mboxrd@z Thu Jan 1 00:00:00 1970 From: Radim =?utf-8?B?S3LEjW3DocWZ?= Subject: Re: [kvm-unit-tests PATCH 3/3] add timeout support Date: Tue, 22 Dec 2015 19:02:21 +0100 Message-ID: <20151222180220.GB12725@potion.redhat.com> References: <1450383054-9724-1-git-send-email-drjones@redhat.com> <1450383054-9724-4-git-send-email-drjones@redhat.com> <20151221170419.GA12725@potion.redhat.com> <20151221194548.GB5123@hawk.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: kvm@vger.kernel.org, pbonzini@redhat.com To: Andrew Jones Return-path: Received: from mx1.redhat.com ([209.132.183.28]:36012 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751721AbbLVSCY (ORCPT ); Tue, 22 Dec 2015 13:02:24 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id 6C25096FF for ; Tue, 22 Dec 2015 18:02:24 +0000 (UTC) Content-Disposition: inline In-Reply-To: <20151221194548.GB5123@hawk.localdomain> Sender: kvm-owner@vger.kernel.org List-ID: 2015-12-21 13:45-0600, Andrew Jones: > On Mon, Dec 21, 2015 at 06:04:20PM +0100, Radim Kr=C4=8Dm=C3=A1=C5=99= wrote: >> 2015-12-17 14:10-0600, Andrew Jones: >> > diff --git a/run_tests.sh b/run_tests.sh >> > @@ -21,6 +21,7 @@ function run() >> > + local timeout=3D"${9:-$TIMEOUT}" >> > diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh >> > @@ -97,8 +98,12 @@ if [ "\$QEMU" ]; then >> > +if [ "$timeout" ]; then >> > + timeout_cmd=3D'timeout --foreground $timeout' >>=20 >> Both would be nicer if they took the TIMEOUT variable as an override= =2E >=20 > Everything already takes TIMEOUT as an override, i.e. >=20 > TIMEOUT=3D3 ./run_tests.sh >=20 > and >=20 > TIMEOUT=3D3 arm/run arm/test.flat >=20 > will both already set a timeout for any test that didn't have a timeo= ut > set in unittests.cfg, or wasn't run with run()/unittests.cfg. Tests made with mkstandalone.sh ignore the TIMEOUT variable ... > Or, did > you mean that you'd prefer TIMEOUT to override the timeout in > unittests.cfg? =2E.. and yes, I think that we could have a - global timeout for all tests. Something far longer than any tests should take (2 minutes?). To automatically handle random hangs. - per-test timeout in unittests.cfg. When the test is known to timeout often and the usual time to fail is much shorter than the global default. (Shouldn't be used much.) - TIMEOUT variable. It has to override the global timeout and I think that if we are ever going to use it, it's because we want something weird. Like using `TIMEOUT=3D0 ./run_tests.sh` to disable all timeouts, prolonging/shortening timeouts because of a special configuration, ... Because we should improve our defaults otherwise. (I'd probably allow something as evil as `eval`ing the TIMEOUT, for unlikely stuff like TIMEOUT=3D'$(( ${timeout:-10} / 2 ))') > That does make some sense, in the case the one in the > config is longer than desired, however it also makes sense the way I > have it now when the one in the config is shorter than TIMEOUT (the > fallback default). I think I like it this way better. Ok, the difference was negligible to begin with. >> We already don't do that for accel and the patch seems ok in other >> regards, >=20 > Hmm, for accel I see a need for a patch allowing us to do >=20 > ACCEL=3D?? ./run_tests.sh Btw. why do we have ACCEL when the project is *kvm*_unit_tests? > as I already have for TIMEOUT. Also, for both I should add a > mkstandalone patch allowing >=20 > TIMEOUT=3D? ACCEL=3D? make standalone I'd also handle TIMEOUT/ACCEL in resulting standalone tests.