all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* G-Expressions for boot-parameters or grub-configuration-file ?
@ 2017-04-16 22:40 Danny Milosavljevic
  2017-04-16 23:02 ` Danny Milosavljevic
  0 siblings, 1 reply; 4+ messages in thread
From: Danny Milosavljevic @ 2017-04-16 22:40 UTC (permalink / raw)
  To: guix-devel

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

Hi,

I'm trying to simplify the bootloader support in master a bit and am running into trouble when trying to refactor operating-system-bootcfg:

When applying the attached patch and then doing

  guix system reconfigure /etc/config.scm --fallback

... I get:

ERROR: In procedure memoize-variable-access!:
ERROR: Unbound variable: ungexp

I'm pretty sure it's because of my change in gnu/system.scm operating-system-bootcfg.  I'm not sure what to put there:

(define* (operating-system-bootcfg os #:optional (old-entries '()))
  "Return the bootloader configuration file for OS.  Use OLD-ENTRIES to
populate the \"old entries\" menu."
  (mlet* %store-monad
      ((system      (operating-system-derivation os))
       (root-fs ->  (operating-system-root-file-system os))
       (root-device -> (if (eq? 'uuid (file-system-title root-fs))
                           (uuid->string (file-system-device root-fs))
                           (file-system-device root-fs)))
       (entries   -> (list (operating-system-boot-parameters
                            os
                            #$(derivation->output-path system) ; XXXX #$system doesn't work either
                            root-device))))
    (grub-configuration-file (operating-system-bootloader os) entries
                             #:old-entries old-entries)))


If I remove just the "#$" in front of "(derivation->output-path ...)", I get:

ERROR: In procedure scm_lreadr: /gnu/store/nn1z49lf1ds9nqcq41lm7n93vy0cgrsa-grub.cfg-builder:1:1797: Unknown # object: #\<
builder for `/gnu/store/3y9rfqn253s7labrw7mppm4gql0cipan-grub.cfg.drv' failed with exit code 1
guix system: error: build failed: build of `/gnu/store/3y9rfqn253s7labrw7mppm4gql0cipan-grub.cfg.drv' failed

It seems it couldn't serialize something... that something is:

'<procedure 7cc4640 at gnu/system.scm:683:2 (state)>) (format port "\\nsubmenu \\"GNU system, old config'

But that is a gexp in grub-configuration-file.

See attachment for WIP patch to current master.

Help...

[-- Attachment #2: bootloader-simplification.patch --]
[-- Type: text/x-patch, Size: 13642 bytes --]

diff --git a/gnu/system.scm b/gnu/system.scm
index 89c4150f9..5f20e8ab5 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.
@@ -735,29 +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)))))
+       (entries   -> (list (operating-system-boot-parameters
+                            os
+                            #$(derivation->output-path system)
+                            root-device))))
     (grub-configuration-file (operating-system-bootloader os) entries
                              #:old-entries old-entries)))
 
@@ -769,6 +764,25 @@ device in a <menu-entry>."
     ((label) (file-system-device fs))
     (else #f)))
 
+(define (operating-system-boot-parameters os system root-device)
+  "Return a <boot-parameters> record that describes the boot parameters of OS.
+SYSTEM is optional.  If given, adds kernel arguments for that system to <boot-parameters>."
+  (let* ((initrd   (operating-system-initrd-file os))
+         (store    (operating-system-store-file-system os))
+         (label    (kernel->boot-label (operating-system-kernel os))))
+    (boot-parameters
+      ;(version 0)
+      (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-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."
@@ -791,6 +805,20 @@ this file is the reconstruction of GRUB menu entries for old configurations."
                     (mount-point #$(file-system-mount-point store))))
                 #:set-load-path? #f)))
 
+;; Better would be instead, if I ever got it to work:
+(define (operating-system-boot-parameters-file os system)
+  "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."
+  (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"
+                #~(operating-system-boot-parameters os system (file-system-device root))
+                #:set-load-path? #f)))
+
 \f
 ;;;
 ;;; Boot parameters
@@ -866,4 +894,22 @@ 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/vm.scm b/gnu/system/vm.scm
index 374d8b663..e372c27b2 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -490,11 +490,9 @@ 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")
+      #~(list
               #$@(if graphic? #~() #~("console=ttyS0"))
-              #+@(operating-system-kernel-arguments os)))
+              #+@(operating-system-all-kernel-arguments os os-drv "/dev/vda1")))
 
     (define qemu-exec
       #~(list (string-append #$qemu "/bin/" #$(qemu-command (%current-system)))
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 9ffdc15ab..0749fbbd9 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -365,14 +365,17 @@ 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* ((file             (string-append system "/parameters"))
-            (params           (call-with-input-file file
-                                read-boot-parameters)))
-       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)
@@ -381,45 +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* ((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* ((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.
@@ -447,18 +411,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 +495,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 +604,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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: G-Expressions for boot-parameters or grub-configuration-file ?
  2017-04-16 22:40 G-Expressions for boot-parameters or grub-configuration-file ? Danny Milosavljevic
@ 2017-04-16 23:02 ` Danny Milosavljevic
  2017-04-17  8:43   ` Mathieu Othacehe
  0 siblings, 1 reply; 4+ messages in thread
From: Danny Milosavljevic @ 2017-04-16 23:02 UTC (permalink / raw)
  To: guix-devel

Forgot to mention the following chunk:

--- gnu/system/grub.scm 2017-04-16 02:01:46.621445795 +0200
+++ /home/dannym/src/guix/gnu/system/grub.scm   2017-04-16 00:26:19.654626204 +0200
@@ -267,6 +267,15 @@
         (#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 @@
 <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 @@
             #$@(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 "}~%"))
                    #~()))))

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: G-Expressions for boot-parameters or grub-configuration-file ?
  2017-04-16 23:02 ` Danny Milosavljevic
