[v12,29/33] ts-freebsd-host-install: add arguments to test memdisk append options

Commit Message

Roger Pau Monné Oct. 20, 2017, 1:47 p.m. UTC

This is needed in order to figure out which memdisk options should be
used to boot the images on each specific box.

Note that when passed the --recordappend argument upon success the
script stores the tentative host property in the runvars.

Signed-off-by: Roger Pau Monné <[e-mail]>
---
Changes since v2:
 - Fix commit message.

Changes since v1:
 - Provide a --recordappend argument to force the recording the
   memdisk parameters.
 - Exit gracefully if a bootonly test is attempted against a
   non-supported architecture.
 - Use NONE instead of an empty string when calling
   setup_netboot_memdisk if nothing should be appended.
 - Do not perform any arch test in ts-freebsd-host-install.
---
 ts-freebsd-host-install | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

Comments

Ian Jackson Oct. 20, 2017, 2:44 p.m. UTC | #1

Roger Pau Monne writes ("[PATCH v12 29/33] ts-freebsd-host-install: add arguments to test memdisk append options"):
> This is needed in order to figure out which memdisk options should be
> used to boot the images on each specific box.
> 
> Note that when passed the --recordappend argument upon success the
> script stores the tentative host property in the runvars.

Last time, I said:

  Roger Pau Monne writes ("[PATCH OSSTEST v2 07/11] ts-freebsd-host-install: add arguments to test memdisk append options"):
  > This is needed in order to figure out which memdisk options should be
  > used to boot the images on each specific box.
  > 
  > Note that upon success the script stores the tentative host property
  > in the runvars.
  ...
  > +    } elsif ($ARGV[0] eq "--recordappend") {
  > +        $record_append = 1;
  ...
  > +if ($bootonly) {
  > +    hostprop_putative_record($ho, "MemdiskAppend", $memdisk_append)
  > +        if $record_append;
  > +    exit 0;

  This is surely wrong.

The same code seems to be here, unchanged:

> +if ($bootonly) {
> +    hostprop_putative_record($ho, "MemdiskAppend", $memdisk_append)
> +        if $record_append;
> +    exit 0;
> +}

What I mean is that you only do the work for $record_append if
$bootonly is also set.  If --record-append is meaningful only with
--test-boot then you should die if it's specified without --test-boot.

Also, I have just noticed that the option names ought to be
   --record-append --test-boot --memdisk-append
(ie with the hyphens that are conventional in multi-word long option
names).

Ian.

Patch

diff --git a/ts-freebsd-host-install b/ts-freebsd-host-install
index 483b9aec..50af5dd3 100755
--- a/ts-freebsd-host-install
+++ b/ts-freebsd-host-install
@@ -41,6 +41,23 @@  use Osstest::TestSupport;
 
 tsreadconfig();
 
+our $bootonly;
+our $memdisk_append;
+our $record_append;
+while (@ARGV && $ARGV[0] =~ m/^-/g) {
+    if ($ARGV[0] =~ m/^--memdiskappend=(.*)/) {
+        $memdisk_append = $1;
+    } elsif ($ARGV[0] eq "--testboot") {
+        $memdisk_append //= "NONE";
+        $bootonly = 1;
+    } elsif ($ARGV[0] eq "--recordappend") {
+        $record_append = 1;
+    } else {
+        die "Unknown argument $ARGV[0]";
+    }
+    shift @ARGV;
+}
+
 our ($whhost) = @ARGV;
 $whhost ||= 'host';
 our $ho= selecthost($whhost);
@@ -95,7 +112,7 @@  END
 
     # Setup the pxelinux config file
     logm("Booting from installer image at $pxeimg");
-    setup_netboot_memdisk($ho, $pxeimg);
+    setup_netboot_memdisk($ho, $pxeimg, $memdisk_append);
 }
 
 sub install () {
@@ -247,6 +264,12 @@  power_state($ho, 1);
 logm("Waiting for the installer to boot");
 await_tcp(get_timeout($ho,'reboot',$timeout), 5, $ho);
 
+if ($bootonly) {
+    hostprop_putative_record($ho, "MemdiskAppend", $memdisk_append)
+        if $record_append;
+    exit 0;
+}
+
 # Next boot will be from local disk
 setup_netboot_local($ho);