bug#26544: [PATCH] system: Move "--load" and other guix-specific parameters from the grub module to the generic system module.

bug#26544: [PATCH] system: Move "--load" and other guix-specific parameters from the grub module to the generic system module.

[Danny Milosavljevic]

* gnu/system.scm (bootable-kernel-arguments): New variable.
(operating-system-parameters-file): Rename to ...
(operating-system-boot-parameters-file): ... this and split off ...
(operating-system-boot-parameters): ... new variable.
(operating-system-bootcfg): Use operating-system-boot-parameters.
(operating-system-directory-base-entries):
Use operating-system-boot-parameters-file instead of
operating-system-parameters-file.
(operating-system-all-kernel-arguments): New variable. Export it.
(operating-system-kernel-arguments): Remove export.
(read-boot-parameters-file): New variable. Export it.
(read-boot-parameters): Remove export.
* gnu/system/grub.scm (boot-parameters->menu-entry): New variable.
(grub-configuration-file): Use boot-parameters->menu-entry.
* gnu/system/vm.scm (system-qemu-image/shared-store-script): Use
operating-system-all-kernel-arguments.
* guix/scripts/system.scm (profile-grub-entries): Rename to ...
(profile-boot-parameters): ... this and return boot-parameters directly.
(reinstall-grub): Use read-boot-parameters-file.
Use profile-boot-parameters instead of profile-grub-entries.
(display-system-generation): Use read-boot-parameters-file.
Use profile-boot-parameters instead of profile-grub-entries.
---
 gnu/system.scm          | 116 ++++++++++++++++++++++++++++++++----------------
 gnu/system/grub.scm     |  13 +++++-
 gnu/system/vm.scm       |  10 ++---
 guix/scripts/system.scm |  56 +++++++----------------
 4 files changed, 109 insertions(+), 86 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index 89c4150f9..f83eb9e58 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -73,7 +73,7 @@
             operating-system-hosts-file
             operating-system-kernel
             operating-system-kernel-file
-            operating-system-kernel-arguments
+            operating-system-all-kernel-arguments
             operating-system-initrd
             operating-system-users
             operating-system-groups
@@ -109,7 +109,7 @@
             boot-parameters-kernel
             boot-parameters-kernel-arguments
             boot-parameters-initrd
-            read-boot-parameters
+            read-boot-parameters-file
 
             local-host-aliases
             %setuid-programs
@@ -122,6 +122,12 @@
 ;;;
 ;;; Code:
 
+(define (bootable-kernel-arguments kernel-arguments system root-device)
+  (cons* (string-append "--root=" root-device)
+         #~(string-append "--system=" #$system)
+         #~(string-append "--load=" #$system "/boot")
+         kernel-arguments))
+
 ;; System-wide configuration.
 ;; TODO: Add per-field docstrings/stexi.
 (define-record-type* <operating-system> operating-system
@@ -182,6 +188,11 @@
   (sudoers-file operating-system-sudoers-file     ; file-like
                 (default %sudoers-specification)))
 
+(define (operating-system-all-kernel-arguments os system root-device)
+  (bootable-kernel-arguments (operating-system-kernel-arguments os)
+                             system
+                             root-device))
+
 \f
 ;;;
 ;;; Services.
