> -----Original Message----- > From: Wei Liu [mailto:wei.l...@citrix.com] > Sent: Thursday, February 12, 2015 6:13 PM > To: Hu, Robert > Cc: Ian Jackson; xen-devel@lists.xen.org; jfeh...@suse.com; > wei.l...@citrix.com; ian.campb...@citrix.com; Pang, LongtaoX > Subject: Re: [PATCH OSSTEST 01/12] Add support of parsing grub which has > 'submenu' primitive > > On Thu, Feb 12, 2015 at 02:01:59AM +0000, Hu, Robert wrote: > > > > > -----Original Message----- > > > From: Ian Jackson [mailto:ian.jack...@eu.citrix.com] > > > Sent: Wednesday, February 11, 2015 10:44 PM > > > To: Hu, Robert > > > Cc: xen-devel@lists.xen.org; jfeh...@suse.com; wei.l...@citrix.com; > > > ian.campb...@citrix.com; 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. We use Debian + linux-stable kernel in the test. Didn't look into details of the grub generating procedure, but my observation is that it does have the submenu. > > > > 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@lists.xen.org http://lists.xen.org/xen-devel
