Add system roll-back and switch-generation commands

unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed

* Add system roll-back and switch-generation commands
@ 2016-10-28 10:07 cmmarusich
  2016-10-28 10:07 ` [PATCH 01/10] * gnu/system.scm (<boot-parameters>): Add 'store-device' and 'store-fs-mount-point' cmmarusich
                   ` (10 more replies)
  0 siblings, 11 replies; 42+ messages in thread
From: cmmarusich @ 2016-10-28 10:07 UTC (permalink / raw)
  To: guix-devel

Hi,

This patch series adds 'guix system roll-back' and 'guix system
switch-generation' commands.  It should apply cleanly to the current master
branch.

I've tested the patches manually on a VM with a single disk, and a VM with
multiple disks (with the store on a separate disk).  Two of the "check-system"
tests fail (nss-mdns and encrypted-root-os), but those are failing on the
current master branch for me anyway, and I can't recall ever seeing them pass,
so I have no reason to suspect that my changes are causing the failures.

I look forward to your feedback.

Thank you,
Chris

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

* [PATCH 01/10] * gnu/system.scm (<boot-parameters>): Add 'store-device' and 'store-fs-mount-point'.
  2016-10-28 10:07 Add system roll-back and switch-generation commands cmmarusich
@ 2016-10-28 10:07 ` cmmarusich
  2016-10-30  0:12   ` Ludovic Courtès
  2016-10-28 10:07 ` [PATCH 02/10] Add 'device' field to <menu-entry> cmmarusich
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 42+ messages in thread
From: cmmarusich @ 2016-10-28 10:07 UTC (permalink / raw)
  To: guix-devel

From: Chris Marusich <cmmarusich@gmail.com>

In a future commit, we will restructure the grub.cfg generation logic to use
this information, to enable the implementation of 'guix system
switch-generation' and 'guix system roll-back'.
---
 gnu/system.scm | 58 +++++++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 49 insertions(+), 9 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index 38ae8f1..ff84b63 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2013, 2014, 2015, 2016 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2015 Mark H Weaver <mhw@netris.org>
 ;;; Copyright © 2015, 2016 Alex Kost <alezost@gmail.com>
+;;; Copyright © 2016 Chris Marusich <cmmarusich@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -99,6 +100,8 @@
             boot-parameters?
             boot-parameters-label
             boot-parameters-root-device
+            boot-parameters-store-device
+            boot-parameters-store-fs-mount-point
             boot-parameters-kernel
             boot-parameters-kernel-arguments
             boot-parameters-initrd
@@ -750,17 +753,25 @@ listed in OS.  The C library expects to find it under
 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->grub-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-kernel-arguments os))
-                                   (initrd #$initrd))
-                #:set-load-path? #f)))
+    (gexp->file
+     "parameters"
+     #~(boot-parameters (version 1)
+                        (label #$label)
+                        (root-device #$(file-system-device root))
+                        (store-device
+                         #$(case (file-system-title store)
+                             ((uuid) (file-system-device store))
+                             ((label) (file-system-device store))
+                             (else #f)))
+                        (store-fs-mount-point #$(file-system-mount-point store))
+                        (kernel #$(operating-system-kernel-file os))
+                        (kernel-arguments
+                         #$(operating-system-kernel-arguments os))
+                        (initrd #$initrd))
+     #:set-load-path? #f)))
 
 \f
 ;;;
@@ -770,7 +781,16 @@ this file is the reconstruction of GRUB menu entries for old configurations."
 (define-record-type* <boot-parameters>
   boot-parameters make-boot-parameters boot-parameters?
   (label            boot-parameters-label)
+  ;; Because we will use the 'store-device' to create the GRUB search command,
+  ;; the 'store-device' has slightly different semantics than 'root-device'.
+  ;; The 'store-device' can be a file system uuid, a file system label, or #f,
+  ;; but it cannot be a device path such as "/dev/sda3", since GRUB would not
+  ;; understand that.  The 'root-device', on the other hand, corresponds
+  ;; exactly to the device field of the <file-system> object representing the
+  ;; OS's root file system, so it might be a device path like "/dev/sda3".
   (root-device      boot-parameters-root-device)
+  (store-device     boot-parameters-store-device)
+  (store-fs-mount-point boot-parameters-store-fs-mount-point)
   (kernel           boot-parameters-kernel)
   (kernel-arguments boot-parameters-kernel-arguments)
   (initrd           boot-parameters-initrd))
@@ -786,6 +806,10 @@ this file is the reconstruction of GRUB menu entries for old configurations."
      (boot-parameters
       (label label)
       (root-device root)
+      ;; For backwards compatibility, we assume the store device and the
+      ;; root device are the same.
+      (store-device root)
+      (store-fs-mount-point "/")
 
       ;; In the past, we would store the directory name of the kernel instead
       ;; of the absolute file name of its image.  Detect that and correct it.
@@ -805,6 +829,22 @@ this file is the reconstruction of GRUB menu entries for old configurations."
           (string-append directory file))
          (('initrd (? string? file))
           file)))))
+    (('boot-parameters ('version 1)
+                       ('label label)
+                       ('root-device root)
+                       ('store-device store)
+                       ('store-fs-mount-point store-fs-mount-point)
+                       ('kernel linux)
+                       ('kernel-arguments arguments)
+                       ('initrd initrd))
+     (boot-parameters
+      (label label)
+      (root-device root)
+      (store-device store)
+      (store-fs-mount-point store-fs-mount-point)
+      (kernel linux)
+      (kernel-arguments arguments)
+      (initrd initrd)))
     (x                                            ;unsupported format
      (warning (_ "unrecognized boot parameters for '~a'~%")
               system)
-- 
2.9.2

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

* Re: [PATCH 01/10] * gnu/system.scm (<boot-parameters>): Add 'store-device' and 'store-fs-mount-point'.
  2016-10-28 10:07 ` [PATCH 01/10] * gnu/system.scm (<boot-parameters>): Add 'store-device' and 'store-fs-mount-point' cmmarusich
@ 2016-10-30  0:12   ` Ludovic Courtès
  2016-10-30  9:41     ` Chris Marusich
  0 siblings, 1 reply; 42+ messages in thread
From: Ludovic Courtès @ 2016-10-30  0:12 UTC (permalink / raw)
  To: cmmarusich; +Cc: guix-devel

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

Hello,

I’ve browsed the whole patch series, and I think we’re pretty much
there.  I do have comments though, as always ;-), but I’m confident that
we’re doing the last round trip here.

If you’d like me to apply some of the the benign changes I suggest on
your behalf, let me know; I thought I’d rather get your opinion first,
and of course it’s always better for me if you do it yourself.  ;-)

cmmarusich@gmail.com skribis:

> From: Chris Marusich <cmmarusich@gmail.com>
>
> In a future commit, we will restructure the grub.cfg generation logic to use
> this information, to enable the implementation of 'guix system
> switch-generation' and 'guix system roll-back'.

General remark on commit logs: I can fix details there, but it would
help me if you could more closely follow conventions.

For instance, each commit needs a subject line prefixed by the area it
touches.  So this one would be “system: Add 'store-device' field to
<boot-parameters>.”

The commit log needs to list all the changes made.  In this case, it
should mention not only <boot-parameters> but also
‘operating-system-parameters-file’.  As a reviewer it helps to see which
parts are touched.

A simple way to get the conventions right is to run ‘git log’ on the
file and mimic what was previously done.

>  (define-record-type* <boot-parameters>
>    boot-parameters make-boot-parameters boot-parameters?
>    (label            boot-parameters-label)
> +  ;; Because we will use the 'store-device' to create the GRUB search command,
> +  ;; the 'store-device' has slightly different semantics than 'root-device'.
> +  ;; The 'store-device' can be a file system uuid, a file system label, or #f,
> +  ;; but it cannot be a device path such as "/dev/sda3", since GRUB would not
> +  ;; understand that.  The 'root-device', on the other hand, corresponds
> +  ;; exactly to the device field of the <file-system> object representing the
> +  ;; OS's root file system, so it might be a device path like "/dev/sda3".
>    (root-device      boot-parameters-root-device)
> +  (store-device     boot-parameters-store-device)
> +  (store-fs-mount-point boot-parameters-store-fs-mount-point)

In your patch series ‘store-fs-mount-point’ ends up being unused.

However, after rereading the whole series, I think what we need is a
‘device-mount-point’ in <menu-entry>, in a symmetrical fashion.

Attached is an updated patch.  I have grouped together the patches of
your series that touch this topic, including the documentation part
(this is all one logical change so it’s best to commit it as such), and
made the above change.

I’ve also followed my suggestion at
<https://lists.gnu.org/archive/html/guix-devel/2016-10/msg00568.html> to
not change the version of the <boot-parameters> sexp.

I’ve tested that the generated grub.cfg is what we expect in several
different scenario.

If that’s fine with you, I’d like to commit this version.  With that
done, the rest of the patch series will be rather easy.

WDYT?

Thank you!

Ludo’.


[-- Attachment #2: the new patch --]
[-- Type: text/x-patch, Size: 17875 bytes --]

From a8129e9282d673edb38fb52e7c373f3b380d35be Mon Sep 17 00:00:00 2001
From: Chris Marusich <cmmarusich@gmail.com>
Date: Fri, 28 Oct 2016 03:07:18 -0700
Subject: [PATCH] system: Record store file system info in each generation.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* gnu/system.scm (<boot-parameters>)[store-device, store-mount-point]:
New fields.
(read-boot-parameters): Initialize them.
(operating-system-grub.cfg): Likewise.  Remove STORE-FS argument from
call to 'grub-configuration-file'.
(operating-system-parameters-file): Add 'store' element in
'boot-parameters'.
* gnu/system/grub.scm (strip-mount-point): Replace 'store-fs' parameter
by 'mount-point'; adjust accordingly.  Adjust callers.
(<menu-entry>)[device, device-mount-point]: New fields.
(eye-candy): Replace 'root-fs' parameter by 'store-device'; add
'store-mount-point'.  Use keyword arguments for 'system' and 'port'.
(grub-root-search): Remove 'root-fs' by 'device' and adjust
accordingly.
(grub-configuration-file): Remove 'store-fs' parameter.  Adjust
accordingly.
* guix/scripts/system.scm (previous-grub-entries): Initialize 'device'
and 'device-mount-point' fields from PARAMS.
* doc/guix.texi (GRUB Configuration): Document 'device' and
'device-mount-point'.  Explain that 'linux' can be prefixed by a GRUB
device name.

Co-authored-by: Ludovic Courtès <ludo@gnu.org>
---
 doc/guix.texi           | 27 ++++++++++++++++
 gnu/system.scm          | 59 ++++++++++++++++++++++++++++------
 gnu/system/grub.scm     | 85 ++++++++++++++++++++++++++++---------------------
 guix/scripts/system.scm |  3 ++
 4 files changed, 128 insertions(+), 46 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 22d353c..a351964 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -11088,6 +11088,17 @@ The Linux kernel image to boot, for example:
 (file-append linux-libre "/bzImage")
 @end example
 
+It is also possible to specify a device explicitly in the file path
+using GRUB's device naming convention (@pxref{Naming convention,,, grub,
+GNU GRUB manual}), for example:
+
+@example
+"(hd0,msdos1)/boot/vmlinuz"
+@end example
+
+If the device is specified explicitly as above, then the @code{device}
+field is ignored entirely.
+
 @item @code{linux-arguments} (default: @code{()})
 The list of extra Linux kernel command-line arguments---e.g.,
 @code{("console=ttyS0")}.
@@ -11096,6 +11107,22 @@ The list of extra Linux kernel command-line arguments---e.g.,
 A G-Expression or string denoting the file name of the initial RAM disk
 to use (@pxref{G-Expressions}).
 
+@item @code{device} (default: @code{#f})
+The device where the kernel and initrd are to be found---i.e., the GRUB
+@dfn{root} for this menu entry (@pxref{root,,, grub, GNU GRUB manual}).
+
+This may be a file system label (a string), a file system UUID (a
+bytevector, @pxref{File Systems}), or @code{#f}, in which case GRUB will
+search the device containing the file specified by the @code{linux}
+field (@pxref{search,,, grub, GNU GRUB manual}).  It must @emph{not} be
+an OS device name such as @file{/dev/sda1}.
+
+@item @code{device-mount-point} (default: @code{"/"})
+The mount point of the above device on the system.  You probably do not
+need to change the default value.  GuixSD uses it to strip the prefix of
+store file names for systems where @file{/gnu} or @file{/gnu/store} is
+on a separate partition.
+
 @end table
 @end deftp
 
diff --git a/gnu/system.scm b/gnu/system.scm
index 38ae8f1..259875d 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2013, 2014, 2015, 2016 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2015 Mark H Weaver <mhw@netris.org>
 ;;; Copyright © 2015, 2016 Alex Kost <alezost@gmail.com>
+;;; Copyright © 2016 Chris Marusich <cmmarusich@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -99,6 +100,8 @@
             boot-parameters?
             boot-parameters-label
             boot-parameters-root-device
+            boot-parameters-store-device
+            boot-parameters-store-mount-point
             boot-parameters-kernel
             boot-parameters-kernel-arguments
             boot-parameters-initrd
@@ -733,6 +736,12 @@ listed in OS.  The C library expects to find it under
                            (file-system-device root-fs)))
        (entries ->  (list (menu-entry
                            (label label)
+
+                           ;; The device where the kernel and initrd live.
+                           (device (file-system-device store-fs))
+                           (device-mount-point
+                            (file-system-mount-point store-fs))
+
                            (linux kernel)
                            (linux-arguments
                             (cons* (string-append "--root=" root-device)
@@ -741,8 +750,7 @@ listed in OS.  The C library expects to find it under
                                                     "/boot")
                                    (operating-system-kernel-arguments os)))
                            (initrd initrd)))))