@ 2017-04-17  8:43   ` Mathieu Othacehe
  2017-04-17  9:57     ` Danny Milosavljevic
  0 siblings, 1 reply; 4+ messages in thread
From: Mathieu Othacehe @ 2017-04-17  8:43 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: guix-devel

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


Hi Danny,

Here's a sligtly modified version of your patch that works for "guix
reconfigure", I didn't tried any other use-cases.

I switched operating-system-boot-parameters into a monadic procedure,
because operating-system-initrd-file is also a monadic procedure.

I'll send a new version of my patch set soon taking your remarks into
account.

Thanks,

Mathieu


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: danny.patch --]
[-- Type: text/x-patch, Size: 15279 bytes --]

diff --git a/gnu/system.scm b/gnu/system.scm
index 89c4150f9..949eeb2bd 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.
@@ -735,30 +746,14 @@ 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
+       (entries  (operating-system-boot-parameters
+                  os
+                  system
+                  root-device)))
+    (grub-configuration-file (operating-system-bootloader os) (list entries)
                              #:old-entries old-entries)))
 
 (define (fs->boot-device fs)
@@ -769,6 +764,24 @@ device in a <menu-entry>."
     ((label) (file-system-device fs))
     (else #f)))
 
+(define (operating-system-boot-parameters os system root-device)
+  "Return a <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-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."
@@ -791,6 +804,20 @@ this file is the reconstruction of GRUB menu entries for old configurations."
                     (mount-point #$(file-system-mount-point store))))
                 #:set-load-path? #f)))
 
+;; Better would be instead, if I ever got it to work:
+(define (operating-system-boot-parameters-file os system)
+  "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."
+  (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"
+                #~(operating-system-boot-parameters os system (file-system-device root))
+                #:set-load-path? #f)))
+
 \f
 ;;;
 ;;; Boot parameters
@@ -866,4 +893,22 @@ 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..e372c27b2 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -490,11 +490,9 @@ 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")
+      #~(list
               #$@(if graphic? #~() #~("console=ttyS0"))
-              #+@(operating-system-kernel-arguments os)))
+              #+@(operating-system-all-kernel-arguments os os-drv "/dev/vda1")))
 
     (define qemu-exec
       #~(list (string-append #$qemu "/bin/" #$(qemu-command (%current-system)))
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 9ffdc15ab..0749fbbd9 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -365,14 +365,17 @@ 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* ((file             (string-append system "/parameters"))
-            (params           (call-with-input-file file
-                                read-boot-parameters)))
-       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)
@@ -381,45 +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* ((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* ((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.
@@ -447,18 +411,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 +495,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 +604,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

[-- Attachment #3: Type: text/plain, Size: 1597 bytes --]


Danny Milosavljevic writes:

> Forgot to mention the following chunk:
>
> --- gnu/system/grub.scm 2017-04-16 02:01:46.621445795 +0200
> +++ /home/dannym/src/guix/gnu/system/grub.scm   2017-04-16 00:26:19.654626204 +0200
> @@ -267,6 +267,15 @@
>          (#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 @@
>  <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 @@
>              #$@(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 "}~%"))
>                     #~()))))


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: G-Expressions for boot-parameters or grub-configuration-file ?
  2017-04-17  8:43   ` Mathieu Othacehe
@ 2017-04-17  9:57     ` Danny Milosavljevic
  0 siblings, 0 replies; 4+ messages in thread
From: Danny Milosavljevic @ 2017-04-17  9:57 UTC (permalink / raw)
  To: Mathieu Othacehe; +Cc: guix-devel

Hi Mathieu,

> Here's a sligtly modified version of your patch that works for "guix
> reconfigure", I didn't tried any other use-cases.

Thanks! I can confirm that it works for that use-case.

I think it would be a possible way to get a handle on the "--load" complexity.  Since it's not possible to store the store reference into the content-addressed "parameters" file this synthesizes it in the procedure that reads from the file.

Also, "--load" is not grub-specific - so having it inside the grub handler module in the first place was ... weird.

What do you think of this approach?

Still open is to make sure that the following doesn't happen (although it didn't happen to me yet so far):
1. read-boot-parameters-file prepends "--load" (etc)
2. guix stores the result into the "parameters" file
3. on the next "system reconfigure", read-boot-parameters-file reads it from the "parameters" file and prepends "--load" again, giving two sets of "--load" in the result.  I think this would eventually lead to ever-changing hash values.

> I switched operating-system-boot-parameters into a monadic procedure,
> because operating-system-initrd-file is also a monadic procedure.

Yes, thanks!

> I'll send a new version of my patch set soon taking your remarks into
> account.

Okay.  I'll review it.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-04-17  9:57 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-04-16 22:40 G-Expressions for boot-parameters or grub-configuration-file ? Danny Milosavljevic
2017-04-16 23:02 ` Danny Milosavljevic
2017-04-17  8:43   ` Mathieu Othacehe
2017-04-17  9:57     ` Danny Milosavljevic

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.