Re: [Xen-devel] [PATCH OSSTEST V2] ts-debian-hvm-install: use text installer frontend

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Thu, May 15, 2014 at 11:10:00AM +0100, Ian Campbell wrote:
> On Thu, 2014-05-15 at 11:03 +0100, Wei Liu wrote:
> > On Thu, May 15, 2014 at 09:56:02AM +0100, Ian Campbell wrote:
> > > On Wed, 2014-05-14 at 22:16 +0100, Wei Liu wrote:
> > > > Factor out di_installcmdline_base in Debian.pm and use that in
> > > > ts-debian-hvm-install. This should improve readability of d-i log in
> > > > various Debian HVM testcases.
> > > > 
> > > > Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> > > > Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> > > > Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > > > ---
> > > > Per:
> > > > https://www.debian.org/releases/stable/amd64/apbs02.html.en
> > > > I didn't see "preseed" on that page so I left that one untouched.
> > > 
> > > I'm about 99% certain that preseed doesn't do anything.
> > > 
> > > > ---
> > > >  Osstest/Debian.pm     |   31 ++++++++++++++++++++-----------
> > > >  ts-debian-hvm-install |    9 +++++++--
> > > >  2 files changed, 27 insertions(+), 13 deletions(-)
> > > > 
> > > > diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
> > > > index ab09abb..4f2b584 100644
> > > > --- a/Osstest/Debian.pm
> > > > +++ b/Osstest/Debian.pm
> > > > @@ -36,6 +36,7 @@ BEGIN {
> > > >                        preseed_base
> > > >                        preseed_create
> > > >                        preseed_hook_command preseed_hook_installscript
> > > > +                      di_installcmdline_base
> > > >                        di_installcmdline_core
> > > >                        );
> > > >      %EXPORT_TAGS = ( );
> > > > @@ -398,6 +399,23 @@ END
> > > >  our %preseed_cmds;
> > > >  # $preseed_cmds{$di_key}[]= $cmd
> > > >  
> > > > +sub di_installcmdline_base($;@) {
> > > > +    my ($tho, %xopts) = @_;
> > > > +    my @cl = qw(
> > > > +                auto=true
> > > 
> > > I don't think it makes sense to move this but not the url=. I'd suggest
> > > either leaving this in the caller or doing:
> > > 
> > >        push @cl, ("auto=true", "url=$xopts{PreseedURL}") if 
> > > $xopts{PreseedURL});
> > > and setting PreseedURL in the callers as appropriate.
> > > 
> > 
> > I moved it because ts-debian-hvm-install is also doing preseed
> > installation. That is, both host and guest install need some basic
> > pressed options. You first suggestion makes ts-debian-hvm-install have
> > to duplicate "auto=true", your later suggestion doesn't work for
> > ts-debian-hvm-install as it requires a file not url...
> 
> Having the auto=true and the preseed file (whether it is url= or file=)
> in different places doesn't make sense IMHO, they should either both be
> in the caller or both in this helper.
> > 

OK, I will come up with a new version this afternoon.

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