[PATCH v0] Require human interaction to go to normal shell if grub.cfg has a problem

[PATCH v0] Require human interaction to go to normal shell if grub.cfg has a problem

[Jon McCune]

The rescue prompt is very useful for human operators, but not so
useful in unattended environments.  Add a facility for rebooting
after a delay, so that, e.g., the system can still PXE boot if
there is no console attached.

Signed-off-by: Jon McCune <jonmccune@google.com>
---
 grub-core/normal/main.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/grub-core/normal/main.c b/grub-core/normal/main.c
index ad36273..f8953d5 100644
--- a/grub-core/normal/main.c
+++ b/grub-core/normal/main.c
@@ -32,6 +32,7 @@
 #include <grub/i18n.h>
 #include <grub/charset.h>
 #include <grub/script_sh.h>
+#include <grub/time.h>
 
 GRUB_MOD_LICENSE ("GPLv3+");
 
@@ -333,6 +334,21 @@ grub_normal_execute (const char *config, int nested, int batch)
     }
 }
 
+/* Copied from grub-core/commands/sleep.c. */
+static int
+grub_interruptible_millisleep (grub_uint32_t ms)
+{
+  grub_uint64_t start;
+
+  start = grub_get_time_ms ();
+
+  while (grub_get_time_ms () - start < ms)
+    if (grub_getkey_noblock () == GRUB_TERM_ESC)
+      return 1;
+
+  return 0;
+}
+
 /* This starts the normal mode.  */
 void
 grub_enter_normal_mode (const char *config)
@@ -340,6 +356,15 @@ grub_enter_normal_mode (const char *config)
   grub_boot_time ("Entering normal mode");
   nested_level++;
   grub_normal_execute (config, 0, 0);
+  /* Control only returns from grub_normal_execute() if there is some kind of
+   * problem with grub.cfg, like it does not exist.  Reboot by default unless
+   * ESC is pressed within 5 seconds. */
+  grub_printf ("Press ESC in 5 seconds for a rescue shell.\n");
+  if (!grub_interruptible_millisleep (5000))
+    {
+      grub_printf ("Rebooting instead of going to rescue shell.\n");
+      grub_reboot ();
+    }
   grub_boot_time ("Entering shell");
   grub_cmdline_run (0);
   nested_level--;
-- 
1.8.4.1

Re: [PATCH v0] Require human interaction to go to normal shell if grub.cfg has a problem

[Vladimir 'phcoder' Serbinenko]

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

Just a wild idea perhaps we should embed osdetect.cfg in normal.mod in some
form. It would be pretty much the easy solution for many cases.
On Nov 26, 2013 12:47 AM, "Jon McCune" <jonmccune@google.com> wrote:

> The rescue prompt is very useful for human operators, but not so
> useful in unattended environments.  Add a facility for rebooting
> after a delay, so that, e.g., the system can still PXE boot if
> there is no console attached.
>
> Signed-off-by: Jon McCune <jonmccune@google.com>
> ---
>  grub-core/normal/main.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/grub-core/normal/main.c b/grub-core/normal/main.c
> index ad36273..f8953d5 100644
> --- a/grub-core/normal/main.c
> +++ b/grub-core/normal/main.c
> @@ -32,6 +32,7 @@
>  #include <grub/i18n.h>
>  #include <grub/charset.h>
>  #include <grub/script_sh.h>
> +#include <grub/time.h>
>
>  GRUB_MOD_LICENSE ("GPLv3+");
>
> @@ -333,6 +334,21 @@ grub_normal_execute (const char *config, int nested,
> int batch)
>      }
>  }
>
> +/* Copied from grub-core/commands/sleep.c. */
> +static int
> +grub_interruptible_millisleep (grub_uint32_t ms)
> +{
> +  grub_uint64_t start;
> +
> +  start = grub_get_time_ms ();
> +
> +  while (grub_get_time_ms () - start < ms)
> +    if (grub_getkey_noblock () == GRUB_TERM_ESC)
> +      return 1;
> +
> +  return 0;
> +}
> +
>  /* This starts the normal mode.  */
>  void
>  grub_enter_normal_mode (const char *config)
> @@ -340,6 +356,15 @@ grub_enter_normal_mode (const char *config)
>    grub_boot_time ("Entering normal mode");
>    nested_level++;
>    grub_normal_execute (config, 0, 0);
> +  /* Control only returns from grub_normal_execute() if there is some
> kind of
> +   * problem with grub.cfg, like it does not exist.  Reboot by default
> unless
> +   * ESC is pressed within 5 seconds. */
> +  grub_printf ("Press ESC in 5 seconds for a rescue shell.\n");
> +  if (!grub_interruptible_millisleep (5000))
> +    {
> +      grub_printf ("Rebooting instead of going to rescue shell.\n");
> +      grub_reboot ();
> +    }
>    grub_boot_time ("Entering shell");
>    grub_cmdline_run (0);
>    nested_level--;
> --
> 1.8.4.1
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

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

