[PATCH OSSTEST v2 0/4] Avoid running Linux on hosts for the given version lacks drivers

[PATCH OSSTEST v2 0/4] Avoid running Linux on hosts for the given version lacks drivers

[Ian Campbell]

As discussed in the thread at 
http://lists.xen.org/archives/html/xen-devel/2015-09/msg01067.html some
older versions of Linux lack drivers for hardware in some of our test
hosts, meaning in particular that the linux-3.4 branch is now stuck trying
to run all of its jobs on hosts which linux-3.4.y cannot possibly work on
and for which a backported driver is thought unlikely right now.

Fix this by introducing support to the resource allocator for filtering
candidates based on resource properties, which will be used specifically to
filter hosts by their LinuxKernelMin property.

Before applying this the following new host properties should be added:

./mg-hosts setprops chardonnay\* -- LinuxKernelMin 3.8
./mg-hosts setprops huxelrebe\* -- LinuxKernelMin 3.5

(strictly speaking chardonnay is actually fixed by 3.7-rc1).

The first patch was just for debugging, via the script attached to v1 and
some spurious host properties added in Cambridge. This could be dropped...

The full diff of ./standalone-generate-dump-flight-runvars is huge, a
representative hunk after sorting both files is:

@@ -501,7 +501,7 @@
 linux-3.0                  build-i386-xsm                                        tree_qemuu                  git://xenbits.xen.org/staging/qemu-upstream-unstable.git                                           
 linux-3.0                  build-i386-xsm                                        tree_seabios                                                                                                                   
 linux-3.0                  build-i386-xsm                                        tree_xen                    git://xenbits.xen.org/xen.git                                                                      
-linux-3.0                  test-amd64-amd64-amd64-pvgrub                         all_hostflags               arch-amd64,arch-xen-amd64,suite-jessie,purpose-test                                                
+linux-3.0                  test-amd64-amd64-amd64-pvgrub                         all_hostflags               arch-amd64,arch-xen-amd64,suite-jessie,purpose-test,PropMinVer:LinuxKernelMin:3.0                  
 linux-3.0                  test-amd64-amd64-amd64-pvgrub                         arch                        amd64                                                                                              
 linux-3.0                  test-amd64-amd64-amd64-pvgrub                         buildjob                    build-amd64                                                                                        
 linux-3.0                  test-amd64-amd64-amd64-pvgrub                         debian_arch                 amd64                                                                                              

This only occurs for linux-X.Y and not for other branches (including not
for non-numeric linux-FOO):

$ diff -u  before-min-linux after-min-linux  | grep ^[+-][^+-]| cut -f1 -d\  | sort | uniq
-linux-3.0
+linux-3.0
-linux-3.10
+linux-3.10
-linux-3.14
+linux-3.14
-linux-3.16
+linux-3.16
-linux-3.18
+linux-3.18
-linux-3.4
+linux-3.4
-linux-4.1
+linux-4.1

Ian.

[PATCH OSSTEST v2 1/4] ts-hosts-allocate-Executive: Allow dry-run

[Ian Campbell]

Provide a new -n command line option which causes no allocations to be
done, for debugging.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 ts-hosts-allocate-Executive | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/ts-hosts-allocate-Executive b/ts-hosts-allocate-Executive
index 781ddaf..c9aae88 100755
--- a/ts-hosts-allocate-Executive
+++ b/ts-hosts-allocate-Executive
@@ -30,6 +30,7 @@ tsreadconfig();
 open DEBUG, ">/dev/null" or die $!;
 
 our $compressdebug=1;
