From: Masayoshi Mizuma <[email protected]>
Cleanup get_grub_index().
Signed-off-by: Masayoshi Mizuma <[email protected]>
---
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;
- }
- 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
--
2.20.1
On Thu, 9 May 2019 13:46:27 -0400
Masayoshi Mizuma <[email protected]> wrote:
> From: Masayoshi Mizuma <[email protected]>
>
> Cleanup get_grub_index().
Hi Masayoshi,
Thanks for the patches, quick comment below.
>
> Signed-off-by: Masayoshi Mizuma <[email protected]>
> ---
> 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 ;-)
-- 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
On Thu, May 09, 2019 at 01:57:21PM -0400, Steven Rostedt wrote:
> On Thu, 9 May 2019 13:46:27 -0400
> Masayoshi Mizuma <[email protected]> wrote:
>
> > From: Masayoshi Mizuma <[email protected]>
> >
> > Cleanup get_grub_index().
>
> Hi Masayoshi,
>
> Thanks for the patches, quick comment below.
>
> >
> > Signed-off-by: Masayoshi Mizuma <[email protected]>
> > ---
> > 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
>
On Thu, 9 May 2019 14:41:59 -0400
Masayoshi Mizuma <[email protected]> wrote:
> On Thu, May 09, 2019 at 01:57:21PM -0400, Steven Rostedt wrote:
> > On Thu, 9 May 2019 13:46:27 -0400
> > Masayoshi Mizuma <[email protected]> wrote:
> >
> > > From: Masayoshi Mizuma <[email protected]>
> > >
> > > Cleanup get_grub_index().
> >
> > Hi Masayoshi,
> >
> > Thanks for the patches, quick comment below.
> >
> > >
> > > Signed-off-by: Masayoshi Mizuma <[email protected]>
> > > ---
> > > 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.
>
> > > - 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");
I missed that you added that, which is basically the same. But I think
just doing a check of "^grub" is a bit cleaner.
Looking forward for a v2 of your patch series.
Note, I do have some travel coming up, so if you don't hear from me in
a week after you post them, please send me a ping, as my email gets
harder to maintain when I travel.
Thanks!
-- Steve
> > >
> > > - 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
> >