diff mbox

[Xen-devel,OSSTEST,v2] Allow per-host TFTP setup

Message ID 1395138188-20176-1-git-send-email-ian.campbell@citrix.com
State New
Headers show

Commit Message

Ian Campbell March 18, 2014, 10:23 a.m. UTC
I run osstest against machines which are in both the XenServer and XenClient
administrative domains, and hence which have different TFTP servers, accessible
locally via different NFS mounted paths.

Make it possible to specify various bits of TFTP path via ~/.xen-osstest/config
by introducing the idea of Tftp "scope" and allowing that to be configurable on
a per-host basis. For example I have:
  TftpDefaultScope xenserver

  TftpPath_xenserver /usr/groups/netboot/
  TftpPxeDir_xenserver pxelinux.cfg/

  TftpPath_xenclient /home/xc_tftpboot/pxe/
  TftpPxeDir_xenclient /
  TftpPxeTemplates_xenclient %ipaddrhex%/pxelinux.cfg

  HostProp_marilith-n4_TftpScope xenclient

and I am now able to install on both cam-st16 (a xenserver world test box) and
marilith-n4 (an osstest machine hosted in the xenclient network) without
messing around with my configuration every time. I ran build-$ARCH,
build-$ARCH-pvops and test-$ARCH-$ARCH-xl in both cases.

Per-host Tftp settings are now in the $ho->{Tftp} hash instead of in $c.

$c{TftpHost} is unused -- remove the setting of its default.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: Do not allow TftpDiVersion to be a host var, this must be push gated.
    Implement scope scheme
    README updates
---
 Osstest.pm             |  3 ++-
 Osstest/Debian.pm      |  3 ++-
 Osstest/TestSupport.pm | 22 +++++++++++++++++++---
 README                 | 25 +++++++++++++++++++++++++
 mg-hosts               |  4 ++--
 ts-host-install        | 14 +++++++-------
 6 files changed, 57 insertions(+), 14 deletions(-)

Comments

Ian Jackson March 18, 2014, 11:19 a.m. UTC | #1
Ian Campbell writes ("[PATCH OSSTEST v2] Allow per-host TFTP setup"):
> I run osstest against machines which are in both the XenServer and
> XenClient administrative domains, and hence which have different
> TFTP servers, accessible locally via different NFS mounted paths.

Thanks for this patch.

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

But I have some suggestions for improvements, too.  Feel free to
ignore, or update the patch, or push and make followup changes:


