timeout: adjust timeout when running nested tests

Commit Message

Roger Pau Monné April 1, 2020, 1:37 p.m. UTC

Expand the timeouts when the host is nested. The current algorithm
uses base_timeout * 2 ^ nesting_level.

This fixes the issues reported by the nested tests on elbling boxes:

http://logs.test-lab.xenproject.org/osstest/logs/149283/

Signed-off-by: Roger Pau Monné <[e-mail]>
---
 Osstest/TestSupport.pm | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Ian Jackson April 1, 2020, 4:45 p.m. UTC | #1

Roger Pau Monne writes ("[PATCH] timeout: adjust timeout when running nested tests"):
>  sub target_adjust_timeout ($$) {
>      my ($ho,$timeoutref) = @_; # $ho might be a $gho
> +    my $nestinglvl = $ho->{NestingLevel} || $ho->{Host}{NestingLevel};

I think this wannts to be // not ||.  If you agree I will fix this up
and commit.

Since what this does otherwise is to take all baremetal hosts and give
them an empty Host hash due to autovivification.

> +    if ($nestinglvl) {
> +        $adjust->(1 << $nestinglvl, "nesting level");
> +    }

I still think the use of << is very odd and I can't resist moaning
about it.  But you're the patch author so I will let you choose the
style here.

Ian.

Roger Pau Monné April 1, 2020, 4:53 p.m. UTC | #2

On Wed, Apr 01, 2020 at 05:45:21PM +0100, Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH] timeout: adjust timeout when running nested tests"):
> >  sub target_adjust_timeout ($$) {
> >      my ($ho,$timeoutref) = @_; # $ho might be a $gho
> > +    my $nestinglvl = $ho->{NestingLevel} || $ho->{Host}{NestingLevel};
> 
> I think this wannts to be // not ||.  If you agree I will fix this up
> and commit.

Yes, I agree.

> Since what this does otherwise is to take all baremetal hosts and give
> them an empty Host hash due to autovivification.
> 
> > +    if ($nestinglvl) {
> > +        $adjust->(1 << $nestinglvl, "nesting level");
> > +    }
> 
> I still think the use of << is very odd and I can't resist moaning
> about it.  But you're the patch author so I will let you choose the
> style here.

Feel free to change to 2 ** $nestinglvl at commit, you are the
maintainer so it's important that you can read the code easily.

Thanks, Roger.

Ian Jackson April 1, 2020, 5:02 p.m. UTC | #3

Roger Pau Monne writes ("Re: [PATCH] timeout: adjust timeout when running nested tests"):
> On Wed, Apr 01, 2020 at 05:45:21PM +0100, Ian Jackson wrote:
> > I think this wannts to be // not ||.  If you agree I will fix this up
> > and commit.
> 
> Yes, I agree.

Thanks, done and pushed.

> > Since what this does otherwise is to take all baremetal hosts and give
> > them an empty Host hash due to autovivification.
> > 
> > > +    if ($nestinglvl) {
> > > +        $adjust->(1 << $nestinglvl, "nesting level");
> > > +    }
> > 
> > I still think the use of << is very odd and I can't resist moaning
> > about it.  But you're the patch author so I will let you choose the
> > style here.
> 
> Feel free to change to 2 ** $nestinglvl at commit, you are the
> maintainer so it's important that you can read the code easily.

I'll keep the version you tested rather than messing about with it...

Ian.

Patch

diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index 1c13e2af..ff749a32 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -436,6 +436,7 @@  END
 
 sub target_adjust_timeout ($$) {
     my ($ho,$timeoutref) = @_; # $ho might be a $gho
+    my $nestinglvl = $ho->{NestingLevel} || $ho->{Host}{NestingLevel};
     my $adjust = sub {
 	my ($factor, $what) = @_;
 	return unless defined $factor;
@@ -450,6 +451,9 @@  sub target_adjust_timeout ($$) {
 	$adjust->(guest_var($ho,$guest_var), "guest variable $guest_var");
     }
     $adjust->(get_target_property($ho,"TimeoutFactor"), "target TimeoutFactor");
+    if ($nestinglvl) {
+        $adjust->(1 << $nestinglvl, "nesting level");
+    }
 }
 
 #---------- running commands eg on targets ----------