linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] ktest: support for Boot Loader Specification
@ 2019-05-09 17:46 Masayoshi Mizuma
  2019-05-09 17:46 ` [PATCH 1/5] ktest: introduce _get_grub_index Masayoshi Mizuma
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Masayoshi Mizuma @ 2019-05-09 17:46 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Masayoshi Mizuma, Masayoshi Mizuma, linux-kernel

From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>

Fedora 30 introduces Boot Loader Specification (BLS) [1],
it changes around grub entry configuration.

This patchset deals with the new configuration.

- Add grub2bls option as REBOOT_TYPE to deal with BLS.
- Some cleanup around getting kernel entries.

[1] https://fedoraproject.org/wiki/Changes/BootLoaderSpecByDefault

Masayoshi Mizuma (5):
  ktest: introduce _get_grub_index
  ktest: cleanup get_grub_index
  ktest: introduce grub2bls REBOOT_TYPE option
  ktest: remove get_grub2_index
  ktest: update sample.conf for grub2bls

 tools/testing/ktest/ktest.pl    | 86 +++++++++++++++------------------
 tools/testing/ktest/sample.conf | 10 +++-
 2 files changed, 48 insertions(+), 48 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/5] ktest: introduce _get_grub_index
  2019-05-09 17:46 [PATCH 0/5] ktest: support for Boot Loader Specification Masayoshi Mizuma
@ 2019-05-09 17:46 ` Masayoshi Mizuma
  2019-05-09 17:46 ` [PATCH 2/5] ktest: cleanup get_grub_index Masayoshi Mizuma
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Masayoshi Mizuma @ 2019-05-09 17:46 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Masayoshi Mizuma, Masayoshi Mizuma, linux-kernel

From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>

Introduce _get_grub_index() to deal with Boot Loader
Specification (BLS) and cleanup.

Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
---
 tools/testing/ktest/ktest.pl | 37 ++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/tools/testing/ktest/ktest.pl b/tools/testing/ktest/ktest.pl
index 751e32a31ed4..3862b23672f7 100755
--- a/tools/testing/ktest/ktest.pl
+++ b/tools/testing/ktest/ktest.pl
@@ -1859,6 +1859,43 @@ sub run_scp_mod {
     return run_scp($src, $dst, $cp_scp);
 }
 
+sub _get_grub_index {
+
+    my ($command, $target, $skip) = @_;
+
+    return if (defined($grub_number) && defined($last_grub_menu) &&
+	       $last_grub_menu eq $grub_menu && defined($last_machine) &&
+	       $last_machine eq $machine);
+
+    doprint "Find $reboot_type menu ... ";
+    $grub_number = -1;
+
+    my $ssh_grub = $ssh_exec;
+    $ssh_grub =~ s,\$SSH_COMMAND,$command,g;
+
+    open(IN, "$ssh_grub |")
+	or dodie "unable to execute $command";
+
+    my $found = 0;
+
+    while (<IN>) {
+	if (/$target/) {
+	    $grub_number++;
+	    $found = 1;
+	    last;
+	} elsif (/$skip/) {
+	    $grub_number++;
+	}
+    }
+    close(IN);
+
+    dodie "Could not find '$grub_menu' through $command on $machine"
+	if (!$found);
+    doprint "$grub_number\n";
+    $last_grub_menu = $grub_menu;
+    $last_machine = $machine;
+}
+
 sub get_grub2_index {
 
     return if (defined($grub_number) && defined($last_grub_menu) &&
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/5] ktest: cleanup get_grub_index
  2019-05-09 17:46 [PATCH 0/5] ktest: support for Boot Loader Specification Masayoshi Mizuma
  2019-05-09 17:46 ` [PATCH 1/5] ktest: introduce _get_grub_index Masayoshi Mizuma
@ 2019-05-09 17:46 ` Masayoshi Mizuma
  2019-05-09 17:57   ` Steven Rostedt
  2019-05-09 17:46 ` [PATCH 3/5] ktest: introduce grub2bls REBOOT_TYPE option Masayoshi Mizuma
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Masayoshi Mizuma @ 2019-05-09 17:46 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Masayoshi Mizuma, Masayoshi Mizuma, linux-kernel

From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>

Cleanup get_grub_index().

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;
-    }
-    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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/5] ktest: introduce grub2bls REBOOT_TYPE option
  2019-05-09 17:46 [PATCH 0/5] ktest: support for Boot Loader Specification Masayoshi Mizuma
  2019-05-09 17:46 ` [PATCH 1/5] ktest: introduce _get_grub_index Masayoshi Mizuma
  2019-05-09 17:46 ` [PATCH 2/5] ktest: cleanup get_grub_index Masayoshi Mizuma
@ 2019-05-09 17:46 ` Masayoshi Mizuma
  2019-05-09 17:46 ` [PATCH 4/5] ktest: remove get_grub2_index Masayoshi Mizuma
  2019-05-09 17:46 ` [PATCH 5/5] ktest: update sample.conf for grub2bls Masayoshi Mizuma
  4 siblings, 0 replies; 9+ messages in thread
