* Re: Specifying "search --label" instead of "search --file" in grub
2015-12-20 23:49 Specifying "search --label" instead of "search --file" in grub Christopher Allan Webber
@ 2016-01-01 22:12 ` Ludovic Courtès
2016-01-01 23:29 ` Ludovic Courtès
0 siblings, 1 reply; 3+ messages in thread
From: Ludovic Courtès @ 2016-01-01 22:12 UTC (permalink / raw)
To: Christopher Allan Webber; +Cc: guix-devel
[-- 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)
^ permalink raw reply related [flat|nested] 3+ messages in thread