From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ian Campbell <ian.campbell@citrix.com>
Subject: Re: [PATCH OSSTEST v4 07/25] Debian: add
	preseed_create_guest helper
Date: Thu, 2 Apr 2015 13:44:15 +0100
Message-ID: <1427978655.4037.71.camel@citrix.com>
References: <1427966183.4037.17.camel@citrix.com>
	<1427966199-5064-7-git-send-email-ian.campbell@citrix.com>
	<21789.10923.888536.641705@mariner.uk.xensource.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <21789.10923.888536.641705@mariner.uk.xensource.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org

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@citrix.com>
> > ---
> > 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.