From: Masayoshi Mizuma @ 2019-05-09 17:46 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Masayoshi Mizuma, Masayoshi Mizuma, linux-kernel

From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>

Fedora 30 introduces Boot Loader Specification (BLS),
it changes around grub entry configuration.

kernel entries aren't in grub.cfg. We can get the entries
by "grubby --info=ALL" command.

Introduce grub2bls as REBOOT_TYPE option for BLS.

Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
---
 tools/testing/ktest/ktest.pl | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/tools/testing/ktest/ktest.pl b/tools/testing/ktest/ktest.pl
index 1255ea0d9df4..c910d7921f48 100755
--- a/tools/testing/ktest/ktest.pl
+++ b/tools/testing/ktest/ktest.pl
@@ -64,6 +64,7 @@ my %default = (
     "STOP_TEST_AFTER"		=> 600,
     "MAX_MONITOR_WAIT"		=> 1800,
     "GRUB_REBOOT"		=> "grub2-reboot",
+    "GRUB_BLS_GET"		=> "grubby --info=ALL",
     "SYSLINUX"			=> "extlinux",
     "SYSLINUX_PATH"		=> "/boot/extlinux",
     "CONNECT_TIMEOUT"		=> 25,
@@ -125,6 +126,7 @@ my $last_grub_menu;
 my $grub_file;
 my $grub_number;
 my $grub_reboot;
+my $grub_bls_get;
 my $syslinux;
 my $syslinux_path;
 my $syslinux_label;
@@ -295,6 +297,7 @@ my %option_map = (
     "GRUB_MENU"			=> \$grub_menu,
     "GRUB_FILE"			=> \$grub_file,
     "GRUB_REBOOT"		=> \$grub_reboot,
+    "GRUB_BLS_GET"		=> \$grub_bls_get,
     "SYSLINUX"			=> \$syslinux,
     "SYSLINUX_PATH"		=> \$syslinux_path,
     "SYSLINUX_LABEL"		=> \$syslinux_label,
@@ -440,7 +443,7 @@ EOF
     ;
 $config_help{"REBOOT_TYPE"} = << "EOF"
  Way to reboot the box to the test kernel.
- Only valid options so far are "grub", "grub2", "syslinux", and "script".
+ Only valid options so far are "grub", "grub2", "grub2bls", "syslinux", and "script".
 
  If you specify grub, it will assume grub version 1
  and will search in /boot/grub/menu.lst for the title \$GRUB_MENU
@@ -454,6 +457,8 @@ $config_help{"REBOOT_TYPE"} = << "EOF"
  If you specify grub2, then you also need to specify both \$GRUB_MENU
  and \$GRUB_FILE.
 
+ If you specify grub2bls, then you also need to specify \$GRUB_MENU.
+
  If you specify syslinux, then you may use SYSLINUX to define the syslinux
  command (defaults to extlinux), and SYSLINUX_PATH to specify the path to
  the syslinux install (defaults to /boot/extlinux). But you have to specify
@@ -479,6 +484,9 @@ $config_help{"GRUB_MENU"} = << "EOF"
  menu must be a non-nested menu. Add the quotes used in the menu
  to guarantee your selection, as the first menuentry with the content
  of \$GRUB_MENU that is found will be used.
+
+ For grub2bls, \$GRUB_MENU is searched on the result of \$GRUB_BLS_GET
+ command for the lines that begin with "title".
 EOF
     ;
 $config_help{"GRUB_FILE"} = << "EOF"
@@ -695,7 +703,7 @@ sub get_mandatory_configs {
 	}
     }
 
-    if ($rtype eq "grub") {
+    if (($rtype eq "grub") or ($rtype eq "grub2bls")) {
 	get_mandatory_config("GRUB_MENU");
     }
 
@@ -1939,7 +1947,8 @@ sub get_grub_index {
     my $skip;
     my $grub_menu_qt;
 
-    return if ($reboot_type ne "grub") and ($reboot_type ne "grub2");
+    return if ($reboot_type ne "grub") and ($reboot_type ne "grub2") and
+	($reboot_type ne "grub2bls");
 
     $grub_menu_qt = quotemeta($grub_menu);
 
@@ -1951,6 +1960,10 @@ sub get_grub_index {
 	$command = "cat $grub_file";
 	$target = '^menuentry.*' . $grub_menu_qt;
 	$skip = '^menuentry\s|^submenu\s';
+    } elsif ($reboot_type eq "grub2bls") {
+        $command = $grub_bls_get;
+        $target = '^title=.*' . $grub_menu_qt;
+        $skip = '^title=';
     }
 
     _get_grub_index($command, $target, $skip);
@@ -4306,7 +4319,7 @@ for (my $i = 1; $i <= $opt{"NUM_TESTS"}; $i++) {
 
     if (!$buildonly) {
 	$target = "$ssh_user\@$machine";
-	if ($reboot_type eq "grub") {
+	if (($reboot_type eq "grub") or ($reboot_type eq "grub2bls")) {
 	    dodie "GRUB_MENU not defined" if (!defined($grub_menu));
 	} elsif ($reboot_type eq "grub2") {
 	    dodie "GRUB_MENU not defined" if (!defined($grub_menu));
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/5] ktest: remove get_grub2_index
  2019-05-09 17:46 [PATCH 0/5] ktest: support for Boot Loader Specification Masayoshi Mizuma
                   ` (2 preceding siblings ...)
  2019-05-09 17:46 ` [PATCH 3/5] ktest: introduce grub2bls REBOOT_TYPE option Masayoshi Mizuma
@ 2019-05-09 17:46 ` Masayoshi Mizuma
  2019-05-09 17:46 ` [PATCH 5/5] ktest: update sample.conf for grub2bls Masayoshi Mizuma
  4 siblings, 0 replies; 9+ messages in thread
From: Masayoshi Mizuma @ 2019-05-09 17:46 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Masayoshi Mizuma, Masayoshi Mizuma, linux-kernel

From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>

Remove get_grub2_index() because it isn't used anywhere.

Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
---
 tools/testing/ktest/ktest.pl | 36 ------------------------------------
 1 file changed, 36 deletions(-)

diff --git a/tools/testing/ktest/ktest.pl b/tools/testing/ktest/ktest.pl
index c910d7921f48..e965751ad2da 100755
--- a/tools/testing/ktest/ktest.pl
+++ b/tools/testing/ktest/ktest.pl
@@ -1904,42 +1904,6 @@ sub _get_grub_index {
     $last_machine = $machine;
 }
 
-sub get_grub2_index {
-
-    return if (defined($grub_number) && defined($last_grub_menu) &&
-	       $last_grub_menu eq $grub_menu && defined($last_machine) &&
-	       $last_machine eq $machine);
-
-    doprint "Find grub2 menu ... ";
-    $grub_number = -1;
-
-    my $ssh_grub = $ssh_exec;
-    $ssh_grub =~ s,\$SSH_COMMAND,cat $grub_file,g;
-
-    open(IN, "$ssh_grub |")
-	or dodie "unable to get $grub_file";
-
-    my $found = 0;
-    my $grub_menu_qt = quotemeta($grub_menu);
-
-    while (<IN>) {
-	if (/^menuentry.*$grub_menu_qt/) {
-	    $grub_number++;
-	    $found = 1;
-	    last;
-	} elsif (/^menuentry\s|^submenu\s/) {
-	    $grub_number++;
-	}
-    }
-    close(IN);
-
-    dodie "Could not find '$grub_menu' in $grub_file on $machine"
-	if (!$found);
-    doprint "$grub_number\n";
-    $last_grub_menu = $grub_menu;
-    $last_machine = $machine;
-}
-
 sub get_grub_index {
 
     my $command;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 5/5] ktest: update sample.conf for grub2bls
  2019-05-09 17:46 [PATCH 0/5] ktest: support for Boot Loader Specification Masayoshi Mizuma
                   ` (3 preceding siblings ...)
  2019-05-09 17:46 ` [PATCH 4/5] ktest: remove get_grub2_index Masayoshi Mizuma
@ 2019-05-09 17:46 ` Masayoshi Mizuma
  4 siblings, 0 replies; 9+ messages in thread
From: Masayoshi Mizuma @ 2019-05-09 17:46 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Masayoshi Mizuma, Masayoshi Mizuma, linux-kernel

From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>

Update sample.conf for grub2bls.

Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
---
 tools/testing/ktest/sample.conf | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/testing/ktest/sample.conf b/tools/testing/ktest/sample.conf
index 8c893a58b68e..5ac32ab1f3bc 100644
--- a/tools/testing/ktest/sample.conf
+++ b/tools/testing/ktest/sample.conf
@@ -349,13 +349,13 @@
 # option to boot to with GRUB_REBOOT
 #GRUB_FILE = /boot/grub2/grub.cfg
 
-# The tool for REBOOT_TYPE = grub2 to set the next reboot kernel
+# The tool for REBOOT_TYPE = grub2 or grub2bls to set the next reboot kernel
 # to boot into (one shot mode).
 # (default grub2_reboot)
 #GRUB_REBOOT = grub2_reboot
 
 # The grub title name for the test kernel to boot
-# (Only mandatory if REBOOT_TYPE = grub or grub2)
+# (Only mandatory if REBOOT_TYPE = grub or grub2 or grub2bls)
 #
 # Note, ktest.pl will not update the grub menu.lst, you need to
 # manually add an option for the test. ktest.pl will search
@@ -374,6 +374,10 @@
 # do a: GRUB_MENU = 'Test Kernel'
 # For customizing, add your entry in /etc/grub.d/40_custom.
 #
+# For grub2bls, a search of "title"s are done. The menu is found
+# by searching for the contents of GRUB_MENU in the line that starts
+# with "title".
+#
 #GRUB_MENU = Test Kernel
 
 # For REBOOT_TYPE = syslinux, the name of the syslinux executable
@@ -593,6 +597,8 @@
 # For REBOOT_TYPE = grub2, you must define both GRUB_MENU and
 # GRUB_FILE.
 #
+# For REBOOT_TYPE = grub2bls, you must define GRUB_MENU.
+#
 # For REBOOT_TYPE = syslinux, you must define SYSLINUX_LABEL, and
 # perhaps modify SYSLINUX (default extlinux) and SYSLINUX_PATH
 # (default /boot/extlinux)
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/5] ktest: cleanup get_grub_index
  2019-05-09 17:46 ` [PATCH 2/5] ktest: cleanup get_grub_index Masayoshi Mizuma
@ 2019-05-09 17:57   ` Steven Rostedt
  2019-05-09 18:41     ` Masayoshi Mizuma
  0 siblings, 1 reply; 9+ messages in thread
From: Steven Rostedt @ 2019-05-09 17:57 UTC (permalink / raw)
  To: Masayoshi Mizuma; +Cc: Masayoshi Mizuma, linux-kernel

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 ;-)

-- 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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/5] ktest: cleanup get_grub_index
  2019-05-09 17:57   ` Steven Rostedt
@ 2019-05-09 18:41     ` Masayoshi Mizuma
  2019-05-09 19:13       ` Steven Rostedt
  0 siblings, 1 reply; 9+ messages in thread
From: Masayoshi Mizuma @ 2019-05-09 18:41 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Masayoshi Mizuma, linux-kernel

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
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/5] ktest: cleanup get_grub_index
  2019-05-09 18:41     ` Masayoshi Mizuma
@ 2019-05-09 19:13       ` Steven Rostedt
  0 siblings, 0 replies; 9+ messages in thread
From: Steven Rostedt @ 2019-05-09 19:13 UTC (permalink / raw)
  To: Masayoshi Mizuma; +Cc: Masayoshi Mizuma, linux-kernel

On Thu, 9 May 2019 14:41:59 -0400
Masayoshi Mizuma <msys.mizuma@gmail.com> 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 <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.
>


> > > -    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  
> >   


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-05-09 19:13 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-09 17:46 [PATCH 0/5] ktest: support for Boot Loader Specification Masayoshi Mizuma
2019-05-09 17:46 ` [PATCH 1/5] ktest: introduce _get_grub_index Masayoshi Mizuma
2019-05-09 17:46 ` [PATCH 2/5] ktest: cleanup get_grub_index Masayoshi Mizuma
2019-05-09 17:57   ` Steven Rostedt
2019-05-09 18:41     ` Masayoshi Mizuma
2019-05-09 19:13       ` Steven Rostedt
2019-05-09 17:46 ` [PATCH 3/5] ktest: introduce grub2bls REBOOT_TYPE option Masayoshi Mizuma
2019-05-09 17:46 ` [PATCH 4/5] ktest: remove get_grub2_index Masayoshi Mizuma
2019-05-09 17:46 ` [PATCH 5/5] ktest: update sample.conf for grub2bls Masayoshi Mizuma

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).