@@ -278,7 +289,7 @@ value of the SYSTEM-SERVICE-TYPE service."
         (mlet %store-monad
             ((kernel  ->  (operating-system-kernel os))
              (initrd      (operating-system-initrd-file os))
-             (params      (operating-system-parameters-file os)))
+             (params      (operating-system-boot-parameters-file os)))
           (return `(("kernel" ,kernel)
                     ("parameters" ,params)
                     ("initrd" ,initrd)
@@ -735,31 +746,13 @@ populate the \"old entries\" menu."
   (mlet* %store-monad
       ((system      (operating-system-derivation os))
        (root-fs ->  (operating-system-root-file-system os))
-       (store-fs -> (operating-system-store-file-system os))
-       (label ->    (kernel->boot-label (operating-system-kernel os)))
-       (kernel ->   (operating-system-kernel-file os))
-       (initrd      (operating-system-initrd-file os))
        (root-device -> (if (eq? 'uuid (file-system-title root-fs))
                            (uuid->string (file-system-device root-fs))
                            (file-system-device root-fs)))
-       (entries ->  (list (menu-entry
-                           (label label)
-
-                           ;; The device where the kernel and initrd live.
-                           (device (fs->boot-device store-fs))
-                           (device-mount-point
-                            (file-system-mount-point store-fs))
-
-                           (linux kernel)
-                           (linux-arguments
-                            (cons* (string-append "--root=" root-device)
-                                   #~(string-append "--system=" #$system)
-                                   #~(string-append "--load=" #$system
-                                                    "/boot")
-                                   (operating-system-kernel-arguments os)))
-                           (initrd initrd)))))
-    (grub-configuration-file (operating-system-bootloader os) entries
-                             #:old-entries old-entries)))
+       (entry (operating-system-boot-parameters os system root-device)))
+    (grub-configuration-file (operating-system-bootloader os)
+                             (list entry)
+                              #:old-entries old-entries)))
 
 (define (fs->boot-device fs)
   "Given FS, a <file-system> object, return a value suitable for use as the
@@ -769,26 +762,51 @@ device in a <menu-entry>."
     ((label) (file-system-device fs))
     (else #f)))
 
-(define (operating-system-parameters-file os)
+(define (operating-system-boot-parameters os system root-device)
+  "Return a monadic <boot-parameters> record that describes the boot parameters of OS.
+SYSTEM is optional.  If given, adds kernel arguments for that system to <boot-parameters>."
+  (mlet* %store-monad
+      ((initrd (operating-system-initrd-file os))
+       (store -> (operating-system-store-file-system os))
+       (label -> (kernel->boot-label (operating-system-kernel os))))
+    (return (boot-parameters
+             (label label)
+             (root-device root-device)
+             (kernel (operating-system-kernel-file os))
+             (kernel-arguments
+              (if system
+               (operating-system-all-kernel-arguments os system root-device)
+               (operating-system-kernel-arguments os)))
+             (initrd initrd)
+             (store-device (fs->boot-device store))
+             (store-mount-point (file-system-mount-point store))))))
+
+(define* (operating-system-boot-parameters-file os #:optional (system #f))
   "Return a file that describes the boot parameters of OS.  The primary use of
-this file is the reconstruction of GRUB menu entries for old configurations."
-  (mlet %store-monad ((initrd   (operating-system-initrd-file os))
-                      (root ->  (operating-system-root-file-system os))
-                      (store -> (operating-system-store-file-system os))
-                      (label -> (kernel->boot-label
-                                 (operating-system-kernel os))))
+this file is the reconstruction of GRUB menu entries for old configurations.
+SYSTEM is optional.  If given, adds kernel arguments for that system to the
+returned file (since the returned file is then usually stored into the
+content-addressed \"system\" directory, it's usually not a good idea
+to give it because the content hash would change by the content hash
+being stored into the \"parameters\" file)."
+  (mlet* %store-monad ( ;(initrd   (operating-system-initrd-file os))
+                       (root ->  (operating-system-root-file-system os))
+                       ;(store -> (operating-system-store-file-system os))
+                       ;(label -> (kernel->boot-label
+                       ;           (operating-system-kernel os)))
+                       (params (operating-system-boot-parameters os system (file-system-device root))))
     (gexp->file "parameters"
                 #~(boot-parameters
                    (version 0)
-                   (label #$label)
-                   (root-device #$(file-system-device root))
-                   (kernel #$(operating-system-kernel-file os))
+                   (label #$(boot-parameters-label params))
+                   (root-device #$(boot-parameters-root-device params))
+                   (kernel #$(boot-parameters-kernel params))
                    (kernel-arguments
-                    #$(operating-system-kernel-arguments os))
-                   (initrd #$initrd)
+                    #$(boot-parameters-kernel-arguments params))
+                   (initrd #$(boot-parameters-initrd params))
                    (store
-                    (device #$(fs->boot-device store))
-                    (mount-point #$(file-system-mount-point store))))
+                    (device #$(boot-parameters-store-device params))
+                    (mount-point #$(boot-parameters-store-mount-point params))))
                 #:set-load-path? #f)))
 
 \f
@@ -866,4 +884,24 @@ this file is the reconstruction of GRUB menu entries for old configurations."
               system)
      #f)))
 
+(define (read-boot-parameters-file sysgen)
+  "Read boot parameters from SYSGEN's (system or generation) \"parameters\"
+file and returns the corresponding <boot-parameters> object or #f if the
+format is unrecognized.
+The object has its kernel-arguments extended in order to make it bootable."
+  (let* ((file (string-append sysgen "/parameters"))
+         (params (call-with-input-file file read-boot-parameters))
+         (root (boot-parameters-root-device params))
+         (root-device (if (bytevector? root)
+                          (uuid->string root)
+                          root))
+         (kernel-arguments (boot-parameters-kernel-arguments params)))
+    (if params
+      (boot-parameters
+        (inherit params)
+        (kernel-arguments (bootable-kernel-arguments kernel-arguments
+                                                     sysgen
+                                                     root-device)))
+      #f)))
+
 ;;; system.scm ends here
diff --git a/gnu/system/grub.scm b/gnu/system/grub.scm
index cde4b9e23..f2838d633 100644
--- a/gnu/system/grub.scm
+++ b/gnu/system/grub.scm
@@ -267,6 +267,15 @@ code."
         (#f
          #~(format #f "search --file --set ~a" #$file)))))
 
+(define (boot-parameters->menu-entry conf)
+  (menu-entry
+   (label (boot-parameters-label conf))
+   (device (boot-parameters-store-device conf))
+   (device-mount-point (boot-parameters-store-mount-point conf))
+   (linux (boot-parameters-kernel conf))
+   (linux-arguments (boot-parameters-kernel-arguments conf))
+   (initrd (boot-parameters-initrd conf))))
+
 (define* (grub-configuration-file config entries
                                   #:key
                                   (system (%current-system))
@@ -276,7 +285,7 @@ code."
 <file-system> object.  OLD-ENTRIES is taken to be a list of menu entries
 corresponding to old generations of the system."
   (define all-entries
-    (append entries
+    (append (map boot-parameters->menu-entry entries)
             (grub-configuration-menu-entries config)))
 
   (define entry->gexp
@@ -323,7 +332,7 @@ set timeout=~a~%"
             #$@(if (pair? old-entries)
                    #~((format port "
 submenu \"GNU system, old configurations...\" {~%")
-                      #$@(map entry->gexp old-entries)
+                      #$@(map entry->gexp (map boot-parameters->menu-entry old-entries))
                       (format port "}~%"))
                    #~()))))
 
diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index 374d8b663..40db61abb 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -490,12 +490,10 @@ it is mostly useful when FULL-BOOT?  is true."
                                 #:full-boot? full-boot?
                                 #:disk-image-size disk-image-size)))
     (define kernel-arguments
-      #~(list "--root=/dev/vda1"
-              (string-append "--system=" #$os-drv)
-              (string-append "--load=" #$os-drv "/boot")
-              #$@(if graphic? #~() #~("console=ttyS0"))
-              #+@(operating-system-kernel-arguments os)))
-
+      #~(list #$@(if graphic? #~() #~("console=ttyS0"))
+              #+@(operating-system-all-kernel-arguments os
+                                                        os-drv
+                                                        "/dev/vda1")))
     (define qemu-exec
       #~(list (string-append #$qemu "/bin/" #$(qemu-command (%current-system)))
               #$@(if full-boot?
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 8fabdb5c1..6a99d1fca 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -370,8 +370,7 @@ NUMBERS, which is a list of generation numbers."
   (define (system->boot-parameters system number time)
     (unless-file-not-found
      (let* ((file             (string-append system "/parameters"))
-            (params           (call-with-input-file file
-                                read-boot-parameters)))
+            (params           (read-boot-parameters-file file)))
        params)))
   (let* ((systems (map (cut generation-file-name profile <>)
                        numbers))
@@ -381,44 +380,26 @@ NUMBERS, which is a list of generation numbers."
                        systems)))
     (filter-map system->boot-parameters systems numbers times)))
 
-(define* (profile-grub-entries #:optional (profile %system-profile)
+(define* (profile-boot-parameters #:optional (profile %system-profile)
                                   (numbers (generation-numbers profile)))
-  "Return a list of 'menu-entry' for the generations of PROFILE specified by
+  "Return a list of 'boot-parameters' for the generations of PROFILE specified by
 NUMBERS, which is a list of generation numbers."
-  (define (system->grub-entry system number time)
+  (define (system->boot-parameters system number time)
     (unless-file-not-found
-     (let* ((file             (string-append system "/parameters"))
-            (params           (call-with-input-file file
-                                read-boot-parameters))
-            (label            (boot-parameters-label params))
-            (root             (boot-parameters-root-device params))
-            (root-device      (if (bytevector? root)
-                                  (uuid->string root)
-                                  root))
-            (kernel           (boot-parameters-kernel params))
-            (kernel-arguments (boot-parameters-kernel-arguments params))
-            (initrd           (boot-parameters-initrd params)))
-       (menu-entry
-        (label (string-append label " (#"
-                              (number->string number) ", "
-                              (seconds->string time) ")"))
-        (device (boot-parameters-store-device params))
-        (device-mount-point (boot-parameters-store-mount-point params))
-        (linux kernel)
-        (linux-arguments
-         (cons* (string-append "--root=" root-device)
-                (string-append "--system=" system)
-                (string-append "--load=" system "/boot")
-                kernel-arguments))
-        (initrd initrd)))))
-
+     (let* ((params           (read-boot-parameters-file system))
+            (label            (boot-parameters-label params)))
+       (boot-parameters
+         (inherit params)
+         (label (string-append label " (#"
+                               (number->string number) ", "
+                               (seconds->string time) ")"))))))
   (let* ((systems (map (cut generation-file-name profile <>)
                        numbers))
          (times   (map (lambda (system)
                          (unless-file-not-found
                           (stat:mtime (lstat system))))
                        systems)))
-    (filter-map system->grub-entry systems numbers times)))
+    (filter-map system->boot-parameters systems numbers times)))
 
 \f
 ;;;
@@ -447,18 +428,16 @@ generation as its default entry.  STORE is an open connection to the store."
   "Re-install grub for existing system profile generation NUMBER.  STORE is an
 open connection to the store."
   (let* ((generation (generation-file-name %system-profile number))
-         (file (string-append generation "/parameters"))
-         (params (unless-file-not-found
-                  (call-with-input-file file read-boot-parameters)))
+         (params (read-boot-parameters-file generation))
          (root-device (boot-parameters-root-device params))
          ;; We don't currently keep track of past menu entries' details.  The
          ;; default values will allow the system to boot, even if they differ
          ;; from the actual past values for this generation's entry.
          (grub-config (grub-configuration (device root-device)))
          ;; Make the specified system generation the default entry.
-         (entries (profile-grub-entries %system-profile (list number)))
+         (entries (profile-boot-parameters %system-profile (list number)))
          (old-generations (delv number (generation-numbers %system-profile)))
-         (old-entries (profile-grub-entries %system-profile old-generations))
+         (old-entries (profile-boot-parameters %system-profile old-generations))
          (grub.cfg (run-with-store store
                      (grub-configuration-file grub-config
                                               entries
@@ -533,8 +512,7 @@ list of services."
   "Display a summary of system generation NUMBER in a human-readable format."
   (unless (zero? number)
     (let* ((generation  (generation-file-name profile number))
-           (param-file  (string-append generation "/parameters"))
-           (params      (call-with-input-file param-file read-boot-parameters))
+           (params      (read-boot-parameters-file generation))
            (label       (boot-parameters-label params))
            (root        (boot-parameters-root-device params))
            (root-device (if (bytevector? root)
@@ -643,7 +621,7 @@ output when building a system derivation, such as a disk image."
                       (operating-system-bootcfg os
                                                 (if (eq? 'init action)
                                                     '()
-                                                    (profile-grub-entries)))))
+                                                    (profile-boot-parameters)))))
 
        ;; For 'init' and 'reconfigure', always build GRUB.CFG, even if
        ;; --no-grub is passed, because GRUB.CFG because we then use it as a GC

bug#26544: [PATCH] system: Move "--load" and other guix-specific parameters from the grub module to the generic system module.

[Mathieu Othacehe]

Hi Danny,

I'll review it more carefully tomorrow, but here are first comments :

> +  (mlet* %store-monad ( ;(initrd   (operating-system-initrd-file os))

Commented lines.

> +                       (root ->  (operating-system-root-file-system os))
> +                       ;(store -> (operating-system-store-file-system os))
> +                       ;(label -> (kernel->boot-label
> +                       ;           (operating-system-kernel os)))
> +                       (params (operating-system-boot-parameters os system (file-system-device root))))

This line is way over 80 chars.

Thanks,

Mathieu

bug#26544: [PATCH] system: Move "--load" and other guix-specific parameters from the grub module to the generic system module.

[Ludovic Courtès]

Hi Danny,

Another quick preliminary review…

Could you explain the “big picture”, the rationale behind this change?

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> +(define (bootable-kernel-arguments kernel-arguments system root-device)
> +  (cons* (string-append "--root=" root-device)
> +         #~(string-append "--system=" #$system)
> +         #~(string-append "--load=" #$system "/boot")
> +         kernel-arguments))

Please add a docstring to top-level procedures:

  https://www.gnu.org/software/guix/manual/html_node/Formatting-Code.html

> +(define (operating-system-all-kernel-arguments os system root-device)
> +  (bootable-kernel-arguments (operating-system-kernel-arguments os)
> +                             system
> +                             root-device))

For services, we have:

  operating-system-user-services
  operating-system-services

So I suggest stick to this convention and thus have:

  operating-system-user-kernel-arguments     ;arguments specified by the user
  operating-system-kernel-arguments          ;all the arguments

> -(define (operating-system-parameters-file os)
> +(define (operating-system-boot-parameters os system root-device)

I think it would be clearer to rename in a separate patch.

> +(define (read-boot-parameters-file sysgen)
> +  "Read boot parameters from SYSGEN's (system or generation) \"parameters\"

Please use full words in variable names, typically ‘system’ here.

Thanks,
Ludo’.

bug#26544: [PATCH] system: Move "--load" and other guix-specific parameters from the grub module to the generic system module.

[Danny Milosavljevic]

Hi Ludo,

> Could you explain the “big picture”, the rationale behind this change?

Currenly, (gnu system grub) prepends the "--system", "--load" and "--root" on its own.  But that's something specific to the guix system and has nothing to do with bootloaders (it's not even a bootloader option - it's just passing it through to the Linux kernel and *that* is passing some through to Shepherd).

In connection with the effort to support other bootloaders we could of course copy such a prepension-block to each of the bootloader modules, but in my opinion it's better if it's moved to (bootloader-independent) boot-parameters so everyone just automatically uses the correct kernel arguments, including which guix system guix should boot.

I suspect that this wasn't done before because the boot-parameters are serialized to a file "parameters" in the "system" directory.  But then "--system=<system>" in that file would have to contain the hash value of the system, which is ... impossible (or at least very very hard to do).

Therefore, this patch makes sure the in-memory <boot-parameters> instances do contain "--system" with the correct hash value, but the stored "parameters" doesn't.

I think that that's the best of both worlds.  Clients in guix can use data read from <boot-parameters> without second-guessing everything and the stored file still doesn't have to solve a hash-functions-really-try-to-prevent-you-from-doing-that problem - the stored file doesn't contain the hash value of itself (and now doesn't need to).

I've successfully tested it by "guix system reconfigure" on my machine and also by "guix system vm" (full-boot and non-full-boot) on my machine, starting from master.

> Danny Milosavljevic <dannym@scratchpost.org> skribis:
> 
> > +(define (bootable-kernel-arguments kernel-arguments system root-device)

> Please add a docstring to top-level procedures:

Something like this?

"Prepend extra arguments to KERNEL-ARGUMENTS that allow the guix system to boot SYSTEM on ROOT-DEVICE."

> So I suggest stick to this convention and thus have:
> 
>   operating-system-user-kernel-arguments     ;arguments specified by the user
>   operating-system-kernel-arguments          ;all the arguments

Hmm, I don't think it's really directly specified by the user, is it?  I'm just trying to avoid having to store the file's (more or less) own hash value into the file.

> > -(define (operating-system-parameters-file os)
> > +(define (operating-system-boot-parameters os system root-device)  
> 
> I think it would be clearer to rename in a separate patch.

I'll try.

> > +(define (read-boot-parameters-file sysgen)
> > +  "Read boot parameters from SYSGEN's (system or generation) \"parameters\"  
> 
> Please use full words in variable names, typically ‘system’ here.

Ok.  Should the docstring say "SYSTEM (system or generation)" or just "SYSTEM"?

bug#26544: [PATCH] system: Move "--load" and other guix-specific parameters from the grub module to the generic system module.

[Ludovic Courtès]

Hello!

Danny Milosavljevic <dannym@scratchpost.org> skribis:

>> Could you explain the “big picture”, the rationale behind this
>> change?
>
> Currenly, (gnu system grub) prepends the "--system", "--load" and
> "--root" on its own.  But that's something specific to the guix system
> and has nothing to do with bootloaders (it's not even a bootloader
> option - it's just passing it through to the Linux kernel and *that*
> is passing some through to Shepherd).
>
> In connection with the effort to support other bootloaders we could of
> course copy such a prepension-block to each of the bootloader modules,
> but in my opinion it's better if it's moved to
> (bootloader-independent) boot-parameters so everyone just
> automatically uses the correct kernel arguments, including which guix
> system guix should boot.

OK, that makes a lot of sense, indeed.

> I suspect that this wasn't done before because the boot-parameters are
> serialized to a file "parameters" in the "system" directory.  But then
> "--system=<system>" in that file would have to contain the hash value
> of the system, which is ... impossible (or at least very very hard to
> do).
>
> Therefore, this patch makes sure the in-memory <boot-parameters>
> instances do contain "--system" with the correct hash value, but the
> stored "parameters" doesn't.

OK, I see.  I’ll look more closely at the patch but I think it’s fine to
treat --system specially, for the reasons you gave.

>> Danny Milosavljevic <dannym@scratchpost.org> skribis:
>> 
>> > +(define (bootable-kernel-arguments kernel-arguments system
>> > root-device)
>
>> Please add a docstring to top-level procedures:
>
> Something like this?
>
> "Prepend extra arguments to KERNEL-ARGUMENTS that allow the guix
> system to boot SYSTEM on ROOT-DEVICE."

Yeah, something like that (“that allow SYSTEM to be botted from
ROOT-DEVICE” maybe.)

>> So I suggest stick to this convention and thus have:
>> 
>>   operating-system-user-kernel-arguments ;arguments specified by the
>> user operating-system-kernel-arguments ;all the arguments
>
> Hmm, I don't think it's really directly specified by the user, is it?
> I'm just trying to avoid having to store the file's (more or less) own
> hash value into the file.

I’ll have to check.

>> > -(define (operating-system-parameters-file os) +(define
>> > (operating-system-boot-parameters os system root-device)
>>  I think it would be clearer to rename in a separate patch.
>
> I'll try.
>
>> > +(define (read-boot-parameters-file sysgen) + "Read boot
>> > parameters from SYSGEN's (system or generation) \"parameters\"
>>  Please use full words in variable names, typically ‘system’ here.
>
> Ok.  Should the docstring say "SYSTEM (system or generation)" or just
> "SYSTEM"?

The former is clearer IMO.

Thank you,
Ludo’.

bug#26544: [PATCH v2 0/8] system: Move "--load" and other guix-specific parameters from the grub module to the generic system module.

[Danny Milosavljevic]

Danny Milosavljevic (8):
  system: Rename operating-system-kernel-arguments to
    operating-system-user-kernel-arguments.
  system: Rename operating-system-parameters-file to
    operating-system-boot-parameters-file.
  system: Factorize operating-system-boot-parameters-file.
  system: Introduce operating-system-kernel-arguments and use it.
  system: Introduce read-boot-parameters-file.
  system: vm: Use operating-system-kernel-arguments.
  system: Use operating-system-boot-parameters directly.
  system: grub: Use boot-parameters instead of menu-entry where
    possible.

 gnu/system.scm          | 133 +++++++++++++++++++++++++++++++-----------------
 gnu/system/grub.scm     |  14 ++++-
 gnu/system/vm.scm       |   7 +--
 guix/scripts/system.scm |  24 ++++-----
 4 files changed, 110 insertions(+), 68 deletions(-)

bug#26544: [PATCH v2 1/8] system: Rename operating-system-kernel-arguments to operating-system-user-kernel-arguments.

[Danny Milosavljevic]

* gnu/system.scm (operating-system-kernel-arguments): Rename to ...
(operating-system-user-kernel-arguments): ... this.
(<operating-system>): Adapt accordingly.
(operating-system-bootcfg): Adapt accordingly.
(operating-system-parameters-file): Adapt accordingly.
* gnu/system/vm.scm (system-qemu-image/shared-store-script): Adapt
accordingly.
---
 gnu/system.scm    | 8 ++++----
 gnu/system/vm.scm | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index f6ab7ded8..4032e8e15 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -73,7 +73,7 @@
             operating-system-hosts-file
             operating-system-kernel
             operating-system-kernel-file
-            operating-system-kernel-arguments
+            operating-system-user-kernel-arguments
             operating-system-initrd
             operating-system-users
             operating-system-groups
@@ -129,7 +129,7 @@
   operating-system?
   (kernel operating-system-kernel                 ; package
           (default linux-libre))
-  (kernel-arguments operating-system-kernel-arguments
+  (kernel-arguments operating-system-user-kernel-arguments
                     (default '()))                ; list of gexps/strings
   (bootloader operating-system-bootloader)        ; <grub-configuration>
 
@@ -756,7 +756,7 @@ populate the \"old entries\" menu."
                                    #~(string-append "--system=" #$system)
                                    #~(string-append "--load=" #$system
                                                     "/boot")
-                                   (operating-system-kernel-arguments os)))
+                                   (operating-system-user-kernel-arguments os)))
                            (initrd initrd)))))
     (grub-configuration-file (operating-system-bootloader os) entries
                              #:old-entries old-entries)))
@@ -784,7 +784,7 @@ this file is the reconstruction of GRUB menu entries for old configurations."
                    (root-device #$(file-system-device root))
                    (kernel #$(operating-system-kernel-file os))
                    (kernel-arguments
-                    #$(operating-system-kernel-arguments os))
+                    #$(operating-system-user-kernel-arguments os))
                    (initrd #$initrd)
                    (store
                     (device #$(fs->boot-device store))
diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index 374d8b663..4f915c4f9 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -494,7 +494,7 @@ it is mostly useful when FULL-BOOT?  is true."
               (string-append "--system=" #$os-drv)
               (string-append "--load=" #$os-drv "/boot")
               #$@(if graphic? #~() #~("console=ttyS0"))
-              #+@(operating-system-kernel-arguments os)))
+              #+@(operating-system-user-kernel-arguments os)))
 
     (define qemu-exec
       #~(list (string-append #$qemu "/bin/" #$(qemu-command (%current-system)))

bug#26544: [PATCH v2 1/8] system: Rename operating-system-kernel-arguments to operating-system-user-kernel-arguments.

[Mathieu Othacehe]

LGTM!

Danny Milosavljevic writes:

> * gnu/system.scm (operating-system-kernel-arguments): Rename to ...
> (operating-system-user-kernel-arguments): ... this.
> (<operating-system>): Adapt accordingly.
> (operating-system-bootcfg): Adapt accordingly.
> (operating-system-parameters-file): Adapt accordingly.
> * gnu/system/vm.scm (system-qemu-image/shared-store-script): Adapt
> accordingly.
> ---
>  gnu/system.scm    | 8 ++++----
>  gnu/system/vm.scm | 2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/gnu/system.scm b/gnu/system.scm
> index f6ab7ded8..4032e8e15 100644
> --- a/gnu/system.scm
> +++ b/gnu/system.scm
> @@ -73,7 +73,7 @@
>              operating-system-hosts-file
>              operating-system-kernel
>              operating-system-kernel-file
> -            operating-system-kernel-arguments
> +            operating-system-user-kernel-arguments
>              operating-system-initrd
>              operating-system-users
>              operating-system-groups
> @@ -129,7 +129,7 @@
>    operating-system?
>    (kernel operating-system-kernel                 ; package
>            (default linux-libre))
> -  (kernel-arguments operating-system-kernel-arguments
> +  (kernel-arguments operating-system-user-kernel-arguments
>                      (default '()))                ; list of gexps/strings
>    (bootloader operating-system-bootloader)        ; <grub-configuration>
>  
> @@ -756,7 +756,7 @@ populate the \"old entries\" menu."
>                                     #~(string-append "--system=" #$system)
>                                     #~(string-append "--load=" #$system
>                                                      "/boot")
> -                                   (operating-system-kernel-arguments os)))
> +                                   (operating-system-user-kernel-arguments os)))
>                             (initrd initrd)))))
>      (grub-configuration-file (operating-system-bootloader os) entries
>                               #:old-entries old-entries)))
> @@ -784,7 +784,7 @@ this file is the reconstruction of GRUB menu entries for old configurations."
>                     (root-device #$(file-system-device root))
>                     (kernel #$(operating-system-kernel-file os))
>                     (kernel-arguments
> -                    #$(operating-system-kernel-arguments os))
> +                    #$(operating-system-user-kernel-arguments os))
>                     (initrd #$initrd)
>                     (store
>                      (device #$(fs->boot-device store))
> diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
> index 374d8b663..4f915c4f9 100644
> --- a/gnu/system/vm.scm
> +++ b/gnu/system/vm.scm
> @@ -494,7 +494,7 @@ it is mostly useful when FULL-BOOT?  is true."
>                (string-append "--system=" #$os-drv)
>                (string-append "--load=" #$os-drv "/boot")
>                #$@(if graphic? #~() #~("console=ttyS0"))
> -              #+@(operating-system-kernel-arguments os)))
> +              #+@(operating-system-user-kernel-arguments os)))
>  
>      (define qemu-exec
>        #~(list (string-append #$qemu "/bin/" #$(qemu-command (%current-system)))

bug#26544: [PATCH v2 1/8] system: Rename operating-system-kernel-arguments to operating-system-user-kernel-arguments.

[Danny Milosavljevic]

Applied this patch to master as af98d25a1286e246e8da36d6d63b4d66e58f2cf8 and pushed.

bug#26544: [PATCH v2 2/8] system: Rename operating-system-parameters-file to operating-system-boot-parameters-file.

[Danny Milosavljevic]

* gnu/system.scm (operating-system-parameters-file): Rename to ...
(operating-system-boot-parameters-file): ... this.
(operating-system-directory-base-entries): Adapt call site.
---
 gnu/system.scm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index 4032e8e15..44190bdfc 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -278,7 +278,7 @@ value of the SYSTEM-SERVICE-TYPE service."
         (mlet %store-monad
             ((kernel  ->  (operating-system-kernel os))
              (initrd      (operating-system-initrd-file os))
-             (params      (operating-system-parameters-file os)))
+             (params      (operating-system-boot-parameters-file os)))
           (return `(("kernel" ,kernel)
                     ("parameters" ,params)
                     ("initrd" ,initrd)
@@ -769,7 +769,7 @@ device in a <menu-entry>."
     ((label) (file-system-device fs))
     (else #f)))
 
-(define (operating-system-parameters-file os)
+(define (operating-system-boot-parameters-file os)
   "Return a file that describes the boot parameters of OS.  The primary use of
 this file is the reconstruction of GRUB menu entries for old configurations."
   (mlet %store-monad ((initrd   (operating-system-initrd-file os))

bug#26544: [PATCH v2 2/8] system: Rename operating-system-parameters-file to operating-system-boot-parameters-file.

[Mathieu Othacehe]

LGTM!

Danny Milosavljevic writes:

> * gnu/system.scm (operating-system-parameters-file): Rename to ...
> (operating-system-boot-parameters-file): ... this.
> (operating-system-directory-base-entries): Adapt call site.
> ---
>  gnu/system.scm | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gnu/system.scm b/gnu/system.scm
> index 4032e8e15..44190bdfc 100644
> --- a/gnu/system.scm
> +++ b/gnu/system.scm
> @@ -278,7 +278,7 @@ value of the SYSTEM-SERVICE-TYPE service."
>          (mlet %store-monad
>              ((kernel  ->  (operating-system-kernel os))
>               (initrd      (operating-system-initrd-file os))
> -             (params      (operating-system-parameters-file os)))
> +             (params      (operating-system-boot-parameters-file os)))
>            (return `(("kernel" ,kernel)
>                      ("parameters" ,params)
>                      ("initrd" ,initrd)
> @@ -769,7 +769,7 @@ device in a <menu-entry>."
>      ((label) (file-system-device fs))
>      (else #f)))
>  
> -(define (operating-system-parameters-file os)
> +(define (operating-system-boot-parameters-file os)
>    "Return a file that describes the boot parameters of OS.  The primary use of
>  this file is the reconstruction of GRUB menu entries for old configurations."
>    (mlet %store-monad ((initrd   (operating-system-initrd-file os))

bug#26544: [PATCH v2 2/8] system: Rename operating-system-parameters-file to operating-system-boot-parameters-file.

[Danny Milosavljevic]

Applied this patch to master as 71d04202026e2061f898a142a8381d55bee5fb00 and pushed.

bug#26544: [PATCH v2 3/8] system: Factorize operating-system-boot-parameters-file.

[Danny Milosavljevic]

* gnu/system.scm (operating-system-boot-parameters): New variable.
(operating-system-boot-parameters-file): Modify.
---
 gnu/system.scm | 61 ++++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 40 insertions(+), 21 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index 44190bdfc..65803e25b 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -769,27 +769,46 @@ device in a <menu-entry>."
     ((label) (file-system-device fs))
     (else #f)))
 
-(define (operating-system-boot-parameters-file os)
-  "Return a file that describes the boot parameters of OS.  The primary use of
-this file is the reconstruction of GRUB menu entries for old configurations."
-  (mlet %store-monad ((initrd   (operating-system-initrd-file os))
-                      (root ->  (operating-system-root-file-system os))
-                      (store -> (operating-system-store-file-system os))
-                      (label -> (kernel->boot-label
-                                 (operating-system-kernel os))))
-    (gexp->file "parameters"
-                #~(boot-parameters
-                   (version 0)
-                   (label #$label)
-                   (root-device #$(file-system-device root))
-                   (kernel #$(operating-system-kernel-file os))
-                   (kernel-arguments
-                    #$(operating-system-user-kernel-arguments os))
-                   (initrd #$initrd)
-                   (store
-                    (device #$(fs->boot-device store))
-                    (mount-point #$(file-system-mount-point store))))
-                #:set-load-path? #f)))
+(define (operating-system-boot-parameters os system root-device)
+  "Return a monadic <boot-parameters> record that describes the boot parameters of OS.
+SYSTEM is optional.  If given, adds kernel arguments for that system to <boot-parameters>."
+  (mlet* %store-monad
+      ((initrd (operating-system-initrd-file os))
+       (store -> (operating-system-store-file-system os))
+       (label -> (kernel->boot-label (operating-system-kernel os))))
+    (return (boot-parameters
+             (label label)
+             (root-device root-device)
+             (kernel (operating-system-kernel-file os))
+             (kernel-arguments
+              (operating-system-user-kernel-arguments os))
+             (initrd initrd)
+             (store-device (fs->boot-device store))
+             (store-mount-point (file-system-mount-point store))))))
+
+(define* (operating-system-boot-parameters-file os #:optional (system #f))
+   "Return a file that describes the boot parameters of OS.  The primary use of
+this file is the reconstruction of GRUB menu entries for old configurations.
+SYSTEM is optional.  If given, adds kernel arguments for that system to the
+returned file (since the returned file is then usually stored into the
+content-addressed \"system\" directory, it's usually not a good idea
+to give it because the content hash would change by the content hash
+being stored into the \"parameters\" file)."
+  (mlet* %store-monad ((root -> (operating-system-root-file-system os))
+                       (params (operating-system-boot-parameters os system (file-system-device root))))
+     (gexp->file "parameters"
+                 #~(boot-parameters
+                    (version 0)
+                    (label #$(boot-parameters-label params))
+                    (root-device #$(boot-parameters-root-device params))
+                    (kernel #$(boot-parameters-kernel params))
+                    (kernel-arguments
+                     #$(boot-parameters-kernel-arguments params))
+                    (initrd #$(boot-parameters-initrd params))
+                    (store
+                     (device #$(boot-parameters-store-device params))
+                     (mount-point #$(boot-parameters-store-mount-point params))))
+                 #:set-load-path? #f)))
 
 \f
 ;;;

bug#26544: [PATCH v2 3/8] system: Factorize operating-system-boot-parameters-file.

[Mathieu Othacehe]

> +                       (params (operating-system-boot-parameters os system (file-system-device root))))

This line is too long.

Otherwise LGTM !

bug#26544: [PATCH v2 4/8] system: Introduce operating-system-kernel-arguments and use it.

[Danny Milosavljevic]

* gnu/system.scm (bootable-kernel-arguments): New variable.
(operating-system-kernel-arguments):  New variable.
(operating-system-bootcfg): Use operating-system-kernel-arguments.
(operating-system-boot-parameters): Use operating-system-kernel-arguments.
---
 gnu/system.scm | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index 65803e25b..31764c830 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -73,7 +73,7 @@
             operating-system-hosts-file
             operating-system-kernel
             operating-system-kernel-file
-            operating-system-user-kernel-arguments
+            operating-system-kernel-arguments
             operating-system-initrd
             operating-system-users
             operating-system-groups
@@ -122,6 +122,14 @@
 ;;;
 ;;; Code:
 
+(define (bootable-kernel-arguments kernel-arguments system root-device)
+  "Prepend extra arguments to KERNEL-ARGUMENTS that allow SYSTEM to be
+booted from ROOT-DEVICE"
+  (cons* (string-append "--root=" root-device)
+         #~(string-append "--system=" #$system)
+         #~(string-append "--load=" #$system "/boot")
+         kernel-arguments))
+
 ;; System-wide configuration.
 ;; TODO: Add per-field docstrings/stexi.
 (define-record-type* <operating-system> operating-system
@@ -182,6 +190,13 @@
   (sudoers-file operating-system-sudoers-file     ; file-like
                 (default %sudoers-specification)))
 
+(define (operating-system-kernel-arguments os system root-device)
+  "Return all the kernel arguments, including the ones not specified
+directly by the user."
+  (bootable-kernel-arguments (operating-system-user-kernel-arguments os)
+                             system
+                             root-device))
+
 \f
 ;;;
 ;;; Services.
@@ -756,7 +771,9 @@ populate the \"old entries\" menu."
                                    #~(string-append "--system=" #$system)
                                    #~(string-append "--load=" #$system
                                                     "/boot")
-                                   (operating-system-user-kernel-arguments os)))
+                                   (operating-system-kernel-arguments os
+                                                                      system
+                                                                      root-device)))
                            (initrd initrd)))))
     (grub-configuration-file (operating-system-bootloader os) entries
                              #:old-entries old-entries)))
@@ -781,7 +798,9 @@ SYSTEM is optional.  If given, adds kernel arguments for that system to <boot-pa
              (root-device root-device)
              (kernel (operating-system-kernel-file os))
              (kernel-arguments
-              (operating-system-user-kernel-arguments os))
+              (if system
+               (operating-system-kernel-arguments os system root-device)
+               (operating-system-user-kernel-arguments os)))
              (initrd initrd)
              (store-device (fs->boot-device store))
              (store-mount-point (file-system-mount-point store))))))
@@ -795,7 +814,10 @@ content-addressed \"system\" directory, it's usually not a good idea
 to give it because the content hash would change by the content hash
 being stored into the \"parameters\" file)."
   (mlet* %store-monad ((root -> (operating-system-root-file-system os))
-                       (params (operating-system-boot-parameters os system (file-system-device root))))
+                       (device -> (file-system-device root))
+                       (params (operating-system-boot-parameters os
+                                                                 system
+                                                                 device)))
      (gexp->file "parameters"
                  #~(boot-parameters
                     (version 0)

bug#26544: [PATCH v2 4/8] system: Introduce operating-system-kernel-arguments and use it.

[Mathieu Othacehe]

> +(define (bootable-kernel-arguments kernel-arguments system root-device)
> +  "Prepend extra arguments to KERNEL-ARGUMENTS that allow SYSTEM to be
> +booted from ROOT-DEVICE"

You could precise in this docstring that system is a derivation.

Mathieu

bug#26544: [PATCH v2 5/8] system: Introduce read-boot-parameters-file.

[Danny Milosavljevic]

* gnu/system.scm (read-boot-parameters): Remove export.
(read-boot-parameters-file): New variable. Export it.
* guix/scripts/system.scm (profile-boot-parameters): Use
read-boot-parameters-file.
(profile-grub-entries): Use read-boot-parameters-file.
(reinstall-grub): Use read-boot-parameters-file.
(display-system-generation): Use read-boot-parameters-file.
---
 gnu/system.scm          | 22 +++++++++++++++++++++-
 guix/scripts/system.scm | 10 ++++------
 2 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index 31764c830..cb166c755 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -109,7 +109,7 @@
             boot-parameters-kernel
             boot-parameters-kernel-arguments
             boot-parameters-initrd
-            read-boot-parameters
+            read-boot-parameters-file
 
             local-host-aliases
             %setuid-programs
@@ -907,4 +907,24 @@ being stored into the \"parameters\" file)."
               system)
      #f)))
 
+(define (read-boot-parameters-file system)
+  "Read boot parameters from SYSTEM's (system or generation) \"parameters\"
+file and returns the corresponding <boot-parameters> object or #f if the
+format is unrecognized.
+The object has its kernel-arguments extended in order to make it bootable."
+  (let* ((file (string-append system "/parameters"))
+         (params (call-with-input-file file read-boot-parameters))
+         (root (boot-parameters-root-device params))
+         (root-device (if (bytevector? root)
+                          (uuid->string root)
+                          root))
+         (kernel-arguments (boot-parameters-kernel-arguments params)))
+    (if params
+      (boot-parameters
+        (inherit params)
+        (kernel-arguments (bootable-kernel-arguments kernel-arguments
+                                                     system
+                                                     root-device)))
+      #f)))
+
 ;;; system.scm ends here
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 8fabdb5c1..378138c26 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -370,8 +370,7 @@ NUMBERS, which is a list of generation numbers."
   (define (system->boot-parameters system number time)
     (unless-file-not-found
      (let* ((file             (string-append system "/parameters"))
-            (params           (call-with-input-file file
-                                read-boot-parameters)))
+            (params           (read-boot-parameters-file file)))
        params)))
   (let* ((systems (map (cut generation-file-name profile <>)
                        numbers))
@@ -388,8 +387,7 @@ NUMBERS, which is a list of generation numbers."
   (define (system->grub-entry system number time)
     (unless-file-not-found
      (let* ((file             (string-append system "/parameters"))
-            (params           (call-with-input-file file
-                                read-boot-parameters))
+            (params           (read-boot-parameters-file file))
             (label            (boot-parameters-label params))
             (root             (boot-parameters-root-device params))
             (root-device      (if (bytevector? root)
@@ -449,7 +447,7 @@ open connection to the store."
   (let* ((generation (generation-file-name %system-profile number))
          (file (string-append generation "/parameters"))
          (params (unless-file-not-found
-                  (call-with-input-file file read-boot-parameters)))
+                  (read-boot-parameters-file file)))
          (root-device (boot-parameters-root-device params))
          ;; We don't currently keep track of past menu entries' details.  The
          ;; default values will allow the system to boot, even if they differ
@@ -534,7 +532,7 @@ list of services."
   (unless (zero? number)
     (let* ((generation  (generation-file-name profile number))
            (param-file  (string-append generation "/parameters"))
-           (params      (call-with-input-file param-file read-boot-parameters))
+           (params      (read-boot-parameters-file param-file))
            (label       (boot-parameters-label params))
            (root        (boot-parameters-root-device params))
            (root-device (if (bytevector? root)

bug#26544: [PATCH v2 5/8] system: Introduce read-boot-parameters-file.

[Mathieu Othacehe]

LGTM!

bug#26544: [PATCH v2 6/8] system: vm: Use operating-system-kernel-arguments.

[Danny Milosavljevic]

* gnu/system/vm.scm (system-qemu-image/shared-store-script):
Use operating-system-kernel-arguments.
---
 gnu/system/vm.scm | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index 4f915c4f9..2c8b954c8 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -490,11 +490,8 @@ it is mostly useful when FULL-BOOT?  is true."
                                 #:full-boot? full-boot?
                                 #:disk-image-size disk-image-size)))
     (define kernel-arguments
-      #~(list "--root=/dev/vda1"
-              (string-append "--system=" #$os-drv)
-              (string-append "--load=" #$os-drv "/boot")
-              #$@(if graphic? #~() #~("console=ttyS0"))
-              #+@(operating-system-user-kernel-arguments os)))
+      #~(list #$@(if graphic? #~() #~("console=ttyS0"))
+              #+@(operating-system-kernel-arguments os os-drv "/dev/vda1")))
 
     (define qemu-exec
       #~(list (string-append #$qemu "/bin/" #$(qemu-command (%current-system)))

bug#26544: [PATCH v2 6/8] system: vm: Use operating-system-kernel-arguments.

[Mathieu Othacehe]

LGTM!

Danny Milosavljevic writes:

> * gnu/system/vm.scm (system-qemu-image/shared-store-script):
> Use operating-system-kernel-arguments.
> ---
>  gnu/system/vm.scm | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
> index 4f915c4f9..2c8b954c8 100644
> --- a/gnu/system/vm.scm
> +++ b/gnu/system/vm.scm
> @@ -490,11 +490,8 @@ it is mostly useful when FULL-BOOT?  is true."
>                                  #:full-boot? full-boot?
>                                  #:disk-image-size disk-image-size)))
>      (define kernel-arguments
> -      #~(list "--root=/dev/vda1"
> -              (string-append "--system=" #$os-drv)
> -              (string-append "--load=" #$os-drv "/boot")
> -              #$@(if graphic? #~() #~("console=ttyS0"))
> -              #+@(operating-system-user-kernel-arguments os)))
> +      #~(list #$@(if graphic? #~() #~("console=ttyS0"))
> +              #+@(operating-system-kernel-arguments os os-drv "/dev/vda1")))
>  
>      (define qemu-exec
>        #~(list (string-append #$qemu "/bin/" #$(qemu-command (%current-system)))

bug#26544: [PATCH v2 7/8] system: Use operating-system-boot-parameters directly.

[Danny Milosavljevic]

* gnu/system.scm (operating-system-bootcfg): Use
operating-system-boot-parameters directly.
---
 gnu/system.scm | 28 ++++------------------------
 1 file changed, 4 insertions(+), 24 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index cb166c755..013bd5356 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -750,33 +750,13 @@ populate the \"old entries\" menu."
   (mlet* %store-monad
       ((system      (operating-system-derivation os))
        (root-fs ->  (operating-system-root-file-system os))
-       (store-fs -> (operating-system-store-file-system os))
-       (label ->    (kernel->boot-label (operating-system-kernel os)))
-       (kernel ->   (operating-system-kernel-file os))
-       (initrd      (operating-system-initrd-file os))
        (root-device -> (if (eq? 'uuid (file-system-title root-fs))
                            (uuid->string (file-system-device root-fs))
                            (file-system-device root-fs)))
-       (entries ->  (list (menu-entry
-                           (label label)
-
-                           ;; The device where the kernel and initrd live.
-                           (device (fs->boot-device store-fs))
-                           (device-mount-point
-                            (file-system-mount-point store-fs))
-
-                           (linux kernel)
-                           (linux-arguments
-                            (cons* (string-append "--root=" root-device)
-                                   #~(string-append "--system=" #$system)
-                                   #~(string-append "--load=" #$system
-                                                    "/boot")
-                                   (operating-system-kernel-arguments os
-                                                                      system
-                                                                      root-device)))
-                           (initrd initrd)))))
-    (grub-configuration-file (operating-system-bootloader os) entries
-                             #:old-entries old-entries)))
+       (entry (operating-system-boot-parameters os system root-device)))
+    (grub-configuration-file (operating-system-bootloader os)
+                             (list entry)
+                              #:old-entries old-entries)))
 
 (define (fs->boot-device fs)
   "Given FS, a <file-system> object, return a value suitable for use as the

bug#26544: [PATCH v2 7/8] system: Use operating-system-boot-parameters directly.

[Mathieu Othacehe]

LGTM!

> * gnu/system.scm (operating-system-bootcfg): Use
> operating-system-boot-parameters directly.
> ---
>  gnu/system.scm | 28 ++++------------------------
>  1 file changed, 4 insertions(+), 24 deletions(-)
>
> diff --git a/gnu/system.scm b/gnu/system.scm
> index cb166c755..013bd5356 100644
> --- a/gnu/system.scm
> +++ b/gnu/system.scm
> @@ -750,33 +750,13 @@ populate the \"old entries\" menu."
>    (mlet* %store-monad
>        ((system      (operating-system-derivation os))
>         (root-fs ->  (operating-system-root-file-system os))
> -       (store-fs -> (operating-system-store-file-system os))
> -       (label ->    (kernel->boot-label (operating-system-kernel os)))
> -       (kernel ->   (operating-system-kernel-file os))
> -       (initrd      (operating-system-initrd-file os))
>         (root-device -> (if (eq? 'uuid (file-system-title root-fs))
>                             (uuid->string (file-system-device root-fs))
>                             (file-system-device root-fs)))
> -       (entries ->  (list (menu-entry
> -                           (label label)
> -
> -                           ;; The device where the kernel and initrd live.
> -                           (device (fs->boot-device store-fs))
> -                           (device-mount-point
> -                            (file-system-mount-point store-fs))
> -
> -                           (linux kernel)
> -                           (linux-arguments
> -                            (cons* (string-append "--root=" root-device)
> -                                   #~(string-append "--system=" #$system)
> -                                   #~(string-append "--load=" #$system
> -                                                    "/boot")
> -                                   (operating-system-kernel-arguments os
> -                                                                      system
> -                                                                      root-device)))
> -                           (initrd initrd)))))
> -    (grub-configuration-file (operating-system-bootloader os) entries
> -                             #:old-entries old-entries)))
> +       (entry (operating-system-boot-parameters os system root-device)))
> +    (grub-configuration-file (operating-system-bootloader os)
> +                             (list entry)
> +                              #:old-entries old-entries)))
>  
>  (define (fs->boot-device fs)
>    "Given FS, a <file-system> object, return a value suitable for use as the

bug#26544: [PATCH v2 8/8] system: grub: Use boot-parameters instead of menu-entry where possible.

[Danny Milosavljevic]

* gnu/system/grub.scm (boot-parameters->menu-entry): New variable.
(grub-configuration-file): Use boot-parameters
instead of menu-entry where possible.
* guix/scripts/system.scm (profile-boot-parameters): Update docstring.
(reinstall-grub): Use profile-boot-parameters.
(perform-action): Use profile-boot-parameters.
---
 gnu/system/grub.scm     | 14 ++++++++++++--
 guix/scripts/system.scm | 22 +++++++++-------------
 2 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/gnu/system/grub.scm b/gnu/system/grub.scm
index cde4b9e23..d2fa984ec 100644
--- a/gnu/system/grub.scm
+++ b/gnu/system/grub.scm
@@ -267,6 +267,16 @@ code."
         (#f
          #~(format #f "search --file --set ~a" #$file)))))
 
+(define (boot-parameters->menu-entry conf)
+  "Convert a <boot-parameters> instance to a corresponding <menu-entry>."
+  (menu-entry
+   (label (boot-parameters-label conf))
+   (device (boot-parameters-store-device conf))
+   (device-mount-point (boot-parameters-store-mount-point conf))
+   (linux (boot-parameters-kernel conf))
+   (linux-arguments (boot-parameters-kernel-arguments conf))
+   (initrd (boot-parameters-initrd conf))))
+
 (define* (grub-configuration-file config entries
                                   #:key
                                   (system (%current-system))
@@ -276,7 +286,7 @@ code."
 <file-system> object.  OLD-ENTRIES is taken to be a list of menu entries
 corresponding to old generations of the system."
   (define all-entries
-    (append entries
+    (append (map boot-parameters->menu-entry entries)
             (grub-configuration-menu-entries config)))
 
   (define entry->gexp
@@ -323,7 +333,7 @@ set timeout=~a~%"
             #$@(if (pair? old-entries)
                    #~((format port "
 submenu \"GNU system, old configurations...\" {~%")
-                      #$@(map entry->gexp old-entries)
+                      #$@(map entry->gexp (map boot-parameters->menu-entry old-entries))
                       (format port "}~%"))
                    #~()))))
 
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 378138c26..dade5eb85 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -365,12 +365,11 @@ it atomically, and then run OS's activation script."
 
 (define* (profile-boot-parameters #:optional (profile %system-profile)
                                   (numbers (generation-numbers profile)))
-  "Return a list of 'menu-entry' for the generations of PROFILE specified by
-NUMBERS, which is a list of generation numbers."
+  "Return a list of 'boot-parameters' for the generations of PROFILE specified
+by NUMBERS, which is a list of generation numbers."
   (define (system->boot-parameters system number time)
     (unless-file-not-found
-     (let* ((file             (string-append system "/parameters"))
-            (params           (read-boot-parameters-file file)))
+     (let* ((params           (read-boot-parameters-file system)))
        params)))
   (let* ((systems (map (cut generation-file-name profile <>)
                        numbers))
@@ -386,8 +385,7 @@ NUMBERS, which is a list of generation numbers."
 NUMBERS, which is a list of generation numbers."
   (define (system->grub-entry system number time)
     (unless-file-not-found
-     (let* ((file             (string-append system "/parameters"))
-            (params           (read-boot-parameters-file file))
+     (let* ((params           (read-boot-parameters-file system))
             (label            (boot-parameters-label params))
             (root             (boot-parameters-root-device params))
             (root-device      (if (bytevector? root)
@@ -445,18 +443,17 @@ generation as its default entry.  STORE is an open connection to the store."
   "Re-install grub for existing system profile generation NUMBER.  STORE is an
 open connection to the store."
   (let* ((generation (generation-file-name %system-profile number))
-         (file (string-append generation "/parameters"))
          (params (unless-file-not-found
-                  (read-boot-parameters-file file)))
+                  (read-boot-parameters-file generation)))
          (root-device (boot-parameters-root-device params))
          ;; We don't currently keep track of past menu entries' details.  The
          ;; default values will allow the system to boot, even if they differ
          ;; from the actual past values for this generation's entry.
          (grub-config (grub-configuration (device root-device)))
          ;; Make the specified system generation the default entry.
-         (entries (profile-grub-entries %system-profile (list number)))
+         (entries (profile-boot-parameters %system-profile (list number)))
          (old-generations (delv number (generation-numbers %system-profile)))
-         (old-entries (profile-grub-entries %system-profile old-generations))
+         (old-entries (profile-boot-parameters %system-profile old-generations))
          (grub.cfg (run-with-store store
                      (grub-configuration-file grub-config
                                               entries
@@ -531,8 +528,7 @@ list of services."
   "Display a summary of system generation NUMBER in a human-readable format."
   (unless (zero? number)
     (let* ((generation  (generation-file-name profile number))
-           (param-file  (string-append generation "/parameters"))
-           (params      (read-boot-parameters-file param-file))
+           (params      (read-boot-parameters-file generation))
            (label       (boot-parameters-label params))
            (root        (boot-parameters-root-device params))
            (root-device (if (bytevector? root)
@@ -641,7 +637,7 @@ output when building a system derivation, such as a disk image."
                       (operating-system-bootcfg os
                                                 (if (eq? 'init action)
                                                     '()
-                                                    (profile-grub-entries)))))
+                                                    (profile-boot-parameters)))))
 
        ;; For 'init' and 'reconfigure', always build GRUB.CFG, even if
        ;; --no-grub is passed, because GRUB.CFG because we then use it as a GC

bug#26544: [PATCH v2 8/8] system: grub: Use boot-parameters instead of menu-entry where possible.

[Mathieu Othacehe]

> (reinstall-grub): Use profile-boot-parameters.
> (perform-action): Use profile-boot-parameters.

profile-grub-entries is now unsued, this serie could remove it.

bug#26544: [PATCH v2 0/8] system: Move "--load" and other guix-specific parameters from the grub module to the generic system module.

[Mathieu Othacehe]

Hi Danny,

Most of the serie LGTM.

However, I noticed that the grub menuentries in /boot/grub/grub.cfg
produced during a reconfigure are loosing their labels (#1, ...).

Thanks,

Mathieu

Danny Milosavljevic writes:

> Danny Milosavljevic (8):
>   system: Rename operating-system-kernel-arguments to
>     operating-system-user-kernel-arguments.
>   system: Rename operating-system-parameters-file to
>     operating-system-boot-parameters-file.
>   system: Factorize operating-system-boot-parameters-file.
>   system: Introduce operating-system-kernel-arguments and use it.
>   system: Introduce read-boot-parameters-file.
>   system: vm: Use operating-system-kernel-arguments.
>   system: Use operating-system-boot-parameters directly.
>   system: grub: Use boot-parameters instead of menu-entry where
>     possible.
>
>  gnu/system.scm          | 133 +++++++++++++++++++++++++++++++-----------------
>  gnu/system/grub.scm     |  14 ++++-
>  gnu/system/vm.scm       |   7 +--
>  guix/scripts/system.scm |  24 ++++-----
>  4 files changed, 110 insertions(+), 68 deletions(-)

bug#26544: [PATCH v3 0/9] system: Move "--load" and other guix-specific parameters from the grub module to the generic system module.

[Danny Milosavljevic]

Danny Milosavljevic (9):
  system: Rename operating-system-parameters-file to
    operating-system-boot-parameters-file.
  system: Factorize operating-system-boot-parameters-file.
  system: Introduce operating-system-kernel-arguments and use it.
  system: Introduce read-boot-parameters-file.
  scripts: Make label include generation number and time.
  system: vm: Use operating-system-kernel-arguments.
  system: Use operating-system-boot-parameters directly.
  system: grub: Use boot-parameters instead of menu-entry where
    possible.
  scripts: Remove profile-grub-entries.

 gnu/system.scm          | 133 +++++++++++++++++++++++++++++++-----------------
 gnu/system/grub.scm     |  14 ++++-
 gnu/system/vm.scm       |   7 +--
 guix/scripts/system.scm |  64 +++++------------------
 4 files changed, 114 insertions(+), 104 deletions(-)

bug#26544: [PATCH v3 1/9] system: Rename operating-system-parameters-file to operating-system-boot-parameters-file.

[Danny Milosavljevic]

* gnu/system.scm (operating-system-parameters-file): Rename to ...
(operating-system-boot-parameters-file): ... this.
(operating-system-directory-base-entries): Adapt call site.
---
 gnu/system.scm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index 4032e8e15..44190bdfc 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -278,7 +278,7 @@ value of the SYSTEM-SERVICE-TYPE service."
         (mlet %store-monad
             ((kernel  ->  (operating-system-kernel os))
              (initrd      (operating-system-initrd-file os))
-             (params      (operating-system-parameters-file os)))
+             (params      (operating-system-boot-parameters-file os)))
           (return `(("kernel" ,kernel)
                     ("parameters" ,params)
                     ("initrd" ,initrd)
@@ -769,7 +769,7 @@ device in a <menu-entry>."
     ((label) (file-system-device fs))
     (else #f)))
 
-(define (operating-system-parameters-file os)
+(define (operating-system-boot-parameters-file os)
   "Return a file that describes the boot parameters of OS.  The primary use of
 this file is the reconstruction of GRUB menu entries for old configurations."
   (mlet %store-monad ((initrd   (operating-system-initrd-file os))

bug#26544: [PATCH v3 2/9] system: Factorize operating-system-boot-parameters-file.

[Danny Milosavljevic]

* gnu/system.scm (operating-system-boot-parameters): New variable.
(operating-system-boot-parameters-file): Modify.
---
 gnu/system.scm | 64 +++++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 43 insertions(+), 21 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index 44190bdfc..6601147e9 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -769,27 +769,49 @@ device in a <menu-entry>."
     ((label) (file-system-device fs))
     (else #f)))
 
-(define (operating-system-boot-parameters-file os)
-  "Return a file that describes the boot parameters of OS.  The primary use of
-this file is the reconstruction of GRUB menu entries for old configurations."
-  (mlet %store-monad ((initrd   (operating-system-initrd-file os))
-                      (root ->  (operating-system-root-file-system os))
-                      (store -> (operating-system-store-file-system os))
-                      (label -> (kernel->boot-label
-                                 (operating-system-kernel os))))
-    (gexp->file "parameters"
-                #~(boot-parameters
-                   (version 0)
-                   (label #$label)
-                   (root-device #$(file-system-device root))
-                   (kernel #$(operating-system-kernel-file os))
-                   (kernel-arguments
-                    #$(operating-system-user-kernel-arguments os))
-                   (initrd #$initrd)
-                   (store
-                    (device #$(fs->boot-device store))
-                    (mount-point #$(file-system-mount-point store))))
-                #:set-load-path? #f)))
+(define (operating-system-boot-parameters os system root-device)
+  "Return a monadic <boot-parameters> record that describes the boot parameters of OS.
+SYSTEM is optional.  If given, adds kernel arguments for that system to <boot-parameters>."
+  (mlet* %store-monad
+      ((initrd (operating-system-initrd-file os))
+       (store -> (operating-system-store-file-system os))
+       (label -> (kernel->boot-label (operating-system-kernel os))))
+    (return (boot-parameters
+             (label label)
+             (root-device root-device)
+             (kernel (operating-system-kernel-file os))
+             (kernel-arguments
+              (operating-system-user-kernel-arguments os))
+             (initrd initrd)
+             (store-device (fs->boot-device store))
+             (store-mount-point (file-system-mount-point store))))))
+
+(define* (operating-system-boot-parameters-file os #:optional (system.drv #f))
+   "Return a file that describes the boot parameters of OS.  The primary use of
+this file is the reconstruction of GRUB menu entries for old configurations.
+SYSTEM.DRV is optional.  If given, adds kernel arguments for that system to the
+returned file (since the returned file is then usually stored into the
+content-addressed \"system\" directory, it's usually not a good idea
+to give it because the content hash would change by the content hash
+being stored into the \"parameters\" file)."
+  (mlet* %store-monad ((root -> (operating-system-root-file-system os))
+                       (device -> (file-system-device root))
+                       (params (operating-system-boot-parameters os
+                                                                 system.drv
+                                                                 device)))
+     (gexp->file "parameters"
+                 #~(boot-parameters
+                    (version 0)
+                    (label #$(boot-parameters-label params))
+                    (root-device #$(boot-parameters-root-device params))
+                    (kernel #$(boot-parameters-kernel params))
+                    (kernel-arguments
+                     #$(boot-parameters-kernel-arguments params))
+                    (initrd #$(boot-parameters-initrd params))
+                    (store
+                     (device #$(boot-parameters-store-device params))
+                     (mount-point #$(boot-parameters-store-mount-point params))))
+                 #:set-load-path? #f)))
 
 \f
 ;;;

bug#26544: [PATCH v3 3/9] system: Introduce operating-system-kernel-arguments and use it.

[Danny Milosavljevic]

* gnu/system.scm (bootable-kernel-arguments): New variable.
(operating-system-kernel-arguments):  New variable.
(operating-system-bootcfg): Use operating-system-kernel-arguments.
(operating-system-boot-parameters): Use operating-system-kernel-arguments.
---
 gnu/system.scm | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index 6601147e9..a4f15f7f5 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -73,7 +73,7 @@
             operating-system-hosts-file
             operating-system-kernel
             operating-system-kernel-file
-            operating-system-user-kernel-arguments
+            operating-system-kernel-arguments
             operating-system-initrd
             operating-system-users
             operating-system-groups
@@ -122,6 +122,14 @@
 ;;;
 ;;; Code:
 
+(define (bootable-kernel-arguments kernel-arguments system.drv root-device)
+  "Prepend extra arguments to KERNEL-ARGUMENTS that allow SYSTEM.DRV to be
+booted from ROOT-DEVICE"
+  (cons* (string-append "--root=" root-device)
+         #~(string-append "--system=" #$system.drv)
+         #~(string-append "--load=" #$system.drv "/boot")
+         kernel-arguments))
+
 ;; System-wide configuration.
 ;; TODO: Add per-field docstrings/stexi.
 (define-record-type* <operating-system> operating-system
@@ -182,6 +190,13 @@
   (sudoers-file operating-system-sudoers-file     ; file-like
                 (default %sudoers-specification)))
 
+(define (operating-system-kernel-arguments os system.drv root-device)
+  "Return all the kernel arguments, including the ones not specified
+directly by the user."
+  (bootable-kernel-arguments (operating-system-user-kernel-arguments os)
+                             system.drv
+                             root-device))
+
 \f
 ;;;
 ;;; Services.
@@ -756,7 +771,9 @@ populate the \"old entries\" menu."
                                    #~(string-append "--system=" #$system)
                                    #~(string-append "--load=" #$system
                                                     "/boot")
-                                   (operating-system-user-kernel-arguments os)))
+                                   (operating-system-kernel-arguments os
+                                                                      system
+                                                                      root-device)))
                            (initrd initrd)))))
     (grub-configuration-file (operating-system-bootloader os) entries
                              #:old-entries old-entries)))
@@ -781,7 +798,9 @@ SYSTEM is optional.  If given, adds kernel arguments for that system to <boot-pa
              (root-device root-device)
              (kernel (operating-system-kernel-file os))
              (kernel-arguments
-              (operating-system-user-kernel-arguments os))
+              (if system
+               (operating-system-kernel-arguments os system root-device)
+               (operating-system-user-kernel-arguments os)))
              (initrd initrd)
              (store-device (fs->boot-device store))
              (store-mount-point (file-system-mount-point store))))))

bug#26544: [PATCH v3 4/9] system: Introduce read-boot-parameters-file.

[Danny Milosavljevic]

* gnu/system.scm (read-boot-parameters): Remove export.
(read-boot-parameters-file): New variable. Export it.
* guix/scripts/system.scm (profile-boot-parameters): Use
read-boot-parameters-file.
(profile-grub-entries): Use read-boot-parameters-file.
(reinstall-grub): Use read-boot-parameters-file.
(display-system-generation): Use read-boot-parameters-file.
---
 gnu/system.scm          | 22 +++++++++++++++++++++-
 guix/scripts/system.scm | 14 ++++----------
 2 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index a4f15f7f5..9921a9786 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -109,7 +109,7 @@
             boot-parameters-kernel
             boot-parameters-kernel-arguments
             boot-parameters-initrd
-            read-boot-parameters
+            read-boot-parameters-file
 
             local-host-aliases
             %setuid-programs
@@ -907,4 +907,24 @@ being stored into the \"parameters\" file)."
               system)
      #f)))
 
+(define (read-boot-parameters-file system)
+  "Read boot parameters from SYSTEM's (system or generation) \"parameters\"
+file and returns the corresponding <boot-parameters> object or #f if the
+format is unrecognized.
+The object has its kernel-arguments extended in order to make it bootable."
+  (let* ((file (string-append system "/parameters"))
+         (params (call-with-input-file file read-boot-parameters))
+         (root (boot-parameters-root-device params))
+         (root-device (if (bytevector? root)
+                          (uuid->string root)
+                          root))
+         (kernel-arguments (boot-parameters-kernel-arguments params)))
+    (if params
+      (boot-parameters
+        (inherit params)
+        (kernel-arguments (bootable-kernel-arguments kernel-arguments
+                                                     system
+                                                     root-device)))
+      #f)))
+
 ;;; system.scm ends here
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 8fabdb5c1..b278b6683 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -369,9 +369,7 @@ it atomically, and then run OS's activation script."
 NUMBERS, which is a list of generation numbers."
   (define (system->boot-parameters system number time)
     (unless-file-not-found
-     (let* ((file             (string-append system "/parameters"))
-            (params           (call-with-input-file file
-                                read-boot-parameters)))
+     (let* ((params           (read-boot-parameters-file system)))
        params)))
   (let* ((systems (map (cut generation-file-name profile <>)
                        numbers))
@@ -387,9 +385,7 @@ NUMBERS, which is a list of generation numbers."
 NUMBERS, which is a list of generation numbers."
   (define (system->grub-entry system number time)
     (unless-file-not-found
-     (let* ((file             (string-append system "/parameters"))
-            (params           (call-with-input-file file
-                                read-boot-parameters))
+     (let* ((params           (read-boot-parameters-file system))
             (label            (boot-parameters-label params))
             (root             (boot-parameters-root-device params))
             (root-device      (if (bytevector? root)
@@ -447,9 +443,8 @@ generation as its default entry.  STORE is an open connection to the store."
   "Re-install grub for existing system profile generation NUMBER.  STORE is an
 open connection to the store."
   (let* ((generation (generation-file-name %system-profile number))
-         (file (string-append generation "/parameters"))
          (params (unless-file-not-found
-                  (call-with-input-file file read-boot-parameters)))
+                  (read-boot-parameters-file generation)))
          (root-device (boot-parameters-root-device params))
          ;; We don't currently keep track of past menu entries' details.  The
          ;; default values will allow the system to boot, even if they differ
@@ -533,8 +528,7 @@ list of services."
   "Display a summary of system generation NUMBER in a human-readable format."
   (unless (zero? number)
     (let* ((generation  (generation-file-name profile number))
-           (param-file  (string-append generation "/parameters"))
-           (params      (call-with-input-file param-file read-boot-parameters))
+           (params      (read-boot-parameters-file generation))
            (label       (boot-parameters-label params))
            (root        (boot-parameters-root-device params))
            (root-device (if (bytevector? root)

bug#26544: [PATCH v3 5/9] scripts: Make boot-parameters label include generation number and time.

[Danny Milosavljevic]

* guix/scripts/system.scm (system->boot-parameters): Make label include
generation number and time.
---
 guix/scripts/system.scm | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index b278b6683..d4cbb9d38 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -370,7 +370,11 @@ NUMBERS, which is a list of generation numbers."
   (define (system->boot-parameters system number time)
     (unless-file-not-found
      (let* ((params           (read-boot-parameters-file system)))
-       params)))
+       (boot-parameters
+         (inherit params)
+         (label (string-append label " (#"
+                               (number->string number) ", "
+                               (seconds->string time) ")"))))))
   (let* ((systems (map (cut generation-file-name profile <>)
                        numbers))
          (times   (map (lambda (system)

bug#26544: [PATCH v3 6/9] system: vm: Use operating-system-kernel-arguments.

[Danny Milosavljevic]

* gnu/system/vm.scm (system-qemu-image/shared-store-script):
Use operating-system-kernel-arguments.
---
 gnu/system/vm.scm | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index 4f915c4f9..2c8b954c8 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -490,11 +490,8 @@ it is mostly useful when FULL-BOOT?  is true."
                                 #:full-boot? full-boot?
                                 #:disk-image-size disk-image-size)))
     (define kernel-arguments
-      #~(list "--root=/dev/vda1"
-              (string-append "--system=" #$os-drv)
-              (string-append "--load=" #$os-drv "/boot")
-              #$@(if graphic? #~() #~("console=ttyS0"))
-              #+@(operating-system-user-kernel-arguments os)))
+      #~(list #$@(if graphic? #~() #~("console=ttyS0"))
+              #+@(operating-system-kernel-arguments os os-drv "/dev/vda1")))
 
     (define qemu-exec
       #~(list (string-append #$qemu "/bin/" #$(qemu-command (%current-system)))

bug#26544: [PATCH v3 7/9] system: Use operating-system-boot-parameters directly.

[Danny Milosavljevic]

* gnu/system.scm (operating-system-bootcfg): Use
operating-system-boot-parameters directly.
---
 gnu/system.scm | 28 ++++------------------------
 1 file changed, 4 insertions(+), 24 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index 9921a9786..90fea9af3 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -750,33 +750,13 @@ populate the \"old entries\" menu."
   (mlet* %store-monad
       ((system      (operating-system-derivation os))
        (root-fs ->  (operating-system-root-file-system os))
-       (store-fs -> (operating-system-store-file-system os))
-       (label ->    (kernel->boot-label (operating-system-kernel os)))
-       (kernel ->   (operating-system-kernel-file os))
-       (initrd      (operating-system-initrd-file os))
        (root-device -> (if (eq? 'uuid (file-system-title root-fs))
                            (uuid->string (file-system-device root-fs))
                            (file-system-device root-fs)))
-       (entries ->  (list (menu-entry
-                           (label label)
-
-                           ;; The device where the kernel and initrd live.
-                           (device (fs->boot-device store-fs))
-                           (device-mount-point
-                            (file-system-mount-point store-fs))
-
-                           (linux kernel)
-                           (linux-arguments
-                            (cons* (string-append "--root=" root-device)
-                                   #~(string-append "--system=" #$system)
-                                   #~(string-append "--load=" #$system
-                                                    "/boot")
-                                   (operating-system-kernel-arguments os
-                                                                      system
-                                                                      root-device)))
-                           (initrd initrd)))))
-    (grub-configuration-file (operating-system-bootloader os) entries
-                             #:old-entries old-entries)))
+       (entry (operating-system-boot-parameters os system root-device)))
+    (grub-configuration-file (operating-system-bootloader os)
+                             (list entry)
+                              #:old-entries old-entries)))
 
 (define (fs->boot-device fs)
   "Given FS, a <file-system> object, return a value suitable for use as the

bug#26544: [PATCH v3 8/9] system: grub: Use boot-parameters instead of menu-entry where possible.

[Danny Milosavljevic]

* gnu/system/grub.scm (boot-parameters->menu-entry): New variable.
(grub-configuration-file): Use boot-parameters
instead of menu-entry where possible.
* guix/scripts/system.scm (profile-boot-parameters): Update docstring.
(reinstall-grub): Use profile-boot-parameters.
(perform-action): Use profile-boot-parameters.
---
 gnu/system/grub.scm     | 14 ++++++++++++--
 guix/scripts/system.scm | 11 ++++++-----
 2 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/gnu/system/grub.scm b/gnu/system/grub.scm
index cde4b9e23..d2fa984ec 100644
--- a/gnu/system/grub.scm
+++ b/gnu/system/grub.scm
@@ -267,6 +267,16 @@ code."
         (#f
          #~(format #f "search --file --set ~a" #$file)))))
 
+(define (boot-parameters->menu-entry conf)
+  "Convert a <boot-parameters> instance to a corresponding <menu-entry>."
+  (menu-entry
+   (label (boot-parameters-label conf))
+   (device (boot-parameters-store-device conf))
+   (device-mount-point (boot-parameters-store-mount-point conf))
+   (linux (boot-parameters-kernel conf))
+   (linux-arguments (boot-parameters-kernel-arguments conf))
+   (initrd (boot-parameters-initrd conf))))
+
 (define* (grub-configuration-file config entries
                                   #:key
                                   (system (%current-system))
@@ -276,7 +286,7 @@ code."
 <file-system> object.  OLD-ENTRIES is taken to be a list of menu entries
 corresponding to old generations of the system."
   (define all-entries
-    (append entries
+    (append (map boot-parameters->menu-entry entries)
             (grub-configuration-menu-entries config)))
 
   (define entry->gexp
@@ -323,7 +333,7 @@ set timeout=~a~%"
             #$@(if (pair? old-entries)
                    #~((format port "
 submenu \"GNU system, old configurations...\" {~%")
-                      #$@(map entry->gexp old-entries)
+                      #$@(map entry->gexp (map boot-parameters->menu-entry old-entries))
                       (format port "}~%"))
                    #~()))))
 
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index d4cbb9d38..242bd8074 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -365,11 +365,12 @@ it atomically, and then run OS's activation script."
 
 (define* (profile-boot-parameters #:optional (profile %system-profile)
                                   (numbers (generation-numbers profile)))
-  "Return a list of 'menu-entry' for the generations of PROFILE specified by
+  "Return a list of 'boot-parameters' for the generations of PROFILE specified by
 NUMBERS, which is a list of generation numbers."
   (define (system->boot-parameters system number time)
     (unless-file-not-found
-     (let* ((params           (read-boot-parameters-file system)))
+     (let* ((params           (read-boot-parameters-file system))
+            (label            (boot-parameters-label params)))
        (boot-parameters
          (inherit params)
          (label (string-append label " (#"
@@ -455,9 +456,9 @@ open connection to the store."
          ;; from the actual past values for this generation's entry.
          (grub-config (grub-configuration (device root-device)))
          ;; Make the specified system generation the default entry.
-         (entries (profile-grub-entries %system-profile (list number)))
+         (entries (profile-boot-parameters %system-profile (list number)))
          (old-generations (delv number (generation-numbers %system-profile)))
-         (old-entries (profile-grub-entries %system-profile old-generations))
+         (old-entries (profile-boot-parameters %system-profile old-generations))
          (grub.cfg (run-with-store store
                      (grub-configuration-file grub-config
                                               entries
@@ -641,7 +642,7 @@ output when building a system derivation, such as a disk image."
                       (operating-system-bootcfg os
                                                 (if (eq? 'init action)
                                                     '()
-                                                    (profile-grub-entries)))))
+                                                    (profile-boot-parameters)))))
 
        ;; For 'init' and 'reconfigure', always build GRUB.CFG, even if
        ;; --no-grub is passed, because GRUB.CFG because we then use it as a GC

bug#26544: [PATCH v3 9/9] scripts: Remove profile-grub-entries.

[Danny Milosavljevic]

* guix/scripts/system.scm (profile-grub-entries): Delete variable.
---
 guix/scripts/system.scm | 37 -------------------------------------
 1 file changed, 37 deletions(-)

diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 242bd8074..3feccb2ab 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -384,43 +384,6 @@ NUMBERS, which is a list of generation numbers."
                        systems)))
     (filter-map system->boot-parameters systems numbers times)))
 
-(define* (profile-grub-entries #:optional (profile %system-profile)
-                                  (numbers (generation-numbers profile)))
-  "Return a list of 'menu-entry' for the generations of PROFILE specified by
-NUMBERS, which is a list of generation numbers."
-  (define (system->grub-entry system number time)
-    (unless-file-not-found
-     (let* ((params           (read-boot-parameters-file system))
-            (label            (boot-parameters-label params))
-            (root             (boot-parameters-root-device params))
-            (root-device      (if (bytevector? root)
-                                  (uuid->string root)
-                                  root))
-            (kernel           (boot-parameters-kernel params))
-            (kernel-arguments (boot-parameters-kernel-arguments params))
-            (initrd           (boot-parameters-initrd params)))
-       (menu-entry
-        (label (string-append label " (#"
-                              (number->string number) ", "
-                              (seconds->string time) ")"))
-        (device (boot-parameters-store-device params))
-        (device-mount-point (boot-parameters-store-mount-point params))
-        (linux kernel)
-        (linux-arguments
-         (cons* (string-append "--root=" root-device)
-                (string-append "--system=" system)
-                (string-append "--load=" system "/boot")
-                kernel-arguments))
-        (initrd initrd)))))
-
-  (let* ((systems (map (cut generation-file-name profile <>)
-                       numbers))
-         (times   (map (lambda (system)
-                         (unless-file-not-found
-                          (stat:mtime (lstat system))))
-                       systems)))
-    (filter-map system->grub-entry systems numbers times)))
-
 \f
 ;;;
 ;;; Roll-back.

bug#26544: [PATCH v4 00/10] system: Move "--load" and other guix-specific parameters from the grub module to the generic system module.

[Danny Milosavljevic]

Danny Milosavljevic (10):
  system: Rename operating-system-kernel-arguments to
    operating-system-user-kernel-arguments.
  system: Rename operating-system-parameters-file to
    operating-system-boot-parameters-file.
  system: Factorize operating-system-boot-parameters-file.
  system: Introduce operating-system-kernel-arguments and use it.
  system: Introduce read-boot-parameters-file.
  scripts: Make boot-parameters label include generation number and
    time.
  system: vm: Use operating-system-kernel-arguments.
  system: Use operating-system-boot-parameters directly.
  system: grub: Use boot-parameters instead of menu-entry  where
    possible.
  scripts: Remove profile-grub-entries.

 gnu/system.scm          | 133 +++++++++++++++++++++++++++++++-----------------
 gnu/system/grub.scm     |  14 ++++-
 gnu/system/vm.scm       |   7 +--
 guix/scripts/system.scm |  64 +++++------------------
 4 files changed, 114 insertions(+), 104 deletions(-)

bug#26544: [PATCH v4 01/10] system: Rename operating-system-kernel-arguments to operating-system-user-kernel-arguments.

[Danny Milosavljevic]

* gnu/system.scm (operating-system-kernel-arguments): Rename to ...
(operating-system-user-kernel-arguments): ... this.
(<operating-system>): Adapt accordingly.
(operating-system-bootcfg): Adapt accordingly.
(operating-system-parameters-file): Adapt accordingly.
* gnu/system/vm.scm (system-qemu-image/shared-store-script): Adapt
accordingly.
---
 gnu/system.scm    | 8 ++++----
 gnu/system/vm.scm | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index f6ab7ded8..4032e8e15 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -73,7 +73,7 @@
             operating-system-hosts-file
             operating-system-kernel
             operating-system-kernel-file
-            operating-system-kernel-arguments
+            operating-system-user-kernel-arguments
             operating-system-initrd
             operating-system-users
             operating-system-groups
@@ -129,7 +129,7 @@
   operating-system?
   (kernel operating-system-kernel                 ; package
           (default linux-libre))
-  (kernel-arguments operating-system-kernel-arguments
+  (kernel-arguments operating-system-user-kernel-arguments
                     (default '()))                ; list of gexps/strings
   (bootloader operating-system-bootloader)        ; <grub-configuration>
 
@@ -756,7 +756,7 @@ populate the \"old entries\" menu."
                                    #~(string-append "--system=" #$system)
                                    #~(string-append "--load=" #$system
                                                     "/boot")
-                                   (operating-system-kernel-arguments os)))
+                                   (operating-system-user-kernel-arguments os)))
                            (initrd initrd)))))
     (grub-configuration-file (operating-system-bootloader os) entries
                              #:old-entries old-entries)))
@@ -784,7 +784,7 @@ this file is the reconstruction of GRUB menu entries for old configurations."
                    (root-device #$(file-system-device root))
                    (kernel #$(operating-system-kernel-file os))
                    (kernel-arguments
-                    #$(operating-system-kernel-arguments os))
+                    #$(operating-system-user-kernel-arguments os))
                    (initrd #$initrd)
                    (store
                     (device #$(fs->boot-device store))
diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index 374d8b663..4f915c4f9 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -494,7 +494,7 @@ it is mostly useful when FULL-BOOT?  is true."
               (string-append "--system=" #$os-drv)
               (string-append "--load=" #$os-drv "/boot")
               #$@(if graphic? #~() #~("console=ttyS0"))
-              #+@(operating-system-kernel-arguments os)))
+              #+@(operating-system-user-kernel-arguments os)))
 
     (define qemu-exec
       #~(list (string-append #$qemu "/bin/" #$(qemu-command (%current-system)))

bug#26544: [PATCH v4 01/10] system: Rename operating-system-kernel-arguments to operating-system-user-kernel-arguments.

[Danny Milosavljevic]

Applied this patch to master as af98d25a1286e246e8da36d6d63b4d66e58f2cf8 and pushed.

bug#26544: [PATCH v4 02/10] system: Rename operating-system-parameters-file to operating-system-boot-parameters-file.

[Danny Milosavljevic]

* gnu/system.scm (operating-system-parameters-file): Rename to ...
(operating-system-boot-parameters-file): ... this.
(operating-system-directory-base-entries): Adapt call site.
---
 gnu/system.scm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index 4032e8e15..44190bdfc 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -278,7 +278,7 @@ value of the SYSTEM-SERVICE-TYPE service."
         (mlet %store-monad
             ((kernel  ->  (operating-system-kernel os))
              (initrd      (operating-system-initrd-file os))
-             (params      (operating-system-parameters-file os)))
+             (params      (operating-system-boot-parameters-file os)))
           (return `(("kernel" ,kernel)
                     ("parameters" ,params)
                     ("initrd" ,initrd)
@@ -769,7 +769,7 @@ device in a <menu-entry>."
     ((label) (file-system-device fs))
     (else #f)))
 
-(define (operating-system-parameters-file os)
+(define (operating-system-boot-parameters-file os)
   "Return a file that describes the boot parameters of OS.  The primary use of
 this file is the reconstruction of GRUB menu entries for old configurations."
   (mlet %store-monad ((initrd   (operating-system-initrd-file os))

bug#26544: [PATCH v4 02/10] system: Rename operating-system-parameters-file to operating-system-boot-parameters-file.

[Danny Milosavljevic]

Applied this patch to master as 71d04202026e2061f898a142a8381d55bee5fb00 and pushed.

bug#26544: [PATCH v4 03/10] system: Factorize operating-system-boot-parameters-file.

[Danny Milosavljevic]

* gnu/system.scm (operating-system-boot-parameters): New variable.
(operating-system-boot-parameters-file): Modify.
---
 gnu/system.scm | 64 +++++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 43 insertions(+), 21 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index 44190bdfc..6601147e9 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -769,27 +769,49 @@ device in a <menu-entry>."
     ((label) (file-system-device fs))
     (else #f)))
 
-(define (operating-system-boot-parameters-file os)
-  "Return a file that describes the boot parameters of OS.  The primary use of
-this file is the reconstruction of GRUB menu entries for old configurations."
-  (mlet %store-monad ((initrd   (operating-system-initrd-file os))
-                      (root ->  (operating-system-root-file-system os))
-                      (store -> (operating-system-store-file-system os))
-                      (label -> (kernel->boot-label
-                                 (operating-system-kernel os))))
-    (gexp->file "parameters"
-                #~(boot-parameters
-                   (version 0)
-                   (label #$label)
-                   (root-device #$(file-system-device root))
-                   (kernel #$(operating-system-kernel-file os))
-                   (kernel-arguments
-                    #$(operating-system-user-kernel-arguments os))
-                   (initrd #$initrd)
-                   (store
-                    (device #$(fs->boot-device store))
-                    (mount-point #$(file-system-mount-point store))))
-                #:set-load-path? #f)))
+(define (operating-system-boot-parameters os system root-device)
+  "Return a monadic <boot-parameters> record that describes the boot parameters of OS.
+SYSTEM is optional.  If given, adds kernel arguments for that system to <boot-parameters>."
+  (mlet* %store-monad
+      ((initrd (operating-system-initrd-file os))
+       (store -> (operating-system-store-file-system os))
+       (label -> (kernel->boot-label (operating-system-kernel os))))
+    (return (boot-parameters
+             (label label)
+             (root-device root-device)
+             (kernel (operating-system-kernel-file os))
+             (kernel-arguments
+              (operating-system-user-kernel-arguments os))
+             (initrd initrd)
+             (store-device (fs->boot-device store))
+             (store-mount-point (file-system-mount-point store))))))
+
+(define* (operating-system-boot-parameters-file os #:optional (system.drv #f))
+   "Return a file that describes the boot parameters of OS.  The primary use of
+this file is the reconstruction of GRUB menu entries for old configurations.
+SYSTEM.DRV is optional.  If given, adds kernel arguments for that system to the
+returned file (since the returned file is then usually stored into the
+content-addressed \"system\" directory, it's usually not a good idea
+to give it because the content hash would change by the content hash
+being stored into the \"parameters\" file)."
+  (mlet* %store-monad ((root -> (operating-system-root-file-system os))
+                       (device -> (file-system-device root))
+                       (params (operating-system-boot-parameters os
+                                                                 system.drv
+                                                                 device)))
+     (gexp->file "parameters"
+                 #~(boot-parameters
+                    (version 0)
+                    (label #$(boot-parameters-label params))
+                    (root-device #$(boot-parameters-root-device params))
+                    (kernel #$(boot-parameters-kernel params))
+                    (kernel-arguments
+                     #$(boot-parameters-kernel-arguments params))
+                    (initrd #$(boot-parameters-initrd params))
+                    (store
+                     (device #$(boot-parameters-store-device params))
+                     (mount-point #$(boot-parameters-store-mount-point params))))
+                 #:set-load-path? #f)))
 
 \f
 ;;;

bug#26544: [PATCH v4 03/10] system: Factorize operating-system-boot-parameters-file.

[Danny Milosavljevic]

On Fri, 21 Apr 2017 14:37:07 +0200
Danny Milosavljevic <dannym@scratchpost.org> wrote:

> +(define (operating-system-boot-parameters os system root-device)

Might make sense to call SYSTEM "SYSTEM.DRV" instead - since it's either a derivation or #f.

> +  "Return a monadic <boot-parameters> record that describes the boot parameters of OS.
> +SYSTEM is optional.  If given, adds kernel arguments for that system to <boot-parameters>."

Might want to elaborate on how SYSTEM is optional (#f stands for "not given").

SYSTEM is not used anywhere in the procedure.  The next patch rectifies that - but I didn't feel like messing up the function signature just to fix it up one patch later.  But even after only patch 03/10 you can give SYSTEM (and patch 03/10 operating-system-boot-parameters-file does that already) - it will just be ignored.

bug#26544: [PATCH v4 04/10] system: Introduce operating-system-kernel-arguments and use it.

[Danny Milosavljevic]

* gnu/system.scm (bootable-kernel-arguments): New variable.
(operating-system-kernel-arguments):  New variable.
(operating-system-bootcfg): Use operating-system-kernel-arguments.
(operating-system-boot-parameters): Use operating-system-kernel-arguments.
---
 gnu/system.scm | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index 6601147e9..a4f15f7f5 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -73,7 +73,7 @@
             operating-system-hosts-file
             operating-system-kernel
             operating-system-kernel-file
-            operating-system-user-kernel-arguments
+            operating-system-kernel-arguments
             operating-system-initrd
             operating-system-users
             operating-system-groups
@@ -122,6 +122,14 @@
 ;;;
 ;;; Code:
 
+(define (bootable-kernel-arguments kernel-arguments system.drv root-device)
+  "Prepend extra arguments to KERNEL-ARGUMENTS that allow SYSTEM.DRV to be
+booted from ROOT-DEVICE"
+  (cons* (string-append "--root=" root-device)
+         #~(string-append "--system=" #$system.drv)
+         #~(string-append "--load=" #$system.drv "/boot")
+         kernel-arguments))
+
 ;; System-wide configuration.
 ;; TODO: Add per-field docstrings/stexi.
 (define-record-type* <operating-system> operating-system
@@ -182,6 +190,13 @@
   (sudoers-file operating-system-sudoers-file     ; file-like
                 (default %sudoers-specification)))
 
+(define (operating-system-kernel-arguments os system.drv root-device)
+  "Return all the kernel arguments, including the ones not specified
+directly by the user."
+  (bootable-kernel-arguments (operating-system-user-kernel-arguments os)
+                             system.drv
+                             root-device))
+
 \f
 ;;;
 ;;; Services.
@@ -756,7 +771,9 @@ populate the \"old entries\" menu."
                                    #~(string-append "--system=" #$system)
                                    #~(string-append "--load=" #$system
                                                     "/boot")
-                                   (operating-system-user-kernel-arguments os)))
+                                   (operating-system-kernel-arguments os
+                                                                      system
+                                                                      root-device)))
                            (initrd initrd)))))
     (grub-configuration-file (operating-system-bootloader os) entries
                              #:old-entries old-entries)))
@@ -781,7 +798,9 @@ SYSTEM is optional.  If given, adds kernel arguments for that system to <boot-pa
              (root-device root-device)
              (kernel (operating-system-kernel-file os))
              (kernel-arguments
-              (operating-system-user-kernel-arguments os))
+              (if system
+               (operating-system-kernel-arguments os system root-device)
+               (operating-system-user-kernel-arguments os)))
              (initrd initrd)
              (store-device (fs->boot-device store))
              (store-mount-point (file-system-mount-point store))))))

bug#26544: [PATCH v4 05/10] system: Introduce read-boot-parameters-file.

[Danny Milosavljevic]

* gnu/system.scm (read-boot-parameters): Remove export.
(read-boot-parameters-file): New variable. Export it.
* guix/scripts/system.scm (profile-boot-parameters): Use
read-boot-parameters-file.
(profile-grub-entries): Use read-boot-parameters-file.
(reinstall-grub): Use read-boot-parameters-file.
(display-system-generation): Use read-boot-parameters-file.
---
 gnu/system.scm          | 22 +++++++++++++++++++++-
 guix/scripts/system.scm | 14 ++++----------
 2 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index a4f15f7f5..9921a9786 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -109,7 +109,7 @@
             boot-parameters-kernel
             boot-parameters-kernel-arguments
             boot-parameters-initrd
-            read-boot-parameters
+            read-boot-parameters-file
 
             local-host-aliases
             %setuid-programs
@@ -907,4 +907,24 @@ being stored into the \"parameters\" file)."
               system)
      #f)))
 
+(define (read-boot-parameters-file system)
+  "Read boot parameters from SYSTEM's (system or generation) \"parameters\"
+file and returns the corresponding <boot-parameters> object or #f if the
+format is unrecognized.
+The object has its kernel-arguments extended in order to make it bootable."
+  (let* ((file (string-append system "/parameters"))
+         (params (call-with-input-file file read-boot-parameters))
+         (root (boot-parameters-root-device params))
+         (root-device (if (bytevector? root)
+                          (uuid->string root)
+                          root))
+         (kernel-arguments (boot-parameters-kernel-arguments params)))
+    (if params
+      (boot-parameters
+        (inherit params)
+        (kernel-arguments (bootable-kernel-arguments kernel-arguments
+                                                     system
+                                                     root-device)))
+      #f)))
+
 ;;; system.scm ends here
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 8fabdb5c1..b278b6683 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -369,9 +369,7 @@ it atomically, and then run OS's activation script."
 NUMBERS, which is a list of generation numbers."
   (define (system->boot-parameters system number time)
     (unless-file-not-found
-     (let* ((file             (string-append system "/parameters"))
-            (params           (call-with-input-file file
-                                read-boot-parameters)))
+     (let* ((params           (read-boot-parameters-file system)))
        params)))
   (let* ((systems (map (cut generation-file-name profile <>)
                        numbers))
@@ -387,9 +385,7 @@ NUMBERS, which is a list of generation numbers."
 NUMBERS, which is a list of generation numbers."
   (define (system->grub-entry system number time)
     (unless-file-not-found
-     (let* ((file             (string-append system "/parameters"))
-            (params           (call-with-input-file file
-                                read-boot-parameters))
+     (let* ((params           (read-boot-parameters-file system))
             (label            (boot-parameters-label params))
             (root             (boot-parameters-root-device params))
             (root-device      (if (bytevector? root)
@@ -447,9 +443,8 @@ generation as its default entry.  STORE is an open connection to the store."
   "Re-install grub for existing system profile generation NUMBER.  STORE is an
 open connection to the store."
   (let* ((generation (generation-file-name %system-profile number))
-         (file (string-append generation "/parameters"))
          (params (unless-file-not-found
-                  (call-with-input-file file read-boot-parameters)))
+                  (read-boot-parameters-file generation)))
          (root-device (boot-parameters-root-device params))
          ;; We don't currently keep track of past menu entries' details.  The
          ;; default values will allow the system to boot, even if they differ
@@ -533,8 +528,7 @@ list of services."
   "Display a summary of system generation NUMBER in a human-readable format."
   (unless (zero? number)
     (let* ((generation  (generation-file-name profile number))
-           (param-file  (string-append generation "/parameters"))
-           (params      (call-with-input-file param-file read-boot-parameters))
+           (params      (read-boot-parameters-file generation))
            (label       (boot-parameters-label params))
            (root        (boot-parameters-root-device params))
            (root-device (if (bytevector? root)

bug#26544: [PATCH v4 06/10] scripts: Make boot-parameters label include generation number and time.

[Danny Milosavljevic]

* guix/scripts/system.scm (system->boot-parameters): Make label include
generation number and time.
---
 guix/scripts/system.scm | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index b278b6683..5e4e0df20 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -369,8 +369,13 @@ it atomically, and then run OS's activation script."
 NUMBERS, which is a list of generation numbers."
   (define (system->boot-parameters system number time)
     (unless-file-not-found
-     (let* ((params           (read-boot-parameters-file system)))
-       params)))
+     (let* ((params           (read-boot-parameters-file system))
+            (label            (boot-parameters-label params)))
+       (boot-parameters
+         (inherit params)
+         (label (string-append label " (#"
+                               (number->string number) ", "
+                               (seconds->string time) ")"))))))
   (let* ((systems (map (cut generation-file-name profile <>)
                        numbers))
          (times   (map (lambda (system)

bug#26544: [PATCH v4 06/10] scripts: Make boot-parameters label include generation number and time.

[Danny Milosavljevic]

On Fri, 21 Apr 2017 14:37:10 +0200
Danny Milosavljevic <dannym@scratchpost.org> wrote:

>    (define (system->boot-parameters system number time)
>      (unless-file-not-found
> -     (let* ((params           (read-boot-parameters-file system)))
> -       params)))
> +     (let* ((params           (read-boot-parameters-file system))
> +            (label            (boot-parameters-label params)))
> +       (boot-parameters
> +         (inherit params)
> +         (label (string-append label " (#"
> +                               (number->string number) ", "
> +                               (seconds->string time) ")"))))))

Every time I see that I wonder whether I don't make some other part (like read-boot-parameters-file or something) to do the label extension.  In theory that would be nicer than fixing it up later - I do the latter here.

And on first sight, one might be able to do that for the old generations (because the timestr in the label is just the mtime of the system directory).  But the label should also contain the generation number - which is currently usually being extracted by the procedure "generation-numbers" for an entire profile (but the caller of system also is able to override the generation numbers of profile-boot-parameters and profile-grub-entries - not sure why); I don't find generation-numbers exactly straightforward - I'd rather not mess with it.  Also, the caller zips the result together with SYSTEMS somehow and it would be easy for me to mess up the association.

Therefore, I opted for this version - which is a lot less risky.

If someone is more familiar with generation-numbers and knows why it's possible to override the generation numbers (usually by a one-element list), please feel free to post a patch that integrates this part into read-boot-parameters-file later.

Note: The current generation doesn't need its label extended.

bug#26544: [PATCH v4 07/10] system: vm: Use operating-system-kernel-arguments.

[Danny Milosavljevic]

* gnu/system/vm.scm (system-qemu-image/shared-store-script):
Use operating-system-kernel-arguments.
---
 gnu/system/vm.scm | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index 4f915c4f9..2c8b954c8 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -490,11 +490,8 @@ it is mostly useful when FULL-BOOT?  is true."
                                 #:full-boot? full-boot?
                                 #:disk-image-size disk-image-size)))
     (define kernel-arguments
-      #~(list "--root=/dev/vda1"
-              (string-append "--system=" #$os-drv)
-              (string-append "--load=" #$os-drv "/boot")
-              #$@(if graphic? #~() #~("console=ttyS0"))
-              #+@(operating-system-user-kernel-arguments os)))
+      #~(list #$@(if graphic? #~() #~("console=ttyS0"))
+              #+@(operating-system-kernel-arguments os os-drv "/dev/vda1")))
 
     (define qemu-exec
       #~(list (string-append #$qemu "/bin/" #$(qemu-command (%current-system)))

bug#26544: [PATCH v4 08/10] system: Use operating-system-boot-parameters directly.

[Danny Milosavljevic]

* gnu/system.scm (operating-system-bootcfg): Use
operating-system-boot-parameters directly.
---
 gnu/system.scm | 28 ++++------------------------
 1 file changed, 4 insertions(+), 24 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index 9921a9786..90fea9af3 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -750,33 +750,13 @@ populate the \"old entries\" menu."
   (mlet* %store-monad
       ((system      (operating-system-derivation os))
        (root-fs ->  (operating-system-root-file-system os))
-       (store-fs -> (operating-system-store-file-system os))
-       (label ->    (kernel->boot-label (operating-system-kernel os)))
-       (kernel ->   (operating-system-kernel-file os))
-       (initrd      (operating-system-initrd-file os))
        (root-device -> (if (eq? 'uuid (file-system-title root-fs))
                            (uuid->string (file-system-device root-fs))
                            (file-system-device root-fs)))
-       (entries ->  (list (menu-entry
-                           (label label)
-
-                           ;; The device where the kernel and initrd live.
-                           (device (fs->boot-device store-fs))
-                           (device-mount-point
-                            (file-system-mount-point store-fs))
-
-                           (linux kernel)
-                           (linux-arguments
-                            (cons* (string-append "--root=" root-device)
-                                   #~(string-append "--system=" #$system)
-                                   #~(string-append "--load=" #$system
-                                                    "/boot")
-                                   (operating-system-kernel-arguments os
-                                                                      system
-                                                                      root-device)))
-                           (initrd initrd)))))
-    (grub-configuration-file (operating-system-bootloader os) entries
-                             #:old-entries old-entries)))
+       (entry (operating-system-boot-parameters os system root-device)))
+    (grub-configuration-file (operating-system-bootloader os)
+                             (list entry)
+                              #:old-entries old-entries)))
 
 (define (fs->boot-device fs)
   "Given FS, a <file-system> object, return a value suitable for use as the

bug#26544: [PATCH v4 09/10] system: grub: Use boot-parameters instead of menu-entry where possible.

[Danny Milosavljevic]

* gnu/system/grub.scm (boot-parameters->menu-entry): New variable.
(grub-configuration-file): Use boot-parameters
instead of menu-entry where possible.
* guix/scripts/system.scm (profile-boot-parameters): Update docstring.
(reinstall-grub): Use profile-boot-parameters.
(perform-action): Use profile-boot-parameters.
---
 gnu/system/grub.scm     | 14 ++++++++++++--
 guix/scripts/system.scm |  8 ++++----
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/gnu/system/grub.scm b/gnu/system/grub.scm
index cde4b9e23..d2fa984ec 100644
--- a/gnu/system/grub.scm
+++ b/gnu/system/grub.scm
@@ -267,6 +267,16 @@ code."
         (#f
          #~(format #f "search --file --set ~a" #$file)))))
 
+(define (boot-parameters->menu-entry conf)
+  "Convert a <boot-parameters> instance to a corresponding <menu-entry>."
+  (menu-entry
+   (label (boot-parameters-label conf))
+   (device (boot-parameters-store-device conf))
+   (device-mount-point (boot-parameters-store-mount-point conf))
+   (linux (boot-parameters-kernel conf))
+   (linux-arguments (boot-parameters-kernel-arguments conf))
+   (initrd (boot-parameters-initrd conf))))
+
 (define* (grub-configuration-file config entries
                                   #:key
                                   (system (%current-system))
@@ -276,7 +286,7 @@ code."
 <file-system> object.  OLD-ENTRIES is taken to be a list of menu entries
 corresponding to old generations of the system."
   (define all-entries
-    (append entries
+    (append (map boot-parameters->menu-entry entries)
             (grub-configuration-menu-entries config)))
 
   (define entry->gexp
@@ -323,7 +333,7 @@ set timeout=~a~%"
             #$@(if (pair? old-entries)
                    #~((format port "
 submenu \"GNU system, old configurations...\" {~%")
-                      #$@(map entry->gexp old-entries)
+                      #$@(map entry->gexp (map boot-parameters->menu-entry old-entries))
                       (format port "}~%"))
                    #~()))))
 
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 5e4e0df20..242bd8074 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -365,7 +365,7 @@ it atomically, and then run OS's activation script."
 
 (define* (profile-boot-parameters #:optional (profile %system-profile)
                                   (numbers (generation-numbers profile)))
-  "Return a list of 'menu-entry' for the generations of PROFILE specified by
+  "Return a list of 'boot-parameters' for the generations of PROFILE specified by
 NUMBERS, which is a list of generation numbers."
   (define (system->boot-parameters system number time)
     (unless-file-not-found
@@ -456,9 +456,9 @@ open connection to the store."
          ;; from the actual past values for this generation's entry.
          (grub-config (grub-configuration (device root-device)))
          ;; Make the specified system generation the default entry.
-         (entries (profile-grub-entries %system-profile (list number)))
+         (entries (profile-boot-parameters %system-profile (list number)))
          (old-generations (delv number (generation-numbers %system-profile)))
-         (old-entries (profile-grub-entries %system-profile old-generations))
+         (old-entries (profile-boot-parameters %system-profile old-generations))
          (grub.cfg (run-with-store store
                      (grub-configuration-file grub-config
                                               entries
@@ -642,7 +642,7 @@ output when building a system derivation, such as a disk image."
                       (operating-system-bootcfg os
                                                 (if (eq? 'init action)
                                                     '()
-                                                    (profile-grub-entries)))))
+                                                    (profile-boot-parameters)))))
 
        ;; For 'init' and 'reconfigure', always build GRUB.CFG, even if
        ;; --no-grub is passed, because GRUB.CFG because we then use it as a GC

bug#26544: [PATCH v4 10/10] scripts: Remove profile-grub-entries.

[Danny Milosavljevic]

* guix/scripts/system.scm (profile-grub-entries): Delete variable.
---
 guix/scripts/system.scm | 37 -------------------------------------
 1 file changed, 37 deletions(-)

diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 242bd8074..3feccb2ab 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -384,43 +384,6 @@ NUMBERS, which is a list of generation numbers."
                        systems)))
     (filter-map system->boot-parameters systems numbers times)))
 
-(define* (profile-grub-entries #:optional (profile %system-profile)
-                                  (numbers (generation-numbers profile)))
-  "Return a list of 'menu-entry' for the generations of PROFILE specified by
-NUMBERS, which is a list of generation numbers."
-  (define (system->grub-entry system number time)
-    (unless-file-not-found
-     (let* ((params           (read-boot-parameters-file system))
-            (label            (boot-parameters-label params))
-            (root             (boot-parameters-root-device params))
-            (root-device      (if (bytevector? root)
-                                  (uuid->string root)
-                                  root))
-            (kernel           (boot-parameters-kernel params))
-            (kernel-arguments (boot-parameters-kernel-arguments params))
-            (initrd           (boot-parameters-initrd params)))
-       (menu-entry
-        (label (string-append label " (#"
-                              (number->string number) ", "
-                              (seconds->string time) ")"))
-        (device (boot-parameters-store-device params))
-        (device-mount-point (boot-parameters-store-mount-point params))
-        (linux kernel)
-        (linux-arguments
-         (cons* (string-append "--root=" root-device)
-                (string-append "--system=" system)
-                (string-append "--load=" system "/boot")
-                kernel-arguments))
-        (initrd initrd)))))
-
-  (let* ((systems (map (cut generation-file-name profile <>)
-                       numbers))
-         (times   (map (lambda (system)
-                         (unless-file-not-found
-                          (stat:mtime (lstat system))))
-                       systems)))
-    (filter-map system->grub-entry systems numbers times)))
-
 \f
 ;;;
 ;;; Roll-back.

bug#26544: [PATCH v4 10/10] scripts: Remove profile-grub-entries.

[Mathieu Othacehe]

Thanks for v3 and v4 !

It seems to me that my remarks have been addressed, except for the
old-entries label ? I can't found a patch fixing this issue.

I know that it is not common on this list, but a brief changelog for
each serie would be nice :)

Thanks,

Mathieu

Danny Milosavljevic writes:

> * guix/scripts/system.scm (profile-grub-entries): Delete variable.
> ---
>  guix/scripts/system.scm | 37 -------------------------------------
>  1 file changed, 37 deletions(-)
>
> diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
> index 242bd8074..3feccb2ab 100644
> --- a/guix/scripts/system.scm
> +++ b/guix/scripts/system.scm
> @@ -384,43 +384,6 @@ NUMBERS, which is a list of generation numbers."
>                         systems)))
>      (filter-map system->boot-parameters systems numbers times)))
>  
> -(define* (profile-grub-entries #:optional (profile %system-profile)
> -                                  (numbers (generation-numbers profile)))
> -  "Return a list of 'menu-entry' for the generations of PROFILE specified by
> -NUMBERS, which is a list of generation numbers."
> -  (define (system->grub-entry system number time)
> -    (unless-file-not-found
> -     (let* ((params           (read-boot-parameters-file system))
> -            (label            (boot-parameters-label params))
> -            (root             (boot-parameters-root-device params))
> -            (root-device      (if (bytevector? root)
> -                                  (uuid->string root)
> -                                  root))
> -            (kernel           (boot-parameters-kernel params))
> -            (kernel-arguments (boot-parameters-kernel-arguments params))
> -            (initrd           (boot-parameters-initrd params)))
> -       (menu-entry
> -        (label (string-append label " (#"
> -                              (number->string number) ", "
> -                              (seconds->string time) ")"))
> -        (device (boot-parameters-store-device params))
> -        (device-mount-point (boot-parameters-store-mount-point params))
> -        (linux kernel)
> -        (linux-arguments
> -         (cons* (string-append "--root=" root-device)
> -                (string-append "--system=" system)
> -                (string-append "--load=" system "/boot")
> -                kernel-arguments))
> -        (initrd initrd)))))
> -
> -  (let* ((systems (map (cut generation-file-name profile <>)
> -                       numbers))
> -         (times   (map (lambda (system)
> -                         (unless-file-not-found
> -                          (stat:mtime (lstat system))))
> -                       systems)))
> -    (filter-map system->grub-entry systems numbers times)))
> -
>  \f
>  ;;;
>  ;;; Roll-back.

bug#26544: [PATCH v4 10/10] scripts: Remove profile-grub-entries.

[Danny Milosavljevic]

Hi Mathieu,

On Fri, 21 Apr 2017 15:04:48 +0200
Mathieu Othacehe <m.othacehe@gmail.com> wrote:

> Thanks for v3 and v4 !
> 
> It seems to me that my remarks have been addressed, except for the
> old-entries label ? I can't found a patch fixing this issue.

PATCH v4 06/10 should also fix it.  Doesn't it?  I tested it here and it seems fine.

Or do I misunderstand which problem you mean?

> I know that it is not common on this list, but a brief changelog for
> each serie would be nice :)

I can do that for large patchsets such as this one.

Personally, I diff the diffs in such cases (i.e. "diff -u v3_a.patch v4_a.patch" etc). It's a bit jarring at first but it helps :)

bug#26544: [PATCH v4 10/10] scripts: Remove profile-grub-entries.

[Mathieu Othacehe]

Hi,

> PATCH v4 06/10 should also fix it.  Doesn't it?  I tested it here and it seems fine.
>
> Or do I misunderstand which problem you mean?

Nope, I just didn't received v4 06/10 when I wrote that, sorry.

I just tested v4 serie (reconfigure, list-generation, switch-generation
and vm), everything seems fine for me !

Thanks,

Mathieu

bug#26544: [PATCH v4 00/10] system: Move "--load" and other guix-specific parameters from the grub module to the generic system module.

[Danny Milosavljevic]

I'll hold off merging the other patches in the series until more time for review has passed (and more testing has been done).

I've merged the first two patches since they are just self-contained renames we have to do anyway.

If you have the time (and a way to recover a potential non-booting system - although it works for Mathieu and me), please test.

bug#26544: [PATCH v4 00/10] system: Move "--load" and other guix-specific parameters from the grub module to the generic system module.

[Danny Milosavljevic]

I've been testing this for quite some time now and will be pushing it in a few hours if there are no objections.

Note: If you get a guile backtrace like the following on "guix system reconfigure", you should run "make" in guix.  It will work just fine then.

Other than that, it's uneventful.

...
In ice-9/r4rs.scm:
  39: 8 [call-with-values #<procedure b7e74b0 at ice-9/eval.scm:416:20 ()> ...]
  39: 7 [call-with-values #<procedure 39ff210 at ice-9/eval.scm:416:20 ()> ...]
  39: 6 [call-with-values #<procedure 39ff1b0 at ice-9/eval.scm:416:20 ()> ...]
  39: 5 [call-with-values #<procedure bdda1b0 at ice-9/eval.scm:416:20 ()> ...]
  39: 4 [call-with-values #<procedure bdda150 at ice-9/eval.scm:416:20 ()> ...]
In ice-9/eval.scm:
 386: 3 [eval #<memoized ((<3> <1>) <0>)> (# # # ...)]
 387: 2 [eval # #]
 387: 1 [eval # #]
In unknown file:
   ?: 0 ["GNU with Linux-Libre 4.11 (beta)" "GNU with Linux-Libre 4.11 (beta)"]

ERROR: In procedure GNU with Linux-Libre 4.11 (beta):
ERROR: Wrong type to apply: "GNU with Linux-Libre 4.11 (beta)"

The guile that guix used is:

/gnu/store/k1bmz7glblfxdgaaas12q707lpjjk4mz-profile/bin/guile --no-auto-compile

guile 2.0.14

To reproduce, there all the ".go" files from guix, then invoke "guix system reconfigure ...".  It will fail with the above.

When you then run "make", wait until it's done, then invoke "guix system reconfigure ...", it will work.

bug#26544: [PATCH v4 00/10] system: Move "--load" and other guix-specific parameters from the grub module to the generic system module.

[Ludovic Courtès]

Hello!

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> I've been testing this for quite some time now and will be pushing it in a few hours if there are no objections.
>
> Note: If you get a guile backtrace like the following on "guix system reconfigure", you should run "make" in guix.  It will work just fine then.
>
> Other than that, it's uneventful.

Awesome.  If “make check-system” doesn’t show anything bad either, then
that’d good.

Thanks,
Ludo’.

bug#26544: Merged to master: [PATCH v4 *] system: Move "--load" and other guix-specific parameters from the grub module to the generic system module.

[Danny Milosavljevic]

I've merged this patchset to master: [PATCH v4 00/10] system: Move "--load" and other guix-specific parameters from the grub module to the generic system module.

Please make sure to run "make" in guix after pulling.