Specifying "search --label" instead of "search --file" in grub

Specifying "search --label" instead of "search --file" in grub

[Christopher Allan Webber]

Heya all,

Today while installing GuixSD I hit a really painful and hard to track
down bug.  It can happen when you've got multiple Guix installs which
both have the same kernel.

  # Set 'root' to the partition that contains the kernel.
  search --file --set ~a/~a~%

Since this searches across multiple devices for something that matches
that file (probably not the most efficient route if the label is known
anyway?) it can break things if multiple filesystems contain the same
kernel file.

In my case, I had the usb key with the Guix install plugged in, and I
had just installed Guix to disk.  I had seemingly messed up my install,
so my system wasn't booting, but then I discovered I couldn't even boot
to the USB key either.

Well, the USB key had the same kernel as my borked install.... so... I
couldn't boot my system!

After nearly an hour of frustration I hit the revelation that this was
probably the case, hit "e" in the grub menu, switched switch from --file
to --label, and I got things working.

I wonder if this might bite anyone else?  I paniced because I seemed
unable to boot into either and thought I had totally borked my fancy
~new machine.  I'd hate for anyone else to go through the same thing!

 - Chris

Re: Specifying "search --label" instead of "search --file" in grub

[Ludovic Courtès]

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

(I’ve opened a bug for this, but debbugs.gnu.org is currently down…)

Christopher Allan Webber <cwebber@dustycloud.org> skribis:

> Today while installing GuixSD I hit a really painful and hard to track
> down bug.  It can happen when you've got multiple Guix installs which
> both have the same kernel.
>
>   # Set 'root' to the partition that contains the kernel.
>   search --file --set ~a/~a~%
>
> Since this searches across multiple devices for something that matches
> that file (probably not the most efficient route if the label is known
> anyway?) it can break things if multiple filesystems contain the same
> kernel file.

I agree that the current approach suboptimal in several ways!

> In my case, I had the usb key with the Guix install plugged in, and I
> had just installed Guix to disk.  I had seemingly messed up my install,
> so my system wasn't booting, but then I discovered I couldn't even boot
> to the USB key either.

Initially I thought that the scenario you were describing (several
partitions containing the same store items) was unusual, but now I
realize that it’s not; leaving the USB key plugged in is a simple way to
reproduce it, indeed.

So, based on your suggestions, I came up with the following patch.  I
haven’t yet tested it on the bare metal, but I’ve confirmed that it
produces sensible grub.cfg files when the root FS has a UUID, label, or
none of these.

Could you give it a try?

In the case of a label, the problem is that, if the user changes the
partitions label at some point in time, then the partition won’t be
found on the next reboot.

I wonder if we should ignore the issue, or if we should have a fallback
case.  For instance, does this work in GRUB?

  if ! search --fs-uuid --set x-y-z
    search --file --set /gnu/store/…-foo/bar
  fi

If it does, we can probably do that.

WDYT?

Thanks a lot for investigating!

Ludo’.


[-- Attachment #2: Type: text/x-patch, Size: 7366 bytes --]

diff --git a/gnu/system.scm b/gnu/system.scm
index 6dfcc0f..7443e57 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -79,6 +79,7 @@
             operating-system-locale-libcs
             operating-system-mapped-devices
             operating-system-file-systems
+            operating-system-store-file-system
             operating-system-activation-script
 
             operating-system-derivation
@@ -666,12 +667,34 @@ listed in OS.  The C library expects to find it under
                  (package-version kernel)
                  " (alpha)"))
 
