GNU bug report logs - #24346
GuixSD: grub.cfg does not support separate /gnu partition

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 24346 in the body.
You can then email your comments to 24346 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: bug-guix <at> gnu.org
Subject: GuixSD: grub.cfg does not support separate /gnu partition
Date: Wed, 31 Aug 2016 23:17:50 +0200

See details at
<https://lists.gnu.org/archive/html/guix-devel/2016-08/msg02082.html>.

Ludo’.

Message #8 received at 24346 <at> debbugs.gnu.org (full text, mbox):

From: csanchezdll <at> gmail.com (Carlos Sánchez de La Lama)
To: 24346 <at> debbugs.gnu.org
Cc: guix-devel <at> gnu.org
Subject: [PATCH] Generate grub.cfg with correct paths when store is not in
 root partition
Date: Tue, 13 Sep 2016 13:23:31 +0200

I had sent it to the wrong bug#, sorry about the noise.

Took a while to produce a working patch :S

A little explanation of what it does:

guix/scripts/system.scm: when constructing old entries, we know
"systems" are the system symlinks in /var/guix/profile, and not a store
derivation. No need for gexps then, which is good as resolving the
symlink using "readlink" cannot be done by the builders /var is not in
the chroot).

gnu/system.scm: for the current system, initrd is inside the system
derivattion in the store. Resolve the symlink at build time.

gnu/system/grub.scm: strip the mount point on the paths GRUB will use,
so GRUB modules, background images. kernels and initrds are correcty
loaded.

This is the first time I do something non-trivial in scheme, so there
might be an easier way to achieve the same result. Nonetheless, I have
tested it on my system and seems to work ok.

* guix/scripts/system.scm (previous-grub-entries): resolve initrd
  symlink for old entries (on the host).
* gnu/system.scm (operating-system-grub.cfg): resolve initrd symlink for
  current system (on the container).
* gnu/system/grub.scm: strip mount-point from GRUB filenames.

---
 gnu/system.scm          |  2 +-
 gnu/system/grub.scm     | 29 ++++++++++++++++++++---------
 guix/scripts/system.scm |  6 +++---
 3 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index 0802010..1346f49 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -723,7 +723,7 @@ listed in OS.  The C library expects to find it under
                                    #~(string-append "--load=" #$system
                                                     "/boot")
                                    (operating-system-kernel-arguments os)))
