[OSSTEST PATCH 00/12] Miscellaneous cleanups

[OSSTEST PATCH 00/12] Miscellaneous cleanups

[Ian Jackson]

This small pre-series contains cleanups arising from my test host
reuse work.  The final patch is a feature removal.

Ian Jackson (12):
  ms-queuedaemon: Update for newer Tcl's socket channel ids
  Executive.pm planner: fix typo
  README.planner: Document magic job hostflags
  sg-run-job: Minor whitespace (formatting) changes
  show_abs_time: Represent undef $timet as <undef>
  duration estimator: Fix a warning
  ts-hosts-allocate-Executive: Add a comment about a warning
  target setup refactoring: Move target_kernkind_check
  target setup refactoring: Move target_kernkind_console_inittab
  target setup refactoring: Merge target_kernkind_*
  target setup refactoring: Add a doc comment
  abolish "kernkind"; desupport non-pvops kernels

 Osstest.pm                  |  1 +
 Osstest/Debian.pm           |  3 +-
 Osstest/Executive.pm        |  5 ++--
 Osstest/TestSupport.pm      | 23 ++++++--------
 README                      |  2 +-
 README.planner              | 60 +++++++++++++++++++++++++++++++++++++
 make-hosts-flight           |  1 -
 mfi-common                  |  7 ++---
 ms-queuedaemon              | 10 +++----
 sg-run-job                  |  8 ++---
 ts-debian-fixup             |  5 ++--
 ts-hosts-allocate-Executive |  4 +++
 12 files changed, 92 insertions(+), 37 deletions(-)

-- 
2.20.1

[OSSTEST PATCH 01/12] ms-queuedaemon: Update for newer Tcl's socket channel ids

[Ian Jackson]

Now we have things like "sock55599edaf050" where previously we had
something like "sock142".  So the output is misaligned.