Re: [PATCH v0] Require human interaction to go to normal shell if grub.cfg has a problem

[Andrey Borzenkov]

В Mon, 25 Nov 2013 15:46:48 -0800
Jon McCune <jonmccune@google.com> пишет:

> The rescue prompt is very useful for human operators, but not so
> useful in unattended environments.  Add a facility for rebooting
> after a delay, so that, e.g., the system can still PXE boot if
> there is no console attached.
> 
> Signed-off-by: Jon McCune <jonmccune@google.com>
> ---
>  grub-core/normal/main.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/grub-core/normal/main.c b/grub-core/normal/main.c

You get rescue prompt when you can *not* launch normal.mod. 

> index ad36273..f8953d5 100644
> --- a/grub-core/normal/main.c
> +++ b/grub-core/normal/main.c
> @@ -32,6 +32,7 @@
>  #include <grub/i18n.h>
>  #include <grub/charset.h>
>  #include <grub/script_sh.h>
> +#include <grub/time.h>
>  
>  GRUB_MOD_LICENSE ("GPLv3+");
>  
> @@ -333,6 +334,21 @@ grub_normal_execute (const char *config, int nested, int batch)
>      }
>  }
>  
> +/* Copied from grub-core/commands/sleep.c. */
> +static int
> +grub_interruptible_millisleep (grub_uint32_t ms)
> +{
> +  grub_uint64_t start;
> +
> +  start = grub_get_time_ms ();
> +
> +  while (grub_get_time_ms () - start < ms)
> +    if (grub_getkey_noblock () == GRUB_TERM_ESC)
> +      return 1;
> +
> +  return 0;
> +}
> +
>  /* This starts the normal mode.  */
>  void
>  grub_enter_normal_mode (const char *config)
> @@ -340,6 +356,15 @@ grub_enter_normal_mode (const char *config)
>    grub_boot_time ("Entering normal mode");
>    nested_level++;
>    grub_normal_execute (config, 0, 0);
> +  /* Control only returns from grub_normal_execute() if there is some kind of
> +   * problem with grub.cfg, like it does not exist.  Reboot by default unless
> +   * ESC is pressed within 5 seconds. */

grub-mkrescue creates CD that starts normal CLI without grub.cfg. So
please - not as default. You can make it dependent on variable which
can be set in embedded grub.cfg for the cases when you really need it.

> +  grub_printf ("Press ESC in 5 seconds for a rescue shell.\n");

"for a command line". This is not rescue shell.

> +  if (!grub_interruptible_millisleep (5000))
> +    {
> +      grub_printf ("Rebooting instead of going to rescue shell.\n");

Ditto.

> +      grub_reboot ();
> +    }
>    grub_boot_time ("Entering shell");
>    grub_cmdline_run (0);
>    nested_level--;

Re: [PATCH v0] Require human interaction to go to normal shell if grub.cfg has a problem

[Jonathan McCune]

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

Replying here instead of the install_c thread to try to keep it clean.  In
response to:

On Mon, Dec 2, 2013 at 11:18 AM, Andrey Borzenkov <arvidjaar@gmail.com>
 wrote:

> On Mon, Dec 2, 2013 at 11:09 PM, Jonathan McCune <jonmccune@google.com>
> wrote:
> > On Sat, Nov 9, 2013 at 4:51 AM, Vladimir 'φ-coder/phcoder' Serbinenko
> > <phcoder@gmail.com> wrote:
> >>
> >> Hello, all. install_c is feature complete and I intent to make it
> >> upstream unless issues are raised until 16 Nov. It's available under
> >> http://git.savannah.gnu.org/cgit/grub.git/log/?h=phcoder/install_c
> >
> >
> > I just realized that the option --grub-mkimage is now accepted silently
> > without having any effect in
> > util/grub-install-common.c:grub_install_parse(). This makes it no longer
> > feasible to easily wrap the mkimage command.  While most of the
> meaningful
> > reasons to wrap grub-mkimage have now been added to grub-install, I've
> run
> > into something I'm struggling to work around:  How do I add custom
> contents
> > to a load.cfg, since util/grub-install.c seems to unconditionally
> > grub_util_unlink(load_cfg)?
> >
>
> Could you provide examples of your grub.cfg usage? I think we need to
> have more real-life use cases to understand better how and when it is
> used.


The real life use case is a headless / unattended environment where it is
better to reboot than to present a console prompt if there is some kind of
GRUB problem, since the machine will just sit there until somebody or
something notices that it never booted up.

I was thinking that adding something in load.cfg (not grub.cfg) might be a
reasonable way to specify the desire to predispose GRUB towards rebooting
in the event of problems for use in headless/unattended environments.
 Whether that is true or not, I see now that the intention is for the
contents of load.cfg to be generated by the appropriate grub tools, and not
from arbitrary shell scripts. No problem.

I'm wondering what is the "right" way to enable rebooting behavior.  Flag
to grub-install? I also think this new functionality should have some kind
of delay and be cancelable at the console (current patch does this).

Thoughts?

Thanks,
-Jon



On Thu, Nov 28, 2013 at 9:34 AM, Andrey Borzenkov <arvidjaar@gmail.com>wrote:

> В Mon, 25 Nov 2013 15:46:48 -0800
> Jon McCune <jonmccune@google.com> пишет:
>
> > The rescue prompt is very useful for human operators, but not so
> > useful in unattended environments.  Add a facility for rebooting
> > after a delay, so that, e.g., the system can still PXE boot if
> > there is no console attached.
> >
> > Signed-off-by: Jon McCune <jonmccune@google.com>
> > ---
> >  grub-core/normal/main.c | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> >
> > diff --git a/grub-core/normal/main.c b/grub-core/normal/main.c
>
> You get rescue prompt when you can *not* launch normal.mod.
>
> > index ad36273..f8953d5 100644
> > --- a/grub-core/normal/main.c
> > +++ b/grub-core/normal/main.c
> > @@ -32,6 +32,7 @@
> >  #include <grub/i18n.h>
> >  #include <grub/charset.h>
> >  #include <grub/script_sh.h>
> > +#include <grub/time.h>
> >
> >  GRUB_MOD_LICENSE ("GPLv3+");
> >
> > @@ -333,6 +334,21 @@ grub_normal_execute (const char *config, int
> nested, int batch)
> >      }
> >  }
> >
> > +/* Copied from grub-core/commands/sleep.c. */
> > +static int
> > +grub_interruptible_millisleep (grub_uint32_t ms)
> > +{
> > +  grub_uint64_t start;
> > +
> > +  start = grub_get_time_ms ();
> > +
> > +  while (grub_get_time_ms () - start < ms)
> > +    if (grub_getkey_noblock () == GRUB_TERM_ESC)
> > +      return 1;
> > +
> > +  return 0;
> > +}
> > +
> >  /* This starts the normal mode.  */
> >  void
> >  grub_enter_normal_mode (const char *config)
> > @@ -340,6 +356,15 @@ grub_enter_normal_mode (const char *config)
> >    grub_boot_time ("Entering normal mode");
> >    nested_level++;
> >    grub_normal_execute (config, 0, 0);
> > +  /* Control only returns from grub_normal_execute() if there is some
> kind of
> > +   * problem with grub.cfg, like it does not exist.  Reboot by default
> unless
> > +   * ESC is pressed within 5 seconds. */
>
> grub-mkrescue creates CD that starts normal CLI without grub.cfg. So
> please - not as default. You can make it dependent on variable which
> can be set in embedded grub.cfg for the cases when you really need it.
>
> > +  grub_printf ("Press ESC in 5 seconds for a rescue shell.\n");
>
> "for a command line". This is not rescue shell.
>
> > +  if (!grub_interruptible_millisleep (5000))
> > +    {
> > +      grub_printf ("Rebooting instead of going to rescue shell.\n");
>
> Ditto.
>
> > +      grub_reboot ();
> > +    }
> >    grub_boot_time ("Entering shell");
> >    grub_cmdline_run (0);
> >    nested_level--;
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

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

