normal does not load if there was an error processing embedded config

normal does not load if there was an error processing embedded config

[Andrei Borzenkov]

Resetting grub_errno to GRUB_ERR_NONE before loading normal does allow
it to proceed.

At this point I'm not sure what is the correct behavior. Either
explicitly check for error and simply do not even try to automatically
jump into normal (it is still possible to load it manually) or reset
grub_errno and ignore.

The problem was exposed by someone trying to build minimal possible
image using grub-mkrescue and so omitting most partition modules. This
resulted in insmod for partition modules failure in embedded config.

I still tend to think the following patch matches current behavior - we
do not abort processing embedded config on error either. Alternative
would be to check and abort processing as soon as error is encountered.



From: Andrei Borzenkov <arvidjaar@gmail.com>
Subject: [PATCH] kernel: reset grub_errno before loading normal

If last command in embedded config set grub_errno, loading of
normal fails. As we do not really check for any errors during
processing of embedded config and cannot handle error that happened
in the middle of it, just ignore error and hope for best.

---
 grub-core/kern/main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/grub-core/kern/main.c b/grub-core/kern/main.c
index 9cad0c4..2e46e2c 100644
--- a/grub-core/kern/main.c
+++ b/grub-core/kern/main.c
@@ -304,6 +304,8 @@ grub_main (void)
 
   if (load_config)
     grub_parser_execute (load_config);
+  /* Reset error, otherwise loading of normal fails */
+  grub_errno = GRUB_ERR_NONE;
 
   grub_boot_time ("After execution of embedded config. Attempt to go to normal mode");
 
-- 
tg: (018f79d..) u/reset-error-before-normal (depends on: master)

Re: normal does not load if there was an error processing embedded config

