[Xen-devel] [OSSTEST PATCH 1/3] cs-adjust-flight: Provide runvar-set-default

[Xen-devel] [OSSTEST PATCH 1/3] cs-adjust-flight: Provide runvar-set-default

[Ian Jackson]

No change to existing code.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 cs-adjust-flight | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/cs-adjust-flight b/cs-adjust-flight
index ae342506..98d40891 100755
--- a/cs-adjust-flight
+++ b/cs-adjust-flight
@@ -11,6 +11,7 @@
 #   jobs-list <job-spec>
 #   jobs-del <job-spec>
 #   runvar-set <job-spec> <var-spec> <value>
+#   runvar-set-default <job-spec> <var-spec> <value>
 #   runvar-del <job-spec> <var-spec>
 #   runvar-change <job-spec> <var-spec> <old-value> <new-value>
 #   runvar-perlop <job-spec> <var-spec> <perl-expr>
@@ -260,6 +261,10 @@ our $runvar_rm_q = db_prepare
 our $runvar_insert_q = db_prepare
     ("INSERT INTO runvars (flight, job, name, val, synth)".
      " VALUES (?, ?, ?, ?, 'f')");
+our $runvar_insert_default_q = db_prepare
+    ("INSERT INTO runvars (flight, job, name, val, synth)".
+     " VALUES (?, ?, ?, ?, 'f')".
+     " ON CONFLICT DO NOTHING");
 
 sub runvar_set ($$$;$) {
     my ($job, $name, $val, $xwhat) = @_;
@@ -270,6 +275,16 @@ sub runvar_set ($$$;$) {
     verbose "\n";
 }
 
+sub runvar_set_default ($$$;$) {
+    my ($job, $name, $val, $xwhat) = @_;
+    my $y = $runvar_insert_default_q->execute($dstflight, $job, $name, $val);
+    if ($y) {
+	verbose "$dstflight.$job $name := \`$val'";
+	verbose $xwhat if defined $xwhat;
+	verbose "\n";
+    }
+}
+
 sub for_runvars ($$$$) {
     my ($jobspec, $varspec, $fn, $ifnone) = @_;
     # calls $fn->($jobname, $varname, $varrow)
@@ -306,6 +321,18 @@ sub change__runvar_set {
     }, 'ANYWAY');
 }
 
+sub change__runvar_set_default {
+    die unless @changes >= 3;
+    my $jobs = shift @changes;
+    my $name = shift @changes;
+    my $val = shift @changes;
+
+    for_jobs($dstflight, $jobs, sub {
+        my ($job) = @_;
+        runvar_set_default($job, $name, $val);
+    }, 'ANYWAY');
+}
+
 sub change__runvar_del {
     die unless @changes >= 2;
     my $jobs = shift @changes;
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [OSSTEST PATCH 2/3] guest_prepare_disk: Only do the umount if we set an env var

[Ian Jackson]

This call to guest_umount_lv is here for the benefit of ad-hoc reruns
of (eg) ts-guest-start tidy up any ad-hoc messing about (eg from
earlier runs of ts-debian-fixup or something).  It is not needed in
production runs.

Serendipitously, this osstest code discovered a bug in the Linux
blkback: when tearing down, it sets the backend state to 6 before it
has closed the underlying block devices.  This ultimately means that
after "xl destroy" or "xl shutdown -w" there is a period when the
guest's open handle onto its storage is still open.  This is wrong.

This detection depends on us winning a tricky race.  So it shows up in
osstest as a very low probability heisenbug.  The bug is currently in
all versions of Linux and causing a bit of a nuisance.

It would be best to add a proper check for this bug.  However, this is
quite fiddly: really, it ought to be done as close to the xl command
completion as possible, in the same ssh invocation.  That would
involve a fair bit of plumbing and ad-hocery.  I don't think that
would be proportionate for such a low-impact bug.

So instead in this patch I just disable this cleanup code in the
troublesome case, unless it is explicitly requested by the user
setting OSSTEST_GUEST_DISK_MOUNT_CLEANUP to a trueish value.  (This
would be reasonably convenient for the ad-hoc testing that this call
serves.)

Thanks to Roger for diagnosing the Linux kernel bug.

CC: Jürgen Groß <jgross@suse.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Roger Pau Monne <roger.pau@citrix.com>
---
 Osstest/TestSupport.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index 78f47480..6b0ee7a2 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -1938,7 +1938,8 @@ sub guest_create_paused ($) {
 sub guest_prepare_disk ($) {
     my ($gho) = @_;
 
-    guest_umount_lv($gho->{Host}, $gho);
+    guest_umount_lv($gho->{Host}, $gho)
+	if $ENV{'OSSTEST_GUEST_DISK_MOUNT_CLEANUP'};
 
     return if ($gho->{Diskfmt} // 'none') eq "none";
 
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [OSSTEST PATCH 3/3] Toolstack/xl: Wrap a long command

[Ian Jackson]

No functional change.

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

diff --git a/Osstest/Toolstack/xl.pm b/Osstest/Toolstack/xl.pm
index d31af8c0..85972753 100644
--- a/Osstest/Toolstack/xl.pm
+++ b/Osstest/Toolstack/xl.pm
@@ -78,7 +78,8 @@ sub shutdown_wait ($$$) {
     my $gn = $gho->{Name};
     my $acpi_fallback = guest_var($gho,'acpi_shutdown','false') eq 'true'
 	&& $self->{Name} eq 'xl' ? "F" : "";
-    target_cmd_root($ho,"$self->{_Command} shutdown -w${acpi_fallback} $gn", $timeout);
+    target_cmd_root($ho,"$self->{_Command} shutdown -w${acpi_fallback} $gn",
+		    $timeout);
 }
 
 sub _check_for_command($$) {
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel