All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] osstest: initial FreeBSD support
@ 2017-05-23 13:51 Roger Pau Monne
  2017-05-23 13:51 ` [PATCH 1/7] osstest: make built_stash_file store a path_ runvar for each file Roger Pau Monne
                   ` (6 more replies)
  0 siblings, 7 replies; 19+ messages in thread
From: Roger Pau Monne @ 2017-05-23 13:51 UTC (permalink / raw)
  To: xen-devel, Ian.Jackson

Hello,

This series introduces initial FreeBSD host support to osstest. The current
series allow installing a bare-metal host with FreeBSD and building FreeBSD on
it in order to generate new install media that can be fed into the installer
script.

This is still very limited, since no Xen testing is done on those hosts,
however it sets the base to add a Xen build test for FreeBSD.

Note that it should be quite easy to add more steps to the build script so that
FreeBSD VM images are also generated, that could be used by osstest.

I've tried to add a detailed commit log at each relevant patch, so not much
more to add here in the cover letter.

The series can also be found on my git repo:

git://xenbits.xen.org/people/royger/osstest.git freebsd_v1

Thanks, Roger.


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/7] osstest: make built_stash_file store a path_ runvar for each file
  2017-05-23 13:51 [PATCH 0/7] osstest: initial FreeBSD support Roger Pau Monne
@ 2017-05-23 13:51 ` Roger Pau Monne
  2017-05-23 14:55   ` Ian Jackson
  2017-05-23 13:51 ` [PATCH 2/7] osstest: move known_hosts generation to TestSupport Roger Pau Monne
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Roger Pau Monne @ 2017-05-23 13:51 UTC (permalink / raw)
  To: xen-devel, Ian.Jackson; +Cc: Ian Jackson, Roger Pau Monne

And introduce built_stash_debugfile in order the keep the previous behavior of
built_stash_file.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
---
 Osstest/TestSupport.pm | 14 ++++++++++++--
 ts-kernel-build        |  4 ++--
 ts-xen-build           |  8 ++++----
 3 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index c23ac135..5f13eb0b 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -83,7 +83,7 @@ BEGIN {
                       get_stashed open_unique_stashfile compress_stashed
                       dir_identify_vcs
                       build_url_vcs build_clone
-                      built_stash built_stash_file
+                      built_stash built_stash_file built_stash_debugfile
                       built_compress_stashed
                       hg_dir_revision git_dir_revision vcs_dir_revision
                       store_revision store_vcs_revision
@@ -1445,7 +1445,7 @@ END
     store_runvar("path_$item", $stashleaf);
 }
 
-sub built_stash_file ($$$$;$) {
+sub built_stash_debugfile ($$$$;$) {
     my ($ho, $builddir, $item, $fname, $optional) = @_;
     my $build= "build";
     my $stashleaf= "$build/$item";
@@ -1458,6 +1458,16 @@ sub built_stash_file ($$$$;$) {
                    "$stash/$stashleaf");
 }
 
+sub built_stash_file ($$$$;$) {
+    my ($ho, $builddir, $item, $fname, $optional) = @_;
+    my $build= "build";
+    my $stashleaf= "$build/$item";
+
+    built_stash_debugfile($ho, $builddir, $item, $fname, $optional);
+    store_runvar("path_$item", $stashleaf);
+}
+
+
 sub built_compress_stashed($) {
     my ($path) = @_;
     compress_stashed("build/$path");
diff --git a/ts-kernel-build b/ts-kernel-build
index 94e67a47..5b87f5a7 100755
--- a/ts-kernel-build
+++ b/ts-kernel-build
@@ -438,9 +438,9 @@ if ($r{tree_linuxfirmware}) {
     fwinstall();
 }
 built_stash($ho, $builddir, 'dist', 'kerndist');
-built_stash_file($ho, $builddir, 'vmlinux', 'linux/vmlinux');
+built_stash_debugfile($ho, $builddir, 'vmlinux', 'linux/vmlinux');
 built_compress_stashed('vmlinux');
-built_stash_file($ho, $builddir, 'config', 'linux/.config');
+built_stash_debugfile($ho, $builddir, 'config', 'linux/.config');
 
 sub enable_xen_config () {
     return <<'END';
diff --git a/ts-xen-build b/ts-xen-build
index 31acb9dd..0152ea05 100755
--- a/ts-xen-build
+++ b/ts-xen-build
@@ -209,10 +209,10 @@ sub stash () {
                     "xen/dist/${part}install",
                     "${part}dist");
     }
-    built_stash_file($ho, $builddir, "xen-syms", "xen/xen/xen-syms", 1);
-    built_stash_file($ho, $builddir, "xen-config", "xen/.config", 1);
-    built_stash_file($ho, $builddir, "xen-hv-config", "xen/xen/.config", 1);
-    built_stash_file($ho, $builddir, "seabios-config",
+    built_stash_debugfile($ho, $builddir, "xen-syms", "xen/xen/xen-syms", 1);
+    built_stash_debugfile($ho, $builddir, "xen-config", "xen/.config", 1);
+    built_stash_debugfile($ho, $builddir, "xen-hv-config", "xen/xen/.config", 1);
+    built_stash_debugfile($ho, $builddir, "seabios-config",
 		     "xen/tools/firmware/seabios-dir-remote/.config", 1);
     built_compress_stashed("xen-syms");
 }
-- 
2.11.0 (Apple Git-81)


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/7] osstest: move known_hosts generation to TestSupport
  2017-05-23 13:51 [PATCH 0/7] osstest: initial FreeBSD support Roger Pau Monne
  2017-05-23 13:51 ` [PATCH 1/7] osstest: make built_stash_file store a path_ runvar for each file Roger Pau Monne
@ 2017-05-23 13:51 ` Roger Pau Monne
  2017-05-23 14:56   ` Ian Jackson
  2017-05-23 13:51 ` [PATCH 3/7] osstest: fix regular expression used to match buildjob in ts-build-check Roger Pau Monne
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Roger Pau Monne @ 2017-05-23 13:51 UTC (permalink / raw)
  To: xen-devel, Ian.Jackson; +Cc: Roger Pau Monne

This is equivalent to the already existing authorized_keys function, and
generates the contents of the known_hosts file that should be installed on
targets.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 Osstest/Debian.pm      | 36 +-----------------------------------
 Osstest/TestSupport.pm | 41 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 41 insertions(+), 36 deletions(-)

diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
index 8ba48bfa..87539822 100644
--- a/Osstest/Debian.pm
+++ b/Osstest/Debian.pm
@@ -709,41 +709,7 @@ sub preseed_ssh ($$) {
     my ($ho,$sfx) = @_;
 
     my $authkeys_url= create_webfile($ho, "authkeys$sfx", authorized_keys());
-
-    my $hostkeyfile= "$c{OverlayLocal}/etc/ssh/ssh_host_rsa_key.pub";
-    my $hostkey= get_filecontents($hostkeyfile);
-    chomp($hostkey); $hostkey.="\n";
-    my $knownhosts= '';
-
-    my $hostsq= $dbh_tests->prepare(<<END);
-        SELECT val FROM runvars
-         WHERE flight=? AND name LIKE '%host'
-         GROUP BY val
-END
-    $hostsq->execute($flight);
-    while (my ($node) = $hostsq->fetchrow_array()) {
-        my $defaultfqdn = $node;
-        $defaultfqdn .= ".$c{TestHostDomain}" unless $defaultfqdn =~ m/\./;
-
-        my %props;
-        $mhostdb->get_properties($node, \%props);
-
-        my $longname= $props{Fqdn} // $defaultfqdn;
-        my (@hostent)= gethostbyname($longname);
-        if (!@hostent) {
-            logm("skipping host key for nonexistent host $longname");
-            next;
-        }
-        my $specs= join ',', $longname, $node, map {
-            join '.', unpack 'W4', $_;
-        } @hostent[4..$#hostent];
-        logm("adding host key for $specs");
-        $knownhosts.= "$specs ".$hostkey;
-    }
-    $hostsq->finish();
-
-    $knownhosts.= "localhost,127.0.0.1 ".$hostkey;
-    my $knownhosts_url= create_webfile($ho, "known_hosts$sfx", $knownhosts);
+    my $knownhosts_url= create_webfile($ho, "known_hosts$sfx", known_hosts());
 
     preseed_hook_command($ho, 'late_command', $sfx, <<END);
 #!/bin/sh
diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index 5f13eb0b..8c7078c5 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -89,7 +89,7 @@ BEGIN {
                       store_revision store_vcs_revision
                       git_massage_url
 
-                      sshopts authorized_keys
+                      sshopts authorized_keys known_hosts
                       cfg_tftp_di_version
                       remote_perl_script_open remote_perl_script_done
                       host_reboot target_reboot target_reboot_hard            
@@ -2356,6 +2356,45 @@ sub authorized_keys () {
     return $authkeys;
 }
 
+sub known_hosts () {
+    my $hostkeyfile= "$c{OverlayLocal}/etc/ssh/ssh_host_rsa_key.pub";
+    my $hostkey= get_filecontents($hostkeyfile);
+    chomp($hostkey); $hostkey.="\n";
+
+    my $knownhosts= '';
+
+    my $hostsq= $dbh_tests->prepare(<<END);
+        SELECT val FROM runvars
+         WHERE flight=? AND name LIKE '%host'
+         GROUP BY val
+END
+    $hostsq->execute($flight);
+    while (my ($node) = $hostsq->fetchrow_array()) {
+        my $defaultfqdn = $node;
+        $defaultfqdn .= ".$c{TestHostDomain}" unless $defaultfqdn =~ m/\./;
+
+        my %props;
+        $mhostdb->get_properties($node, \%props);
+
+        my $longname= $props{Fqdn} // $defaultfqdn;
+        my (@hostent)= gethostbyname($longname);
+        if (!@hostent) {
+            logm("skipping host key for nonexistent host $longname");
+            next;
+        }
+        my $specs= join ',', $longname, $node, map {
+            join '.', unpack 'W4', $_;
+        } @hostent[4..$#hostent];
+        logm("adding host key for $specs");
+        $knownhosts.= "$specs ".$hostkey;
+    }
+    $hostsq->finish();
+
+    $knownhosts.= "localhost,127.0.0.1 ".$hostkey;
+
+    return $knownhosts;
+}
+
 sub cfg_tftp_di_version ($) {
     my ($suite) = @_;
     $suite //= 'x def suite'; # will not find $c{...}
-- 
2.11.0 (Apple Git-81)


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 3/7] osstest: fix regular expression used to match buildjob in ts-build-check
  2017-05-23 13:51 [PATCH 0/7] osstest: initial FreeBSD support Roger Pau Monne
  2017-05-23 13:51 ` [PATCH 1/7] osstest: make built_stash_file store a path_ runvar for each file Roger Pau Monne
  2017-05-23 13:51 ` [PATCH 2/7] osstest: move known_hosts generation to TestSupport Roger Pau Monne
@ 2017-05-23 13:51 ` Roger Pau Monne
  2017-05-23 15:01   ` Ian Jackson
  2017-05-23 13:51 ` [PATCH 4/7] osstest: add a FreeBSD host install recipe Roger Pau Monne
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Roger Pau Monne @ 2017-05-23 13:51 UTC (permalink / raw)
  To: xen-devel, Ian.Jackson; +Cc: Roger Pau Monne

Current regular expression used to match the buildjob works correctly when the
buildjob runvar has the <job_name>buildjob format, but not when the format is
<job_name>_buildjob (the first match group is empty in this case). Change it so
that it works for both formats.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 ts-build-check | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ts-build-check b/ts-build-check
index 92e19fb0..b9ade876 100755
--- a/ts-build-check
+++ b/ts-build-check
@@ -26,7 +26,7 @@ die if @ARGV && $ARGV[0] =~ m/^-/;
 logm("checking builds ...");
 
 foreach my $k (sort keys %r) {
-    next unless $k =~ m/^(?:.*_)?([^_]*)buildjob$/;
+    next unless $k =~ m/^(.*?)(_?)buildjob$/;
     my $part= $1;
     my $path= "path_${part}dist";
     logm("checking $k $path");
-- 
2.11.0 (Apple Git-81)


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 4/7] osstest: add a FreeBSD host install recipe
  2017-05-23 13:51 [PATCH 0/7] osstest: initial FreeBSD support Roger Pau Monne
                   ` (2 preceding siblings ...)
  2017-05-23 13:51 ` [PATCH 3/7] osstest: fix regular expression used to match buildjob in ts-build-check Roger Pau Monne
@ 2017-05-23 13:51 ` Roger Pau Monne
  2017-05-23 16:04   ` Ian Jackson
  2017-05-23 17:14   ` Ian Jackson
  2017-05-23 13:51 ` [PATCH 5/7] osstest: introduce a FreeBSD build script Roger Pau Monne
                   ` (2 subsequent siblings)
  6 siblings, 2 replies; 19+ messages in thread
From: Roger Pau Monne @ 2017-05-23 13:51 UTC (permalink / raw)
  To: xen-devel, Ian.Jackson; +Cc: Roger Pau Monne

The installation is performed using the bsdinstall tool, which is part of the
FreeBSD base system. The installer image is setup with the osstest ssh keys and
sshd enabled by default, which allows the test harness to just ssh into the
box, create the install config file and launch the scripted install.

Currently the installation is done with ZFS only, in stripe mode, and a single
disk.

In order to support the FreeBSD installer a new method is added, that allows
setting the pxe boot of a host using a memdisk. Also, a new tftp path is added
in order to point to the place where the FreeBSD installed images should be
stored.

The install script either picks the binary images from the output of a previous
FreeBSD buildjob (yet to be introduced), or from the freebsd_image and
freebsd_sets runvars, that should point to a FreeBSD installer image and to the
folder that contain the install sets respectively.

When relying on the output from a previous FreeBSD buildjob (freebsd_buildjob
runvar is set), the install image is picked from the path_freebsd-image runvar,
and the sets from path_freebsd-{base,kernel,manifest} of the previous job.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 Osstest.pm              |   1 +
 Osstest/TestSupport.pm  |  18 +++-
 ts-freebsd-host-install | 259 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 276 insertions(+), 2 deletions(-)
 create mode 100755 ts-freebsd-host-install

diff --git a/Osstest.pm b/Osstest.pm
index a78728cd..a0c0339c 100644
--- a/Osstest.pm
+++ b/Osstest.pm
@@ -227,6 +227,7 @@ sub readglobalconfig () {
     $c{TftpTmpDir} ||= "$c{TftpPlayDir}tmp/";
 
     $c{TftpDiBase} ||= "$c{TftpPlayDir}debian-installer";
+    $c{TftpFreeBSDBase} ||= "$c{TftpPlayDir}freebsd-installer";
     $c{TftpDiVersion} ||= $c{ "TftpDiVersion_$c{DebianSuite}" } // 'current';
 
     $c{TftpGrubBase} ||= "$c{TftpPlayDir}grub";
diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index 8c7078c5..4418f024 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -118,7 +118,7 @@ BEGIN {
                       await_webspace_fetch_byleaf create_webfile
                       file_link_contents get_timeout
                       setup_netboot_di setup_netboot_local host_netboot_file
-		      subst_netboot_template
+                      subst_netboot_template setup_netboot_memdisk
 
                       ether_prefix
 
@@ -1032,7 +1032,7 @@ sub selecthost ($) {
     $ho->{Tftp} = { };
     $ho->{Tftp}{$_} = $c{"Tftp${_}_${tftpscope}"} || $c{"Tftp${_}"}
         foreach qw(Path TmpDir PxeDir NetbootGroup PxeTemplates PxeTemplatesReal
-                   DiBase GrubBase
+                   DiBase GrubBase FreeBSDBase
                    NetGrubDir NetGrubTemplates NetGrubTemplatesReal
                    PxeGroup);
     $ho->{TftpNetbootGroup} //= $ho->{TftpPxeGroup}; # compatibility
@@ -2553,6 +2553,20 @@ default local
 END
 }
 
+sub setup_netboot_memdisk ($$) {
+    my ($ho, $img) = @_;
+    setup_netboot_bootcfg($ho, <<END);
+serial 0 $c{Baud}
+timeout 5
+label overwrite
+	menu label ^Overwrite
+	menu default
+	kernel memdisk
+	append initrd=$img
+default overwrite
+END
+}
+
 # uboot emulates pxelinux, so reuse BIOS stuff
 sub setup_netboot_di_uboot ($$$$$;%) { return &setup_netboot_di_bios; }
 sub setup_netboot_local_uboot ($) { return &setup_netboot_local_bios; }
diff --git a/ts-freebsd-host-install b/ts-freebsd-host-install
new file mode 100755
index 00000000..d59b66a4
--- /dev/null
+++ b/ts-freebsd-host-install
@@ -0,0 +1,259 @@
+#!/usr/bin/perl -w
+# This is part of "osstest", an automated testing framework for Xen.
+# Copyright (C) 2017 Citrix Inc.
+# 
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+# 
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+# 
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+use strict qw(vars);
+use DBI;
+use POSIX;
+
+unshift @INC, qw(.);
+use Osstest;
+use Osstest::TestSupport;
+
+tsreadconfig();
+
+our %xopts;
+
+our ($whhost) = @ARGV;
+$whhost ||= 'host';
+our $ho= selecthost($whhost);
+exit 0 if $ho->{Flags}{'no-reinstall'};
+exit 0 if $ho->{SharedReady};
+
+our %timeout= qw(Sshd 1000);
+
+our @sets = qw(base.txz kernel.txz);
+
+our $image = $r{"freebsd_image"} ||
+             get_stashed("path_freebsd-image", $r{"freebsd_buildjob"});
+
+sub get_sets_path () {
+    my @paths;
+
+    foreach (@sets, "MANIFEST") {
+        my $path;
+
+        if (!defined($r{"freebsd_sets"})) {
+            # Get everything before the "." (ie: get base from base.txz)
+            my $stash_name = lc((split /\./)[0]);
+            $path = get_stashed("path_freebsd-$stash_name",
+                                $r{"freebsd_buildjob"});
+        } else {
+            $path = $r{"freebsd_sets"} . "/$_";
+        }
+
+        push @paths, { name => "$_", path => "$path" };
+    }
+
+    return @paths;
+}
+
+sub create_ssh_overlay () {
+    my $url = create_webfile($ho, "ssh.tar", sub {
+        my ($fh) = @_;
+        contents_make_cpio($fh, 'ustar',  "$c{OverlayLocal}/etc/ssh/");
+    });
+
+    return $url;
+}
+
+sub setup_netboot_installer () {
+    my $pxeimg = "$ho->{Tftp}{FreeBSDBase}/install-$ho->{Name}.img";
+    my $script = <<END;
+set -ex
+basedir=$ho->{Tftp}{Path}/$ho->{Tftp}{FreeBSDBase}/
+hash=`sha256sum $image|head -c 64`
+localpath="$r{arch}/\$hash/install.img";
+mkdir -p \$basedir
+(
+flock -x -w 600 200
+cd \$basedir
+if [ ! -f \$localpath ]; then
+    mkdir -p `dirname \$localpath`
+    cp $image \$localpath
+fi
+rm -f install-$ho->{Name}.img
+ln \$localpath install-$ho->{Name}.img
+for hash in `ls $r{arch}/`; do
+    count=`stat -c %h $r{arch}/\$hash/install.img`
+    if [ \$count -eq 1 ]; then
+        rm -rf $r{arch}/\$hash
+    fi
+done
+) 200<\$basedir
+END
+
+    logm("Executing:\n$script");
+    my $ret = system("/bin/bash -c '$script'");
+    $ret == 0 or die "Unable to setup netboot";
+
+    # Setup the pxelinux config file
+    logm("Booting from installer image at $pxeimg");
+    setup_netboot_memdisk($ho, $pxeimg);
+}
+
+sub install () {
+    my $authkeys = authorized_keys();
+    my $knownhosts = known_hosts();
+    my $sshd_keys_url = create_ssh_overlay();
+    my $disk_names = "ada0 da0 ad0";
+    my $installer_sets = join(" ", @sets);
+    my $target_sets = "/tmp/osstest_sets";
+    my $disk;
+    my $nic;
+
+    target_cmd_root($ho, 'chsh -s /bin/sh', 10);
+
+    logm("Trying to find a disk to install to");
+    $disk = target_cmd_output_root($ho, <<END, 30);
+for disk in $disk_names; do
+    if [ -c "/dev/\$disk" ]; then
+        echo \$disk
+        exit 0
+    fi
+done
+exit 1
+END
+    defined($disk) or die "Unable to find a valid disk";
+    logm("Using $disk as destination disk device");
+
+    logm("Trying to figure out primary nic device name");
+    $nic = target_cmd_output_root($ho, <<END, 30);
+nics=`ifconfig -l`
+for nic in \$nics; do
+    addr=`ifconfig \$nic inet|grep inet|awk {'print \$2'}`
+    if [ "\$addr" = "$ho->{Ip}" ]; then
+        echo \$nic
+        exit 0
+    fi
+done
+exit 1
+END
+    defined($nic) or die "Unable to find primary network interface";
+    logm("Using $nic as primary network interface");
+
+    logm("Uploading the install sets to the system");
+    target_cmd_root($ho, <<END, 30);
+mkdir -p $target_sets
+mount -o size=1G -t tmpfs tmpfs $target_sets
+END
+
+    foreach (get_sets_path()) {
+        target_putfile_root($ho, 600, $_->{path}, "$target_sets/$_->{name}");
+    }
+
+    logm("Creating the installer script");
+    target_cmd_root($ho, <<END, 60);
+        set -e
+        cat << ENDSCRIPT > installscript
+set -a
+BSDINSTALL_DISTDIR="$target_sets"
+ZFSBOOT_DISKS="$disk"
+DISTRIBUTIONS="$installer_sets"
+nonInteractive=1
+
+#!/bin/sh
+set -ex
+
+# Setup nic and sshd
+echo "ifconfig_$nic=DHCP" >> /etc/rc.conf
+echo "sshd_enable=YES" >> /etc/rc.conf
+
+# Disable sendmail
+echo "sendmail_enable=NONE" >> /etc/rc.conf
+
+# Set proxy for the pkg manager
+mkdir -p /usr/local/etc/
+cat << ENDPKG >> /usr/local/etc/pkg.conf
+pkg_env: { http_proxy = $c{HttpProxy} }
+default_always_yes: true
+assume_always_yes: true
+ENDPKG
+
+# Bootstap the package manager
+export HTTP_PROXY=$c{HttpProxy}
+export ASSUME_ALWAYS_YES=yes
+pkg update
+
+# Allow root user login and setup ssh keys
+chsh -s /bin/sh root
+echo 'PermitRootLogin yes' >> /etc/ssh/sshd_config
+mkdir -p /root/.ssh
+cat << ENDKEYS > /root/.ssh/authorized_keys
+$authkeys
+ENDKEYS
+cat << ENDHOSTS > /root/.ssh/known_hosts
+$knownhosts
+ENDHOSTS
+
+# Fetch host keys
+fetch $sshd_keys_url -o - | tar -xf - -C /etc/ssh/
+# Set correct permissions
+chown root:wheel /etc/ssh/ssh_host_*_key*
+chmod 0600 /etc/ssh/ssh_host_*_key
+chmod 0644 /etc/ssh/ssh_host_*_key.pub
+
+# Add a osstest user
+pw useradd osstest -m
+chsh -s /bin/sh osstest
+mkdir -p /home/osstest/.ssh
+cat << ENDKEYS > /home/osstest/.ssh/authorized_keys
+$authkeys
+ENDKEYS
+cat << ENDHOSTS > /home/osstest/.ssh/known_hosts
+$knownhosts
+ENDHOSTS
+
+# Setup serial console
+printf "%s" "-h -S$c{Baud}" >> /boot.config
+cat << ENDBOOT >> /boot/loader.conf
+boot_serial="YES"
+comconsole_speed="$c{Baud}"
+console="comconsole"
+boot_verbose="YES"
+beastie_disable="YES"
+ENDBOOT
+
+ENDSCRIPT
+END
+
+    logm("Launch the installer");
+    target_cmd_root($ho, 'bsdinstall script installscript', 1200);
+
+    target_reboot($ho);
+
+    logm("Waiting for the host to boot");
+    await_tcp(get_timeout($ho,'reboot',$timeout{Sshd}), 14, $ho);
+
+    logm("FreeBSD installed succesfully");
+}
+
+# Switch off, setup PXE and switch on to the installer
+power_state($ho, 0);
+setup_netboot_installer();
+power_cycle_sleep($ho);
+power_state($ho, 1);
+
+# Wait for the host to finish booting
+logm("Waiting for the installer to boot");
+await_tcp(get_timeout($ho,'reboot',$timeout{Sshd}), 14, $ho);
+
+# Next boot will be from local disk
+setup_netboot_local($ho);
+
+# Proceed with the install
+install();
+
-- 
2.11.0 (Apple Git-81)


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 5/7] osstest: introduce a FreeBSD build script
  2017-05-23 13:51 [PATCH 0/7] osstest: initial FreeBSD support Roger Pau Monne
                   ` (3 preceding siblings ...)
  2017-05-23 13:51 ` [PATCH 4/7] osstest: add a FreeBSD host install recipe Roger Pau Monne
@ 2017-05-23 13:51 ` Roger Pau Monne
  2017-05-23 17:58   ` Ian Jackson
  2017-05-23 13:51 ` [PATCH 6/7] osstest: add a FreeBSD build to flights Roger Pau Monne
  2017-05-23 13:51 ` [PATCH 7/7] osstest: introduce make-freebsd-flight Roger Pau Monne
  6 siblings, 1 reply; 19+ messages in thread
From: Roger Pau Monne @ 2017-05-23 13:51 UTC (permalink / raw)
  To: xen-devel, Ian.Jackson; +Cc: Roger Pau Monne

In order to generate the FreeBSD installer image and the install media.

The install sets are the vanilla ones generated by the 'ftp' release target.
The installer image is handcrafted based on the filesystem created by the
'bootonly' target, which is then populated with the ssh host keys, and setup
in order to use the serial console. The other difference from upstream FreeBSD
installer images is that the one built by osstest uses a ramdisk instead of
relying on the installer media to be somehow attached, either on a CD or USB
drive. This is required in order to boot the image from pxelinux (where no CD
or USB is actually attached to the host, and everything is fetched from tftp).

Due to the nature of the FreeBSD build, the outputs are different from what
osstest expects from a buildjob, more specifically there's no path_freebsdist
produced, and thus ts-build-check needs to be modified in order to accommodate
this. The FreeBSD build output is going to produce the following binaries
path_freebsd-{base,kernel,manifest,image}.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 ts-build-check   |  14 +++-
 ts-freebsd-build | 230 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 241 insertions(+), 3 deletions(-)
 create mode 100755 ts-freebsd-build

diff --git a/ts-build-check b/ts-build-check
index b9ade876..68d6cc2d 100755
--- a/ts-build-check
+++ b/ts-build-check
@@ -28,9 +28,17 @@ logm("checking builds ...");
 foreach my $k (sort keys %r) {
     next unless $k =~ m/^(.*?)(_?)buildjob$/;
     my $part= $1;
-    my $path= "path_${part}dist";
-    logm("checking $k $path");
-    get_stashed($path, $r{$k});
+    if ("$part" eq "freebsd") {
+        foreach (qw(base kernel manifest image)) {
+            my $path= "path_${part}-$_";
+            logm("checking $k $path");
+            get_stashed($path, $r{$k});
+        }
+    } else {
+        my $path= "path_${part}dist";
+        logm("checking $k $path");
+        get_stashed($path, $r{$k});
+    }
 }
 
 logm("all ok.");
diff --git a/ts-freebsd-build b/ts-freebsd-build
new file mode 100755
index 00000000..c0366c97
--- /dev/null
+++ b/ts-freebsd-build
@@ -0,0 +1,230 @@
+#!/usr/bin/perl -w
+# This is part of "osstest", an automated testing framework for Xen.
+# Copyright (C) 2017 Citrix Inc.
+# 
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+# 
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+# 
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+use strict qw(vars);
+use DBI;
+use POSIX;
+
+unshift @INC, qw(.);
+use Osstest;
+use Osstest::TestSupport;
+use Osstest::BuildSupport;
+
+tsreadconfig();
+
+selectbuildhost(\@ARGV);
+builddirsprops();
+
+sub install_deps () {
+    target_cmd_root($ho, 'pkg install git', 300);
+}
+
+sub checkout () {
+    prepbuilddirs();
+
+    # Remove the directory as root, there might be files owned by root
+    target_cmd_root($ho, <<END, 300);
+chflags -fR noschg $builddir/freebsd
+rm -rf $builddir/freebsd
+END
+
+    logm("Checkout the FreeBSD source tree");
+    build_clone($ho, 'freebsd', $builddir, 'freebsd');
+}
+
+sub build () {
+    my $authkeys = authorized_keys();
+    my $target = "bootonly";
+
+    # Build process as documented in the handbook:
+    # https://www.freebsd.org/doc/handbook/updating-src.html
+
+    logm("Cleaning up previous builds");
+    target_cmd_build($ho, 300, $builddir, <<END);
+cd freebsd
+export MAKEOBJDIRPREFIX=$builddir/obj
+make cleanworld
+END
+
+    logm("Building world");
+    target_cmd_build($ho, 7200, $builddir, <<END);
+cd freebsd
+export MAKEOBJDIRPREFIX=$builddir/obj
+(make $makeflags TARGET=$r{arch} buildworld 2>&1 && touch ../build-ok-stamp) \\
+    |tee ../log
+test -f ../build-ok-stamp
+echo ok.
+END
+
+    logm("Building kernel");
+    target_cmd_build($ho, 3600, $builddir, <<END);
+cd freebsd
+export MAKEOBJDIRPREFIX=$builddir/obj
+(make $makeflags TARGET=$r{arch} buildkernel 2>&1 && touch ../build-ok-stamp) \\
+    |tee ../log
+test -f ../build-ok-stamp
+echo ok.
+END
+
+    logm("Creating the install sets");
+    # NB: the steps below need to be done as root or the permissions
+    # of the files won't be properly set (and the target will fail).
+    target_cmd_root($ho, <<END, 900);
+cd $builddir/freebsd
+export MAKEOBJDIRPREFIX=$builddir/obj
+(make -C release TARGET=$r{arch} ftp 2>&1 && touch ../build-ok-stamp) \\
+    |tee ../log
+test -f ../build-ok-stamp
+echo ok.
+END
+
+    logm("Populating the installer image");
+    target_cmd_root($ho, <<END, 900);
+cd $builddir/freebsd
+export MAKEOBJDIRPREFIX=$builddir/obj
+(make -C release TARGET=$r{arch} $target 2>&1 && touch ../build-ok-stamp) \\
+    |tee ../log
+test -f ../build-ok-stamp
+echo ok.
+END
+
+    logm("Placing ssh host keys");
+    foreach my $file (<$c{OverlayLocal}/etc/ssh/ssh_host_*_key*>) {
+        target_putfile_root($ho, 30, $file,
+                            "$builddir/freebsd/release/$target/etc/ssh/");
+    }
+
+    logm("Configuring the installer image");
+    target_cmd_root($ho, <<END, 30);
+set -ex
+target=$builddir/freebsd/release/$target
+# Enable sshd by default
+echo 'sshd_enable="YES"' >> \$target/etc/rc.conf
+
+# Allow root login and copy the keys
+echo 'PermitRootLogin yes' >> \$target/etc/ssh/sshd_config
+mkdir -p \$target/root/.ssh
+cat << ENDKEYS > \$target/root/.ssh/authorized_keys
+$authkeys
+ENDKEYS
+
+# Set host keys permissions
+chown root:wheel \$target/etc/ssh/ssh_host_*_key*
+chmod 0600 \$target/etc/ssh/ssh_host_*_key
+chmod 0644 \$target/etc/ssh/ssh_host_*_key.pub
+
+# Setup serial console output for stage1
+printf "%s" "-h -S$c{Baud}" >> \$target/boot.config
+cat << ENDBOOT >> \$target/boot/loader.conf
+# Serial console configuration
+boot_serial="YES"
+comconsole_speed="$c{Baud}"
+console="comconsole"
+boot_verbose="YES"
+beastie_disable="YES"
+
+# mfs boot parameters
+geom_uzip_load="YES"
+tmpfs_load="YES"
+mfs_load="YES"
+mfs_type="mfs_root"
+mfs_name="/mfsroot"
+vfs.root.mountfrom="ufs:/dev/ufs/FreeBSD_Install"
+ENDBOOT
+
+# Enable DHCP on all network interfaces
+echo 'ifconfig_DEFAULT="DHCP"' >> \$target/etc/rc.conf
+
+# Remove the local script that launches the installer by default
+rm -rf \$target/etc/rc.local
+
+# Create a temporary fstab with the root dir
+echo '/dev/ufs/FreeBSD_Install / ufs rw 1 1' > \$target/etc/fstab
+
+# Remove the linked resolv.conf
+rm -rf \$target/etc/resolv.conf
+END
+
+    logm("Create the installer");
+    target_cmd_root($ho, <<END, 600);
+set -ex
+target=$builddir/freebsd/release/$target
+output=$builddir/install.img
+mkdir -p \$output.tmp
+
+# Do some pruning
+rm -rf \$target/usr/share/man
+rm -rf \$target/usr/share/examples
+rm -rf \$target/usr/share/doc
+rm -rf \$target/usr/share/dtrace
+
+# Create a mfs root image
+makefs -b 10% -B little -o label=FreeBSD_Install \$output.tmp/mfsroot \$target
+# Compress image
+gzip \$output.tmp/mfsroot
+
+# Copy boot to the staging dir
+cp -r \$target/boot \$output.tmp/
+cp \$target/boot.config \$output.tmp/
+
+# The loader only needs the tmpfs and geom_uzip modules in order
+# to boot into the mfs root, the rest of the modules can be loaded from
+# the mfs root itself.
+for module in `ls \$output.tmp/boot/kernel/*.ko`; do
+    if [ `basename "\$module"` != "geom_uzip.ko" ] && \\
+       [ `basename "\$module"` != "tmpfs.ko" ]; then
+        rm -f \$module
+    fi
+done
+
+makefs -B little \$output.part \$output.tmp
+
+# Make the image bootable
+mkimg -s gpt -b \$target/boot/pmbr -p efi:=\$target/boot/boot1.efifat \\
+    -p freebsd-boot:=\$target/boot/gptboot -p freebsd-ufs:=\$output.part \\
+    -p freebsd-swap::1M -o \$output
+
+rm \$output.part
+rm -rf \$output.tmp
+END
+}
+
+sub stash () {
+    logm("Stashing FreeBSD build output");
+    built_stash_file($ho, $builddir, "freebsd-image", "install.img", 0);
+    built_stash_file($ho, $builddir, "freebsd-manifest",
+                     "freebsd/release/ftp/MANIFEST", 0);
+    built_stash_file($ho, $builddir, "freebsd-base",
+                     "freebsd/release/ftp/base.txz", 0);
+    built_stash_file($ho, $builddir, "freebsd-kernel",
+                     "freebsd/release/ftp/kernel.txz", 0);
+    built_stash_debugfile($ho, $builddir, "freebsd-kernel-dbg",
+                          "freebsd/release/ftp/kernel-dbg.txz", 0);
+    my $srcversion = target_cmd_output_root($ho, <<END, 30);
+awk '/^\\\#define[[:space:]]*__FreeBSD_version/ { print \$3 }' \\
+    $builddir/freebsd/sys/sys/param.h | cut -c1-2
+END
+    store_runvar("freebsd_version", "$srcversion");
+}
+
+install_deps();
+checkout();
+build();
+stash();
+
+logm("FreeBSD build successful");
+
-- 
2.11.0 (Apple Git-81)


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 6/7] osstest: add a FreeBSD build to flights
  2017-05-23 13:51 [PATCH 0/7] osstest: initial FreeBSD support Roger Pau Monne
                   ` (4 preceding siblings ...)
  2017-05-23 13:51 ` [PATCH 5/7] osstest: introduce a FreeBSD build script Roger Pau Monne
@ 2017-05-23 13:51 ` Roger Pau Monne
  2017-05-23 13:51 ` [PATCH 7/7] osstest: introduce make-freebsd-flight Roger Pau Monne
  6 siblings, 0 replies; 19+ messages in thread
From: Roger Pau Monne @ 2017-05-23 13:51 UTC (permalink / raw)
  To: xen-devel, Ian.Jackson; +Cc: Roger Pau Monne

This requires changes in several places in order to accommodate the FreeBSD
build, which although it's a build job, it doesn't have the same set of
dependencies as the current builds.

First, a new build recipe is added to sg-run-job, and accordingly sg-run-job is
also made aware about the differences between a Linux and a FreeBSD build job.
A Linux build job requires ts-host-allocate + ts-host-install-twice +
ts-xen-build-prep, while a FreeBSD build job requires ts-host-allocate +
ts-freebsd-host-install.

All the current build jobs are kept to use the Linux build recipe, while the
newly added build-freebsd is using the new FreeBSD recipe.

cri-getconfig is also modified to introduce two new helpers, used to fetch a
runvar and a stashed file from a specific job. This is needed by the FreeBSD
build job creator in order to fetch information from a previous buildjob (so
the output from a previous buildjob can be used as input to a new buildjob).

Finally, the logic to create a FreeBSD build job is added to mfi-common. This
includes creating a FreeBSD build job, and also testing the output of that
build job (by creating another build job that depends on the output of the
first).

Note that the FreeBSD build job needs some input in order to setup a FreeBSD
host, and that can be fetched from different places:

1. Env variable FREEBSD_BUILDJOB: use the output from a previous
build-<arch>-freebsd.

2. Env variables FREEBSD_IMAGE, FREEBSD_SETS, FREEBSD_VERSION: set before
calling into make-flight, provide the installer image, the sets to install and
the version being installed.

3. Config file FreeBSDImage, FreeBSDSets and FreeBSDVersion: same as 2. except
that they are set on the config file.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 ap-common     |  4 ++++
 cri-getconfig | 37 ++++++++++++++++++++++++++++++++
 mfi-common    | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 sg-run-job    | 39 +++++++++++++++++++++++-----------
 4 files changed, 136 insertions(+), 12 deletions(-)

diff --git a/ap-common b/ap-common
index cbb815ce..d4fa7aef 100644
--- a/ap-common
+++ b/ap-common
@@ -37,6 +37,10 @@
 : ${PUSH_TREE_XTF:=$XENBITS:/home/xen/git/xtf.git}
 : ${BASE_TREE_XTF:=git://xenbits.xen.org/xtf.git}
 
+: ${TREE_FREEBSD:=git://github.com/freebsd/freebsd.git}
+: ${PUSH_TREE_FREEBSD:=$XENBITS:/home/xen/git/freebsd.git}
+: ${BASE_TREE_FREEBSD:=git://xenbits.xen.org/freebsd.git}
+
 : ${TREE_LIBVIRT:=git://libvirt.org/libvirt.git}
 : ${PUSH_TREE_LIBVIRT:=$XENBITS:/home/xen/git/libvirt.git}
 : ${BASE_TREE_LIBVIRT:=git://xenbits.xen.org/libvirt.git}
diff --git a/cri-getconfig b/cri-getconfig
index b2c91ac7..b7f7ae0c 100644
--- a/cri-getconfig
+++ b/cri-getconfig
@@ -25,6 +25,43 @@ getconfig () {
         '
 }
 
+# Get the path to a stashed file from another job.
+#
+# $1: current flight
+# $2: current job
+# $3: name of the stashed file
+# $4: flight (optional) and job where to fetch the stashed file from
+#     (in $flight.$job format or $job).
+getstashed() {
+	perl -e '
+		use Osstest;
+		use Osstest::TestSupport;
+		csreadconfig();
+		$flight = "'$1'";
+		$job = "'$2'";
+		print get_stashed("'$3'", "'$4'") or die $!;
+	'
+}
+
+# Get a runvar from another job.
+#
+# $1: current flight
+# $2: current job
+# $3: name of the runvar
+# $4: flight (optional) and job where to fetch the runvar from
+#     (in $flight.$job format or $job).
+
+getrunvar() {
+	perl -e '
+		use Osstest;
+		use Osstest::TestSupport;
+		csreadconfig();
+		$flight = "'$1'";
+		$job = "'$2'";
+		print get_runvar("'$3'", "'$4'") or die $!;
+	'
+}
+
 getconfig_TftpDiVersion_suite () {
 	perl -e '
 		use Osstest;
diff --git a/mfi-common b/mfi-common
index ec31e2ef..0f85e90a 100644
--- a/mfi-common
+++ b/mfi-common
@@ -96,6 +96,54 @@ set_hostos_runvars () {
   esac
 }
 
+set_freebsd_build_job_flags () {
+    arch=$1
+    job="build-$arch-freebsd"
+
+    # Figure out where are the installer binaries. The order is the following:
+    #
+    # 1. Env variable FREEBSD_BUILDJOB: use the output from a previous
+    # build-<arch>-freebsd.
+    #
+    # 2. Env variables FREEBSD_IMAGE, FREEBSD_SETS, FREEBSD_VERSION: set
+    # before calling into make-flight, provide the installer image, the sets
+    # to install and the version being installed.
+    #
+    # 3. Config file FreeBSDImage, FreeBSDSets and FreeBSDVersion: same as 2.
+    # except that they are set on the config file.
+    #
+    # This is slightly convoluted because in order to fetch a runvar or stashed
+    # file osstest needs a current flight and job, so the FreeBSD build flight
+    # is created with missing runvars, then this newly created job is used to
+    # fetch runvars from other jobs if needed, and finally the missing runvars
+    # in the newly created flight are set using cs-adjust-flight.
+    if [ -n "$FREEBSD_BUILDJOB" ]; then
+        image=`getstashed $flight $job "path_freebsd-image" $FREEBSD_BUILDJOB`
+        version=`getrunvar $flight $job "freebsd_version" $FREEBSD_BUILDJOB`
+        ./cs-adjust-flight $flight runvar-set $job freebsd_buildjob \
+                           $FREEBSD_BUILDJOB
+    elif [ -n "$FREEBSD_IMAGE" ] && [ -n "$FREEBSD_SETS" ] && \
+         [ -n "$FREEBSD_VERSION" ]; then
+        image=$FREEBSD_IMAGE
+        version=$FREEBSD_VERSION
+        ./cs-adjust-flight $flight runvar-set $job freebsd_image $FREEBSD_IMAGE
+        ./cs-adjust-flight $flight runvar-set $job freebsd_sets $FREEBSD_SETS
+    else
+        image=`getconfig "FreeBSDImage"`
+        sets=`getconfig "FreeBSDSets"`
+        version=`getconfig "FreeBSDVersion"`
+        ./cs-adjust-flight $flight runvar-set $job freebsd_image $image
+        ./cs-adjust-flight $flight runvar-set $job freebsd_sets $sets
+    fi
+
+    hash=`sha256sum $image | head -c 16`
+    ./cs-adjust-flight $flight runvar-set $job                          \
+        host_hostflags                                                  \
+        share-build-freebsd-$arch-$hash,arch-$arch,freebsd-$version,purpose-build
+
+    echo $version
+}
+
 create_build_jobs () {
 
   local arch
@@ -325,6 +373,26 @@ create_build_jobs () {
 
     fi
 
+    case $arch in
+    amd64)
+    job_create_build build-$arch-freebsd build-freebsd                       \
+                arch=$arch                                                   \
+                $RUNVARS $BUILD_RUNVARS $BUILD_FREEBSD_RUNVARS $arch_runvars \
+                tree_freebsd=$TREE_FREEBSD                                   \
+                revision_freebsd=$REVISION_FREEBSD
+    freebsd_version=`set_freebsd_build_job_flags $arch`
+    # Pass new hostflags, now the buildhost must not be shared, since we are
+    # testing the newly built image
+    job_create_build build-$arch-freebsd-again build-freebsd                 \
+                arch=$arch                                                   \
+                $RUNVARS $BUILD_RUNVARS $BUILD_FREEBSD_RUNVARS $arch_runvars \
+                host_hostflags=arch-$arch,freebsd-$freebsd_version,purpose-build \
+                tree_freebsd=$TREE_FREEBSD                                   \
+                revision_freebsd=$REVISION_FREEBSD                           \
+                freebsd_buildjob=build-$arch-freebsd
+    ;;
+    esac
+
     case "$arch" in
     arm*) continue;; # don't do any other kernel builds
     esac
diff --git a/sg-run-job b/sg-run-job
index ceb79800..c480a02b 100755
--- a/sg-run-job
+++ b/sg-run-job
@@ -53,12 +53,15 @@ proc run-job {job} {
     set skip_globs     [jobdb::read-runvar $flight $job skip_testids]
 
     set nh [need-hosts/$jobinfo(recipe)]
-    if {![string compare $nh BUILD]} {
+    if {![string compare $nh BUILD_LINUX]} {
         set need_xen_hosts {}
-        set need_build_host 1
+        set need_build_host LINUX
+    } elseif {![string compare $nh BUILD_FREEBSD]} {
+        set need_xen_hosts {}
+        set need_build_host FREEBSD
     } else {
         set need_xen_hosts $nh
-        set need_build_host 0
+        set need_build_host ""
     }
     set nested_layers_hosts {}
 
@@ -69,7 +72,8 @@ proc run-job {job} {
         eval run-ts broken  =             ts-hosts-allocate + $need_xen_hosts
     }
 
-    if {$need_build_host} { catching-otherwise broken prepare-build-host }
+    if {![string compare $need_build_host LINUX]} { catching-otherwise broken prepare-build-host-linux }
+    if {![string compare $need_build_host FREEBSD]}  { catching-otherwise broken prepare-build-host-freebsd }
 
     if {$ok} { setstatus running                                          }
 
@@ -89,7 +93,7 @@ proc run-job {job} {
         set need_xen_hosts [lunappend nested_layers_hosts]
     }
 
-    if {$need_build_host && $anyfailed} {
+    if {[string length $need_build_host] && $anyfailed} {
 	run-ts  !broken capture-logs      ts-logs-capture + host
     }
 
@@ -106,7 +110,7 @@ proc run-job {job} {
 
     if {$ok} { setstatus pass                                             }
 
-    if {$need_build_host && $ok} { jobdb::preserve-task 90 }
+    if {[string length $need_build_host] && $ok} { jobdb::preserve-task 90 }
 
     if {$anyfailed} {
         jobdb::logputs stdout "at least one test failed"
@@ -535,11 +539,12 @@ proc need-hosts/host-examine-linux {} {
 
 #---------- builds ----------
 
-proc need-hosts/build {} { return BUILD }
-proc need-hosts/build-kern {} { return BUILD }
-proc need-hosts/build-libvirt {} { return BUILD }
-proc need-hosts/build-rumprun {} { return BUILD }
-proc need-hosts/build-xtf {} { return BUILD }
+proc need-hosts/build {} { return BUILD_LINUX }
+proc need-hosts/build-kern {} { return BUILD_LINUX }
+proc need-hosts/build-libvirt {} { return BUILD_LINUX }
+proc need-hosts/build-rumprun {} { return BUILD_LINUX }
+proc need-hosts/build-xtf {} { return BUILD_LINUX }
+proc need-hosts/build-freebsd {} { return BUILD_FREEBSD }
 
 proc run-job/build {} {
     run-ts . = ts-xen-build
@@ -566,13 +571,23 @@ proc run-job/build-xtf {} {
     run-ts . = ts-xtf-build
 }
 
-proc prepare-build-host {} {
+proc run-job/build-freebsd {} {
+    run-ts . = ts-freebsd-build
+}
+
+proc prepare-build-host-linux {} {
     global jobinfo
     run-ts broken = ts-hosts-allocate + host
     run-ts broken host-install(*) ts-host-install-twice
     run-ts . host-build-prep ts-xen-build-prep
 }
 
+proc prepare-build-host-freebsd {} {
+    global jobinfo
+    run-ts broken = ts-hosts-allocate + host
+    run-ts broken host-install(*) ts-freebsd-host-install
+}
+
 proc need-hosts/coverity {} { return BUILD }
 proc run-job/coverity {} {
     run-ts . = ts-coverity-build + host
-- 
2.11.0 (Apple Git-81)


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 7/7] osstest: introduce make-freebsd-flight
  2017-05-23 13:51 [PATCH 0/7] osstest: initial FreeBSD support Roger Pau Monne
                   ` (5 preceding siblings ...)
  2017-05-23 13:51 ` [PATCH 6/7] osstest: add a FreeBSD build to flights Roger Pau Monne
@ 2017-05-23 13:51 ` Roger Pau Monne
  6 siblings, 0 replies; 19+ messages in thread
From: Roger Pau Monne @ 2017-05-23 13:51 UTC (permalink / raw)
  To: xen-devel, Ian.Jackson; +Cc: Roger Pau Monne

At the moment this flight only contains a build-amd64-freebsd and
build-amd64-freebsd-again jobs, because that's all osstest can do with FreeBSD
now.

This allows testing FreeBSD specific functionality without generating a
fully-fledged flight, that also includes the Linux jobs.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 make-freebsd-flight | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)
 create mode 100755 make-freebsd-flight

diff --git a/make-freebsd-flight b/make-freebsd-flight
new file mode 100755
index 00000000..f67d0171
--- /dev/null
+++ b/make-freebsd-flight
@@ -0,0 +1,62 @@
+#!/bin/bash
+
+# This is part of "osstest", an automated testing framework for Xen.
+# Copyright (C) 2017 Citrix Inc.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+
+set -e -o posix
+
+branch=$1
+xenbranch=$2
+blessing=$3
+buildflight=$4
+
+flight=`./cs-flight-create $blessing $branch`
+
+. ./cri-common
+. ./ap-common
+. ./mfi-common
+
+arch=amd64
+
+job_create_build_filter_callback () {
+    :
+}
+
+job_create_build build-$arch-freebsd build-freebsd                       \
+            arch=$arch                                                   \
+            $RUNVARS $BUILD_RUNVARS $BUILD_FREEBSD_RUNVARS $arch_runvars \
+            tree_freebsd=$TREE_FREEBSD                                   \
+            revision_freebsd=$REVISION_FREEBSD
+version=`set_freebsd_build_job_flags $arch`
+
+# Pass new hostflags, now the buildhost must not be shared, since we are
+# testing the newly built image
+job_create_build build-$arch-freebsd-again build-freebsd                 \
+            arch=$arch                                                   \
+            $RUNVARS $BUILD_RUNVARS $BUILD_FREEBSD_RUNVARS $arch_runvars \
+            host_hostflags=arch-$arch,freebsd-$version,purpose-build     \
+            tree_freebsd=$TREE_FREEBSD                                   \
+            revision_freebsd=$REVISION_FREEBSD                           \
+            freebsd_buildjob=build-$arch-freebsd
+
+echo $flight
+
+# Local variables:
+# mode: sh
+# sh-basic-offset: 2
+# indent-tabs-mode: nil
+# End:
-- 
2.11.0 (Apple Git-81)


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/7] osstest: make built_stash_file store a path_ runvar for each file
  2017-05-23 13:51 ` [PATCH 1/7] osstest: make built_stash_file store a path_ runvar for each file Roger Pau Monne
@ 2017-05-23 14:55   ` Ian Jackson
  0 siblings, 0 replies; 19+ messages in thread
From: Ian Jackson @ 2017-05-23 14:55 UTC (permalink / raw)
  To: Roger Pau Monne; +Cc: xen-devel

Roger Pau Monne writes ("[PATCH 1/7] osstest: make built_stash_file store a path_ runvar for each file"):
> And introduce built_stash_debugfile in order the keep the previous behavior of
> built_stash_file.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/7] osstest: move known_hosts generation to TestSupport
  2017-05-23 13:51 ` [PATCH 2/7] osstest: move known_hosts generation to TestSupport Roger Pau Monne
@ 2017-05-23 14:56   ` Ian Jackson
  0 siblings, 0 replies; 19+ messages in thread
From: Ian Jackson @ 2017-05-23 14:56 UTC (permalink / raw)
  To: Roger Pau Monne; +Cc: xen-devel

Roger Pau Monne writes ("[PATCH 2/7] osstest: move known_hosts generation to TestSupport"):
> This is equivalent to the already existing authorized_keys function, and
> generates the contents of the known_hosts file that should be installed on
> targets.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/7] osstest: fix regular expression used to match buildjob in ts-build-check
  2017-05-23 13:51 ` [PATCH 3/7] osstest: fix regular expression used to match buildjob in ts-build-check Roger Pau Monne
@ 2017-05-23 15:01   ` Ian Jackson
  2017-05-24  7:18     ` Roger Pau Monne
  0 siblings, 1 reply; 19+ messages in thread
From: Ian Jackson @ 2017-05-23 15:01 UTC (permalink / raw)
  To: Roger Pau Monne; +Cc: xen-devel

Roger Pau Monne writes ("[PATCH 3/7] osstest: fix regular expression used to match buildjob in ts-build-check"):
> Current regular expression used to match the buildjob works correctly when the
> buildjob runvar has the <job_name>buildjob format, but not when the format is
> <job_name>_buildjob (the first match group is empty in this case). Change it so
> that it works for both formats.

I think I have misled you.  Looking at the database I see there are
runvars with names like this

  src_host_xenbuildjob
  guests_rumpuserxenbuildjob

Ie, the part before the _ (if any), is a scope (like a host ident).
So this patch is wrong, and also, your other patches have to use
`freebsdbuildjob'.

Sorry :-/.

Ian.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/7] osstest: add a FreeBSD host install recipe
  2017-05-23 13:51 ` [PATCH 4/7] osstest: add a FreeBSD host install recipe Roger Pau Monne
@ 2017-05-23 16:04   ` Ian Jackson
  2017-05-24 10:48     ` Roger Pau Monne
  2017-05-23 17:14   ` Ian Jackson
  1 sibling, 1 reply; 19+ messages in thread
From: Ian Jackson @ 2017-05-23 16:04 UTC (permalink / raw)
  To: Roger Pau Monne; +Cc: xen-devel

Roger Pau Monne writes ("[PATCH 4/7] osstest: add a FreeBSD host install recipe"):
> The installation is performed using the bsdinstall tool, which is
> part of the FreeBSD base system. The installer image is setup with
> the osstest ssh keys and sshd enabled by default, which allows the
> test harness to just ssh into the box, create the install config
> file and launch the scripted install.

Thanks.  I've read this in some detail now.  My comments follow.

> In order to support the FreeBSD installer a new method is added,
> that allows setting the pxe boot of a host using a memdisk. Also, a
> new tftp path is added in order to point to the place where the
> FreeBSD installed images should be stored.

Can you please add to the commit message a deployment note that the
tftp root needs to be provide with a copy of memdisk, eg
   /home/tftp/memdisk -> /usr/lib/syslinux/memdisk

> diff --git a/Osstest.pm b/Osstest.pm
> index a78728cd..a0c0339c 100644
> --- a/Osstest.pm
> +++ b/Osstest.pm
> @@ -227,6 +227,7 @@ sub readglobalconfig () {
>      $c{TftpTmpDir} ||= "$c{TftpPlayDir}tmp/";
>  
>      $c{TftpDiBase} ||= "$c{TftpPlayDir}debian-installer";
> +    $c{TftpFreeBSDBase} ||= "$c{TftpPlayDir}freebsd-installer";

You need to document this in README.

> +    foreach (@sets, "MANIFEST") {

I think it would be better to give this loop control variable a name.
When the scope of a use of $_ is too big, $_ has a tendency to get
trashed; this makes the code fragile in the face of future changes.

> +        if (!defined($r{"freebsd_sets"})) {
> +            # Get everything before the "." (ie: get base from base.txz)
> +            my $stash_name = lc((split /\./)[0]);

IMO that's not very idomatic perl.  I won't insist you change it, but
maybe this would be better

               my $stash_name = $set;
               $stash_name =~ s/[^.]+$//;

or

               $set =~ m/^[^.]+/ or die "$set ?";
               $stash_name = $&;

(Do you intend to split on the first `.' ?)

> +sub setup_netboot_installer () {
> +    my $pxeimg = "$ho->{Tftp}{FreeBSDBase}/install-$ho->{Name}.img";

I think this would be better under TftpTmpDir.  Cf ts-host-install's
placement of $ho--initrd.gz.

Maybe we want a function target_tftp_prefix or something which returns
   $ho->{Tftp}{TmpDir}".hostnamepath($ho)
?

Also the sha names could be in {TmpDir}/freebsd-images.  Does this, in
fact, need to be configurable at all ?  I think probably not.

> +    my $script = <<END;
> +set -ex
> +basedir=$ho->{Tftp}{Path}/$ho->{Tftp}{FreeBSDBase}/
> +hash=`sha256sum $image|head -c 64`
> +localpath="$r{arch}/\$hash/install.img";
> +mkdir -p \$basedir
> +(
> +flock -x -w 600 200

In osstest we generally use with-lock-ex from chiark-utils, rather
than flock from util-linux.  (Bonus: it should be more portable for if
someone wants to run a controller on BSD...)

with-lock-ex needs to be used in the wrapper way.
(Also, calling flock on a directory is rather outre'.)

You've written all this in bash because you found it too hard or
annoying in Perl ?  I don't much mind that, but the escaping is a bit
irritating.  Maybe you want to pass the script its parameters as
arguments so that you can use <<'END' rather than <<END ?

However,

> +    logm("Executing:\n$script");
> +    my $ret = system("/bin/bash -c '$script'");

This is quite wrong.  Your unparsing is unsound.  Amazingly your
script doesn't currently contain ' so this doesn't matter right now,
but that could change.

You should use the multi-argument form of system(3perl).  And, of
course, you should use Osstest::system_checked which does the error
handling for you.

You might want to build a command in an array @cmd, like copydir in
cr-publish-flight-logs does.  (Not sure why that doesn't use
system_checked...)

> +sub install () {
> +    my $authkeys = authorized_keys();
> +    my $knownhosts = known_hosts();
> +    my $sshd_keys_url = create_ssh_overlay();
> +    my $disk_names = "ada0 da0 ad0";

This should probably be
       my @disk_names = qw(ada0 da0 ad0);
in case anyone wants to manipulate it in perl.  You can substitute
it straight into the here document; perl will put in the " " again.

> +    my $installer_sets = join(" ", @sets);
> +    my $target_sets = "/tmp/osstest_sets";

Hardcoded /tmp antipattern.  Maybe this is technicallty OK because
it's an installer environment, but I think it sets a very bad
example.  Is there some other path you could use ?

> +    target_cmd_root($ho, 'chsh -s /bin/sh', 10);

!!  What's the default ?

> +for disk in $disk_names; do
> +    if [ -c "/dev/\$disk" ]; then
> +        echo \$disk
> +        exit 0
> +    fi
> +done
> +exit 1
> +END
> +    defined($disk) or die "Unable to find a valid disk";
> +    logm("Using $disk as destination disk device");

I have found that on some hosts, when installing Debian GNU/Linux, I
have to expect a nonstandard disk name.  Currently in the DB I can
only find this
   hydrazine          disk-device                    /dev/cciss/c0d0
(in Cambridge; referring to a gone-away machine; NB that the property
name is in the obsolete containing-spaces syntax and is equivalent to
DiskDevice.)

I think you may want to check a host property.  It should probably be
called Freebsd_DiskDevice or something.  (Weird capitalisation required
by the word splitting name transformation rules for host propertiess.)

> +    logm("Trying to figure out primary nic device name");
> +    $nic = target_cmd_output_root($ho, <<END, 30);
> +nics=`ifconfig -l`
> +for nic in \$nics; do
> +    addr=`ifconfig \$nic inet|grep inet|awk {'print \$2'}`
> +    if [ "\$addr" = "$ho->{Ip}" ]; then
> +        echo \$nic
> +        exit 0
> +    fi
> +done
> +exit 1

I have quite a lot of this kind of thing:

  gall-mite           'interface force'              eth0
  itch-mite           'interface force'              eth0
  grain-weevil        'interface force'              eth1
  rice-weevil         'interface force'              eth1
  arndale-bluewater   Interface_Force                eth0
  arndale-lakeside    Interface_Force                eth0
  arndale-metrocentre Interface_Force                eth0
  arndale-westfield   Interface_Force                eth0

This may not be needed for FreeBSD, but I thought I would mention it.

What would happen if you tried to run this setup on a host where
FreeBSD's idea of the first network interface is not the one which
osstest did the install on ?

IIRC there is a way for pxelinux to pass the NIC MAC address to the OS
it is running.  Also, the host database contains the MAC address of
the host (and this is generally necessary for osstest to work even in
standalone mode I think), so you might use that rather than the IP
address ?

> +    foreach (get_sets_path()) {
> +        target_putfile_root($ho, 600, $_->{path}, "$target_sets/$_->{name}"
);

Needs linewrap.

> +    logm("Creating the installer script");
> +    target_cmd_root($ho, <<END, 60);
> +        set -e
> +        cat << ENDSCRIPT > installscript

OK, my brain is fully bent now.  Can we not create installscript on
the controller and transfer it with target_putfilecontents_root_stash ?
That way the logs would contain a copy, too.

> +# Setup serial console
> +printf "%s" "-h -S$c{Baud}" >> /boot.config
> +cat << ENDBOOT >> /boot/loader.conf
> +boot_serial="YES"
> +comconsole_speed="$c{Baud}"
> +console="comconsole"
> +boot_verbose="YES"
> +beastie_disable="YES"
> +ENDBOOT

Where does the installer's output go ?  Ie, does booting the installer
image produce serial log output ?

Ian.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/7] osstest: add a FreeBSD host install recipe
  2017-05-23 13:51 ` [PATCH 4/7] osstest: add a FreeBSD host install recipe Roger Pau Monne
  2017-05-23 16:04   ` Ian Jackson
@ 2017-05-23 17:14   ` Ian Jackson
  1 sibling, 0 replies; 19+ messages in thread
From: Ian Jackson @ 2017-05-23 17:14 UTC (permalink / raw)
  To: Roger Pau Monne; +Cc: xen-devel

Roger Pau Monne writes ("[PATCH 4/7] osstest: add a FreeBSD host install recipe"):
> The installation is performed using the bsdinstall tool, which is
> part of the FreeBSD base system. The installer image is setup with
> the osstest ssh keys and sshd enabled by default, which allows the
> test harness to just ssh into the box, create the install config
> file and launch the scripted install.
...
> +our $image = $r{"freebsd_image"} ||
> +             get_stashed("path_freebsd-image", $r{"freebsd_buildjob"});

I've had a thought about this.

Maybe it would be worth mentioning near the top of the script the
runvars it consumes (and any it sets) ?

I realise I haven't documented the runvars very much so far but maybe
we should start.  What do you think ?

Ian.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 5/7] osstest: introduce a FreeBSD build script
  2017-05-23 13:51 ` [PATCH 5/7] osstest: introduce a FreeBSD build script Roger Pau Monne
@ 2017-05-23 17:58   ` Ian Jackson
  2017-05-24 14:15     ` Roger Pau Monne
  0 siblings, 1 reply; 19+ messages in thread
From: Ian Jackson @ 2017-05-23 17:58 UTC (permalink / raw)
  To: Roger Pau Monne; +Cc: xen-devel

Roger Pau Monne writes ("[PATCH 5/7] osstest: introduce a FreeBSD build script"):
> -    my $path= "path_${part}dist";
> -    logm("checking $k $path");
> -    get_stashed($path, $r{$k});
> +    if ("$part" eq "freebsd") {
> +        foreach (qw(base kernel manifest image)) {
> +            my $path= "path_${part}-$_";
> +            logm("checking $k $path");
> +            get_stashed($path, $r{$k});
> +        }
> +    } else {
> +        my $path= "path_${part}dist";
> +        logm("checking $k $path");
> +        get_stashed($path, $r{$k});

This is quite ugly.  I don't think this knowledge should be in
ts-build-check.

...

I think in fact that the right answer is probably to have
ts-build-check be more general somehow.

I have looked through the history of ts-{,xen-}build-check and I think
the current approach is a historical accident.  In the beginning it
was a wrapper around ts-xen-install which used a special check flag;
then that gradually generalised to what we have now - but it still
retains the same origins.

I was going to suggest checking the job status, but might be an
inconvenience during by-hand testing.

I considered having sg-run-job specify the parts it's going to use, as
command line arguments, with plumbing in sg-run-job from the recipe,
but that's still going to have to be buildjob-runvar-specific.

I considered controlling this via runvars from make-flight:
   freebsdbuildjob=391031.build-amd64-freebsd
   freebsdbuildjobpaths=-base,-kernel,-manifest,-image
But it's also quite ugly.

I have a radical suggestion:

Suppose we have ts-freebsd-build set
    path_freebsddist=$stash/build/freebsd/
and have it put the files in there with fixed, known, names
    path_freebsddist=$stash/build/freebsd/image
    path_freebsddist=$stash/build/freebsd/manifest
    path_freebsddist=$stash/build/freebsd/kernel.sets
    path_freebsddist=$stash/build/freebsd/base.sets
or something ?

Is there a reason why that wouldn't work ?

The stashing process would have to take care to set the runvar only
after it had created all the files.

> +    target_cmd_build($ho, 7200, $builddir, <<END);
> +cd freebsd
> +export MAKEOBJDIRPREFIX=$builddir/obj
> +(make $makeflags TARGET=$r{arch} buildworld 2>&1 && touch ../build-ok-stamp) \\
> +    |tee ../log

How about using Osstest::BuildSupport::buildcmd_stamped_logged ?

> +    logm("Creating the install sets");
> +    # NB: the steps below need to be done as root or the permissions
> +    # of the files won't be properly set (and the target will fail).
> +    target_cmd_root($ho, <<END, 900);

target_cmd_root does not imply set -e; only target_cmd_build does.

You may want to invent buildcmd_stamped_logged_root or something.

> +# Enable DHCP on all network interfaces
> +echo 'ifconfig_DEFAULT="DHCP"' >> \$target/etc/rc.conf

Is this wise ?  We may at some point have hosts which have two network
interfaces connected (perhaps to the test network, or to each other,
or something) in which case this is probably wrong.

There are a lot of \.  I wonder if you might find
<<'ENDQ'.<<END.<<'ENDQ' a useful construct.

Ian.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/7] osstest: fix regular expression used to match buildjob in ts-build-check
  2017-05-23 15:01   ` Ian Jackson
@ 2017-05-24  7:18     ` Roger Pau Monne
  0 siblings, 0 replies; 19+ messages in thread
From: Roger Pau Monne @ 2017-05-24  7:18 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel

On Tue, May 23, 2017 at 04:01:33PM +0100, Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH 3/7] osstest: fix regular expression used to match buildjob in ts-build-check"):
> > Current regular expression used to match the buildjob works correctly when the
> > buildjob runvar has the <job_name>buildjob format, but not when the format is
> > <job_name>_buildjob (the first match group is empty in this case). Change it so
> > that it works for both formats.
> 
> I think I have misled you.  Looking at the database I see there are
> runvars with names like this
> 
>   src_host_xenbuildjob
>   guests_rumpuserxenbuildjob
> 
> Ie, the part before the _ (if any), is a scope (like a host ident).
> So this patch is wrong, and also, your other patches have to use
> `freebsdbuildjob'.
> 
> Sorry :-/.

NP, it's a simple s/freebsd_buildjob/freebsdbuildjob/, now done (and this patch
dropped).

Thanks, Roger.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/7] osstest: add a FreeBSD host install recipe
  2017-05-23 16:04   ` Ian Jackson
@ 2017-05-24 10:48     ` Roger Pau Monne
  2017-05-24 11:12       ` Ian Jackson
  0 siblings, 1 reply; 19+ messages in thread
From: Roger Pau Monne @ 2017-05-24 10:48 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel

On Tue, May 23, 2017 at 05:04:10PM +0100, Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH 4/7] osstest: add a FreeBSD host install recipe"):
> > The installation is performed using the bsdinstall tool, which is
> > part of the FreeBSD base system. The installer image is setup with
> > the osstest ssh keys and sshd enabled by default, which allows the
> > test harness to just ssh into the box, create the install config
> > file and launch the scripted install.
> 
> Thanks.  I've read this in some detail now.  My comments follow.
> 
> > In order to support the FreeBSD installer a new method is added,
> > that allows setting the pxe boot of a host using a memdisk. Also, a
> > new tftp path is added in order to point to the place where the
> > FreeBSD installed images should be stored.
> 
> Can you please add to the commit message a deployment note that the
> tftp root needs to be provide with a copy of memdisk, eg
>    /home/tftp/memdisk -> /usr/lib/syslinux/memdisk

Done.

> > diff --git a/Osstest.pm b/Osstest.pm
> > index a78728cd..a0c0339c 100644
> > --- a/Osstest.pm
> > +++ b/Osstest.pm
> > @@ -227,6 +227,7 @@ sub readglobalconfig () {
> >      $c{TftpTmpDir} ||= "$c{TftpPlayDir}tmp/";
> >  
> >      $c{TftpDiBase} ||= "$c{TftpPlayDir}debian-installer";
> > +    $c{TftpFreeBSDBase} ||= "$c{TftpPlayDir}freebsd-installer";
> 
> You need to document this in README.

(not needed anymore since we got rid of this).

> > +    foreach (@sets, "MANIFEST") {
> 
> I think it would be better to give this loop control variable a name.
> When the scope of a use of $_ is too big, $_ has a tendency to get
> trashed; this makes the code fragile in the face of future changes.

Done.

> 
> > +        if (!defined($r{"freebsd_sets"})) {
> > +            # Get everything before the "." (ie: get base from base.txz)
> > +            my $stash_name = lc((split /\./)[0]);
> 
> IMO that's not very idomatic perl.  I won't insist you change it, but
> maybe this would be better
> 
>                my $stash_name = $set;
>                $stash_name =~ s/[^.]+$//;
> 
> or
> 
>                $set =~ m/^[^.]+/ or die "$set ?";
>                $stash_name = $&;
> 
> (Do you intend to split on the first `.' ?)

I don't expect this names to ever have more than one dot, this is not
something that changes, so there should really be only one dot (or
none in the MANIFEST case).

I prefer the second one because it strips the ".", while the first one
doesn't.

(as you probably already guessed my perl/regexp is not very good)

> 
> > +sub setup_netboot_installer () {
> > +    my $pxeimg = "$ho->{Tftp}{FreeBSDBase}/install-$ho->{Name}.img";
> 
> I think this would be better under TftpTmpDir.  Cf ts-host-install's
> placement of $ho--initrd.gz.
> 
> Maybe we want a function target_tftp_prefix or something which returns
>    $ho->{Tftp}{TmpDir}".hostnamepath($ho)
> ?

Sounds fine, I guess I will add this as a pre-patch since there's already a
consumer in ts-host-install.

> Also the sha names could be in {TmpDir}/freebsd-images.  Does this, in
> fact, need to be configurable at all ?  I think probably not.

OK, so then I will just drop FreeBSDBase and just use
$ho->{Tftp}{TmpDir}/freebsd-images. I don't think there's a lot of
value in have it in a standard folder, since those are just random
builds. I guess this makes more sense for Debian because they are
actual releases, and thus can be used for other stuff also?

> > +    my $script = <<END;
> > +set -ex
> > +basedir=$ho->{Tftp}{Path}/$ho->{Tftp}{FreeBSDBase}/
> > +hash=`sha256sum $image|head -c 64`
> > +localpath="$r{arch}/\$hash/install.img";
> > +mkdir -p \$basedir
> > +(
> > +flock -x -w 600 200
> 
> In osstest we generally use with-lock-ex from chiark-utils, rather
> than flock from util-linux.  (Bonus: it should be more portable for if
> someone wants to run a controller on BSD...)
> 
> with-lock-ex needs to be used in the wrapper way.
> (Also, calling flock on a directory is rather outre'.)

I don't have any problems using with-lock-ex, I just didn't know it
existed :).

> You've written all this in bash because you found it too hard or
> annoying in Perl ?  I don't much mind that, but the escaping is a bit
> irritating.  Maybe you want to pass the script its parameters as
> arguments so that you can use <<'END' rather than <<END ?

Hm, yes, I wasn't really confident of writing this in perl and being
sure that the lock was always released. I'm more confident with shell.

> However,
> 
> > +    logm("Executing:\n$script");
> > +    my $ret = system("/bin/bash -c '$script'");
> 
> This is quite wrong.  Your unparsing is unsound.  Amazingly your
> script doesn't currently contain ' so this doesn't matter right now,
> but that could change.
> 
> You should use the multi-argument form of system(3perl).  And, of
> course, you should use Osstest::system_checked which does the error
> handling for you.
> 
> You might want to build a command in an array @cmd, like copydir in
> cr-publish-flight-logs does.  (Not sure why that doesn't use
> system_checked...)

I've changed this to use an array with all the parameters and
system_checked, it looks much better now IMHO, thanks.

> > +sub install () {
> > +    my $authkeys = authorized_keys();
> > +    my $knownhosts = known_hosts();
> > +    my $sshd_keys_url = create_ssh_overlay();
> > +    my $disk_names = "ada0 da0 ad0";
> 
> This should probably be
>        my @disk_names = qw(ada0 da0 ad0);
> in case anyone wants to manipulate it in perl.  You can substitute
> it straight into the here document; perl will put in the " " again.
> 
> > +    my $installer_sets = join(" ", @sets);
> > +    my $target_sets = "/tmp/osstest_sets";
> 
> Hardcoded /tmp antipattern.  Maybe this is technicallty OK because
> it's an installer environment, but I think it sets a very bad
> example.  Is there some other path you could use ?

I'm open to suggestions. We could also use ~/osstest_sets. I don't
really have a preference. I've used /tmp because I though it would be
less controversial.

> > +    target_cmd_root($ho, 'chsh -s /bin/sh', 10);
> 
> !!  What's the default ?

csh.

> > +for disk in $disk_names; do
> > +    if [ -c "/dev/\$disk" ]; then
> > +        echo \$disk
> > +        exit 0
> > +    fi
> > +done
> > +exit 1
> > +END
> > +    defined($disk) or die "Unable to find a valid disk";
> > +    logm("Using $disk as destination disk device");
> 
> I have found that on some hosts, when installing Debian GNU/Linux, I
> have to expect a nonstandard disk name.  Currently in the DB I can
> only find this
>    hydrazine          disk-device                    /dev/cciss/c0d0
> (in Cambridge; referring to a gone-away machine; NB that the property
> name is in the obsolete containing-spaces syntax and is equivalent to
> DiskDevice.)
> 
> I think you may want to check a host property.  It should probably be
> called Freebsd_DiskDevice or something.  (Weird capitalisation required
> by the word splitting name transformation rules for host propertiess.)

Yes, I can do that, but we would have to fill the DB manually. Would
you be OK with leaving this as-is in this patch and me adding the
property fetching later on?

> > +    logm("Trying to figure out primary nic device name");
> > +    $nic = target_cmd_output_root($ho, <<END, 30);
> > +nics=`ifconfig -l`
> > +for nic in \$nics; do
> > +    addr=`ifconfig \$nic inet|grep inet|awk {'print \$2'}`
> > +    if [ "\$addr" = "$ho->{Ip}" ]; then
> > +        echo \$nic
> > +        exit 0
> > +    fi
> > +done
> > +exit 1
> 
> I have quite a lot of this kind of thing:
> 
>   gall-mite           'interface force'              eth0
>   itch-mite           'interface force'              eth0
>   grain-weevil        'interface force'              eth1
>   rice-weevil         'interface force'              eth1
>   arndale-bluewater   Interface_Force                eth0
>   arndale-lakeside    Interface_Force                eth0
>   arndale-metrocentre Interface_Force                eth0
>   arndale-westfield   Interface_Force                eth0
> 
> This may not be needed for FreeBSD, but I thought I would mention it.
> 
> What would happen if you tried to run this setup on a host where
> FreeBSD's idea of the first network interface is not the one which
> osstest did the install on ?

FreeBSD doesn't really have an idea of the first network interface
(unless you count the order in the output from ifconfig -l).

Also, on FreeBSD each driver has it's own device, with different name,
ie: there are em0, em1, bge0, bce0... interfaces. We could add
a host property like Freebsd_NicDevice, but this fallback should stay
in case the parameter is not defined?

> IIRC there is a way for pxelinux to pass the NIC MAC address to the OS
> it is running.  Also, the host database contains the MAC address of
> the host (and this is generally necessary for osstest to work even in
> standalone mode I think), so you might use that rather than the IP
> address ?

This is not suitable in this case because from FreeBSD installer PoV
it has been booted from a disk (because memdisk is used), so none of
the PXE variables are propagated.

> > +    foreach (get_sets_path()) {
> > +        target_putfile_root($ho, 600, $_->{path}, "$target_sets/$_->{name}"
> );
> 
> Needs linewrap.
> 
> > +    logm("Creating the installer script");
> > +    target_cmd_root($ho, <<END, 60);
> > +        set -e
> > +        cat << ENDSCRIPT > installscript
> 
> OK, my brain is fully bent now.  Can we not create installscript on
> the controller and transfer it with target_putfilecontents_root_stash ?
> That way the logs would contain a copy, too.

Yes, that's much better, thanks!

> > +# Setup serial console
> > +printf "%s" "-h -S$c{Baud}" >> /boot.config
> > +cat << ENDBOOT >> /boot/loader.conf
> > +boot_serial="YES"
> > +comconsole_speed="$c{Baud}"
> > +console="comconsole"
> > +boot_verbose="YES"
> > +beastie_disable="YES"
> > +ENDBOOT
> 
> Where does the installer's output go ?  Ie, does booting the installer
> image produce serial log output ?

Yes, the installer image is built with serial output already.

The output of the installer itself (bsdinstall) goes to the job log
file.

Thanks, Roger.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/7] osstest: add a FreeBSD host install recipe
  2017-05-24 10:48     ` Roger Pau Monne
@ 2017-05-24 11:12       ` Ian Jackson
  0 siblings, 0 replies; 19+ messages in thread
From: Ian Jackson @ 2017-05-24 11:12 UTC (permalink / raw)
  To: Roger Pau Monne; +Cc: xen-devel

Roger Pau Monne writes ("Re: [PATCH 4/7] osstest: add a FreeBSD host install recipe"):
> OK, so then I will just drop FreeBSDBase and just use
> $ho->{Tftp}{TmpDir}/freebsd-images. I don't think there's a lot of
> value in have it in a standard folder, since those are just random
> builds. I guess this makes more sense for Debian because they are
> actual releases, and thus can be used for other stuff also?

Yes.  Indeed they might not be managed by osstest at all.

> > > +    my $installer_sets = join(" ", @sets);
> > > +    my $target_sets = "/tmp/osstest_sets";
> > 
> > Hardcoded /tmp antipattern.  Maybe this is technicallty OK because
> > it's an installer environment, but I think it sets a very bad
> > example.  Is there some other path you could use ?
> 
> I'm open to suggestions. We could also use ~/osstest_sets. I don't
> really have a preference. I've used /tmp because I though it would be
> less controversial.

Hah :-).

I'm fine with almost any path other than /tmp/FIXED_STRING.

> > > +    target_cmd_root($ho, 'chsh -s /bin/sh', 10);
> > 
> > !!  What's the default ?
> 
> csh.

omg wtf bbq.  Right.  Fine.

> > I have found that on some hosts, when installing Debian GNU/Linux, I
> > have to expect a nonstandard disk name.  Currently in the DB I can
> > only find this
> >    hydrazine          disk-device                    /dev/cciss/c0d0
> > (in Cambridge; referring to a gone-away machine; NB that the property
> > name is in the obsolete containing-spaces syntax and is equivalent to
> > DiskDevice.)
> > 
> > I think you may want to check a host property.  It should probably be
> > called Freebsd_DiskDevice or something.  (Weird capitalisation required
> > by the word splitting name transformation rules for host propertiess.)
> 
> Yes, I can do that, but we would have to fill the DB manually. Would
> you be OK with leaving this as-is in this patch and me adding the
> property fetching later on?

OK.  Hopefully it won't come up.

> > What would happen if you tried to run this setup on a host where
> > FreeBSD's idea of the first network interface is not the one which
> > osstest did the install on ?
> 
> FreeBSD doesn't really have an idea of the first network interface
> (unless you count the order in the output from ifconfig -l).
> 
> Also, on FreeBSD each driver has it's own device, with different name,
> ie: there are em0, em1, bge0, bce0... interfaces. We could add
> a host property like Freebsd_NicDevice, but this fallback should stay
> in case the parameter is not defined?

I think this is too complicated and hypothetical.  Let's leave it for
now, as you have it.

> > > +    logm("Creating the installer script");
> > > +    target_cmd_root($ho, <<END, 60);
> > > +        set -e
> > > +        cat << ENDSCRIPT > installscript
> > 
> > OK, my brain is fully bent now.  Can we not create installscript on
> > the controller and transfer it with target_putfilecontents_root_stash ?
> > That way the logs would contain a copy, too.
> 
> Yes, that's much better, thanks!

:-)

> > > +# Setup serial console
> > > +printf "%s" "-h -S$c{Baud}" >> /boot.config
> > > +cat << ENDBOOT >> /boot/loader.conf
> > > +boot_serial="YES"
> > > +comconsole_speed="$c{Baud}"
> > > +console="comconsole"
> > > +boot_verbose="YES"
> > > +beastie_disable="YES"
> > > +ENDBOOT
> > 
> > Where does the installer's output go ?  Ie, does booting the installer
> > image produce serial log output ?
> 
> Yes, the installer image is built with serial output already.
> 
> The output of the installer itself (bsdinstall) goes to the job log
> file.

Great.  Looking good :-).

Thanks,
Ian./

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 5/7] osstest: introduce a FreeBSD build script
  2017-05-23 17:58   ` Ian Jackson
@ 2017-05-24 14:15     ` Roger Pau Monne
  2017-05-24 14:19       ` Ian Jackson
  0 siblings, 1 reply; 19+ messages in thread
From: Roger Pau Monne @ 2017-05-24 14:15 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel

On Tue, May 23, 2017 at 06:58:59PM +0100, Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH 5/7] osstest: introduce a FreeBSD build script"):
> > -    my $path= "path_${part}dist";
> > -    logm("checking $k $path");
> > -    get_stashed($path, $r{$k});
> > +    if ("$part" eq "freebsd") {
> > +        foreach (qw(base kernel manifest image)) {
> > +            my $path= "path_${part}-$_";
> > +            logm("checking $k $path");
> > +            get_stashed($path, $r{$k});
> > +        }
> > +    } else {
> > +        my $path= "path_${part}dist";
> > +        logm("checking $k $path");
> > +        get_stashed($path, $r{$k});
> 
> This is quite ugly.  I don't think this knowledge should be in
> ts-build-check.
> 
> ...
> 
> I think in fact that the right answer is probably to have
> ts-build-check be more general somehow.
> 
> I have looked through the history of ts-{,xen-}build-check and I think
> the current approach is a historical accident.  In the beginning it
> was a wrapper around ts-xen-install which used a special check flag;
> then that gradually generalised to what we have now - but it still
> retains the same origins.
> 
> I was going to suggest checking the job status, but might be an
> inconvenience during by-hand testing.
> 
> I considered having sg-run-job specify the parts it's going to use, as
> command line arguments, with plumbing in sg-run-job from the recipe,
> but that's still going to have to be buildjob-runvar-specific.
> 
> I considered controlling this via runvars from make-flight:
>    freebsdbuildjob=391031.build-amd64-freebsd
>    freebsdbuildjobpaths=-base,-kernel,-manifest,-image
> But it's also quite ugly.
> 
> I have a radical suggestion:
> 
> Suppose we have ts-freebsd-build set
>     path_freebsddist=$stash/build/freebsd/
> and have it put the files in there with fixed, known, names
>     path_freebsddist=$stash/build/freebsd/image
>     path_freebsddist=$stash/build/freebsd/manifest
>     path_freebsddist=$stash/build/freebsd/kernel.sets
>     path_freebsddist=$stash/build/freebsd/base.sets
> or something ?
> 
> Is there a reason why that wouldn't work ?
> 
> The stashing process would have to take care to set the runvar only
> after it had created all the files.

That seems fine, and then osstest would rely on the fact that
path_freebsddist must only be set when all the files have been
uploaded, because ts-build-check itself won't check that the files are
there anymore.

> > +    target_cmd_build($ho, 7200, $builddir, <<END);
> > +cd freebsd
> > +export MAKEOBJDIRPREFIX=$builddir/obj
> > +(make $makeflags TARGET=$r{arch} buildworld 2>&1 && touch ../build-ok-stamp) \\
> > +    |tee ../log
> 
> How about using Osstest::BuildSupport::buildcmd_stamped_logged ?

That seems suitable, I've used this rune because it's what
ts-kernel-build was using, now I realize ts-xen-build is indeed using
buildcmd_stamped_logged.

> > +    logm("Creating the install sets");
> > +    # NB: the steps below need to be done as root or the permissions
> > +    # of the files won't be properly set (and the target will fail).
> > +    target_cmd_root($ho, <<END, 900);
> 
> target_cmd_root does not imply set -e; only target_cmd_build does.
> 
> You may want to invent buildcmd_stamped_logged_root or something.

Yes, that sounds right.

> 
> > +# Enable DHCP on all network interfaces
> > +echo 'ifconfig_DEFAULT="DHCP"' >> \$target/etc/rc.conf
> 
> Is this wise ?  We may at some point have hosts which have two network
> interfaces connected (perhaps to the test network, or to each other,
> or something) in which case this is probably wrong.

This just means that on the installer all the network interfaces will
try to get a DHCP address. This is for the installer image itself, the
installed system will only setup DHCP on the primary interface (ie:
the one that matches the IP address at $ho->{Ip}.

> There are a lot of \.  I wonder if you might find
> <<'ENDQ'.<<END.<<'ENDQ' a useful construct.

In fact I can define two perl variables and use them instead. There's
really no reason they have to be shell variables ($target and $output).

Roger.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 5/7] osstest: introduce a FreeBSD build script
  2017-05-24 14:15     ` Roger Pau Monne
@ 2017-05-24 14:19       ` Ian Jackson
  0 siblings, 0 replies; 19+ messages in thread
From: Ian Jackson @ 2017-05-24 14:19 UTC (permalink / raw)
  To: Roger Pau Monne; +Cc: xen-devel

Roger Pau Monne writes ("Re: [PATCH 5/7] osstest: introduce a FreeBSD build script"):
> On Tue, May 23, 2017 at 06:58:59PM +0100, Ian Jackson wrote:
> > Suppose we have ts-freebsd-build set
> >     path_freebsddist=$stash/build/freebsd/
> > and have it put the files in there with fixed, known, names
> >     path_freebsddist=$stash/build/freebsd/image
> >     path_freebsddist=$stash/build/freebsd/manifest
> >     path_freebsddist=$stash/build/freebsd/kernel.sets
> >     path_freebsddist=$stash/build/freebsd/base.sets
> > or something ?
> > 
> > Is there a reason why that wouldn't work ?
> > 
> > The stashing process would have to take care to set the runvar only
> > after it had created all the files.
> 
> That seems fine, and then osstest would rely on the fact that
> path_freebsddist must only be set when all the files have been
> uploaded, because ts-build-check itself won't check that the files are
> there anymore.

Exactly.

> > > +# Enable DHCP on all network interfaces
> > > +echo 'ifconfig_DEFAULT="DHCP"' >> \$target/etc/rc.conf
> > 
> > Is this wise ?  We may at some point have hosts which have two network
> > interfaces connected (perhaps to the test network, or to each other,
> > or something) in which case this is probably wrong.
> 
> This just means that on the installer all the network interfaces will
> try to get a DHCP address. This is for the installer image itself, the
> installed system will only setup DHCP on the primary interface (ie:
> the one that matches the IP address at $ho->{Ip}.

OK, good.

> > There are a lot of \.  I wonder if you might find
> > <<'ENDQ'.<<END.<<'ENDQ' a useful construct.
> 
> In fact I can define two perl variables and use them instead. There's
> really no reason they have to be shell variables ($target and
> $output).

OK, I guess.  I typically prefer to avoid feeding large quantities of
sh through the perl interpolator.  What you suggest means the reader
sees a shell script full of $variable references which actually refer
to perl variables, not shell ones.  I think that's a bit confusing.

But, ultimately, up to you.

Ian.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2017-05-24 14:20 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-23 13:51 [PATCH 0/7] osstest: initial FreeBSD support Roger Pau Monne
2017-05-23 13:51 ` [PATCH 1/7] osstest: make built_stash_file store a path_ runvar for each file Roger Pau Monne
2017-05-23 14:55   ` Ian Jackson
2017-05-23 13:51 ` [PATCH 2/7] osstest: move known_hosts generation to TestSupport Roger Pau Monne
2017-05-23 14:56   ` Ian Jackson
2017-05-23 13:51 ` [PATCH 3/7] osstest: fix regular expression used to match buildjob in ts-build-check Roger Pau Monne
2017-05-23 15:01   ` Ian Jackson
2017-05-24  7:18     ` Roger Pau Monne
2017-05-23 13:51 ` [PATCH 4/7] osstest: add a FreeBSD host install recipe Roger Pau Monne
2017-05-23 16:04   ` Ian Jackson
2017-05-24 10:48     ` Roger Pau Monne
2017-05-24 11:12       ` Ian Jackson
2017-05-23 17:14   ` Ian Jackson
2017-05-23 13:51 ` [PATCH 5/7] osstest: introduce a FreeBSD build script Roger Pau Monne
2017-05-23 17:58   ` Ian Jackson
2017-05-24 14:15     ` Roger Pau Monne
2017-05-24 14:19       ` Ian Jackson
2017-05-23 13:51 ` [PATCH 6/7] osstest: add a FreeBSD build to flights Roger Pau Monne
2017-05-23 13:51 ` [PATCH 7/7] osstest: introduce make-freebsd-flight Roger Pau Monne

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.