Bump the sizes.  And with these longer names, when showing the front
of the queue only print the full first entry and the start of the next
one.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 ms-queuedaemon | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/ms-queuedaemon b/ms-queuedaemon
index a3a009ca..dc858863 100755
--- a/ms-queuedaemon
+++ b/ms-queuedaemon
@@ -91,7 +91,7 @@ proc log-event {m} {
 proc log-state {m} {
     global need_queue_run queue
 
-    set lhs [format "N=%d Q=%d (%-15.15s) " \
+    set lhs [format "N=%d Q=%d (%-20.20s) " \
                  $need_queue_run [llength $queue] $queue]
 
     foreach-walker w {
@@ -99,13 +99,13 @@ proc log-state {m} {
 	if {[info exists queue_running]} {
 	    append lhs [format "R=%-3d " [llength $queue_running]]
 	    if {[info exists thinking]} {
-		append lhs [format "T=%-7s " $thinking]
+		append lhs [format "T=%-16s " $thinking]
 	    } else {
-		append lhs [format "          "]
+		append lhs [format "                   "]
 	    }
-	    append lhs [format "(%-15.15s) " $queue_running]
+	    append lhs [format "(%-20.20s) " $queue_running]
 	} else {
-	    append lhs "                                  "
+	    append lhs "                                                "
 	}
     }
 
-- 
2.20.1

[OSSTEST PATCH 02/12] Executive.pm planner: fix typo

[Ian Jackson]

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 Osstest/Executive.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index 61a99bc3..4151aa6c 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -693,7 +693,7 @@ sub plan_search ($$$$) {
 
 	    next PERIOD if $endevt->{Time} <= $try_time;
             # this period is entirely before the proposed slot;
-            # it doesn't overlap, but most check subsequent periods
+            # it doesn't overlap, but must check subsequent periods
 
 	  CHECK:
 	    {
-- 
2.20.1

[OSSTEST PATCH 03/12] README.planner: Document magic job hostflags

[Ian Jackson]

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 README.planner | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/README.planner b/README.planner
index c33aae11..f134d716 100644
--- a/README.planner
+++ b/README.planner
@@ -203,6 +203,66 @@ that shared systems get regrooved occasionally even if nothing decides
 to unshare them.
 
 
+ts-hosts-allocate-Executive and hostflags
+----------------------------------------
+
+Within a job, the allocations are actually done by
+ts-hosts-allocate-Executive.  It is told what to do by its command
+line arguments, which are (usually) simply IDENTs.
+
+The IDENTs provide the key for runvars which control the host
+allocation algorithm.  Principally, these are the runvars which define
+the job's hostflags
+    all_hostflags
+    IDENT_hostflags
+    runtime_IDENT_hostflags
+(all of these are comma-separated lists).
+
+Each such hostflag must, in general, be set for a particular host, for
+that host to be eligible.  But there are some special forms of job
+hostflag:
+
+  share-SHARING
+
+    The host may be shared with other jobs.  Typically used for
+    builds.  SHARING is a string which denotes the "scope" of the
+    sharing - jobs with the same SHARING should set the host up
+    identically.  The osstest test harness revision is automatically
+    appended and therefore does not need to be included.
+
+  equiv-FORMALTOKEN
+
+    For each equiv-FORMALTOKEN job flag set on one or more IDENTs, a
+    corresponding equiv-ACTUALTOKEN host flag must be set on the
+    corresponding hosts.  So, for example, if the IDENTs src_host and
+    dst_host both have equiv-1 specified, then the two hosts chosen
+    for src_host and dst_host will have an actual hostflag in common
+    which matches the pattern equiv-*.
+
+  diverse-FORMATLTOKEN
+
+    For each diverse-FORMALTOKEN flag, the selected host will *not* be
+    the same as any other allocation with the same diverse-FORMALTOKEN
+    flag in the same *flight*.
+
+  CONDNAME:CONDARGS...
+
+    Looks up CONDNAME as Osstest::ResourceCondition::PROPNAME.
+    The selected host must match the appropriate condition.
+    CONDNAMEs are:
+
+  PropEq:HOSTPROP:VAL
+
+    Require the host property HOSTPROP to be equal to VAL,
+    according to string comparison.  (Unset properties are
+    match an empty VAL.)
+
+  PropMinVer:HOSTPROP:VAL
+
+    Require the host property HOSTPROP to be at least VAL, according
+    to version number comparison (as implemented by dpkg and
+    coreutils).
+
 Flights
 -------
 
-- 
2.20.1

[OSSTEST PATCH 04/12] sg-run-job: Minor whitespace (formatting) changes

[Ian Jackson]

No functional change.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 sg-run-job | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/sg-run-job b/sg-run-job
index df3d08d0..3db05b34 100755
--- a/sg-run-job
+++ b/sg-run-job
@@ -30,12 +30,12 @@ proc per-host-prep {} {
     per-host-ts .       xen-boot/@        ts-host-reboot
 
     per-host-ts .       host-ping-check-xen/@ ts-host-ping-check
-    per-host-ts .       =(*)             {ts-leak-check basis}
+    per-host-ts .       =(*)            { ts-leak-check basis }
 }
 
 proc per-host-finish {} {
     if {[nested-hosts-p]} { set broken fail } { set broken broken }
-    per-host-ts .       =                {ts-leak-check check}
+    per-host-ts .       =               { ts-leak-check check }
     per-host-ts !$broken capture-logs/@(*) ts-logs-capture
 }
 
@@ -96,7 +96,7 @@ proc run-job {job} {
 	
 	if {![nested-hosts-p]} break
 
-	per-host-ts . final-poweroff {ts-host-powercycle --power=0}
+	per-host-ts . final-poweroff { ts-host-powercycle --power=0 }
 
         set need_xen_hosts [lunappend nested_layers_hosts]
     }
@@ -549,7 +549,7 @@ proc setup-test-pair {} {
     run-ts . =              ts-debian-install      dst_host
     run-ts . =              ts-debian-fixup        dst_host          + debian
     run-ts . =              ts-guests-nbd-mirror + dst_host src_host + debian
-    per-host-ts . =(*)     {ts-leak-check basis}
+    per-host-ts . =(*)    { ts-leak-check basis }
     run-ts . =              ts-guest-start       + src_host          + debian
 }
 proc need-hosts/test-pair {} { return {src_host dst_host} }
-- 
2.20.1

[OSSTEST PATCH 05/12] show_abs_time: Represent undef $timet as <undef>

[Ian Jackson]

This can happen, for example, if a badly broken flight has steps which
are STARTING and have NULL in the start time column, and is then
reported using sg-report-flight.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 Osstest.pm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Osstest.pm b/Osstest.pm
index b2b6b741..734c0ef6 100644
--- a/Osstest.pm
+++ b/Osstest.pm
@@ -457,6 +457,7 @@ sub visible_undef ($) {
 
 sub show_abs_time ($) {
     my ($timet) = @_;
+    return '<undef>' unless defined $timet;
     return strftime "%Y-%m-%d %H:%M:%S Z", gmtime $timet;
 }
 
-- 
2.20.1

[OSSTEST PATCH 06/12] duration estimator: Fix a warning

[Ian Jackson]

$onhost can be undef.  Fixes this warning:

Use of uninitialized value $onhost in concatenation (.) or string at Osstest/Executive.pm line 1293.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 Osstest/Executive.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index 4151aa6c..b3aecfc5 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -1274,7 +1274,8 @@ END
     return sub {
         my ($job, $hostidname, $onhost, $uptoincl_testid) = @_;
 
-	my $memokey = "$job $hostidname $onhost ".($uptoincl_testid//"");
+	my $memokey =
+	  "$job $hostidname ".($onhost//"")." ".($uptoincl_testid//"");
 	my $memo = $our_memo->{$memokey};
 	return @$memo if $memo;
 
-- 
2.20.1

[OSSTEST PATCH 07/12] ts-hosts-allocate-Executive: Add a comment about a warning

[Ian Jackson]

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 ts-hosts-allocate-Executive | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/ts-hosts-allocate-Executive b/ts-hosts-allocate-Executive
index 698437c0..8f7a1af0 100755
--- a/ts-hosts-allocate-Executive
+++ b/ts-hosts-allocate-Executive
@@ -1035,6 +1035,10 @@ sub actual_allocation ($) {
 	    if ($shared->{ntasks}) {
 		warn "resource $shrestype $shared->{resname} allegedly".
                     " available but wrong state $shared->{state} and tasks";
+		# This can happen if following a failed prep by
+		# another job, the other shares are still owned by the
+		# now-dead task.  If so that share will become allocatable
+		# (and therefore not be counted in `ntasks') in due course.
 		return undef;
 
                 # someone was preparing it but they aren't any more
-- 
2.20.1

[OSSTEST PATCH 08/12] target setup refactoring: Move target_kernkind_check

[Ian Jackson]

This is OK because nothing in access() looks at the rootdev or console
runvars, which are what target_kernkind_check sets.

No functional change other than perhaps to log output.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 ts-debian-fixup | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ts-debian-fixup b/ts-debian-fixup
index 528fb03b..34051137 100755
--- a/ts-debian-fixup
+++ b/ts-debian-fixup
@@ -209,8 +209,8 @@ sub writecfg () {
 
 savecfg();
 ether();
-target_kernkind_check($gho);
 access();
+target_kernkind_check($gho);
 
 debian_overlays($ho, \&overlay);
 target_cmd_root($ho, <<END.debian_overlays_fixup_cmd($ho, $mountpoint));
-- 
2.20.1

[OSSTEST PATCH 09/12] target setup refactoring: Move target_kernkind_console_inittab

[Ian Jackson]

We move this earlier.  This is OK because it depends only on the
console runvar (inside the sub; this is set by target_kernkind_check),
$ho and $gho (which are set by this point); and $mountpoint$ (which is
set by access().

No functional change.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 ts-debian-fixup | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ts-debian-fixup b/ts-debian-fixup
index 34051137..2184212b 100755
--- a/ts-debian-fixup
+++ b/ts-debian-fixup
@@ -79,10 +79,9 @@ END
 }
 
 our $extra;
+our $console;
 
 sub console () {
-    my $console=
-        target_kernkind_console_inittab($ho,$gho,"$mountpoint");
     return unless length $console;
     
     my $xextra= "console=$console earlyprintk=xen";
@@ -211,6 +210,7 @@ savecfg();
 ether();
 access();
 target_kernkind_check($gho);
+$console = target_kernkind_console_inittab($ho,$gho,"$mountpoint");
 
 debian_overlays($ho, \&overlay);
 target_cmd_root($ho, <<END.debian_overlays_fixup_cmd($ho, $mountpoint));
-- 
2.20.1

[OSSTEST PATCH 10/12] target setup refactoring: Merge target_kernkind_*

[Ian Jackson]

Combine these two functions.  Rename them to a name which doesn't
mention "kernkind".

No functional change.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 Osstest/Debian.pm      |  3 +--
 Osstest/TestSupport.pm | 11 ++++-------
 ts-debian-fixup        |  3 +--
 3 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
index b140ede2..85fd16da 100644
--- a/Osstest/Debian.pm
+++ b/Osstest/Debian.pm
@@ -68,8 +68,7 @@ sub debian_boot_setup ($$$$$;$) {
     # $xenhopt==undef => is actually a guest, do not set up a hypervisor
     my ($ho, $want_kernver, $want_xsm, $xenhopt, $distpath, $hooks) = @_;
 
-    target_kernkind_check($ho);
-    target_kernkind_console_inittab($ho,$ho,"/");
+    target_setup_rootdev_console_inittab($ho,$ho,"/");
 
     my $kopt;
     my $console= target_var($ho,'console');
diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index faac106f..fd7b238b 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -105,7 +105,7 @@ BEGIN {
                       host_get_free_memory
 
                       target_ping_check_down target_ping_check_up
-                      target_kernkind_check target_kernkind_console_inittab
+                      target_setup_rootdev_console_inittab
                       target_var target_var_prefix
                       selectguest prepareguest more_prepareguest_hvm
 		      guest_template
@@ -2562,8 +2562,9 @@ sub target_var ($$) {
     return undef;
 }
 
-sub target_kernkind_check ($) {
-    my ($gho) = @_;
+sub target_setup_rootdev_console_inittab ($$$) {
+    my ($ho, $gho, $root) = @_;
+
     my $pfx= target_var_prefix($gho);
     my $kernkind= $r{$pfx."kernkind"} // 'pvops';
     my $isguest= exists $gho->{Guest};
@@ -2573,10 +2574,6 @@ sub target_kernkind_check ($) {
     } elsif ($kernkind !~ m/2618/) {
         store_runvar($pfx."console", 'xvc0') if $isguest;
     }
-}
-
-sub target_kernkind_console_inittab ($$$) {
-    my ($ho, $gho, $root) = @_;
 
     my $inittabpath= "$root/etc/inittab";
     my $console= target_var($gho,'console');
diff --git a/ts-debian-fixup b/ts-debian-fixup
index 2184212b..a878fe50 100755
--- a/ts-debian-fixup
+++ b/ts-debian-fixup
@@ -209,8 +209,7 @@ sub writecfg () {
 savecfg();
 ether();
 access();
-target_kernkind_check($gho);
-$console = target_kernkind_console_inittab($ho,$gho,"$mountpoint");
+$console = target_setup_rootdev_console_inittab($ho,$gho,"$mountpoint");
 
 debian_overlays($ho, \&overlay);
 target_cmd_root($ho, <<END.debian_overlays_fixup_cmd($ho, $mountpoint));
-- 
2.20.1

[OSSTEST PATCH 11/12] target setup refactoring: Add a doc comment

[Ian Jackson]

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 Osstest/TestSupport.pm | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index fd7b238b..d9bb2585 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -2563,6 +2563,9 @@ sub target_var ($$) {
 }
 
 sub target_setup_rootdev_console_inittab ($$$) {
+    # Operators on $gho.
+    # $gho's filesystem is accessed via $ho and $mountpoint;
+    # so maybe $gho is $ho and $mountpoint is "/".
     my ($ho, $gho, $root) = @_;
 
     my $pfx= target_var_prefix($gho);
-- 
2.20.1

[OSSTEST PATCH 12/12] abolish "kernkind"; desupport non-pvops kernels

[Ian Jackson]

This was for distinguishing the old-style Xenolinux kernels from pvops
kernels.

We have not actually tested any non-pvops kernels for a very very long
time.  Delete this now because the runvar is slightly in the way of
test host reuse.

(Sorry for the wide CC but it seems better to make sure anyone who
might object can do so.)

All this machinery exists just to configure the guest console
device (Xenolinux used "xvc" rather than "hvc") and the guest root
block device (Xenolinux stole "hda"/"sda" rather than using "xvda").

Specifically, in this commit:
 * In what is now target_setup_rootdev_console_inittab, do not
   look at any kernkind runvar and simply do what we would if
   it were "pvops" or unset, as it is in all current jobs.
 * Remove the runvar from all jobs creation and example runes.
   (This has no functional change even for jobs running with
   the previous osstest code because we have defaulted to "pvops"
   for a very long time.)

We retain the setting of the shell variable "kernbuild", because that
ends up in build jobs names.  All our kernel build jobs now end in
-pvops and I intend to retain that name component since abolishing it
is nontrivial.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: Juergen Gross <jgross@suse.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wei.liu@kernel.org>
CC: Paul Durrant <paul@xen.org>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Juergen Gross <jgross@suse.com>
CC: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
CC: Andrew Cooper <Andrew.Cooper3@citrix.com>
CC: Olivier Lambert <olivier.lambert@vates.fr>
---
 Osstest/TestSupport.pm | 9 ++-------
 README                 | 2 +-
 make-hosts-flight      | 1 -
 mfi-common             | 7 ++-----
 4 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index d9bb2585..99c7654d 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -2569,14 +2569,9 @@ sub target_setup_rootdev_console_inittab ($$$) {
     my ($ho, $gho, $root) = @_;
 
     my $pfx= target_var_prefix($gho);
-    my $kernkind= $r{$pfx."kernkind"} // 'pvops';
     my $isguest= exists $gho->{Guest};
-    if ($kernkind eq 'pvops') {
-        store_runvar($pfx."rootdev", 'xvda') if $isguest;
-        store_runvar($pfx."console", 'hvc0');
-    } elsif ($kernkind !~ m/2618/) {
-        store_runvar($pfx."console", 'xvc0') if $isguest;
-    }
+    store_runvar($pfx."rootdev", 'xvda') if $isguest;
+    store_runvar($pfx."console", 'hvc0');
 
     my $inittabpath= "$root/etc/inittab";
     my $console= target_var($gho,'console');
diff --git a/README b/README
index 2804ecf3..ba4bea1d 100644
--- a/README
+++ b/README
@@ -861,7 +861,7 @@ echo $flight
 job=play-amd64-amd64-xen-boot
 ./cs-job-create $flight $job play-xen-boot-x5 \
     all_hostflags=arch-amd64,arch-xen-amd64,suite-wheezy,purpose-test \
-    arch=amd64 toolstack=xl enable_xsm=false kernkind=pvops \
+    arch=amd64 toolstack=xl enable_xsm=false \
     host=$host
 
 # Reuse the binaries from the Xen template job for both the hypervisor
diff --git a/make-hosts-flight b/make-hosts-flight
index e2c3776a..63ac7b71 100755
--- a/make-hosts-flight
+++ b/make-hosts-flight
@@ -73,7 +73,6 @@ hosts_iterate () {
         local freebsd_runvars
         set_freebsd_runvars true
         runvars+=" 
-                   kernkind=pvops
                    all_host_di_version=$di_version
                    all_host_suite=$suite
                    $freebsd_runvars
diff --git a/mfi-common b/mfi-common
index e577449f..34b0c116 100644
--- a/mfi-common
+++ b/mfi-common
@@ -619,9 +619,8 @@ test_matrix_iterate () {
       case $kern in
       '')
                   kernbuild=pvops
-                  kernkind=pvops
                   ;;
-      *)          echo >&2 "kernkind ?  $kern"; exit 1 ;;
+      *)          echo >&2 "kernbuild ?  $kern"; exit 1 ;;
       esac
 
       for dom0arch in i386 amd64 armhf arm64; do
@@ -639,8 +638,7 @@ test_matrix_iterate () {
             arch_runvars=\"\$ARCH_RUNVARS_$dom0arch\"
         "
 
-        debian_runvars="debian_kernkind=$kernkind \
-                        debian_arch=$dom0arch \
+        debian_runvars="debian_arch=$dom0arch \
                         debian_suite=$guestsuite \
                         "
 
@@ -659,7 +657,6 @@ test_matrix_iterate () {
         most_runvars="
                   arch=$dom0arch                                  \
                   kernbuildjob=${bfi}build-$dom0arch-$kernbuild   \
-                  kernkind=$kernkind                              \
                   $arch_runvars $hostos_runvars
                   "
 
-- 
2.20.1