[Vladimir 'phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 2146 bytes --]

Could you use grub-print-error rather than just resetting error? And we
should do it after every command in embedded config
Le 2015-02-28 6:45 PM, "Andrei Borzenkov" <arvidjaar@gmail.com> a écrit :

> Resetting grub_errno to GRUB_ERR_NONE before loading normal does allow
> it to proceed.
>
> At this point I'm not sure what is the correct behavior. Either
> explicitly check for error and simply do not even try to automatically
> jump into normal (it is still possible to load it manually) or reset
> grub_errno and ignore.
>
> The problem was exposed by someone trying to build minimal possible
> image using grub-mkrescue and so omitting most partition modules. This
> resulted in insmod for partition modules failure in embedded config.
>
> I still tend to think the following patch matches current behavior - we
> do not abort processing embedded config on error either. Alternative
> would be to check and abort processing as soon as error is encountered.
>
>
>
> From: Andrei Borzenkov <arvidjaar@gmail.com>
> Subject: [PATCH] kernel: reset grub_errno before loading normal
>
> If last command in embedded config set grub_errno, loading of
> normal fails. As we do not really check for any errors during
> processing of embedded config and cannot handle error that happened
> in the middle of it, just ignore error and hope for best.
>
> ---
>  grub-core/kern/main.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/grub-core/kern/main.c b/grub-core/kern/main.c
> index 9cad0c4..2e46e2c 100644
> --- a/grub-core/kern/main.c
> +++ b/grub-core/kern/main.c
> @@ -304,6 +304,8 @@ grub_main (void)
>
>    if (load_config)
>      grub_parser_execute (load_config);
> +  /* Reset error, otherwise loading of normal fails */
> +  grub_errno = GRUB_ERR_NONE;
>
>    grub_boot_time ("After execution of embedded config. Attempt to go to
> normal mode");
>
> --
> tg: (018f79d..) u/reset-error-before-normal (depends on: master)
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

[-- Attachment #2: Type: text/html, Size: 2685 bytes --]

Re: normal does not load if there was an error processing embedded config

[Andrei Borzenkov]

В Sat, 28 Feb 2015 19:11:51 +0100
"Vladimir 'phcoder' Serbinenko" <phcoder@gmail.com> пишет:

> Could you use grub-print-error rather than just resetting error? And we
> should do it after every command in embedded config


Like below? Note that if loading of normal.mod succeeds these errors are
lost anyway. There was at least one report where skipping loading in
this case would actually make diagnostic easier (signed EFI image with
missing filesystem driver for /boot). But I'm not sure if anyone
depends on current behavior. grub-mkrescue definitely does :)

From: Andrei Borzenkov <arvidjaar@gmail.com>
Subject: [PATCH] kernel: reset grub_errno before loading normal

If last command in embedded config set grub_errno, loading of
normal fails. As we do not really check for any errors during
processing of embedded config and cannot handle error that happened
in the middle of it, just ignore error and hope for the best.

Add also printing of errors during embedded config processing to
facilitate diagnostic as suggested by Vladimir.

---
 grub-core/kern/main.c   | 2 ++
 grub-core/kern/parser.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/grub-core/kern/main.c b/grub-core/kern/main.c
index 9cad0c4..22483b6 100644
--- a/grub-core/kern/main.c
+++ b/grub-core/kern/main.c
@@ -304,6 +304,8 @@ grub_main (void)
 
   if (load_config)
     grub_parser_execute (load_config);
+  /* Reset error, otherwise loading of normal.mod fails */
+  grub_errno = GRUB_ERR_NONE;
 
   grub_boot_time ("After execution of embedded config. Attempt to go to normal mode");
 
diff --git a/grub-core/kern/parser.c b/grub-core/kern/parser.c
index b9bd123..13acb6c 100644
--- a/grub-core/kern/parser.c
+++ b/grub-core/kern/parser.c
@@ -265,9 +265,11 @@ grub_parser_execute (char *source)
     {
       char *line;
 
+      grub_errno = GRUB_ERR_NONE;
       grub_parser_execute_getline (&line, 0, &source);
       grub_rescue_parse_line (line, grub_parser_execute_getline, &source);
       grub_free (line);
+      grub_print_error ();
     }
 
   return grub_errno;
-- 
tg: (018f79d..) u/reset-error-before-normal (depends on: master)

Re: normal does not load if there was an error processing embedded config

[Andrei Borzenkov]

28.02.2015 22:33, Andrei Borzenkov пишет:
> В Sat, 28 Feb 2015 19:11:51 +0100
> "Vladimir 'phcoder' Serbinenko" <phcoder@gmail.com> пишет:
> 
>> Could you use grub-print-error rather than just resetting error? And we
>> should do it after every command in embedded config
> 
> 
> Like below? Note that if loading of normal.mod succeeds these errors are
> lost anyway. There was at least one report where skipping loading in
> this case would actually make diagnostic easier (signed EFI image with
> missing filesystem driver for /boot). But I'm not sure if anyone
> depends on current behavior. grub-mkrescue definitely does :)
> 

I committed simplified version (caused also failure in embedded config
processing itself). Before embedded config the only error we can get is
OOM. Not sure if we need extra code for this corner case.

> From: Andrei Borzenkov <arvidjaar@gmail.com>
> Subject: [PATCH] kernel: reset grub_errno before loading normal
> 
> If last command in embedded config set grub_errno, loading of
> normal fails. As we do not really check for any errors during
> processing of embedded config and cannot handle error that happened
> in the middle of it, just ignore error and hope for the best.
> 
> Add also printing of errors during embedded config processing to
> facilitate diagnostic as suggested by Vladimir.
> 
> ---
>  grub-core/kern/main.c   | 2 ++
>  grub-core/kern/parser.c | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/grub-core/kern/main.c b/grub-core/kern/main.c
> index 9cad0c4..22483b6 100644
> --- a/grub-core/kern/main.c
> +++ b/grub-core/kern/main.c
> @@ -304,6 +304,8 @@ grub_main (void)
>  
>    if (load_config)
>      grub_parser_execute (load_config);
> +  /* Reset error, otherwise loading of normal.mod fails */
> +  grub_errno = GRUB_ERR_NONE;
>  
>    grub_boot_time ("After execution of embedded config. Attempt to go to normal mode");
>  
> diff --git a/grub-core/kern/parser.c b/grub-core/kern/parser.c
> index b9bd123..13acb6c 100644
> --- a/grub-core/kern/parser.c
> +++ b/grub-core/kern/parser.c
> @@ -265,9 +265,11 @@ grub_parser_execute (char *source)
>      {
>        char *line;
>  
> +      grub_errno = GRUB_ERR_NONE;
>        grub_parser_execute_getline (&line, 0, &source);
>        grub_rescue_parse_line (line, grub_parser_execute_getline, &source);
>        grub_free (line);
> +      grub_print_error ();
>      }
>  
>    return grub_errno;
>