> +    $ho->{Tftp} = {
> +	Path => $c{"TftpPath_$tftpscope"} || $c{TftpPath},
> +	TmpDir => $c{"TftpTmpDir_$tftpscope"} || $c{TftpTmpDir},
> +	PxeDir => $c{"TftpPxeDir_$tftpscope"} || $c{TftpPxeDir},
> +	PxeGroup => $c{"TftpPxeGroup_$tftpscope"} || $c{TftpPxeGroup},
> +	PxeTemplates => $c{"TftpPxeTemplates_$tftpscope"} || $c{TftpPxeTemplates},
> +	DiBase => $c{"TftpDiBase_$tftpscope"} || $c{TftpDiBase},

What do you think of this:

  +    $ho->{Tftp} = { };
  +    $ho->{Tftp}{$_} = $c{"Tftp${_}_${tftpscope}"} || $c{"Tftp${_}"}
  +        foreach qw(Path TmpDir PxeDir PxeGroup PxeTemplates);

?



> +TftpFoo_<scope> and TftpFoo
> +
> +    Describes various properties relating to Tftp in a given <scope>,
> +    where a <scope> is a given subnet, DHCP server etc. Valid
> +    properties are:
> +
> +        Path          The path to the root of the directory which is exposed by
> +                      the tftpserver (e.g. /tftpboot).
> +        TmpDir        A directory under `Path' to use for temporary files.
> +
> +        PxeDir        The path under `Path' to the PXE configuration directory
> +                      (e.g. pxelinux.cfg)
> +        PxeGroup      The Unix group which should own files under `PxeDir'.
> +        PxeTemplates  See TftpPxeTemplates
> +
> +        DiBase        The path under `Path' to the root of the debian installer
> +                      images.
> +
> +    The <scope> is a host property which defaults to TftpDefaultScope
> +    or "default".  TftpFoo_default takes precedence of TftpFoo.

This last sentence is a bit unclear.  And the behaviour is subtle
(TftpFoo_default is `less defaulty' than plain TftpFoo).  I would say
something like:

       For hosts in scope "default", TftpFoo_default (if set) takes
       precedence over TftpFoo.  TftpFoo is used when the setting Foo
       is not defined for the host's scope.


Thanks,
Ian.
Ian Campbell March 18, 2014, 11:21 a.m. UTC | #2
On Tue, 2014-03-18 at 11:19 +0000, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH OSSTEST v2] Allow per-host TFTP setup"):
> > I run osstest against machines which are in both the XenServer and
> > XenClient administrative domains, and hence which have different
> > TFTP servers, accessible locally via different NFS mounted paths.
> 
> Thanks for this patch.
> 
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> 
> But I have some suggestions for improvements, too.  Feel free to
> ignore, or update the patch, or push and make followup changes:

They sound good. I'll update the patch.

> > +    $ho->{Tftp} = {
> > +	Path => $c{"TftpPath_$tftpscope"} || $c{TftpPath},
> > +	TmpDir => $c{"TftpTmpDir_$tftpscope"} || $c{TftpTmpDir},
> > +	PxeDir => $c{"TftpPxeDir_$tftpscope"} || $c{TftpPxeDir},
> > +	PxeGroup => $c{"TftpPxeGroup_$tftpscope"} || $c{TftpPxeGroup},
> > +	PxeTemplates => $c{"TftpPxeTemplates_$tftpscope"} || $c{TftpPxeTemplates},
> > +	DiBase => $c{"TftpDiBase_$tftpscope"} || $c{TftpDiBase},
> 
> What do you think of this:
> 
>   +    $ho->{Tftp} = { };
>   +    $ho->{Tftp}{$_} = $c{"Tftp${_}_${tftpscope}"} || $c{"Tftp${_}"}
>   +        foreach qw(Path TmpDir PxeDir PxeGroup PxeTemplates);
> 
> ?

OK.

> > +TftpFoo_<scope> and TftpFoo
> > +
> > +    Describes various properties relating to Tftp in a given <scope>,
> > +    where a <scope> is a given subnet, DHCP server etc. Valid
> > +    properties are:
> > +
> > +        Path          The path to the root of the directory which is exposed by
> > +                      the tftpserver (e.g. /tftpboot).
> > +        TmpDir        A directory under `Path' to use for temporary files.
> > +
> > +        PxeDir        The path under `Path' to the PXE configuration directory
> > +                      (e.g. pxelinux.cfg)
> > +        PxeGroup      The Unix group which should own files under `PxeDir'.
> > +        PxeTemplates  See TftpPxeTemplates
> > +
> > +        DiBase        The path under `Path' to the root of the debian installer
> > +                      images.
> > +
> > +    The <scope> is a host property which defaults to TftpDefaultScope
> > +    or "default".  TftpFoo_default takes precedence of TftpFoo.
> 
> This last sentence is a bit unclear.  And the behaviour is subtle
> (TftpFoo_default is `less defaulty' than plain TftpFoo).  I would say
> something like:
> 
>        For hosts in scope "default", TftpFoo_default (if set) takes
>        precedence over TftpFoo.  TftpFoo is used when the setting Foo

Sounds good, thanks.

Ian.
diff mbox

Patch

diff --git a/Osstest.pm b/Osstest.pm
index 4600709..1033dd1 100644
--- a/Osstest.pm
+++ b/Osstest.pm
@@ -161,8 +161,9 @@  sub readglobalconfig () {
     chomp($nodename) or die;
     my $myfqdn = "$nodename.$c{DnsDomain}";
 
+    $c{TftpDefaultScope} ||= "default";
+
     $c{TftpPath} ||= "/tftpboot/";
-    $c{TftpHost} ||= $myfqdn;
     $c{TftpPxeDir} ||= "pxelinux.cfg/";
     $c{TftpPxeTemplates} ||= '%ipaddrhex% 01-%etherhyph%';
     $c{TftpPlayDir} ||= "$whoami/osstest/";
diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
index 336043c..1ca6fb0 100644
--- a/Osstest/Debian.pm
+++ b/Osstest/Debian.pm
@@ -554,7 +554,8 @@  END
     foreach my $kp (keys %{ $ho->{Flags} }) {
 	$kp =~ s/need-kernel-deb-// or next;
 
-	my $d_i= $c{TftpPath}.'/'.$c{TftpDiBase}.'/'.$r{arch}.'/'.$c{TftpDiVersion}.'-'.$ho->{Suite};
+	my $d_i= $ho->{Tftp}{Path}.'/'.$ho->{Tftp}{DiBase}.'/'.$r{arch}.'/'.
+	    $c{TftpDiVersion}.'-'.$ho->{Suite};
 
 	my $kurl = create_webfile($ho, "kernel", sub {
 	    copy("$d_i/$kp.deb", $_[0])
diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index b6a6fb5..f6c2bef 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -795,6 +795,21 @@  sub selecthost ($) {
     }
     $ho->{Ip}= $ho->{IpStatic};
 
+    #----- tftp -----
+
+    my $tftpscope = get_host_property($ho, 'TftpScope', $c{TftpDefaultScope});
+    logm("TftpScope is $tftpscope");
+    $ho->{Tftp} = {
+	Path => $c{"TftpPath_$tftpscope"} || $c{TftpPath},
+	TmpDir => $c{"TftpTmpDir_$tftpscope"} || $c{TftpTmpDir},
+	PxeDir => $c{"TftpPxeDir_$tftpscope"} || $c{TftpPxeDir},
+	PxeGroup => $c{"TftpPxeGroup_$tftpscope"} || $c{TftpPxeGroup},
+	PxeTemplates => $c{"TftpPxeTemplates_$tftpscope"} || $c{TftpPxeTemplates},
+	DiBase => $c{"TftpDiBase_$tftpscope"} || $c{TftpDiBase},
+    };
+
+    #----- finalise -----
+
     $mjobdb->host_check_allocated($ho);
 
     logm("host: selected $ho->{Name} ".
@@ -1181,6 +1196,7 @@  sub selectguest ($$) {
         Guest => $gn,
         Name => $r{"${gn}_hostname"},
         CfgPath => $r{"${gn}_cfgpath"},
+        Tftp => $ho->{Tftp},
 	Host => $ho,
     };
     foreach my $opt (guest_var_commalist($gho,'options')) {
@@ -1841,7 +1857,7 @@  sub host_pxefile ($) {
 	$v{'ipaddr'} = $ip;
 	$v{'ipaddrhex'} = sprintf "%02X%02X%02X%02X", split /\./, $ip;
     }
-    foreach my $pat (split /\s+/, $c{TftpPxeTemplates}) {
+    foreach my $pat (split /\s+/, $ho->{Tftp}{PxeTemplates}) {
 	# we skip patterns that contain any references to undefined %var%s
 	$pat =~ s{\%(\w*)\%}{
 		    $1 eq '' ? '%' :
@@ -1851,14 +1867,14 @@  sub host_pxefile ($) {
 	# and return the first pattern we managed to completely substitute
         return $pat;
     }
-    die "no pxe template matched $c{TftpPxeTemplates} ".
+    die "no pxe template matched $ho->{Tftp}{PxeTemplates} ".
         (join ",", sort keys %v)." ?";
 }
 
 sub setup_pxeboot ($$) {
     my ($ho, $bootfile) = @_;
     my $f= host_pxefile($ho);
-    file_link_contents("$c{TftpPath}$c{TftpPxeDir}$f", $bootfile);
+    file_link_contents("$ho->{Tftp}{Path}$ho->{Tftp}{PxeDir}$f", $bootfile);
 }
 
 sub setup_pxeboot_local ($) {
diff --git a/README b/README
index 60379c4..8325975 100644
--- a/README
+++ b/README
@@ -326,6 +326,10 @@  HostProp_<testbox>_RebootTimeExtra
    where Xen takes really long time to boot (typically because of
    the "Scrubbing free RAM" phase).
 
+HostProp_<testbox>_TftpScope
+   Defines the Tftp scope (i.e. subnet) where this host resides. See
+   "TftpFoo_<scope> and TftpFoo" below.
+
 DebianPreseed
    Text to add to the debian-installer preseed file.  Optional
    but you will need to set some NTP servers here if your firewall
@@ -391,6 +395,27 @@  GuestDebianSuite   defaults to DebianSuite
 
 DebianPreseed      added to existing preseed file
 
+TftpFoo_<scope> and TftpFoo
+
+    Describes various properties relating to Tftp in a given <scope>,
+    where a <scope> is a given subnet, DHCP server etc. Valid
+    properties are:
+
+        Path          The path to the root of the directory which is exposed by
+                      the tftpserver (e.g. /tftpboot).
+        TmpDir        A directory under `Path' to use for temporary files.
+
+        PxeDir        The path under `Path' to the PXE configuration directory
+                      (e.g. pxelinux.cfg)
+        PxeGroup      The Unix group which should own files under `PxeDir'.
+        PxeTemplates  See TftpPxeTemplates
+
+        DiBase        The path under `Path' to the root of the debian installer
+                      images.
+
+    The <scope> is a host property which defaults to TftpDefaultScope
+    or "default".  TftpFoo_default takes precedence of TftpFoo.
+
 TftpPxeTemplates
     List (space-separated) of template filenames for writing
     The templates contain variable substitutions %var%
diff --git a/mg-hosts b/mg-hosts
index e409201..d7223a1 100755
--- a/mg-hosts
+++ b/mg-hosts
@@ -44,8 +44,8 @@  sub cmd_mkpxedir () {
         $macdir =~ s/\:/-/g;
         system_checked(<<END);
             set -e
-	    cd $c{TftpPath}$c{TftpPxeDir}
-            sudo chown root.$c{TftpPxeGroup} $macdir
+	    cd $ho->{Tftp}{Path}$ho->{Tftp}{PxeDir}
+            sudo chown root.$ho->{Tftp}{PxeGroup} $macdir
             sudo chmod 2775 $macdir
             sudo rm -f $hn
             sudo ln -s $macdir $hn
diff --git a/ts-host-install b/ts-host-install
index 8e119ca..95ce845 100755
--- a/ts-host-install
+++ b/ts-host-install
@@ -122,19 +122,19 @@  END
 sub setup_pxeboot_firstboot($) {
     my ($ps_url) = @_;
     
-    my $d_i= $c{TftpDiBase}.'/'.$r{arch}.'/'.$c{TftpDiVersion}.'-'.$ho->{Suite};
+    my $d_i= $ho->{Tftp}{DiBase}.'/'.$r{arch}.'/'.$c{TftpDiVersion}.'-'.$ho->{Suite};
     
     my @installcmdline= qw(vga=normal);
     push @installcmdline, di_installcmdline_core($ho, $ps_url, %xopts);
 
     my $src_initrd= "$d_i/initrd.gz";
-    my @initrds= "$c{TftpPath}/$src_initrd";
+    my @initrds= "$ho->{Tftp}{Path}/$src_initrd";
 
     my $kernel;
 
     foreach my $fp (keys %{ $ho->{Flags} }) {
         $fp =~ s/^need-firmware-deb-// or next;
-        my $cpio= "$c{TftpPath}/$d_i/$fp.cpio.gz";
+        my $cpio= "$ho->{Tftp}{Path}/$d_i/$fp.cpio.gz";
         if (stat $cpio) {
             logm("using firmware from: $cpio");
             push @initrds, $cpio;
@@ -147,7 +147,7 @@  sub setup_pxeboot_firstboot($) {
 
     foreach my $kp (keys %{ $ho->{Flags} }) {
         $kp =~ s/need-kernel-deb-// or next;
-        my $kern= "$c{TftpPath}/$d_i/linux.$kp";
+        my $kern= "$ho->{Tftp}{Path}/$d_i/linux.$kp";
         if (stat $kern) {
             logm("using kernel from: $kern");
             $kernel = "/$d_i/linux.$kp";
@@ -157,7 +157,7 @@  sub setup_pxeboot_firstboot($) {
             die "$kp $kern $!";
         }
 
-        my $cpio= "$c{TftpPath}/$d_i/$kp.cpio.gz";
+        my $cpio= "$ho->{Tftp}{Path}/$d_i/$kp.cpio.gz";
         if (stat $cpio) {
             logm("using kernel modules from: $cpio");
             push @initrds, $cpio;
@@ -196,8 +196,8 @@  END
     push @initrds, "$initrd_overlay.cpio.gz";
 
     logm("using initrds: @initrds");
-    my $initrd= "$c{TftpTmpDir}$ho->{Name}--initrd.gz";
-    system_checked("cat -- @initrds >$c{TftpPath}$initrd");
+    my $initrd= "$ho->{Tftp}{TmpDir}$ho->{Name}--initrd.gz";
+    system_checked("cat -- @initrds >$ho->{Tftp}{Path}$initrd");
     
     push @installcmdline, ("initrd=/$initrd",
                            "domain=$c{TestHostDomain}",