Re: [PATCH] tell the user why they are in rescue mode

[Sam Morris <sam@robots.org.uk>]

On Fri, 08 Feb 2008 02:10:29 +0100, Robert Millan wrote:

> Hi Sam,
> 
> On Thu, Feb 07, 2008 at 06:34:26PM +0000, Sam Morris wrote:
>> diff -ru grub2-1.96+20080203+orig/kern/main.c
>> grub2-1.96+20080203+sam/kern/main.c ---
>> grub2-1.96+20080203+orig/kern/main.c	2008-01-05 12:04:35.000000000
>> +0000 +++ grub2-1.96+20080203+sam/kern/main.c	2008-02-07
>> 08:41:01.000000000 +0000 @@ -102,8 +102,13 @@
>>    /* Load the module.  */
>>    grub_dl_load ("normal");
>>    
>> -  /* Ignore any error, because we have the rescue mode anyway.  */ - 
>> grub_errno = GRUB_ERR_NONE;
>> +  if (grub_errno != GRUB_ERR_NONE)
>> +    {
>> +      grub_printf ("Unable to enter 'normal' mode (error %d: %s)\n",
>> grub_errno, grub_errmsg); +
>> +      /* We're about to continue into rescue mode, so clear the error.
>> */ +      grub_errno = GRUB_ERR_NONE;
>> +    }
> 
> I just checked, and it seems we already have a function for this:
> grub_print_error().  If you just invoke this function, do you get the
> desired result?

Yes, it works fine.

> Also, I wonder why the existing grub_print_error() call in the rescue
> loop doesn't handle this already (in kern/rescue.c).
> 
> Perhaps all you need to do is remove these two lines?
> 
> diff -ur grub2/kern/main.c tmp/kern/main.c --- grub2/kern/main.c  
> 2008-01-05 13:04:35.000000000 +0100 +++ tmp/kern/main.c     2008-02-08
> 02:09:03.000000000 +0100 @@ -101,9 +101,6 @@
>  {
>    /* Load the module.  */
>    grub_dl_load ("normal");
> -
> -  /* Ignore any error, because we have the rescue mode anyway.  */ - 
> grub_errno = GRUB_ERR_NONE;
>  }
> 
>  /* The main routine.  */

While that works, the output looks like this:

 Entering rescue mode...
 error: out of disk
 > _

Which does not present the information in the clearest possible way: it 
looks like the error happened in the rescue mode code, instead of the 
normal mode code.

Also, the error message could be lost if any of the code between the 
start of grub_enter_rescue_mode and its call to grub_print_error itself 
triggers an error.

For these reasons I think it's better to call grub_print_error from 
within grub_load_normal.

BTW, does anyone know the first thing grub_enter_rescue_mode does is call 
attempt_normal_mode?

-- 
Sam Morris
http://robots.org.uk/
 
PGP key id 1024D/5EA01078
3412 EA18 1277 354B 991B  C869 B219 7FDB 5EA0 1078