diff mbox

[OSSTEST,07/11] ts-freebsd-host-install: add option to test memdisk options

Message ID 20170728152637.20301-8-roger.pau@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Roger Pau Monne July 28, 2017, 3:26 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 upon success the script stores the tentative host property
in the runvars.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 ts-freebsd-host-install | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

Comments

Ian Jackson July 28, 2017, 3:45 p.m. UTC | #1
Roger Pau Monne writes ("[PATCH OSSTEST 07/11] ts-freebsd-host-install: add option to test memdisk options"):
> This is needed in order to figure out which memdisk options should be
> used to boot the images on each specific box.
...
> +if ($r{'arch'} !~ m/amd64/g) {
> +    logm("Arch $r{'arch'} not supported!");

This clearly can't be right because presumably at least i386 would
work too.  I don't know why you need this check.

> +    exit 0;

WTF ?  You don't want ts-freebsd-host-install to exit 0 if it gets an
unknown architecture.

...  Oh I see.  You have misplaced this check, which should only be
effective when we are testing boot arguments.

Ian.
Ian Jackson July 28, 2017, 3:46 p.m. UTC | #2
Roger Pau Monne writes ("[PATCH OSSTEST 07/11] ts-freebsd-host-install: add option to test memdisk options"):
...
> +if ($bootonly) {
> +    hostprop_putative_record($ho, "MemdiskAppend", $memdisk_append)
> +        if $memdisk_append;
> +    exit 0;

I think this should be a separate option --record-memdisk or
something.  Since you might want to do a boot test for other reasons
and having it mess with the host properties would be very unexpected.

Ian.
Roger Pau Monne Aug. 1, 2017, 8:58 a.m. UTC | #3
On Fri, Jul 28, 2017 at 04:45:20PM +0100, Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH OSSTEST 07/11] ts-freebsd-host-install: add option to test memdisk options"):
> > This is needed in order to figure out which memdisk options should be
> > used to boot the images on each specific box.
> ...
> > +if ($r{'arch'} !~ m/amd64/g) {
> > +    logm("Arch $r{'arch'} not supported!");
> 
> This clearly can't be right because presumably at least i386 would
> work too.  I don't know why you need this check.

Yes, we could test memdisk with i386 also, except that osstest doesn't
generate i386 images yet.

> > +    exit 0;
> 
> WTF ?  You don't want ts-freebsd-host-install to exit 0 if it gets an
> unknown architecture.
>
> ...  Oh I see.  You have misplaced this check, which should only be
> effective when we are testing boot arguments.

Yes, we cannot test this for ARM (or else it's going to fail and
block the other steps of the examine job), that's why it returns 0.

So I think this should indeed be conditional on bootonly being set.

Roger.
diff mbox

Patch

diff --git a/ts-freebsd-host-install b/ts-freebsd-host-install
index 483b9aec..3b0ab970 100755
--- a/ts-freebsd-host-install
+++ b/ts-freebsd-host-install
@@ -41,6 +41,25 @@  use Osstest::TestSupport;
 
 tsreadconfig();
 
+if ($r{'arch'} !~ m/amd64/g) {
+    logm("Arch $r{'arch'} not supported!");
+    exit 0;
+}
+
+our $bootonly;
+our $memdisk_append;
+while (@ARGV && $ARGV[0] =~ m/^-/g) {
+    if ($ARGV[0] =~ m/^--memdiskappend=(.*)/) {
+        $memdisk_append = $1;
+    } elsif ($ARGV[0] eq "--testboot") {
+        $memdisk_append //= "";
+        $bootonly = 1;
+    } else {
+        die "Unknown argument $ARGV[0]";
+    }
+    shift @ARGV;
+}
+
 our ($whhost) = @ARGV;
 $whhost ||= 'host';
 our $ho= selecthost($whhost);
@@ -95,7 +114,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 +266,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 $memdisk_append;
+    exit 0;
+}
+
 # Next boot will be from local disk
 setup_netboot_local($ho);