(General note: please try to look at how the existing code is formatted
and follow its style; this file uses a two-space indent with GNU brace
style and tab-for-every-eight-spaces, and there are a few other details
to match as well.  When you use a different style for parts of your
changes, it makes the changes very difficult to read.  The third chunk
of this patch in particular is hard work because the indentation has
ended up reversed.)

I think it's very wrong to force a delay in normal/menu.c like this.
Firstly, it's a hardcoded unconfigurable magic number.  Secondly, if you
asked for no timeout in your GRUB configuration, you should get no
timeout, even at the cost of hotkeys; for example, we have had
commercial projects with extremely tight boot deadlines that would be
sensitive to a 100ms delay.  So, while I do see where you're going, and
it would probably work as a temporary patch in a commercial project, I'm
afraid I would not approve this approach upstream or in Ubuntu.

My recommendations would be as follows.  I don't promise that my
colleagues upstream will feel exactly the same way, but I think this has
a much better chance of being acceptable.

 * This configuration should require a non-zero GRUB_HIDDEN_TIMEOUT, in
order that firmware key repeat has had a chance to kick in and something
in GRUB has had a chance to see the keyboard state.

 * The terminal layer should be modified so that it's possible to save a
keystroke somewhere after reading it.  This would permit a keystroke to
interrupt a sleep and then also be consumed by subsequent menu interface
code.  While doing this, be careful that the terminal layer is in the
space-constrained kernel, where every byte has a cost; it would, I
think, be reasonable to allow saving only a single keystroke rather than
maintaining any more sophisticated structure.

 * There will need to be some interface change to the sleep command.
Perhaps any keystroke should interrupt it, rather than just Escape and
(in an Ubuntu-specific patch) Shift.  It should then save/unget it using
the interface above, so that the menu interface can get at it.

 * The menu code should then look at the saved keystroke and decide
whether it's a hotkey, and if so set auto-boot, somewhat as your current
patch does.  (Here's where it matters that the terminal operation is an
out-of-band save rather than pushing the keystroke back into the input
buffer: you don't want pressing Escape to interrupt the hidden timeout
and then *also* cancel the menu.)

** Changed in: grub2 (Ubuntu)
       Status: New => Triaged

** Changed in: grub2 (Ubuntu)
   Importance: Undecided => Medium

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/1178618

Title:
  --hotkey option does not work when the menu is hidden

To manage notifications about this bug go to:
https://bugs.launchpad.net/grub/+bug/1178618/+subscriptions

-- 
ubuntu-bugs mailing list
[email protected]
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

Reply via email to