-    (grub-configuration-file (operating-system-bootloader os)
-                             store-fs entries
+    (grub-configuration-file (operating-system-bootloader os) entries
                              #:old-entries old-entries)))
 
 (define (operating-system-parameters-file os)
@@ -750,16 +758,24 @@ listed in OS.  The C library expects to find it under
 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->grub-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-kernel-arguments os))
-                                   (initrd #$initrd))
+                #~(boot-parameters
+                   (version 0)
+                   (label #$label)
+                   (root-device #$(file-system-device root))
+                   (kernel #$(operating-system-kernel-file os))
+                   (kernel-arguments
+                    #$(operating-system-kernel-arguments os))
+                   (initrd #$initrd)
+                   (store
+                    (device #$(case (file-system-title store)
+                                ((uuid) (file-system-device store))
+                                ((label) (file-system-device store))
+                                (else #f)))
+                    (mount-point #$(file-system-mount-point store))))
                 #:set-load-path? #f)))
 
 \f
@@ -770,7 +786,16 @@ this file is the reconstruction of GRUB menu entries for old configurations."
 (define-record-type* <boot-parameters>
   boot-parameters make-boot-parameters boot-parameters?
   (label            boot-parameters-label)
+  ;; Because we will use the 'store-device' to create the GRUB search command,
+  ;; the 'store-device' has slightly different semantics than 'root-device'.
+  ;; The 'store-device' can be a file system uuid, a file system label, or #f,
+  ;; but it cannot be a device path such as "/dev/sda3", since GRUB would not
+  ;; understand that.  The 'root-device', on the other hand, corresponds
+  ;; exactly to the device field of the <file-system> object representing the
+  ;; OS's root file system, so it might be a device path like "/dev/sda3".
   (root-device      boot-parameters-root-device)
+  (store-device     boot-parameters-store-device)
+  (store-mount-point boot-parameters-store-mount-point)
   (kernel           boot-parameters-kernel)
   (kernel-arguments boot-parameters-kernel-arguments)
   (initrd           boot-parameters-initrd))
@@ -804,7 +829,21 @@ this file is the reconstruction of GRUB menu entries for old configurations."
          (('initrd ('string-append directory file)) ;the old format
           (string-append directory file))
          (('initrd (? string? file))
-          file)))))
+          file)))
+
+      (store-device
+       (match (assq 'store rest)
+         (('store ('device device) _ ...)
+          device)
+         (_                                       ;the old format
+          root)))
+
+      (store-mount-point
+       (match (assq 'store rest)
+         (('store ('device _) ('mount-point mount-point) _ ...)
+          mount-point)
+         (_                                       ;the old format
+          "/")))))
     (x                                            ;unsupported format
      (warning (_ "unrecognized boot parameters for '~a'~%")
               system)
diff --git a/gnu/system/grub.scm b/gnu/system/grub.scm
index 249b415..5c9d0f1 100644
--- a/gnu/system/grub.scm
+++ b/gnu/system/grub.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2013, 2014, 2015, 2016 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2016 Chris Marusich <cmmarusich@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -32,6 +33,7 @@
   #:use-module (ice-9 match)
   #:use-module (ice-9 regex)
   #:use-module (srfi srfi-1)
+  #:use-module (rnrs bytevectors)
   #:export (grub-image
             grub-image?
             grub-image-aspect-ratio
@@ -61,16 +63,15 @@
 ;;;
 ;;; Code:
 
-(define (strip-mount-point fs file)
-  "Strip the mount point of FS from FILE, which is a gexp or other lowerable
-object denoting a file name."
-  (let ((mount-point (file-system-mount-point fs)))
-    (if (string=? mount-point "/")
-	file
-	#~(let ((file #$file))
-            (if (string-prefix? #$mount-point file)
-                (substring #$file #$(string-length mount-point))
-                file)))))
+(define (strip-mount-point mount-point file)
+  "Strip MOUNT-POINT from FILE, which is a gexp or other lowerable object
+denoting a file name."
+  (if (string=? mount-point "/")
+      file
+      #~(let ((file #$file))
+          (if (string-prefix? #$mount-point file)
+              (substring #$file #$(string-length mount-point))
+              file))))
 
 (define-record-type* <grub-image>
   grub-image make-grub-image
@@ -121,6 +122,10 @@ object denoting a file name."
   menu-entry make-menu-entry
   menu-entry?
   (label           menu-entry-label)
+  (device          menu-entry-device       ; file system uuid, label, or #f
+                   (default #f))
+  (device-mount-point menu-entry-device-mount-point
+                      (default "/"))
   (linux           menu-entry-linux)
   (linux-arguments menu-entry-linux-arguments
                    (default '()))          ; list of string-valued gexps
@@ -162,12 +167,14 @@ WIDTH/HEIGHT, or #f if none was found."
         (with-monad %store-monad
           (return #f)))))
 
-(define (eye-candy config root-fs system port)
+(define* (eye-candy config store-device store-mount-point
+                    #:key system port)
   "Return in %STORE-MONAD a gexp that writes to PORT (a port-valued gexp) the
 'grub.cfg' part concerned with graphics mode, background images, colors, and
-all that.  ROOT-FS is a file-system object denoting the root file system where
-the store is.  SYSTEM must be the target system string---e.g.,
-\"x86_64-linux\"."
+all that.  STORE-DEVICE designates the device holding the store, and
+STORE-MOUNT-POINT is its mount point; these are used to determine where the
+background image and fonts must be searched for.  SYSTEM must be the target
+system string---e.g., \"x86_64-linux\"."
   (define setup-gfxterm-body
     ;; Intel systems need to be switched into graphics mode, whereas most
     ;; other modern architectures have no other mode and therefore don't need
@@ -191,7 +198,7 @@ the store is.  SYSTEM must be the target system string---e.g.,
                      (symbol->string (assoc-ref colors 'bg)))))
 
   (define font-file
-    (strip-mount-point root-fs
+    (strip-mount-point store-mount-point
                        (file-append grub "/share/grub/unicode.pf2")))
 
   (mlet* %store-monad ((image (grub-background-image config)))
@@ -215,10 +222,10 @@ else
   set menu_color_highlight=white/blue
 fi~%"
                            #$setup-gfxterm-body
-                           #$(grub-root-search root-fs font-file)
+                           #$(grub-root-search store-device font-file)
                            #$font-file
 
-                           #$(strip-mount-point root-fs image)
+                           #$(strip-mount-point store-mount-point image)
                            #$(theme-colors grub-theme-color-normal)
                            #$(theme-colors grub-theme-color-highlight))))))
 
@@ -227,8 +234,8 @@ fi~%"
 ;;; Configuration file.
 ;;;
 
-(define (grub-root-search root-fs file)
-  "Return the GRUB 'search' command to look for ROOT-FS, which contains FILE,
+(define (grub-root-search device file)
+  "Return the GRUB 'search' command to look for DEVICE, which contains FILE,
 a gexp.  The result is a gexp that can be inserted in the grub.cfg-generation
 code."
   ;; Usually FILE is a file name gexp like "/gnu/store/…-linux/vmlinuz", but
@@ -236,20 +243,18 @@ code."
   ;; custom menu entries.  In the latter case, don't emit a 'search' command.
   (if (and (string? file) (not (string-prefix? "/" file)))
       ""
-      (case (file-system-title root-fs)
-        ;; Preferably refer to ROOT-FS by its UUID or label.  This is more
+      (match device
+        ;; Preferably refer to DEVICE by its UUID or label.  This is more
         ;; efficient and less ambiguous, see <>.
-        ((uuid)
+        ((? bytevector? uuid)
          (format #f "search --fs-uuid --set ~a"
-                 (uuid->string (file-system-device root-fs))))
-        ((label)
-         (format #f "search --label --set ~a"
-                 (file-system-device root-fs)))
-        (else
-         ;; As a last resort, look for any device containing FILE.
+                 (uuid->string device)))
+        ((? string? label)
+         (format #f "search --label --set ~a" label))
+        (#f
          #~(format #f "search --file --set ~a" #$file)))))
 
-(define* (grub-configuration-file config store-fs entries
+(define* (grub-configuration-file config entries
                                   #:key
                                   (system (%current-system))
                                   (old-entries '()))
@@ -262,22 +267,30 @@ corresponding to old generations of the system."
 
   (define entry->gexp
     (match-lambda
-     (($ <menu-entry> label linux arguments initrd)
-      ;; Use the right file names for LINUX and STORE-FS in case STORE-FS is
-      ;; not the "/" file system.
-      (let ((linux  (strip-mount-point store-fs linux))
-            (initrd (strip-mount-point store-fs initrd)))
+     (($ <menu-entry> label device device-mount-point
+                      linux arguments initrd)
+      ;; Here DEVICE is the store and DEVICE-MOUNT-POINT is its mount point.
+      ;; Use the right file names for LINUX and INITRD in case
+      ;; DEVICE-MOUNT-POINT is not "/", meaning that the store is on a
+      ;; separate partition.
+      (let ((linux  (strip-mount-point device-mount-point linux))
+            (initrd (strip-mount-point device-mount-point initrd)))
         #~(format port "menuentry ~s {
   ~a
   linux ~a ~a
   initrd ~a
 }~%"
                   #$label
-                  #$(grub-root-search store-fs linux)
+                  #$(grub-root-search device linux)
                   #$linux (string-join (list #$@arguments))
                   #$initrd)))))
 
-  (mlet %store-monad ((sugar (eye-candy config store-fs system #~port)))
+  (mlet %store-monad ((sugar (eye-candy config
+                                        (menu-entry-device (first entries))
+                                        (menu-entry-device-mount-point
+                                         (first entries))
+                                        #:system system
+                                        #:port #~port)))
     (define builder
       #~(call-with-output-file #$output
           (lambda (port)
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 0519ab8..e548be6 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2014, 2015, 2016 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2016 Alex Kost <alezost@gmail.com>
+;;; Copyright © 2016 Chris Marusich <cmmarusich@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -384,6 +385,8 @@ it atomically, and then run OS's activation script."
         (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)
-- 
2.10.1


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

* Re: [PATCH 01/10] * gnu/system.scm (<boot-parameters>): Add 'store-device' and 'store-fs-mount-point'.
  2016-10-30  0:12   ` Ludovic Courtès
@ 2016-10-30  9:41     ` Chris Marusich
  2016-10-30 22:19       ` Ludovic Courtès
  0 siblings, 1 reply; 42+ messages in thread
From: Chris Marusich @ 2016-10-30  9:41 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guix-devel

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

ludo@gnu.org (Ludovic Courtès) writes:

> General remark on commit logs: I can fix details there, but it would
> help me if you could more closely follow conventions.

Understood.  I read the manual ((standards) Change Logs) and tried to
make it look like existing commits, but I see I missed some stuff, so
I'll do better next time.  Thank you for pointing out the spots that
need improvement.

>>  (define-record-type* <boot-parameters>
>>    boot-parameters make-boot-parameters boot-parameters?
>>    (label            boot-parameters-label)
>> +  ;; Because we will use the 'store-device' to create the GRUB search command,
>> +  ;; the 'store-device' has slightly different semantics than 'root-device'.
>> +  ;; The 'store-device' can be a file system uuid, a file system label, or #f,
>> +  ;; but it cannot be a device path such as "/dev/sda3", since GRUB would not
>> +  ;; understand that.  The 'root-device', on the other hand, corresponds
>> +  ;; exactly to the device field of the <file-system> object representing the
>> +  ;; OS's root file system, so it might be a device path like "/dev/sda3".
>>    (root-device      boot-parameters-root-device)
>> +  (store-device     boot-parameters-store-device)
>> +  (store-fs-mount-point boot-parameters-store-fs-mount-point)
>
> In your patch series ‘store-fs-mount-point’ ends up being unused.

Is this true?  I think we're using it.  For example, in commit
18813be0053b4797071e20f373a8a9e8d669e32a (Implement switch-generation
and roll-back), in procedure 'reinstall-grub', don't we use it to get
the mount point for each <boot-parameters>?

> However, after rereading the whole series, I think what we need is a
> ‘device-mount-point’ in <menu-entry>, in a symmetrical fashion.

That would work, and I'm open to it.  However, before we decide to do
that, what do you think about the alternative in which we simply require
that all the paths in the <boot-parameters> and <menu-entry> objects
must be relative paths?  By relative, I mean relative to the GRUB root.
If all the paths were relative, then we would not need a
'device-mount-point' field in the first place.

However, I suspect it might be tricky to get all the information we need
if we do it that way.  For example, during switch-generation, how would
we know the correct paths to use for the files in the GRUB
configuration's 'eye-candy' section?  In light of that, I suspect it
would be better to do what you've suggested: include the mount point in
both <boot-parameters> and <menu-entry>.

> Attached is an updated patch.  I have grouped together the patches of
> your series that touch this topic, including the documentation part
> (this is all one logical change so it’s best to commit it as such), and
> made the above change.

Thank you for doing this.  I've looked it over.  It looks good to me.
Unless you think strongly that we should use relative paths in the
<boot-parameters> and <menu-entry> objects, I think it's good as-is!

> I’ve also followed my suggestion at
> <https://lists.gnu.org/archive/html/guix-devel/2016-10/msg00568.html> to
> not change the version of the <boot-parameters> sexp.

OK.  FYI, I bumped the version number in my patch series because I
thought it would simplify the matching logic (it was still backwards
compatible).  However, looking at your version, I see now that it wasn't
too hard to keep the version number the same.  Your version looks good
to me!

> I’ve tested that the generated grub.cfg is what we expect in several
> different scenario.

Thank you for taking the extra time to test it.

> If that’s fine with you, I’d like to commit this version.  With that
> done, the rest of the patch series will be rather easy.
>
> WDYT?
>

Sounds good to me.  Unless you think strongly that we should use
relative paths in the <boot-parameters> and <menu-entry> objects, I will
re-submit my patches once you have committed this change to the official
repo.

> Thank you!

Thank you for all of your help and tips!  I really appreciate it, and
it's all been very helpful.

-- 
Chris

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

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

* Re: [PATCH 01/10] * gnu/system.scm (<boot-parameters>): Add 'store-device' and 'store-fs-mount-point'.
  2016-10-30  9:41     ` Chris Marusich
@ 2016-10-30 22:19       ` Ludovic Courtès
  2016-11-02  5:48         ` Follow-up: Add system roll-back and switch-generation commands cmmarusich
  2016-11-03  0:19         ` [PATCH 01/10] * gnu/system.scm (<boot-parameters>): Add 'store-device' and 'store-fs-mount-point' Leo Famulari
  0 siblings, 2 replies; 42+ messages in thread
From: Ludovic Courtès @ 2016-10-30 22:19 UTC (permalink / raw)
  To: Chris Marusich; +Cc: guix-devel

Chris Marusich <cmmarusich@gmail.com> skribis:

> ludo@gnu.org (Ludovic Courtès) writes:

>> However, after rereading the whole series, I think what we need is a
>> ‘device-mount-point’ in <menu-entry>, in a symmetrical fashion.
>
> That would work, and I'm open to it.  However, before we decide to do
> that, what do you think about the alternative in which we simply require
> that all the paths in the <boot-parameters> and <menu-entry> objects
> must be relative paths?  By relative, I mean relative to the GRUB root.
> If all the paths were relative, then we would not need a
> 'device-mount-point' field in the first place.
>
> However, I suspect it might be tricky to get all the information we need
> if we do it that way.  For example, during switch-generation, how would
> we know the correct paths to use for the files in the GRUB
> configuration's 'eye-candy' section?  In light of that, I suspect it
> would be better to do what you've suggested: include the mount point in
> both <boot-parameters> and <menu-entry>.

Yeah, I’m not sure we could easily manage to have only relative file
names because most of them are computed when the derivations get built
(that’s why ‘strip-mount-point’ currently has to return a gexp instead
of stripping the mount point directly.)

>> Attached is an updated patch.  I have grouped together the patches of
>> your series that touch this topic, including the documentation part
>> (this is all one logical change so it’s best to commit it as such), and
>> made the above change.
>
> Thank you for doing this.  I've looked it over.  It looks good to me.
> Unless you think strongly that we should use relative paths in the
> <boot-parameters> and <menu-entry> objects, I think it's good as-is!

Good!

>> If that’s fine with you, I’d like to commit this version.  With that
>> done, the rest of the patch series will be rather easy.
>>
>> WDYT?
>>
>
> Sounds good to me.

Pushed as 1ef8b72a7f87afe7cffe52393d99e1b14e4770e1!

> Unless you think strongly that we should use relative paths in the
> <boot-parameters> and <menu-entry> objects, I will re-submit my
> patches once you have committed this change to the official repo.

That would be awesome.

Thanks again for all your work and patience!

Ludo’.

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

* Follow-up: Add system roll-back and switch-generation commands
  2016-10-30 22:19       ` Ludovic Courtès
@ 2016-11-02  5:48         ` cmmarusich
  2016-11-02  5:48           ` [PATCH 1/5] profiles: Extract a procedure for getting relative generation numbers cmmarusich
                             ` (4 more replies)
  2016-11-03  0:19         ` [PATCH 01/10] * gnu/system.scm (<boot-parameters>): Add 'store-device' and 'store-fs-mount-point' Leo Famulari
  1 sibling, 5 replies; 42+ messages in thread
From: cmmarusich @ 2016-11-02  5:48 UTC (permalink / raw)
  To: guix-devel

This patch series builds on 1ef8b72a7f87afe7cffe52393d99e1b14e4770e1 to add
two new 'guix system' actions: roll-back and switch-generation.

I've verified that both 'make check' and 'make check-system' pass (with the
exception of the system tests 'nss-mdns' and 'encrypted-root-os', which both
fail on my system even without my patches).  I've manually verified the
following for a VM with a single disk and single partition, as well as for a
VM with multiple disks and the store on a separate partition:

* The grub.cfg file that 'roll-back' and 'switch-generation' generates looks
  correct and is correctly registered as a GC root.

* The system can boot after 'roll-back' and 'switch-generation', the default
  entry is correct, and the resources necessary for the GRUB theme are loaded
  without error.

* Any attempt to switch to a nonexistent generation fail, regardless
  of whether a relative or absolute generation spec is used.

* The existing 'list-generations' command continues to show the
  generations correctly, even after switching generations.

Hopefully everything is OK this time!  Thank you for taking the time to review
it.

--
Chris

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

* [PATCH 1/5] profiles: Extract a procedure for getting relative generation numbers.
  2016-11-02  5:48         ` Follow-up: Add system roll-back and switch-generation commands cmmarusich
@ 2016-11-02  5:48           ` cmmarusich
  2016-11-06 16:56             ` Ludovic Courtès
  2016-11-02  5:48           ` [PATCH 2/5] system: Rename previous-grub-entries to profile-grub-entries cmmarusich
                             ` (3 subsequent siblings)
  4 siblings, 1 reply; 42+ messages in thread
From: cmmarusich @ 2016-11-02  5:48 UTC (permalink / raw)
  To: guix-devel

From: Chris Marusich <cmmarusich@gmail.com>

* guix/profiles.scm (relative-generation-spec->number): New procedure.
* guix/scripts/package.scm (switch-generation-action): Use it.
---
 guix/profiles.scm        | 19 +++++++++++++++++++
 guix/scripts/package.scm |  8 ++------
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/guix/profiles.scm b/guix/profiles.scm
index 96f8b65..33a5f50 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -5,6 +5,7 @@
 ;;; Copyright © 2015 Mark H Weaver <mhw@netris.org>
 ;;; Copyright © 2015 Sou Bunnbu <iyzsong@gmail.com>
 ;;; Copyright © 2016 Ricardo Wurmus <rekado@elephly.net>
+;;; Copyright © 2016 Chris Marusich <cmmarusich@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -97,6 +98,7 @@
             generation-number
             generation-numbers
             profile-generations
+            relative-generation-spec->number
             relative-generation
             previous-generation-number
             generation-time
@@ -1039,6 +1041,23 @@ former profiles were found."
         '()
         generations)))
 
+(define (relative-generation-spec->number profile spec)
+  "Return PROFILE's generation specified by SPEC, which is a string.  The SPEC
+may be a N, -N, or +N, where N is a number.  If the spec is N, then the number
+returned is N.  If it is -N, then the number returned is the profile's current
+generation number minus N.  If it is +N, then the number returned is the
+profile's current generation number plus N.  Return #f if there is no such
+generation."
+  (let ((number (string->number spec)))
+    (and number
+         (case (string-ref spec 0)
+           ((#\+ #\-)
+            (relative-generation profile number))
+           (else (if (memv number (profile-generations profile))
+                     number
+                     #f))))))
+
+
 (define* (relative-generation profile shift #:optional
                               (current (generation-number profile)))
   "Return PROFILE's generation shifted from the CURRENT generation by SHIFT.
diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index 70ed0a7..96a22f6 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -5,6 +5,7 @@
 ;;; Copyright © 2014, 2016 Alex Kost <alezost@gmail.com>
 ;;; Copyright © 2016 Roel Janssen <roel@gnu.org>
 ;;; Copyright © 2016 Benz Schenk <benz.schenk@uzh.ch>
+;;; Copyright © 2016 Chris Marusich <cmmarusich@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -782,12 +783,7 @@ processed, #f otherwise."
                                    #:key dry-run?)
   "Switch PROFILE to the generation specified by SPEC."
   (unless dry-run?
-    (let* ((number (string->number spec))
-           (number (and number
-                        (case (string-ref spec 0)
-                          ((#\+ #\-)
-                           (relative-generation profile number))
-                          (else number)))))
+    (let ((number (relative-generation-spec->number profile spec)))
       (if number
           (switch-to-generation* profile number)
           (leave (_ "cannot switch to generation '~a'~%") spec)))))
-- 
2.9.2

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

* Re: [PATCH 1/5] profiles: Extract a procedure for getting relative generation numbers.
  2016-11-02  5:48           ` [PATCH 1/5] profiles: Extract a procedure for getting relative generation numbers cmmarusich
@ 2016-11-06 16:56             ` Ludovic Courtès
  0 siblings, 0 replies; 42+ messages in thread
From: Ludovic Courtès @ 2016-11-06 16:56 UTC (permalink / raw)
  To: cmmarusich; +Cc: guix-devel

cmmarusich@gmail.com skribis:

> From: Chris Marusich <cmmarusich@gmail.com>
>
> * guix/profiles.scm (relative-generation-spec->number): New procedure.
> * guix/scripts/package.scm (switch-generation-action): Use it.

Applied!

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

* [PATCH 2/5] system: Rename previous-grub-entries to profile-grub-entries.
  2016-11-02  5:48         ` Follow-up: Add system roll-back and switch-generation commands cmmarusich
  2016-11-02  5:48           ` [PATCH 1/5] profiles: Extract a procedure for getting relative generation numbers cmmarusich
@ 2016-11-02  5:48           ` cmmarusich
  2016-11-06 16:56             ` Ludovic Courtès
  2016-11-02  5:48           ` [PATCH 3/5] system: Optionally limit the entries returned by profile-grub-entries cmmarusich
                             ` (2 subsequent siblings)
  4 siblings, 1 reply; 42+ messages in thread
From: cmmarusich @ 2016-11-02  5:48 UTC (permalink / raw)
  To: guix-devel

From: Chris Marusich <cmmarusich@gmail.com>

* guix/scripts/system.scm (previous-grub-entries, profile-grub-entries):
Rename previous-grub-entries to profile-grub-entries to indicate that it is
stateful and returns the entries for all profile generations, not just the
previous ones.  Update all callers.
---
 guix/scripts/system.scm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index e548be6..eee4422 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -366,7 +366,7 @@ it atomically, and then run OS's activation script."
     (date->string (time-utc->date time)
                   "~Y-~m-~d ~H:~M")))
 
-(define* (previous-grub-entries #:optional (profile %system-profile))
+(define* (profile-grub-entries #:optional (profile %system-profile))
   "Return a list of 'menu-entry' for the generations of PROFILE."
   (define (system->grub-entry system number time)
     (unless-file-not-found
@@ -563,7 +563,7 @@ building anything."
                       (operating-system-grub.cfg os
                                                  (if (eq? 'init action)
                                                      '()
-                                                     (previous-grub-entries)))))
+                                                     (profile-grub-entries)))))
 
        ;; 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
-- 
2.9.2

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

* Re: [PATCH 2/5] system: Rename previous-grub-entries to profile-grub-entries.
  2016-11-02  5:48           ` [PATCH 2/5] system: Rename previous-grub-entries to profile-grub-entries cmmarusich
@ 2016-11-06 16:56             ` Ludovic Courtès
  0 siblings, 0 replies; 42+ messages in thread
From: Ludovic Courtès @ 2016-11-06 16:56 UTC (permalink / raw)
  To: cmmarusich; +Cc: guix-devel

cmmarusich@gmail.com skribis:

> From: Chris Marusich <cmmarusich@gmail.com>
>
> * guix/scripts/system.scm (previous-grub-entries, profile-grub-entries):
> Rename previous-grub-entries to profile-grub-entries to indicate that it is
> stateful and returns the entries for all profile generations, not just the
> previous ones.  Update all callers.

Applied!

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

* [PATCH 3/5] system: Optionally limit the entries returned by profile-grub-entries.
  2016-11-02  5:48         ` Follow-up: Add system roll-back and switch-generation commands cmmarusich
  2016-11-02  5:48           ` [PATCH 1/5] profiles: Extract a procedure for getting relative generation numbers cmmarusich
  2016-11-02  5:48           ` [PATCH 2/5] system: Rename previous-grub-entries to profile-grub-entries cmmarusich
@ 2016-11-02  5:48           ` cmmarusich
  2016-11-06 16:57             ` Ludovic Courtès
  2016-11-02  5:48           ` [PATCH 4/5] install: Extract procedure: install-grub-config cmmarusich
  2016-11-02  5:48           ` [PATCH 5/5] system: Add 'guix system' actions: switch-generation and roll-back cmmarusich
  4 siblings, 1 reply; 42+ messages in thread
From: cmmarusich @ 2016-11-02  5:48 UTC (permalink / raw)
  To: guix-devel

From: Chris Marusich <cmmarusich@gmail.com>

* guix/scripts/system.scm (profile-grub-entries): Add an optional parameter
which allows the caller to limit the number of returned grub entries.
---
 guix/scripts/system.scm | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index eee4422..5bc40ed 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -366,8 +366,10 @@ it atomically, and then run OS's activation script."
     (date->string (time-utc->date time)
                   "~Y-~m-~d ~H:~M")))
 
-(define* (profile-grub-entries #:optional (profile %system-profile))
-  "Return a list of 'menu-entry' for the generations of PROFILE."
+(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"))
@@ -395,8 +397,7 @@ it atomically, and then run OS's activation script."
                 kernel-arguments))
         (initrd initrd)))))
 
-  (let* ((numbers (generation-numbers profile))
-         (systems (map (cut generation-file-name profile <>)
+  (let* ((systems (map (cut generation-file-name profile <>)
                        numbers))
          (times   (map (lambda (system)
                          (unless-file-not-found
-- 
2.9.2

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

* Re: [PATCH 3/5] system: Optionally limit the entries returned by profile-grub-entries.
  2016-11-02  5:48           ` [PATCH 3/5] system: Optionally limit the entries returned by profile-grub-entries cmmarusich
@ 2016-11-06 16:57             ` Ludovic Courtès
  0 siblings, 0 replies; 42+ messages in thread
From: Ludovic Courtès @ 2016-11-06 16:57 UTC (permalink / raw)
  To: cmmarusich; +Cc: guix-devel

cmmarusich@gmail.com skribis:

> From: Chris Marusich <cmmarusich@gmail.com>
>
> * guix/scripts/system.scm (profile-grub-entries): Add an optional parameter
> which allows the caller to limit the number of returned grub entries.

Applied!

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

* [PATCH 4/5] install: Extract procedure: install-grub-config.
  2016-11-02  5:48         ` Follow-up: Add system roll-back and switch-generation commands cmmarusich
                             ` (2 preceding siblings ...)
  2016-11-02  5:48           ` [PATCH 3/5] system: Optionally limit the entries returned by profile-grub-entries cmmarusich
@ 2016-11-02  5:48           ` cmmarusich
  2016-11-06 16:59             ` Ludovic Courtès
  2016-11-06 21:00             ` Danny Milosavljevic
  2016-11-02  5:48           ` [PATCH 5/5] system: Add 'guix system' actions: switch-generation and roll-back cmmarusich
  4 siblings, 2 replies; 42+ messages in thread
From: cmmarusich @ 2016-11-02  5:48 UTC (permalink / raw)
  To: guix-devel

From: Chris Marusich <cmmarusich@gmail.com>

* gnu/build/install.scm (install-grub-config): New procedure.
(install-grub): Use it.
---
 gnu/build/install.scm | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/gnu/build/install.scm b/gnu/build/install.scm
index 7431a09..3d1594e 100644
--- a/gnu/build/install.scm
+++ b/gnu/build/install.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2013, 2014, 2015, 2016 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2016 Chris Marusich <cmmarusich@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -22,6 +23,7 @@
   #:use-module (srfi srfi-26)
   #:use-module (ice-9 match)
   #:export (install-grub
+            install-grub-config
             populate-root-file-system
             reset-timestamps
             register-closure
@@ -36,13 +38,24 @@
 ;;;
 ;;; Code:
 
-(define* (install-grub grub.cfg device mount-point)
+(define (install-grub grub.cfg device mount-point)
   "Install GRUB with GRUB.CFG on DEVICE, which is assumed to be mounted on
 MOUNT-POINT.
 
 Note that the caller must make sure that GRUB.CFG is registered as a GC root
 so that the fonts, background images, etc. referred to by GRUB.CFG are not
 GC'd."
+  (install-grub-config grub.cfg mount-point)
+  (unless (zero? (system* "grub-install" "--no-floppy"
+                          "--boot-directory"
+                          (string-append mount-point "/boot")
+                          device))
+    (error "failed to install GRUB")))
+
+(define (install-grub-config grub.cfg mount-point)
+  "Atomically copy GRUB.CFG into boot/grub/grub.cfg on the MOUNT-POINT.  Note
+that the caller must make sure that GRUB.CFG is registered as a GC root so
+that the fonts, background images, etc. referred to by GRUB.CFG are not GC'd."
   (let* ((target (string-append mount-point "/boot/grub/grub.cfg"))
          (pivot  (string-append target ".new")))
     (mkdir-p (dirname target))
@@ -50,13 +63,7 @@ GC'd."
     ;; Copy GRUB.CFG instead of just symlinking it, because symlinks won't
     ;; work when /boot is on a separate partition.  Do that atomically.
     (copy-file grub.cfg pivot)
-    (rename-file pivot target)
-
-    (unless (zero? (system* "grub-install" "--no-floppy"
-                            "--boot-directory"
-                            (string-append mount-point "/boot")
-                            device))
-      (error "failed to install GRUB"))))
+    (rename-file pivot target)))
 
 (define (evaluate-populate-directive directive target)
   "Evaluate DIRECTIVE, an sexp describing a file or directory to create under
-- 
2.9.2

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

* Re: [PATCH 4/5] install: Extract procedure: install-grub-config.
  2016-11-02  5:48           ` [PATCH 4/5] install: Extract procedure: install-grub-config cmmarusich
@ 2016-11-06 16:59             ` Ludovic Courtès
  2016-11-06 21:00             ` Danny Milosavljevic
  1 sibling, 0 replies; 42+ messages in thread
From: Ludovic Courtès @ 2016-11-06 16:59 UTC (permalink / raw)
  To: cmmarusich; +Cc: guix-devel

cmmarusich@gmail.com skribis:

> From: Chris Marusich <cmmarusich@gmail.com>
>
> * gnu/build/install.scm (install-grub-config): New procedure.
> (install-grub): Use it.

Applied!

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

* Re: [PATCH 4/5] install: Extract procedure: install-grub-config.
  2016-11-02  5:48           ` [PATCH 4/5] install: Extract procedure: install-grub-config cmmarusich
  2016-11-06 16:59             ` Ludovic Courtès
@ 2016-11-06 21:00             ` Danny Milosavljevic
  2016-11-07  1:25               ` Chris Marusich
  1 sibling, 1 reply; 42+ messages in thread
From: Danny Milosavljevic @ 2016-11-06 21:00 UTC (permalink / raw)
  To: cmmarusich; +Cc: guix-devel

Thanks for this!

> -(define* (install-grub grub.cfg device mount-point)
> +(define (install-grub grub.cfg device mount-point)
>    "Install GRUB with GRUB.CFG on DEVICE, which is assumed to be mounted on
>  MOUNT-POINT.
>  
>  Note that the caller must make sure that GRUB.CFG is registered as a GC root
>  so that the fonts, background images, etc. referred to by GRUB.CFG are not
>  GC'd."
> +  (install-grub-config grub.cfg mount-point)
> +  (unless (zero? (system* "grub-install" "--no-floppy"

^^^ since you have touched it, maybe you know: How does it know which package's (which directory's) grub-install to invoke here?

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

* Re: [PATCH 4/5] install: Extract procedure: install-grub-config.
  2016-11-06 21:00             ` Danny Milosavljevic
@ 2016-11-07  1:25               ` Chris Marusich
  2016-11-07 10:32                 ` Danny Milosavljevic
  0 siblings, 1 reply; 42+ messages in thread
From: Chris Marusich @ 2016-11-07  1:25 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: guix-devel

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

Danny Milosavljevic <dannym@scratchpost.org> writes:

> Thanks for this!
>
>> -(define* (install-grub grub.cfg device mount-point)
>> +(define (install-grub grub.cfg device mount-point)
>>    "Install GRUB with GRUB.CFG on DEVICE, which is assumed to be mounted on
>>  MOUNT-POINT.
>>  
>>  Note that the caller must make sure that GRUB.CFG is registered as a GC root
>>  so that the fonts, background images, etc. referred to by GRUB.CFG are not
>>  GC'd."
>> +  (install-grub-config grub.cfg mount-point)
>> +  (unless (zero? (system* "grub-install" "--no-floppy"
>
> ^^^ since you have touched it, maybe you know: How does it know which package's (which directory's) grub-install to invoke here?

There is logic in the perform-action procedure in (guix scripts system)
which adds GRUB to the PATH environment variable.  That's where the
determination occurs.  So, currently, it looks like the 'grub-install'
command comes from whatever version of grub happens to be defined in
(gnu packages grub) at the time perform-action gets called.  In
practice, I believe this means that whatever grub version is currently
available in guix (e.g., as seen via 'guix package --show=grub') will be
used.

Hope that helps!

-- 
Chris

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

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

* Re: [PATCH 4/5] install: Extract procedure: install-grub-config.
  2016-11-07  1:25               ` Chris Marusich
@ 2016-11-07 10:32                 ` Danny Milosavljevic
  0 siblings, 0 replies; 42+ messages in thread
From: Danny Milosavljevic @ 2016-11-07 10:32 UTC (permalink / raw)
  To: Chris Marusich; +Cc: guix-devel

Hi Chris,

> Hope that helps!

Yeah, thank you!

(I think it would be a lot less confusing to just pass (derivation->output-path grub) to (install-grub* ...) as well instead of that O_o)

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

* [PATCH 5/5] system: Add 'guix system' actions: switch-generation and roll-back.
  2016-11-02  5:48         ` Follow-up: Add system roll-back and switch-generation commands cmmarusich
                             ` (3 preceding siblings ...)
  2016-11-02  5:48           ` [PATCH 4/5] install: Extract procedure: install-grub-config cmmarusich
@ 2016-11-02  5:48           ` cmmarusich
  2016-11-03  4:51             ` One more patch: doc: Add details to the 'guix system switch-generation' section Chris Marusich
  2016-11-06 17:13             ` [PATCH 5/5] system: Add 'guix system' actions: switch-generation and roll-back Ludovic Courtès
  4 siblings, 2 replies; 42+ messages in thread
From: cmmarusich @ 2016-11-02  5:48 UTC (permalink / raw)
  To: guix-devel

From: Chris Marusich <cmmarusich@gmail.com>

* guix/scripts/system.scm (roll-back-system, switch-to-system-generation): new
actions.
(reinstall-grub): New procedure, used by switch-to-system-generation.
(show-help, process-command, guix-system): Honor the new actions.
* doc/guix.texi (Invoking guix system) <switch-generation, roll-back>: Add the
new actions.
<reconfigure>: In the footnote, mention that the new actions also only work on
GuixSD.
---
 doc/guix.texi           | 49 +++++++++++++++++++++++--
 guix/scripts/system.scm | 95 +++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 136 insertions(+), 8 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 1075a7e..fbcf3b0 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -11155,8 +11155,9 @@ supported:
 @table @code
 @item reconfigure
 Build the operating system described in @var{file}, activate it, and
-switch to it@footnote{This action is usable only on systems already
-running GuixSD.}.
+switch to it@footnote{This action (and the related actions
+@code{switch-generation} and @code{roll-back}) are usable only on
+systems already running GuixSD.}.
 
 This effects all the configuration specified in @var{file}: user
 accounts, system services, global package list, setuid programs, etc.
@@ -11178,6 +11179,50 @@ guix pull}).  Failing to do that you would see an older version of Guix
 once @command{reconfigure} has completed.
 @end quotation
 
+@item switch-generation
+Switch to an existing system generation.  This action rearranges the
+existing GRUB menu entries.  It makes the menu entry for the target
+system generation the default, and it moves the entries for the other
+system generations to a submenu.  The next time the system boots, it
+will use the specified system generation.
+
+The target generation can be specified explicitly by its generation
+number.  For example, the following invocation would switch to system
+generation 7:
+
+@example
+guix system switch-generation 7
+@end example
+
+The target generation can also be specified relative to the current
+generation with the form @code{+N} or @code{-N}, where @code{+3} means
+``3 generations ahead of the current generation,'' and @code{-1} means
+``1 generation prior to the current generation.''  When specifying a
+negative value such as @code{-1}, you must precede it with @code{--} to
+prevent it from being parsed as an option.  For example:
+
+@example
+guix system switch-generation -- -1
+@end example
+
+Currently, the effect of invoking this action is only to rearrange the
+GRUB menu entries.  To actually start using the target system
+generation, you must reboot after running this action.  In the future,
+it will be updated to do the same things as @command{reconfigure}, like
+activating and deactivating services.
+
+This action will fail if the specified generation does not exist.
+
+@item roll-back
+Switch to the preceding system generation.  The next time the system
+boots, it will use the preceding system generation.  This is the inverse
+of @command{reconfigure}, and it is exactly the same as invoking
+@command{switch-generation} with an argument of @code{-1}.
+
+Currently, as with @command{switch-generation}, you must reboot after
+running this action to actually start using the preceding system
+generation.
+
 @item build
 Build the derivation of the operating system, which includes all the
 configuration files and programs needed to boot and run the system.
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 5bc40ed..df9b37d 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -407,6 +407,65 @@ NUMBERS, which is a list of generation numbers."
 
 \f
 ;;;
+;;; Roll-back.
+;;;
+(define (roll-back-system store)
+  "Roll back the system profile to its previous generation.  STORE is an open
+connection to the store."
+  (switch-to-system-generation store "-1"))
+\f
+;;;
+;;; Switch generations.
+;;;
+(define (switch-to-system-generation store spec)
+  "Switch the system profile to the generation specified by SPEC, and
+re-install grub with a grub configuration file that uses the specified system
+generation as its default entry.  STORE is an open connection to the store."
+  (let ((number (relative-generation-spec->number %system-profile spec)))
+    (if number
+        (begin
+          (reinstall-grub store number)
+          (switch-to-generation* %system-profile number))
+        (leave (_ "cannot switch to system generation '~a'~%") spec))))
+
+(define (reinstall-grub store number)
+  "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)))
+         (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)))
+         (old-generations (delv number (generation-numbers %system-profile)))
+         (old-entries (profile-grub-entries %system-profile old-generations))
+         (grub.cfg (run-with-store store
+                     (grub-configuration-file grub-config
+                                              entries
+                                              #:old-entries old-entries))))
+    (show-what-to-build store (list grub.cfg))
+    (build-derivations store (list grub.cfg))
+    ;; This is basically the same as install-grub*, but for now we avoid
+    ;; re-installing the GRUB boot loader itself onto a device, mainly because
+    ;; we don't in general have access to the same version of the GRUB package
+    ;; which was used when installing this other system generation.
+    (let* ((grub.cfg-path (derivation->output-path grub.cfg))
+           (gc-root (string-append %gc-roots-directory "/grub.cfg"))
+           (temp-gc-root (string-append gc-root ".new")))
+      (switch-symlinks temp-gc-root grub.cfg-path)
+      (unless (false-if-exception (install-grub-config grub.cfg-path "/"))
+        (delete-file temp-gc-root)
+        (leave (_ "failed to re-install GRUB configuration file: '~a'~%")
+               grub.cfg-path))
+      (rename-file temp-gc-root gc-root))))
+
+\f
+;;;
 ;;; Graphs.
 ;;;
 
@@ -641,14 +700,19 @@ building anything."
 ;;;
 
 (define (show-help)
-  (display (_ "Usage: guix system [OPTION] ACTION [FILE]
-Build the operating system declared in FILE according to ACTION.\n"))
+  (display (_ "Usage: guix system [OPTION ...] ACTION [ARG ...] [FILE]
+Build the operating system declared in FILE according to ACTION.
+Some ACTIONS support additional ARGS.\n"))
   (newline)
   (display (_ "The valid values for ACTION are:\n"))
   (newline)
   (display (_ "\
    reconfigure      switch to a new operating system configuration\n"))
   (display (_ "\
+   roll-back        switch to the previous operating system configuration\n"))
+  (display (_ "\
+   switch-generation switch to an existing operating system configuration\n"))
+  (display (_ "\
    list-generations list the system generations\n"))
   (display (_ "\
    build            build the operating system without installing anything\n"))
@@ -809,15 +873,33 @@ resulting from command-line parsing."
   "Process COMMAND, one of the 'guix system' sub-commands.  ARGS is its
 argument list and OPTS is the option alist."
   (case command
+    ;; The following commands do not need to use the store, and they do not need
+    ;; an operating system configuration file.
     ((list-generations)
-     ;; List generations.  No need to connect to the daemon, etc.
      (let ((pattern (match args
                       (() "")
                       ((pattern) pattern)
                       (x (leave (_ "wrong number of arguments~%"))))))
        (list-generations pattern)))
-    (else
-     (process-action command args opts))))
+    ;; The following commands need to use the store, but they do not need an
+    ;; operating system configuration file.
+    ((switch-generation)
+     (let ((pattern (match args
+                      ((pattern) pattern)
+                      (x (leave (_ "wrong number of arguments~%"))))))
+       (with-store store
+         (set-build-options-from-command-line store opts)
+         (switch-to-system-generation store pattern))))
+    ((roll-back)
+     (let ((pattern (match args
+                      (() "")
+                      (x (leave (_ "wrong number of arguments~%"))))))
+       (with-store store
+         (set-build-options-from-command-line store opts)
+         (roll-back-system store))))
+    ;; The following commands need to use the store, and they also
+    ;; need an operating system configuration file.
+    (else (process-action command args opts))))
 
 (define (guix-system . args)
   (define (parse-sub-command arg result)
@@ -827,7 +909,8 @@ argument list and OPTS is the option alist."
         (let ((action (string->symbol arg)))
           (case action
             ((build container vm vm-image disk-image reconfigure init
-              extension-graph shepherd-graph list-generations)
+              extension-graph shepherd-graph list-generations roll-back
+              switch-generation)
              (alist-cons 'action action result))
             (else (leave (_ "~a: unknown action~%") action))))))
 
-- 
2.9.2

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

* One more patch: doc: Add details to the 'guix system switch-generation' section.
  2016-11-02  5:48           ` [PATCH 5/5] system: Add 'guix system' actions: switch-generation and roll-back cmmarusich
@ 2016-11-03  4:51             ` Chris Marusich
  2016-11-06 17:13             ` [PATCH 5/5] system: Add 'guix system' actions: switch-generation and roll-back Ludovic Courtès
  1 sibling, 0 replies; 42+ messages in thread
From: Chris Marusich @ 2016-11-03  4:51 UTC (permalink / raw)
  To: guix-devel


[-- Attachment #1.1: Type: text/plain, Size: 143 bytes --]

Hi,

I realized I forgot to include some info in the manual, so here's one
more patch to add the missing info.

Thank you,

-- 
Chris

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-doc-Add-details-to-the-guix-system-switch-generation.patch --]
[-- Type: text/x-patch, Size: 2602 bytes --]

From faee634b5ae6a3033671e6bca07fc5ed1bc699c8 Mon Sep 17 00:00:00 2001
From: Chris Marusich <cmmarusich@gmail.com>
Date: Wed, 2 Nov 2016 20:38:26 -0700
Subject: [PATCH] doc: Add details to the 'guix system switch-generation'
 section.

* doc/guix.texi (Invoking guix system) <switch-generation>: Mention the fact
that we also switch the profile to an existing generation (i.e., flip
symlinks), and that we do it atomically.
---
 doc/guix.texi | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index fbcf3b0..b8cb01f 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -11180,11 +11180,12 @@ once @command{reconfigure} has completed.
 @end quotation
 
 @item switch-generation
-Switch to an existing system generation.  This action rearranges the
-existing GRUB menu entries.  It makes the menu entry for the target
-system generation the default, and it moves the entries for the other
-system generations to a submenu.  The next time the system boots, it
-will use the specified system generation.
+Switch to an existing system generation.  This action atomically
+switches the system profile to the specified system generation.  It also
+rearranges the system's existing GRUB menu entries.  It makes the menu
+entry for the specified system generation the default, and it moves the
+entries for the other generations to a submenu.  The next time the
+system boots, it will use the specified system generation.
 
 The target generation can be specified explicitly by its generation
 number.  For example, the following invocation would switch to system
@@ -11205,11 +11206,12 @@ prevent it from being parsed as an option.  For example:
 guix system switch-generation -- -1
 @end example
 
-Currently, the effect of invoking this action is only to rearrange the
-GRUB menu entries.  To actually start using the target system
-generation, you must reboot after running this action.  In the future,
-it will be updated to do the same things as @command{reconfigure}, like
-activating and deactivating services.
+Currently, the effect of invoking this action is @emph{only} to switch
+the system profile to an existing generation and rearrange the GRUB menu
+entries.  To actually start using the target system generation, you must
+reboot after running this action.  In the future, it will be updated to
+do the same things as @command{reconfigure}, like activating and
+deactivating services.
 
 This action will fail if the specified generation does not exist.
 
-- 
2.10.2


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

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

* Re: [PATCH 5/5] system: Add 'guix system' actions: switch-generation and roll-back.
  2016-11-02  5:48           ` [PATCH 5/5] system: Add 'guix system' actions: switch-generation and roll-back cmmarusich
  2016-11-03  4:51             ` One more patch: doc: Add details to the 'guix system switch-generation' section Chris Marusich
@ 2016-11-06 17:13             ` Ludovic Courtès
  2016-11-07  3:17               ` Chris Marusich
  1 sibling, 1 reply; 42+ messages in thread
From: Ludovic Courtès @ 2016-11-06 17:13 UTC (permalink / raw)
  To: cmmarusich; +Cc: guix-devel

cmmarusich@gmail.com skribis:

> From: Chris Marusich <cmmarusich@gmail.com>
>
> * guix/scripts/system.scm (roll-back-system, switch-to-system-generation): new
> actions.
> (reinstall-grub): New procedure, used by switch-to-system-generation.
> (show-help, process-command, guix-system): Honor the new actions.
> * doc/guix.texi (Invoking guix system) <switch-generation, roll-back>: Add the
> new actions.
> <reconfigure>: In the footnote, mention that the new actions also only work on
> GuixSD.

Applied.

> I realized I forgot to include some info in the manual, so here's one
> more patch to add the missing info.

I merged it with the one above.

I’m really happy we have these two new commands now, it fixes a glaring
lack.

Thanks for your hard work and patience!

With this in place, and once you’ve recovered from this patch series
;-), it should be relatively easy to add ‘guix system
delete-generations’ (similar to ‘guix package --delete-generations’.)
WDYT?

The next more complicated thing would be to see what needs to be done so
we can restart system services upon roll-back.

Ludo’.

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

* Re: [PATCH 5/5] system: Add 'guix system' actions: switch-generation and roll-back.
  2016-11-06 17:13             ` [PATCH 5/5] system: Add 'guix system' actions: switch-generation and roll-back Ludovic Courtès
@ 2016-11-07  3:17               ` Chris Marusich
  0 siblings, 0 replies; 42+ messages in thread
From: Chris Marusich @ 2016-11-07  3:17 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guix-devel

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

ludo@gnu.org (Ludovic Courtès) writes:

> I’m really happy we have these two new commands now, it fixes a glaring
> lack.
>
> Thanks for your hard work and patience!

I'm glad we have it now, too!  There's a lot more to do, but it's a good
start.

> With this in place, and once you’ve recovered from this patch series
> ;-), it should be relatively easy to add ‘guix system
> delete-generations’ (similar to ‘guix package --delete-generations’.)
> WDYT?

Sounds like a good idea!

> The next more complicated thing would be to see what needs to be done so
> we can restart system services upon roll-back.

Right.  Eventually we should have all the stuff discussed here:

https://lists.gnu.org/archive/html/guix-devel/2016-06/msg00173.html

Thank you for all your help and feedback.  It's helped me to understand
Guile and Guix better.  I hope to keep helping out where I can!

-- 
Chris

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

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

* Re: [PATCH 01/10] * gnu/system.scm (<boot-parameters>): Add 'store-device' and 'store-fs-mount-point'.
  2016-10-30 22:19       ` Ludovic Courtès
  2016-11-02  5:48         ` Follow-up: Add system roll-back and switch-generation commands cmmarusich
@ 2016-11-03  0:19         ` Leo Famulari
  2016-11-03  4:36           ` Chris Marusich
  2016-11-03 13:10           ` Fix a boot problem reported by ng0 cmmarusich
  1 sibling, 2 replies; 42+ messages in thread
From: Leo Famulari @ 2016-11-03  0:19 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guix-devel

On Sun, Oct 30, 2016 at 11:19:56PM +0100, Ludovic Courtès wrote:
> Chris Marusich <cmmarusich@gmail.com> skribis:
> >> If that’s fine with you, I’d like to commit this version.  With that
> >> done, the rest of the patch series will be rather easy.
> >>
> >> WDYT?
> >>
> >
> > Sounds good to me.
> 
> Pushed as 1ef8b72a7f87afe7cffe52393d99e1b14e4770e1!

I wonder, has anybody besides ng0 tried using this with libreboot? They
recently reconfigured their GuixSD system after this was committed, and
since then they aren't able to boot.

The IRC discussion begins here:

https://gnunet.org/bot/log/guix/2016-11-02#T1191881

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

* Re: [PATCH 01/10] * gnu/system.scm (<boot-parameters>): Add 'store-device' and 'store-fs-mount-point'.
  2016-11-03  0:19         ` [PATCH 01/10] * gnu/system.scm (<boot-parameters>): Add 'store-device' and 'store-fs-mount-point' Leo Famulari
@ 2016-11-03  4:36           ` Chris Marusich
  2016-11-03 10:35             ` Chris Marusich
  2016-11-03 13:10           ` Fix a boot problem reported by ng0 cmmarusich
  1 sibling, 1 reply; 42+ messages in thread
From: Chris Marusich @ 2016-11-03  4:36 UTC (permalink / raw)
  To: Leo Famulari; +Cc: guix-devel

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

Leo Famulari <leo@famulari.name> writes:

> I wonder, has anybody besides ng0 tried using this with libreboot? They
> recently reconfigured their GuixSD system after this was committed, and
> since then they aren't able to boot.
>
> The IRC discussion begins here:
>
> https://gnunet.org/bot/log/guix/2016-11-02#T1191881

I'm using Libreboot on a Lenovo X200 laptop.  I've reconfigured my
system using the change mentioned, and it boots without issue.

I reconfigured by running 'sudo guix system reconfigure config.scm'
after running 'sudo guix pull' to update root's Guix installation.  The
version of guix I used to do the reconfigure was definitely after commit
1ef8b72a7f87afe7cffe52393d99e1b14e4770e1.  I know this because I've
confirmed that the changes introduced by that commit are present in
/root/.config/guix/latest, and as I understand it that the code in that
directory is what gets used when running 'guix system reconfigure
config.scm' as root.

FWIW, my Libreboot details (as reported by running the command
'lscoreboot' after pressing "C" in the Libreboot GRUB menu when my
computer first boots) are as follows:

* version: `04f4afb-dirty'
* extra version: `'
* build: `Sat May 14 01:48:09 UTC 2016'

I hope that helps.

-- 
Chris

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

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

* Re: [PATCH 01/10] * gnu/system.scm (<boot-parameters>): Add 'store-device' and 'store-fs-mount-point'.
  2016-11-03  4:36           ` Chris Marusich
@ 2016-11-03 10:35             ` Chris Marusich
  2016-11-03 22:34               ` Danny Milosavljevic
  0 siblings, 1 reply; 42+ messages in thread
From: Chris Marusich @ 2016-11-03 10:35 UTC (permalink / raw)
  To: Leo Famulari; +Cc: guix-devel

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

Chris Marusich <cmmarusich@gmail.com> writes:

> Leo Famulari <leo@famulari.name> writes:
>
>> I wonder, has anybody besides ng0 tried using this with libreboot? They
>> recently reconfigured their GuixSD system after this was committed, and
>> since then they aren't able to boot.
>>
>> The IRC discussion begins here:
>>
>> https://gnunet.org/bot/log/guix/2016-11-02#T1191881
>
> I'm using Libreboot on a Lenovo X200 laptop.  I've reconfigured my
> system using the change mentioned, and it boots without issue.
>
> I reconfigured by running 'sudo guix system reconfigure config.scm'
> after running 'sudo guix pull' to update root's Guix installation.  The
> version of guix I used to do the reconfigure was definitely after commit
> 1ef8b72a7f87afe7cffe52393d99e1b14e4770e1.  I know this because I've
> confirmed that the changes introduced by that commit are present in
> /root/.config/guix/latest, and as I understand it that the code in that
> directory is what gets used when running 'guix system reconfigure
> config.scm' as root.
>
> FWIW, my Libreboot details (as reported by running the command
> 'lscoreboot' after pressing "C" in the Libreboot GRUB menu when my
> computer first boots) are as follows:
>
> * version: `04f4afb-dirty'
> * extra version: `'
> * build: `Sat May 14 01:48:09 UTC 2016'
>
> I hope that helps.

Additional info: I've just now upgraded my x200 laptop to the latest
release of Libreboot (r20160907), and I can still boot into GuixSD
normally.

-- 
Chris

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

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

* Re: [PATCH 01/10] * gnu/system.scm (<boot-parameters>): Add 'store-device' and 'store-fs-mount-point'.
  2016-11-03 10:35             ` Chris Marusich
@ 2016-11-03 22:34               ` Danny Milosavljevic
  2016-11-04  3:34                 ` Chris Marusich
  0 siblings, 1 reply; 42+ messages in thread
From: Danny Milosavljevic @ 2016-11-03 22:34 UTC (permalink / raw)
  To: Chris Marusich; +Cc: guix-devel

Hi,

On Thu, 03 Nov 2016 03:35:42 -0700
Chris Marusich <cmmarusich@gmail.com> wrote:
> Additional info: I've just now upgraded my x200 laptop to the latest
> release of Libreboot (r20160907), and I can still boot into GuixSD
> normally.

Really? I have an X200 with libreboot and I had to manually create a symlink /boot/grub/libreboot_grub.cfg -> /boot/grub/grub.cfg - otherwise I'd require complicated incantations for each boot.

Do you have such a symlink?

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

* Re: [PATCH 01/10] * gnu/system.scm (<boot-parameters>): Add 'store-device' and 'store-fs-mount-point'.
  2016-11-03 22:34               ` Danny Milosavljevic
@ 2016-11-04  3:34                 ` Chris Marusich
  2016-11-04  3:55                   ` Chris Marusich
  0 siblings, 1 reply; 42+ messages in thread
From: Chris Marusich @ 2016-11-04  3:34 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: guix-devel

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

Danny Milosavljevic <dannym@scratchpost.org> writes:

> Hi,
>
> On Thu, 03 Nov 2016 03:35:42 -0700
> Chris Marusich <cmmarusich@gmail.com> wrote:
>> Additional info: I've just now upgraded my x200 laptop to the latest
>> release of Libreboot (r20160907), and I can still boot into GuixSD
>> normally.
>
> Really? I have an X200 with libreboot and I had to manually create a
> symlink /boot/grub/libreboot_grub.cfg -> /boot/grub/grub.cfg -
> otherwise I'd require complicated incantations for each boot.
>
> Do you have such a symlink?

I do not:

--8<---------------cut here---------------start------------->8---
$ ls -l /boot/grub/
total 28
drwxr-xr-x 2 root root  4096 Mar 27  2016 fonts/
-r--r--r-- 1 root root  1877 Nov  3 00:03 grub.cfg
-rw-r--r-- 1 root root  1024 Mar 27  2016 grubenv
drwxr-xr-x 2 root root 12288 Nov  3 00:03 i386-pc/
drwxr-xr-x 2 root root  4096 Nov  3 00:03 locale/
--8<---------------cut here---------------end--------------->8---

I wonder if you are using a different version of Libreboot than me?  You
can check your version by following the instructions here:

https://libreboot.org/docs/index.html#version

Here is some other information which might help you figure out why my
Libreboot works without a libreboot_grub.cfg symlink, but yours doesn't:

* I use the GRUB payload (I presume you do, too).  Specifically, the ROM
I used was
libreboot_r20160907_grub_x200_8mb/x200_8mb_ukqwerty_vesafb.rom from the
release.

* My /boot/grub/grub.cfg file is on my root file system, which is ext4
and exists on the /dev/sda1 device.  I only have one disk in my laptop,
/dev/sda, and that's the one I installed GRUB onto (via 'guix system
reconfigure').  I encrypt my home partition (in my case, /dev/sda2) with
LUKS, but the rest is unencrypted.

I'm actually not sure what fancy tricks Libreboot (and its GRUB payload)
might be using to find the grub.cfg file.  But hopefully that
information helps.

-- 
Chris

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

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

* Re: [PATCH 01/10] * gnu/system.scm (<boot-parameters>): Add 'store-device' and 'store-fs-mount-point'.
  2016-11-04  3:34                 ` Chris Marusich
@ 2016-11-04  3:55                   ` Chris Marusich
  0 siblings, 0 replies; 42+ messages in thread
From: Chris Marusich @ 2016-11-04  3:55 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: guix-devel

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

Chris Marusich <cmmarusich@gmail.com> writes:

> I'm actually not sure what fancy tricks Libreboot (and its GRUB payload)
> might be using to find the grub.cfg file.  But hopefully that
> information helps.

Upon further investigation, it seems that Libreboot searches for
grub.cfg in addition to a few other configuration files [1].  So, if
you've got a recent version, you shouldn't need a libreboot_grub.cfg
symlink.

[1] https://notabug.org/vimuser/libreboot/src/master/resources/grub/config/menuentries/common.cfg

-- 
Chris

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

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

* Fix a boot problem reported by ng0
  2016-11-03  0:19         ` [PATCH 01/10] * gnu/system.scm (<boot-parameters>): Add 'store-device' and 'store-fs-mount-point' Leo Famulari
  2016-11-03  4:36           ` Chris Marusich
@ 2016-11-03 13:10           ` cmmarusich
  2016-11-03 13:10             ` [PATCH] system: Avoid using device paths in <menu-entry> device field cmmarusich
  1 sibling, 1 reply; 42+ messages in thread
From: cmmarusich @ 2016-11-03 13:10 UTC (permalink / raw)
  To: guix-devel

As you know, ng0 was having trouble booting into GuixSD after
1ef8b72a7f87afe7cffe52393d99e1b14e4770e1.  When I looked closely at the
change, I realized we were not doing the right thing in all cases.  Here is a
patch to fix that.

I've confirmed via manual testing in a QEMU image that when using a device
path for the file system containing the store, 'guix system reconfigure' does
the right thing: it uses a file-based GRUB root search in the new GRUB menu
entry.  That's what it originally did, but
1ef8b72a7f87afe7cffe52393d99e1b14e4770e1 broke that behavior.

I didn't catch this regression because our existing system tests did not catch
it, and in all my manual tests, I was using file system labels, not device
paths.

--
Chris

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

* [PATCH] system: Avoid using device paths in <menu-entry> device field.
  2016-11-03 13:10           ` Fix a boot problem reported by ng0 cmmarusich
@ 2016-11-03 13:10             ` cmmarusich
  2016-11-04 15:49               ` Leo Famulari
  2016-11-06 16:51               ` Ludovic Courtès
  0 siblings, 2 replies; 42+ messages in thread
From: cmmarusich @ 2016-11-03 13:10 UTC (permalink / raw)
  To: guix-devel

From: Chris Marusich <cmmarusich@gmail.com>

This fixes a regression introduced by
1ef8b72a7f87afe7cffe52393d99e1b14e4770e1, in which we would incorrectly use a
device path in a label-based grub root search command, e.g. 'search --label
--set /dev/sda4'.

* gnu/system.scm (grub-device): New procedure.
(operating-system-grub.cfg, operating-system-parameters-file): Use it.
(read-boot-parameters): Handle device paths correctly.
---
 gnu/system.scm | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index 259875d..bbda2c2 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -60,6 +60,7 @@
   #:use-module (srfi srfi-26)
   #:use-module (srfi srfi-34)
   #:use-module (srfi srfi-35)
+  #:use-module (rnrs bytevectors)
   #:export (operating-system
             operating-system?
 
@@ -738,7 +739,7 @@ listed in OS.  The C library expects to find it under
                            (label label)
 
                            ;; The device where the kernel and initrd live.
-                           (device (file-system-device store-fs))
+                           (device (grub-device store-fs))
                            (device-mount-point
                             (file-system-mount-point store-fs))
 
@@ -753,6 +754,14 @@ listed in OS.  The C library expects to find it under
     (grub-configuration-file (operating-system-bootloader os) entries
                              #:old-entries old-entries)))
 
+(define (grub-device fs)
+  "Given FS, a <file-system> object, return a value suitable for use as the
+device in a <menu-entry>."
+  (case (file-system-title fs)
+    ((uuid) (file-system-device fs))
+    ((label) (file-system-device fs))
+    (else #f)))
+
 (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."
@@ -771,10 +780,7 @@ this file is the reconstruction of GRUB menu entries for old configurations."
                     #$(operating-system-kernel-arguments os))
                    (initrd #$initrd)
                    (store
-                    (device #$(case (file-system-title store)
-                                ((uuid) (file-system-device store))
-                                ((label) (file-system-device store))
-                                (else #f)))
+                    (device #$(grub-device store))
                     (mount-point #$(file-system-mount-point store))))
                 #:set-load-path? #f)))
 
@@ -836,7 +842,10 @@ this file is the reconstruction of GRUB menu entries for old configurations."
          (('store ('device device) _ ...)
           device)
          (_                                       ;the old format
-          root)))
+          ;; Root might be a device path like "/dev/sda1".
+          (if (string-prefix? "/" root)
+              #f
+              root))))
 
       (store-mount-point
        (match (assq 'store rest)
-- 
2.10.2

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

* Re: [PATCH] system: Avoid using device paths in <menu-entry> device field.
  2016-11-03 13:10             ` [PATCH] system: Avoid using device paths in <menu-entry> device field cmmarusich
@ 2016-11-04 15:49               ` Leo Famulari
  2016-11-06 16:51               ` Ludovic Courtès
  1 sibling, 0 replies; 42+ messages in thread
From: Leo Famulari @ 2016-11-04 15:49 UTC (permalink / raw)
  To: cmmarusich; +Cc: guix-devel

On Thu, Nov 03, 2016 at 06:10:28AM -0700, cmmarusich@gmail.com wrote:
> From: Chris Marusich <cmmarusich@gmail.com>
> 
> This fixes a regression introduced by
> 1ef8b72a7f87afe7cffe52393d99e1b14e4770e1, in which we would incorrectly use a
> device path in a label-based grub root search command, e.g. 'search --label
> --set /dev/sda4'.
> 
> * gnu/system.scm (grub-device): New procedure.
> (operating-system-grub.cfg, operating-system-parameters-file): Use it.
> (read-boot-parameters): Handle device paths correctly.

I'm not able to test this patch, but I think we should resolve the
underlying issue before the next release.

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

* Re: [PATCH] system: Avoid using device paths in <menu-entry> device field.
  2016-11-03 13:10             ` [PATCH] system: Avoid using device paths in <menu-entry> device field cmmarusich
  2016-11-04 15:49               ` Leo Famulari
@ 2016-11-06 16:51               ` Ludovic Courtès
  1 sibling, 0 replies; 42+ messages in thread
From: Ludovic Courtès @ 2016-11-06 16:51 UTC (permalink / raw)
  To: cmmarusich; +Cc: guix-devel

cmmarusich@gmail.com skribis:

> From: Chris Marusich <cmmarusich@gmail.com>
>
> This fixes a regression introduced by
> 1ef8b72a7f87afe7cffe52393d99e1b14e4770e1, in which we would incorrectly use a
> device path in a label-based grub root search command, e.g. 'search --label
> --set /dev/sda4'.
>
> * gnu/system.scm (grub-device): New procedure.
> (operating-system-grub.cfg, operating-system-parameters-file): Use it.
> (read-boot-parameters): Handle device paths correctly.

Good catch.  Applied, thanks!

And sorry for the delay!

Ludo’.

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

* [PATCH 02/10] Add 'device' field to <menu-entry>
  2016-10-28 10:07 Add system roll-back and switch-generation commands cmmarusich
  2016-10-28 10:07 ` [PATCH 01/10] * gnu/system.scm (<boot-parameters>): Add 'store-device' and 'store-fs-mount-point' cmmarusich
@ 2016-10-28 10:07 ` cmmarusich
  2016-10-28 10:07 ` [PATCH 03/10] Refactor grub.cfg generation logic cmmarusich
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 42+ messages in thread
From: cmmarusich @ 2016-10-28 10:07 UTC (permalink / raw)
  To: guix-devel

From: Chris Marusich <cmmarusich@gmail.com>

* gnu/system/grub.scm (<menu-entry>): add field 'device'.

* gnu/system.scm (grub-configuration-file): use a dummy value for 'device'.

* guix/scripts/system.scm (previous-grub-entries): use a dummy value for
  'device'.

In a future commit, we will restructure the grub.cfg generation logic to use
this information, to enable the implementation of 'guix system
switch-generation' and 'guix system roll-back'.
---
 gnu/system.scm          | 1 +
 gnu/system/grub.scm     | 4 +++-
 guix/scripts/system.scm | 2 ++
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index ff84b63..f9f63a0 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -736,6 +736,7 @@ listed in OS.  The C library expects to find it under
                            (file-system-device root-fs)))
        (entries ->  (list (menu-entry
                            (label label)
+                           (device #f) ;; stub value, not used yet
                            (linux kernel)
                            (linux-arguments
                             (cons* (string-append "--root=" root-device)
diff --git a/gnu/system/grub.scm b/gnu/system/grub.scm
index 249b415..859f33a 100644
--- a/gnu/system/grub.scm
+++ b/gnu/system/grub.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2013, 2014, 2015, 2016 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2016 Chris Marusich <cmmarusich@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -121,6 +122,7 @@ object denoting a file name."
   menu-entry make-menu-entry
   menu-entry?
   (label           menu-entry-label)
+  (device          menu-entry-device)      ; file system uuid, label, or #f
   (linux           menu-entry-linux)
   (linux-arguments menu-entry-linux-arguments
                    (default '()))          ; list of string-valued gexps
@@ -262,7 +264,7 @@ corresponding to old generations of the system."
 
   (define entry->gexp
     (match-lambda
-     (($ <menu-entry> label linux arguments initrd)
+     (($ <menu-entry> label device linux arguments initrd)
       ;; Use the right file names for LINUX and STORE-FS in case STORE-FS is
       ;; not the "/" file system.
       (let ((linux  (strip-mount-point store-fs linux))
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 0519ab8..8f0b8cd 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2014, 2015, 2016 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2016 Alex Kost <alezost@gmail.com>
+;;; Copyright © 2016 Chris Marusich <cmmarusich@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -384,6 +385,7 @@ it atomically, and then run OS's activation script."
         (label (string-append label " (#"
                               (number->string number) ", "
                               (seconds->string time) ")"))
+        (device #f) ; stub value, not used yet
         (linux kernel)
         (linux-arguments
          (cons* (string-append "--root=" root-device)
-- 
2.9.2

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

* [PATCH 03/10] Refactor grub.cfg generation logic
  2016-10-28 10:07 Add system roll-back and switch-generation commands cmmarusich
  2016-10-28 10:07 ` [PATCH 01/10] * gnu/system.scm (<boot-parameters>): Add 'store-device' and 'store-fs-mount-point' cmmarusich
  2016-10-28 10:07 ` [PATCH 02/10] Add 'device' field to <menu-entry> cmmarusich
@ 2016-10-28 10:07 ` cmmarusich
  2016-10-28 10:07 ` [PATCH 04/10] Extract procedure: relative-generation-spec->number cmmarusich
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 42+ messages in thread
From: cmmarusich @ 2016-10-28 10:07 UTC (permalink / raw)
  To: guix-devel

From: Chris Marusich <cmmarusich@gmail.com>

This enables the implementation of 'guix system switch-generation' and 'guix
system roll-back'.  Those new commands will only be able to determine the
store device and mount point for a given system generation by reading them
from that generation's boot parameters file, which does not contain a
<file-system> object.  This change makes it possible for those commands
regenerate grub.cfg using that information.

* gnu/system.scm (operating-system-grub.cfg): Instead of passing store-fs
  directly as a parameter to grub-configuration-file, pass in its mount point
  and (basically) its device.

* gnu/system/grub.scm (strip-mount-point, eye-candy, grub-root-search,
  grub-configuration-file, previous-grub-entries): Refactor these procedures
  to take a mount point and/or (basically) a device as parameters instead of a
  full-fledged <file-system> object.
---
 gnu/system.scm          | 10 ++++-
 gnu/system/grub.scm     | 98 ++++++++++++++++++++++++++-----------------------
 guix/scripts/system.scm |  3 +-
 3 files changed, 63 insertions(+), 48 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index f9f63a0..0d8c25a 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -728,6 +728,10 @@ listed in OS.  The C library expects to find it under
       ((system      (operating-system-derivation os))
        (root-fs ->  (operating-system-root-file-system os))
        (store-fs -> (operating-system-store-file-system os))
+       (grub-root-search-device -> (case (file-system-title store-fs)
+                                         ((uuid) (file-system-device store-fs))
+                                         ((label) (file-system-device store-fs))
+                                         (else #f)))
        (label ->    (kernel->grub-label (operating-system-kernel os)))
        (kernel ->   (operating-system-kernel-file os))
        (initrd      (operating-system-initrd-file os))
@@ -736,7 +740,7 @@ listed in OS.  The C library expects to find it under
                            (file-system-device root-fs)))
        (entries ->  (list (menu-entry
                            (label label)
-                           (device #f) ;; stub value, not used yet
+                           (device grub-root-search-device)
                            (linux kernel)
                            (linux-arguments
                             (cons* (string-append "--root=" root-device)
@@ -746,7 +750,9 @@ listed in OS.  The C library expects to find it under
                                    (operating-system-kernel-arguments os)))
                            (initrd initrd)))))
     (grub-configuration-file (operating-system-bootloader os)
-                             store-fs entries
+                             (file-system-mount-point store-fs)
+                             grub-root-search-device
+                             entries
                              #:old-entries old-entries)))
 
 (define (operating-system-parameters-file os)
diff --git a/gnu/system/grub.scm b/gnu/system/grub.scm
index 859f33a..d45fdca 100644
--- a/gnu/system/grub.scm
+++ b/gnu/system/grub.scm
@@ -62,16 +62,15 @@
 ;;;
 ;;; Code:
 
-(define (strip-mount-point fs file)
-  "Strip the mount point of FS from FILE, which is a gexp or other lowerable
+(define (strip-mount-point mount-point file)
+  "Strip MOUNT-POINT from FILE, which is a gexp or other lowerable
 object denoting a file name."
-  (let ((mount-point (file-system-mount-point fs)))
-    (if (string=? mount-point "/")
-	file
-	#~(let ((file #$file))
-            (if (string-prefix? #$mount-point file)
-                (substring #$file #$(string-length mount-point))
-                file)))))
+  (if (string=? mount-point "/")
+      file
+      #~(let ((file #$file))
+          (if (string-prefix? #$mount-point file)
+              (substring #$file #$(string-length mount-point))
+              file))))
 
 (define-record-type* <grub-image>
   grub-image make-grub-image
@@ -164,12 +163,15 @@ WIDTH/HEIGHT, or #f if none was found."
         (with-monad %store-monad
           (return #f)))))
 
-(define (eye-candy config root-fs system port)
-  "Return in %STORE-MONAD a gexp that writes to PORT (a port-valued gexp) the
-'grub.cfg' part concerned with graphics mode, background images, colors, and
-all that.  ROOT-FS is a file-system object denoting the root file system where
-the store is.  SYSTEM must be the target system string---e.g.,
-\"x86_64-linux\"."
+(define (eye-candy config store-fs-mount-point store-device system port)
+  "Return a gexp that writes to PORT (a port-valued gexp) the 'grub.cfg' part
+concerned with graphics mode, background images, colors, and all that.
+STORE-FS-MOUNT-POINT is the mount point of the file system containing the
+store.  STORE-DEVICE is a file system UUID, a file system label, or #f.  The
+value of STORE-DEVICE determines the GRUB search command that will be used to
+find and set the GRUB root; for details, please refer to the
+'grub-root-search' procedure's docstring.  SYSTEM must be the target system
+string---e.g., \"x86_64-linux\"."
   (define setup-gfxterm-body
     ;; Intel systems need to be switched into graphics mode, whereas most
     ;; other modern architectures have no other mode and therefore don't need
@@ -193,7 +195,7 @@ the store is.  SYSTEM must be the target system string---e.g.,
                      (symbol->string (assoc-ref colors 'bg)))))
 
   (define font-file
-    (strip-mount-point root-fs
+    (strip-mount-point store-fs-mount-point
                        (file-append grub "/share/grub/unicode.pf2")))
 
   (mlet* %store-monad ((image (grub-background-image config)))
@@ -201,7 +203,7 @@ the store is.  SYSTEM must be the target system string---e.g.,
                  #~(format #$port "
 function setup_gfxterm {~a}
 
-# Set 'root' to the partition that contains /gnu/store.
+# Set GRUB's 'root' to the device that contains the store.
 ~a
 
 if loadfont ~a; then
@@ -217,10 +219,10 @@ else
   set menu_color_highlight=white/blue
 fi~%"
                            #$setup-gfxterm-body
-                           #$(grub-root-search root-fs font-file)
+                           #$(grub-root-search store-device font-file)
                            #$font-file
 
-                           #$(strip-mount-point root-fs image)
+                           #$(strip-mount-point store-fs-mount-point image)
                            #$(theme-colors grub-theme-color-normal)
                            #$(theme-colors grub-theme-color-highlight))))))
 
@@ -229,57 +231,63 @@ fi~%"
 ;;; Configuration file.
 ;;;
 
-(define (grub-root-search root-fs file)
-  "Return the GRUB 'search' command to look for ROOT-FS, which contains FILE,
-a gexp.  The result is a gexp that can be inserted in the grub.cfg-generation
+(define (grub-root-search device file)
+  "Return a GRUB 'search' command (@pxref{search,,, grub, GNU GRUB manual})
+which will find the device indicated by DEVICE and which will set GRUB's
+'root' to it (@pxref{root,,, grub, GNU GRUB manual}).  DEVICE may be a file
+system UUID or label, in which case the search command will find the device
+containing the specified file system and set the root to it, or it may be #f,
+in which case the search command will find the device containing the specified
+FILE.  The result is a gexp that can be inserted into grub.cfg-generation
 code."
   ;; Usually FILE is a file name gexp like "/gnu/store/…-linux/vmlinuz", but
   ;; it can also be something like "(hd0,msdos1)/vmlinuz" in the case of
   ;; custom menu entries.  In the latter case, don't emit a 'search' command.
-  (if (and (string? file) (not (string-prefix? "/" file)))
-      ""
-      (case (file-system-title root-fs)
-        ;; Preferably refer to ROOT-FS by its UUID or label.  This is more
-        ;; efficient and less ambiguous, see <>.
-        ((uuid)
-         (format #f "search --fs-uuid --set ~a"
-                 (uuid->string (file-system-device root-fs))))
-        ((label)
-         (format #f "search --label --set ~a"
-                 (file-system-device root-fs)))
+  (cond ((and (string? file) (not (string-prefix? "/" file)))
+         "")
+        ((not device)
+         #~(format #f "search --file --set ~a" #$file))
+        ((string? device)
+         (format #f "search --label --set ~a" device))
         (else
-         ;; As a last resort, look for any device containing FILE.
-         #~(format #f "search --file --set ~a" #$file)))))
+         (format #f "search --fs-uuid --set ~a" (uuid->string device)))))
 
-(define* (grub-configuration-file config store-fs entries
+(define* (grub-configuration-file config
+                                  store-fs-mount-point
+                                  store-device
+                                  entries
                                   #:key
                                   (system (%current-system))
                                   (old-entries '()))
   "Return the GRUB configuration file corresponding to CONFIG, a
-<grub-configuration> object, and where the store is available at STORE-FS, a
-<file-system> object.  OLD-ENTRIES is taken to be a list of menu entries
-corresponding to old generations of the system."
+<grub-configuration> object.  STORE-FS-MOUNT-POINT is the mount point of the
+file system containing the store.  STORE-DEVICE is a file system UUID, a file
+system label, or #f.  The value of STORE-DEVICE determines the GRUB search
+command that will be used to find and set the GRUB root; for details, please
+refer to the 'grub-root-search' procedure's docstring.  OLD-ENTRIES is taken
+to be a list of menu entries corresponding to old generations of the system."
   (define all-entries
     (append entries (grub-configuration-menu-entries config)))
 
   (define entry->gexp
     (match-lambda
      (($ <menu-entry> label device linux arguments initrd)
-      ;; Use the right file names for LINUX and STORE-FS in case STORE-FS is
-      ;; not the "/" file system.
-      (let ((linux  (strip-mount-point store-fs linux))
-            (initrd (strip-mount-point store-fs initrd)))
+      ;; Use the right paths in case the file system containing the store is
+      ;; not mounted at "/".
+      (let ((linux  (strip-mount-point store-fs-mount-point linux))
+            (initrd (strip-mount-point store-fs-mount-point initrd)))
         #~(format port "menuentry ~s {
   ~a
   linux ~a ~a
   initrd ~a
 }~%"
                   #$label
-                  #$(grub-root-search store-fs linux)
+                  #$(grub-root-search device linux)
                   #$linux (string-join (list #$@arguments))
                   #$initrd)))))
 
-  (mlet %store-monad ((sugar (eye-candy config store-fs system #~port)))
+  (mlet %store-monad
+      ((sugar (eye-candy config store-fs-mount-point store-device system #~port)))
     (define builder
       #~(call-with-output-file #$output
           (lambda (port)
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 8f0b8cd..4edaa0f 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -375,6 +375,7 @@ it atomically, and then run OS's activation script."
                                 read-boot-parameters))
             (label            (boot-parameters-label params))
             (root             (boot-parameters-root-device params))
+            (store            (boot-parameters-store-device params))
             (root-device      (if (bytevector? root)
                                   (uuid->string root)
                                   root))
@@ -385,7 +386,7 @@ it atomically, and then run OS's activation script."
         (label (string-append label " (#"
                               (number->string number) ", "
                               (seconds->string time) ")"))
-        (device #f) ; stub value, not used yet
+        (device store)
         (linux kernel)
         (linux-arguments
          (cons* (string-append "--root=" root-device)
-- 
2.9.2

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

* [PATCH 04/10] Extract procedure: relative-generation-spec->number
  2016-10-28 10:07 Add system roll-back and switch-generation commands cmmarusich
                   ` (2 preceding siblings ...)
  2016-10-28 10:07 ` [PATCH 03/10] Refactor grub.cfg generation logic cmmarusich
@ 2016-10-28 10:07 ` cmmarusich
  2016-10-28 10:07 ` [PATCH 05/10] Rename previous-grub-entries to grub-entries cmmarusich
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 42+ messages in thread
From: cmmarusich @ 2016-10-28 10:07 UTC (permalink / raw)
  To: guix-devel

From: Chris Marusich <cmmarusich@gmail.com>

* guix/scripts/package.scm (switch-generation-action): Extract code from
  here...

* guix/profiles.scm (relative-generation-spec->number): into this new
  procedure here.
---
 guix/profiles.scm        | 17 +++++++++++++++++
 guix/scripts/package.scm |  8 ++------
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/guix/profiles.scm b/guix/profiles.scm
index e7319a8..6d707b1 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -5,6 +5,7 @@
 ;;; Copyright © 2015 Mark H Weaver <mhw@netris.org>
 ;;; Copyright © 2015 Sou Bunnbu <iyzsong@gmail.com>
 ;;; Copyright © 2016 Ricardo Wurmus <rekado@elephly.net>
+;;; Copyright © 2016 Chris Marusich <cmmarusich@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -97,6 +98,7 @@
             generation-number
             generation-numbers
             profile-generations
+            relative-generation-spec->number
             relative-generation
             previous-generation-number
             generation-time
@@ -1039,6 +1041,21 @@ former profiles were found."
         '()
         generations)))
 
+(define (relative-generation-spec->number profile spec)
+  "Return PROFILE's generation specified by SPEC, which is a string.  The SPEC
+may be a N, -N, or +N, where N is a number.  If the spec is N, then the number
+returned is N.  If it is -N, then the number returned is the profile's current
+generation number minus N.  If it is +N, then the number returned is the
+profile's current generation number plus N.  Return #f if there is no such
+generation."
+  (let ((number (string->number spec)))
+    (and number
+         (case (string-ref spec 0)
+           ((#\+ #\-)
+            (relative-generation profile number))
+           (else number)))))
+
+
 (define* (relative-generation profile shift #:optional
                               (current (generation-number profile)))
   "Return PROFILE's generation shifted from the CURRENT generation by SHIFT.
diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index 70ed0a7..96a22f6 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -5,6 +5,7 @@
 ;;; Copyright © 2014, 2016 Alex Kost <alezost@gmail.com>
 ;;; Copyright © 2016 Roel Janssen <roel@gnu.org>
 ;;; Copyright © 2016 Benz Schenk <benz.schenk@uzh.ch>
+;;; Copyright © 2016 Chris Marusich <cmmarusich@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -782,12 +783,7 @@ processed, #f otherwise."
                                    #:key dry-run?)
   "Switch PROFILE to the generation specified by SPEC."
   (unless dry-run?
-    (let* ((number (string->number spec))
-           (number (and number
-                        (case (string-ref spec 0)
-                          ((#\+ #\-)
-                           (relative-generation profile number))
-                          (else number)))))
+    (let ((number (relative-generation-spec->number profile spec)))
       (if number
           (switch-to-generation* profile number)
           (leave (_ "cannot switch to generation '~a'~%") spec)))))
-- 
2.9.2

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

* [PATCH 05/10] Rename previous-grub-entries to grub-entries
  2016-10-28 10:07 Add system roll-back and switch-generation commands cmmarusich
                   ` (3 preceding siblings ...)
  2016-10-28 10:07 ` [PATCH 04/10] Extract procedure: relative-generation-spec->number cmmarusich
@ 2016-10-28 10:07 ` cmmarusich
  2016-10-28 10:07 ` [PATCH 06/10] grub-entries: take a list of numbers on input cmmarusich
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 42+ messages in thread
From: cmmarusich @ 2016-10-28 10:07 UTC (permalink / raw)
  To: guix-devel

From: Chris Marusich <cmmarusich@gmail.com>

* guix/scripts/system.scm (previous-grub-entries): rename this procedure to
  grub-entries.  Update all callers.

This procedure actually returns an entry for every generation of the profile,
so its name is confusing if it suggests that it only returns "previous"
entries.
---
 guix/scripts/system.scm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 4edaa0f..a4f5c7b 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -366,7 +366,7 @@ it atomically, and then run OS's activation script."
     (date->string (time-utc->date time)
                   "~Y-~m-~d ~H:~M")))
 
-(define* (previous-grub-entries #:optional (profile %system-profile))
+(define* (grub-entries #:optional (profile %system-profile))
   "Return a list of 'menu-entry' for the generations of PROFILE."
   (define (system->grub-entry system number time)
     (unless-file-not-found
@@ -563,7 +563,7 @@ building anything."
                       (operating-system-grub.cfg os
                                                  (if (eq? 'init action)
                                                      '()
-                                                     (previous-grub-entries)))))
+                                                     (grub-entries)))))
 
        ;; 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
-- 
2.9.2

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

* [PATCH 06/10] grub-entries: take a list of numbers on input
  2016-10-28 10:07 Add system roll-back and switch-generation commands cmmarusich
                   ` (4 preceding siblings ...)
  2016-10-28 10:07 ` [PATCH 05/10] Rename previous-grub-entries to grub-entries cmmarusich
@ 2016-10-28 10:07 ` cmmarusich
  2016-10-28 10:07 ` [PATCH 07/10] Factor out procedure: install-grub-config cmmarusich
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 42+ messages in thread
From: cmmarusich @ 2016-10-28 10:07 UTC (permalink / raw)
  To: guix-devel

From: Chris Marusich <cmmarusich@gmail.com>

* guix/scripts/system.scm (grub-entries): Add an optional parameter which
  allows the caller to limit the grub entries that get returned.
---
 guix/scripts/system.scm | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index a4f5c7b..2c80c2e 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -366,8 +366,10 @@ it atomically, and then run OS's activation script."
     (date->string (time-utc->date time)
                   "~Y-~m-~d ~H:~M")))
 
-(define* (grub-entries #:optional (profile %system-profile))
-  "Return a list of 'menu-entry' for the generations of PROFILE."
+(define* (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"))
@@ -395,8 +397,7 @@ it atomically, and then run OS's activation script."
                 kernel-arguments))
         (initrd initrd)))))
 
-  (let* ((numbers (generation-numbers profile))
-         (systems (map (cut generation-file-name profile <>)
+  (let* ((systems (map (cut generation-file-name profile <>)
                        numbers))
          (times   (map (lambda (system)
                          (unless-file-not-found
-- 
2.9.2

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

* [PATCH 07/10] Factor out procedure: install-grub-config
  2016-10-28 10:07 Add system roll-back and switch-generation commands cmmarusich
                   ` (5 preceding siblings ...)
  2016-10-28 10:07 ` [PATCH 06/10] grub-entries: take a list of numbers on input cmmarusich
@ 2016-10-28 10:07 ` cmmarusich
  2016-10-28 10:07 ` [PATCH 08/10] Implement switch-generation and roll-back cmmarusich
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 42+ messages in thread
From: cmmarusich @ 2016-10-28 10:07 UTC (permalink / raw)
  To: guix-devel

From: Chris Marusich <cmmarusich@gmail.com>

* gnu/build/install (install-grub, install-grub-config): Extract code from
  'install-grub' into a new procedure 'install-grub-config'.  Also, define
  'install-grub' using define, not define*, since it makes no use of the extra
  features that define* provides.
---
 gnu/build/install.scm | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/gnu/build/install.scm b/gnu/build/install.scm
index 7431a09..3d1594e 100644
--- a/gnu/build/install.scm
+++ b/gnu/build/install.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2013, 2014, 2015, 2016 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2016 Chris Marusich <cmmarusich@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -22,6 +23,7 @@
   #:use-module (srfi srfi-26)
   #:use-module (ice-9 match)
   #:export (install-grub
+            install-grub-config
             populate-root-file-system
             reset-timestamps
             register-closure
@@ -36,13 +38,24 @@
 ;;;
 ;;; Code:
 
-(define* (install-grub grub.cfg device mount-point)
+(define (install-grub grub.cfg device mount-point)
   "Install GRUB with GRUB.CFG on DEVICE, which is assumed to be mounted on
 MOUNT-POINT.
 
 Note that the caller must make sure that GRUB.CFG is registered as a GC root
 so that the fonts, background images, etc. referred to by GRUB.CFG are not
 GC'd."
+  (install-grub-config grub.cfg mount-point)
+  (unless (zero? (system* "grub-install" "--no-floppy"
+                          "--boot-directory"
+                          (string-append mount-point "/boot")
+                          device))
+    (error "failed to install GRUB")))
+
+(define (install-grub-config grub.cfg mount-point)
+  "Atomically copy GRUB.CFG into boot/grub/grub.cfg on the MOUNT-POINT.  Note
+that the caller must make sure that GRUB.CFG is registered as a GC root so
+that the fonts, background images, etc. referred to by GRUB.CFG are not GC'd."
   (let* ((target (string-append mount-point "/boot/grub/grub.cfg"))
          (pivot  (string-append target ".new")))
     (mkdir-p (dirname target))
@@ -50,13 +63,7 @@ GC'd."
     ;; Copy GRUB.CFG instead of just symlinking it, because symlinks won't
     ;; work when /boot is on a separate partition.  Do that atomically.
     (copy-file grub.cfg pivot)
-    (rename-file pivot target)
-
-    (unless (zero? (system* "grub-install" "--no-floppy"
-                            "--boot-directory"
-                            (string-append mount-point "/boot")
-                            device))
-      (error "failed to install GRUB"))))
+    (rename-file pivot target)))
 
 (define (evaluate-populate-directive directive target)
   "Evaluate DIRECTIVE, an sexp describing a file or directory to create under
-- 
2.9.2

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

* [PATCH 08/10] Implement switch-generation and roll-back
  2016-10-28 10:07 Add system roll-back and switch-generation commands cmmarusich
                   ` (6 preceding siblings ...)
  2016-10-28 10:07 ` [PATCH 07/10] Factor out procedure: install-grub-config cmmarusich
@ 2016-10-28 10:07 ` cmmarusich
  2016-10-28 10:07 ` [PATCH 09/10] Rename grub-entries to profile-grub-entries cmmarusich
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 42+ messages in thread
From: cmmarusich @ 2016-10-28 10:07 UTC (permalink / raw)
  To: guix-devel

From: Chris Marusich <cmmarusich@gmail.com>

* guix/scripts/system.scm (roll-back-system, switch-to-system-generation,
  reinstall-grub): New procedures.

* guix/scripts/system.scm (show-help, process-command, guix-system): Add
  references to the new actions.
---
 guix/scripts/system.scm | 97 ++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 91 insertions(+), 6 deletions(-)

diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 2c80c2e..086b431 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -407,6 +407,67 @@ NUMBERS, which is a list of generation numbers."
 
 \f
 ;;;
+;;; Roll-back.
+;;;
+(define (roll-back-system store)
+  "Roll back the system profile to its previous generation."
+  (switch-to-system-generation store "-1"))
+\f
+;;;
+;;; Switch generations.
+;;;
+(define (switch-to-system-generation store spec)
+  "Switch the system profile to the generation specified by SPEC, and
+re-install grub with a grub configuration file that uses the specified system
+generation as its default entry."
+  (let ((number (relative-generation-spec->number %system-profile spec)))
+    (if number
+        (begin
+          (reinstall-grub store number)
+          (switch-to-generation* %system-profile number))
+        (leave (_ "cannot switch to system generation '~a'~%") spec))))
+
+(define (reinstall-grub store number)
+  "Re-install grub for existing system profile generation NUMBER."
+  (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)))
+         (root-device (boot-parameters-root-device params))
+         (store-mount-point (boot-parameters-store-fs-mount-point params))
+         (grub-root-search-device (boot-parameters-store-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 (grub-entries %system-profile (list number)))
+         (old-generations (delv number (generation-numbers %system-profile)))
+         (old-entries (grub-entries %system-profile old-generations))
+         (grub.cfg (run-with-store store
+                     (grub-configuration-file grub-config
+                                              store-mount-point
+                                              grub-root-search-device
+                                              entries
+                                              #:old-entries old-entries))))
+    (show-what-to-build store (list grub.cfg))
+    (build-derivations store (list grub.cfg))
+    ;; This is basically the same as install-grub*, but for now we avoid
+    ;; re-installing the GRUB boot loader itself onto a device, mainly because
+    ;; we don't in general have access to the same version of the GRUB package
+    ;; which was used when installing this other system generation.
+    (let* ((grub.cfg-path (derivation->output-path grub.cfg))
+           (gc-root (string-append %gc-roots-directory "/grub.cfg"))
+           (temp-gc-root (string-append gc-root ".new")))
+      (switch-symlinks temp-gc-root grub.cfg-path)
+      (unless (false-if-exception (install-grub-config grub.cfg-path "/"))
+        (delete-file temp-gc-root)
+        (leave (_ "failed to re-install GRUB configuration file: '~a'~%")
+               grub.cfg-path))
+      (rename-file temp-gc-root gc-root))))
+
+\f
+;;;
 ;;; Graphs.
 ;;;
 
@@ -641,14 +702,19 @@ building anything."
 ;;;
 
 (define (show-help)
-  (display (_ "Usage: guix system [OPTION] ACTION [FILE]
-Build the operating system declared in FILE according to ACTION.\n"))
+  (display (_ "Usage: guix system [OPTION ...] ACTION [ARG ...] [FILE]
+Build the operating system declared in FILE according to ACTION.
+Some ACTIONS support additional ARGS.\n"))
   (newline)
   (display (_ "The valid values for ACTION are:\n"))
   (newline)
   (display (_ "\
    reconfigure      switch to a new operating system configuration\n"))
   (display (_ "\
+   roll-back        switch to the previous operating system configuration\n"))
+  (display (_ "\
+   switch-generation switch to an existing operating system configuration\n"))
+  (display (_ "\
    list-generations list the system generations\n"))
   (display (_ "\
    build            build the operating system without installing anything\n"))
@@ -809,15 +875,33 @@ resulting from command-line parsing."
   "Process COMMAND, one of the 'guix system' sub-commands.  ARGS is its
 argument list and OPTS is the option alist."
   (case command
+    ;; The following commands do not need to use the store, and they do not need
+    ;; an operating system configuration file.
     ((list-generations)
-     ;; List generations.  No need to connect to the daemon, etc.
      (let ((pattern (match args
                       (() "")
                       ((pattern) pattern)
                       (x (leave (_ "wrong number of arguments~%"))))))
        (list-generations pattern)))
-    (else
-     (process-action command args opts))))
+    ;; The following commands need to use the store, but they do not need an
+    ;; operating system configuration file.
+    ((switch-generation)
+     (let ((pattern (match args
+                      ((pattern) pattern)
+                      (x (leave (_ "wrong number of arguments~%"))))))
+       (with-store store
+         (set-build-options-from-command-line store opts)
+         (switch-to-system-generation store pattern))))
+    ((roll-back)
+     (let ((pattern (match args
+                      (() "")
+                      (x (leave (_ "wrong number of arguments~%"))))))
+       (with-store store
+         (set-build-options-from-command-line store opts)
+         (roll-back-system store))))
+    ;; The following commands need to use the store, and they also
+    ;; need an operating system configuration file.
+    (else (process-action command args opts))))
 
 (define (guix-system . args)
   (define (parse-sub-command arg result)
@@ -827,7 +911,8 @@ argument list and OPTS is the option alist."
         (let ((action (string->symbol arg)))
           (case action
             ((build container vm vm-image disk-image reconfigure init
-              extension-graph shepherd-graph list-generations)
+              extension-graph shepherd-graph list-generations roll-back
+              switch-generation)
              (alist-cons 'action action result))
             (else (leave (_ "~a: unknown action~%") action))))))
 
-- 
2.9.2

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

* [PATCH 09/10] Rename grub-entries to profile-grub-entries
  2016-10-28 10:07 Add system roll-back and switch-generation commands cmmarusich
                   ` (7 preceding siblings ...)
  2016-10-28 10:07 ` [PATCH 08/10] Implement switch-generation and roll-back cmmarusich
@ 2016-10-28 10:07 ` cmmarusich
  2016-10-28 10:07 ` [PATCH 10/10] Mention new 'guix system' features in the manual cmmarusich
  2016-10-29 21:13 ` Add system roll-back and switch-generation commands Ludovic Courtès
  10 siblings, 0 replies; 42+ messages in thread
From: cmmarusich @ 2016-10-28 10:07 UTC (permalink / raw)
  To: guix-devel

From: Chris Marusich <cmmarusich@gmail.com>

* guix/scripts/system.scm (grub-entries): Rename this procedure to
  'profile-grub-entries'.  Update all callers.
---
 guix/scripts/system.scm | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 086b431..2ee145a 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -366,7 +366,7 @@ it atomically, and then run OS's activation script."
     (date->string (time-utc->date time)
                   "~Y-~m-~d ~H:~M")))
 
-(define* (grub-entries #:optional (profile %system-profile)
+(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."
@@ -441,9 +441,9 @@ generation as its default entry."
          ;; 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 (grub-entries %system-profile (list number)))
+         (entries (profile-grub-entries %system-profile (list number)))
          (old-generations (delv number (generation-numbers %system-profile)))
-         (old-entries (grub-entries %system-profile old-generations))
+         (old-entries (profile-grub-entries %system-profile old-generations))
          (grub.cfg (run-with-store store
                      (grub-configuration-file grub-config
                                               store-mount-point
@@ -625,7 +625,7 @@ building anything."
                       (operating-system-grub.cfg os
                                                  (if (eq? 'init action)
                                                      '()
-                                                     (grub-entries)))))
+                                                     (profile-grub-entries)))))
 
        ;; 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
-- 
2.9.2

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

* [PATCH 10/10] Mention new 'guix system' features in the manual
  2016-10-28 10:07 Add system roll-back and switch-generation commands cmmarusich
                   ` (8 preceding siblings ...)
  2016-10-28 10:07 ` [PATCH 09/10] Rename grub-entries to profile-grub-entries cmmarusich
@ 2016-10-28 10:07 ` cmmarusich
  2016-10-29 21:13 ` Add system roll-back and switch-generation commands Ludovic Courtès
  10 siblings, 0 replies; 42+ messages in thread
From: cmmarusich @ 2016-10-28 10:07 UTC (permalink / raw)
  To: guix-devel

From: Chris Marusich <cmmarusich@gmail.com>

* doc/guix.texi (GRUB Configuration, Invoking guix system): Mention the new
  <menu-entry> field 'device', update the custom <menu-entry> example, and
  mention the new actions 'guix system roll-back' and 'guix system
  switch-generation' in the manual.

* gnu/system/grub.scm (strip-mount-point): Add a TODO comment.

* guix/scripts/system.scm (show-help): Point users to the manual for details.
---
 doc/guix.texi           | 90 +++++++++++++++++++++++++++++++++++++++++++------
 gnu/system/grub.scm     |  4 +++
 guix/scripts/system.scm |  2 +-
 3 files changed, 85 insertions(+), 11 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 1293b8b..26fb930 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -11064,35 +11064,66 @@ along these lines:
 @example
 (menu-entry
   (label "The Other Distro")
-  (linux "/boot/old/vmlinux-2.6.32")
+  (device #f)
+  (linux "(hd0,msdos2)/boot/old/vmlinux-2.6.32")
   (linux-arguments '("root=/dev/sda2"))
-  (initrd "/boot/old/initrd"))
+  (initrd "(hd0,msdos2)/boot/old/initrd"))
 @end example
 
 Details below.
 
 @deftp {Data Type} menu-entry
-The type of an entry in the GRUB boot menu.
+The type of an entry in the GRUB boot menu.  Below, the phrase
+``string-valued G-Expression'' means a string or any object which can be
+inserted (via @code{ungexp}) as a string in a G-Expression
+(@pxref{G-Expressions}).
 
 @table @asis
 
 @item @code{label}
-The label to show in the menu---e.g., @code{"GNU"}.
+The label to show in the menu---e.g., @code{"GNU"}.  This is a string-valued G-Expression.
 
 @item @code{linux}
-The Linux kernel image to boot, for example:
+The Linux kernel image to boot.  This is a string-valued G-Expression.
+For example:
 
 @example
 (file-append linux-libre "/bzImage")
 @end example
 
+It is also possible to specify a device explicitly in the file path
+using GRUB's device naming convention (@pxref{Naming convention,,, grub,
+GNU GRUB manual}), for example:
+
+@example
+"(hd0,msdos1)/bzImage"
+@end example
+
+If the device is specified explicitly as above, then the @code{device}
+field is ignored entirely.
+
+@item @code{device}
+The device to use as GRUB's root for this menu entry (@pxref{root,,,
+grub, GNU GRUB manual}).  This may be a file system label (a string), a
+file system UUID (@pxref{File Systems}), or @code{#f}, in which case
+GRUB will automatically find the device containing the file specified by
+the @code{linux} field via a file-based search (@pxref{search,,, grub,
+GNU GRUB manual}).
+
+@c For details, see (guix gnu system).
+Currently, the @code{device} field is not suitable for use in custom
+menu entries.  Therefore, if you need to specify a custom menu entry,
+for now it is best to just pass a dummy value to it (such as @code{#f})
+and specify the device explicitly using GRUB's device naming convention,
+as in the example above.
+
 @item @code{linux-arguments} (default: @code{()})
 The list of extra Linux kernel command-line arguments---e.g.,
-@code{("console=ttyS0")}.
+@code{("console=ttyS0")}.  This is a list of string-valued
+G-Expressions.
 
 @item @code{initrd}
-A G-Expression or string denoting the file name of the initial RAM disk
-to use (@pxref{G-Expressions}).
+The initial RAM disk file to use.  This is a string-valued G-Expression.
 
 @end table
 @end deftp
@@ -11126,8 +11157,9 @@ supported:
 @table @code
 @item reconfigure
 Build the operating system described in @var{file}, activate it, and
-switch to it@footnote{This action is usable only on systems already
-running GuixSD.}.
+switch to it@footnote{This action and the related actions
+@code{switch-generation} and @code{roll-back}) are usable only on
+systems already running GuixSD.}.
 
 This effects all the configuration specified in @var{file}: user
 accounts, system services, global package list, setuid programs, etc.
@@ -11149,6 +11181,44 @@ guix pull}).  Failing to do that you would see an older version of Guix
 once @command{reconfigure} has completed.
 @end quotation
 
+@item switch-generation
+Switch to an existing system generation.  This action rearranges the
+existing GRUB menu entries.  It makes the menu entry for the target
+system generation the default, and it moves the entries for the other
+system generations to a submenu.  The next time the system boots, it
+will use the specified system generation.
+
+The target generation can be specified explicitly by its generation
+number.  For example, the following invocation would switch to system
+generation 7:
+
+@example
+guix system switch-generation 7
+@end example
+
+The target generation can also be specified relative to the current
+generation with the form @code{+N} or @code{-N}, where @code{+3} means
+``3 generations ahead of the current generation,'' and @code{-1} means
+``1 generation prior to the current generation.''
+
+Currently, the effect of invoking this action is only to rearrange the
+GRUB menu entries.  Therefore, to actually start using the target system
+generation, you must reboot after running this action.  In the future,
+it will be updated to do the same things as @command{reconfigure}, like
+activating and deactivating services.
+
+This action will fail if the specified generation does not exist.
+
+@item roll-back
+Switch to the preceding system generation.  The next time the system
+boots, it will use the preceding system generation.  This is the inverse
+of @command{reconfigure}, and it is exactly the same as invoking
+@code{switch-generation -1}.
+
+Currently, as with @command{switch-system}, you must reboot after
+running this action to actually start using the preceding system
+generation.
+
 @item build
 Build the derivation of the operating system, which includes all the
 configuration files and programs needed to boot and run the system.
diff --git a/gnu/system/grub.scm b/gnu/system/grub.scm
index d45fdca..5b520ca 100644
--- a/gnu/system/grub.scm
+++ b/gnu/system/grub.scm
@@ -65,6 +65,10 @@
 (define (strip-mount-point mount-point file)
   "Strip MOUNT-POINT from FILE, which is a gexp or other lowerable
 object denoting a file name."
+  ;; TODO: gracefully handle the case where the file resides in a location
+  ;; outside of the store, for example "/home/alice/vmlinuz".  Currently, any
+  ;; attempt to create a custom <menu-entry> with a 'linux' field set to
+  ;; something like that will likely result in an un-bootable GRUB entry.
   (if (string=? mount-point "/")
       file
       #~(let ((file #$file))
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 2ee145a..54c6eeb 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -704,7 +704,7 @@ building anything."
 (define (show-help)
   (display (_ "Usage: guix system [OPTION ...] ACTION [ARG ...] [FILE]
 Build the operating system declared in FILE according to ACTION.
-Some ACTIONS support additional ARGS.\n"))
+Some ACTIONS support additional ARGS.  See 'info guix' for details.\n"))
   (newline)
   (display (_ "The valid values for ACTION are:\n"))
   (newline)
-- 
2.9.2

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

* Re: Add system roll-back and switch-generation commands
  2016-10-28 10:07 Add system roll-back and switch-generation commands cmmarusich
                   ` (9 preceding siblings ...)
  2016-10-28 10:07 ` [PATCH 10/10] Mention new 'guix system' features in the manual cmmarusich
@ 2016-10-29 21:13 ` Ludovic Courtès
  2016-10-29 21:22   ` Chris Marusich
  10 siblings, 1 reply; 42+ messages in thread
From: Ludovic Courtès @ 2016-10-29 21:13 UTC (permalink / raw)
  To: cmmarusich; +Cc: guix-devel

Hi Chris!

cmmarusich@gmail.com skribis:

> This patch series adds 'guix system roll-back' and 'guix system
> switch-generation' commands.  It should apply cleanly to the current master
> branch.

Cool, I’ll start looking.

> I've tested the patches manually on a VM with a single disk, and a VM with
> multiple disks (with the store on a separate disk).  Two of the "check-system"
> tests fail (nss-mdns and encrypted-root-os), but those are failing on the
> current master branch for me anyway, and I can't recall ever seeing them pass,
> so I have no reason to suspect that my changes are causing the failures.

Great that you took the time to test thoroughly.

‘nss-mdns’ passes, but intermittently fails in my experience, but it’s a
tough problem.

‘encrypted-root-os’ was committed by mistake and the code to make it
pass has not been committed yet.  :-)

Of all the system tests, only the 3 in (gnu tests install) really test
the ‘guix system’ command, which is what we’re interested in here.

Ludo’.

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

* Re: Add system roll-back and switch-generation commands
  2016-10-29 21:13 ` Add system roll-back and switch-generation commands Ludovic Courtès
@ 2016-10-29 21:22   ` Chris Marusich
  0 siblings, 0 replies; 42+ messages in thread
From: Chris Marusich @ 2016-10-29 21:22 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guix-devel

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

Hi,

ludo@gnu.org (Ludovic Courtès) writes:

> ‘nss-mdns’ passes, but intermittently fails in my experience, but it’s a
> tough problem.
>
> ‘encrypted-root-os’ was committed by mistake and the code to make it
> pass has not been committed yet.  :-)
>
> Of all the system tests, only the 3 in (gnu tests install) really test
> the ‘guix system’ command, which is what we’re interested in here.

Good to know!  All the other tests passed.  I'll look into adding new
tests for these new actions.

-- 
Chris

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

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

end of thread, other threads:[~2016-11-07 10:33 UTC | newest]

Thread overview: 42+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-28 10:07 Add system roll-back and switch-generation commands cmmarusich
2016-10-28 10:07 ` [PATCH 01/10] * gnu/system.scm (<boot-parameters>): Add 'store-device' and 'store-fs-mount-point' cmmarusich
2016-10-30  0:12   ` Ludovic Courtès
2016-10-30  9:41     ` Chris Marusich
2016-10-30 22:19       ` Ludovic Courtès
2016-11-02  5:48         ` Follow-up: Add system roll-back and switch-generation commands cmmarusich
2016-11-02  5:48           ` [PATCH 1/5] profiles: Extract a procedure for getting relative generation numbers cmmarusich
2016-11-06 16:56             ` Ludovic Courtès
2016-11-02  5:48           ` [PATCH 2/5] system: Rename previous-grub-entries to profile-grub-entries cmmarusich
2016-11-06 16:56             ` Ludovic Courtès
2016-11-02  5:48           ` [PATCH 3/5] system: Optionally limit the entries returned by profile-grub-entries cmmarusich
2016-11-06 16:57             ` Ludovic Courtès
2016-11-02  5:48           ` [PATCH 4/5] install: Extract procedure: install-grub-config cmmarusich
2016-11-06 16:59             ` Ludovic Courtès
2016-11-06 21:00             ` Danny Milosavljevic
2016-11-07  1:25               ` Chris Marusich
2016-11-07 10:32                 ` Danny Milosavljevic
2016-11-02  5:48           ` [PATCH 5/5] system: Add 'guix system' actions: switch-generation and roll-back cmmarusich
2016-11-03  4:51             ` One more patch: doc: Add details to the 'guix system switch-generation' section Chris Marusich
2016-11-06 17:13             ` [PATCH 5/5] system: Add 'guix system' actions: switch-generation and roll-back Ludovic Courtès
2016-11-07  3:17               ` Chris Marusich
2016-11-03  0:19         ` [PATCH 01/10] * gnu/system.scm (<boot-parameters>): Add 'store-device' and 'store-fs-mount-point' Leo Famulari
2016-11-03  4:36           ` Chris Marusich
2016-11-03 10:35             ` Chris Marusich
2016-11-03 22:34               ` Danny Milosavljevic
2016-11-04  3:34                 ` Chris Marusich
2016-11-04  3:55                   ` Chris Marusich
2016-11-03 13:10           ` Fix a boot problem reported by ng0 cmmarusich
2016-11-03 13:10             ` [PATCH] system: Avoid using device paths in <menu-entry> device field cmmarusich
2016-11-04 15:49               ` Leo Famulari
2016-11-06 16:51               ` Ludovic Courtès
2016-10-28 10:07 ` [PATCH 02/10] Add 'device' field to <menu-entry> cmmarusich
2016-10-28 10:07 ` [PATCH 03/10] Refactor grub.cfg generation logic cmmarusich
2016-10-28 10:07 ` [PATCH 04/10] Extract procedure: relative-generation-spec->number cmmarusich
2016-10-28 10:07 ` [PATCH 05/10] Rename previous-grub-entries to grub-entries cmmarusich
2016-10-28 10:07 ` [PATCH 06/10] grub-entries: take a list of numbers on input cmmarusich
2016-10-28 10:07 ` [PATCH 07/10] Factor out procedure: install-grub-config cmmarusich
2016-10-28 10:07 ` [PATCH 08/10] Implement switch-generation and roll-back cmmarusich
2016-10-28 10:07 ` [PATCH 09/10] Rename grub-entries to profile-grub-entries cmmarusich
2016-10-28 10:07 ` [PATCH 10/10] Mention new 'guix system' features in the manual cmmarusich
2016-10-29 21:13 ` Add system roll-back and switch-generation commands Ludovic Courtès
2016-10-29 21:22   ` Chris Marusich

Code repositories for project(s) associated with this public inbox

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).