Re: [Xen-devel] [PATCH OSSTEST 01/12] Add support of parsing grub which has 'submenu' primitive

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Thu, Feb 12, 2015 at 02:01:59AM +0000, Hu, Robert wrote:
> 
> > -----Original Message-----
> > From: Ian Jackson [mailto:Ian.Jackson@xxxxxxxxxxxxx]
> > Sent: Wednesday, February 11, 2015 10:44 PM
> > To: Hu, Robert
> > Cc: xen-devel@xxxxxxxxxxxxx; jfehlig@xxxxxxxx; wei.liu2@xxxxxxxxxx;
> > ian.campbell@xxxxxxxxxx; Pang, LongtaoX
> > Subject: Re: [PATCH OSSTEST 01/12] Add support of parsing grub which has
> > 'submenu' primitive
> > 
> > Robert Ho writes ("[PATCH OSSTEST 01/12] Add support of parsing grub which
> > has 'submenu' primitive"):
> > >  From a hvm kernel build from Linux stable Kernel tree,
> > >  the auto generated grub2 menu will have 'submenu' primitive, upon the
> > >  'menuentry' items. Xen boot entries will be grouped into a submenu. This
> > >  patch adds capability to support such grub formats. Also, this patch 
> > > adjust
> > >  some indent alignments.
> > 
> > Thanks for this submission.  Dealing with submenus is definitely
> > something we want to do.
> > 
> > I haven't looked at the code in detail yet but I have a general
> > question: we currently count menu entries and eventually write
> > GRUB_DEFAULT=<some number>  into /etc/default/grub.
> > 
> > Does this work properly if the entry is in a submenu ?  I guess you
> > have probably tested this but I thought I should ask...
> > 
> Yes, this minor change just get 'parsemenu' subroutine capability of 
> recognizing 'submenu'.
> The outer layer logic isn't affected.
> Actually, the Xen boot menuentry we choose, is inside a submenu. It works and 
> /etc/default/grub
> is assigned properly.

In any case this is a very useful improvement.

Out of interest what Linux are you running?  If you're running Debian
and the overlay 20_linux_xen (inside $OSSTEST/overlay/etc/etc/grub.d) is
copied to your test host, there shouldn't be any submenu entries in your
grub.cfg, I think.

Wei.

> > Can you please not adjust the whitespace ?  osstest in general doesn't
> > have a requirement for any particular whitespace use, and certainly if
> > there are to be any whitespace changes they ought to be in a separate
> > patch.
> I adjust those because some one in last version's reply told us that
> osstest prefers white space substitution to tab, and traditionally 4
> white space of 1 tab. (This align with my previous coding experience as well)
> And I indeed find that this hunk of code doesn't looks well in my editor.
> Its unalignment increases difficulty of reading.
> I would suggest to adjust the this hunk's indentation and use white space
> substitution to tab to have best suitability across different editors.
> > 
> > Thanks,
> > Ian.

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