From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Jones Subject: Re: [kvm-unit-tests PATCH 3/3] add timeout support Date: Tue, 22 Dec 2015 13:51:02 -0600 Message-ID: <20151222195102.GA5286@hawk.localdomain> 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> <20151222180220.GB12725@potion.redhat.com> 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: Radim =?utf-8?B?S3LEjW3DocWZ?= Return-path: Received: from mx1.redhat.com ([209.132.183.28]:46857 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751832AbbLVTvH (ORCPT ); Tue, 22 Dec 2015 14:51:07 -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 882FE693C6 for ; Tue, 22 Dec 2015 19:51:07 +0000 (UTC) Content-Disposition: inline In-Reply-To: <20151222180220.GB12725@potion.redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Tue, Dec 22, 2015 at 07:02:21PM +0100, Radim Kr=C4=8Dm=C3=A1=C5=99 w= rote: > 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 overri= de. > >=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 tim= eout > > set in unittests.cfg, or wasn't run with run()/unittests.cfg. >=20 > Tests made with mkstandalone.sh ignore the TIMEOUT variable ... >=20 > > Or, d= id > > you mean that you'd prefer TIMEOUT to override the timeout in > > unittests.cfg? >=20 > ... 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. >=20 > - per-test timeout in unittests.cfg. When the test is known to timeo= ut > often and the usual time to fail is much shorter than the global > default. (Shouldn't be used much.) >=20 > - TIMEOUT variable. It has to override the global timeout and I thin= k > 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. OK, I'll do something allowing us to easily enable a long default timeout. >=20 > (I'd probably allow something as evil as `eval`ing the TIMEOUT, for > unlikely stuff like TIMEOUT=3D'$(( ${timeout:-10} / 2 ))') I'd prefer to avoid the evil^Weval stuff... And the timeout duration ca= n already be a floating point. >=20 > > That does make some sense, in the case the one in th= e > > 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. >=20 > Ok, the difference was negligible to begin with. >=20 > >> 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 >=20 > Btw. why do we have ACCEL when the project is *kvm*_unit_tests? arm tests are sometimes tcg only. Hey, we'll take what we get for arm, as we're sadly missing everything... >=20 > > as I already have for TIMEOUT. Also, for both I should add a > > mkstandalone patch allowing > >=20 > > TIMEOUT=3D? ACCEL=3D? make standalone >=20 > I'd also handle TIMEOUT/ACCEL in resulting standalone tests. OK Thanks, drew > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html