[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Stefan]

* gnu/bootloader/grub.scm (grub-efi-net-bootloader): New efi bootloader for
network booting via tftp/nfs and possibly images, prepared for chain loading.
(install-grub-efi-net): New bootloader installer for tftp and possibly images,
does not need root rights.
(grub-root-search): Adding support for tftp root.
(eye-candy): Enable gfxterm support for all systems.
* gnu/system.scm (read-boot-parameters): Prevent devices with ":/" from being
treated as a file system label.
---
 gnu/bootloader/grub.scm | 107 +++++++++++++++++++++++++++++++---------
 gnu/system.scm          |   3 +-
 2 files changed, 86 insertions(+), 24 deletions(-)

diff --git a/gnu/bootloader/grub.scm b/gnu/bootloader/grub.scm
index 190b717163..9ca4f016f6 100644
--- a/gnu/bootloader/grub.scm
+++ b/gnu/bootloader/grub.scm
@@ -23,7 +23,7 @@
 
 (define-module (gnu bootloader grub)
   #:use-module (guix records)
-  #:use-module ((guix utils) #:select (%current-system))
+  #:use-module ((guix utils) #:select (%current-system %current-target-system))
   #:use-module (guix gexp)
   #:use-module (gnu artwork)
   #:use-module (gnu bootloader)
@@ -53,6 +53,7 @@
 
             grub-bootloader
             grub-efi-bootloader
+            grub-efi-net-bootloader
             grub-mkrescue-bootloader
 
             grub-configuration))
