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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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


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