Re: [OSSTEST Nested PATCH v7 1/6] parsing grub which has 'submenu' primitive

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

On Fri, 2015-03-27 at 19:06 -0400, longtao.pang wrote:
> 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.
> 
> Signed-off-by: longtao.pang <longtaox.pang@intel.com>
> ---
> Changes in v7:
> Remove the reformatting change for Debian.pm and keep the original format.

Thank you.

> ---
>  Osstest/Debian.pm |   21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
> index 6784024..35163a0 100644
> --- a/Osstest/Debian.pm
> +++ b/Osstest/Debian.pm
> @@ -398,10 +398,18 @@ sub setupboot_grub2 ($$$$) {
>      
>          my $count= 0;
>          my $entry;
> +        my $submenu;
>          while (<$f>) {
>              next if m/^\s*\#/ || !m/\S/;
>              if (m/^\s*\}\s*$/) {
> -                die unless $entry;
> +                die unless $entry || $submenu;
> +                if(!defined $entry && defined $submenu){
> +                    logm("Met end of a submenu starting from ".
> +                        "$submenu->{StartLine}. ".
> +                        "Our want kern is $want_kernver");
> +                    $submenu=undef;
> +                    next;
> +                }
>                  my (@missing) =
>                      grep { !defined $entry->{$_} } 
>  		        (defined $xenhopt
> @@ -432,21 +440,24 @@ sub setupboot_grub2 ($$$$) {
>                  $entry= { Title => $1, StartLine => $., Number => $count };
>                  $count++;
>              }
> -            if (m/^\s*multiboot\s*\/(xen\-[0-9][-+.0-9a-z]*\S+)/) {
> +            if (m/^submenu\s+[\'\"](.*)[\'\"].*\{\s*$/) {
> +                $submenu={ StartLine =>$.};
> +            }

This looks reasonable enough to support a single nesting, I suppose we
can leave more deeply nested submenus for another time.

So in that regard this patch looks ok to me.

> +            if (m/^\s*multiboot\s*(?:\/boot)*\/(xen\S+)/) {
>                  die unless $entry;
>                  $entry->{Hv}= $1;
>              }
> -            if (m/^\s*multiboot\s*\/(vmlinu[xz]-(\S+))/) {
> +            if (m/^\s*multiboot\s*(?:\/boot)*\/(vmlinu[xz]-(\S+))/) {

What are these changes all about? I think they must be unrelated to the
use of submenu (perhaps relate to having a separate /boot or not?). If
so then please do in a separate patch.

If this is somehow to do with submenu then please explain how/why in the
commit log.

BTW, your regex as it stand will accept /boot/boot/boot/boot/vmlinuz. I
think you maybe meant to add "(?:\/boot)?" to match zero or one
occurrences?

Ian.