Re: [PATCH v0] Require human interaction to go to normal shell if grub.cfg has a problem

[Andrey Borzenkov]

On Tue, Dec 3, 2013 at 12:16 AM, Jonathan McCune <jonmccune@google.com> wrote:
> Replying here instead of the install_c thread to try to keep it clean.  In
> response to:
>
> On Mon, Dec 2, 2013 at 11:18 AM, Andrey Borzenkov <arvidjaar@gmail.com>
> wrote:
>>
>> On Mon, Dec 2, 2013 at 11:09 PM, Jonathan McCune <jonmccune@google.com>
>> wrote:
>> > On Sat, Nov 9, 2013 at 4:51 AM, Vladimir 'φ-coder/phcoder' Serbinenko
>> > <phcoder@gmail.com> wrote:
>> >>
>> >> Hello, all. install_c is feature complete and I intent to make it
>> >> upstream unless issues are raised until 16 Nov. It's available under
>> >> http://git.savannah.gnu.org/cgit/grub.git/log/?h=phcoder/install_c
>> >
>> >
>> > I just realized that the option --grub-mkimage is now accepted silently
>> > without having any effect in
>> > util/grub-install-common.c:grub_install_parse(). This makes it no longer
>> > feasible to easily wrap the mkimage command.  While most of the
>> > meaningful
>> > reasons to wrap grub-mkimage have now been added to grub-install, I've
>> > run
>> > into something I'm struggling to work around:  How do I add custom
>> > contents
>> > to a load.cfg, since util/grub-install.c seems to unconditionally
>> > grub_util_unlink(load_cfg)?
>> >
>>
>> Could you provide examples of your grub.cfg usage? I think we need to
>> have more real-life use cases to understand better how and when it is
>> used.
>
>
> The real life use case is a headless / unattended environment where it is
> better to reboot than to present a console prompt if there is some kind of
> GRUB problem, since the machine will just sit there until somebody or
> something notices that it never booted up.
>

I still do not quite understand how rebooting can fix the problem. The
only case it may happen is when you have intermittent network issues
where grub fails to read some file.

I have a feeling that you attempt to paper over some problem outside of grub.

Re: [PATCH v0] Require human interaction to go to normal shell if grub.cfg has a problem

[Jonathan McCune]

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

On Mon, Dec 2, 2013 at 12:38 PM, Andrey Borzenkov <arvidjaar@gmail.com>wrote:

>
> I still do not quite understand how rebooting can fix the problem. The
> only case it may happen is when you have intermittent network issues
> where grub fails to read some file.
>

Ah, rebooting allows a machine to network boot, e.g., PXE boot using its
NIC.


> I have a feeling that you attempt to paper over some problem outside of
> grub.
>

This is somewhat true, in that grub's own commands should not get the
machine into a state where this functionality is useful.  But furthering
the real life example, users / administrators might make a mistake and
create a broken config.  If the machine is unattended, it seems reasonable
that the user might prefer for it to reboot.  Otherwise, it becomes
necessary to somehow cause a reboot out-of-band.  These out-of-band
solutions are generally proprietary and I think it's a good idea to have
support for avoiding them if desired.

Thanks,
-Jon

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

Re: [PATCH v0] Require human interaction to go to normal shell if grub.cfg has a problem

[Andrey Borzenkov]

В Mon, 2 Dec 2013 12:49:03 -0800
Jonathan McCune <jonmccune@google.com> пишет:

> On Mon, Dec 2, 2013 at 12:38 PM, Andrey Borzenkov <arvidjaar@gmail.com>wrote:
> 
> >
> > I still do not quite understand how rebooting can fix the problem. The
> > only case it may happen is when you have intermittent network issues
> > where grub fails to read some file.
> >
> 
> Ah, rebooting allows a machine to network boot, e.g., PXE boot using its
> NIC.
> 

Hmm ... how does it work? After reboot you use the same default boot
order and end up in the same non-working grub, right? May be you want
to not reboot but simply exit grub letting firmware to continue with
next boot choice; I'm not sure whether this works reliably in case of
BIOS.

