[OSSTEST PATCH v4 00/28] xen.git#staging smoke tests

[OSSTEST PATCH v4 00/28] xen.git#staging smoke tests

[Ian Jackson]

This time, for sure.  Specifically, I have:

 * Run this as a nearly-live cr-daily-branch in my account on
   in the production colo, and checked that the versions used
   and result emails look sane.

 * Double-checked the standalone-generate-dump-flight-runvars
   again.

 * Ad-hoc tested 28/28 in the Cambridge instance with intrusive
   monitoring and checked that it does what I expect.

I am sending out only those patchbomb mails which (a) lack an ack or
(b) have changed in v4.  They are these:

 +  05/28  sg-report-flight: Better searching for used revisions
 !- 14/28  Provide xen-unstable-smoke branch
 *a 15/28  cr-daily-branch: Use mg-adjust-flight to have smoke tests...
 +  19/28  ts-debian-hvm-install: Defer preseed generation
 *- 20/28  ts-debian-hvm-install: Cope with images containing only isolinux
 *- 22/28  ts-debian-hvm-install: Do not create EFI partition if EFI...
 +  28/28  Executive: Delay releasing build host shares

Where:

 +  New patch
 *  Modified
 !  Modified and combined several previous patches
 a  Acked
 -  Ack dropped due to changes

[OSSTEST PATCH 05/28] sg-report-flight: Better searching for used revisions

[Ian Jackson]

The old algorithm used for determining which flight might be a
suitable test of a particular revision was rather crude, in two ways:

 * It would look at _all_ jobs in a flight referred to from the flight
   of interest, not just at the relevant jobs;

 * It would only look at the direct referents of the flight in
   question.  So for example, if a flight of interest contained
   test-amd64-i386-libvirt, it would find a referenced
   build-i386-libvirt in another flight, but that build refers to
   build-i386, and it would not look at that (unless it happened to be
   in the same flight).

Fix this by redoing the revision archaeology, with some $why tracking
to explain how we found a particular revision.

cs-bisection-step and sg-check-tested arguably ought to do do it this
way too.  But I am leaving centralising this new logic, and using it
in those other programs, for another day.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
v4: New patch
---
 sg-report-flight |   79 ++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 62 insertions(+), 17 deletions(-)

diff --git a/sg-report-flight b/sg-report-flight
index ef3fd6b..3d0bf64 100755
--- a/sg-report-flight
+++ b/sg-report-flight
@@ -201,15 +201,26 @@ END
     $flightsq= db_prepare($flightsq);
     $flightsq->execute(@flightsq_params);
 
-    my $buildflightsq= db_prepare(<<END);
-        SELECT val FROM runvars
+    my $jobsq= db_prepare(<<END);
+        SELECT '' AS why,
+               job
+	  FROM jobs
 	 WHERE flight = ?
+END
+
+    my $buildjobsq= db_prepare(<<END);
+        SELECT name || '= ' AS why,
+               val AS job
+	  FROM runvars
+	 WHERE flight = ?
+           AND job = ?
            AND name LIKE '%buildjob'
 END
 
     my $revisionsq= <<END;
             SELECT flight, job, val FROM runvars
                 WHERE flight=?
+                  AND job=?
 		  AND name=?
                   AND ${\ main_revision_job_cond('job') }
 		GROUP BY flight, job, val
