Re: [PATCH OSSTEST 3/7] ts-debian-di-install: Install pv-menu-list in ARM guests, always.

[Ian Campbell <ian.campbell@citrix.com>]

On Wed, 2015-08-12 at 16:35 +0100, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH OSSTEST 3/7] ts-debian-di-install: Install 
> pv-menu-list in ARM guests, always."):
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> 
> This package seems to be called `pv-grub-menu', not `pv-menu-list'.

I get that wrong more often than not it seems...

> 
> > @@ -190,9 +190,18 @@ END
> >  
> >  	$method_cfg = setup_netboot($tmpdir, $arch, $suite);
> >  
> > +	# We need the pv-menu-list package:
> > +	# - On x86 when running pvgrub, since it only speaks grub1
> > +	#   menu.lst syntax and by grub2 is now the only grub in
> > +	#   Debian.
> 
> This comment seems mangled.  "and by grub2" ?  Also in "it only speaks
> grub1", it would be clearer if "it" said "pvgrub".

Sure.

> > +	# - On ARM, which uses pygrub, because grub2 is not installed
> > +	#   on ARM by default (except for, maybe, UEFI guests in the
> > +	#   future, but not today).
> > +	my $pvmenulst = ($bl eq "pvgrub" || $arch =~ /arm/);
> 
> "pvgrub" here is pvgrub1, not pv grub2.  This could perhaps be
> clearer...

Yes, although pvgrub is what we use in production now. I could go and
change it everywhere if you like, or we could defer to the point where we
add pv grub 2 testing?

> 
> Thanks,
> Ian.