And I still do not really see how useful it is. Booting from network
will presumably land you into some sort of remote installation
environment. How does it help? How do know it landed there in the first
place?

> 
> > I have a feeling that you attempt to paper over some problem outside of
> > grub.
> >
> 
> This is somewhat true, in that grub's own commands should not get the
> machine into a state where this functionality is useful.  But furthering
> the real life example, users / administrators might make a mistake and
> create a broken config.  If the machine is unattended, it seems reasonable
> that the user might prefer for it to reboot.  Otherwise, it becomes
> necessary to somehow cause a reboot out-of-band.  These out-of-band
> solutions are generally proprietary and I think it's a good idea to have
> support for avoiding them if desired.
> 

Well, I spent last 10+ years doing remote maintenance and I know
pretty well, that if you do not have remote access to console and
possibility to remotely trigger reboot, you will get in trouble in any
case. There are much more situations that require it and they are far
more probable than grub misconfiguration.

Re: [PATCH v0] Require human interaction to go to normal shell if grub.cfg has a problem

[Jonathan McCune]

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

Inline...

On Sat, Dec 7, 2013 at 4:35 AM, Andrey Borzenkov <arvidjaar@gmail.com>wrote:

> В Mon, 2 Dec 2013 12:49:03 -0800
> Jonathan McCune <jonmccune@google.com> пишет:
>
> > On Mon, Dec 2, 2013 at 12:38 PM, Andrey Borzenkov <arvidjaar@gmail.com
> >wrote:
> >
> > >
> > > I still do not quite understand how rebooting can fix the problem. The
> > > only case it may happen is when you have intermittent network issues
> > > where grub fails to read some file.
> > >
> >
> > Ah, rebooting allows a machine to network boot, e.g., PXE boot using its
> > NIC.
> >
>
> Hmm ... how does it work? After reboot you use the same default boot
> order and end up in the same non-working grub, right? May be you want
> to not reboot but simply exit grub letting firmware to continue with
> next boot choice; I'm not sure whether this works reliably in case of
> BIOS.
>

Good point.  This is another interesting option.  I'm not sure how
generally reliable it is for BIOS systems either, but it seems like a case
worth considering.


> And I still do not really see how useful it is. Booting from network
> will presumably land you into some sort of remote installation
> environment. How does it help? How do know it landed there in the first
> place?
>

I agree that a naive always-netboot-to-reinstall is not a useful
configuration, but DHCP servers can be easily configured to enable or
disable netboot for individual machines.  Sometimes this can be a nice
control point for diagnosing or reconfiguring machines.


> > > I have a feeling that you attempt to paper over some problem outside of
> > > grub.
> > >
> >
> > This is somewhat true, in that grub's own commands should not get the
> > machine into a state where this functionality is useful.  But furthering
> > the real life example, users / administrators might make a mistake and
> > create a broken config.  If the machine is unattended, it seems
> reasonable
> > that the user might prefer for it to reboot.  Otherwise, it becomes
> > necessary to somehow cause a reboot out-of-band.  These out-of-band
> > solutions are generally proprietary and I think it's a good idea to have
> > support for avoiding them if desired.
> >
>
> Well, I spent last 10+ years doing remote maintenance and I know
> pretty well, that if you do not have remote access to console and
> possibility to remotely trigger reboot, you will get in trouble in any
> case. There are much more situations that require it and they are far
> more probable than grub misconfiguration.
>

I concede that some problems inevitably require manual intervention, but
the probabilities of different misconfigurations seem likely to be
deployment-specific.   Remote console / serial concentrators / remote
reboot / humans to press buttons  / etc are neither instantaneous nor free.
 Anyways, I will try my hand at another iteration on this patch.

-Jon

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

Re: [PATCH v0] Require human interaction to go to normal shell if grub.cfg has a problem

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 09.12.2013 16:25, Jonathan McCune wrote:
> Good point.  This is another interesting option.  I'm not sure how
> generally reliable it is for BIOS systems either, but it seems like a
> case worth considering.
The call has been around for a long time. I think originally it launched
BASIC in ROM.
It hasn't been used by microsoft so may not be reliable.
On the other hand it's used by option roms to indicate to proceed to
next boot option so I expect it to work as long as next boot option is
defined.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 291 bytes --]