@@ -222,42 +233,76 @@ END
 END
 
     while (my ($tflight) = $flightsq->fetchrow_array) {
-	my @bflights;
-	push @bflights, $tflight;
-	$buildflightsq->execute($tflight);
-	while (my $bflightrow = $buildflightsq->fetchrow_hashref()) {
-	    my $val = $bflightrow->{val};
-	    next unless $val =~ m/\./; # same flight, already added
-	    push @bflights, $`;
+	# Recurse from the starting flight looking for relevant build
+	# jobs.  We start with all jobs in $tflight, and for each job
+	# we also process any other jobs it refers to in *buildjob runvars.
+	#
+	# We don't actually use a recursive algorithm because that
+	# would involve recursive use of the same sql query object;
+	# hence the @binfos_todo queue.
+	my @binfos_todo;
+	my $binfos_queue = sub {
+	    my ($inflight,$q,$why) = @_;
+	    while (my ($morewhy,$jobref) = $q->fetchrow_array()) {
+		my @info = "$why $morewhy$jobref";
+		push @info, flight_otherjob($inflight,$jobref);
+		push @binfos_todo, \@info;
+	    }
+	    $q->finish();
+	};
+	$jobsq->execute($tflight);
+	$binfos_queue->($tflight,$jobsq,"$tflight");
+
+	my %binfos;
+	while (@binfos_todo) {
+	    my ($why,$bflight,$bjob) = @{ shift @binfos_todo };
+	    next if $binfos{$bflight}{$bjob};
+	    $binfos{$bflight}{$bjob} = $why;
+	    $buildjobsq->execute($bflight,$bjob);
+	    $binfos_queue->($bflight,$buildjobsq,$why);
 	}
+
+	my @binfos;
+	foreach my $bflight (sort { $a <=> $b } keys %binfos) {
+	    my $bjobs = $binfos{$bflight};
+	    foreach my $bjob (sort keys %$bjobs) {
+		my $why = $bjobs->{$bjob};
+		#print DEBUG " relevant $bflight.$bjob because $why\n";
+		push @binfos, [ $why, $bflight, $bjob ];
+	    }
+	}
+
 	my $whynot;
 	foreach my $tree (keys %{ $specver{$thisthat} }) {
 	    my @revisions;
 	    my $v= $specver{$thisthat}{$tree};
-	    foreach my $bflight (@bflights) {
+	    foreach my $binfo (@binfos) {
+		my ($bwhy,@bfj) = @$binfo;
 		my $revisions;
 		if ($tree ne 'osstest') {
-		    $revisionsq->execute($bflight, "built_revision_$tree");
+		    $revisionsq->execute(@bfj, "built_revision_$tree");
 		    $revisions= $revisionsq->fetchall_arrayref({});
 		} else {
-		    $revisionsosstestq->execute($bflight);
+		    $revisionsosstestq->execute($bfj[0]);
 		    $revisions= $revisionsosstestq->fetchall_arrayref({});
 		}
-		push @revisions, @$revisions;
+		push @revisions, map { [ $bwhy, $_ ] } @$revisions;
 	    }
             if (!@revisions) {
                 $whynot= "no built/used $tree";
                 last;
             }
-            my ($wrong) = grep {
-                $_->{val} !~ m/^(?: .*: )? $v /x;
+            my ($wronginfo) = grep {
+                $_->[1]{val} !~ m/^(?: .*: )? $v /x;
             } @revisions;
 
-            if (defined $wrong) {
+            if (defined $wronginfo) {
+		my ($why,$wrong) = @$wronginfo;
                 $whynot= "mismatch $tree ".
                     (defined $wrong->{job} ?
 		     "$wrong->{flight}.$wrong->{job}" : "(osstest)").
-                    " $wrong->{val} != $v";
+                    " $wrong->{val} != $v".
+		    " ($why)";
                 last;
             }
 	}
-- 
1.7.10.4

Re: [OSSTEST PATCH 05/28] sg-report-flight: Better searching for used revisions

[Ian Campbell]

On Tue, 2015-09-22 at 16:12 +0100, Ian Jackson wrote:
> The old algorithm used for determining which flight might be a
> suitable test of a particular revision was rather crude, in two ways:
> 
>  * It would look at _all_ jobs in a flight referred to from the flight
>    of interest, not just at the relevant jobs;
> 
>  * It would only look at the direct referents of the flight in
>    question.  So for example, if a flight of interest contained
>    test-amd64-i386-libvirt, it would find a referenced
>    build-i386-libvirt in another flight, but that build refers to
>    build-i386, and it would not look at that (unless it happened to be
>    in the same flight).
> 
> Fix this by redoing the revision archaeology, with some $why tracking
> to explain how we found a particular revision.
> 
> cs-bisection-step and sg-check-tested arguably ought to do do it this
> way too.  But I am leaving centralising this new logic, and using it
> in those other programs, for another day.
> 
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

[OSSTEST PATCH 14/28] Provide xen-unstable-smoke branch

[Ian Jackson]

Introduce support for branch=qemu-xen-unstable-smoke which has
xenbranch=xen-unstable-smoke.

In make-flight, this contains a very limited set of jobs
    test-amd64-amd64-libvirt
    test-amd64-amd64-xl-qemuu-debianhvm-i386
    test-armhf-armhf-xl
and the builds they depend on.

The debianhvm job exists only in this flight, and is generated by
having branch_debianhvm_arch return i386 instead of amd64.  This is so
that this branch contains a 32-bit x86 guest as well as a 64-bit one.

We override host allocator parameters to make this flight not care
about host stickiness: it just takes whatever comes to hand.  These
runvars are marked `synth' so that cs-bisection-step and
cs-adjust-flight do not copy them, as discussed in previous patches.

Later we will arrange to reuse previous builds for the build artefacts
which aren't intended subjects of the smoke test.

(Deployment note: This needs images/debian-7.2.0-i386-CD-1.iso which I
have already placed in the Cambridge and Xen Project instances.)

In ap-common we need to arrange to use the same qemu trees as for
xen-unstable, rather than looking for special smoke ones.

In select_xenbranch xen-unstable-smoke is mostly like xen-unstable.

There are only two places in osstest where xenbranch `xen-unstable' is
treated specially and only one of them needs adjusting to match
xen-unstable-smoke too.

The new branch `xen-unstable-smoke' has a `prev' branch of
`xen-unstable' according to cri-getprevxenbranch, which is technically
wrong, but this is not important because xen-unstable-smoke has no
prev tests.

We are going to sort out the push gate ref plumbing in xen.git in the
next osstest patch.

Also, use a branch-settings file to set the new branch's resource
priority to -20 to make it run ahead of anything else automatic.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
v4: Introduce xenbranch_forqemu into ap-common.
    Combine patches for make-flight and cr-*.  make-flight includes
    cri-common and ap-common (which is arguably a layering violation,
    but there we are).
    Dropped ack from Ian Campbell.
    Set `qemuubranch' correctly in select_xenbranch.
v2: Generate all the jobs that this flight's tests use, and add
    note about this to the commit message.
    Mention `synth'-ness of hostalloc runvars in commit message.
    Image is in Xen Project test colo too.
---
 README.planner                     |    1 +
 ap-common                          |    8 +++++---
 branch-settings.xen-unstable-smoke |    1 +
 cr-daily-branch                    |    2 +-
 cri-common                         |    1 +
 make-flight                        |   31 ++++++++++++++++++++++++++++++-
 6 files changed, 39 insertions(+), 5 deletions(-)
 create mode 100644 branch-settings.xen-unstable-smoke

diff --git a/README.planner b/README.planner
index dfa623e..3a491a4 100644
--- a/README.planner
+++ b/README.planner
@@ -87,6 +87,7 @@ in the more distant future.
 Values for OSSTEST_RESOURCE_PRIORITY:
   mg-allocate                                    -1000000
   mg-blockage                                    -2000000
+  xen-unstable-smoke cr-daily-branch                  -20
   default Executive.pm with controlling terminal      -10
   mg-execute-flight                                    -8
   default                                               0
diff --git a/ap-common b/ap-common
index dfeab9e..91425a9 100644
--- a/ap-common
+++ b/ap-common
@@ -19,6 +19,8 @@
 
 # $xenbranch must already be set
 
+xenbranch_forqemu=${xenbranch/xen-unstable-smoke/xen-unstable}
+
 : ${XENBITS:=osstest@xenbits.xen.org}
 
 : ${TREEBRANCH_OSSTEST_UPSTREAM=`getconfig OsstestUpstream`}
@@ -26,8 +28,8 @@
 : ${TREE_XEN:=git://xenbits.xen.org/xen.git}
 : ${PUSH_TREE_XEN:=$XENBITS:/home/xen/git/xen.git}
 
-#: ${TREE_QEMU:=git://mariner.uk.xensource.com/qemu-$xenbranch.git}
-: ${TREE_QEMU:=git://xenbits.xen.org/staging/qemu-$xenbranch.git}
+#: ${TREE_QEMU:=git://mariner.uk.xensource.com/qemu-$xenbranch_forqemu.git}
+: ${TREE_QEMU:=git://xenbits.xen.org/staging/qemu-$xenbranch_forqemu.git}
 
 
 : ${GIT_KERNEL_ORG:=git://git.kernel.org}
@@ -87,7 +89,7 @@ fi
 
 : ${TREEBASE_LINUX_XCP:=http://hg.uk.xensource.com/carbon/trunk/linux-2.6.27}
 
-: ${TREE_QEMU_UPSTREAM:=git://xenbits.xen.org/staging/qemu-upstream-${xenbranch#xen-}.git}
+: ${TREE_QEMU_UPSTREAM:=git://xenbits.xen.org/staging/qemu-upstream-${xenbranch_forqemu#xen-}.git}
 : ${LOCALREV_QEMU_UPSTREAM:=daily-cron.$branch}
 
 : ${TREE_QEMU_MAINLINE:=git://git.qemu.org/qemu.git}
diff --git a/branch-settings.xen-unstable-smoke b/branch-settings.xen-unstable-smoke
new file mode 100644
index 0000000..b73bc6a
--- /dev/null
+++ b/branch-settings.xen-unstable-smoke
@@ -0,0 +1 @@
+export OSSTEST_RESOURCE_PRIORITY=-20
diff --git a/cr-daily-branch b/cr-daily-branch
index 9500bdc..141bce5 100755
--- a/cr-daily-branch
+++ b/cr-daily-branch
@@ -328,7 +328,7 @@ case "$NEW_REVISION/$OLD_REVISION" in
                         "$treeurl#$OLD_REVISION-$NEW_REVISION" \
 
                 case "$realtree" in
-                xen-4*|xen-unstable)
+                xen-4*|xen-unstable*)
                     oldqemu=`./ap-qemu-revision $realtree $OLD_REVISION`
                     newqemu=`./ap-qemu-revision $realtree $NEW_REVISION`
                     if [ "$oldqemu" ] && [ "$newqemu" ]; then
diff --git a/cri-common b/cri-common
index 2669485..17b916d 100644
--- a/cri-common
+++ b/cri-common
@@ -66,6 +66,7 @@ select_prevxenbranch () {
 
 select_xenbranch () {
 	case "$branch" in
+	xen-unstable-smoke)	tree=xen;	xenbranch=$branch; qemuubranch=qemu-upstream-unstable;;
 	xen-*)			tree=xen;	xenbranch=$branch ;;
 	qemu-mainline)		tree=qemuu;	xenbranch=xen-unstable	qemuubranch=qemu-mainline;;
         qemu-upstream-*)    tree=qemuu; xenbranch=xen-${branch#qemu-upstream-};;
diff --git a/make-flight b/make-flight
index 13674b8..c1f40b9 100755
--- a/make-flight
+++ b/make-flight
@@ -34,8 +34,28 @@ flight=`./cs-flight-create $blessing $branch`
 defsuite=`getconfig DebianSuite`
 defguestsuite=`getconfig GuestDebianSuite`
 
+case "$branch" in
+xen-unstable-smoke)
+	global_runvars+=" hostalloc_maxbonus_variation~=0 "
+	global_runvars+=" hostalloc_bonus_previousfail~=0 "
+        ;;
+esac
+
 job_create_build_filter_callback () {
-    :
+  local job=$1; shift
+  case "$branch" in
+    xen-unstable-smoke)
+      case "$job" in
+        build-amd64)		;;
+        build-amd64-pvops)	;;
+        build-amd64-libvirt)	;;
+        build-armhf)		;;
+        build-armhf-pvops)	;;
+        *)			return 1 ;;
+      esac
+    ;;
+  esac
+  return 0
 }
 
 if [ x$buildflight = x ]; then
@@ -56,6 +76,14 @@ job_create_test_filter_callback () {
   local dom0arch=$1; shift
 
   case "$branch" in
+    xen-unstable-smoke)
+      case "$job" in
+        test-amd64-amd64-libvirt)                  return 0 ;;
+        test-armhf-armhf-xl)                       return 0 ;;
+        test-amd64-amd64-xl-qemuu-debianhvm-i386)  return 0 ;;
+        *)                                         return 1 ;;
+      esac
+      ;;
     qemu-upstream-4.2-testing)
       case " $* " in
         *" device_model_version=qemu-xen "*)
@@ -222,6 +250,7 @@ do_hvm_win7_x64_tests () {
 
 branch_debianhvm_arch () {
   case $branch in
+    xen-unstable-smoke) echo i386;;
     *) echo amd64;;
   esac
 }
-- 
1.7.10.4

Re: [OSSTEST PATCH 14/28] Provide xen-unstable-smoke branch

[Ian Campbell]

On Tue, 2015-09-22 at 16:12 +0100, Ian Jackson wrote:

> (Deployment note: This needs images/debian-7.2.0-i386-CD-1.iso which I
> have already placed in the Cambridge and Xen Project instances.)

Which made me thing, what will be the effect of this series on the
Cambridge instance, and in particular on the "OSSTEST_BASELINES_ONLY=y ./cr
-for-branches" ?

I suppose it will just do a single test of every new #smoke branch which
appears in xen.git? I think that is probably fine or even desirable.

[...]
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Re: [OSSTEST PATCH 14/28] Provide xen-unstable-smoke branch

[Ian Jackson]

Ian Campbell writes ("Re: [OSSTEST PATCH 14/28] Provide xen-unstable-smoke branch"):
> On Tue, 2015-09-22 at 16:12 +0100, Ian Jackson wrote:
> > (Deployment note: This needs images/debian-7.2.0-i386-CD-1.iso which I
> > have already placed in the Cambridge and Xen Project instances.)
> 
> Which made me thing, what will be the effect of this series on the
> Cambridge instance, and in particular on the "OSSTEST_BASELINES_ONLY=y ./cr
> -for-branches" ?
> 
> I suppose it will just do a single test of every new #smoke branch which
> appears in xen.git? I think that is probably fine or even desirable.

Yes, I think it will do a single smoke test of every new #smoke.

> > Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>

Thanks,
Ian.

[OSSTEST PATCH 15/28] cr-daily-branch: Use mg-adjust-flight to have smoke tests reuse builds

[Ian Jackson]

The smoke tests are for testing xen-unstable.  We want to avoid
building anything else.  So arrange to reuse previous builds by
calling mg-adjust-flight-makexrefs.

We rebuild libvirt too.  This is necessary because libvirt is built
against xen.git, and uses ABI-unstable APIs, so we need a libvirt
built against the right xen.git.  This means, for the smoke tests, we
need to build libvirt ourselves.  Currently this build seems to take
416 sends (from host allocation, which we - perhaps naively - hope
will be able to reuse the host from the just-finished build job).

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v4: Keep build-amd64-libvirt too.
v3: Add a comment about the --blessings=real
v2: New patch
---
 cr-daily-branch |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/cr-daily-branch b/cr-daily-branch
index 141bce5..dd9c30a 100755
--- a/cr-daily-branch
+++ b/cr-daily-branch
@@ -276,7 +276,19 @@ if [ "x$NEW_REVISION" = "x$OLD_REVISION" ]; then
 fi
 
 $DAILY_BRANCH_PREMAKE_HOOK
+
 flight=`$makeflight $branch $xenbranch $OSSTEST_BLESSING "$@"`
+
+case $branch in
+xen-unstable-smoke)
+	./mg-adjust-flight-makexrefs -v $flight \
+		'!build-amd64 !build-amd64-libvirt !build-armhf build-*' \
+		--debug --branch=xen-unstable --blessings=real
+	# Even adhoc or play flights ought to reuse only real
+	# previous builds.
+	;;
+esac
+
 $DAILY_BRANCH_POSTMAKE_HOOK
 
 heading=tmp/$flight.heading-info
-- 
1.7.10.4

Re: [OSSTEST PATCH 15/28] cr-daily-branch: Use mg-adjust-flight to have smoke tests reuse builds

[Ian Campbell]

On Tue, 2015-09-22 at 16:12 +0100, Ian Jackson wrote:

Title is missing -makexrefs?

> The smoke tests are for testing xen-unstable.  We want to avoid
> building anything else.  So arrange to reuse previous builds by
> calling mg-adjust-flight-makexrefs.
> 
> We rebuild libvirt too.  This is necessary because libvirt is built
> against xen.git, and uses ABI-unstable APIs, so we need a libvirt
> built against the right xen.git.  This means, for the smoke tests, we
> need to build libvirt ourselves.  Currently this build seems to take
> 416 sends (from host allocation, which we - perhaps naively - hope
> will be able to reuse the host from the just-finished build job).
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>

Reaffirmed.

Re: [OSSTEST PATCH 15/28] cr-daily-branch: Use mg-adjust-flight to have smoke tests reuse builds

[Ian Jackson]

Ian Campbell writes ("Re: [OSSTEST PATCH 15/28] cr-daily-branch: Use mg-adjust-flight to have smoke tests reuse builds"):
> On Tue, 2015-09-22 at 16:12 +0100, Ian Jackson wrote:
> 
> Title is missing -makexrefs?

Yes, fixed.

> > Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> Reaffirmed.

Thanks,
Ian.

[OSSTEST PATCH 19/28] ts-debian-hvm-install: Defer preseed generation

[Ian Jackson]

Defer preseed file generation until after we have fetched and looked
inside the install image, because we are going to want to make changes
to the preseed file based on the image contents.

No overall functional change, although some things happen in a
different order now, and the ISO manipulation takes place in two calls
to target_cmd_root rather than one.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
v4: New patch.  Needed because otherwise the test for the grub install
    image (to be introduced in the next patch) happens before the ISO
    is unpacked, and we would then always fall back to isolinux.
---
 ts-debian-hvm-install |   14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/ts-debian-hvm-install b/ts-debian-hvm-install
index 3b93ebd..bb79d59 100755
--- a/ts-debian-hvm-install
+++ b/ts-debian-hvm-install
@@ -200,13 +200,9 @@ sub prep () {
                           -b boot/grub/efi.img
                           -no-emul-boot
                           -r);
-    my @isogen_opts = (iso_gen_flags_basic(), @isogen_extra);
 
     iso_create_empty($ho, $emptyiso, $emptydir);
 
-    target_putfilecontents_root_stash($ho, 10, preseed(),
-                                      $preseed_file_path);
-
     # If host has >8G free memory, create a guest with 4G memory to catch
     # any error that triggers cross 4G boundary
     my $host_freemem_mb = host_get_free_memory($ho);
@@ -224,8 +220,16 @@ sub prep () {
                           Bios => $r{bios},
                           PostImageHook => sub {
         my $cmds = iso_copy_content_from_image($gho, $newiso);
-        $cmds .= prepare_initrd($initrddir,$newiso,$preseed_file_path);
         target_cmd_root($ho, $cmds, $isotimeout);
+
+        my @isogen_opts = (iso_gen_flags_basic(), @isogen_extra);
+
+        target_putfilecontents_root_stash($ho, 10, preseed(),
+                                          $preseed_file_path);
+
+        $cmds = prepare_initrd($initrddir,$newiso,$preseed_file_path);
+        target_cmd_root($ho, $cmds, $isotimeout);
+
         target_putfilecontents_root_stash($ho, 10, grub_cfg(),
                                           "$newiso/debian/boot/grub/grub.cfg");
 
-- 
1.7.10.4

Re: [OSSTEST PATCH 19/28] ts-debian-hvm-install: Defer preseed generation

[Ian Campbell]

On Tue, 2015-09-22 at 16:12 +0100, Ian Jackson wrote:
> Defer preseed file generation until after we have fetched and looked
> inside the install image, because we are going to want to make changes
> to the preseed file based on the image contents.
> 
> No overall functional change, although some things happen in a
> different order now, and the ISO manipulation takes place in two calls
> to target_cmd_root rather than one.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

[OSSTEST PATCH 20/28] ts-debian-hvm-install: Cope with images containing only isolinux

[Ian Jackson]

debian-7.2.0-i386-CD-1.iso contains no grub, only isolinux.

If the specified EFI grub file does not exist, fall back to isolinux.
This requires a -c option as well, according to
  https://wiki.debian.org/DebianInstaller/Modify/CD

Only try to set up a grub config if we are booting grub.  (The i386
image in question does not contain a [debian]/boot/grub directory.)

If boot/grub/efi.img _does_ exist (ie, for other existing tests), the
only difference in behaviour is to reorder slightly the options to
genisoimage: `-b boot/grub/efi.img' now occurs after `-no-emul-boot
-r' rather than before.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
v4: Log $bootfile value.
    Preseed generation now happens later due to previous patch;
      so $bootfile setting now also deferred.  Context change only.
---
 ts-debian-hvm-install |   12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/ts-debian-hvm-install b/ts-debian-hvm-install
index bb79d59..5197f9b 100755
--- a/ts-debian-hvm-install
+++ b/ts-debian-hvm-install
@@ -197,7 +197,6 @@ sub prep () {
     my $preseed_file_path = $base . "preseed";
 
     my @isogen_extra = qw(-eltorito-alt-boot
-                          -b boot/grub/efi.img
                           -no-emul-boot
                           -r);
 
@@ -222,6 +221,14 @@ sub prep () {
         my $cmds = iso_copy_content_from_image($gho, $newiso);
         target_cmd_root($ho, $cmds, $isotimeout);
 
+        my $bootfile = 'boot/grub/efi.img';
+        if (!target_file_exists($ho, "$newiso/$bootfile")) {
+            $bootfile = "isolinux/isolinux.bin";
+            push @isogen_extra, qw(-c isolinux/boot.cat);
+        }
+        logm("using boot image $bootfile");
+        push @isogen_extra, '-b', $bootfile;
+
         my @isogen_opts = (iso_gen_flags_basic(), @isogen_extra);
 
         target_putfilecontents_root_stash($ho, 10, preseed(),
@@ -231,7 +238,8 @@ sub prep () {
         target_cmd_root($ho, $cmds, $isotimeout);
 
         target_putfilecontents_root_stash($ho, 10, grub_cfg(),
-                                          "$newiso/debian/boot/grub/grub.cfg");
+                                          "$newiso/debian/boot/grub/grub.cfg")
+	    if $bootfile =~ m/grub/;
 
         target_putfilecontents_root_stash($ho, 10, isolinux_cfg(),
                                           "$newiso/isolinux/isolinux.cfg");
-- 
1.7.10.4

Re: [OSSTEST PATCH 20/28] ts-debian-hvm-install: Cope with images containing only isolinux

[Ian Campbell]

On Tue, 2015-09-22 at 16:12 +0100, Ian Jackson wrote:
> debian-7.2.0-i386-CD-1.iso contains no grub, only isolinux.
> 
> If the specified EFI grub file does not exist, fall back to isolinux.
> This requires a -c option as well, according to
>   https://wiki.debian.org/DebianInstaller/Modify/CD
> 
> Only try to set up a grub config if we are booting grub.  (The i386
> image in question does not contain a [debian]/boot/grub directory.)
> 
> If boot/grub/efi.img _does_ exist (ie, for other existing tests), the
> only difference in behaviour is to reorder slightly the options to
> genisoimage: `-b boot/grub/efi.img' now occurs after `-no-emul-boot
> -r' rather than before.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

[OSSTEST PATCH 22/28] ts-debian-hvm-install: Do not create EFI partition if EFI not in use

[Ian Jackson]

If we are booting our install ISO using a non-EFI executable, don't
try to provide an EFI for the installed system either.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
v4: Fix regexp to match efi.img (!)
    Rebased, fixed code motion conflict.
---
 ts-debian-hvm-install |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/ts-debian-hvm-install b/ts-debian-hvm-install
index b9f9025..6646a5a 100755
--- a/ts-debian-hvm-install
+++ b/ts-debian-hvm-install
@@ -49,6 +49,7 @@ our $guesthost= "$gn.guest.osstest";
 our $gho;
 
 our ($kernel, $ramdisk);
+our $bootfile;
 
 our $gsuite;
 
@@ -56,7 +57,7 @@ sub preseed () {
 
     my $preseed_file = preseed_base($gho,$gsuite,'','',());
 
-    $preseed_file .= (<<END);
+    $preseed_file .= (<<END.($bootfile =~ m/\befi\b/ ? <<END : '').<<END);
 d-i netcfg/get_hostname string $gn
 
 d-i partman-auto/disk string /dev/xvda
@@ -64,12 +65,14 @@ d-i partman-auto/method string  regular
 
 d-i partman-auto/expert_recipe string \\
         boot-root :: \\
+END
                 512 50 512 vfat \\
                         \$primary{ } \$bootable{ } \\
                         method{ efi } format{ } \\
                         use_filesystem{ } filesystem{ vfat } \\
                         mountpoint{ /boot/efi } \\
                 . \\
+END
                 5000 50 5000 ext4 \\
                         method{ format } format{ } \\
                         use_filesystem{ } filesystem{ ext4 } \\
@@ -222,7 +225,7 @@ sub prep () {
         my $cmds = iso_copy_content_from_image($gho, $newiso);
         target_cmd_root($ho, $cmds, $isotimeout);
 
-        my $bootfile = 'boot/grub/efi.img';
+        $bootfile = 'boot/grub/efi.img';
         if (!target_file_exists($ho, "$newiso/$bootfile")) {
             $bootfile = "isolinux/isolinux.bin";
             push @isogen_extra, qw(-c isolinux/boot.cat);
-- 
1.7.10.4

Re: [OSSTEST PATCH 22/28] ts-debian-hvm-install: Do not create EFI partition if EFI not in use

[Ian Campbell]

On Tue, 2015-09-22 at 16:12 +0100, Ian Jackson wrote:
> If we are booting our install ISO using a non-EFI executable, don't
> try to provide an EFI for the installed system either.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>


Acked-by: Ian Campbell <ian.campbell@citrix.com>

[OSSTEST PATCH 28/28] Executive: Delay releasing build host shares by 90s

[Ian Jackson]

When a build job finishes, the same flight may well want to do a
subsequent build that depended on the first.  When this happens, we
have a race:

One the one hand, we have the flight: after sg-run-job exits,
sg-execute-flight needs to double-check the job status, and search the
flight for more jobs to run; it will spawn ts-allocate-hosts-Executive
for the new job, which needs to get its head together, parse its
arguments, become a client of the queue daemon, and ask to be put in
the queue.

On the other hand, we have the planning system: currently, as soon as
sg-run-job exits, the connection to the ownerdaemon closes.  The
ownerdaemon tells the queue daemon, and the planning queue is
restarted.  It might even happen that coincidentally the planning
queue is about to start.

If the planning system wins the race, another job will pick up the
newly-freed resource.  Often this will mean unsharing the build host,
which is very wasteful if the releasing flight hasn't finished its
builds for that architecture: it means that the next build job needs
to regroove a host for builds.

Add a bodge to try to make the race go the other way: after a build
job completes successfuly, do not give up the share for a further 90
seconds.  (We have to use setsid because sg-execute-flight kills the
process group to clean up stray processes, which this sleep definitely
is.)

A better solution would be to move the wait-for-referenced-job logic
from sg-execute-flight to ts-hosts-allocate-*.  But that would be much
more complicated.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
v4: New patch
---
 sg-run-job               |    2 ++
 tcl/JobDB-Executive.tcl  |    6 ++++++
 tcl/JobDB-Standalone.tcl |    1 +
 3 files changed, 9 insertions(+)

diff --git a/sg-run-job b/sg-run-job
index c51a508..66145b8 100755
--- a/sg-run-job
+++ b/sg-run-job
@@ -71,6 +71,8 @@ proc run-job {job} {
 
     if {$ok} { setstatus pass                                             }
 
+    if {$need_build_host && $ok} { jobdb::preserve-task 90 }
+
     if {$anyfailed} {
         jobdb::logputs stdout "at least one test failed"
     }
diff --git a/tcl/JobDB-Executive.tcl b/tcl/JobDB-Executive.tcl
index d61d2a2..f37bbaf 100644
--- a/tcl/JobDB-Executive.tcl
+++ b/tcl/JobDB-Executive.tcl
@@ -280,6 +280,12 @@ proc become-task {comment} {
     }
 }
 
+proc preserve-task {seconds} {
+    # This keeps the owner daemon connection open: our `sleep'
+    # will continue to own our resources for $seconds longer
+    exec setsid sleep $seconds > /dev/null < /dev/null 2> /dev/null &
+}
+
 proc step-log-filename {flight job stepno ts} {
     global c
     set logdir $c(Logs)/$flight/$job
diff --git a/tcl/JobDB-Standalone.tcl b/tcl/JobDB-Standalone.tcl
index a2b8dd9..d7d8422 100644
--- a/tcl/JobDB-Standalone.tcl
+++ b/tcl/JobDB-Standalone.tcl
@@ -74,6 +74,7 @@ proc step-set-status {flight job stepno st} {
 }
 
 proc become-task {argv} { }
+proc preserve-task {argv} { }
 
 proc step-log-filename {flight job stepno ts} {
     return {}
-- 
1.7.10.4

Re: [OSSTEST PATCH 28/28] Executive: Delay releasing build host shares by 90s

[Ian Campbell]

On Tue, 2015-09-22 at 16:12 +0100, Ian Jackson wrote:
> When a build job finishes, the same flight may well want to do a
> subsequent build that depended on the first.  When this happens, we
> have a race:
> 
> One the one hand, we have the flight: after sg-run-job exits,
> sg-execute-flight needs to double-check the job status, and search the
> flight for more jobs to run; it will spawn ts-allocate-hosts-Executive
> for the new job, which needs to get its head together, parse its
> arguments, become a client of the queue daemon, and ask to be put in
> the queue.
> 
> On the other hand, we have the planning system: currently, as soon as
> sg-run-job exits, the connection to the ownerdaemon closes.  The
> ownerdaemon tells the queue daemon, and the planning queue is
> restarted.  It might even happen that coincidentally the planning
> queue is about to start.
> 
> If the planning system wins the race, another job will pick up the
> newly-freed resource.  Often this will mean unsharing the build host,
> which is very wasteful if the releasing flight hasn't finished its
> builds for that architecture: it means that the next build job needs
> to regroove a host for builds.
> 
> Add a bodge to try to make the race go the other way: after a build
> job completes successfuly, do not give up the share for a further 90
> seconds.  (We have to use setsid because sg-execute-flight kills the
> process group to clean up stray processes, which this sleep definitely
> is.)
> 
> A better solution would be to move the wait-for-referenced-job logic
> from sg-execute-flight to ts-hosts-allocate-*.  But that would be much
> more complicated.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>