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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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.
> 
> > Also, FWIW, you can just say "auto" without the =true. No need to make
> > that change here/now though.
> > 
> 
> Not really, as per that webpage:
> 
> "The auto boot choice is not yet defined on all arches. The same effect
> may be achieved by simply adding the two parameters auto=true
> priority=critical to the kernel command line."

Ah, auto is actually a syslinux entry not a kernel command line, I
misread it.

Ian.


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