Re: [Xen-devel] [PATCH OSSTEST v2] Allow per-host TFTP setup

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

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@xxxxxxxxxxxxx>

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.

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