+our $fake; # never allocate
 
 while (@ARGV and $ARGV[0] =~ m/^-/) {
     $_= shift @ARGV;
@@ -37,6 +38,8 @@ while (@ARGV and $ARGV[0] =~ m/^-/) {
     while (m/^-./) {
         if (s/^-U/-/) {
 	    $compressdebug=0;
+	} elsif (s/^-n/-/) {
+	    $fake=1;
         } else {
             die "$_ ?";
         }
@@ -610,6 +613,8 @@ sub attempt_allocation {
     ($plan, $mayalloc) = @_;
     undef $best;
 
+    $mayalloc = undef if $fake;
+
     logm("allocating hosts: ".join(' ', map { $_->{Ident} } @hids));
 
     prepare_statements();
@@ -654,6 +659,11 @@ sub attempt_allocation {
 	logm("host allocation: planned start in $best->{Start} seconds.");
     }
 
+    if ($fake) {
+	logm("Fake allocation, returning error");
+	$retval = -1;
+    }
+
     my $booklist= compute_booking_list();
 
     return ($retval, $booklist);
-- 
2.5.1

[PATCH OSSTEST v2 2/4] ts-hosts-allocate-Executive: add a label to loop over candidates

[Ian Campbell]

I'm going to want to "next CANDIDATE" from within a nested loop.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 ts-hosts-allocate-Executive | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/ts-hosts-allocate-Executive b/ts-hosts-allocate-Executive
index c9aae88..294395d 100755
--- a/ts-hosts-allocate-Executive
+++ b/ts-hosts-allocate-Executive
@@ -313,6 +313,7 @@ END
     my @candidates;
     my $any=0;
 
+  CANDIDATE:
     while (my $candrow= $findhostsq->fetchrow_hashref()) {
         $candrow->{Warnings}= [ ];
         $candrow->{Reso}= "$candrow->{restype} $candrow->{resname}";
@@ -343,7 +344,7 @@ END
 	    my $erow= $equivflagscheckq->fetchrow_hashref();
 	    if (!$erow) {
 		print DEBUG "$dbg EQUIV $equiv->{FormalClass} NO-CLASSES\n";
-		next;
+		next CANDIDATE;
 	    }
 	    my $eq= $erow->{hostflag};
 	    print DEBUG "$dbg EQUIV $equiv->{FormalClass} MAYBE $eq\n";
@@ -358,7 +359,7 @@ END
 
         print DEBUG "$dbg FLAGS MISSINGFLAGS: @missingflags.\n";
         if (@missingflags) {
-            next unless defined $use;
+            next CANDIDATE unless defined $use;
             push @{ $candrow->{Warnings} },
                 "specified host lacks flags @missingflags";
         }
-- 
2.5.1

[PATCH OSSTEST v2 3/4] Add support for selecting resources based on their properties.

[Ian Campbell]

In particular for allocating hosts based on host properties.

To do this we extend the hostflags syntax with "condition:arg1:arg2".
This specifies that the candidate host must pass the condition given
the arguments.

Each "condition" is a new module in the Osstest::ResourceCondition
namespace. For each condition an object is constructed using the given
arguments (split on ':') and stored in $hid.

When allocating for each candidate host the object's ->check method is
called giving $restype and $resname and will return true or false
depending on whether the given host meets the condition.

Only a single condition is implemented here "PropMinVer" which
requires that a given property on the resource has at least the given
value when compared as a version string. Enforce that the database and
the resource property both use the canonical CamelCase naming through
the use of the newly added here propname_check function. Lack of the
property being compared is taken a "no restriction" and hence is
allowed.

Osstest::cfgvar_re is exported for use in the new propname_check
function.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v2:
  - ">=(v)" in stringify.
  - s/required minimum/maximum minimum/
  - Require cond name to match \w+ (avoiding spaces or other meta character
    magic and associated pitfalls)
  - Spaces inside various @{ ... }
  - Add and use propname_check
---
 Osstest.pm                              |  6 +--
 Osstest/ResourceCondition/PropMinVer.pm | 77 +++++++++++++++++++++++++++++++++
 Osstest/TestSupport.pm                  | 16 ++++++-
 ts-hosts-allocate-Executive             | 25 +++++++++--
 4 files changed, 117 insertions(+), 7 deletions(-)
 create mode 100644 Osstest/ResourceCondition/PropMinVer.pm

diff --git a/Osstest.pm b/Osstest.pm
index 1fe16fc..4a763c6 100644
--- a/Osstest.pm
+++ b/Osstest.pm
@@ -40,7 +40,7 @@ BEGIN {
                       db_begin_work db_prepare
                       ensuredir get_filecontents_core_quiet system_checked
                       nonempty visible_undef show_abs_time
-                      %arch_debian2xen %arch_xen2debian
+                      %arch_debian2xen %arch_xen2debian $cfgvar_re
                       );
     %EXPORT_TAGS = ( );
 
@@ -62,6 +62,8 @@ our %arch_debian2xen = qw(i386 x86_32
 our %arch_xen2debian;
 $arch_xen2debian{$arch_debian2xen{$_}} = $_ foreach keys %arch_debian2xen;
 
+our $cfgvar_re = '[A-Z][0-9a-zA-Z-_]*';
+
 #---------- static default config settings ----------
 
 our %c = qw(
@@ -121,8 +123,6 @@ sub readglobalconfig () {
     $c{AuthorizedKeysFiles} = '';
     $c{AuthorizedKeysAppend} = '';
 
-    my $cfgvar_re = '[A-Z][0-9a-zA-Z-_]*';
-
     my $cfgfiles = $ENV{'OSSTEST_CONFIG'} || "$ENV{'HOME'}/.xen-osstest/config";
 
     my $readcfg;
diff --git a/Osstest/ResourceCondition/PropMinVer.pm b/Osstest/ResourceCondition/PropMinVer.pm
new file mode 100644
index 0000000..155aefb
--- /dev/null
+++ b/Osstest/ResourceCondition/PropMinVer.pm
@@ -0,0 +1,77 @@
+# This is part of "osstest", an automated testing framework for Xen.
+# Copyright (C) 2015 Citrix Inc.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package Osstest::ResourceCondition::PropMinVer;
+
+use strict;
+use warnings;
+
+use Osstest;
+use Osstest::TestSupport;
+
+use Sort::Versions;
+
+use overload '""' => 'stringify';
+
+BEGIN {
+    use Exporter ();
+    our ($VERSION, @ISA, @EXPORT, @EXPORT_OK, %EXPORT_TAGS);
+    $VERSION     = 1.00;
+    @ISA         = qw(Exporter);
+    @EXPORT      = qw();
+    %EXPORT_TAGS = ( );
+
+    @EXPORT_OK   = qw();
+}
+
+sub new {
+    my ($class, $name, $prop, $val) = @_;
+
+    die "propname: $prop?" unless propname_check($prop);
+
+    return bless {
+	Prop => $prop,
+	MinVal => $val
+    }, $class;
+}
+
+sub stringify {
+    my ($pmv) = @_;
+    return "$pmv->{MinVal} >=(v) property $pmv->{Prop}";
+}
+
+sub check {
+    my ($pmv, $restype, $resname) = @_;
+
+    # Using _cached avoids needing to worry about $dbh_tests being
+    # closed/reopened between invocations
+    my $hpropq = $dbh_tests->prepare_cached(<<END);
+       SELECT val FROM resource_properties
+	WHERE restype = ? AND resname = ? AND name = ?
+END
+    $hpropq->execute($restype, $resname, $pmv->{Prop});
+
+    my $row= $hpropq->fetchrow_arrayref();
+    $hpropq->finish();
+
+    return 1 unless $row; # No prop == no restriction.
+
+    # If the maximum minimum is >= to the resource's minimum then the
+    # resource meets the requirement.
+    return versioncmp($pmv->{MinVal}, $row->[0]) >= 0;
+}
+
+1;
diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index 0afaeab..8032fac 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -71,7 +71,7 @@ BEGIN {
                       get_target_property get_host_native_linux_console
                       power_state power_cycle power_cycle_sleep
                       serial_fetch_logs
-                      propname_massage
+                      propname_massage propname_check
          
                       get_stashed open_unique_stashfile compress_stashed
                       dir_identify_vcs build_clone built_stash built_stash_file
@@ -933,6 +933,20 @@ sub propname_massage ($) {
     return $prop;
 }
 
+sub propname_check ($) {
+    # ensure propname is valid and in the canonical WordWordWord form
+    # (rather than, say, one of the legacy forms described in
+    # propname_massage).
+    my ($prop) = @_;
+
+    # Check only valid characters
+    return 0 if $prop !~ m|^${cfgvar_re}$|;
+    # Check canonical form
+    return 0 if propname_massage($prop) ne $prop;
+    # Is ok
+    return 1;
+}
+
 # It is fine to call this on a guest object too, in which case it will
 # always return $defval.
 sub get_host_property ($$;$) {
diff --git a/ts-hosts-allocate-Executive b/ts-hosts-allocate-Executive
index 294395d..35a9347 100755
--- a/ts-hosts-allocate-Executive
+++ b/ts-hosts-allocate-Executive
@@ -213,7 +213,7 @@ sub compute_hids () {
     our %equivs;
 
     foreach my $ident (@ARGV) {
-        my $hid= { };
+        my $hid= { Conds => { host => [] } };
         my $override_use;
         if ($ident =~ m/\=/) {
             $hid->{OverrideUse}= $'; #'
@@ -251,11 +251,23 @@ sub compute_hids () {
                 my $equiv= $hid->{Equiv}= $equivs{$formalclass};
                 print DEBUG "HID $ident FLAG $flag EQUIV $equiv->{Wanted}\n";
                 next;
-            }
+            } elsif ($flag =~ m/^\w+:/) {
+		my (@c) = split /:/, $flag;
+		my $o;
+		eval ("use Osstest::ResourceCondition::$c[0];".
+		      "\$o = Osstest::ResourceCondition::$c[0]->new(\@c);")
+		    or die "get ResourceCondition $@";
+
+		push @{ $hid->{Conds}{host} }, $o;
+
+		print DEBUG "HID $ident FLAG $flag HCOND $o\n";
+		next;
+	    }
             $flags{$flag}= 1;
         }
         $hid->{Flags}= \%flags;
-        print DEBUG "HID $ident FLAGS ".(join ',', sort keys %flags)."\n";
+        print DEBUG "HID $ident FLAGS ".(join ',', sort keys %flags).
+	    " + ".scalar @{ $hid->{Conds}{host} }." host conditions(s)\n";
         push @hids, $hid;
     }
 }
@@ -363,6 +375,13 @@ END
             push @{ $candrow->{Warnings} },
                 "specified host lacks flags @missingflags";
         }
+
+	foreach my $c (@{ $hid->{Conds}{$candrow->{restype}} }) {
+	    if (!$c->check($candrow->{restype}, $candrow->{resname})) {
+		print DEBUG "$dbg failed $c condition\n";
+		next CANDIDATE unless defined $use;
+	    }
+	}
         $any++;
 
         print DEBUG "$dbg GOOD\n";
-- 
2.5.1

[PATCH OSSTEST v2 4/4] make-flight: Add a minimum linux version requirement to all linux-* branches

[Ian Campbell]

We have some hosts in the colo which are not supported by older Linux
versions.

Add a suitable hostflag using the new resource conditions syntax to
cause this to occur.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 mfi-common | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/mfi-common b/mfi-common
index 991baf5..212a41d 100644
--- a/mfi-common
+++ b/mfi-common
@@ -365,6 +365,14 @@ test_matrix_iterate () {
   *)                    pairtoolstack="xl libvirt" ;;
   esac
 
+  case "$branch" in
+    linux-[3-9].[0-9]*)
+      min_linux_hostflag=PropMinVer:LinuxKernelMin:${branch#linux-}
+      ;;
+    *)
+      ;;
+  esac
+
   for xenarch in ${TEST_ARCHES- i386 amd64 armhf } ; do
 
     if [ "x$xenarch" = xdisable ]; then continue; fi
@@ -436,6 +444,9 @@ test_matrix_iterate () {
         fi
 
         most_hostflags="arch-$dom0arch,arch-xen-$xenarch,suite-$suite,purpose-test"
+        if [ "x$min_linux_hostflag" != "x" ] ; then
+            most_hostflags="$most_hostflags,$min_linux_hostflag"
+        fi
 
         most_runvars="
                   arch=$dom0arch                                  \
-- 
2.5.1

Re: [PATCH OSSTEST v2 3/4] Add support for selecting resources based on their properties.

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST v2 3/4] Add support for selecting resources based on their properties."):
> In particular for allocating hosts based on host properties.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>


> +    die "propname: $prop?" unless propname_check($prop);
...
> +sub propname_check ($) {
> +    # ensure propname is valid and in the canonical WordWordWord form
> +    # (rather than, say, one of the legacy forms described in
> +    # propname_massage).

As a matter of taste, I would have had propname_check throw rather
than making it be a predicate.

Ian.

Re: [PATCH OSSTEST v2 3/4] Add support for selecting resources based on their properties.

[Ian Campbell]

On Wed, 2015-09-16 at 14:50 +0100, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH OSSTEST v2 3/4] Add support for selecting
> resources based on their properties."):
> > In particular for allocating hosts based on host properties.
> 
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> 
> 
> > +    die "propname: $prop?" unless propname_check($prop);
> ...
> > +sub propname_check ($) {
> > +    # ensure propname is valid and in the canonical WordWordWord form
> > +    # (rather than, say, one of the legacy forms described in
> > +    # propname_massage).
> 
> As a matter of taste, I would have had propname_check throw rather
> than making it be a predicate.

I did wonder about that. I'm happy to change if you want me to.

Ian.

Re: [PATCH OSSTEST v2 3/4] Add support for selecting resources based on their properties.

[Ian Jackson]

Ian Campbell writes ("Re: [PATCH OSSTEST v2 3/4] Add support for selecting resources based on their properties."):
> On Wed, 2015-09-16 at 14:50 +0100, Ian Jackson wrote:
> > As a matter of taste, I would have had propname_check throw rather
> > than making it be a predicate.
> 
> I did wonder about that. I'm happy to change if you want me to.

Your code - up to you :-).

Ian.

Re: [PATCH OSSTEST v2 3/4] Add support for selecting resources based on their properties.

[Ian Campbell]

On Wed, 2015-09-16 at 15:00 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [PATCH OSSTEST v2 3/4] Add support for
> selecting resources based on their properties."):
> > On Wed, 2015-09-16 at 14:50 +0100, Ian Jackson wrote:
> > > As a matter of taste, I would have had propname_check throw rather
> > > than making it be a predicate.
> > 
> > I did wonder about that. I'm happy to change if you want me to.
> 
> Your code - up to you :-).

I'll take the lazy way out ;-)

(IOW I'll leave it how it is)