Re: [Xen-devel] [PATCH OSSTEST v4 07/25] Debian: add preseed_create_guest helper

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Thu, 2015-04-02 at 12:40 +0100, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH OSSTEST v4 07/25] Debian: add 
> preseed_create_guest helper"):
> > Creates a preseed file suitable for use in a PV guest
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > ---
> > v4: Rebase, pass $ho to preseed_base
> > v3: Handle $xopts{ExtraPreseed} undefined in preseed_base
> > ---
> >  Osstest/Debian.pm | 29 +++++++++++++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> > 
> > diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
> > index 13cd147..993acc7 100644
> > --- a/Osstest/Debian.pm
> > +++ b/Osstest/Debian.pm
> > @@ -37,6 +37,7 @@ BEGIN {
> >                        %preseed_cmds
> >                        preseed_base
> >                        preseed_create
> > +                      preseed_create_guest
> >                        preseed_ssh
> >                        preseed_hook_command preseed_hook_installscript
> >                        preseed_hook_overlay
> > @@ -611,6 +612,9 @@ END
> >  sub preseed_base ($$$;@) {
> >      my ($ho,$suite,$extra_packages,%xopts) = @_;
> >  
> > +    $extra_packages ||= '';
> 
> This is a little odd.  You are changing preseed_base to tolerate an
> unspecified $extra_packages, but it's still a mandatory argument.
> Maybe you should mvoe the `;' in the sub prototype.
> 
> Or maybe you just wanted to write:
>   +    my $extra_packages = '';
> instead of:
> > +    my $extra_packages;

It looks like I actually do, I suspect that defaulting must be a
leftover.

> 
> > +    preseed_ssh($ho, $sfx);
> > +    preseed_hook_overlay($ho, $sfx, $c{OverlayLocal}, 'overlay-local.tar');
> 
> Perhaps the OverlayLocal should always be done, rather than having to
> be explicitly specified by all the callers of preseed_base ?

Having moved preseed_ssh to preseed_base this certainly makes sense.

Ian.


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