+(define (store-file-system file-systems)
+  "Return the file system object among FILE-SYSTEMS that contains the store."
+  (match (filter (lambda (fs)
+                   (and (file-system-mount? fs)
+                        (not (memq 'bind-mount (file-system-flags fs)))
+                        (string-prefix? (file-system-mount-point fs)
+                                        (%store-prefix))))
+                 file-systems)
+    ((and candidates (head . tail))
+     (reduce (lambda (fs1 fs2)
+               (if (> (string-length (file-system-mount-point fs1))
+                      (string-length (file-system-mount-point fs2)))
+                   fs1
+                   fs2))
+             head
+             candidates))))
+
+(define (operating-system-store-file-system os)
+  "Return the file system that contains the store of OS."
+  (store-file-system (operating-system-file-systems os)))
+
 (define* (operating-system-grub.cfg os #:optional (old-entries '()))
   "Return the GRUB configuration file for OS.  Use OLD-ENTRIES to populate the
 \"old entries\" menu."
   (mlet* %store-monad
       ((system      (operating-system-derivation os))
        (root-fs ->  (operating-system-root-file-system os))
+       (store-fs -> (operating-system-store-file-system os))
        (kernel ->   (operating-system-kernel os))
        (root-device -> (if (eq? 'uuid (file-system-title root-fs))
                            (uuid->string (file-system-device root-fs))
@@ -686,7 +709,8 @@ listed in OS.  The C library expects to find it under
                                                     "/boot")
                                    (operating-system-kernel-arguments os)))
                            (initrd #~(string-append #$system "/initrd"))))))
-    (grub-configuration-file (operating-system-bootloader os) entries
+    (grub-configuration-file (operating-system-bootloader os)
+                             store-fs entries
                              #:old-entries old-entries)))
 
 (define (operating-system-parameters-file os)
diff --git a/gnu/system/grub.scm b/gnu/system/grub.scm
index 5b82482..b8b9d3c 100644
--- a/gnu/system/grub.scm
+++ b/gnu/system/grub.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2013, 2014, 2015 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2013, 2014, 2015, 2016 Ludovic Courtès <ludo@gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -25,6 +25,7 @@
   #:use-module (guix gexp)
   #:use-module (guix download)
   #:use-module (gnu artwork)
+  #:use-module (gnu system file-systems)
   #:autoload   (gnu packages grub) (grub)
   #:autoload   (gnu packages inkscape) (inkscape)
   #:autoload   (gnu packages imagemagick) (imagemagick)
@@ -153,7 +154,7 @@ WIDTH/HEIGHT, or #f if none was found."
         (with-monad %store-monad
           (return #f)))))
 
-(define (eye-candy config system port)
+(define (eye-candy config root-fs system port)
   "Return in %STORE-MONAD a gexp that writes to PORT (a port-valued gexp) the
 'grub.cfg' part concerned with graphics mode, background images, colors, and
 all that."
@@ -179,15 +180,18 @@ all that."
       (string-append (symbol->string (assoc-ref colors 'fg)) "/"
                      (symbol->string (assoc-ref colors 'bg)))))
 
+  (define font-file
+    #~(string-append #$grub "/share/grub/unicode.pf2"))
+
   (mlet* %store-monad ((image (grub-background-image config)))
     (return (and image
                  #~(format #$port "
 function setup_gfxterm {~a}
 
 # Set 'root' to the partition that contains /gnu/store.
-search --file --set ~a/share/grub/unicode.pf2
+~a
 
-if loadfont ~a/share/grub/unicode.pf2; then
+if loadfont ~a; then
   setup_gfxterm
 fi
 
@@ -200,7 +204,9 @@ else
   set menu_color_highlight=white/blue
 fi~%"
                            #$setup-gfxterm-body
-                           #$grub #$grub
+                           #$(grub-root-search root-fs font-file)
+                           #$font-file
+
                            #$image
                            #$(theme-colors grub-theme-color-normal)
                            #$(theme-colors grub-theme-color-highlight))))))
@@ -210,13 +216,31 @@ fi~%"
 ;;; Configuration file.
 ;;;
 
-(define* (grub-configuration-file config entries
+(define (grub-root-search root-fs file)
+  "Return the GRUB 'search' command to look for ROOT-FS, which contains FILE,
+a gexp.  The result is a gexp that can be inserted in the grub.cfg-generation
+code."
+  (case (file-system-title root-fs)
+    ;; Preferably refer to ROOT-FS by its UUID or label.  This is more
+    ;; efficient and less ambiguous, see <>.
+    ((uuid)
+     (format #f "search --fs-uuid --set ~a"
+             (uuid->string (file-system-device root-fs))))
+    ((label)
+     (format #f "search --label --set ~a"
+             (file-system-device root-fs)))
+    (else
+     ;; As a last resort, look for any device containing FILE.
+     #~(format #f "search --file --set ~a" #$file))))
+
+(define* (grub-configuration-file config store-fs entries
                                   #:key
                                   (system (%current-system))
                                   (old-entries '()))
   "Return the GRUB configuration file corresponding to CONFIG, a
-<grub-configuration> object.  OLD-ENTRIES is taken to be a list of menu
-entries corresponding to old generations of the system."
+<grub-configuration> object, and where the store is available at STORE-FS, a
+<file-system> object.  OLD-ENTRIES is taken to be a list of menu entries
+corresponding to old generations of the system."
   (define linux-image-name
     (if (string-prefix? "mips" system)
         "vmlinuz"
@@ -229,18 +253,18 @@ entries corresponding to old generations of the system."
     (match-lambda
      (($ <menu-entry> label linux arguments initrd)
       #~(format port "menuentry ~s {
-  # Set 'root' to the partition that contains the kernel.
-  search --file --set ~a/~a~%
-
+  ~a
   linux ~a/~a ~a
   initrd ~a
 }~%"
                 #$label
-                #$linux #$linux-image-name
+                #$(grub-root-search store-fs
+                                    #~(string-append #$linux "/"
+                                                     #$linux-image-name))
                 #$linux #$linux-image-name (string-join (list #$@arguments))
                 #$initrd))))
 
-  (mlet %store-monad ((sugar (eye-candy config system #~port)))
+  (mlet %store-monad ((sugar (eye-candy config store-fs system #~port)))
     (define builder
       #~(call-with-output-file #$output
           (lambda (port)

Re: Specifying "search --label" instead of "search --file" in grub

[Ludovic Courtès]

ludo@gnu.org (Ludovic Courtès) skribis:

> So, based on your suggestions, I came up with the following patch.  I
> haven’t yet tested it on the bare metal, but I’ve confirmed that it
> produces sensible grub.cfg files when the root FS has a UUID, label, or
> none of these.

I confirm it works on the bare metal.  My root file system is identified
with a partition label, as in the OS config examples, so I get:

  search --label --set my-root-partition-label

Ludo’.