-                           (initrd #~(string-append #$system "/initrd"))))))
+                           (initrd #~(readlink (string-append #$system "/initrd")))))))
     (grub-configuration-file (operating-system-bootloader os)
                              store-fs entries
                              #:old-entries old-entries)))
diff --git a/gnu/system/grub.scm b/gnu/system/grub.scm
index 45b46ca..56cb081 100644
--- a/gnu/system/grub.scm
+++ b/gnu/system/grub.scm
@@ -62,6 +62,14 @@
 ;;;
 ;;; Code:
 
+(define (strip-mount-point fs file)
+  (let ((mount-point (file-system-mount-point fs)))
+    (if (string=? mount-point "/")
+	file
+	#~(if (string-prefix? #$mount-point #$file)
+	      (substring #$file (string-length #$mount-point))
+	      #$file))))
+
 (define-record-type* <grub-image>
   grub-image make-grub-image
   grub-image?
@@ -183,7 +191,8 @@ the store is.  SYSTEM must be the target system string---e.g.,
                      (symbol->string (assoc-ref colors 'bg)))))
 
   (define font-file
-    #~(string-append #$grub "/share/grub/unicode.pf2"))
+    (strip-mount-point root-fs
+		       #~(string-append #$grub "/share/grub/unicode.pf2")))
 
   (mlet* %store-monad ((image (grub-background-image config)))
     (return (and image
@@ -209,7 +218,7 @@ fi~%"
                            #$(grub-root-search root-fs font-file)
                            #$font-file
 
-                           #$image
+                           #$(strip-mount-point root-fs image)
                            #$(theme-colors grub-theme-color-normal)
                            #$(theme-colors grub-theme-color-highlight))))))
 
@@ -254,17 +263,19 @@ corresponding to old generations of the system."
   (define entry->gexp
     (match-lambda
      (($ <menu-entry> label linux arguments initrd)
-      #~(format port "menuentry ~s {
+      (let ((linux (strip-mount-point store-fs linux))
+            (initrd (strip-mount-point store-fs initrd)))
+        #~(format port "menuentry ~s {
   ~a
   linux ~a/~a ~a
   initrd ~a
 }~%"
-                #$label
-                #$(grub-root-search store-fs
-                                    #~(string-append #$linux "/"
-                                                     #$linux-image-name))
-                #$linux #$linux-image-name (string-join (list #$@arguments))
-                #$initrd))))
+                  #$label
+                  #$(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 store-fs system #~port)))
     (define builder
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 953c624..5ff98a0 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -391,10 +391,10 @@ it atomically, and then run OS's activation script."
         (linux kernel)
         (linux-arguments
          (cons* (string-append "--root=" root-device)
-                #~(string-append "--system=" #$system)
-                #~(string-append "--load=" #$system "/boot")
+                (string-append "--system=" system)
+                (string-append "--load=" system "/boot")
                 kernel-arguments))
-        (initrd #~(string-append #$system "/initrd"))))))
+        (initrd (readlink (string-append system "/initrd")))))))
 
   (let* ((numbers (generation-numbers profile))
          (systems (map (cut generation-file-name profile <>)
-- 

Message #11 received at 24346 <at> debbugs.gnu.org (full text, mbox):

From: csanchezdll <at> gmail.com (Carlos Sánchez de La Lama)
To: 24346 <at> debbugs.gnu.org
Subject: Re: [PATCH] Generate grub.cfg with correct paths when store is not in
 root partition
Date: Wed, 14 Sep 2016 16:13:24 +0200

Rebasing to current master (76f31f0b), as new file-append made merge
non-trivial.

* guix/scripts/system.scm (previous-grub-entries): resolve initrd
  symlink for old entries (on the host).
* gnu/system.scm (operating-system-grub.cfg): resolve initrd symlink for
  current system (on the container).
* gnu/system/grub.scm: strip mount-point from GRUB filenames.

---
 gnu/system.scm          |  2 +-
 gnu/system/grub.scm     | 25 ++++++++++++++++++-------
 guix/scripts/system.scm |  6 +++---
 3 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index 7edb018..281447f 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -738,7 +738,7 @@ listed in OS.  The C library expects to find it under
                                    #~(string-append "--load=" #$system
                                                     "/boot")
                                    (operating-system-kernel-arguments os)))
-                           (initrd (file-append system "/initrd"))))))
+                           (initrd #~(readlink #$(file-append system "/initrd")))))))
     (grub-configuration-file (operating-system-bootloader os)
                              store-fs entries
                              #:old-entries old-entries)))
diff --git a/gnu/system/grub.scm b/gnu/system/grub.scm
index 4592747..a00f568 100644
--- a/gnu/system/grub.scm
+++ b/gnu/system/grub.scm
@@ -62,6 +62,14 @@
 ;;;
 ;;; Code:
 
+(define (strip-mount-point fs file)
+  (let ((mount-point (file-system-mount-point fs)))
+    (if (string=? mount-point "/")
+	file
+	#~(if (string-prefix? #$mount-point #$file)
+	      (substring #$file (string-length #$mount-point))
+	      #$file))))
+
 (define-record-type* <grub-image>
   grub-image make-grub-image
   grub-image?
@@ -183,7 +191,8 @@ the store is.  SYSTEM must be the target system string---e.g.,
                      (symbol->string (assoc-ref colors 'bg)))))
 
   (define font-file
-    #~(string-append #$grub "/share/grub/unicode.pf2"))
+    (strip-mount-point root-fs
+		       #~(string-append #$grub "/share/grub/unicode.pf2")))
 
   (mlet* %store-monad ((image (grub-background-image config)))
     (return (and image
@@ -209,7 +218,7 @@ fi~%"
                            #$(grub-root-search root-fs font-file)
                            #$font-file
 
-                           #$image
+                           #$(strip-mount-point root-fs image)
                            #$(theme-colors grub-theme-color-normal)
                            #$(theme-colors grub-theme-color-highlight))))))
 
@@ -249,15 +258,17 @@ corresponding to old generations of the system."
   (define entry->gexp
     (match-lambda
      (($ <menu-entry> label linux arguments initrd)
-      #~(format port "menuentry ~s {
+      (let ((linux (strip-mount-point store-fs linux))
+            (initrd (strip-mount-point store-fs initrd)))
+        #~(format port "menuentry ~s {
   ~a
   linux ~a ~a
   initrd ~a
 }~%"
-                #$label
-                #$(grub-root-search store-fs linux)
-                #$linux (string-join (list #$@arguments))
-                #$initrd))))
+                  #$label
+                  #$(grub-root-search store-fs linux)
+                  #$linux (string-join (list #$@arguments))
+                  #$initrd)))))
 
   (mlet %store-monad ((sugar (eye-candy config store-fs system #~port)))
     (define builder
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 953c624..5ff98a0 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -391,10 +391,10 @@ it atomically, and then run OS's activation script."
         (linux kernel)
         (linux-arguments
          (cons* (string-append "--root=" root-device)
-                #~(string-append "--system=" #$system)
-                #~(string-append "--load=" #$system "/boot")
+                (string-append "--system=" system)
+                (string-append "--load=" system "/boot")
                 kernel-arguments))
-        (initrd #~(string-append #$system "/initrd"))))))
+        (initrd (readlink (string-append system "/initrd")))))))
 
   (let* ((numbers (generation-numbers profile))
          (systems (map (cut generation-file-name profile <>)
-- 
2.9.2

Message #16 received at 24346 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: csanchezdll <at> gmail.com (Carlos Sánchez de La Lama)
Cc: 24346 <at> debbugs.gnu.org
Subject: Re: bug#24346: [PATCH] Generate grub.cfg with correct paths when
 store is not in root partition
Date: Sat, 24 Sep 2016 17:53:32 +0900

Hi Carlos!

csanchezdll <at> gmail.com (Carlos Sánchez de La Lama) skribis:

> Rebasing to current master (76f31f0b), as new file-append made merge
> non-trivial.
>
> * guix/scripts/system.scm (previous-grub-entries): resolve initrd
>   symlink for old entries (on the host).
> * gnu/system.scm (operating-system-grub.cfg): resolve initrd symlink for
>   current system (on the container).
> * gnu/system/grub.scm: strip mount-point from GRUB filenames.

I ended up pushing a slightly modified version of this patch as
0f65f54ebd76324653fd5506a7dab42ee44d9255.

Essentially, I was dissatisfied with those ‘readlink’ calls, which do
I/O and could always fail, so I changed them to use the procedures we
have that return the store file name of the initrd.

> +(define (strip-mount-point fs file)

I added a docstring here.

> +  (let ((mount-point (file-system-mount-point fs)))
> +    (if (string=? mount-point "/")
> +	file
> +	#~(if (string-prefix? #$mount-point #$file)
> +	      (substring #$file (string-length #$mount-point))
> +	      #$file))))

I introduced a ‘file’ variable in the staged code, to avoid having that
(string-append …) expression several times in the output.

I think that’s it!

I verified the grub.cfg that ‘guix system reconfigure’ generates and it
seems to work fine.  If you can confirm that it still works for you,
please say so to 24346-done <at> debbugs.gnu.org.  :-)

Thanks a lot for addressing this bug!

Ludo’.

Message #19 received at 24346 <at> debbugs.gnu.org (full text, mbox):

From: csanchezdll <at> gmail.com (Carlos Sánchez de La Lama)
To: ludo <at> gnu.org (Ludovic Courtès)
Cc: 24346 <at> debbugs.gnu.org
Subject: Re: bug#24346: [PATCH] Generate grub.cfg with correct paths when
 store is not in root partition
Date: Mon, 26 Sep 2016 13:08:10 +0200

Hi Ludo,

> I ended up pushing a slightly modified version of this patch as
> 0f65f54ebd76324653fd5506a7dab42ee44d9255.

Great! Your version looks definitely better :-)

> I verified the grub.cfg that ‘guix system reconfigure’ generates and it
> seems to work fine.  If you can confirm that it still works for you,
> please say so to 24346-done <at> debbugs.gnu.org.  :-)

Works on my system which has store on a separate partition. I have
checked grub module and image gets correctly found as well as kernel and
initrd.

Thanks!

Carlos

Message #24 received at 24346-done <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: csanchezdll <at> gmail.com (Carlos Sánchez de La Lama)
Cc: 24346-done <at> debbugs.gnu.org
Subject: Re: bug#24346: [PATCH] Generate grub.cfg with correct paths when
 store is not in root partition
Date: Mon, 26 Sep 2016 17:53:54 +0200

csanchezdll <at> gmail.com (Carlos Sánchez de La Lama) skribis:

>> I ended up pushing a slightly modified version of this patch as
>> 0f65f54ebd76324653fd5506a7dab42ee44d9255.
>
> Great! Your version looks definitely better :-)
>
>> I verified the grub.cfg that ‘guix system reconfigure’ generates and it
>> seems to work fine.  If you can confirm that it still works for you,
>> please say so to 24346-done <at> debbugs.gnu.org.  :-)
>
> Works on my system which has store on a separate partition. I have
> checked grub module and image gets correctly found as well as kernel and
> initrd.

Great!

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.