Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 9 May 2019 14:41:59 -0400
From:   Masayoshi Mizuma <msys.mizuma@gmail.com>
To:     Steven Rostedt <rostedt@goodmis.org>
Cc:     Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/5] ktest: cleanup get_grub_index
Message-ID: <20190509184158.te2cglg2nuvgm6gm@gabell>
References: <20190509174630.26713-1-msys.mizuma@gmail.com>
 <20190509174630.26713-3-msys.mizuma@gmail.com>
 <20190509135721.4802161b@gandalf.local.home>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20190509135721.4802161b@gandalf.local.home>
User-Agent: NeoMutt/20180716
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, May 09, 2019 at 01:57:21PM -0400, Steven Rostedt wrote:
> On Thu,  9 May 2019 13:46:27 -0400
> Masayoshi Mizuma <msys.mizuma@gmail.com> wrote:
> 
> > From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
> > 
> > Cleanup get_grub_index().
> 
> Hi Masayoshi,
> 
> Thanks for the patches, quick comment below.
> 
> > 
> > Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
> > ---
> >  tools/testing/ktest/ktest.pl | 50 +++++++++++-------------------------
> >  1 file changed, 15 insertions(+), 35 deletions(-)
> > 
> > diff --git a/tools/testing/ktest/ktest.pl b/tools/testing/ktest/ktest.pl
> > index 3862b23672f7..1255ea0d9df4 100755
> > --- a/tools/testing/ktest/ktest.pl
> > +++ b/tools/testing/ktest/ktest.pl
> > @@ -1934,46 +1934,26 @@ sub get_grub2_index {
> >  
> >  sub get_grub_index {
> >  
> > -    if ($reboot_type eq "grub2") {
> > -	get_grub2_index;
> > -	return;
> > -    }
> > -
> > -    if ($reboot_type ne "grub") {
> > -	return;
> > -    }
> 
> We still need something like:
> 
> 	if ($reboot_type !~ /^grub/) {
> 		return;
> 	}
> 
> Because I believe this will run (and probably error) for syslinux boot
> systems. I have a couple, I could test it and find out ;-)

Thank you for your review! I'll add the check.

Thanks!
Masa

> 
> -- Steve
> 
> > -    return if (defined($grub_number) && defined($last_grub_menu) &&
> > -	       $last_grub_menu eq $grub_menu && defined($last_machine) &&
> > -	       $last_machine eq $machine);
> > -
> > -    doprint "Find grub menu ... ";
> > -    $grub_number = -1;
> > +    my $command;
> > +    my $target;
> > +    my $skip;
> > +    my $grub_menu_qt;
> >  
> > -    my $ssh_grub = $ssh_exec;
> > -    $ssh_grub =~ s,\$SSH_COMMAND,cat /boot/grub/menu.lst,g;
> > +    return if ($reboot_type ne "grub") and ($reboot_type ne "grub2");
> >  
> > -    open(IN, "$ssh_grub |")
> > -	or dodie "unable to get menu.lst";
> > -
> > -    my $found = 0;
> > -    my $grub_menu_qt = quotemeta($grub_menu);
> > +    $grub_menu_qt = quotemeta($grub_menu);
> >  
> > -    while (<IN>) {
> > -	if (/^\s*title\s+$grub_menu_qt\s*$/) {
> > -	    $grub_number++;
> > -	    $found = 1;
> > -	    last;
> > -	} elsif (/^\s*title\s/) {
> > -	    $grub_number++;
> > -	}
> > +    if ($reboot_type eq "grub") {
> > +	$command = "cat /boot/grub/menu.lst";
> > +	$target = '^\s*title\s+' . $grub_menu_qt . '\s*$';
> > +	$skip = '^\s*title\s';
> > +    } elsif ($reboot_type eq "grub2") {
> > +	$command = "cat $grub_file";
> > +	$target = '^menuentry.*' . $grub_menu_qt;
> > +	$skip = '^menuentry\s|^submenu\s';
> >      }
> > -    close(IN);
> >  
> > -    dodie "Could not find '$grub_menu' in /boot/grub/menu on $machine"
> > -	if (!$found);
> > -    doprint "$grub_number\n";
> > -    $last_grub_menu = $grub_menu;
> > -    $last_machine = $machine;
> > +    _get_grub_index($command, $target, $skip);
> >  }
> >  
> >  sub wait_for_input
>