@@ -142,34 +143,20 @@ WIDTH/HEIGHT, or #f if none was found."
                    #:width width #:height height))))
 
 (define* (eye-candy config store-device store-mount-point
-                    #:key system port)
+                    #:key port)
   "Return a gexp that writes to PORT (a port-valued gexp) the
 'grub.cfg' part concerned with graphics mode, background images, colors, and
 all that.  STORE-DEVICE designates the device holding the store, and
 STORE-MOUNT-POINT is its mount point; these are used to determine where the
-background image and fonts must be searched for.  SYSTEM must be the target
-system string---e.g., \"x86_64-linux\"."
+background image and fonts must be searched for."
   (define setup-gfxterm-body
-    (let ((gfxmode
-           (or (and-let* ((theme (bootloader-configuration-theme config))
-                          (gfxmode (grub-gfxmode theme)))
-                 (string-join gfxmode ";"))
-               "auto")))
-
-      ;; Intel and EFI systems need to be switched into graphics mode, whereas
-      ;; most other modern architectures have no other mode and therefore
-      ;; don't need to be switched.
-
-      ;; XXX: Do we really need to restrict to x86 systems?  We could imitate
-      ;; what the GRUB default configuration does and decide based on whether
-      ;; a user provided 'gfxterm' in the terminal-outputs field of their
-      ;; bootloader-configuration record.
-      (if (string-match "^(x86_64|i[3-6]86)-" system)
-          (format #f "
+    (format #f "
   set gfxmode=~a
   insmod all_video
-  insmod gfxterm~%" gfxmode)
-          "")))
+  insmod gfxterm~%"
+            (string-join
+             (grub-gfxmode (bootloader-theme config))
+             ";")))
 
   (define (setup-gfxterm config font-file)
     (if (memq 'gfxterm (bootloader-configuration-terminal-outputs config))
@@ -316,6 +303,9 @@ code."
         ((? file-system-label? label)
          (format #f "search --label --set ~a"
                  (file-system-label->string label)))
+        ((? (lambda (device)
+              (and (string? device) (string-contains device ":/"))) nfs-uri)
+         "set root=(tftp)")
         ((or #f (? string?))
          #~(format #f "search --file --set ~a" #$file)))))
 
@@ -355,7 +345,6 @@ entries corresponding to old generations of the system."
     (eye-candy config
                (menu-entry-device (first all-entries))
                (menu-entry-device-mount-point (first all-entries))
-               #:system system
                #:port #~port))
 
   (define keyboard-layout-config
@@ -443,6 +432,68 @@ fi~%"))))
                       "--bootloader-id=Guix"
                       "--efi-directory" target-esp))))
 
+(define (install-grub-efi-net efi-subdir)
+  "Define a grub-efi bootloader installer for installation in EFI-SUBDIR,
+which is usually \"efi/guix\" or \"efi/boot\"."
+  (let* ((arch (car (string-split (or (%current-target-system)
+                                      (%current-system))
+                                  #\-)))
+         (efi-bootloader-link (string-append "boot"
+                                       (match arch
+                                         ("i686" "ia32")
+                                         ("x86_64" "x64")
+                                         ("armhf" "arm")
+                                         ("aarch64" "aa64")
+                                         ("riscv" "riscv32")
+                                         ("riscv64" "riscv64"))
+                                       ".efi"))
+         (efi-bootloader (string-append (match arch
+                                         ("i686" "i386")
+                                         ("x86_64" "x86_64")
+                                         ("armhf" "arm")
+                                         ("aarch64" "arm64")
+                                         ("riscv" "riscv32")
+                                         ("riscv64" "riscv64"))
+                                       "-efi/core.efi")))
+    #~(lambda (bootloader target mount-point)
+        "Install GRUB as e.g. \"bootx64.efi\" or \"bootarm64.efi\" \"into
+EFI-SUBDIR, which is usually \"efi/guix\" or \"efi/boot\" below the directory
+TARGET for the system whose root is mounted at MOUNT-POINT."
+        (let* ((mount-point-list (delete "" (string-split mount-point #\/)))
+               (target-list (delete "" (string-split target #\/)))
+               (net-dir
+                (string-append "/" (string-join (append
+                                                 mount-point-list
+                                                 target-list)
+                                                "/")))
+               (subdir #$efi-subdir)
+               (efi-bootloader-link
+                (string-append net-dir "/" subdir "/" #$efi-bootloader-link))
+               (store-name (car (delete "" (string-split bootloader #\/))))
+               (store
+                ;; Use target-list to construct a "../gnu" link with a correct
+                ;; number of "../" to the store.
+                (string-join (append (make-list (length target-list) "..")
+                                     (list store-name))
+                             "/"))
+               (store-link (string-append net-dir "/" store-name)))
+          ;; Tell 'grub-install' that there might be a LUKS-encrypted /boot or
+          ;; root partition.
+          (setenv "GRUB_ENABLE_CRYPTODISK" "y")
+          (invoke/quiet (string-append bootloader "/bin/grub-mknetdir")
+                        (string-append "--net-directory=" net-dir)
+                        (string-append "--subdir=" subdir))
+          (catch 'system-error
+            (lambda () (delete-file efi-bootloader-link))
+            (lambda _ #f))
+          (symlink #$efi-bootloader
+                   efi-bootloader-link)
+          (catch 'system-error
+            (lambda () (delete-file store-link))
+            (lambda _ #f))
+          (symlink store
+                   store-link)))))
+
 ^L
 
 ;;;
@@ -464,6 +515,16 @@ fi~%"))))
    (name 'grub-efi)
    (package grub-efi)))
 
+(define* (grub-efi-net-bootloader #:key (target #f) (efi-subdir #f))
+  (let ((target (or target "boot"))
+        (efi-subdir (or efi-subdir "efi/boot")))
+    (bootloader
+     (inherit grub-bootloader)
+     (name 'grub-efi-net-bootloader)
+     (package grub-efi)
+     (installer (install-grub-efi-net efi-subdir))
+     (configuration-file (string-append target "/" efi-subdir "/grub.cfg")))))
+
 (define* grub-mkrescue-bootloader
   (bootloader
    (inherit grub-efi-bootloader)
diff --git a/gnu/system.scm b/gnu/system.scm
index 29e622872d..540f0e4a9e 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -297,7 +297,8 @@ file system labels."
       ((? string? device)
        ;; It used to be that we would not distinguish between labels and
        ;; device names.  Try to infer the right thing here.
-       (if (string-prefix? "/dev/" device)
+       (if (or (string-prefix? "/dev/" device)
+               (string-contains device ":/")) ; nfs
            device
            (file-system-label device)))))
 
-- 
2.26.0

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Mathieu Othacehe]

Hello Stefan,

This patch does not apply here. Could you rebase it on top of master?

> -
> -      ;; Intel and EFI systems need to be switched into graphics mode, whereas
> -      ;; most other modern architectures have no other mode and therefore
> -      ;; don't need to be switched.
> -
> -      ;; XXX: Do we really need to restrict to x86 systems?  We could imitate
> -      ;; what the GRUB default configuration does and decide based on whether
> -      ;; a user provided 'gfxterm' in the terminal-outputs field of their
> -      ;; bootloader-configuration record.
> -      (if (string-match "^(x86_64|i[3-6]86)-" system)
> -          (format #f "
> +    (format #f "
>    set gfxmode=~a
>    insmod all_video
> -  insmod gfxterm~%" gfxmode)
> -          "")))
> +  insmod gfxterm~%"
> +            (string-join
> +             (grub-gfxmode (bootloader-theme config))
> +             ";")))

Why not enable graphic mode only if 'gfxterm' is provided in
terminal-outputs fields, like suggested by the comment?

> +         (efi-bootloader-link (string-append "boot"
> +                                       (match arch
> +                                         ("i686" "ia32")
> +                                         ("x86_64" "x64")
> +                                         ("armhf" "arm")

If cross-building for "arm-linux-gnueabihf", arch will be "arm" and
won't match anything here.

> +          (catch 'system-error
> +            (lambda () (delete-file efi-bootloader-link))
> +            (lambda _ #f))

You can use "false-if-exception" here I think.

> +          (symlink #$efi-bootloader
> +                   efi-bootloader-link)
> +          (catch 'system-error
> +            (lambda () (delete-file store-link))
> +            (lambda _ #f))

Same here.

> +(define* (grub-efi-net-bootloader #:key (target #f) (efi-subdir #f))

#f if implicit if omitted.

> +  (let ((target (or target "boot"))
> +        (efi-subdir (or efi-subdir "efi/boot")))

It would be better to keep grub-efi-net-bootloader as a variable, like
all other bootloaders. You could default configuration-file to
"boot/efi/boot/grub.cfg" instead?

Thanks,

Mathieu

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Stefan]

Hi Mathieu!

Thanks for your reply again! :-)

> Am 10.05.2020 um 10:20 schrieb Mathieu Othacehe <m.othacehe@gmail.com>:
> 
> This patch does not apply here. Could you rebase it on top of master?

I’ll try.

>> -      ;; Intel and EFI systems need to be switched into graphics mode, whereas
>> -      ;; most other modern architectures have no other mode and therefore
>> -      ;; don't need to be switched.
>> -
>> -      ;; XXX: Do we really need to restrict to x86 systems?  We could imitate
>> -      ;; what the GRUB default configuration does and decide based on whether
>> -      ;; a user provided 'gfxterm' in the terminal-outputs field of their
>> -      ;; bootloader-configuration record.
>> -      (if (string-match "^(x86_64|i[3-6]86)-" system)
>> -          (format #f "
>> +    (format #f "
>>   set gfxmode=~a
>>   insmod all_video
>> -  insmod gfxterm~%" gfxmode)
>> -          "")))
>> +  insmod gfxterm~%"
>> +            (string-join
>> +             (grub-gfxmode (bootloader-theme config))
>> +             ";")))
> 
> Why not enable graphic mode only if 'gfxterm' is provided in
> terminal-outputs fields, like suggested by the comment?

Good point.

Looking into this topic it seem's questionable to me that the default of (bootloader-configuration (terminal-output …)) with '(gfxterm) is grub-specific. This doesn't make sense for other boot-loaders, e.g. U-Boot. I expect this to be changed in future. ;-)

>> +         (efi-bootloader-link (string-append "boot"
>> +                                       (match arch
>> +                                         ("i686" "ia32")
>> +                                         ("x86_64" "x64")
>> +                                         ("armhf" "arm")
> 
> If cross-building for "arm-linux-gnueabihf", arch will be "arm" and
> won't match anything here.

Good catch!

>> +          (catch 'system-error
>> +            (lambda () (delete-file efi-bootloader-link))
>> +            (lambda _ #f))
> 
> You can use "false-if-exception" here I think.

Nice trick.

>> +          (symlink #$efi-bootloader
>> +                   efi-bootloader-link)
>> +          (catch 'system-error
>> +            (lambda () (delete-file store-link))
>> +            (lambda _ #f))
> 
> Same here.

Sure.

>> +(define* (grub-efi-net-bootloader #:key (target #f) (efi-subdir #f))
> 
> #f if implicit if omitted.

I wasn’t aware of this.

>> +  (let ((target (or target "boot"))
>> +        (efi-subdir (or efi-subdir "efi/boot")))
> 
> It would be better to keep grub-efi-net-bootloader as a variable, like
> all other bootloaders. You could default configuration-file to
> "boot/efi/boot/grub.cfg" instead?

Actually there is a problem with all this in guix: There is (bootloader (target …)), which gives the impression that one is able to freely chose a folder for the bootloader files. However, the path “/boot/grub.cfg” is kind of hard coded.

Yes, it’s kind of possible to inherit from grub-efi-bootloader and overwrite the (configuration-file) field. In a first step this seems to work. But when e.g. deleting a system generation, in guix/scripts/system.scm (reinstall-bootloader) there is this code:

    ;; Use the detected bootloader with default configuration.
    ;; It will be enough to allow the system to boot.
    (bootloader-config (bootloader-configuration
                        (bootloader bootloader))) 

It reads this information form /var/guix/profiles/system/parameters: (bootloader-name grub-efi-bootloader).
With this again the hard-coded path “/boot/grub.cfg” of is used, ignoring any value overwritten in (configuration-file).

Another issue is (install-dir (string-append mount-point "/boot")) in (install-grub-efi), which ignores any (configuration-file) setting, too – well, it has no access to that setting –, and implies at least “/boot” to be the prefix of (bootloader (target …)). 

Beside the wish to avoid this hard-coded “/boot“ assumption, I made this a function with two parameters for two more reasons. 

One is simply to suite my needs. The folder for my tftp server is not “boot” but “boot-nfs”. For my SBC I’m using different operating systems from time to time, e.g. LibreELEC. As I have bad experiences with unreliable micro SD cards and as an nfs root file system is nice for tinkering, I copy such operating systems onto my tftp/nfs server. This includes of corse their “boot” folder. The build-in update functionalities overwrite stuff inside there. But I need to modify stuff for network booting. To not loose these modifications during updates and for later comparisons I keep such modifications in a copy as “boot-nfs”.

The other reason is that I’m not sure, if the efi-dir for network booting should be “efi/Guix” instead of “efi/boot” in other constellations. U-Boot expects “efi/boot“ over tftp like for a removable media by default. I guess this can be changed with DNS options. Also for real UEFI firmware this might be configurable. I don’t know, so I don’t want these paths to be hard-coded.

However, digging up all this and now re-trying to delete a system generation, I get this error with my new boot-efi-net-bootloader as a function:

stefan@guix ~/development/guix$ sudo guix system delete-generations 151
/var/guix/profiles/system-151-link wird gelöscht
guix system: error: grub-efi-net-bootloader: no such bootloader

So thanks for your hint, it can’t be a function! (Not now …)


Bye

Stefan

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Stefan]

* gnu/bootloader/grub.scm (grub-efi-net-bootloader): New efi bootloader for
network booting via tftp/nfs, prepared for chain loading.
(make-grub-efi-net-bootloader): New macro to define a customized bootloader
based on 'grub-efi-net-bootloader'.
(install-grub-efi-net): New bootloader installer for tftp.
(grub-root-search): Adding support for tftp root.
(eye-candy): Use 'gfxterm' for all systems if selected via 'terminal-outputs'.
* gnu/system.scm (read-boot-parameters): Prevent devices with ":/" from being
treated as a file system label.
---
 gnu/bootloader/grub.scm | 128 +++++++++++++++++++++++++++++-----------
 gnu/system.scm          |   3 +-
 2 files changed, 95 insertions(+), 36 deletions(-)

diff --git a/gnu/bootloader/grub.scm b/gnu/bootloader/grub.scm
index 3f61b4a963..6ea1b38eb7 100644
--- a/gnu/bootloader/grub.scm
+++ b/gnu/bootloader/grub.scm
@@ -23,7 +23,7 @@
 
 (define-module (gnu bootloader grub)
   #:use-module (guix records)
-  #:use-module ((guix utils) #:select (%current-system))
+  #:use-module ((guix utils) #:select (%current-system %current-target-system))
   #:use-module (guix gexp)
   #:use-module (gnu artwork)
   #:use-module (gnu bootloader)
@@ -53,6 +53,8 @@
 
             grub-bootloader
             grub-efi-bootloader
+            make-grub-efi-net-bootloader
+            grub-efi-net-bootloader
             grub-mkrescue-bootloader
             grub-minimal-bootloader
 
@@ -93,7 +95,7 @@ denoting a file name."
                    (default '((fg . cyan) (bg . blue))))
   (color-highlight grub-theme-color-highlight
                    (default '((fg . white) (bg . blue))))
-  (gfxmode         grub-gfxmode
+  (gfxmode         grub-theme-gfxmode
                    (default '("auto"))))          ;list of string
 
 (define %background-image
@@ -143,40 +145,24 @@ WIDTH/HEIGHT, or #f if none was found."
                    #:width width #:height height))))
 
 (define* (eye-candy config store-device store-mount-point
-                    #:key system port)
+                    #:key port)
   "Return a gexp that writes to PORT (a port-valued gexp) the
 'grub.cfg' part concerned with graphics mode, background images, colors, and
 all that.  STORE-DEVICE designates the device holding the store, and
 STORE-MOUNT-POINT is its mount point; these are used to determine where the
-background image and fonts must be searched for.  SYSTEM must be the target
-system string---e.g., \"x86_64-linux\"."
-  (define setup-gfxterm-body
-    (let ((gfxmode
-           (or (and-let* ((theme (bootloader-configuration-theme config))
-                          (gfxmode (grub-gfxmode theme)))
-                 (string-join gfxmode ";"))
-               "auto")))
-
-      ;; Intel and EFI systems need to be switched into graphics mode, whereas
-      ;; most other modern architectures have no other mode and therefore
-      ;; don't need to be switched.
-
-      ;; XXX: Do we really need to restrict to x86 systems?  We could imitate
-      ;; what the GRUB default configuration does and decide based on whether
-      ;; a user provided 'gfxterm' in the terminal-outputs field of their
-      ;; bootloader-configuration record.
-      (if (string-match "^(x86_64|i[3-6]86)-" system)
-          (format #f "
-  set gfxmode=~a
-  insmod all_video
-  insmod gfxterm~%" gfxmode)
-          "")))
-
+background image and fonts must be searched for."
   (define (setup-gfxterm config font-file)
     (if (memq 'gfxterm (bootloader-configuration-terminal-outputs config))
-        #~(format #f "if loadfont ~a; then
-  setup_gfxterm
-fi~%" #$font-file)
+        #~(format #f "
+if loadfont ~a; then
+  set gfxmode=~a
+  insmod all_video
+  insmod gfxterm
+fi~%"
+                  #$font-file
+                  #$(string-join
+                      (grub-theme-gfxmode (bootloader-theme config))
+                      ";"))
         ""))
 
   (define (theme-colors type)
@@ -194,8 +180,6 @@ fi~%" #$font-file)
 
   (and image
        #~(format #$port "
-function setup_gfxterm {~a}
-
 # Set 'root' to the partition that contains /gnu/store.
 ~a
 
@@ -210,7 +194,6 @@ else
   set menu_color_normal=cyan/blue
   set menu_color_highlight=white/blue
 fi~%"
-                 #$setup-gfxterm-body
                  #$(grub-root-search store-device font-file)
                  #$(setup-gfxterm config font-file)
                  #$(grub-setup-io config)
@@ -317,6 +300,9 @@ code."
         ((? file-system-label? label)
          (format #f "search --label --set ~a"
                  (file-system-label->string label)))
+        ((? (lambda (device)
+              (and (string? device) (string-contains device ":/"))) nfs-uri)
+         "set root=(tftp)")
         ((or #f (? string?))
          #~(format #f "search --file --set ~a" #$file)))))
 
@@ -356,7 +342,6 @@ entries corresponding to old generations of the system."
     (eye-candy config
                (menu-entry-device (first all-entries))
                (menu-entry-device-mount-point (first all-entries))
-               #:system system
                #:port #~port))
 
   (define keyboard-layout-config
@@ -444,6 +429,68 @@ fi~%"))))
                       "--bootloader-id=Guix"
                       "--efi-directory" target-esp))))
 
+(define (install-grub-efi-net efi-subdir)
+  "Define a grub-efi bootloader installer for installation in EFI-SUBDIR,
+which is usually \"efi/guix\" or \"efi/boot\"."
+  (let* ((arch (car (string-split (or (%current-target-system)
+                                      (%current-system))
+                                  #\-)))
+         (efi-bootloader-link (string-append "boot"
+                                             (match arch
+                                               ("i686" "ia32")
+                                               ("x86_64" "x64")
+                                               ("arm" "arm")
+                                               ("armhf" "arm")
+                                               ("aarch64" "aa64")
+                                               ("riscv" "riscv32")
+                                               ("riscv64" "riscv64"))
+                                             ".efi"))
+         (efi-bootloader (string-append (match arch
+                                          ("i686" "i386")
+                                          ("x86_64" "x86_64")
+                                          ("arm" "arm")
+                                          ("armhf" "arm")
+                                          ("aarch64" "arm64")
+                                          ("riscv" "riscv32")
+                                          ("riscv64" "riscv64"))
+                                        "-efi/core.efi")))
+    #~(lambda (bootloader target mount-point)
+        "Install GRUB as e.g. \"bootx64.efi\" or \"bootarm64.efi\" \"into
+EFI-SUBDIR, which is usually \"efi/guix\" or \"efi/boot\" below the directory
+TARGET for the system whose root is mounted at MOUNT-POINT."
+        (let* ((mount-point-list (delete "" (string-split mount-point #\/)))
+               (target-list (delete "" (string-split target #\/)))
+               (net-dir
+                (string-append "/" (string-join (append
+                                                 mount-point-list
+                                                 target-list)
+                                                "/")))
+               (subdir #$efi-subdir)
+               (efi-bootloader-link
+                (string-append net-dir "/" subdir "/" #$efi-bootloader-link))
+               (store-name (car (delete "" (string-split bootloader #\/))))
+               (store
+                ;; Use target-list to construct a "../gnu" link with a correct
+                ;; number of "../" to the store.
+                (string-join (append (make-list (length target-list) "..")
+                                     (list store-name))
+                             "/"))
+               (store-link (string-append net-dir "/" store-name)))
+          ;; Tell 'grub-install' that there might be a LUKS-encrypted /boot or
+          ;; root partition.
+          (setenv "GRUB_ENABLE_CRYPTODISK" "y")
+          (invoke/quiet (string-append bootloader "/bin/grub-mknetdir")
+                        (string-append "--net-directory=" net-dir)
+                        (string-append "--subdir=" subdir))
+          (false-if-exception
+            (delete-file efi-bootloader-link))
+          (symlink #$efi-bootloader
+                   efi-bootloader-link)
+          (false-if-exception
+            (delete-file store-link))
+          (symlink store
+                   store-link)))))
+
 ^L
 
 ;;;
@@ -473,7 +520,18 @@ fi~%"))))
    (name 'grub-efi)
    (package grub-efi)))
 
-(define* grub-mkrescue-bootloader
+(define-syntax-rule (make-grub-efi-net-bootloader bootloader-name target efi-subdir)
+    (define bootloader-name
+       (bootloader
+          (inherit grub-bootloader)
+          (name (quote bootloader-name))
+          (package grub-efi)
+          (installer (install-grub-efi-net efi-subdir))
+          (configuration-file (string-append target "/" efi-subdir "/grub.cfg")))))
+
+(make-grub-efi-net-bootloader grub-efi-net-bootloader "boot" "efi/boot")
+
+(define grub-mkrescue-bootloader
   (bootloader
    (inherit grub-efi-bootloader)
    (package grub-hybrid)))
diff --git a/gnu/system.scm b/gnu/system.scm
index cd75e4d4ba..c92bf57f1e 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -300,7 +300,8 @@ file system labels."
       ((? string? device)
        ;; It used to be that we would not distinguish between labels and
        ;; device names.  Try to infer the right thing here.
-       (if (string-prefix? "/dev/" device)
+       (if (or (string-prefix? "/dev/" device)
+               (string-contains device ":/")) ; nfs
            device
            (file-system-label device)))))
 
-- 
2.26.0

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Stefan]

Hi Mathieu!

This patch has conflicts meanwhile and needs to be rebased. I’ll send a new patch soon.


Bye

Stefan

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Stefan]

* gnu/bootloader/grub.scm (grub-efi-net-bootloader): New efi bootloader for
network booting via tftp/nfs, prepared for chain loading.
(make-grub-efi-net-bootloader): New macro to define a customized bootloader
based on 'grub-efi-net-bootloader'.
(install-grub-efi-net): New bootloader installer for tftp.
(grub-root-search): Adding support for tftp root.
(eye-candy): Use 'gfxterm' for all systems if selected via 'terminal-outputs'.
* gnu/system.scm (read-boot-parameters): Prevent devices with ":/" from being
treated as a file system label.
---
 gnu/bootloader/grub.scm | 125 +++++++++++++++++++++++++++++-----------
 gnu/system.scm          |   3 +-
 2 files changed, 94 insertions(+), 34 deletions(-)

diff --git a/gnu/bootloader/grub.scm b/gnu/bootloader/grub.scm
index bb40c551a7..2b0ecec279 100644
--- a/gnu/bootloader/grub.scm
+++ b/gnu/bootloader/grub.scm
@@ -23,7 +23,7 @@
 
 (define-module (gnu bootloader grub)
   #:use-module (guix records)
-  #:use-module ((guix utils) #:select (%current-system))
+  #:use-module ((guix utils) #:select (%current-system %current-target-system))
   #:use-module (guix gexp)
   #:use-module (gnu artwork)
   #:use-module (gnu bootloader)
@@ -47,6 +47,8 @@
 
             grub-bootloader
             grub-efi-bootloader
+            make-grub-efi-net-bootloader
+            grub-efi-net-bootloader
             grub-mkrescue-bootloader
             grub-minimal-bootloader
 
@@ -140,36 +142,20 @@ file with the resolution provided in CONFIG."
 concerned with graphics mode, background images, colors, and all that.
 STORE-DEVICE designates the device holding the store, and STORE-MOUNT-POINT is
 its mount point; these are used to determine where the background image and
-fonts must be searched for.  SYSTEM must be the target system string---e.g.,
-\"x86_64-linux\".  BTRFS-STORE-SUBVOLUME-FILE-NAME is the file name of the
-Btrfs subvolume, to be prepended to any store path, if any."
-  (define setup-gfxterm-body
-    (let ((gfxmode
-           (or (and-let* ((theme (bootloader-configuration-theme config))
-                          (gfxmode (grub-theme-gfxmode theme)))
-                 (string-join gfxmode ";"))
-               "auto")))
-
-      ;; Intel and EFI systems need to be switched into graphics mode, whereas
-      ;; most other modern architectures have no other mode and therefore
-      ;; don't need to be switched.
-
-      ;; XXX: Do we really need to restrict to x86 systems?  We could imitate
-      ;; what the GRUB default configuration does and decide based on whether
-      ;; a user provided 'gfxterm' in the terminal-outputs field of their
-      ;; bootloader-configuration record.
-      (if (string-match "^(x86_64|i[3-6]86)-" system)
-          (format #f "
-  set gfxmode=~a
-  insmod all_video
-  insmod gfxterm~%" gfxmode)
-          "")))
-
+fonts must be searched for.  BTRFS-STORE-SUBVOLUME-FILE-NAME is the file name
+of the Btrfs subvolume, to be prepended to any store path, if any."
   (define (setup-gfxterm config font-file)
     (if (memq 'gfxterm (bootloader-configuration-terminal-outputs config))
-        #~(format #f "if loadfont ~a; then
-  setup_gfxterm
-fi~%" #+font-file)
+        #~(format #f "
+if loadfont ~a; then
+  set gfxmode=~a
+  insmod all_video
+  insmod gfxterm
+fi~%"
+                  #$font-file
+                  #$(string-join
+                      (grub-theme-gfxmode (bootloader-theme config))
+                      ";"))
         ""))
 
   (define (theme-colors type)
@@ -190,8 +176,6 @@ fi~%" #+font-file)
 
   (and image
        #~(format #$port "
-function setup_gfxterm {~a}
-
 # Set 'root' to the partition that contains /gnu/store.
 ~a
 
@@ -206,7 +190,6 @@ else
   set menu_color_normal=cyan/blue
   set menu_color_highlight=white/blue
 fi~%"
-                 #$setup-gfxterm-body
                  #$(grub-root-search store-device font-file)
                  #$(setup-gfxterm config font-file)
                  #$(grub-setup-io config)
@@ -313,6 +296,9 @@ code."
         ((? file-system-label? label)
          (format #f "search --label --set ~a"
                  (file-system-label->string label)))
+        ((? (lambda (device)
+              (and (string? device) (string-contains device ":/"))) nfs-uri)
+         "set root=(tftp)")
         ((or #f (? string?))
          #~(format #f "search --file --set ~a" #$file)))))
 
@@ -454,6 +440,68 @@ fi~%"))))
                       "--bootloader-id=Guix"
                       "--efi-directory" target-esp))))
 
+(define (install-grub-efi-net efi-subdir)
+  "Define a grub-efi bootloader installer for installation in EFI-SUBDIR,
+which is usually \"efi/boot\" or \"efi/Guix\"."
+  (let* ((arch (car (string-split (or (%current-target-system)
+                                      (%current-system))
+                                  #\-)))
+         (efi-bootloader-link (string-append "boot"
+                                             (match arch
+                                               ("i686" "ia32")
+                                               ("x86_64" "x64")
+                                               ("arm" "arm")
+                                               ("armhf" "arm")
+                                               ("aarch64" "aa64")
+                                               ("riscv" "riscv32")
+                                               ("riscv64" "riscv64"))
+                                             ".efi"))
+         (efi-bootloader (string-append (match arch
+                                          ("i686" "i386")
+                                          ("x86_64" "x86_64")
+                                          ("arm" "arm")
+                                          ("armhf" "arm")
+                                          ("aarch64" "arm64")
+                                          ("riscv" "riscv32")
+                                          ("riscv64" "riscv64"))
+                                        "-efi/core.efi")))
+    #~(lambda (bootloader target mount-point)
+        "Install GRUB as e.g. \"bootx64.efi\" or \"bootarm64.efi\" \"into
+EFI-SUBDIR, which is usually \"efi/boot\" or \"efi/Guix\" below the directory
+TARGET for the system whose root is mounted at MOUNT-POINT."
+        (let* ((mount-point-list (delete "" (string-split mount-point #\/)))
+               (target-list (delete "" (string-split target #\/)))
+               (net-dir
+                (string-append "/" (string-join (append
+                                                 mount-point-list
+                                                 target-list)
+                                                "/")))
+               (subdir #$efi-subdir)
+               (efi-bootloader-link
+                (string-append net-dir "/" subdir "/" #$efi-bootloader-link))
+               (store-name (car (delete "" (string-split bootloader #\/))))
+               (store
+                ;; Use target-list to construct a "../gnu" link with a correct
+                ;; number of "../" to the store.
+                (string-join (append (make-list (length target-list) "..")
+                                     (list store-name))
+                             "/"))
+               (store-link (string-append net-dir "/" store-name)))
+          ;; Tell 'grub-install' that there might be a LUKS-encrypted /boot or
+          ;; root partition.
+          (setenv "GRUB_ENABLE_CRYPTODISK" "y")
+          (invoke/quiet (string-append bootloader "/bin/grub-mknetdir")
+                        (string-append "--net-directory=" net-dir)
+                        (string-append "--subdir=" subdir))
+          (false-if-exception
+            (delete-file efi-bootloader-link))
+          (symlink #$efi-bootloader
+                   efi-bootloader-link)
+          (false-if-exception
+            (delete-file store-link))
+          (symlink store
+                   store-link)))))
+
 ^L
 
 ;;;
@@ -483,7 +531,18 @@ fi~%"))))
    (name 'grub-efi)
    (package grub-efi)))
 
-(define* grub-mkrescue-bootloader
+(define-syntax-rule (make-grub-efi-net-bootloader bootloader-name target efi-subdir)
+    (define bootloader-name
+       (bootloader
+          (inherit grub-bootloader)
+          (name (quote bootloader-name))
+          (package grub-efi)
+          (installer (install-grub-efi-net efi-subdir))
+          (configuration-file (string-append target "/" efi-subdir "/grub.cfg")))))
+
+(make-grub-efi-net-bootloader grub-efi-net-bootloader "boot" "efi/boot")
+
+(define grub-mkrescue-bootloader
   (bootloader
    (inherit grub-efi-bootloader)
    (package grub-hybrid)))
diff --git a/gnu/system.scm b/gnu/system.scm
index d929187695..2035235549 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -301,7 +301,8 @@ file system labels."
       ((? string? device)
        ;; It used to be that we would not distinguish between labels and
        ;; device names.  Try to infer the right thing here.
-       (if (string-prefix? "/dev/" device)
+       (if (or (string-prefix? "/dev/" device)
+               (string-contains device ":/")) ; nfs
            device
            (file-system-label device)))))
 
-- 
2.26.0

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Mathieu Othacehe]

Hey Stefan,

Thanks for rebasing!

> +    #~(lambda (bootloader target mount-point)
> +        "Install GRUB as e.g. \"bootx64.efi\" or \"bootarm64.efi\" \"into
> +EFI-SUBDIR, which is usually \"efi/boot\" or \"efi/Guix\" below the directory

Is that TARGET or EFI-SUBDIR?

> +TARGET for the system whose root is mounted at MOUNT-POINT."
> +        (let* ((mount-point-list (delete "" (string-split mount-point #\/)))
> +               (target-list (delete "" (string-split target #\/)))
> +               (net-dir
> +                (string-append "/" (string-join (append
> +                                                 mount-point-list
> +                                                 target-list)
> +                                                "/")))

I think you can use something like "(in-vicinity mount-point target)"
to do the same job.

> +          ;; Tell 'grub-install' that there might be a LUKS-encrypted /boot or
> +          ;; root partition.
> +          (setenv "GRUB_ENABLE_CRYPTODISK" "y")
> +          (invoke/quiet (string-append bootloader "/bin/grub-mknetdir")
> +                        (string-append "--net-directory=" net-dir)
> +                        (string-append "--subdir=" subdir))
> +          (false-if-exception
> +            (delete-file efi-bootloader-link))
> +          (symlink #$efi-bootloader
> +                   efi-bootloader-link)
> +          (false-if-exception
> +            (delete-file store-link))
> +          (symlink store
> +                   store-link)))))

What's the purpose of those two symlinks, isn't grub-mknetdir taking
care of all this?

Creating a system test for this may be a bit difficult, but if you could
add a section in the documentation describing how to setup a
'grub-efi-net-bootloader, that would be great!

Thanks,

Mathieu

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Stefan]

Hi Mathieu!

> Am 23.05.2020 um 10:10 schrieb Mathieu Othacehe <othacehe@gnu.org>:
> 
>> +    #~(lambda (bootloader target mount-point)
>> +        "Install GRUB as e.g. \"bootx64.efi\" or \"bootarm64.efi\" \"into
>> +EFI-SUBDIR, which is usually \"efi/boot\" or \"efi/Guix\" below the directory
> 
> Is that TARGET or EFI-SUBDIR?

I really mean with EFI-SUBDIR the argument from the enclosing install-grub-efi-net function, and with TARGET the argument of the lambda function. The prototype of the lambda function is fixed by some other guix machinery, it is not possible to pass an EFI-SUBDIR argument directly.

Take for example this path, the default: "/mnt/boot/efi/boot". Then MOUNT-POINT is "/mnt" (like in 'guix system init config.scm /mnt'), TARGET is "/boot" (which you also have to specify inside (operating-system (bootloader (target "/boot") …) …), and EFI-SUBDIR is the remaining "efi/boot".

In my personal case I need MOUNT-POINT as "/" (well, 'guix system init …' was only necessary once), TARGET as "boot-nfs" (my special setup), and EFI-SUBDIR as "efi/boot" (because the U-Boot is using this path by default).

For a TFFP server this TARGET with /boot (or /boot-nfs in my case) is the directory whose content has to be served. It can’t be /boot/efi (like for the normal grub-efi bootloader), as the TFTP server needs to serve several firmware files and the U-Boot.¹ When the U-Boot takes control, by default it tries to load /efi/boot/bootaa64.efi via TFTP.

It may be possible to configure the U-Boot to look for files below a /efi/Guix directory or any other path, I’m not really sure about this yet. At least /efi/boot is the documented last resort for UEFI systems to look for some “removable madia” to boot over TFTP.

Next GRUB takes control and looks for its grub.cfg and all its other files via TFTP at the path specified during 'grub-mknetdir --subdir=…'. As the TFTP root is /boot (where U-Boot and other stuff may live), this now has to be efi/boot. So you get all GRUB files like /boot/efi/boot/grub.cfg served via TFTP from the directory /boot/efi/boot.

For the usual grub-efi-bootloader this is a bit different. Here the normal path is /mnt/boot/efi/Guix and for the 'grub-install' command these parameters are required: '—-boot-directory /mnt/boot --bootloader-id Guix --efi-directory /boot/efi'. Here the "boot" part to '--boot-directory' is hard coded, as well as "Guix". So the only part you are free to chose is "efi" in (operating-system (bootloader (target "boot/efi") …) …). Additionally the usual grub-efi-bootloader looks for /boot/grub.cfg which thus is not residing inside the efi partition like all its other files.


>> +TARGET for the system whose root is mounted at MOUNT-POINT."
>> +        (let* ((mount-point-list (delete "" (string-split mount-point #\/)))
>> +               (target-list (delete "" (string-split target #\/)))
>> +               (net-dir
>> +                (string-append "/" (string-join (append
>> +                                                 mount-point-list
>> +                                                 target-list)
>> +                                                "/")))
> 
> I think you can use something like "(in-vicinity mount-point target)"
> to do the same job.

MOUNT-POINT may be just "/", TARGET is "/boot". I’m avoiding double slashes, like "//boot", also for all the other paths. In an earlier version I had the “problem” that GRUB searched files with the prefix "//efi/boot" via TFTP. This confused me on one side, but on the other side I feared that another TFTP server might get problems with this.

Avoiding double slashes seems not to be something that in-vicinity is taking care about. And TARGET with its "/boot" looks like an absolute path, making it an improper argument to in-vicinity: “in-vicinity should allow filename to override vicinity when filename is an absolute pathname and vicinity is equal to the value of (user-vicinity). The behavior of in-vicinity when filename is absolute and vicinity is not equal to the value of (user-vicinity) is unspecified.”

A bit further down I need to count the directory levels up to the "/gnu" store to construct a proper link, so I need the result of a string-split anyway. 

So all in all – although it looks a bit complicated – I’d like to keep this.

Hm, but I wasn’t that strict with the EFI-SUBDIR argument. And potentially MOUNT-POINT may be a relative path, may it? Do you know? In that case there is a bug in prepending the "/" to net-dir, turning a relative into an absolute path.

> 
>> +          ;; Tell 'grub-install' that there might be a LUKS-encrypted /boot or
>> +          ;; root partition.
>> +          (setenv "GRUB_ENABLE_CRYPTODISK" "y")
>> +          (invoke/quiet (string-append bootloader "/bin/grub-mknetdir")
>> +                        (string-append "--net-directory=" net-dir)
>> +                        (string-append "--subdir=" subdir))
>> +          (false-if-exception
>> +            (delete-file efi-bootloader-link))
>> +          (symlink #$efi-bootloader
>> +                   efi-bootloader-link)
>> +          (false-if-exception
>> +            (delete-file store-link))
>> +          (symlink store
>> +                   store-link)))))
> 
> What's the purpose of those two symlinks, isn't grub-mknetdir taking
> care of all this?

No, it doesn’t. The final .efi file after calling 'grub-mknetdir' is /boot/efi/boot/arm64-efi/core.efi. I guess when using PXE there is a way to point to this efi file, so GRUB doesn’t care. But without this the U-Boot looks for the standardised /efi/boot/bootaa64.efi file via TFTP. So the first symlink fixes this gap without removing other possibilities.

The second symlink is for GRUB itself. Inside /boot/efi/boot/grub.cfg there are commands to specify the root device, usually via a file system label or a UUID. But in this case the root device specification is just ”set root=(tftp)”. Following lines look up files by e.g. /gnu/store/…-grub-2.04/share/grub/unicode.pf2, which GRUB will then try to access via TFTP. But the root for the TFTP server is /boot from the TARGET argument, and not /. So GRUB trying to access /gnu/store/… results in an access to /boot/gnu/store/…. Therefore the symlink from /boot/gnu to ../gnu is needed. And as TARGET can freely be chosen the number of up-levels needs to be calculated.

Where the usual grub-efi-bootloader is able to find and access the real root device directly, the grub-efi-net-bootloader can only use TFTP. Therefore we have to make the real “root device” accessible via TFPT. I’m not sure if this is wise from a security point of view. But otherwise all files listed in the grub.cfg would need to be copied. This would be an overhead and leads to more complications, when deleting system generations; then obsolete copies need determined and removed again. 

> Creating a system test for this may be a bit difficult, but if you could
> add a section in the documentation describing how to setup a
> 'grub-efi-net-bootloader, that would be great!

I have some difficulties with this – despite not having knowledge with texi yet. So help is very welcome.

You need to make use of an NFS server, serving your / directory. 

You need a TFTP server set up to serve the files of our /boot directory with access to your /gnu store via the /boot/gnu symlink as well. 

You need to setup your DNS server to send some boot-options. 

I use a “DiskStation” with its UI for all this but needed to fiddle around with dnsmasq for the DNS. So this is nothing for a guix documentation. However, this might be even out of scope. But ideally there would be examples of how to achieve all this with guix itself. I have no experience using guix for anything of this.

Then I use all this to boot a Raspberry Pi 3b in aarch64 mode. This requires a none free firmware, additional configuration files for the firmware, the (modified) U-Boot. And there is even more. I compiled the kernel linux (currently from the mainline) with a Raspberry specific defconfig-package as an input for the linux package and an additional function to modify the defconfig on the fly.

There are more patches to follow for at least parts of all this.

But what I actually want to say with this: Even to reproduce my setup, still a lot other stuff is missing. And I have no other computer for tinkering. I can’t try out this patch on a usual x86…64 machine. I can't really tell, if this will work right out of the box for other people on different computers.

Before documenting this for the public, I would wish that at least someone else with access to an x86_64 UEFI system gives it a try. I'm pretty sure it will work.

Besides all the server stuff with NFS, TFTP and DNS, there are only few specialities to know:

(operating-system
  (file-systems (cons (file-system
                        (mount-point "/")
                        (type "nfs")
                        (device ":/your/servers/path/to/guix-root")
                        (options "addr=10.11.12.2,vers=4.1"))
                       %base-file-systems)) 
  (bootloader 
    (target "/boot")
    (bootloader-configuration (bootloader grub-efi-net-bootloader)
    …))
  …)

The IP address of your server needs to be specified via an “addr=” option. To make use of the defaults of the new grub-efi-net-bootloader, the target field has to be "/boot".


Bye

Stefan


¹ Well, actually – as for grub-efi-net-bootloader you are now free to use any path for target – you can use "/boot/efi". But then you need to make this your TFTP server’s root, and you need to use the make-grub-efi-net-bootloader macro with "/boot/efi" as well, and you will end up in having double efi folders as in /boot/efi/efi/boot/grub.cfg etc. So this doesn’t make sense.

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Mathieu Othacehe]

Hello Stefan,

> Yes, it’s kind of possible to inherit from grub-efi-bootloader and overwrite the (configuration-file) field. In a first step this seems to work. But when e.g. deleting a system generation, in guix/scripts/system.scm (reinstall-bootloader) there is this code:
>
>     ;; Use the detected bootloader with default configuration.
>     ;; It will be enough to allow the system to boot.
>     (bootloader-config (bootloader-configuration
>                         (bootloader bootloader))) 
>
> It reads this information form /var/guix/profiles/system/parameters: (bootloader-name grub-efi-bootloader).
> With this again the hard-coded path “/boot/grub.cfg” of is used, ignoring any value overwritten in (configuration-file).

Oh, we need to fix that! It means that we would need to add a
"bootloader-configuration-file" field to <boot-parameters> record, is
that correct?

> Another issue is (install-dir (string-append mount-point "/boot")) in (install-grub-efi), which ignores any (configuration-file) setting, too – well, it has no access to that setting –, and implies at least “/boot” to be the prefix of (bootloader (target …)). 
>
> Beside the wish to avoid this hard-coded “/boot“ assumption, I made this a function with two parameters for two more reasons. 

Could it be an option to infer the bootloader installation directory and
the efi subdir from the install-grub-efi/install-grub-efi-net functions?
If TARGET is /boot-nfs/efi/Guix", could we suppose that the
boot-directory is "/boot-nfs" and the efi-subdir is "efi/Guix"?

The make-grub-efi-net-bootloader macro is a nice hack, but I fear that
it makes the bootloader configuration (even more) difficult.

Thanks,

Mathieu

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Stefan]

Hi Mathieu!


> Am 23.05.2020 um 10:02 schrieb Mathieu Othacehe <othacehe@gnu.org>:
> 
>> It reads this information form /var/guix/profiles/system/parameters: (bootloader-name grub-efi-bootloader).
>> With this again the hard-coded path “/boot/grub.cfg” is used, ignoring any value overwritten in (configuration-file).
> 
> Oh, we need to fix that! It means that we would need to add a
> "bootloader-configuration-file" field to <boot-parameters> record, is
> that correct?

Yes, I would guess so. But I’m not sure, if the field bootloader-name can be dropped then from <boot-parameters>. But if, then we could probably also drop the field name from the <bootloader> record. 

>> Another issue is (install-dir (string-append mount-point "/boot")) in (install-grub-efi), which ignores any (configuration-file) setting, too – well, it has no access to that setting –, and implies at least “/boot” to be the prefix of (bootloader (target …)). 
>> 
>> Beside the wish to avoid this hard-coded “/boot“ assumption, I made this a function with two parameters for two more reasons. 
> 
> Could it be an option to infer the bootloader installation directory and
> the efi subdir from the install-grub-efi/install-grub-efi-net functions?
> If TARGET is /boot-nfs/efi/Guix", could we suppose that the
> boot-directory is "/boot-nfs" and the efi-subdir is "efi/Guix"?

For the new install-grub-efi-net I see first of all no issue in keeping it a function. This gives the needed flexibility.

For the existing grub-efi-bootloader the assumption seems to be that there will always be a /boot/grub.cfg. This is just not stated in the documentation. But it gave me the impression that there is some control about it with (bootloader (target …) …). But this is not the case. For the legacy grub-bootlouder the (bootloader (target …) …) needs to be a device, and the /boot/grub.cfg is implied and hard coded as well.

Actually thinking about it again, mounting the efi partition at e.g. /foo/efi, doesn't brake anything in the first place. Then GRUB will be installed at the target /foo/efi, basically into the efi partition it belongs. It will just read its configuration from /boot/grub.cfg, from a different partition.

The actual difference to the new grub-efi-net-bootloader is that it has only TFTP access to its files and its configuration file; there is only one place to lookup both, instead of two partitions in case of the grub-efi-bootloader.

For deleting system generations the path to the always present, not configurable /boot/grub.cfg is looked up. This works for the existing grub-efi-bootloader and grub-bootloader. But it does not work for the grub-efi-net-bootloader, because its configuration file does not live at /boot/grub.cfg, as its path is now implicitly configurable via (bootloader (target …) …). In addition for my setup I used a /boot-nfs folder instead of a /boot folder, and saw no benefit then in keeping the /boot folder.

I think there is a second possibility. It may be possible to create a symlink from /boot-nfs/efi/boot/grub.cfg to ../../../boot/grub.cfg. Then the assumption that there will always be a /boot/grub.cfg file stays valid. 

But personally I do not like this idea.

In <boot-parameters> it seems – but I’m not sure yet – that we only keep a symbol name to figure out the path to the grub.cfg, although it is possible to just put that path directly there instead. And using the symbol makes it hard do get a configurable bootloader: A new bootloader has to be a variable, tricks with macros come up. Also inheriting from a bootloader and overwriting the configuration-file field – for whatever reason – is problematic: It seems to work at the beginning, but only fails badly when removing a system generation. It seems to have subtle bugs. It’s not usable like other parts in guix. It’s not hackable. I’d really prefer to change this.

> The make-grub-efi-net-bootloader macro is a nice hack, but I fear that
> it makes the bootloader configuration (even more) difficult.

At least it gives me the flexibility which was missing so far. I suggest to keep it for the moment and do a different patch once we are clear which way to go. If we add a bootloader-configuration-file field to the <boot-parameters> record, than the macro can be removed anyway.


Bye

Stefan

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Danny Milosavljevic]

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

Hi Stefan,

On Sun, 24 May 2020 12:18:38 +0200
Stefan <stefan-guix@vodafonemail.de> wrote:

> But I’m not sure, if the field bootloader-name can be dropped then[after adding bootloader-configuration-file to boot-parameters] from <boot-parameters>. But if, then we could probably also drop the field name from the <bootloader> record. 

We definitely can't drop it.  The name is required in order to know which
bootloader to restore when deleting system generations.  After all you could be
deleting the generation that switched from extlinux to grub.  How to boot then?

(see lookup-bootloader-by-name call site)

Since we are trying to have the bootloader to use part of the Guix system
configuration (for better or for worse), we have to be really careful to
pick the right bootloader and generate the configuration for the right
bootloader, otherwise the computer won't boot anymore *and* you couldn't
select the before-fuckup generation anymore either.

> Yes, it’s kind of possible to inherit from grub-efi-bootloader and overwrite
the (configuration-file) field. In a first step this seems to work. But when
e.g. deleting a system generation, in guix/scripts/system.scm
(reinstall-bootloader) there is this code:
>
>     ;; Use the detected bootloader with default configuration.
>     ;; It will be enough to allow the system to boot.
>     (bootloader-config (bootloader-configuration
>                         (bootloader bootloader))) 

>In <boot-parameters> it seems – but I’m not sure yet – that we only keep a symbol name to figure out the path to the grub.cfg, although it is possible to just put that path directly there instead. And using the symbol makes it hard do get a configurable bootloader: A new bootloader has to be a variable, tricks with macros come up. Also inheriting from a bootloader and overwriting the configuration-file field – for whatever reason – is problematic: It seems to work at the beginning, but only fails badly when removing a system generation. It seems to have subtle bugs. It’s not usable like other parts in guix. It’s not hackable. I’d really prefer to change this.

Yeah, well...  that is the only way we could think of to make sure it actually
boots in all cases, as it is right now.

(Though if the user had custom entries, that would nuke all of them--but
that's still better than not being able to boot at all)

Using a symbol was to make it clear that this is supposed to be a reference
to an actual variable and not some weird mini-programming language inside a
string or whatever.

It would be better to have some kind of abstract representation of *all*
the bootloader things one could want, in Guix in config.scm.
That would make Guix system a lot more complicated, though (chainloading
bootloaders for one--I saw you working on this, too).

Just to be clear, I'm fine with changing boot-parameters, but be very very
careful that all old versions of Guix and new versions of Guix can handle
all the boot-parameters--at least falling back to something.  It's not fun
if you can't boot anymore.  Source: I modified a lot of that stuff and
wasn't able to boot quite often until I finally stopped overcomplicating
the boot-parameters.

>Actually there is a problem with all this in guix: There is (bootloader (target …)),
>which gives the impression that one is able to freely chose a folder for the
>bootloader files. However, the path “/boot/grub.cfg” is kind of hard coded.

Could you elaborate why having that hard-coded file path is bad?

It makes booting a lot more resilient if it's hard-coded as opposed to having
basic stuff like that configurable--and being careful all the time it's
actually configured correctly for all the parties to it, some of them maybe
not even inside Guix.

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

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Stefan]

Hi Danny!

> Am 24.05.2020 um 13:00 schrieb Danny Milosavljevic <dannym@scratchpost.org>:
> 
>> But I’m not sure, if the field bootloader-name can be dropped then[after adding bootloader-configuration-file to boot-parameters] from <boot-parameters>. But if, then we could probably also drop the field name from the <bootloader> record. 
> 
> We definitely can't drop it.  The name is required in order to know which
> bootloader to restore when deleting system generations.  After all you could be
> deleting the generation that switched from extlinux to grub.  How to boot then?
> 
> (see lookup-bootloader-by-name call site)

OK, I understand. I will take a look at it.

> Since we are trying to have the bootloader to use part of the Guix system
> configuration (for better or for worse), we have to be really careful to
> pick the right bootloader and generate the configuration for the right
> bootloader, otherwise the computer won't boot anymore *and* you couldn't
> select the before-fuckup generation anymore either.

Hm, if I select an older system generation in GRUB, than that older one is booted. But this doesn't change the bootloader.
If I then delete some system generations – as I’ve seen so far, but I might be wrong – the bootloader is not reinstalled either. Only the grub.cfg is regenerated to remove the deleted generations. If I reboot, then I'm still using the latest generation GRUB, but boot some older system generation, which would not be able by itself to install this very recent GRUB in use.

If I then reconfigure the system, only then another GRUB - or even a different bootloader, depending on my etc/config.scm – will be installed and the according configuration file will be generated as well. Then again all will fit. In the worst case the (bootloader (bootloader-configuration …) …) in my etc/config.scm is still newer than this older guix system in use is able to handle. 

Oh, by the way, does booting an older system generation also change the guix version in use from the latest 'guix pull'? I don't think it does.

And does booting an older generation change the config.scm? I don’t think so either.

Actually, I don’t really understand what you mean. Are there circumstances beside a 'guix system reconfigure' in which the bootloader gets reinstalled? And with reinstall I don’t mean to only regenerate the grub.cfg, but calling /sbin/grub-install.

Isn’t the actual problem for an older running system generation to know which bootloder is currently in use? I think this can't be inferred by the currently running system generation. It may happen, that you use a brand new bootloader which is not known by the older system generation you just switched to.

But still then, if you invoke a 'sudo -E guix system delete-generations' or a 'sudo -E guix system reconfigure' I think you still use the very latest guix version that you 'guix pull'-ed last. And that guix version should still know all brand new bootloader. The problem may “only” be to know for 'sudo -E guix system delete-generations' which one to use. But actually the bootloader-name field in /var/guix/profiles/system/parameters can't tell either, as it must be an older bootloader than the brand new one.

Maybe the information about the bootloader version in use needs to reside with the installed bootloader somewhere below /boot/efi/…? But this may be impossible for the legacy grub-bootloader.

>> Yes, it’s kind of possible to inherit from grub-efi-bootloader and overwrite
> the (configuration-file) field. In a first step this seems to work. But when
> e.g. deleting a system generation, in guix/scripts/system.scm
> (reinstall-bootloader) there is this code:
>> 
>>    ;; Use the detected bootloader with default configuration.
>>    ;; It will be enough to allow the system to boot.
>>    (bootloader-config (bootloader-configuration
>>                        (bootloader bootloader))) 
> 
>> In <boot-parameters> it seems – but I’m not sure yet – that we only keep a symbol name to figure out the path to the grub.cfg, although it is possible to just put that path directly there instead. And using the symbol makes it hard do get a configurable bootloader: A new bootloader has to be a variable, tricks with macros come up. Also inheriting from a bootloader and overwriting the configuration-file field – for whatever reason – is problematic: It seems to work at the beginning, but only fails badly when removing a system generation. It seems to have subtle bugs. It’s not usable like other parts in guix. It’s not hackable. I’d really prefer to change this.
> 
> Yeah, well...  that is the only way we could think of to make sure it actually
> boots in all cases, as it is right now.

I think after switching to an older system generation this is not true, as explained above. Am I wrong about this?

> Just to be clear, I'm fine with changing boot-parameters, but be very very
> careful that all old versions of Guix and new versions of Guix can handle
> all the boot-parameters--at least falling back to something.

I see.

> Could you elaborate why having that hard-coded file path is bad?
> 
> It makes booting a lot more resilient if it's hard-coded as opposed to having
> basic stuff like that configurable--and being careful all the time it's
> actually configured correctly for all the parties to it, some of them maybe
> not even inside Guix.

As I wrote a bit below: I think there is a second possibility. It may be possible to create a symlink from /boot-nfs/efi/boot/grub.cfg to ../../../boot/grub.cfg. Then the assumption that there will always be a /boot/grub.cfg file stays valid. But Im still unsure about this.


Bye

Stefan

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Danny Milosavljevic]

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

Hi Stefan,

On Sun, 24 May 2020 15:09:02 +0200
Stefan <stefan-guix@vodafonemail.de> wrote:

> Hm, if I select an older system generation in GRUB, than that older one is booted. But this doesn't change the bootloader.

Correct.  It doesn't need to since it just changes which Linux kernel will be booted temporarily, including what system and so on.

"System" here excludes $HOME.

Since the guix package manager itself is in $HOME, I think that that doesn't revert though.

> If I then delete some system generations – as I’ve seen so far, but I might be wrong – the bootloader is not reinstalled either.
> Only the grub.cfg is regenerated to remove the deleted generations.

You are totally right O_O

reinstall-bootloader says:

>          ;; Only install bootloader configuration file.

What happened here?  Why?!

(I think we should document these bootloader Guix intricacies in some better place than the mailing list archives; can't keep the thing straight otherwise :P)

>If I reboot, then I'm still using the latest generation GRUB, but boot some older system generation, which would not be able by itself to install this very recent GRUB in use.

It should be able to since guix package manager including package definitions is in $HOME, which is not rolled back, I think.  The only way to install the very recent GRUB is guix system reconfigure, and that will totally be able to see the newest guix packages.

> If I then reconfigure the system, only then another GRUB - or even a different bootloader, depending on my etc/config.scm – will be installed and the according configuration file will be generated as well. Then again all will fit.

Yes, that case is fine.

> In the worst case the (bootloader (bootloader-configuration …) …) in my etc/config.scm is still newer than this older guix system in use is able to handle.

I think guix-the-package-manager is not reverted, so it will be able to see the newest installable stuff. 

> Oh, by the way, does booting an older system generation also change the guix version in use from the latest 'guix pull'? I don't think it does.

No, that is per-user and not per-system.

> And does booting an older generation change the config.scm? I don’t think so either.

No.

> Actually, I don’t really understand what you mean. Are there circumstances beside a 'guix system reconfigure' in which the bootloader gets reinstalled? And with reinstall I don’t mean to only regenerate the grub.cfg, but calling /sbin/grub-install.

I think the actual bootloader (any of them those worked before) should be reinstalled by guix system delete-generations too, but apparently it doesn't do it right now.
Sounds very dangerous.  Doesn't that mean if one changed bootloaders in the past and then keeps using guix system delete-generations, that one eventually couldn't boot anymore?  O_O

> Isn’t the actual problem for an older running system generation to know which bootloder is currently in use? I think this can't be inferred by the currently running system generation. It may happen, that you use a brand new bootloader which is not known by the older system generation you just switched to.

I think guix-the-package-manager is still the newest one even after selecting an older system generation.

So the "brand new bootloader" case should be fine.
But the delete-generation case basically would have had to do the actual bootloader installation too.  Like it is now, it totally has a huge problem.

A possible way around having to know which bootloader is in use would be to just always install the configurations for all the known bootloaders.

> But still then, if you invoke a 'sudo -E guix system delete-generations' or a 'sudo -E guix system reconfigure' I think you still use the very latest guix version that you 'guix pull'-ed last.

Yes.

> And that guix version should still know all brand new bootloader. The problem may “only” be to know for 'sudo -E guix system delete-generations' which one to use.

Yep.

> But actually the bootloader-name field in /var/guix/profiles/system/parameters can't tell either, as it must be an older bootloader than the brand new one.

Correct.

> Maybe the information about the bootloader version in use needs to reside with the installed bootloader somewhere below /boot/efi/…? But this may be impossible for the legacy grub-bootloader.

That sounds like a huge can of worms to open.  Better would be some kind of bootloader detector (can package "os-prober" do it maybe?)--or better yet, just also install the bootloader each time a system generation is deleted and/or system in reconfigured.  That was the original plan.

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

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Danny Milosavljevic]

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

> > But actually the bootloader-name field in /var/guix/profiles/system/parameters can't tell either, as it must be an older bootloader than the brand new one.  

Yes, but as the comment in the source says, the goal is to be able to boot at all, not be perfect.

So basically we could assume that if a previous system generation has used a bootloader [name] (and we can still apparently run guix commands) then that bootloader actually could boot that system, and presumably can boot it again if we installed it now.

Might make sense to use the newest generation possible for that.  (I think even if you used the boot menu or guix system switch-generation, the newest generation would still be present, just not current; so if that used the bootloader name of the newest generation that exists we might be safe)

All this complication is just because we don't have the file name of the os configuration (/etc/config.scm) in guix system delete-generations.  What about us just requiring it from the user and then installing the bootloader&config that is specified there?  That would be a lot less magical--it basically would do the same that guix system reconfigure does now, then.

(What about guix system switch-generation?  What does it do regarding bootloader installation? *mumbles* *checks*  Aha, it also only installs the bootloader configuration)

guix/scripts/system.scm says:

>         (bootloader (lookup-bootloader-by-name (system-bootloader-name)))

What is that (system-bootloader-name) magical parameterless call?  Which generation does that use?  *shakes head*

Our source code there totally could use some more comments...

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

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Stefan]

Hi Danny!

> Am 24.05.2020 um 15:58 schrieb Danny Milosavljevic <dannym@scratchpost.org>:
> 
> All this complication is just because we don't have the file name of the os configuration (/etc/config.scm) in guix system delete-generations.

Hm, somehow true.

> What about us just requiring it from the user and then installing the bootloader&config that is specified there?  That would be a lot less magical--it basically would do the same that guix system reconfigure does now, then.

But it would mean to always install the bootloader without actual need, which is a point of failure. However, this is the case for reconfigure, too.


Bye

Stefan

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Stefan]

Hi Danny!

> Am 24.05.2020 um 15:42 schrieb Danny Milosavljevic <dannym@scratchpost.org>:
> 
> You are totally right O_O
> 
> reinstall-bootloader says:
> 
>>         ;; Only install bootloader configuration file.
> 
> What happened here?  Why?!

From my point of o newbie view nothing, this has ever been so. :-)

> I think the actual bootloader (any of them those worked before) should be reinstalled by guix system delete-generations too, but apparently it doesn't do it right now.
> Sounds very dangerous.  Doesn't that mean if one changed bootloaders in the past and then keeps using guix system delete-generations, that one eventually couldn't boot anymore?  O_O

I think this may not be needed. The task of the bootloader is to parse the generated configuration file and load the initrd and kernel. If the current latest bootloader is able do this, than all is fine. The loaded system generation does not matter for this.

Why should 'guix system switch-generation' or 'guix system delete-generations' brake anything here, as long as the generated configuration file is still readable by the bootloader in use?

Ah, now I got your point! An older generation now potentially has a different value in the bootloader-name field of the <boot-parameters> record. Then it will generate the wrong bootloader configuration file, e.g. for extlinux instead of for grub-efi. 

Well, the bootloader-name information of an older system generation does not help.

> So the "brand new bootloader" case should be fine.
> But the delete-generation case basically would have had to do the actual bootloader installation too.  Like it is now, it totally has a huge problem.
> 
> A possible way around having to know which bootloader is in use would be to just always install the configurations for all the known bootloaders.

Hm, not sure.

>> Maybe the information about the bootloader version in use needs to reside with the installed bootloader somewhere below /boot/efi/…? But this may be impossible for the legacy grub-bootloader.
> 
> That sounds like a huge can of worms to open.  Better would be some kind of bootloader detector (can package "os-prober" do it maybe?)--or better yet, just also install the bootloader each time a system generation is deleted and/or system is reconfigured.  That was the original plan.

Some information from the installed bootloader could very well be passed via kernel arguments. This way the bootloader would actively spread information about itself and this will work with any bootloader. 

We could use for example these two kernel arguments:

--bootloader-configuration-file-generator=grub-configuration-file --bootloader-configuration-file=/boot/grub.cfg

If we – like today – do not reinstall the bootloader when switching generations, these are the needed information: how and where to generate the configuration file.

If the guix in $HOME does not know the passed generator procedure, then it there would automatically be a fallback: /var/guix/profiles/system/parameters contains a kernel-arguments field already! That would contain a matching generator function then. If the generator is neither in kernel-arguments, then we could fall-back to the bootloader-name field and even reinstall the bootloader.

As it is today, the bootloader is a state. And to reduce the risk of failures (power-loss), it makes sense to install the bootloader only if absolutely necessary.


Bye

Stefan

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Stefan]

Hi Mathieu!

I changed my mind. I think it is better in a first step to keep the convention to have a fixed /boot/grub/grub.cfg. Therefore I create another link at /<target>/efi/boot/grub.cfg pointing to that file. Then there is no trouble with the <boot-parameters> yet, and the grub-efi-net-bootloader doesn't need to know about the target directory before installation.

I removed the macro as well, hence fixing the path to efi/boot. To boot different systems – because of the grub.cfg link – there needs to be some redirection via PXE or DNS on some higher level. With the macro you could have several /<target>/efi/Guix<n> directories on the same tftp server, but their grub.cfg link would still all point to the same location: ../../../boot/grub/grub. So the macro makes no sense anymore.


Bye

Stefan

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Stefan]

* gnu/bootloader/grub.scm (grub-efi-net-bootloader): New efi bootloader for
network booting via tftp/nfs.
(install-grub-efi-net): New bootloader installer for tftp.
(grub-root-search): Adding support for tftp root.
(eye-candy): Use 'gfxterm' for all systems if selected via 'terminal-outputs'.
* gnu/system.scm (read-boot-parameters): Prevent devices with ":/" from being
treated as a file system label.
* gnu/build/linux-boot.scm (device-string->file-system-device): Prevent devices
with ":/" from being treated as a file system label.
---
 gnu/bootloader/grub.scm  | 127 +++++++++++++++++++++++++++------------
 gnu/build/linux-boot.scm |   1 +
 gnu/system.scm           |   3 +-
 3 files changed, 93 insertions(+), 38 deletions(-)

diff --git a/gnu/bootloader/grub.scm b/gnu/bootloader/grub.scm
index 2d9a39afc3..60b3a12037 100644
--- a/gnu/bootloader/grub.scm
+++ b/gnu/bootloader/grub.scm
@@ -23,7 +23,7 @@
 
 (define-module (gnu bootloader grub)
   #:use-module (guix records)
-  #:use-module ((guix utils) #:select (%current-system))
+  #:use-module ((guix utils) #:select (%current-system %current-target-system))
   #:use-module (guix gexp)
   #:use-module (gnu artwork)
   #:use-module (gnu bootloader)
@@ -47,6 +47,8 @@
 
             grub-bootloader
             grub-efi-bootloader
+            make-grub-efi-net-bootloader
+            grub-efi-net-bootloader
             grub-mkrescue-bootloader
             grub-minimal-bootloader
 
@@ -135,41 +137,25 @@ file with the resolution provided in CONFIG."
            (_ #f)))))
 
 (define* (eye-candy config store-device store-mount-point
-                    #:key store-directory-prefix system port)
+                    #:key store-directory-prefix port)
   "Return a gexp that writes to PORT (a port-valued gexp) the 'grub.cfg' part
 concerned with graphics mode, background images, colors, and all that.
 STORE-DEVICE designates the device holding the store, and STORE-MOUNT-POINT is
 its mount point; these are used to determine where the background image and
-fonts must be searched for.  SYSTEM must be the target system string---e.g.,
-\"x86_64-linux\".  STORE-DIRECTORY-PREFIX is a directory prefix to prepend to
-any store file name."
-  (define setup-gfxterm-body
-    (let ((gfxmode
-           (or (and-let* ((theme (bootloader-configuration-theme config))
-                          (gfxmode (grub-theme-gfxmode theme)))
-                 (string-join gfxmode ";"))
-               "auto")))
-
-      ;; Intel and EFI systems need to be switched into graphics mode, whereas
-      ;; most other modern architectures have no other mode and therefore
-      ;; don't need to be switched.
-
-      ;; XXX: Do we really need to restrict to x86 systems?  We could imitate
-      ;; what the GRUB default configuration does and decide based on whether
-      ;; a user provided 'gfxterm' in the terminal-outputs field of their
-      ;; bootloader-configuration record.
-      (if (string-match "^(x86_64|i[3-6]86)-" system)
-          (format #f "
-  set gfxmode=~a
-  insmod all_video
-  insmod gfxterm~%" gfxmode)
-          "")))
-
+fonts must be searched for.  STORE-DIRECTORY-PREFIX is a directory prefix to
+prepend to any store file name."
   (define (setup-gfxterm config font-file)
     (if (memq 'gfxterm (bootloader-configuration-terminal-outputs config))
-        #~(format #f "if loadfont ~a; then
-  setup_gfxterm
-fi~%" #+font-file)
+        #~(format #f "
+if loadfont ~a; then
+  set gfxmode=~a
+  insmod all_video
+  insmod gfxterm
+fi~%"
+                  #$font-file
+                  #$(string-join
+                      (grub-theme-gfxmode (bootloader-theme config))
+                      ";"))
         ""))
 
   (define (theme-colors type)
@@ -190,8 +176,6 @@ fi~%" #+font-file)
 
   (and image
        #~(format #$port "
-function setup_gfxterm {~a}
-
 # Set 'root' to the partition that contains /gnu/store.
 ~a
 
@@ -206,7 +190,6 @@ else
   set menu_color_normal=cyan/blue
   set menu_color_highlight=white/blue
 fi~%"
-                 #$setup-gfxterm-body
                  #$(grub-root-search store-device font-file)
                  #$(setup-gfxterm config font-file)
                  #$(grub-setup-io config)
@@ -313,6 +296,9 @@ code."
         ((? file-system-label? label)
          (format #f "search --label --set ~a"
                  (file-system-label->string label)))
+        ((? (lambda (device)
+              (and (string? device) (string-contains device ":/"))) nfs-uri)
+         "set root=(tftp)")
         ((or #f (? string?))
          #~(format #f "search --file --set ~a" #$file)))))
 
@@ -358,7 +344,6 @@ when booting a root file system on a Btrfs subvolume."
                (menu-entry-device (first all-entries))
                (menu-entry-device-mount-point (first all-entries))
                #:store-directory-prefix store-directory-prefix
-               #:system system
                #:port #~port))
 
   (define keyboard-layout-config
@@ -498,11 +483,73 @@ fi~%"))))
                       "--bootloader-id=Guix"
                       "--efi-directory" target-esp))))
 
+(define (install-grub-efi-net subdir)
+  "Define a grub-efi bootloader installer for installation in SUBDIR,
+which is usually \"efi/boot\" or \"efi/Guix\"."
+  (let* ((arch (car (string-split (or (%current-target-system)
+                                      (%current-system))
+                                  #\-)))
+         (efi-bootloader-link (string-append "/boot"
+                                             (match arch
+                                               ("i686" "ia32")
+                                               ("x86_64" "x64")
+                                               ("arm" "arm")
+                                               ("armhf" "arm")
+                                               ("aarch64" "aa64")
+                                               ("riscv" "riscv32")
+                                               ("riscv64" "riscv64"))
+                                             ".efi"))
+         (efi-bootloader (string-append (match arch
+                                          ("i686" "i386")
+                                          ("x86_64" "x86_64")
+                                          ("arm" "arm")
+                                          ("armhf" "arm")
+                                          ("aarch64" "arm64")
+                                          ("riscv" "riscv32")
+                                          ("riscv64" "riscv64"))
+                                        "-efi/core.efi")))
+    #~(lambda (bootloader target mount-point)
+        "Install GRUB as e.g. \"bootx64.efi\" or \"bootarm64.efi\" \"into
+SUBDIR, which is usually \"efi/boot\" or \"efi/Guix\" below the directory TARGET
+for the system whose root is mounted at MOUNT-POINT."
+        (let* (;; Use target-depth and subdir-depth to construct links to
+               ;; "../gnu" and "../../../boot/grub/grub.cfg" with the correct
+               ;; number of "../". Note: This doesn't consider ".." or ".",
+               ;; which may appear inside target or subdir.
+               (target-depth (length (delete "" (string-split target #\/))))
+               (subdir-depth (length (delete "" (string-split #$subdir #\/))))
+               (up1 (string-join (make-list target-depth "..") "/" 'suffix))
+               (up2 (string-join (make-list subdir-depth "..") "/" 'suffix))
+               (net-dir (string-append mount-point target "/"))
+               (store-name (car (delete "" (string-split bootloader #\/))))
+               (store (string-append up1 store-name))
+               (store-link (string-append net-dir store-name))
+               (grub-cfg (string-append up1 up2 "boot/grub/grub.cfg"))
+               (grub-cfg-link (string-append net-dir #$subdir "/grub.cfg"))
+               (efi-bootloader-link
+                (string-append net-dir #$subdir #$efi-bootloader-link)))
+          ;; Tell 'grub-install' that there might be a LUKS-encrypted /boot or
+          ;; root partition.
+          (setenv "GRUB_ENABLE_CRYPTODISK" "y")
+          (invoke/quiet (string-append bootloader "/bin/grub-mknetdir")
+                        (string-append "--net-directory=" net-dir)
+                        (string-append "--subdir=" #$subdir))
+          (false-if-exception (delete-file store-link))
+          (symlink store store-link)
+          (false-if-exception (delete-file grub-cfg-link))
+          (symlink grub-cfg grub-cfg-link)
+          (false-if-exception (delete-file efi-bootloader-link))
+          (symlink #$efi-bootloader efi-bootloader-link)))))
+
 ^L
 
 ;;;
 ;;; Bootloader definitions.
 ;;;
+;;; For all these grub-bootloader variables the path to /boot/grub/grub.cfg
+;;; is fixed.  Inheriting and overwriting the field 'configuration-file' will
+;;; break 'guix system delete-generations', 'guix system switch-generation',
+;;; and 'guix system roll-back'.
 
 (define grub-bootloader
   (bootloader
@@ -513,12 +560,12 @@ fi~%"))))
    (configuration-file "/boot/grub/grub.cfg")
    (configuration-file-generator grub-configuration-file)))
 
-(define* grub-minimal-bootloader
+(define grub-minimal-bootloader
   (bootloader
    (inherit grub-bootloader)
    (package grub-minimal)))
 
-(define* grub-efi-bootloader
+(define grub-efi-bootloader
   (bootloader
    (inherit grub-bootloader)
    (installer install-grub-efi)
@@ -526,7 +573,13 @@ fi~%"))))
    (name 'grub-efi)
    (package grub-efi)))
 
-(define* grub-mkrescue-bootloader
+(define grub-efi-net-bootloader
+  (bootloader
+   (inherit grub-efi-bootloader)
+   (name 'grub-efi-net-bootloader)
+   (installer (install-grub-efi-net "efi/boot"))))
+
+(define grub-mkrescue-bootloader
   (bootloader
    (inherit grub-efi-bootloader)
    (package grub-hybrid)))
diff --git a/gnu/build/linux-boot.scm b/gnu/build/linux-boot.scm
index f08bb11514..3a79cfd461 100644
--- a/gnu/build/linux-boot.scm
+++ b/gnu/build/linux-boot.scm
@@ -503,6 +503,7 @@ upon error."
     ;; string, but the string can represent a device, a UUID, or a
     ;; label.  So check for all three.
     (cond ((string-prefix? "/" device-string) device-string)
+          ((string-contains device-string ":/") device-string)
           ((uuid device-string) => identity)
           (else (file-system-label device-string))))
 
diff --git a/gnu/system.scm b/gnu/system.scm
index ac8bbd1d16..91caba7012 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -301,7 +301,8 @@ file system labels."
       ((? string? device)
        ;; It used to be that we would not distinguish between labels and
        ;; device names.  Try to infer the right thing here.
-       (if (string-prefix? "/dev/" device)
+       (if (or (string-prefix? "/dev/" device)
+               (string-contains device ":/")) ; nfs
            device
            (file-system-label device)))))
 
-- 
2.26.0

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Danny Milosavljevic]

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

Hi Stefan,

thanks for the patch!

Could you create a new record type (something like <nfs-mount>)
and use that instead of magical ":/" where possible (i.e. everywhere except
gnu/build/linux-boot.scm)?

See also ./gnu/system/file-systems.scm file-system-label for how that is done
(just copy the record and the serializer for the initrd parameter and adapt it).

I know that previously we did magical string parsing, but I'd like not to add
to it.

Also, is the eye-candy change necessary?  I'm not opposed to it, but it
would be nicer in an extra patch (separation of concerns), if necessary in
this patch series.

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

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Danny Milosavljevic]

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

Hi Stefan,

On Tue, 9 Jun 2020 14:16:18 +0200
Stefan <stefan-guix@vodafonemail.de> wrote:

> I made your requested change (using <nfs-share>), but when trying a 'guix system reconfigure …' I only get this error:
> 
> guix system: error: #<nfs-share ":/volume5/guix-system">: invalid G-expression input
> 
> There is no backtrace, no nothing. I can’t figure out, which part of the code tries to read this serialisation. Do you have a clue?

I think it's a problem in the transfer of the record from host side to build
side, in this case mostly via Linux kernel command line arguments, which are
strings.

The host side code can use these records, but eventually the build side code
has to get a string and reconstruct it.

I don't know how to debug it except for running the translation in my head.
The error reporting facility could be improved a lot :(

gnu/build/linux-boot.scm is build side.  It runs when the system boots.

In there it has 

  (define (device-string->file-system-device device-string)
    ;; The "--root=SPEC" kernel command-line option always provides a
    ;; string, but the string can represent a device, a UUID, a
    ;; label or a NFS spec.  So check for all three.
    (cond ((string-prefix? "/" device-string) device-string)
          ((uuid device-string) => identity)
          (else (file-system-label device-string))))

But looking at the condition (uuid device-string) I have no idea what that means,
or is bound to!

It was introduced by:

commit 281d80d8e547fe663aaacb3226119166dd3100f9
Author: Maxim Cournoyer <maxim.cournoyer@gmail.com>
Date:   Tue Feb 11 14:00:06 2020 -0500

    linux-boot: Refactor boot-system.
    
    The --root option can now be omitted, and inferred from the root file system
    declaration instead.
    
    * gnu/build/file-systems.scm (canonicalize-device-spec): Extend to support NFS
    directly, and...
    * gnu/build/linux-boot.scm (boot-system): ...remove NFS special casing from
    here.  Remove nested definitions for root-fs-type, root-fs-flags and
    root-fs-options, and bind those inside the let* instead.  Make "--root" take
    precedence over the device field string representation of the root file
    system.
    * doc/guix.texi (Initial RAM Disk): Document that "--root" can be left
    unspecified.

If "--root" is not specified, it should then pick up the root from the root
file system declaration. 

@Maxim: How can we use your commit to boot from NFS?  I see no way to leave
"--root" off in the first place for a regular user.  (Stefan's patch adds
support for network boot via tftp/nfs via grub efi net) 

Cheers,
   Danny

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

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Stefan]

Hi!

> Am 09.06.2020 um 15:44 schrieb Danny Milosavljevic <dannym@scratchpost.org>:
> 
> gnu/build/linux-boot.scm is build side.  It runs when the system boots.
> 
> In there it has 
> 
>  (define (device-string->file-system-device device-string)
>    ;; The "--root=SPEC" kernel command-line option always provides a
>    ;; string, but the string can represent a device, a UUID, a
>    ;; label or a NFS spec.  So check for all three.
>    (cond ((string-prefix? "/" device-string) device-string)
>          ((uuid device-string) => identity)
>          (else (file-system-label device-string))))

Yes, that code runs when the system boots. But in my case the error is thrown already during the reconfigure itself. The trouble must be way before. 


Bye

Stefan

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Maxim Cournoyer]

Hello Danny,

Danny Milosavljevic <dannym@scratchpost.org> writes:

> Hi Stefan,
>
> On Tue, 9 Jun 2020 14:16:18 +0200
> Stefan <stefan-guix@vodafonemail.de> wrote:
>
>> I made your requested change (using <nfs-share>), but when trying a 'guix system reconfigure …' I only get this error:
>> 
>> guix system: error: #<nfs-share ":/volume5/guix-system">: invalid G-expression input
>> 
>> There is no backtrace, no nothing. I can’t figure out, which part of the code tries to read this serialisation. Do you have a clue?
>
> I think it's a problem in the transfer of the record from host side to build
> side, in this case mostly via Linux kernel command line arguments, which are
> strings.
>
> The host side code can use these records, but eventually the build side code
> has to get a string and reconstruct it.
>
> I don't know how to debug it except for running the translation in my head.
> The error reporting facility could be improved a lot :(
>
> gnu/build/linux-boot.scm is build side.  It runs when the system boots.
>
> In there it has 
>
>   (define (device-string->file-system-device device-string)
>     ;; The "--root=SPEC" kernel command-line option always provides a
>     ;; string, but the string can represent a device, a UUID, a
>     ;; label or a NFS spec.  So check for all three.
>     (cond ((string-prefix? "/" device-string) device-string)
>           ((uuid device-string) => identity)
>           (else (file-system-label device-string))))
>
> But looking at the condition (uuid device-string) I have no idea what that means,
> or is bound to!

It means that if the device-string (a string as its name imply) contains
something that represent a UUID, return its corresponding UUID object.
`uuid' comes from (gnu system uuid).  Does that answer your question?

> It was introduced by:
>
> commit 281d80d8e547fe663aaacb3226119166dd3100f9
> Author: Maxim Cournoyer <maxim.cournoyer@gmail.com>
> Date:   Tue Feb 11 14:00:06 2020 -0500
>
>     linux-boot: Refactor boot-system.
>     
>     The --root option can now be omitted, and inferred from the root file system
>     declaration instead.
>     
>     * gnu/build/file-systems.scm (canonicalize-device-spec): Extend to support NFS
>     directly, and...
>     * gnu/build/linux-boot.scm (boot-system): ...remove NFS special casing from
>     here.  Remove nested definitions for root-fs-type, root-fs-flags and
>     root-fs-options, and bind those inside the let* instead.  Make "--root" take
>     precedence over the device field string representation of the root file
>     system.
>     * doc/guix.texi (Initial RAM Disk): Document that "--root" can be left
>     unspecified.
>
> If "--root" is not specified, it should then pick up the root from the root
> file system declaration. 
>
> @Maxim: How can we use your commit to boot from NFS?  I see no way to leave
> "--root" off in the first place for a regular user.  (Stefan's patch adds
> support for network boot via tftp/nfs via grub efi net) 

This commit just makes it so that if --root was ever removed from the
generated GRUB configuration, it'd still be able to find it
automatically.  It was added to be consistent with the fact that mount
options are automatically taken from the root file system without any
user option (and I initially had a rootflags user option but this was
deemed unnecessary at the time).  I'll probably add it back soon now
that I've found a valid use case for it (passing the 'degraded' mount
option for booting from a degraded Btrfs RAID is one).

Does it cause a problem for the NFS boot via 'grub efi net' (I know
nothing about it -- any link for a recommended reading?)  When booting
from NFS using the nfsroot Linux option, it's possible to specify a
'/dev/nfs' as the root kernel parameter.  /dev/nfs is not a real block
device, it's just a stub hinting the kernel that its root file system is
on NFS.  Perhaps that can be used?

Maxim

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Stefan]

Hi!

I found the root-cause of my problem: I missed a conversion in gnu/system.scm (device-sexp). Sorry for the noise.


Bye

Stefan

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Maxim Cournoyer]

Hi Stefan,

Stefan <stefan-guix@vodafonemail.de> writes:

> Hi!
>
> I found the root-cause of my problem: I missed a conversion in gnu/system.scm (device-sexp). Sorry for the noise.

No worries; thanks for working on bettering NFS support!

Maxim

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Danny Milosavljevic]

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

Hi Maxim,
Hi Stefan,

On Thu, 11 Jun 2020 00:21:11 -0400
Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:

> >   (define (device-string->file-system-device device-string)
> >     ;; The "--root=SPEC" kernel command-line option always provides a
> >     ;; string, but the string can represent a device, a UUID, a
> >     ;; label or a NFS spec.  So check for all three.
> >     (cond ((string-prefix? "/" device-string) device-string)
> >           ((uuid device-string) => identity)
> >           (else (file-system-label device-string))))
> >
> > But looking at the condition (uuid device-string) I have no idea what that means,
> > or is bound to!  
> 
> It means that if the device-string (a string as its name imply) contains
> something that represent a UUID, return its corresponding UUID object.
> `uuid' comes from (gnu system uuid).  Does that answer your question?

Oh!  I've looked at now it but I still don't get it.

How can that be a cond condition?

Or asked differently, what if device-string does not represent an uuid, how
come device-string->file-system-device does not enter the branch "=> identity" ?

> Does it cause a problem for the NFS boot via 'grub efi net' (I know
> nothing about it -- any link for a recommended reading?)

https://manpages.debian.org/testing/grub-common/grub-mknetdir.1.en.html

Stefan has written a patch supporting it for Guix.

(canonicalize-device-spec seems to expect a nfs share reference to be a
string, too.  Is that on purpose?  No <nfs-share> record?  That's kinda
weird when we even have records for device labels and uuids--but we don't
have them for something that's actually complicated to specify? [1] :) )

>  When booting
> from NFS using the nfsroot Linux option, it's possible to specify a
> '/dev/nfs' as the root kernel parameter.  /dev/nfs is not a real block
> device, it's just a stub hinting the kernel that its root file system is
> on NFS.  Perhaps that can be used?

Hmm maybe.  @Stefan?

Also, could we have a system test testing this stuff?  I can write the
actual test--but could you tell me how to use the functionality introduced
in this patch?

[1] Reference docs for nfsroot: https://www.kernel.org/doc/Documentation/filesystems/nfs/nfsroot.txt

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

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Stefan]

Hi Danny!

> (canonicalize-device-spec seems to expect a nfs share reference to be a
> string, too.  Is that on purpose?  No <nfs-share> record?  That's kinda
> weird when we even have records for device labels and uuids--but we don't
> have them for something that's actually complicated to specify? [1] :) )

I think you are already able today to specify a (file-system (type "nfs") (device "my-server:/srv/nfs/music") …). It is just not sufficient yet for an NFS root file system. And using this style is still possible.

Therefore I started to use a simple string for an NFS share, too. That canonicalize-device-spec expects a string is due to an older patch from me in (boot-system), which Maxim moved from there.

Actually I see a difference between an NFS share and a file system label and an UUID, and this is (file-system (type …) …). If you specify for example "ext4" or "btrfs" the ‘device’ field can still have multiple representations, whereas for the type "nfs" the device field can only have one representation.

>> When booting
>> from NFS using the nfsroot Linux option, it's possible to specify a
>> '/dev/nfs' as the root kernel parameter.  /dev/nfs is not a real block
>> device, it's just a stub hinting the kernel that its root file system is
>> on NFS.  Perhaps that can be used?
> 
> Hmm maybe.  @Stefan?

As explained in another e-mail, no, that’s not an option.

> Also, could we have a system test testing this stuff?  I can write the
> actual test--but could you tell me how to use the functionality introduced
> in this patch?

A system test would be great!

You need a TFTP server serving the content of some /boot… folder as its root, which gets filled by the new grub-efi-net bootloader. This folder name is specified by (bootloader-configuration (target …) …). So lets assume you will use /boot-tftp for this.

The bootloader installer will create a link /boot-tftp/gnu pointing to ../gnu, and a link /boot-tftp/efi/boot/grub.cfg pointing to ../../../boot/grub/grub.cfg. So the TFTP server needs to have access to the store and the grub.cfg through these links as well.

You need a DHCP server pointing the guix machine to the TFTP server and to the file to boot. I use dnsmasq for this. My relevant configuration lines are the “TFTP server name” option number 66 and the “Bootfile name” option 67:

dhcp-option-force=66,10.10.10.10
dhcp-option-force=67,efi/boot/bootaa64.efi

The dnsmasq program can be configured with 

enable-tftp
tftp-root=/srv/tftp/

to act as a TFTP server, too. This is probably preferable.

You need an NFS server serving the root file system.

And you need a guix machine, capable to boot via network. Possibly any UEFI system could work, I think using qemu is possible as well.

I’m using a Raspberry Pi 3b. It is not PXE compliant, but capable to boot via network. For example it can only handle an IP address but no hostname inside the “TFTP server name” option. After specific firmware blobs have been loaded, it is loading the U-Boot, which then acts as an UEFI implementation and uses PXE. However, I do not use special PXE functionality, but finally the U-Boot will try to load /efi/boot/bootaa64.efi via TFTP, which in fact is GRUB.

GRUB then loads more files via TFTP from /efi/boot, most interestingly /efi/boot/grub.cfg, which is a link to ../../../boot/grub/grub.cfg. And that file points GRUB to the store.

Up to here no NFS is involved at all. GRUB will load the initrd and start Linux. Then the guix system is running and will mount its root file system via NFS.

If it is possible to start qemu with just an initrd, a kernel, and kernel arguments, then it would be possible to avoid DHCP, TFTP, and GRUB. This might be a much simpler first step for a test, only requiring an NFS server.


Bye

Stefan

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Maxim Cournoyer]

Hello Danny,

Danny Milosavljevic <dannym@scratchpost.org> writes:

> Hi Maxim,
> Hi Stefan,
>
> On Thu, 11 Jun 2020 00:21:11 -0400
> Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
>> >   (define (device-string->file-system-device device-string)
>> >     ;; The "--root=SPEC" kernel command-line option always provides a
>> >     ;; string, but the string can represent a device, a UUID, a
>> >     ;; label or a NFS spec.  So check for all three.
>> >     (cond ((string-prefix? "/" device-string) device-string)
>> >           ((uuid device-string) => identity)
>> >           (else (file-system-label device-string))))
>> >
>> > But looking at the condition (uuid device-string) I have no idea what that means,
>> > or is bound to!  
>> 
>> It means that if the device-string (a string as its name imply) contains
>> something that represent a UUID, return its corresponding UUID object.
>> `uuid' comes from (gnu system uuid).  Does that answer your question?
>
> Oh!  I've looked at now it but I still don't get it.

Perhaps the '=>' syntax is the reason why? The the Guile Reference info
manual defines it as such:

     For the ‘=>’ clause type, EXPRESSION is
     evaluated and the resulting procedure is applied to the value of
     TEST.  The result of this procedure application is then the result
     of the ‘cond’-expression.

(uuid device-string) returns either #f or a uuid object.  So applying
the identity function to a uuid object yields that same object.

HTH!

Maxim

[bug#41820] [PATCH] file-systems: Add record type <nfs-share> for a file system device.

[Stefan]

Hi!

Now that the new record <nfs-share> is working and seeing the amount of changes to make this working, I get the impression that this is unnecessarily complicated.

In the end there will be the "--root=" option for the kernel, which is only a plain string. And most of the device record related functions in the end target to produce a string.

And then there is (device-string->file-system-device) in gnu/build/linux-boot.scm to convert this string back into a device record.

As long as this conversion to and from string is necessary, there is no real benefit in having different record types for (file-system (device …)), it could just be a string. Then there would only be the need for a simple parser function like (device-string->device-type) to determine the type of device to be used in places where the type matters.


Bye

Stefan

[bug#41820] [PATCH] file-systems: Add record type <nfs-share> for a file system device.

[Stefan]

Hi!

A friendly ping.

What about this patch? This is what has been requested. Can it be pushed or does someone have further requests?


Bye

Stefan

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Stefan]

Hi Maxim!

> Does it cause a problem for the NFS boot via 'grub efi net' (I know
> nothing about it -- any link for a recommended reading?)

No, that’s no problem.

Regarding ‘grub efi net’: Just take a look at the GRUB manual. Actually GRUB is not dealing with NFS at all, it is using TFTP to load files. Only Linux uses NFS to mount its root file system. 

> When booting
> from NFS using the nfsroot Linux option, it's possible to specify a
> '/dev/nfs' as the root kernel parameter.  /dev/nfs is not a real block
> device, it's just a stub hinting the kernel that its root file system is
> on NFS.  Perhaps that can be used?

These “root=/dev/nfs rootfstype=nfs nfsroot=… ip=…” kernel arguments only make sense if an initrd can be omitted. Either the initrd or such a root-nfs becomes the root file system at startup. As the guix system is currently relying on an initrd, this is not an option.

Further you would need to ensure that certain CONFIG_NFS…, CONFIG_IP… and even more options for your network interface are set to ‘=y’ to ensure that Linux can make use of an nfs-root.


Bye

Stefan

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Maxim Cournoyer]

Hello Stefan!

Stefan <stefan-guix@vodafonemail.de> writes:

[...]

> Regarding ‘grub efi net’: Just take a look at the GRUB
> manual. Actually GRUB is not dealing with NFS at all, it is using TFTP
> to load files. Only Linux uses NFS to mount its root file system.

I see. Then we're talking about TFTP support in GRUB, and it doesn't
seem to depend on EFI at all (which is good!).

>> When booting
>> from NFS using the nfsroot Linux option, it's possible to specify a
>> '/dev/nfs' as the root kernel parameter.  /dev/nfs is not a real block
>> device, it's just a stub hinting the kernel that its root file system is
>> on NFS.  Perhaps that can be used?
>
> These “root=/dev/nfs rootfstype=nfs nfsroot=… ip=…” kernel arguments
> only make sense if an initrd can be omitted. Either the initrd or such
> a root-nfs becomes the root file system at startup. As the guix system
> is currently relying on an initrd, this is not an option.

I see. Thanks for explaining, I understand the plan better now.

> Further you would need to ensure that certain CONFIG_NFS…, CONFIG_IP…
> and even more options for your network interface are set to ‘=y’ to
> ensure that Linux can make use of an nfs-root.

Yes. For having done it recently, enabling the NFS support in the kernel
doesn't require much change, but then supporting a vast array of network
cards directly in the kernel doesn't sound too appealing. IIUC, going
through an initrd allows dynamically loading kernel modules instead of
having them statically built in the kernel (so that a user could add a
network module of their choice to their OS declaration without having to
rebuild the default kernel), which is why it's better to go that route
in Guix.

Am I understanding things correctly?

Thanks!

Maxim

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Stefan]

Hi Maxim!

> I see. Then we're talking about TFTP support in GRUB, and it doesn't
> seem to depend on EFI at all (which is good!).

Well, the data transfer in GRUB is using TFTP, but the actual network driver, the facilities to start a kernel and so on, happens by GRUB via the EFI-API in my case, which is provided by the U-Boot.

It may be a runtime detection, if the EFI-API or the legacy BIOS functions are used. I don’t know.

In that case I should probably rename grub-efi-net to grub-net.

> Yes. For having done it recently, enabling the NFS support in the kernel
> doesn't require much change, but then supporting a vast array of network
> cards directly in the kernel doesn't sound too appealing. IIUC, going
> through an initrd allows dynamically loading kernel modules instead of
> having them statically built in the kernel (so that a user could add a
> network module of their choice to their OS declaration without having to
> rebuild the default kernel), which is why it's better to go that route
> in Guix.
> 
> Am I understanding things correctly?

Yes. And additionally as a beginner I simply don’t want to try to remove the initrd, I can’t even imagine the effort to do so. :-)


Bye

Stefan

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Stefan]

* gnu/bootloader/grub.scm (grub-net-bootloader): New bootloader for
network booting via tftp.
(install-grub-net): New bootloader installer for tftp.
---
 gnu/bootloader/grub.scm | 79 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 75 insertions(+), 4 deletions(-)

diff --git a/gnu/bootloader/grub.scm b/gnu/bootloader/grub.scm
index b905ae360c..8f078dc2ac 100644
--- a/gnu/bootloader/grub.scm
+++ b/gnu/bootloader/grub.scm
@@ -24,7 +24,7 @@
 
 (define-module (gnu bootloader grub)
   #:use-module (guix records)
-  #:use-module ((guix utils) #:select (%current-system))
+  #:use-module ((guix utils) #:select (%current-system %current-target-system))
   #:use-module (guix gexp)
   #:use-module (gnu artwork)
   #:use-module (gnu bootloader)
@@ -46,8 +46,11 @@
             grub-theme-color-highlight
             grub-theme-gfxmode
 
+            install-grub-net
+
             grub-bootloader
             grub-efi-bootloader
+            grub-net-bootloader
             grub-mkrescue-bootloader
             grub-minimal-bootloader
 
@@ -501,11 +504,73 @@ fi~%"))))
                       "--bootloader-id=Guix"
                       "--efi-directory" target-esp))))
 
+(define (install-grub-net subdir)
+  "Define a grub-net bootloader installer for installation in SUBDIR,
+which is usually \"efi/boot\" or \"efi/Guix\"."
+  (let* ((arch (car (string-split (or (%current-target-system)
+                                      (%current-system))
+                                  #\-)))
+         (efi-bootloader-link (string-append "/boot"
+                                             (match arch
+                                               ("i686" "ia32")
+                                               ("x86_64" "x64")
+                                               ("arm" "arm")
+                                               ("armhf" "arm")
+                                               ("aarch64" "aa64")
+                                               ("riscv" "riscv32")
+                                               ("riscv64" "riscv64"))
+                                             ".efi"))
+         (efi-bootloader (string-append (match arch
+                                          ("i686" "i386")
+                                          ("x86_64" "x86_64")
+                                          ("arm" "arm")
+                                          ("armhf" "arm")
+                                          ("aarch64" "arm64")
+                                          ("riscv" "riscv32")
+                                          ("riscv64" "riscv64"))
+                                        "-efi/core.efi")))
+    #~(lambda (bootloader target mount-point)
+        "Install GRUB as e.g. \"bootx64.efi\" or \"bootarm64.efi\" \"into
+SUBDIR, which is usually \"efi/boot\" or \"efi/Guix\" below the directory TARGET
+for the system whose root is mounted at MOUNT-POINT."
+        (let* (;; Use target-depth and subdir-depth to construct links to
+               ;; "../gnu" and "../../../boot/grub/grub.cfg" with the correct
+               ;; number of "../". Note: This doesn't consider ".." or ".",
+               ;; which may appear inside target or subdir.
+               (target-depth (length (delete "" (string-split target #\/))))
+               (subdir-depth (length (delete "" (string-split #$subdir #\/))))
+               (up1 (string-join (make-list target-depth "..") "/" 'suffix))
+               (up2 (string-join (make-list subdir-depth "..") "/" 'suffix))
+               (net-dir (string-append mount-point target "/"))
+               (store-name (car (delete "" (string-split bootloader #\/))))
+               (store (string-append up1 store-name))
+               (store-link (string-append net-dir store-name))
+               (grub-cfg (string-append up1 up2 "boot/grub/grub.cfg"))
+               (grub-cfg-link (string-append net-dir #$subdir "/grub.cfg"))
+               (efi-bootloader-link
+                (string-append net-dir #$subdir #$efi-bootloader-link)))
+          ;; Tell 'grub-install' that there might be a LUKS-encrypted /boot or
+          ;; root partition.
+          (setenv "GRUB_ENABLE_CRYPTODISK" "y")
+          (invoke/quiet (string-append bootloader "/bin/grub-mknetdir")
+                        (string-append "--net-directory=" net-dir)
+                        (string-append "--subdir=" #$subdir))
+          (false-if-exception (delete-file store-link))
+          (symlink store store-link)
+          (false-if-exception (delete-file grub-cfg-link))
+          (symlink grub-cfg grub-cfg-link)
+          (false-if-exception (delete-file efi-bootloader-link))
+          (symlink #$efi-bootloader efi-bootloader-link)))))
+
 ^L
 
 ;;;
 ;;; Bootloader definitions.
 ;;;
+;;; For all these grub-bootloader variables the path to /boot/grub/grub.cfg
+;;; is fixed.  Inheriting and overwriting the field 'configuration-file' will
+;;; break 'guix system delete-generations', 'guix system switch-generation',
+;;; and 'guix system roll-back'.
 
 (define grub-bootloader
   (bootloader
@@ -516,12 +581,12 @@ fi~%"))))
    (configuration-file "/boot/grub/grub.cfg")
    (configuration-file-generator grub-configuration-file)))
 
-(define* grub-minimal-bootloader
+(define grub-minimal-bootloader
   (bootloader
    (inherit grub-bootloader)
    (package grub-minimal)))
 
-(define* grub-efi-bootloader
+(define grub-efi-bootloader
   (bootloader
    (inherit grub-bootloader)
    (installer install-grub-efi)
@@ -529,7 +594,13 @@ fi~%"))))
    (name 'grub-efi)
    (package grub-efi)))
 
-(define* grub-mkrescue-bootloader
+(define grub-net-bootloader
+  (bootloader
+   (inherit grub-efi-bootloader)
+   (name 'grub-net-bootloader)
+   (installer (install-grub-net "efi/Guix"))))
+
+(define grub-mkrescue-bootloader
   (bootloader
    (inherit grub-efi-bootloader)
    (package grub-hybrid)))
-- 
2.26.0

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Stefan]

Hi!

This debbugs thread got already very long. Therefore I would like to focus on the grub changes in this ticket done with my last patch, see <https://debbugs.gnu.org/cgi/bugreport.cgi?bug=41011#95>

This patch only introduces a new grub-net bootloader, which basically installs grub via its grub-mknetdir command.

There is no modification of other bootloaders. The restriction of the “/boot/grub/grub.cfg” file remains.

As usually the DHCP option 67 “Bootfile name” is involved to point the EFI of a machine to the file to boot via TFTP, I chose to export (install-grub-net subdir) as well to be able to modify that path, whose default is /boot/efi/Guix/boot[x64|aa64|…].efi, to something else, e.g.

(bootloader (inherit grub-net-bootloader (installer (install-grub-net "efi/machine-1"))))

To be able to boot different machines with their own guix installation, however, there is still the problem to provide to each an own grub.cfg file via TFTP. This file – as you know – has still a hard coded path of /boot/grub/grub.cfg. But this is a different issue and should not be tackled with this patch.


Bye

Stefan

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Danny Milosavljevic]

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

Hi Stefan,

I think this looks good in general.

I'd like to do some nitpicking on the names--especially since the procedure is
exported and thus presumably can't have its signature modified later without
breaking backward compatibility.

In this case, the man page grub-mknetdir(8) mentions "netboot" ?
Do you think "net" or "netboot" is a better name for this functionality ?

On Sat, 5 Sep 2020 13:25:24 +0200
Stefan <stefan-guix@vodafonemail.de> wrote:

> +            install-grub-net

I'm fine with whatever--but the man page says "netboot".  If that's the usual
name, let's use it.  If "net"'s the usual name, let's use that.

> +  (let* ((arch (car (string-split (or (%current-target-system)
> +                                      (%current-system))
> +                                  #\-)))

Let's not use arcane Scheme anachronisms like "car".  I know most Scheme
programmers probably know what it does--but still, better not to use
names of registers of a machine no one uses anymore.

Better something like this:

(let* ((system-parts (string-split (or (%current-target-system)
                                       (%current-system))
                            #\-)))

> +         (efi-bootloader-link (string-append "/boot"

> +                                             (match arch
> +                                               ("i686" "ia32")
> +                                               ("x86_64" "x64")
> +                                               ("arm" "arm")
> +                                               ("armhf" "arm")
> +                                               ("aarch64" "aa64")
> +                                               ("riscv" "riscv32")
> +                                               ("riscv64" "riscv64"))
> +                                             ".efi"))

Also, I have a slight preference for greppable file names even when it's a
little more redundant, so more like that:

(match system-parts
 (("i686" _ ...) "ia32.efi")
 (("x86_64" _ ...) "x64.efi")
 (("arm" _ ...) "arm.efi")
 (("armhf" _ ...) "arm.efi")
 (("aarch64" _ ...) "aa64.efi")
 (("riscv" _ ...) "riscv32.efi")
 (("riscv64" _ ...) "riscv64.efi"))

> +         (efi-bootloader (string-append (match arch
> +                                          ("i686" "i386")
> +                                          ("x86_64" "x86_64")
> +                                          ("arm" "arm")
> +                                          ("armhf" "arm")
> +                                          ("aarch64" "arm64")
> +                                          ("riscv" "riscv32")
> +                                          ("riscv64" "riscv64"))
> +                                        "-efi/core.efi")))

Likewise:

         (efi-bootloader (match system-parts
                          (("i686" _ ...) "i386-efi/core.efi")
                          (("x86_64" _ ...) "x86_64-efi/core.efi")
                          (("arm" _ ...) "arm-efi/core.efi")
                          (("armhf" _ ...) "arm-efi/core.efi")
                          (("aarch64" _ ...) "arm64-efi/core.efi")
                          (("riscv" _ ...) "riscv32-efi/core.efi")
                          (("riscv64" _ ...) "riscv64-efi/core.efi"))))

> +    #~(lambda (bootloader target mount-point)

> +        "Install GRUB as e.g. \"bootx64.efi\" or \"bootarm64.efi\" \"into
> +SUBDIR, which is usually \"efi/boot\" or \"efi/Guix\" below the directory TARGET
> +for the system whose root is mounted at MOUNT-POINT."

I think you mean:

> +        "Install GRUB as e.g. \"bootx64.efi\" or \"bootarm64.efi\" \"into
> +SUBDIR (which is usually \"efi/boot\" or \"efi/Guix\") below the directory TARGET
> +for the system whose root is mounted at MOUNT-POINT."

> +        (let* (;; Use target-depth and subdir-depth to construct links to
> +               ;; "../gnu" and "../../../boot/grub/grub.cfg" with the correct
> +               ;; number of "../". Note: This doesn't consider ".." or ".",
> +               ;; which may appear inside target or subdir.

Uhhhh... that could use some more explanationary comments in the source code
of why it is done in the first place.

Also, is TARGET itself assumed to be an absolute path or is it relative to
something else ?  According to the rest of the patch it's relative to
MOUNT-POINT--but please state this explicitly in the docstring.

> +               (target-depth (length (delete "" (string-split target #\/))))

> +               (subdir-depth (length (delete "" (string-split #$subdir #\/))))

> +               (up1 (string-join (make-list target-depth "..") "/" 'suffix))

Maybe better name: escape-target or something.

> +               (up2 (string-join (make-list subdir-depth "..") "/" 'suffix))

Maybe better name: escape-subdir or something.

So this is in order to get out of (string-append TARGET "/" SUBDIR), correct?
Does the (string-append TARGET "/" SUBDIR) have an official name ?
If not, fine.

> +               (net-dir (string-append mount-point target "/"))

So TARGET is relative to MOUNT-POINT ?
And MOUNT-POINT is assumed to have a slash at the end ?

> +               (store-name (car (delete "" (string-split bootloader #\/))))

Maybe use match.

Also isn't there an official way to find out how the store is called ?
(%store-prefix) ?

> +               (store (string-append up1 store-name))

(string-append escape-target store-name)

> +               (store-link (string-append net-dir store-name))

*mumbles to self* (string-append MOUNT-POINT TARGET) is net-dir.
So it tries to get to (string-append MOUNT-POINT "/gnu").

I vaguely remember our docker pack adding some serious plumbing to support
symlinks like that.

I'll try to find it.  I just wanted to send this E-Mail because of the following:

>  ;;;
>  ;;; Bootloader definitions.
>  ;;;
> +;;; For all these grub-bootloader variables the path to /boot/grub/grub.cfg
> +;;; is fixed.  Inheriting and overwriting the field 'configuration-file' will
> +;;; break 'guix system delete-generations', 'guix system switch-generation',
> +;;; and 'guix system roll-back'.

I've added that comment to the source code in an extra commit
3f2bd9df410e85795ec656052f44d2cddec2a060 in guix master.
Thank you very much for it.

> -(define* grub-minimal-bootloader
> +(define grub-minimal-bootloader
>    (bootloader

> -(define* grub-efi-bootloader
> +(define grub-efi-bootloader
>    (bootloader

> -(define* grub-mkrescue-bootloader
> +(define grub-mkrescue-bootloader

I've applied this hunk to guix master as commit
8664c35d6d7fd6e9ce1ca8adefa8070a8e556db4.

Thanks.

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

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Danny Milosavljevic]

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

> I vaguely remember our docker pack adding some serious plumbing to support
> symlinks like that.

                       ((guix build union) #:select (relative-file-name))

symlink-relative in (guix build union):

  "Assuming both OLD and NEW are absolute file names, make NEW a symlink to
OLD, but using a relative file name."

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

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Stefan]

Hi Danny!

> In this case, the man page grub-mknetdir(8) mentions "netboot" ?
> Do you think "net" or "netboot" is a better name for this functionality ?

At <https://www.gnu.org/software/grub/manual/grub/grub.html> there is only a single hit for ‘netboot’ which is in “To generate a netbootable directory, run:”.

All GRUB variables and commands have the prefix ‘net_’.

But I agree, grub-netboot seems to be a more describing name.

In the end this is a grub-efi for booting over network. Would grub-efi-netboot be an even better name? It will not work with BIOS machines.

> Let's not use arcane Scheme anachronisms like "car".  I know most Scheme
> programmers probably know what it does--but still, better not to use
> names of registers of a machine no one uses anymore.
> 
> Better something like this:
> 
> (let* ((system-parts (string-split (or (%current-target-system)
>                                       (%current-system))
>                            #\-)))

I only need the first list element here. I will use (first …).

>> +         (efi-bootloader-link (string-append "/boot"
> 
>> +                                             (match arch
>> +                                               ("i686" "ia32")
>> +                                               ("x86_64" "x64")
>> +                                               ("arm" "arm")
>> +                                               ("armhf" "arm")
>> +                                               ("aarch64" "aa64")
>> +                                               ("riscv" "riscv32")
>> +                                               ("riscv64" "riscv64"))
>> +                                             ".efi"))
> 
> Also, I have a slight preference for greppable file names even when it's a
> little more redundant, so more like that:
> 
> (match system-parts
> (("i686" _ ...) "ia32.efi")
> (("x86_64" _ ...) "x64.efi")
> (("arm" _ ...) "arm.efi")
> (("armhf" _ ...) "arm.efi")
> (("aarch64" _ ...) "aa64.efi")
> (("riscv" _ ...) "riscv32.efi")
> (("riscv64" _ ...) "riscv64.efi"))

I’m not familiar with the match syntax yet. For me using the first element as arch seems simpler.

>> +         (efi-bootloader (string-append (match arch
>> +                                          ("i686" "i386")
>> +                                          ("x86_64" "x86_64")
>> +                                          ("arm" "arm")
>> +                                          ("armhf" "arm")
>> +                                          ("aarch64" "arm64")
>> +                                          ("riscv" "riscv32")
>> +                                          ("riscv64" "riscv64"))
>> +                                        "-efi/core.efi")))
> 
> Likewise:
> 
>         (efi-bootloader (match system-parts
>                          (("i686" _ ...) "i386-efi/core.efi")
>                          (("x86_64" _ ...) "x86_64-efi/core.efi")
>                          (("arm" _ ...) "arm-efi/core.efi")
>                          (("armhf" _ ...) "arm-efi/core.efi")
>                          (("aarch64" _ ...) "arm64-efi/core.efi")
>                          (("riscv" _ ...) "riscv32-efi/core.efi")
>                          (("riscv64" _ ...) "riscv64-efi/core.efi"))))

I’d prefer to keep the still grepable “/core.efi” separate.

>> +    #~(lambda (bootloader target mount-point)
> 
>> +        "Install GRUB as e.g. \"bootx64.efi\" or \"bootarm64.efi\" \"into
>> +SUBDIR, which is usually \"efi/boot\" or \"efi/Guix\" below the directory TARGET
>> +for the system whose root is mounted at MOUNT-POINT."
> 
> I think you mean:
> 
>> +        "Install GRUB as e.g. \"bootx64.efi\" or \"bootarm64.efi\" \"into
>> +SUBDIR (which is usually \"efi/boot\" or \"efi/Guix\") below the directory TARGET
>> +for the system whose root is mounted at MOUNT-POINT."

Yes.

>> +        (let* (;; Use target-depth and subdir-depth to construct links to
>> +               ;; "../gnu" and "../../../boot/grub/grub.cfg" with the correct
>> +               ;; number of "../". Note: This doesn't consider ".." or ".",
>> +               ;; which may appear inside target or subdir.
> 
> Uhhhh... that could use some more explanationary comments in the source code
> of why it is done in the first place.

I’ll put an explanation into the doc-string. This is because the grub.cfg and the store both need to be accessible to GRUB via TFTP, but the TFTP root is TARGET, which is usually /boot.

> Also, is TARGET itself assumed to be an absolute path or is it relative to
> something else ?  According to the rest of the patch it's relative to
> MOUNT-POINT--but please state this explicitly in the docstring.

TARGET is the (operating-system (boot-loader (target "/boot") …) …). I think this has to be an absolute path, but I didn’t find any checks for this. But the manual doesn’t mention this, just all examples are using absolute paths.

And yes, when using ‘guix system init /etc/config.scm /mnt/here’, then MOUNT-POINT and TARGET are concatenated. But this is nothing specific to the new installer, this is the usual behaviour of Guix and the reason for the two parameters TARGET and MOUNT-POINT to any bootloader installer. I don’t think stating this inside the new doc-string is the right place.

>> +               (target-depth (length (delete "" (string-split target #\/))))
> 
>> +               (subdir-depth (length (delete "" (string-split #$subdir #\/))))
> 
>> +               (up1 (string-join (make-list target-depth "..") "/" 'suffix))
> 
> Maybe better name: escape-target or something.
> 
>> +               (up2 (string-join (make-list subdir-depth "..") "/" 'suffix))
> 
> Maybe better name: escape-subdir or something.
> 
> So this is in order to get out of (string-append TARGET "/" SUBDIR), correct?

Yes, correct. I’ll rework this with the (symlink-relative) function you mentioned.

> Does the (string-append TARGET "/" SUBDIR) have an official name ?
> If not, fine.

No. The TARGET becomes the TFTP root for GRUB, the SUBDIR becomes the ‘prefix’ variable for GRUB.

>> +               (net-dir (string-append mount-point target "/"))
> 
> So TARGET is relative to MOUNT-POINT ?
> And MOUNT-POINT is assumed to have a slash at the end ?

MOUNT-POINT is either ‘/’ or depends on the argument to ‘guix system init’. On the other side TARGET has to be an absolute path, so it should be safe. At least (install-grub-efi) makes the same mistake. What do you think?

>> +               (store-name (car (delete "" (string-split bootloader #\/))))
> 
> Maybe use match.

I’ll use (first …).

> Also isn't there an official way to find out how the store is called ?
> (%store-prefix) ?

I only need the first path element to the store, which is usually /gnu. The %store-prefix contains /gnu/store then. So it makes no difference.

>> +               (store (string-append up1 store-name))
> 
> (string-append escape-target store-name)
> 
>> +               (store-link (string-append net-dir store-name))
> 
> *mumbles to self* (string-append MOUNT-POINT TARGET) is net-dir.
> So it tries to get to (string-append MOUNT-POINT "/gnu").

The trouble is that GRUB shall load a file like /gnu/store/…-linux…/Image via TFTP, but the TFTP root is actually Guix’ final /boot folder.

In the end this creates a relative symlink as ../gnu pointing from /mnt/here/boot/gnu to /mnt/here/gnu.

And GRUB’s “working directory” to search for its modules and the grub.cfg is defined by its ‘prefix’ variable, which is set through the SUBDIR argument, which defaults to Guix’ final /boot/efi/Guix.

This requires a relative symlink as ../../../boot/grub/grub.cfg pointing from /mnt/here/boot/efi/Guix/grub.cfg to /mnt/here/boot/grub/grub.cfg.

And be aware that TARGET may be /boot, but could be something else like /tftp-root. Then the symlink would point from /mnt/here/tftp-root/efi/Guix/grub.cfg to /mnt/here/boot/grub/grub.cfg, as the later is kind of hard-coded.

>> ;;; Bootloader definitions.
>> ;;;
>> +;;; For all these grub-bootloader variables the path to /boot/grub/grub.cfg
>> +;;; is fixed.  Inheriting and overwriting the field 'configuration-file' will
>> +;;; break 'guix system delete-generations', 'guix system switch-generation',
>> +;;; and 'guix system roll-back'.
> 
> I've added that comment to the source code in an extra commit
> 3f2bd9df410e85795ec656052f44d2cddec2a060 in guix master.
> Thank you very much for it.
> 
>> -(define* grub-minimal-bootloader
>> +(define grub-minimal-bootloader
>>   (bootloader
> 
>> -(define* grub-efi-bootloader
>> +(define grub-efi-bootloader
>>   (bootloader
> 
>> -(define* grub-mkrescue-bootloader
>> +(define grub-mkrescue-bootloader
> 
> I've applied this hunk to guix master as commit
> 8664c35d6d7fd6e9ce1ca8adefa8070a8e556db4.
> 
> Thanks.


Thanks!


Bye

Stefan

[bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

[Danny Milosavljevic]

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

Hi Stefan,

On Tue, 8 Sep 2020 00:59:38 +0200
Stefan <stefan-guix@vodafonemail.de> wrote:

> In the end this is a grub-efi for booting over network. 

>Would grub-efi-netboot be an even better name? It will not work with BIOS machines.

Oh, then definitely let's use that name.

> I only need the first list element here. I will use (first …).

Okay.

(I leave it to the others to comment on here if they have a problem with it--I
see no downside in this case)

> >> +         (efi-bootloader-link (string-append "/boot"  
> >   
> >> +                                             (match arch
> >> +                                               ("i686" "ia32")
> >> +                                               ("x86_64" "x64")
> >> +                                               ("arm" "arm")
> >> +                                               ("armhf" "arm")
> >> +                                               ("aarch64" "aa64")
> >> +                                               ("riscv" "riscv32")
> >> +                                               ("riscv64" "riscv64"))
> >> +                                             ".efi"))  
> > 
> > Also, I have a slight preference for greppable file names even when it's a
> > little more redundant, so more like that:
> > 
> > (match system-parts
> > (("i686" _ ...) "ia32.efi")
> > (("x86_64" _ ...) "x64.efi")
> > (("arm" _ ...) "arm.efi")
> > (("armhf" _ ...) "arm.efi")
> > (("aarch64" _ ...) "aa64.efi")
> > (("riscv" _ ...) "riscv32.efi")
> > (("riscv64" _ ...) "riscv64.efi"))  
> 
> I’m not familiar with the match syntax yet. For me using the first element as arch seems simpler.

Match just does pattern matching.  The pattern here is for example ("i686" _ ...).
That means it will match anything that is a list that is starting with "i686".
It will put the remainder (...) into the variable "_" (which is customary to
use as "don't care" variable).

The major advantage of using "match" is its failure mode.  If the thing matched
on is not a list (for some unfathomable reason) or if the first element is not
matched on (!) then you get an exception--which is much better than doing weird
unknown stuff.

You have used "match" before--but only on parts of the list.  Why not use it
on the whole list?  It makes little sense to do manual destructuring and then
use match--when match would have done the destructuring bind anyway.

> > Likewise:
> > 
> >         (efi-bootloader (match system-parts
> >                          (("i686" _ ...) "i386-efi/core.efi")
> >                          (("x86_64" _ ...) "x86_64-efi/core.efi")
> >                          (("arm" _ ...) "arm-efi/core.efi")
> >                          (("armhf" _ ...) "arm-efi/core.efi")
> >                          (("aarch64" _ ...) "arm64-efi/core.efi")
> >                          (("riscv" _ ...) "riscv32-efi/core.efi")
> >                          (("riscv64" _ ...) "riscv64-efi/core.efi"))))  
> 
> I’d prefer to keep the still grepable “/core.efi” separate.

Sure.

> And yes, when using ‘guix system init /etc/config.scm /mnt/here’, then MOUNT-POINT and TARGET are concatenated. But this is nothing specific to the new installer, this is the usual behaviour of Guix and the reason for the two parameters TARGET and MOUNT-POINT to any bootloader installer. I don’t think stating this inside the new doc-string is the right place.

Ah, so that's what it means.

Well, it should be stated *somewhere* at least.  It probably is and I just
didn't see it.

> Yes, correct. I’ll rework this with the (symlink-relative) function you mentioned.

Thanks!

> > So TARGET is relative to MOUNT-POINT ?
> > And MOUNT-POINT is assumed to have a slash at the end ?  
> 
> MOUNT-POINT is either ‘/’ or depends on the argument to ‘guix system init’. On the other side TARGET has to be an absolute path, so it should be safe. At least (install-grub-efi) makes the same mistake. What do you think?

If grub-efi does it then it seems to be fine to do it--at least we didn't get
bug reports caused by it.  Let's just keep using it for the time being.

> >> +               (store-name (car (delete "" (string-split bootloader #\/))))  
> > 
> > Maybe use match.  
> 
> I’ll use (first …).
> 
> > Also isn't there an official way to find out how the store is called ?
> > (%store-prefix) ?  
> 
> I only need the first path element to the store, which is usually /gnu. The %store-prefix contains /gnu/store then. So it makes no difference.

I have no strong opinion either way, except please add a comment that you
are extracting part of the store prefix (or whatever) from the in-store
name of the bootloader store item.  It seems weird to me to do that--but
then again I don't get why Guix has two directories (/gnu and /gnu/store)
to the store anyway.

Fine, I guess.

I'm not sure whether it would be technically possible to have a custom
store directory like "/foo" without "/gnu" as the store.  That would be
a problem--and I'm sure someone somewhere does that--otherwise, why have
%store-prefix as a variable otherwise?

> >> +               (store (string-append up1 store-name))  
> > 
> > (string-append escape-target store-name)
> >   
> >> +               (store-link (string-append net-dir store-name))  
> > 
> > *mumbles to self* (string-append MOUNT-POINT TARGET) is net-dir.
> > So it tries to get to (string-append MOUNT-POINT "/gnu").  
> 
> The trouble is that GRUB shall load a file like /gnu/store/…-linux…/Image via TFTP, but the TFTP root is actually Guix’ final /boot folder.
> 
> In the end this creates a relative symlink as ../gnu pointing from /mnt/here/boot/gnu to /mnt/here/gnu.
> 
> And GRUB’s “working directory” to search for its modules and the grub.cfg is defined by its ‘prefix’ variable, which is set through the SUBDIR argument, which defaults to Guix’ final /boot/efi/Guix.
> 
> This requires a relative symlink as ../../../boot/grub/grub.cfg pointing from /mnt/here/boot/efi/Guix/grub.cfg to /mnt/here/boot/grub/grub.cfg.
> 
> And be aware that TARGET may be /boot, but could be something else like /tftp-root. Then the symlink would point from /mnt/here/tftp-root/efi/Guix/grub.cfg to /mnt/here/boot/grub/grub.cfg, as the later is kind of hard-coded.

Please add that to comments in the source code.  Otherwise, it would be
very probable to be broken by further maintenance.

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

[bug#41011] [PATCH] gnu: grub: Support for network boot via TFTP.

[Stefan]

* gnu/bootloader/grub.scm (grub-efi-netboot-bootloader): New bootloader for
network booting.
(install-grub-efi-netboot): New bootloader installer for network booting.
(grub-root-search): Set the root to "(tftp)" if the searched file is not stored
on a local devices, i.e. an NFS share.
---
 gnu/bootloader/grub.scm | 97 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 96 insertions(+), 1 deletion(-)

diff --git a/gnu/bootloader/grub.scm b/gnu/bootloader/grub.scm
index e3febeefd0..552bc34f5a 100644
--- a/gnu/bootloader/grub.scm
+++ b/gnu/bootloader/grub.scm
@@ -23,8 +23,10 @@
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
 (define-module (gnu bootloader grub)
+  #:use-module (guix build union)
   #:use-module (guix records)
-  #:use-module ((guix utils) #:select (%current-system))
+  #:use-module (guix store)
+  #:use-module ((guix utils) #:select (%current-system %current-target-system))
   #:use-module (guix gexp)
   #:use-module (gnu artwork)
   #:use-module (gnu bootloader)
@@ -46,8 +48,11 @@
             grub-theme-color-highlight
             grub-theme-gfxmode
 
+            install-grub-efi-netboot
+
             grub-bootloader
             grub-efi-bootloader
+            grub-efi-netboot-bootloader
             grub-mkrescue-bootloader
             grub-minimal-bootloader
 
@@ -295,6 +300,9 @@ code."
         ((? file-system-label? label)
          (format #f "search --label --set ~a"
                  (file-system-label->string label)))
+        ((? (lambda (device)
+              (and (string? device) (string-contains device ":/"))) nfs-uri)
+         "set root=(tftp)")
         ((or #f (? string?))
          #~(format #f "search --file --set ~a" #$file)))))
 
@@ -501,6 +509,87 @@ fi~%"))))
                       "--bootloader-id=Guix"
                       "--efi-directory" target-esp))))
 
+(define (install-grub-efi-netboot subdir)
+  "Define a grub-efi-netboot bootloader installer for installation in SUBDIR,
+which is usually efi/Guix or efi/boot."
+  (let* ((system (string-split (or (%current-target-system)
+                                   (%current-system))
+                               #\-))
+         (boot-efi-link (match system
+                          (("i686" _ ...) "/bootia32.efi")
+                          (("x86_64" _ ...) "/bootx64.efi")
+                          (("arm" _ ...) "/bootarm.efi")
+                          (("armhf" _ ...) "/bootarm.efi")
+                          (("aarch64" _ ...) "/bootaa64.efi")
+                          (("riscv" _ ...) "/bootriscv32.efi")
+                          (("riscv64" _ ...) "/bootriscv64.efi")))
+         (efi-bootloader (string-append (match system
+                                          (("i686" _ ...) "i386-efi")
+                                          (("x86_64" _ ...) "x86_64-efi")
+                                          (("arm" _ ...) "arm-efi")
+                                          (("armhf" _ ...) "arm-efi")
+                                          (("aarch64" _ ...) "arm64-efi")
+                                          (("riscv" _ ...) "riscv32-efi")
+                                          (("riscv64" _ ...) "riscv64-efi"))
+                                        "/core.efi")))
+    (with-imported-modules
+     '((guix build union))
+     #~(lambda (bootloader target mount-point)
+         "Install the BOOTLOADER, which must be the package grub, as e.g.
+bootx64.efi or bootarm64.efi into SUBDIR, which is usually efi/Guix or efi/boot,
+below the directory TARGET for the system whose root is mounted at MOUNT-POINT.
+
+MOUNT-POINT is the last argument in 'guix system init /etc/config.scm mnt/point'
+or '/' for other 'guix system' commands.
+
+TARGET is the target argument given to the bootloader-configuration in
+(operating-system
+ (bootloader (bootloader-configuration
+              (target \"/boot\")
+              …))
+ …)
+TARGET is required to be an absolute path and must be provided by a TFTP server
+as the TFTP root directory.
+
+GRUB will load tftp://server/SUBDIR/grub.cfg and this file will instruct it to
+load more files from the store like tftp://server/gnu/store/…-linux…/Image.
+
+To make this possible two symlinks will be created. The first symlink points
+relatively form TARGET/SUBDIR/grub.cfg to /boot/grub/grub.cfg. And the second
+symlink points relatively from TARGET/%store-prefix to %store-prefix.
+
+It is important to note that these symlinks need to be relativ, as the absolute
+paths on the TFTP server side are unknown.
+
+It is also important to note that both symlinks will point outside the TFTP root
+directory and that the TARGET/%store-prefix symlink makes the whole store
+accessible via TFTP. Possibly the TFTP server must be configured
+to allow accesses outside its TFTP root directory. This may need to be
+considered for security aspects."
+         (use-modules ((guix build union) #:select (symlink-relative)))
+         (let* ((net-dir (string-append mount-point target "/"))
+                (sub-dir (string-append net-dir #$subdir "/"))
+                (store-link (string-append net-dir (%store-prefix)))
+                (grub-cfg "/boot/grub/grub.cfg")
+                (grub-cfg-link (string-append sub-dir (basename grub-cfg)))
+                (boot-efi-link (string-append sub-dir #$boot-efi-link)))
+           ;; Prepare the symlink to the store.
+           (mkdir-p (dirname store-link))
+           (false-if-exception (delete-file store-link))
+           (symlink-relative (%store-prefix) store-link)
+           ;; Prepare the symlink to the grub.cfg, which points into the store.
+           (false-if-exception (delete-file grub-cfg-link))
+           (symlink-relative grub-cfg grub-cfg-link)
+           ;; Install GRUB, which refers to the grub.cfg, with support for
+           ;; encrypted partitions,
+           (setenv "GRUB_ENABLE_CRYPTODISK" "y")
+           (invoke/quiet (string-append bootloader "/bin/grub-mknetdir")
+                         (string-append "--net-directory=" net-dir)
+                         (string-append "--subdir=" #$subdir))
+           ;; Prepare the bootloader symlink, which points to GRUB.
+           (false-if-exception (delete-file boot-efi-link))
+           (symlink #$efi-bootloader boot-efi-link))))))
+
 ^L
 
 ;;;
@@ -533,6 +622,12 @@ fi~%"))))
    (name 'grub-efi)
    (package grub-efi)))
 
+(define grub-efi-netboot-bootloader
+  (bootloader
+   (inherit grub-efi-bootloader)
+   (name 'grub-efi-netboot-bootloader)
+   (installer (install-grub-efi-netboot "efi/Guix"))))
+
 (define grub-mkrescue-bootloader
   (bootloader
    (inherit grub-efi-bootloader)
-- 
2.26.0

[bug#41011] [PATCH] gnu: grub: Support for network boot via TFTP.

[Efraim Flashner]

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

On Sun, Sep 13, 2020 at 07:46:01PM +0200, Stefan wrote:
> * gnu/bootloader/grub.scm (grub-efi-netboot-bootloader): New bootloader for
> network booting.
> (install-grub-efi-netboot): New bootloader installer for network booting.
> (grub-root-search): Set the root to "(tftp)" if the searched file is not stored
> on a local devices, i.e. an NFS share.
> ---
>  gnu/bootloader/grub.scm | 97 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 96 insertions(+), 1 deletion(-)
> 
> diff --git a/gnu/bootloader/grub.scm b/gnu/bootloader/grub.scm
> index e3febeefd0..552bc34f5a 100644
> --- a/gnu/bootloader/grub.scm
> +++ b/gnu/bootloader/grub.scm
> @@ -23,8 +23,10 @@
>  ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
>  
>  (define-module (gnu bootloader grub)
> +  #:use-module (guix build union)
>    #:use-module (guix records)
> -  #:use-module ((guix utils) #:select (%current-system))
> +  #:use-module (guix store)
> +  #:use-module ((guix utils) #:select (%current-system %current-target-system))
>    #:use-module (guix gexp)
>    #:use-module (gnu artwork)
>    #:use-module (gnu bootloader)
> @@ -46,8 +48,11 @@
>              grub-theme-color-highlight
>              grub-theme-gfxmode
>  
> +            install-grub-efi-netboot
> +
>              grub-bootloader
>              grub-efi-bootloader
> +            grub-efi-netboot-bootloader
>              grub-mkrescue-bootloader
>              grub-minimal-bootloader
>  
> @@ -295,6 +300,9 @@ code."
>          ((? file-system-label? label)
>           (format #f "search --label --set ~a"
>                   (file-system-label->string label)))
> +        ((? (lambda (device)
> +              (and (string? device) (string-contains device ":/"))) nfs-uri)
> +         "set root=(tftp)")
>          ((or #f (? string?))
>           #~(format #f "search --file --set ~a" #$file)))))
>  
> @@ -501,6 +509,87 @@ fi~%"))))
>                        "--bootloader-id=Guix"
>                        "--efi-directory" target-esp))))
>  
> +(define (install-grub-efi-netboot subdir)
> +  "Define a grub-efi-netboot bootloader installer for installation in SUBDIR,
> +which is usually efi/Guix or efi/boot."
> +  (let* ((system (string-split (or (%current-target-system)
> +                                   (%current-system))
> +                               #\-))
> +         (boot-efi-link (match system
> +                          (("i686" _ ...) "/bootia32.efi")
> +                          (("x86_64" _ ...) "/bootx64.efi")
> +                          (("arm" _ ...) "/bootarm.efi")
> +                          (("armhf" _ ...) "/bootarm.efi")
> +                          (("aarch64" _ ...) "/bootaa64.efi")
> +                          (("riscv" _ ...) "/bootriscv32.efi")
> +                          (("riscv64" _ ...) "/bootriscv64.efi")))

Don't forget the fall-through case, even if it's just
((_ ...) "/bootTODO.efi")

> +         (efi-bootloader (string-append (match system
> +                                          (("i686" _ ...) "i386-efi")
> +                                          (("x86_64" _ ...) "x86_64-efi")
> +                                          (("arm" _ ...) "arm-efi")
> +                                          (("armhf" _ ...) "arm-efi")
> +                                          (("aarch64" _ ...) "arm64-efi")
> +                                          (("riscv" _ ...) "riscv32-efi")
> +                                          (("riscv64" _ ...) "riscv64-efi"))
> +                                        "/core.efi")))

With the fall-through case here, can this be changed to
(("i686" _ ...) "i386-efi")
(("aarch64" _ ...) "arm64-efi")
(("riscv" _ ...) "riscv32-efi")
((_ ...) (string-append (first
                         (string-split
                           (nix-system->gnu-triplet
                             (or (%current-system)
                                 (%current-target-system)))
                           #\_))
                        "-efi"))

> +    (with-imported-modules
> +     '((guix build union))
> +     #~(lambda (bootloader target mount-point)
> +         "Install the BOOTLOADER, which must be the package grub, as e.g.
> +bootx64.efi or bootarm64.efi into SUBDIR, which is usually efi/Guix or efi/boot,
> +below the directory TARGET for the system whose root is mounted at MOUNT-POINT.
> +
> +MOUNT-POINT is the last argument in 'guix system init /etc/config.scm mnt/point'
> +or '/' for other 'guix system' commands.
> +
> +TARGET is the target argument given to the bootloader-configuration in
> +(operating-system
> + (bootloader (bootloader-configuration
> +              (target \"/boot\")
> +              …))
> + …)
> +TARGET is required to be an absolute path and must be provided by a TFTP server
> +as the TFTP root directory.
> +
> +GRUB will load tftp://server/SUBDIR/grub.cfg and this file will instruct it to
> +load more files from the store like tftp://server/gnu/store/…-linux…/Image.
> +
> +To make this possible two symlinks will be created. The first symlink points
> +relatively form TARGET/SUBDIR/grub.cfg to /boot/grub/grub.cfg. And the second
> +symlink points relatively from TARGET/%store-prefix to %store-prefix.
> +
> +It is important to note that these symlinks need to be relativ, as the absolute
> +paths on the TFTP server side are unknown.
> +
> +It is also important to note that both symlinks will point outside the TFTP root
> +directory and that the TARGET/%store-prefix symlink makes the whole store
> +accessible via TFTP. Possibly the TFTP server must be configured
> +to allow accesses outside its TFTP root directory. This may need to be
> +considered for security aspects."
> +         (use-modules ((guix build union) #:select (symlink-relative)))
> +         (let* ((net-dir (string-append mount-point target "/"))
> +                (sub-dir (string-append net-dir #$subdir "/"))
> +                (store-link (string-append net-dir (%store-prefix)))
> +                (grub-cfg "/boot/grub/grub.cfg")
> +                (grub-cfg-link (string-append sub-dir (basename grub-cfg)))
> +                (boot-efi-link (string-append sub-dir #$boot-efi-link)))
> +           ;; Prepare the symlink to the store.
> +           (mkdir-p (dirname store-link))
> +           (false-if-exception (delete-file store-link))
> +           (symlink-relative (%store-prefix) store-link)
> +           ;; Prepare the symlink to the grub.cfg, which points into the store.
> +           (false-if-exception (delete-file grub-cfg-link))
> +           (symlink-relative grub-cfg grub-cfg-link)
> +           ;; Install GRUB, which refers to the grub.cfg, with support for
> +           ;; encrypted partitions,
> +           (setenv "GRUB_ENABLE_CRYPTODISK" "y")
> +           (invoke/quiet (string-append bootloader "/bin/grub-mknetdir")
> +                         (string-append "--net-directory=" net-dir)
> +                         (string-append "--subdir=" #$subdir))
> +           ;; Prepare the bootloader symlink, which points to GRUB.
> +           (false-if-exception (delete-file boot-efi-link))
> +           (symlink #$efi-bootloader boot-efi-link))))))
> +
>  ^L
>  
>  ;;;
> @@ -533,6 +622,12 @@ fi~%"))))
>     (name 'grub-efi)
>     (package grub-efi)))
>  
> +(define grub-efi-netboot-bootloader
> +  (bootloader
> +   (inherit grub-efi-bootloader)
> +   (name 'grub-efi-netboot-bootloader)
> +   (installer (install-grub-efi-netboot "efi/Guix"))))
> +
>  (define grub-mkrescue-bootloader
>    (bootloader
>     (inherit grub-efi-bootloader)
> -- 
> 2.26.0
> 
> 
> 
> 

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[bug#41011] [PATCH] gnu: grub: Support for network boot via TFTP.

[Stefan]

Hi Efraim!

>> +         (boot-efi-link (match system
>> +                          (("i686" _ ...) "/bootia32.efi")
>> +                          (("x86_64" _ ...) "/bootx64.efi")
>> +                          (("arm" _ ...) "/bootarm.efi")
>> +                          (("armhf" _ ...) "/bootarm.efi")
>> +                          (("aarch64" _ ...) "/bootaa64.efi")
>> +                          (("riscv" _ ...) "/bootriscv32.efi")
>> +                          (("riscv64" _ ...) "/bootriscv64.efi")))
> 
> Don't forget the fall-through case, even if it's just
> ((_ ...) "/bootTODO.efi")

There was a contradicting remark by Danny:

> The major advantage of using "match" is its failure mode.  If the thing matched
> on is not a list (for some unfathomable reason) or if the first element is not
> matched on (!) then you get an exception--which is much better than doing weird
> unknown stuff.

Actually I would prefer an error here as well. Imagine a successful ‘guix system init’ silently creating a bootTODO.efi. Booting that system will certainly fail and someone will have a hard time to figure out that the generated bootTODO.efi is the reason.

>> +         (efi-bootloader (string-append (match system
>> +                                          (("i686" _ ...) "i386-efi")
>> +                                          (("x86_64" _ ...) "x86_64-efi")
>> +                                          (("arm" _ ...) "arm-efi")
>> +                                          (("armhf" _ ...) "arm-efi")
>> +                                          (("aarch64" _ ...) "arm64-efi")
>> +                                          (("riscv" _ ...) "riscv32-efi")
>> +                                          (("riscv64" _ ...) "riscv64-efi"))
>> +                                        "/core.efi")))
> 
> With the fall-through case here, can this be changed to
> (("i686" _ ...) "i386-efi")
> (("aarch64" _ ...) "arm64-efi")
> (("riscv" _ ...) "riscv32-efi")
> ((_ ...) (string-append (first
>                         (string-split
>                           (nix-system->gnu-triplet
>                             (or (%current-system)
>                                 (%current-target-system)))
>                           #\_))
>                        "-efi"))

There was a contradicting remark by Danny, which applies to the first part as well:

> Also, I have a slight preference for greppable file names even when it's a
> little more redundant

I understand your point in generating the arch part. I also understand the usage of (nix-system->gnu-triplet) to convert armhf to arm. I also think the risk is low that the first part raises no error and this part is doing something wrong without raising an error, too, leading to a not booting system. So having a default here seems fine to me.

However, Danny’s argument is convincing, too. And keeping this block similar to the first block eases the understanding, at least mine. My first thought seeing your suggestion was: “Why does this block need to be different from the other and what is this code doing differently?”

What do you think?


Bye

Stefan

[bug#41011] [PATCH] gnu: grub: Support for network boot via TFTP.

[Efraim Flashner]

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

On Tue, Sep 15, 2020 at 10:28:53PM +0200, Stefan wrote:
> Hi Efraim!
> 
> >> +         (boot-efi-link (match system
> >> +                          (("i686" _ ...) "/bootia32.efi")
> >> +                          (("x86_64" _ ...) "/bootx64.efi")
> >> +                          (("arm" _ ...) "/bootarm.efi")
> >> +                          (("armhf" _ ...) "/bootarm.efi")
> >> +                          (("aarch64" _ ...) "/bootaa64.efi")
> >> +                          (("riscv" _ ...) "/bootriscv32.efi")
> >> +                          (("riscv64" _ ...) "/bootriscv64.efi")))
> > 
> > Don't forget the fall-through case, even if it's just
> > ((_ ...) "/bootTODO.efi")
> 
> There was a contradicting remark by Danny:
> 
> > The major advantage of using "match" is its failure mode.  If the thing matched
> > on is not a list (for some unfathomable reason) or if the first element is not
> > matched on (!) then you get an exception--which is much better than doing weird
> > unknown stuff.
> 
> Actually I would prefer an error here as well. Imagine a successful ‘guix system init’ silently creating a bootTODO.efi. Booting that system will certainly fail and someone will have a hard time to figure out that the generated bootTODO.efi is the reason.
> 

My concern is more for architectures which aren't on the list having
unfortunate errors while doing something unrelated. Another option then
I suppose would be
((_ ...) #f)
It should still fail if you try to use it but there's still a code path
for, say, ppc64el. I like the #f idea better than bootTODO.efi.

> >> +         (efi-bootloader (string-append (match system
> >> +                                          (("i686" _ ...) "i386-efi")
> >> +                                          (("x86_64" _ ...) "x86_64-efi")
> >> +                                          (("arm" _ ...) "arm-efi")
> >> +                                          (("armhf" _ ...) "arm-efi")
> >> +                                          (("aarch64" _ ...) "arm64-efi")
> >> +                                          (("riscv" _ ...) "riscv32-efi")
> >> +                                          (("riscv64" _ ...) "riscv64-efi"))
> >> +                                        "/core.efi")))
> > 
> > With the fall-through case here, can this be changed to
> > (("i686" _ ...) "i386-efi")
> > (("aarch64" _ ...) "arm64-efi")
> > (("riscv" _ ...) "riscv32-efi")
> > ((_ ...) (string-append (first
> >                           (string-split
> >                             (nix-system->gnu-triplet
> >                               (or (%current-system)
> >                                   (%current-target-system)))
> >                           #\_))
> >                         "-efi"))

I re-noticed that system is passed above so my code block could be a bit
more contained

((_ ...) (string-append (first
                          (string-split
                            (nix-system->gnu-triplet system)
                          #\_))
                        "-efi"))

> 
> There was a contradicting remark by Danny, which applies to the first part as well:
> 
> > Also, I have a slight preference for greppable file names even when it's a
> > little more redundant
> 
> I understand your point in generating the arch part. I also understand the usage of (nix-system->gnu-triplet) to convert armhf to arm. I also think the risk is low that the first part raises no error and this part is doing something wrong without raising an error, too, leading to a not booting system. So having a default here seems fine to me.
> 
> However, Danny’s argument is convincing, too. And keeping this block similar to the first block eases the understanding, at least mine. My first thought seeing your suggestion was: “Why does this block need to be different from the other and what is this code doing differently?”
> 
> What do you think?
> 

The benefit is that there's less chance of typoing a mistake in the
code, either when writing it or when editing it later. There's also the
benefit again of not dismissing architectures which are currently not
listed in the list.

For something greppable, I don't really have a counter argument. Perhaps
a hand-wavey "code correctness" of reusing macros.

For unsupported architectures, ((_ ...) #f) again would work to make
sure there is at least a code path which would definitely fail if
someone tried to use it. That's my primary concern.

> 
> Bye
> 
> Stefan
> 

Thanks

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[bug#41011] [PATCH] gnu: grub: Support for network boot via TFTP.

[Stefan]

* gnu/bootloader/grub.scm (grub-efi-netboot-bootloader): New bootloader for
network booting.
(install-grub-efi-netboot): New bootloader installer for network booting.
(grub-root-search): Set the root to "(tftp)" if the searched file is not stored
on a local devices, i.e. an NFS share.
---
 gnu/bootloader/grub.scm | 106 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 105 insertions(+), 1 deletion(-)

diff --git a/gnu/bootloader/grub.scm b/gnu/bootloader/grub.scm
index e3febeefd0..211c314f4d 100644
--- a/gnu/bootloader/grub.scm
+++ b/gnu/bootloader/grub.scm
@@ -23,8 +23,10 @@
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
 (define-module (gnu bootloader grub)
+  #:use-module (guix build union)
   #:use-module (guix records)
-  #:use-module ((guix utils) #:select (%current-system))
+  #:use-module (guix store)
+  #:use-module (guix utils)
   #:use-module (guix gexp)
   #:use-module (gnu artwork)
   #:use-module (gnu bootloader)
@@ -46,8 +48,11 @@
             grub-theme-color-highlight
             grub-theme-gfxmode
 
+            install-grub-efi-netboot
+
             grub-bootloader
             grub-efi-bootloader
+            grub-efi-netboot-bootloader
             grub-mkrescue-bootloader
             grub-minimal-bootloader
 
@@ -295,6 +300,9 @@ code."
         ((? file-system-label? label)
          (format #f "search --label --set ~a"
                  (file-system-label->string label)))
+        ((? (lambda (device)
+              (and (string? device) (string-contains device ":/"))) nfs-uri)
+         "set root=(tftp)")
         ((or #f (? string?))
          #~(format #f "search --file --set ~a" #$file)))))
 
@@ -501,6 +509,96 @@ fi~%"))))
                       "--bootloader-id=Guix"
                       "--efi-directory" target-esp))))
 
+(define (install-grub-efi-netboot subdir)
+  "Define a grub-efi-netboot bootloader installer for installation in SUBDIR,
+which is usually efi/Guix or efi/boot."
+  (let* ((system (string-split (nix-system->gnu-triplet
+                                (or (%current-target-system)
+                                    (%current-system)))
+                               #\-))
+         (arch (first system))
+         (boot-efi-link (match system
+                          ;; These are the supportend systems and the names
+                          ;; defined by the UEFI standard for removable media.
+                          (("i686" _ ...)        "/bootia32.efi")
+                          (("x86_64" _ ...)      "/bootx64.efi")
+                          (("arm" _ ...)         "/bootarm.efi")
+                          (("aarch64" _ ...)     "/bootaa64.efi")
+                          (("riscv" _ ...)       "/bootriscv32.efi")
+                          (("riscv64" _ ...)     "/bootriscv64.efi")
+                          ;; Other systems are not supported, although defined.
+                          ;; (("riscv128" _ ...) "/bootriscv128.efi")
+                          ;; (("ia64" _ ...)     "/bootia64.efi")
+                          ((_ ...)               #f)))
+         (efi-bootloader (string-append
+                          ;; This is the arch dependent file name of GRUB, e.g.
+                          ;; i368-efi/core.efi or arm64-efi/core.efi.
+                          (match arch
+                            ("i686"    "i386")
+                            ("aarch64" "arm64")
+                            ("riscv"   "riscv32")
+                            (_         arch))
+                          "-efi/core.efi")))
+    (with-imported-modules
+     '((guix build union))
+     #~(lambda (bootloader target mount-point)
+         "Install the BOOTLOADER, which must be the package grub, as e.g.
+bootx64.efi or bootaa64.efi into SUBDIR, which is usually efi/Guix or efi/boot,
+below the directory TARGET for the system whose root is mounted at MOUNT-POINT.
+
+MOUNT-POINT is the last argument in 'guix system init /etc/config.scm mnt/point'
+or '/' for other 'guix system' commands.
+
+TARGET is the target argument given to the bootloader-configuration in
+
+(operating-system
+ (bootloader (bootloader-configuration
+              (target \"/boot\")
+              …))
+ …)
+
+TARGET is required to be an absolute directory name, usually mounted via NFS,
+and finally needs to be provided by a TFTP server as the TFTP root directory.
+
+GRUB will load tftp://server/SUBDIR/grub.cfg and this file will instruct it to
+load more files from the store like tftp://server/gnu/store/…-linux…/Image.
+
+To make this possible two symlinks will be created. The first symlink points
+relatively form TARGET/SUBDIR/grub.cfg to /boot/grub/grub.cfg. And the second
+symlink points relatively from TARGET/%store-prefix to %store-prefix.
+
+It is important to note that these symlinks need to be relativ, as the absolute
+paths on the TFTP server side are unknown.
+
+It is also important to note that both symlinks will point outside the TFTP root
+directory and that the TARGET/%store-prefix symlink makes the whole store
+accessible via TFTP. Possibly the TFTP server must be configured
+to allow accesses outside its TFTP root directory. This may need to be
+considered for security aspects."
+         (use-modules ((guix build union) #:select (symlink-relative)))
+         (let* ((net-dir (string-append mount-point target "/"))
+                (sub-dir (string-append net-dir #$subdir "/"))
+                (store-link (string-append net-dir (%store-prefix)))
+                (grub-cfg "/boot/grub/grub.cfg")
+                (grub-cfg-link (string-append sub-dir (basename grub-cfg)))
+                (boot-efi-link (string-append sub-dir #$boot-efi-link)))
+           ;; Prepare the symlink to the store.
+           (mkdir-p (dirname store-link))
+           (false-if-exception (delete-file store-link))
+           (symlink-relative (%store-prefix) store-link)
+           ;; Prepare the symlink to the grub.cfg, which points into the store.
+           (false-if-exception (delete-file grub-cfg-link))
+           (symlink-relative grub-cfg grub-cfg-link)
+           ;; Install GRUB, which refers to the grub.cfg, with support for
+           ;; encrypted partitions,
+           (setenv "GRUB_ENABLE_CRYPTODISK" "y")
+           (invoke/quiet (string-append bootloader "/bin/grub-mknetdir")
+                         (string-append "--net-directory=" net-dir)
+                         (string-append "--subdir=" #$subdir))
+           ;; Prepare the bootloader symlink, which points to GRUB.
+           (false-if-exception (delete-file boot-efi-link))
+           (symlink #$efi-bootloader boot-efi-link))))))
+
 ^L
 
 ;;;
@@ -533,6 +631,12 @@ fi~%"))))
    (name 'grub-efi)
    (package grub-efi)))
 
+(define grub-efi-netboot-bootloader
+  (bootloader
+   (inherit grub-efi-bootloader)
+   (name 'grub-efi-netboot-bootloader)
+   (installer (install-grub-efi-netboot "efi/Guix"))))
+
 (define grub-mkrescue-bootloader
   (bootloader
    (inherit grub-efi-bootloader)
-- 
2.26.0

[bug#41011] [PATCH] gnu: grub: Support for network boot via TFTP.

[Stefan]

* gnu/bootloader/grub.scm (grub-efi-netboot-bootloader): New bootloader for
network booting.
(install-grub-efi-netboot): New bootloader installer for network booting.
(grub-root-search): Set the root to "(tftp)" if the searched file is not stored
on a local devices, i.e. an NFS share.
---
 gnu/bootloader/grub.scm | 111 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 107 insertions(+), 4 deletions(-)

diff --git a/gnu/bootloader/grub.scm b/gnu/bootloader/grub.scm
index f69bf8ed4d..fe771b2631 100644
--- a/gnu/bootloader/grub.scm
+++ b/gnu/bootloader/grub.scm
@@ -23,8 +23,10 @@
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
 (define-module (gnu bootloader grub)
+  #:use-module (guix build union)
   #:use-module (guix records)
-  #:use-module ((guix utils) #:select (%current-system))
+  #:use-module (guix store)
+  #:use-module (guix utils)
   #:use-module (guix gexp)
   #:use-module (gnu artwork)
   #:use-module (gnu bootloader)
@@ -46,8 +48,11 @@
             grub-theme-color-highlight
             grub-theme-gfxmode
 
+            install-grub-efi-netboot
+
             grub-bootloader
             grub-efi-bootloader
+            grub-efi-netboot-bootloader
             grub-mkrescue-bootloader
             grub-minimal-bootloader
 
@@ -297,9 +302,11 @@ code."
                  (file-system-label->string label)))
         ((? (lambda (device)
               (and (string? device) (string-contains device ":/"))) nfs-uri)
-         ;; This assumes that if your root file system is on NFS, then
-         ;; you also want to load your grub extra files, kernel and initrd
-         ;; from there.
+         ;; If the device is an NFS share, then we assume that the expected
+         ;; file on that device (e.g. the GRUB background image or the kernel)
+         ;; has to be loaded over the network.  Otherwise we would need an
+         ;; additional device information for some local disk to look for that
+         ;; file, which we do not have.
          ;;
          ;; We explicitly set "root=(tftp)" here even though if grub.cfg
          ;; had been loaded via TFTP, Grub would have set "root=(tftp)"
@@ -528,6 +535,96 @@ fi~%"))))
                       "--bootloader-id=Guix"
                       "--efi-directory" target-esp))))
 
+(define (install-grub-efi-netboot subdir)
+  "Define a grub-efi-netboot bootloader installer for installation in SUBDIR,
+which is usually efi/Guix or efi/boot."
+  (let* ((system (string-split (nix-system->gnu-triplet
+                                (or (%current-target-system)
+                                    (%current-system)))
+                               #\-))
+         (arch (first system))
+         (boot-efi-link (match system
+                          ;; These are the supportend systems and the names
+                          ;; defined by the UEFI standard for removable media.
+                          (("i686" _ ...)        "/bootia32.efi")
+                          (("x86_64" _ ...)      "/bootx64.efi")
+                          (("arm" _ ...)         "/bootarm.efi")
+                          (("aarch64" _ ...)     "/bootaa64.efi")
+                          (("riscv" _ ...)       "/bootriscv32.efi")
+                          (("riscv64" _ ...)     "/bootriscv64.efi")
+                          ;; Other systems are not supported, although defined.
+                          ;; (("riscv128" _ ...) "/bootriscv128.efi")
+                          ;; (("ia64" _ ...)     "/bootia64.efi")
+                          ((_ ...)               #f)))
+         (efi-bootloader (string-append
+                          ;; This is the arch dependent file name of GRUB, e.g.
+                          ;; i368-efi/core.efi or arm64-efi/core.efi.
+                          (match arch
+                            ("i686"    "i386")
+                            ("aarch64" "arm64")
+                            ("riscv"   "riscv32")
+                            (_         arch))
+                          "-efi/core.efi")))
+    (with-imported-modules
+     '((guix build union))
+     #~(lambda (bootloader target mount-point)
+         "Install the BOOTLOADER, which must be the package grub, as e.g.
+bootx64.efi or bootaa64.efi into SUBDIR, which is usually efi/Guix or efi/boot,
+below the directory TARGET for the system whose root is mounted at MOUNT-POINT.
+
+MOUNT-POINT is the last argument in 'guix system init /etc/config.scm mnt/point'
+or '/' for other 'guix system' commands.
+
+TARGET is the target argument given to the bootloader-configuration in
+
+(operating-system
+ (bootloader (bootloader-configuration
+              (target \"/boot\")
+              …))
+ …)
+
+TARGET is required to be an absolute directory name, usually mounted via NFS,
+and finally needs to be provided by a TFTP server as the TFTP root directory.
+
+GRUB will load tftp://server/SUBDIR/grub.cfg and this file will instruct it to
+load more files from the store like tftp://server/gnu/store/…-linux…/Image.
+
+To make this possible two symlinks will be created. The first symlink points
+relatively form TARGET/SUBDIR/grub.cfg to /boot/grub/grub.cfg. And the second
+symlink points relatively from TARGET/%store-prefix to %store-prefix.
+
+It is important to note that these symlinks need to be relativ, as the absolute
+paths on the TFTP server side are unknown.
+
+It is also important to note that both symlinks will point outside the TFTP root
+directory and that the TARGET/%store-prefix symlink makes the whole store
+accessible via TFTP. Possibly the TFTP server must be configured
+to allow accesses outside its TFTP root directory. This may need to be
+considered for security aspects."
+         (use-modules ((guix build union) #:select (symlink-relative)))
+         (let* ((net-dir (string-append mount-point target "/"))
+                (sub-dir (string-append net-dir #$subdir "/"))
+                (store-link (string-append net-dir (%store-prefix)))
+                (grub-cfg "/boot/grub/grub.cfg")
+                (grub-cfg-link (string-append sub-dir (basename grub-cfg)))
+                (boot-efi-link (string-append sub-dir #$boot-efi-link)))
+           ;; Prepare the symlink to the store.
+           (mkdir-p (dirname store-link))
+           (false-if-exception (delete-file store-link))
+           (symlink-relative (%store-prefix) store-link)
+           ;; Prepare the symlink to the grub.cfg, which points into the store.
+           (false-if-exception (delete-file grub-cfg-link))
+           (symlink-relative grub-cfg grub-cfg-link)
+           ;; Install GRUB, which refers to the grub.cfg, with support for
+           ;; encrypted partitions,
+           (setenv "GRUB_ENABLE_CRYPTODISK" "y")
+           (invoke/quiet (string-append bootloader "/bin/grub-mknetdir")
+                         (string-append "--net-directory=" net-dir)
+                         (string-append "--subdir=" #$subdir))
+           ;; Prepare the bootloader symlink, which points to GRUB.
+           (false-if-exception (delete-file boot-efi-link))
+           (symlink #$efi-bootloader boot-efi-link))))))
+
 ^L
 
 ;;;
@@ -560,6 +657,12 @@ fi~%"))))
    (name 'grub-efi)
    (package grub-efi)))
 
+(define grub-efi-netboot-bootloader
+  (bootloader
+   (inherit grub-efi-bootloader)
+   (name 'grub-efi-netboot-bootloader)
+   (installer (install-grub-efi-netboot "efi/Guix"))))
+
 (define grub-mkrescue-bootloader
   (bootloader
    (inherit grub-efi-bootloader)
-- 
2.26.0

[bug#41011] [PATCH] gnu: grub: Support for network boot via TFTP.

[Stefan]

Hi Danny!

When rebasing my commit onto the current origin/master, there was a conflict with your last commit. While reading your comment block, I noticed that the first paragraph is not quite right, so I did a small correction.


Bye

Stefan

[bug#41011] [PATCH] gnu: grub: Support for network boot via TFTP.

[Stefan]

Hi Danny!

I realised a bug in my last patch: I tried to create the link to the grub.cfg before the needed sub-directory has been created by grub-mknetdir.

The problem didn't show up before, because due to a previous bootloader installation that directory was already present.

An updated patch with a corrected order follows.


Bye

Stefan

[bug#41011] [PATCH] gnu: grub: Support for network boot via TFTP.

[Stefan]

* gnu/bootloader/grub.scm (grub-efi-netboot-bootloader): New bootloader for
network booting.
(install-grub-efi-netboot): New bootloader installer for network booting.
(grub-root-search): Set the root to "(tftp)" if the searched file is not stored
on a local devices, i.e. an NFS share.
---
 gnu/bootloader/grub.scm | 111 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 107 insertions(+), 4 deletions(-)

diff --git a/gnu/bootloader/grub.scm b/gnu/bootloader/grub.scm
index f69bf8ed4d..346a9cac7a 100644
--- a/gnu/bootloader/grub.scm
+++ b/gnu/bootloader/grub.scm
@@ -23,8 +23,10 @@
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
 (define-module (gnu bootloader grub)
+  #:use-module (guix build union)
   #:use-module (guix records)
-  #:use-module ((guix utils) #:select (%current-system))
+  #:use-module (guix store)
+  #:use-module (guix utils)
   #:use-module (guix gexp)
   #:use-module (gnu artwork)
   #:use-module (gnu bootloader)
@@ -46,8 +48,11 @@
             grub-theme-color-highlight
             grub-theme-gfxmode
 
+            install-grub-efi-netboot
+
             grub-bootloader
             grub-efi-bootloader
+            grub-efi-netboot-bootloader
             grub-mkrescue-bootloader
             grub-minimal-bootloader
 
@@ -297,9 +302,11 @@ code."
                  (file-system-label->string label)))
         ((? (lambda (device)
               (and (string? device) (string-contains device ":/"))) nfs-uri)
-         ;; This assumes that if your root file system is on NFS, then
-         ;; you also want to load your grub extra files, kernel and initrd
-         ;; from there.
+         ;; If the device is an NFS share, then we assume that the expected
+         ;; file on that device (e.g. the GRUB background image or the kernel)
+         ;; has to be loaded over the network.  Otherwise we would need an
+         ;; additional device information for some local disk to look for that
+         ;; file, which we do not have.
          ;;
          ;; We explicitly set "root=(tftp)" here even though if grub.cfg
          ;; had been loaded via TFTP, Grub would have set "root=(tftp)"
@@ -528,6 +535,96 @@ fi~%"))))
                       "--bootloader-id=Guix"
                       "--efi-directory" target-esp))))
 
+(define (install-grub-efi-netboot subdir)
+  "Define a grub-efi-netboot bootloader installer for installation in SUBDIR,
+which is usually efi/Guix or efi/boot."
+  (let* ((system (string-split (nix-system->gnu-triplet
+                                (or (%current-target-system)
+                                    (%current-system)))
+                               #\-))
+         (arch (first system))
+         (boot-efi-link (match system
+                          ;; These are the supportend systems and the names
+                          ;; defined by the UEFI standard for removable media.
+                          (("i686" _ ...)        "/bootia32.efi")
+                          (("x86_64" _ ...)      "/bootx64.efi")
+                          (("arm" _ ...)         "/bootarm.efi")
+                          (("aarch64" _ ...)     "/bootaa64.efi")
+                          (("riscv" _ ...)       "/bootriscv32.efi")
+                          (("riscv64" _ ...)     "/bootriscv64.efi")
+                          ;; Other systems are not supported, although defined.
+                          ;; (("riscv128" _ ...) "/bootriscv128.efi")
+                          ;; (("ia64" _ ...)     "/bootia64.efi")
+                          ((_ ...)               #f)))
+         (efi-bootloader (string-append
+                          ;; This is the arch dependent file name of GRUB, e.g.
+                          ;; i368-efi/core.efi or arm64-efi/core.efi.
+                          (match arch
+                            ("i686"    "i386")
+                            ("aarch64" "arm64")
+                            ("riscv"   "riscv32")
+                            (_         arch))
+                          "-efi/core.efi")))
+    (with-imported-modules
+     '((guix build union))
+     #~(lambda (bootloader target mount-point)
+         "Install the BOOTLOADER, which must be the package grub, as e.g.
+bootx64.efi or bootaa64.efi into SUBDIR, which is usually efi/Guix or efi/boot,
+below the directory TARGET for the system whose root is mounted at MOUNT-POINT.
+
+MOUNT-POINT is the last argument in 'guix system init /etc/config.scm mnt/point'
+or '/' for other 'guix system' commands.
+
+TARGET is the target argument given to the bootloader-configuration in
+
+(operating-system
+ (bootloader (bootloader-configuration
+              (target \"/boot\")
+              …))
+ …)
+
+TARGET is required to be an absolute directory name, usually mounted via NFS,
+and finally needs to be provided by a TFTP server as the TFTP root directory.
+
+GRUB will load tftp://server/SUBDIR/grub.cfg and this file will instruct it to
+load more files from the store like tftp://server/gnu/store/…-linux…/Image.
+
+To make this possible two symlinks will be created. The first symlink points
+relatively form TARGET/SUBDIR/grub.cfg to /boot/grub/grub.cfg. And the second
+symlink points relatively from TARGET/%store-prefix to %store-prefix.
+
+It is important to note that these symlinks need to be relativ, as the absolute
+paths on the TFTP server side are unknown.
+
+It is also important to note that both symlinks will point outside the TFTP root
+directory and that the TARGET/%store-prefix symlink makes the whole store
+accessible via TFTP. Possibly the TFTP server must be configured
+to allow accesses outside its TFTP root directory. This may need to be
+considered for security aspects."
+         (use-modules ((guix build union) #:select (symlink-relative)))
+         (let* ((net-dir (string-append mount-point target "/"))
+                (sub-dir (string-append net-dir #$subdir "/"))
+                (store-link (string-append net-dir (%store-prefix)))
+                (grub-cfg "/boot/grub/grub.cfg")
+                (grub-cfg-link (string-append sub-dir (basename grub-cfg)))
+                (boot-efi-link (string-append sub-dir #$boot-efi-link)))
+           ;; Prepare the symlink to the store.
+           (mkdir-p (dirname store-link))
+           (false-if-exception (delete-file store-link))
+           (symlink-relative (%store-prefix) store-link)
+           ;; Install GRUB, which refers to the grub.cfg, with support for
+           ;; encrypted partitions,
+           (setenv "GRUB_ENABLE_CRYPTODISK" "y")
+           (invoke/quiet (string-append bootloader "/bin/grub-mknetdir")
+                         (string-append "--net-directory=" net-dir)
+                         (string-append "--subdir=" #$subdir))
+           ;; Prepare the symlink to the grub.cfg, which points into the store.
+           (false-if-exception (delete-file grub-cfg-link))
+           (symlink-relative grub-cfg grub-cfg-link)
+           ;; Prepare the bootloader symlink, which points to GRUB.
+           (false-if-exception (delete-file boot-efi-link))
+           (symlink #$efi-bootloader boot-efi-link))))))
+
 ^L
 
 ;;;
@@ -560,6 +657,12 @@ fi~%"))))
    (name 'grub-efi)
    (package grub-efi)))
 
+(define grub-efi-netboot-bootloader
+  (bootloader
+   (inherit grub-efi-bootloader)
+   (name 'grub-efi-netboot-bootloader)
+   (installer (install-grub-efi-netboot "efi/Guix"))))
+
 (define grub-mkrescue-bootloader
   (bootloader
    (inherit grub-efi-bootloader)
-- 
2.26.0

[bug#41011] [PATCH] gnu: grub: Support for network boot via TFTP.

[Danny Milosavljevic]

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

Hi Stefan,

I think this is almost ready to be merged.

There's still one thing I'm unclear about.

I tested it this way:

(1) create ~/config.scm with the following contents:

(use-modules (gnu))

(operating-system
  (timezone "UTC")
  (bootloader
    (bootloader-configuration
      (bootloader grub-efi-netboot-bootloader)
      (target "/boot")))
  (file-systems
    (cons* (file-system
             (mount-point "/")
             (device ":/foo")
             (type "nfs"))
            %base-file-systems))
  (host-name "client1"))

(2) mkdir ~/tf
(3) ./pre-inst-env guix system init ~/config.scm ~/tf

Now, ~/tf contains the system to be booted over the network, right?

But ~/tf/boot/efi/Guix/grub.cfg points to the tftp machine's /boot .
Is that a bug?  Shouldn't it point to the /boot of ~/tf, the system
to be booted?

Likewise, the gnu store directly inside ~/tf is not used, but
~/tf/boot/gnu points to the tftp machines's /gnu.  Shouldn't it
point to the former?

On Sat, 26 Sep 2020 12:54:00 +0200
Stefan <stefan-guix@vodafonemail.de> wrote:

> +         (let* ((net-dir (string-append mount-point target "/"))
> +                (sub-dir (string-append net-dir #$subdir "/"))
> +                (store-link (string-append net-dir (%store-prefix)))
> +                (grub-cfg "/boot/grub/grub.cfg")

Shouldn't that be

 (string-append mount-point "/boot/grub/grub.cfg")

?

> +                (grub-cfg-link (string-append sub-dir (basename grub-cfg)))
> +                (boot-efi-link (string-append sub-dir #$boot-efi-link)))
> +           ;; Prepare the symlink to the store.
> +           (mkdir-p (dirname store-link))
> +           (false-if-exception (delete-file store-link))
> +           (symlink-relative (%store-prefix) store-link)

Shouldn't that be

  (symlink-relative (string-append mount-point (%store-prefix)) store-link)

?

> +           ;; Install GRUB, which refers to the grub.cfg, with support for
> +           ;; encrypted partitions,
> +           (setenv "GRUB_ENABLE_CRYPTODISK" "y")
> +           (invoke/quiet (string-append bootloader "/bin/grub-mknetdir")
> +                         (string-append "--net-directory=" net-dir)
> +                         (string-append "--subdir=" #$subdir))
> +           ;; Prepare the symlink to the grub.cfg, which points into the store.
> +           (false-if-exception (delete-file grub-cfg-link))
> +           (symlink-relative grub-cfg grub-cfg-link)
> +           ;; Prepare the bootloader symlink, which points to GRUB.
> +           (false-if-exception (delete-file boot-efi-link))
> +           (symlink #$efi-bootloader boot-efi-link))))))

Okay.

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

[bug#41011] [PATCH] gnu: grub: Support for network boot via TFTP.

[Stefan]

Hi Danny!

> But ~/tf/boot/efi/Guix/grub.cfg points to the tftp machine's /boot .
> Is that a bug?  Shouldn't it point to the /boot of ~/tf, the system
> to be booted?
> 
> Likewise, the gnu store directly inside ~/tf is not used, but
> ~/tf/boot/gnu points to the tftp machines's /gnu.  Shouldn't it
> point to the former?

You are absolutely right, thanks. It seems that I only tested with ‘guix system reconfigure’. My bad.

I fixed it and this time used ‘guix system init’ for verification. I also correct the description accordingly.


Bye

Stefan

[bug#41011] [PATCH] gnu: grub: Support for network boot via TFTP.

[Stefan]

* gnu/bootloader/grub.scm (grub-efi-netboot-bootloader): New bootloader for
network booting.
(install-grub-efi-netboot): New bootloader installer for network booting.
(grub-root-search): Set the root to "(tftp)" if the searched file is not stored
on a local devices, i.e. an NFS share.
---
 gnu/bootloader/grub.scm | 114 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 110 insertions(+), 4 deletions(-)

diff --git a/gnu/bootloader/grub.scm b/gnu/bootloader/grub.scm
index f69bf8ed4d..516a7d48c8 100644
--- a/gnu/bootloader/grub.scm
+++ b/gnu/bootloader/grub.scm
@@ -23,8 +23,10 @@
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
 (define-module (gnu bootloader grub)
+  #:use-module (guix build union)
   #:use-module (guix records)
-  #:use-module ((guix utils) #:select (%current-system))
+  #:use-module (guix store)
+  #:use-module (guix utils)
   #:use-module (guix gexp)
   #:use-module (gnu artwork)
   #:use-module (gnu bootloader)
@@ -46,8 +48,11 @@
             grub-theme-color-highlight
             grub-theme-gfxmode
 
+            install-grub-efi-netboot
+
             grub-bootloader
             grub-efi-bootloader
+            grub-efi-netboot-bootloader
             grub-mkrescue-bootloader
             grub-minimal-bootloader
 
@@ -297,9 +302,11 @@ code."
                  (file-system-label->string label)))
         ((? (lambda (device)
               (and (string? device) (string-contains device ":/"))) nfs-uri)
-         ;; This assumes that if your root file system is on NFS, then
-         ;; you also want to load your grub extra files, kernel and initrd
-         ;; from there.
+         ;; If the device is an NFS share, then we assume that the expected
+         ;; file on that device (e.g. the GRUB background image or the kernel)
+         ;; has to be loaded over the network.  Otherwise we would need an
+         ;; additional device information for some local disk to look for that
+         ;; file, which we do not have.
          ;;
          ;; We explicitly set "root=(tftp)" here even though if grub.cfg
          ;; had been loaded via TFTP, Grub would have set "root=(tftp)"
@@ -528,6 +535,99 @@ fi~%"))))
                       "--bootloader-id=Guix"
                       "--efi-directory" target-esp))))
 
+(define (install-grub-efi-netboot subdir)
+  "Define a grub-efi-netboot bootloader installer for installation in SUBDIR,
+which is usually efi/Guix or efi/boot."
+  (let* ((system (string-split (nix-system->gnu-triplet
+                                (or (%current-target-system)
+                                    (%current-system)))
+                               #\-))
+         (arch (first system))
+         (boot-efi-link (match system
+                          ;; These are the supportend systems and the names
+                          ;; defined by the UEFI standard for removable media.
+                          (("i686" _ ...)        "/bootia32.efi")
+                          (("x86_64" _ ...)      "/bootx64.efi")
+                          (("arm" _ ...)         "/bootarm.efi")
+                          (("aarch64" _ ...)     "/bootaa64.efi")
+                          (("riscv" _ ...)       "/bootriscv32.efi")
+                          (("riscv64" _ ...)     "/bootriscv64.efi")
+                          ;; Other systems are not supported, although defined.
+                          ;; (("riscv128" _ ...) "/bootriscv128.efi")
+                          ;; (("ia64" _ ...)     "/bootia64.efi")
+                          ((_ ...)               #f)))
+         (core-efi (string-append
+                    ;; This is the arch dependent file name of GRUB, e.g.
+                    ;; i368-efi/core.efi or arm64-efi/core.efi.
+                    (match arch
+                      ("i686"    "i386")
+                      ("aarch64" "arm64")
+                      ("riscv"   "riscv32")
+                      (_         arch))
+                    "-efi/core.efi")))
+    (with-imported-modules
+     '((guix build union))
+     #~(lambda (bootloader target mount-point)
+         "Install the BOOTLOADER, which must be the package grub, as e.g.
+bootx64.efi or bootaa64.efi into SUBDIR, which is usually efi/Guix or efi/boot,
+below the directory TARGET for the system whose root is mounted at MOUNT-POINT.
+
+MOUNT-POINT is the last argument in 'guix system init /etc/config.scm mnt/point'
+or '/' for other 'guix system' commands.
+
+TARGET is the target argument given to the bootloader-configuration in
+
+(operating-system
+ (bootloader (bootloader-configuration
+              (target \"/boot\")
+              …))
+ …)
+
+TARGET is required to be an absolute directory name, usually mounted via NFS,
+and finally needs to be provided by a TFTP server as the TFTP root directory.
+
+GRUB will load tftp://server/SUBDIR/grub.cfg and this file will instruct it to
+load more files from the store like tftp://server/gnu/store/…-linux…/Image.
+
+To make this possible two symlinks will be created. The first symlink points
+relatively form MOUNT-POINT/TARGET/SUBDIR/grub.cfg to
+MOUNT-POINT/boot/grub/grub.cfg, and the second symlink points relatively from
+MOUNT-POINT/TARGET/%store-prefix to MOUNT-POINT/%store-prefix.
+
+It is important to note that these symlinks need to be relativ, as the absolute
+paths on the TFTP server side are unknown.
+
+It is also important to note that both symlinks will point outside the TFTP root
+directory and that the TARGET/%store-prefix symlink makes the whole store
+accessible via TFTP. Possibly the TFTP server must be configured
+to allow accesses outside its TFTP root directory. This may need to be
+considered for security aspects."
+         (use-modules ((guix build union) #:select (symlink-relative)))
+         (let* ((net-dir (string-append mount-point target "/"))
+                (sub-dir (string-append net-dir #$subdir "/"))
+                (store (string-append mount-point (%store-prefix)))
+                (store-link (string-append net-dir (%store-prefix)))
+                (grub-cfg (string-append mount-point "/boot/grub/grub.cfg"))
+                (grub-cfg-link (string-append sub-dir (basename grub-cfg)))
+                (boot-efi-link (string-append sub-dir #$boot-efi-link)))
+           ;; Prepare the symlink to the store.
+           (mkdir-p (dirname store-link))
+           (false-if-exception (delete-file store-link))
+           (symlink-relative store store-link)
+           ;; Prepare the symlink to the grub.cfg, which points into the store.
+           (mkdir-p (dirname grub-cfg-link))
+           (false-if-exception (delete-file grub-cfg-link))
+           (symlink-relative grub-cfg grub-cfg-link)
+           ;; Install GRUB, which refers to the grub.cfg, with support for
+           ;; encrypted partitions,
+           (setenv "GRUB_ENABLE_CRYPTODISK" "y")
+           (invoke/quiet (string-append bootloader "/bin/grub-mknetdir")
+                         (string-append "--net-directory=" net-dir)
+                         (string-append "--subdir=" #$subdir))
+           ;; Prepare the bootloader symlink, which points to core.efi of GRUB.
+           (false-if-exception (delete-file boot-efi-link))
+           (symlink #$core-efi boot-efi-link))))))
+
 ^L
 
 ;;;
@@ -560,6 +660,12 @@ fi~%"))))
    (name 'grub-efi)
    (package grub-efi)))
 
+(define grub-efi-netboot-bootloader
+  (bootloader
+   (inherit grub-efi-bootloader)
+   (name 'grub-efi-netboot-bootloader)
+   (installer (install-grub-efi-netboot "efi/Guix"))))
+
 (define grub-mkrescue-bootloader
   (bootloader
    (inherit grub-efi-bootloader)
-- 
2.26.0

[bug#41011] [PATCH] gnu: grub: Support for network boot via TFTP.

[Danny Milosavljevic]

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

Hi Stefan,

pushed to guix master as commits c85f316ae986331cc5181da4bcc757078d7a9412 (implementation)
and 740fd97ebeadebc02448cb0482084f359938b5fe (documentation), after successful test.

Thanks!

If you think this concludes this bug report, please close it (any mail to
41011-done@debbugs.gnu.org).

Are there other parts still missing that you need?

(I have a patch to dnsmasq to make it support configuring the built-in
TFTP server that I've been using.  I'll just post it to guix-patches@gnu.org
so it gets a new number)


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

[bug#41011] [PATCH] gnu: grub: Support for network boot via TFTP.

[Danny Milosavljevic]

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

Hi Stefan,

On Sun, 13 Sep 2020 19:46:01 +0200
Stefan <stefan-guix@vodafonemail.de> wrote:

> diff --git a/gnu/bootloader/grub.scm b/gnu/bootloader/grub.scm
> index e3febeefd0..552bc34f5a 100644
> --- a/gnu/bootloader/grub.scm
> +++ b/gnu/bootloader/grub.scm
> @@ -295,6 +300,9 @@ code."
>          ((? file-system-label? label)
>           (format #f "search --label --set ~a"
>                   (file-system-label->string label)))
> +        ((? (lambda (device)
> +              (and (string? device) (string-contains device ":/"))) nfs-uri)
> +         "set root=(tftp)")
>          ((or #f (? string?))
>           #~(format #f "search --file --set ~a" #$file)))))

After careful consideration, I've pushed this to guix master, with a big
comment in the source code as to why we are doing what we are doing.

I can see no downside to defaulting to TFTP for the time being.

I do wonder if there are cases now where Grub tries to use TFTP when the user
meant to boot locally--but nothing comes to mind. 

Thank you for your elaborative E-Mails.

I will still review the remainder.  Please take Efraim's comments into
consideration.

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

[bug#41011] [PATCH] gnu: grub: Support for network boot via TFTP.

[Danny Milosavljevic]

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

Hello Stefan,

I have reviewed this new patch and it looks good to me.

There's one thing, though: Where does this new bootloader get used?

I think--from reading the source code and from your previous comments--that the
bootloader gets installed on the TFTP server, but as files to be served, not to
boot the TFTP server itself.  One is basically using "guix system init" to
initialize an operating system, complete with bootloader--only it is served
by an TFTP server instead of booted locally.

Is that correct?  If so, I think we should document that fact--because that's
absolutely not obvious to a casual observer.

Also, the same files also need to be exported by an NFS server on the same
host as the TFTP server (which is a limitation.  I understand the rationale,
but the limitation has to be documented).  This should be documented not in
the bootloader (which does not require NFS at all, right?), but in
doc/guix.texi--where ideally we'd document an example system configuration
for it.

I'll leave a week for comments and then will merge either this version or an
eventual newer version to guix master.

Thanks.

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

bug#41011: [PATCH] gnu: grub: Support for network boot via TFTP.

[Stefan]