* bug#40998: Guix System's initrd doesn't honor rootflags
@ 2020-05-01 13:53 maxim.cournoyer
2022-02-18 15:33 ` bug#40998: [PATCH 1/3] initrd: Rename the '--root' initrd option to just 'root' Maxim Cournoyer
2022-02-19 7:01 ` bug#40998: [PATCH v2 1/4] system: Add a version field to the <boot-parameters> record Maxim Cournoyer
0 siblings, 2 replies; 19+ messages in thread
From: maxim.cournoyer @ 2020-05-01 13:53 UTC (permalink / raw)
To: 40998
Which means users cannot pass a 'rootflags=degraded' command line option
to the kernel line in GRUB to attempt booting a system whose RAID array
is degraded.
rootflags is standard across distributions, and is what users expect.
Such support was added in earlier version of the patches proposed here:
https://issues.guix.gnu.org/37305#0 but is later simplified out.
Maxim
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#40998: [PATCH 1/3] initrd: Rename the '--root' initrd option to just 'root'.
2020-05-01 13:53 bug#40998: Guix System's initrd doesn't honor rootflags maxim.cournoyer
@ 2022-02-18 15:33 ` Maxim Cournoyer
2022-02-18 15:33 ` bug#40998: [PATCH 2/3] initrd: Honor rootfstype and rootflags command-line parameters Maxim Cournoyer
2022-02-18 15:33 ` bug#40998: [PATCH 3/3] initrd: Print its " Maxim Cournoyer
2022-02-19 7:01 ` bug#40998: [PATCH v2 1/4] system: Add a version field to the <boot-parameters> record Maxim Cournoyer
1 sibling, 2 replies; 19+ messages in thread
From: Maxim Cournoyer @ 2022-02-18 15:33 UTC (permalink / raw)
To: 40998; +Cc: Maxim Cournoyer
This is to make it less surprising, given the common convention sets forth by
the kernel Linux command-line parameters.
* gnu/build/linux-boot.scm (boot-system): Rename the '--root' option to
'root'. Adjust doc.
* gnu/installer/parted.scm (installer-root-partition-path): Use 'root' instead
of '--root'.
* gnu/system.scm (bootable-kernel-arguments): Likewise.
(operating-system-boot-parameters): Adjust doc.
(operating-system-boot-parameters-file): Likewise.
* gnu/system/linux-initrd.scm (base-initrd): Likewise.
* doc/guix.texi: Adjust doc.
---
doc/guix.texi | 6 +++---
gnu/build/linux-boot.scm | 20 ++++++++++----------
gnu/installer/parted.scm | 2 +-
gnu/system.scm | 8 ++++----
gnu/system/linux-initrd.scm | 4 ++--
5 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/doc/guix.texi b/doc/guix.texi
index a4145af7fd..ba613ea31f 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -34909,7 +34909,7 @@ Guix uses this option to yield control to a boot program that runs the
service activation programs and then spawns the GNU@tie{}Shepherd, the
initialization system.
-@item --root=@var{root}
+@item root=@var{root}
Mount @var{root} as the root file system. @var{root} can be a device
name like @code{/dev/sda1}, a file system label, or a file system UUID.
When unspecified, the device name from the root file system of the
@@ -34967,7 +34967,7 @@ here is how to use it and customize it further.
[#:helper-packages '()] [#:qemu-networking? #f] [#:volatile-root? #f]
Return a derivation that builds a raw initrd. @var{file-systems} is
a list of file systems to be mounted by the initrd, possibly in addition to
-the root file system specified on the kernel command line via @option{--root}.
+the root file system specified on the kernel command line via @option{root}.
@var{linux-modules} is a list of kernel modules to be loaded at boot time.
@var{mapped-devices} is a list of device mappings to realize before
@var{file-systems} are mounted (@pxref{Mapped Devices}).
@@ -34997,7 +34997,7 @@ to it are lost.
Return as a file-like object a generic initrd, with kernel
modules taken from @var{linux}. @var{file-systems} is a list of file-systems to be
mounted by the initrd, possibly in addition to the root file system specified
-on the kernel command line via @option{--root}. @var{mapped-devices} is a list of device
+on the kernel command line via @option{root}. @var{mapped-devices} is a list of device
mappings to realize before @var{file-systems} are mounted.
When true, @var{keyboard-layout} is a @code{<keyboard-layout>} record denoting
diff --git a/gnu/build/linux-boot.scm b/gnu/build/linux-boot.scm
index 0ae316849e..d36601d824 100644
--- a/gnu/build/linux-boot.scm
+++ b/gnu/build/linux-boot.scm
@@ -499,12 +499,12 @@ (define* (boot-system #:key
KEYMAP-FILE is true), then setting up QEMU guest networking if
QEMU-GUEST-NETWORKING? is true, calling PRE-MOUNT, mounting the file systems
specified in MOUNTS, and finally booting into the new root if any. The initrd
-supports kernel command-line options '--load', '--root', and '--repl'. It
-also honors a subset of the documented Linux kernel command-line parameters
-such as 'fsck.mode', 'resume' and 'rootdelay'.
+supports kernel command-line options '--load' and '--repl'. It also honors a
+subset of the Linux kernel command-line parameters such as 'fsck.mode',
+'resume', 'root' and 'rootdelay'.
-Mount the root file system, specified by the '--root' command-line argument,
-if any.
+Mount the root file system, specified by the 'root' command-line argument, if
+any.
MOUNTS must be a list of <file-system> objects.
@@ -517,9 +517,9 @@ (define (root-mount-point? fs)
(string=? (file-system-mount-point fs) "/"))
(define (device-string->file-system-device device-string)
- ;; The "--root=SPEC" kernel command-line option always provides a
- ;; string, but the string can represent a device, an nfs-root, a UUID, or a
- ;; label. So check for all four.
+ ;; The "root=SPEC" kernel command-line option always provides a string,
+ ;; but the string can represent a device, an nfs-root, a UUID, or a label.
+ ;; So check for all four.
(cond ((string-prefix? "/" device-string) device-string)
((string-contains device-string ":/") device-string) ; nfs-root
((uuid device-string) => identity)
@@ -533,9 +533,9 @@ (define (device-string->file-system-device device-string)
(mount-essential-file-systems)
(let* ((args (linux-command-line))
(to-load (find-long-option "--load" args))
- ;; If present, ‘--root’ on the kernel command line takes precedence
+ ;; If present, ‘root’ on the kernel command line takes precedence
;; over the ‘device’ field of the root <file-system> record.
- (root-device (and=> (find-long-option "--root" args)
+ (root-device (and=> (find-long-option "root" args)
device-string->file-system-device))
(root-fs (or (find root-mount-point? mounts)
;; Fall back to fictitious defaults.
diff --git a/gnu/installer/parted.scm b/gnu/installer/parted.scm
index e33ef5f8fd..94ef9b42bc 100644
--- a/gnu/installer/parted.scm
+++ b/gnu/installer/parted.scm
@@ -348,7 +348,7 @@ (define (remove-logical-devices)
(define (installer-root-partition-path)
"Return the root partition path, or #f if it could not be detected."
(let* ((cmdline (linux-command-line))
- (root (find-long-option "--root" cmdline)))
+ (root (find-long-option "root" cmdline)))
(and root
(or (and (access? root F_OK) root)
(find-partition-by-label root)
diff --git a/gnu/system.scm b/gnu/system.scm
index cc925de16f..f14186a64f 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -187,9 +187,9 @@ (define-module (gnu system)
(define (bootable-kernel-arguments system root-device)
"Return a list of kernel arguments (gexps) to boot SYSTEM from ROOT-DEVICE."
- (list (string-append "--root="
+ (list (string-append "root="
;; Note: Always use the DCE format because that's what
- ;; (gnu build linux-boot) expects for the '--root'
+ ;; (gnu build linux-boot) expects for the 'root'
;; kernel command-line option.
(file-system-device->string root-device
#:uuid-type 'dce))
@@ -1441,7 +1441,7 @@ (define* (operating-system-boot-parameters os root-device
#:key system-kernel-arguments?)
"Return a monadic <boot-parameters> record that describes the boot
parameters of OS. When SYSTEM-KERNEL-ARGUMENTS? is true, add kernel arguments
-such as '--root' and '--load' to <boot-parameters>."
+such as 'root' and '--load' to <boot-parameters>."
(let* ((initrd (and (not (operating-system-hurd os))
(operating-system-initrd-file os)))
(store (operating-system-store-file-system os))
@@ -1487,7 +1487,7 @@ (define* (operating-system-boot-parameters-file os
"Return a file that describes the boot parameters of OS. The primary use of
this file is the reconstruction of GRUB menu entries for old configurations.
-When SYSTEM-KERNEL-ARGUMENTS? is true, add kernel arguments such as '--root'
+When SYSTEM-KERNEL-ARGUMENTS? is true, add kernel arguments such as 'root'
and '--load' to the returned file (since the returned file is then usually
stored into the content-addressed \"system\" directory, it's usually not a
good idea to give it because the content hash would change by the content hash
diff --git a/gnu/system/linux-initrd.scm b/gnu/system/linux-initrd.scm
index 329cd38cd6..4c4c78e444 100644
--- a/gnu/system/linux-initrd.scm
+++ b/gnu/system/linux-initrd.scm
@@ -181,7 +181,7 @@ (define* (raw-initrd file-systems
"Return as a file-like object a raw initrd, with kernel
modules taken from LINUX. FILE-SYSTEMS is a list of file-systems to be
mounted by the initrd, possibly in addition to the root file system specified
-on the kernel command line via '--root'. LINUX-MODULES is a list of kernel
+on the kernel command line via 'root'. LINUX-MODULES is a list of kernel
modules to be loaded at boot time. MAPPED-DEVICES is a list of device
mappings to realize before FILE-SYSTEMS are mounted.
HELPER-PACKAGES is a list of packages to be copied in the initrd. It may include
@@ -375,7 +375,7 @@ (define* (base-initrd file-systems
"Return as a file-like object a generic initrd, with kernel
modules taken from LINUX. FILE-SYSTEMS is a list of file-systems to be
mounted by the initrd, possibly in addition to the root file system specified
-on the kernel command line via '--root'. MAPPED-DEVICES is a list of device
+on the kernel command line via 'root'. MAPPED-DEVICES is a list of device
mappings to realize before FILE-SYSTEMS are mounted.
When true, KEYBOARD-LAYOUT is a <keyboard-layout> record denoting the desired
--
2.34.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* bug#40998: [PATCH 2/3] initrd: Honor rootfstype and rootflags command-line parameters.
2022-02-18 15:33 ` bug#40998: [PATCH 1/3] initrd: Rename the '--root' initrd option to just 'root' Maxim Cournoyer
@ 2022-02-18 15:33 ` Maxim Cournoyer
2022-02-18 15:33 ` bug#40998: [PATCH 3/3] initrd: Print its " Maxim Cournoyer
1 sibling, 0 replies; 19+ messages in thread
From: Maxim Cournoyer @ 2022-02-18 15:33 UTC (permalink / raw)
To: 40998; +Cc: Maxim Cournoyer
* gnu/build/linux-boot.scm (boot-system): Honor rootfstype and rootflags
arguments. Update doc. Error out in case there is insufficient information
with regard to the root file system.
Restore the behavior of inferring the root device from the root file system
from the operating system in case the root argument is not provided.
* doc/guix.texi (Initial RAM Disk): Document the new command-line parameters.
---
doc/guix.texi | 10 +++++++
gnu/build/linux-boot.scm | 59 +++++++++++++++++++++++++---------------
2 files changed, 47 insertions(+), 22 deletions(-)
diff --git a/doc/guix.texi b/doc/guix.texi
index ba613ea31f..cbfd9f68f2 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -34915,6 +34915,16 @@ name like @code{/dev/sda1}, a file system label, or a file system UUID.
When unspecified, the device name from the root file system of the
operating system declaration is used.
+@item rootfstype=@var{type}
+Set the type of the root file system. It overrides the @code{type}
+field of the root file system specified via the @code{operating-system}
+declaration, if any.
+
+@item rootflags=@var{options}
+Set the mount @emph{options} of the root file system. It overrides the
+@code{options} field of the root file system specified via the
+@code{operating-system} declaration, if any.
+
@item fsck.mode=@var{mode}
Whether to check the @var{root} file system for errors before mounting
it. @var{mode} is one of @code{skip} (never check), @code{force} (always
diff --git a/gnu/build/linux-boot.scm b/gnu/build/linux-boot.scm
index d36601d824..b1573fe757 100644
--- a/gnu/build/linux-boot.scm
+++ b/gnu/build/linux-boot.scm
@@ -499,9 +499,9 @@ (define* (boot-system #:key
KEYMAP-FILE is true), then setting up QEMU guest networking if
QEMU-GUEST-NETWORKING? is true, calling PRE-MOUNT, mounting the file systems
specified in MOUNTS, and finally booting into the new root if any. The initrd
-supports kernel command-line options '--load' and '--repl'. It also honors a
-subset of the Linux kernel command-line parameters such as 'fsck.mode',
-'resume', 'root' and 'rootdelay'.
+supports the kernel command-line options '--load' and '--repl'. It
+also honors a subset of the Linux kernel command-line parameters such as
+'fsck.mode', 'resume', 'rootdelay', rootflags and rootfstype.
Mount the root file system, specified by the 'root' command-line argument, if
any.
@@ -533,17 +533,32 @@ (define (device-string->file-system-device device-string)
(mount-essential-file-systems)
(let* ((args (linux-command-line))
(to-load (find-long-option "--load" args))
- ;; If present, ‘root’ on the kernel command line takes precedence
- ;; over the ‘device’ field of the root <file-system> record.
(root-device (and=> (find-long-option "root" args)
device-string->file-system-device))
- (root-fs (or (find root-mount-point? mounts)
- ;; Fall back to fictitious defaults.
- (file-system (device (or root-device "/dev/root"))
- (mount-point "/")
- (type "ext4"))))
+ (rootfstype (find-long-option "rootfstype" args))
+ (rootflags (find-long-option "rootflags" args))
+ (root-fs* (find root-mount-point? mounts))
(fsck.mode (find-long-option "fsck.mode" args)))
+ (unless (or root-fs* (and root-device rootfstype))
+ (error "no root file system or 'root' and 'rootfstype' parameters"))
+
+ ;; If present, ‘root’ on the kernel command line takes precedence over
+ ;; the ‘device’ field of the root <file-system> record; likewise for
+ ;; the 'rootfstype' and 'rootflags' arguments.
+ (define root-fs
+ (if root-fs*
+ (file-system
+ (inherit root-fs*)
+ (device (or root-device (file-system-device root-fs*)))
+ (type (or rootfstype (file-system-type root-fs*)))
+ (options (or rootflags (file-system-options root-fs*))))
+ (file-system
+ (device root-device)
+ (mount-point "/")
+ (type rootfstype)
+ (options rootflags))))
+
(define (check? fs)
(match fsck.mode
("skip" #f)
@@ -615,18 +630,18 @@ (define (repair fs)
(setenv "EXT2FS_NO_MTAB_OK" "1")
- (if root-device
- (mount-root-file-system (canonicalize-device-spec root-device)
- (file-system-type root-fs)
- #:volatile-root? volatile-root?
- #:flags (mount-flags->bit-mask
- (file-system-flags root-fs))
- #:options (file-system-options root-fs)
- #:check? (check? root-fs)
- #:skip-check-if-clean?
- (skip-check-if-clean? root-fs)
- #:repair (repair root-fs))
- (mount "none" "/root" "tmpfs"))
+ ;; Mount the root file system.
+ (mount-root-file-system (canonicalize-device-spec
+ (file-system-device root-fs))
+ (file-system-type root-fs)
+ #:volatile-root? volatile-root?
+ #:flags (mount-flags->bit-mask
+ (file-system-flags root-fs))
+ #:options (file-system-options root-fs)
+ #:check? (check? root-fs)
+ #:skip-check-if-clean?
+ (skip-check-if-clean? root-fs)
+ #:repair (repair root-fs))
;; Mount the specified non-root file systems.
(for-each (lambda (fs)
--
2.34.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* bug#40998: [PATCH 3/3] initrd: Print its command-line parameters.
2022-02-18 15:33 ` bug#40998: [PATCH 1/3] initrd: Rename the '--root' initrd option to just 'root' Maxim Cournoyer
2022-02-18 15:33 ` bug#40998: [PATCH 2/3] initrd: Honor rootfstype and rootflags command-line parameters Maxim Cournoyer
@ 2022-02-18 15:33 ` Maxim Cournoyer
1 sibling, 0 replies; 19+ messages in thread
From: Maxim Cournoyer @ 2022-02-18 15:33 UTC (permalink / raw)
To: 40998; +Cc: Maxim Cournoyer
* gnu/build/linux-boot.scm (boot-system): Print command-line parameters to
standard output.
---
gnu/build/linux-boot.scm | 2 ++
1 file changed, 2 insertions(+)
diff --git a/gnu/build/linux-boot.scm b/gnu/build/linux-boot.scm
index b1573fe757..45be05d705 100644
--- a/gnu/build/linux-boot.scm
+++ b/gnu/build/linux-boot.scm
@@ -540,6 +540,8 @@ (define (device-string->file-system-device device-string)
(root-fs* (find root-mount-point? mounts))
(fsck.mode (find-long-option "fsck.mode" args)))
+ (format #t "initrd command-line parameters: ~a~%" args)
+
(unless (or root-fs* (and root-device rootfstype))
(error "no root file system or 'root' and 'rootfstype' parameters"))
--
2.34.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* bug#40998: [PATCH v2 1/4] system: Add a version field to the <boot-parameters> record.
2020-05-01 13:53 bug#40998: Guix System's initrd doesn't honor rootflags maxim.cournoyer
2022-02-18 15:33 ` bug#40998: [PATCH 1/3] initrd: Rename the '--root' initrd option to just 'root' Maxim Cournoyer
@ 2022-02-19 7:01 ` Maxim Cournoyer
2022-02-19 7:01 ` bug#40998: [PATCH v2 2/4] initrd: Use non-hyphenated kernel command-line parameter names Maxim Cournoyer
` (3 more replies)
1 sibling, 4 replies; 19+ messages in thread
From: Maxim Cournoyer @ 2022-02-19 7:01 UTC (permalink / raw)
To: 40998; +Cc: Maxim Cournoyer
This version field exposes the (already present) version information of a boot
parameters file.
* gnu/system.scm (%boot-parameters-version): New variable.
(<boot-parameters>)[version]: New field.
(read-boot-parameters): Use it.
(operating-system-boot-parameters-file): Likewise.
* tests/boot-parameters.scm (test-read-boot-parameters): Use
%boot-parameters-version as the default version value in the template.
---
gnu/system.scm | 18 ++++++++++++++----
tests/boot-parameters.scm | 2 +-
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/gnu/system.scm b/gnu/system.scm
index cc925de16f..c29a44111b 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -9,7 +9,7 @@
;;; Copyright © 2020 Danny Milosavljevic <dannym@scratchpost.org>
;;; Copyright © 2020, 2021 Brice Waegeneire <brice@waegenei.re>
;;; Copyright © 2020 Florian Pelz <pelzflorian@pelzflorian.de>
-;;; Copyright © 2020 Maxim Cournoyer <maxim.cournoyer@gmail.com>
+;;; Copyright © 2020, 2022 Maxim Cournoyer <maxim.cournoyer@gmail.com>
;;; Copyright © 2020 Jan (janneke) Nieuwenhuizen <jannek@gnu.org>
;;; Copyright © 2020 Efraim Flashner <efraim@flashner.co.il>
;;; Copyright © 2021 Maxime Devos <maximedevos@telenet.be>
@@ -161,6 +161,8 @@ (define-module (gnu system)
boot-parameters-kernel-arguments
boot-parameters-initrd
boot-parameters-multiboot-modules
+ boot-parameters-version
+ %boot-parameters-version
read-boot-parameters
read-boot-parameters-file
boot-parameters->menu-entry
@@ -295,6 +297,8 @@ (define (operating-system-kernel-arguments os root-device)
;;; Boot parameters
;;;
+(define %boot-parameters-version 0)
+
(define-record-type* <boot-parameters>
boot-parameters make-boot-parameters boot-parameters?
(label boot-parameters-label)
@@ -322,7 +326,9 @@ (define-record-type* <boot-parameters>
(kernel boot-parameters-kernel)
(kernel-arguments boot-parameters-kernel-arguments)
(initrd boot-parameters-initrd)
- (multiboot-modules boot-parameters-multiboot-modules))
+ (multiboot-modules boot-parameters-multiboot-modules)
+ (version boot-parameters-version ;positive integer
+ (default %boot-parameters-version)))
(define (ensure-not-/dev device)
"If DEVICE starts with a slash, return #f. This is meant to filter out
@@ -359,12 +365,16 @@ (define uuid-sexp->uuid
(warning (G_ "unrecognized uuid ~a at '~a'~%") x (port-filename port))
#f)))
+ (define (version? n)
+ (and (exact-integer? n) (not (negative? n))))
+
(match (read port)
- (('boot-parameters ('version 0)
+ (('boot-parameters ('version (? version? version))
('label label) ('root-device root)
('kernel kernel)
rest ...)
(boot-parameters
+ (version version)
(label label)
(root-device (device-sexp->device root))
@@ -1500,7 +1510,7 @@ (define* (operating-system-boot-parameters-file os
system-kernel-arguments?)))
(scheme-file "parameters"
#~(boot-parameters
- (version 0)
+ (version #$(boot-parameters-version params))
(label #$(boot-parameters-label params))
(root-device
#$(device->sexp
diff --git a/tests/boot-parameters.scm b/tests/boot-parameters.scm
index b2799d0596..d4b680df2e 100644
--- a/tests/boot-parameters.scm
+++ b/tests/boot-parameters.scm
@@ -101,7 +101,7 @@ (define (quote-uuid uuid)
;; Call read-boot-parameters with the desired string as input.
(define* (test-read-boot-parameters
#:key
- (version 0)
+ (version %boot-parameters-version)
(bootloader-name 'grub)
(bootloader-menu-entries '())
(label %default-label)
--
2.34.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* bug#40998: [PATCH v2 2/4] initrd: Use non-hyphenated kernel command-line parameter names.
2022-02-19 7:01 ` bug#40998: [PATCH v2 1/4] system: Add a version field to the <boot-parameters> record Maxim Cournoyer
@ 2022-02-19 7:01 ` Maxim Cournoyer
2022-02-27 21:03 ` bug#40998: Guix System's initrd doesn't honor rootflags Ludovic Courtès
2022-02-19 7:01 ` bug#40998: [PATCH v2 3/4] initrd: Honor rootfstype and rootflags command-line parameters Maxim Cournoyer
` (2 subsequent siblings)
3 siblings, 1 reply; 19+ messages in thread
From: Maxim Cournoyer @ 2022-02-19 7:01 UTC (permalink / raw)
To: 40998; +Cc: Maxim Cournoyer
This is to make it less surprising, given the common convention sets forth by
the kernel Linux command-line parameters.
* gnu/build/linux-boot.scm (boot-system): Rename '--load', '--repl', '--root'
and '--system' to 'gnu.load', 'gnu.repl', 'root' and 'gnu.system',
respectively. Adjust doc.
(find-long-option): Adjust doc.
* gnu/installer/parted.scm (installer-root-partition-path): Adjust accordingly.
* gnu/system.scm (bootable-kernel-arguments): Add a VERSION argument and
update doc. Use VERSION to conditionally return old style vs new style initrd
arguments.
(%boot-parameters-version): Increment to 1.
(operating-system-boot-parameters): Adjust doc.
(operating-system-boot-parameters-file): Likewise.
* gnu/system/linux-initrd.scm (raw-initrd, base-initrd): Likewise.
* doc/guix.texi: Adjust doc.
* gnu/build/activation.scm (boot-time-system): Adjust accordingly.
* gnu/build/hurd-boot.scm (boot-hurd-system): Likewise.
* gnu/packages/commencement.scm (%final-inputs-riscv64): Adjust comment.
---
doc/guix.texi | 12 +++++-----
gnu/build/activation.scm | 4 ++--
gnu/build/hurd-boot.scm | 12 +++++-----
gnu/build/linux-boot.scm | 30 +++++++++++------------
gnu/installer/parted.scm | 2 +-
gnu/machine/ssh.scm | 5 ++--
gnu/packages/commencement.scm | 4 ++--
gnu/system.scm | 45 ++++++++++++++++++++++-------------
gnu/system/linux-initrd.scm | 4 ++--
9 files changed, 66 insertions(+), 52 deletions(-)
diff --git a/doc/guix.texi b/doc/guix.texi
index a4145af7fd..d3f9216563 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -34901,7 +34901,7 @@ honors several options passed on the Linux kernel command line
@code{-append} option of QEMU), notably:
@table @code
-@item --load=@var{boot}
+@item gnu.load=@var{boot}
Tell the initial RAM disk to load @var{boot}, a file containing a Scheme
program, once it has mounted the root file system.
@@ -34909,7 +34909,7 @@ Guix uses this option to yield control to a boot program that runs the
service activation programs and then spawns the GNU@tie{}Shepherd, the
initialization system.
-@item --root=@var{root}
+@item root=@var{root}
Mount @var{root} as the root file system. @var{root} can be a device
name like @code{/dev/sda1}, a file system label, or a file system UUID.
When unspecified, the device name from the root file system of the
@@ -34934,7 +34934,7 @@ or @code{preen} to repair problems considered safe to repair automatically.
@code{preen} is the default if this option is not present or if @var{level}
is not one of the above.
-@item --system=@var{system}
+@item gnu.system=@var{system}
Have @file{/run/booted-system} and @file{/run/current-system} point to
@var{system}.
@@ -34946,7 +34946,7 @@ Instruct the initial RAM disk as well as the @command{modprobe} command
must be a comma-separated list of module names---e.g.,
@code{usbkbd,9pnet}.
-@item --repl
+@item gnu.repl
Start a read-eval-print loop (REPL) from the initial RAM disk before it
tries to load kernel modules and to mount the root file system. Our
marketing team calls it @dfn{boot-to-Guile}. The Schemer in you will
@@ -34967,7 +34967,7 @@ here is how to use it and customize it further.
[#:helper-packages '()] [#:qemu-networking? #f] [#:volatile-root? #f]
Return a derivation that builds a raw initrd. @var{file-systems} is
a list of file systems to be mounted by the initrd, possibly in addition to
-the root file system specified on the kernel command line via @option{--root}.
+the root file system specified on the kernel command line via @option{root}.
@var{linux-modules} is a list of kernel modules to be loaded at boot time.
@var{mapped-devices} is a list of device mappings to realize before
@var{file-systems} are mounted (@pxref{Mapped Devices}).
@@ -34997,7 +34997,7 @@ to it are lost.
Return as a file-like object a generic initrd, with kernel
modules taken from @var{linux}. @var{file-systems} is a list of file-systems to be
mounted by the initrd, possibly in addition to the root file system specified
-on the kernel command line via @option{--root}. @var{mapped-devices} is a list of device
+on the kernel command line via @option{root}. @var{mapped-devices} is a list of device
mappings to realize before @var{file-systems} are mounted.
When true, @var{keyboard-layout} is a @code{<keyboard-layout>} record denoting
diff --git a/gnu/build/activation.scm b/gnu/build/activation.scm
index 9f6126023c..10c9045740 100644
--- a/gnu/build/activation.scm
+++ b/gnu/build/activation.scm
@@ -389,8 +389,8 @@ (define %current-system
"/run/current-system")
(define (boot-time-system)
- "Return the '--system' argument passed on the kernel command line."
- (find-long-option "--system" (if (string-contains %host-type "linux-gnu")
+ "Return the 'gnu.system' argument passed on the kernel command line."
+ (find-long-option "gnu.system" (if (string-contains %host-type "linux-gnu")
(linux-command-line)
(command-line))))
diff --git a/gnu/build/hurd-boot.scm b/gnu/build/hurd-boot.scm
index ac36bd17d4..ad3c50d61e 100644
--- a/gnu/build/hurd-boot.scm
+++ b/gnu/build/hurd-boot.scm
@@ -254,7 +254,7 @@ (define* (boot-hurd-system #:key (on-error 'debug))
"This procedure is meant to be called from an early RC script.
Install the relevant passive translators on the first boot. Then, run system
-activation by using the kernel command-line options '--system' and '--load';
+activation by using the kernel command-line options 'gnu.system' and 'gnu.load';
starting the Shepherd.
XXX TODO: see linux-boot.scm:boot-system.
@@ -265,14 +265,14 @@ (define* (boot-hurd-system #:key (on-error 'debug))
"
(display "Welcome, this is GNU's early boot Guile.\n")
- (display "Use '--repl' for an initrd REPL.\n\n")
+ (display "Use 'gnu.repl' for an initrd REPL.\n\n")
(call-with-error-handling
(lambda ()
(let* ((args (command-line))
- (system (find-long-option "--system" args))
- (to-load (find-long-option "--load" args)))
+ (system (find-long-option "gnu.system" args))
+ (to-load (find-long-option "gnu.load" args)))
(format #t "Setting-up essential translators...\n")
(setenv "PATH" (string-append system "/profile/bin"))
@@ -286,7 +286,7 @@ (define* (boot-hurd-system #:key (on-error 'debug))
(unless (zero? (system* "/hurd/mach-defpager"))
(format #t "FAILED...Good luck!\n"))
- (cond ((member "--repl" args)
+ (cond ((member "gnu.repl" args)
(format #t "Starting repl...\n")
(start-repl))
(to-load
@@ -298,7 +298,7 @@ (define* (boot-hurd-system #:key (on-error 'debug))
(sleep 2)
(reboot))
(else
- (display "no boot file passed via '--load'\n")
+ (display "no boot file passed via 'gnu.load'\n")
(display "entering a warm and cozy REPL\n")
(start-repl)))))
#:on-error on-error))
diff --git a/gnu/build/linux-boot.scm b/gnu/build/linux-boot.scm
index 0ae316849e..179fb12a2b 100644
--- a/gnu/build/linux-boot.scm
+++ b/gnu/build/linux-boot.scm
@@ -93,7 +93,7 @@ (define (linux-command-line)
get-string-all)))
(define (find-long-option option arguments)
- "Find OPTION among ARGUMENTS, where OPTION is something like \"--load\".
+ "Find OPTION among ARGUMENTS, where OPTION is something like \"gnu.load\".
Return the value associated with OPTION, or #f on failure."
(let ((opt (string-append option "=")))
(and=> (find (cut string-prefix? opt <>)
@@ -499,12 +499,12 @@ (define* (boot-system #:key
KEYMAP-FILE is true), then setting up QEMU guest networking if
QEMU-GUEST-NETWORKING? is true, calling PRE-MOUNT, mounting the file systems
specified in MOUNTS, and finally booting into the new root if any. The initrd
-supports kernel command-line options '--load', '--root', and '--repl'. It
-also honors a subset of the documented Linux kernel command-line parameters
-such as 'fsck.mode', 'resume' and 'rootdelay'.
+supports kernel command-line parameters 'gnu.load' and 'gnu.repl'. It also
+honors a subset of the Linux kernel command-line parameters such as
+'fsck.mode', 'resume', 'root' and 'rootdelay'.
-Mount the root file system, specified by the '--root' command-line argument,
-if any.
+Mount the root file system, specified by the 'root' command-line argument, if
+any.
MOUNTS must be a list of <file-system> objects.
@@ -517,25 +517,25 @@ (define (root-mount-point? fs)
(string=? (file-system-mount-point fs) "/"))
(define (device-string->file-system-device device-string)
- ;; The "--root=SPEC" kernel command-line option always provides a
- ;; string, but the string can represent a device, an nfs-root, a UUID, or a
- ;; label. So check for all four.
+ ;; The "root=SPEC" kernel command-line option always provides a string,
+ ;; but the string can represent a device, an nfs-root, a UUID, or a label.
+ ;; So check for all four.
(cond ((string-prefix? "/" device-string) device-string)
((string-contains device-string ":/") device-string) ; nfs-root
((uuid device-string) => identity)
(else (file-system-label device-string))))
(display "Welcome, this is GNU's early boot Guile.\n")
- (display "Use '--repl' for an initrd REPL.\n\n")
+ (display "Use 'gnu.repl' for an initrd REPL.\n\n")
(call-with-error-handling
(lambda ()
(mount-essential-file-systems)
(let* ((args (linux-command-line))
- (to-load (find-long-option "--load" args))
- ;; If present, ‘--root’ on the kernel command line takes precedence
+ (to-load (find-long-option "gnu.load" args))
+ ;; If present, ‘root’ on the kernel command line takes precedence
;; over the ‘device’ field of the root <file-system> record.
- (root-device (and=> (find-long-option "--root" args)
+ (root-device (and=> (find-long-option "root" args)
device-string->file-system-device))
(root-fs (or (find root-mount-point? mounts)
;; Fall back to fictitious defaults.
@@ -564,7 +564,7 @@ (define (repair fs)
(_ 'preen))
(file-system-repair fs))))
- (when (member "--repl" args)
+ (when (member "gnu.repl" args)
(start-repl))
(display "loading kernel modules...\n")
@@ -652,7 +652,7 @@ (define (repair fs)
(sleep 2)
(reboot))
(begin
- (display "no boot file passed via '--load'\n")
+ (display "no boot file passed via 'gnu.load'\n")
(display "entering a warm and cozy REPL\n")
(start-repl)))))
#:on-error on-error))
diff --git a/gnu/installer/parted.scm b/gnu/installer/parted.scm
index e33ef5f8fd..94ef9b42bc 100644
--- a/gnu/installer/parted.scm
+++ b/gnu/installer/parted.scm
@@ -348,7 +348,7 @@ (define (remove-logical-devices)
(define (installer-root-partition-path)
"Return the root partition path, or #f if it could not be detected."
(let* ((cmdline (linux-command-line))
- (root (find-long-option "--root" cmdline)))
+ (root (find-long-option "root" cmdline)))
(and root
(or (and (access? root F_OK) root)
(find-partition-by-label root)
diff --git a/gnu/machine/ssh.scm b/gnu/machine/ssh.scm
index 0dc8933c82..550c989c34 100644
--- a/gnu/machine/ssh.scm
+++ b/gnu/machine/ssh.scm
@@ -422,7 +422,8 @@ (define (read-file path)
(let* ((params (call-with-input-string serialized-params
read-boot-parameters))
(root (boot-parameters-root-device params))
- (label (boot-parameters-label params)))
+ (label (boot-parameters-label params))
+ (version (boot-parameters-version params)))
(boot-parameters
(inherit params)
(label
@@ -433,7 +434,7 @@ (define (read-file path)
"~Y-~m-~d ~H:~M"))
")"))
(kernel-arguments
- (append (bootable-kernel-arguments system-path root)
+ (append (bootable-kernel-arguments system-path root version)
(boot-parameters-kernel-arguments params))))))))
generations))))
diff --git a/gnu/packages/commencement.scm b/gnu/packages/commencement.scm
index cdc9c1d621..9b495d8d84 100644
--- a/gnu/packages/commencement.scm
+++ b/gnu/packages/commencement.scm
@@ -3740,10 +3740,10 @@ (define-public %final-inputs
(define-public %final-inputs-riscv64
;; This is similar to the added (list gcc "lib") elsewhere in this file, but
;; due to how (%current-system) is re-defined when performing builds with the
- ;; '--system' flag, %final-inputs is too early in the evaulation pipeline to
+ ;; 'gnu.system' flag, %final-inputs is too early in the evaulation pipeline to
;; correctly identify the system for which a derivation will be built. Thus,
;; since (%current-system) is re-determined by (guix build-system gnu) after
- ;; loading %final-inputs but before taking into account the '--system' flag,
+ ;; loading %final-inputs but before taking into account the 'gnu.system' flag,
;; the test for (target-riscv64?) needs to be in (guix build-system gnu),
;; with %final-inputs-riscv64 already available at the same time that
;; %final-inputs is available.
diff --git a/gnu/system.scm b/gnu/system.scm
index c29a44111b..97669daa1b 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -187,16 +187,23 @@ (define-module (gnu system)
;;;
;;; Code:
-(define (bootable-kernel-arguments system root-device)
- "Return a list of kernel arguments (gexps) to boot SYSTEM from ROOT-DEVICE."
- (list (string-append "--root="
+(define* (bootable-kernel-arguments system root-device version)
+ "Return a list of kernel arguments (gexps) to boot SYSTEM from ROOT-DEVICE.
+VERSION is the target version of the boot-parameters record."
+ ;; If the version is newer than 0, we use the new style initrd parameter
+ ;; names, otherwise we use the legacy ones. This is to maintain backward
+ ;; compatibility when producing bootloader configurations for older
+ ;; generations.
+ (define version>0? (> version 0))
+ (list (string-append (if version>0? "root=" "--root=")
;; Note: Always use the DCE format because that's what
- ;; (gnu build linux-boot) expects for the '--root'
+ ;; (gnu build linux-boot) expects for the 'root'
;; kernel command-line option.
(file-system-device->string root-device
#:uuid-type 'dce))
- #~(string-append "--system=" #$system)
- #~(string-append "--load=" #$system "/boot")))
+ #~(string-append (if #$version>0? "gnu.system=" "--system=") #$system)
+ #~(string-append (if #$version>0? "gnu.load=" "--load=")
+ #$system "/boot")))
;; System-wide configuration.
;; TODO: Add per-field docstrings/stexi.
@@ -286,10 +293,12 @@ (define-record-type* <operating-system> operating-system
source-properties->location))
(innate)))
-(define (operating-system-kernel-arguments os root-device)
- "Return all the kernel arguments, including the ones not specified
-directly by the user."
- (append (bootable-kernel-arguments os root-device)
+(define* (operating-system-kernel-arguments
+ os root-device #:key (version %boot-parameters-version))
+ "Return all the kernel arguments, including the ones not specified directly
+by the user. VERSION should match that of the target <boot-parameter> record
+object that will contain the kernel parameters."
+ (append (bootable-kernel-arguments os root-device version)
(operating-system-user-kernel-arguments os)))
\f
@@ -297,7 +306,10 @@ (define (operating-system-kernel-arguments os root-device)
;;; Boot parameters
;;;
-(define %boot-parameters-version 0)
+;;; Version 1 was introduced early 2022 to mark the departure from long option
+;;; names such as '--load' to the more conventional initrd option names like
+;;; 'gnu.load'.
+(define %boot-parameters-version 1)
(define-record-type* <boot-parameters>
boot-parameters make-boot-parameters boot-parameters?
@@ -476,10 +488,11 @@ (define (read-boot-parameters-file system)
The object has its kernel-arguments extended in order to make it bootable."
(let* ((file (string-append system "/parameters"))
(params (call-with-input-file file read-boot-parameters))
- (root (boot-parameters-root-device params)))
+ (root (boot-parameters-root-device params))
+ (version (boot-parameters-version params)))
(boot-parameters
(inherit params)
- (kernel-arguments (append (bootable-kernel-arguments system root)
+ (kernel-arguments (append (bootable-kernel-arguments system root version)
(boot-parameters-kernel-arguments params))))))
(define (boot-parameters->menu-entry conf)
@@ -1451,7 +1464,7 @@ (define* (operating-system-boot-parameters os root-device
#:key system-kernel-arguments?)
"Return a monadic <boot-parameters> record that describes the boot
parameters of OS. When SYSTEM-KERNEL-ARGUMENTS? is true, add kernel arguments
-such as '--root' and '--load' to <boot-parameters>."
+such as 'root' and 'gnu.load' to <boot-parameters>."
(let* ((initrd (and (not (operating-system-hurd os))
(operating-system-initrd-file os)))
(store (operating-system-store-file-system os))
@@ -1497,8 +1510,8 @@ (define* (operating-system-boot-parameters-file os
"Return a file that describes the boot parameters of OS. The primary use of
this file is the reconstruction of GRUB menu entries for old configurations.
-When SYSTEM-KERNEL-ARGUMENTS? is true, add kernel arguments such as '--root'
-and '--load' to the returned file (since the returned file is then usually
+When SYSTEM-KERNEL-ARGUMENTS? is true, add kernel arguments such as 'root'
+and 'gnu.load' to the returned file (since the returned file is then usually
stored into the content-addressed \"system\" directory, it's usually not a
good idea to give it because the content hash would change by the content hash
being stored into the \"parameters\" file)."
diff --git a/gnu/system/linux-initrd.scm b/gnu/system/linux-initrd.scm
index 329cd38cd6..4c4c78e444 100644
--- a/gnu/system/linux-initrd.scm
+++ b/gnu/system/linux-initrd.scm
@@ -181,7 +181,7 @@ (define* (raw-initrd file-systems
"Return as a file-like object a raw initrd, with kernel
modules taken from LINUX. FILE-SYSTEMS is a list of file-systems to be
mounted by the initrd, possibly in addition to the root file system specified
-on the kernel command line via '--root'. LINUX-MODULES is a list of kernel
+on the kernel command line via 'root'. LINUX-MODULES is a list of kernel
modules to be loaded at boot time. MAPPED-DEVICES is a list of device
mappings to realize before FILE-SYSTEMS are mounted.
HELPER-PACKAGES is a list of packages to be copied in the initrd. It may include
@@ -375,7 +375,7 @@ (define* (base-initrd file-systems
"Return as a file-like object a generic initrd, with kernel
modules taken from LINUX. FILE-SYSTEMS is a list of file-systems to be
mounted by the initrd, possibly in addition to the root file system specified
-on the kernel command line via '--root'. MAPPED-DEVICES is a list of device
+on the kernel command line via 'root'. MAPPED-DEVICES is a list of device
mappings to realize before FILE-SYSTEMS are mounted.
When true, KEYBOARD-LAYOUT is a <keyboard-layout> record denoting the desired
--
2.34.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* bug#40998: [PATCH v2 3/4] initrd: Honor rootfstype and rootflags command-line parameters.
2022-02-19 7:01 ` bug#40998: [PATCH v2 1/4] system: Add a version field to the <boot-parameters> record Maxim Cournoyer
2022-02-19 7:01 ` bug#40998: [PATCH v2 2/4] initrd: Use non-hyphenated kernel command-line parameter names Maxim Cournoyer
@ 2022-02-19 7:01 ` Maxim Cournoyer
2022-02-27 21:06 ` bug#40998: Guix System's initrd doesn't honor rootflags Ludovic Courtès
2022-02-19 7:01 ` bug#40998: [PATCH v2 4/4] initrd: Print its command-line parameters Maxim Cournoyer
2022-02-27 20:54 ` bug#40998: Guix System's initrd doesn't honor rootflags Ludovic Courtès
3 siblings, 1 reply; 19+ messages in thread
From: Maxim Cournoyer @ 2022-02-19 7:01 UTC (permalink / raw)
To: 40998; +Cc: Maxim Cournoyer
* gnu/build/linux-boot.scm (boot-system): Honor rootfstype and rootflags
arguments. Update doc. Error out in case there is insufficient information
with regard to the root file system.
Restore the behavior of inferring the root device from the root file system
from the operating system in case the root argument is not provided.
* doc/guix.texi (Initial RAM Disk): Document the new command-line parameters.
---
doc/guix.texi | 10 ++++++++
gnu/build/linux-boot.scm | 55 ++++++++++++++++++++++++++--------------
2 files changed, 46 insertions(+), 19 deletions(-)
diff --git a/doc/guix.texi b/doc/guix.texi
index d3f9216563..ec1e49630f 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -34915,6 +34915,16 @@ name like @code{/dev/sda1}, a file system label, or a file system UUID.
When unspecified, the device name from the root file system of the
operating system declaration is used.
+@item rootfstype=@var{type}
+Set the type of the root file system. It overrides the @code{type}
+field of the root file system specified via the @code{operating-system}
+declaration, if any.
+
+@item rootflags=@var{options}
+Set the mount @emph{options} of the root file system. It overrides the
+@code{options} field of the root file system specified via the
+@code{operating-system} declaration, if any.
+
@item fsck.mode=@var{mode}
Whether to check the @var{root} file system for errors before mounting
it. @var{mode} is one of @code{skip} (never check), @code{force} (always
diff --git a/gnu/build/linux-boot.scm b/gnu/build/linux-boot.scm
index 179fb12a2b..2f8b114806 100644
--- a/gnu/build/linux-boot.scm
+++ b/gnu/build/linux-boot.scm
@@ -499,9 +499,9 @@ (define* (boot-system #:key
KEYMAP-FILE is true), then setting up QEMU guest networking if
QEMU-GUEST-NETWORKING? is true, calling PRE-MOUNT, mounting the file systems
specified in MOUNTS, and finally booting into the new root if any. The initrd
-supports kernel command-line parameters 'gnu.load' and 'gnu.repl'. It also
+supports the kernel command-line options 'gnu.load' and 'gnu.repl'. It also
honors a subset of the Linux kernel command-line parameters such as
-'fsck.mode', 'resume', 'root' and 'rootdelay'.
+'fsck.mode', 'resume', 'rootdelay', rootflags and rootfstype.
Mount the root file system, specified by the 'root' command-line argument, if
any.
@@ -537,13 +537,30 @@ (define (device-string->file-system-device device-string)
;; over the ‘device’ field of the root <file-system> record.
(root-device (and=> (find-long-option "root" args)
device-string->file-system-device))
- (root-fs (or (find root-mount-point? mounts)
- ;; Fall back to fictitious defaults.
- (file-system (device (or root-device "/dev/root"))
- (mount-point "/")
- (type "ext4"))))
+ (rootfstype (find-long-option "rootfstype" args))
+ (rootflags (find-long-option "rootflags" args))
+ (root-fs* (find root-mount-point? mounts))
(fsck.mode (find-long-option "fsck.mode" args)))
+ (unless (or root-fs* (and root-device rootfstype))
+ (error "no root file system or 'root' and 'rootfstype' parameters"))
+
+ ;; If present, ‘root’ on the kernel command line takes precedence over
+ ;; the ‘device’ field of the root <file-system> record; likewise for
+ ;; the 'rootfstype' and 'rootflags' arguments.
+ (define root-fs
+ (if root-fs*
+ (file-system
+ (inherit root-fs*)
+ (device (or root-device (file-system-device root-fs*)))
+ (type (or rootfstype (file-system-type root-fs*)))
+ (options (or rootflags (file-system-options root-fs*))))
+ (file-system
+ (device root-device)
+ (mount-point "/")
+ (type rootfstype)
+ (options rootflags))))
+
(define (check? fs)
(match fsck.mode
("skip" #f)
@@ -615,18 +632,18 @@ (define (repair fs)
(setenv "EXT2FS_NO_MTAB_OK" "1")
- (if root-device
- (mount-root-file-system (canonicalize-device-spec root-device)
- (file-system-type root-fs)
- #:volatile-root? volatile-root?
- #:flags (mount-flags->bit-mask
- (file-system-flags root-fs))
- #:options (file-system-options root-fs)
- #:check? (check? root-fs)
- #:skip-check-if-clean?
- (skip-check-if-clean? root-fs)
- #:repair (repair root-fs))
- (mount "none" "/root" "tmpfs"))
+ ;; Mount the root file system.
+ (mount-root-file-system (canonicalize-device-spec
+ (file-system-device root-fs))
+ (file-system-type root-fs)
+ #:volatile-root? volatile-root?
+ #:flags (mount-flags->bit-mask
+ (file-system-flags root-fs))
+ #:options (file-system-options root-fs)
+ #:check? (check? root-fs)
+ #:skip-check-if-clean?
+ (skip-check-if-clean? root-fs)
+ #:repair (repair root-fs))
;; Mount the specified non-root file systems.
(for-each (lambda (fs)
--
2.34.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* bug#40998: [PATCH v2 4/4] initrd: Print its command-line parameters.
2022-02-19 7:01 ` bug#40998: [PATCH v2 1/4] system: Add a version field to the <boot-parameters> record Maxim Cournoyer
2022-02-19 7:01 ` bug#40998: [PATCH v2 2/4] initrd: Use non-hyphenated kernel command-line parameter names Maxim Cournoyer
2022-02-19 7:01 ` bug#40998: [PATCH v2 3/4] initrd: Honor rootfstype and rootflags command-line parameters Maxim Cournoyer
@ 2022-02-19 7:01 ` Maxim Cournoyer
2022-02-27 21:08 ` bug#40998: Guix System's initrd doesn't honor rootflags Ludovic Courtès
2022-02-28 3:45 ` bug#40998: [PATCH v2 4/4] initrd: Print its command-line parameters Tobias Geerinckx-Rice via Bug reports for GNU Guix
2022-02-27 20:54 ` bug#40998: Guix System's initrd doesn't honor rootflags Ludovic Courtès
3 siblings, 2 replies; 19+ messages in thread
From: Maxim Cournoyer @ 2022-02-19 7:01 UTC (permalink / raw)
To: 40998; +Cc: Maxim Cournoyer
* gnu/build/linux-boot.scm (boot-system): Print command-line parameters to
standard output.
---
gnu/build/linux-boot.scm | 2 ++
1 file changed, 2 insertions(+)
diff --git a/gnu/build/linux-boot.scm b/gnu/build/linux-boot.scm
index 2f8b114806..30442ec8f8 100644
--- a/gnu/build/linux-boot.scm
+++ b/gnu/build/linux-boot.scm
@@ -542,6 +542,8 @@ (define (device-string->file-system-device device-string)
(root-fs* (find root-mount-point? mounts))
(fsck.mode (find-long-option "fsck.mode" args)))
+ (format #t "initrd command-line parameters: ~a~%" args)
+
(unless (or root-fs* (and root-device rootfstype))
(error "no root file system or 'root' and 'rootfstype' parameters"))
--
2.34.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* bug#40998: Guix System's initrd doesn't honor rootflags
2022-02-19 7:01 ` bug#40998: [PATCH v2 1/4] system: Add a version field to the <boot-parameters> record Maxim Cournoyer
` (2 preceding siblings ...)
2022-02-19 7:01 ` bug#40998: [PATCH v2 4/4] initrd: Print its command-line parameters Maxim Cournoyer
@ 2022-02-27 20:54 ` Ludovic Courtès
2022-02-28 20:02 ` Maxim Cournoyer
3 siblings, 1 reply; 19+ messages in thread
From: Ludovic Courtès @ 2022-02-27 20:54 UTC (permalink / raw)
To: Maxim Cournoyer; +Cc: 40998
Hi!
Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
> This version field exposes the (already present) version information of a boot
> parameters file.
>
> * gnu/system.scm (%boot-parameters-version): New variable.
> (<boot-parameters>)[version]: New field.
> (read-boot-parameters): Use it.
> (operating-system-boot-parameters-file): Likewise.
> * tests/boot-parameters.scm (test-read-boot-parameters): Use
> %boot-parameters-version as the default version value in the template.
[...]
> (define-record-type* <boot-parameters>
> boot-parameters make-boot-parameters boot-parameters?
> (label boot-parameters-label)
> @@ -322,7 +326,9 @@ (define-record-type* <boot-parameters>
> (kernel boot-parameters-kernel)
> (kernel-arguments boot-parameters-kernel-arguments)
> (initrd boot-parameters-initrd)
> - (multiboot-modules boot-parameters-multiboot-modules))
> + (multiboot-modules boot-parameters-multiboot-modules)
> + (version boot-parameters-version ;positive integer
> + (default %boot-parameters-version)))
[...]
> (match (read port)
> - (('boot-parameters ('version 0)
> + (('boot-parameters ('version (? version? version))
> ('label label) ('root-device root)
> ('kernel kernel)
> rest ...)
> (boot-parameters
> + (version version)
> (label label)
> (root-device (device-sexp->device root))
There’s no need to have a ‘version’ field in live <boot-parameters>
records: have the ‘version’ field in the serialized format (the sexp)
and make sure the deserializer correctly converts to the internal
representation.
Here, I think you can bump the version number in the serialized form,
and have ‘read-boot-parameters’ automatically augment ‘kernel-arguments’
when VERSION is 0 with “--root=XYZ”.
(It might be that you can even do that without bumping the version
number. Bumping is clearer but the downside is that an older Guix will
abort when attempting to read ‘parameters’. This could happen if you
roll back to an earlier generation and try to run ‘guix system
reconfigure’ or similar from there.)
Also, you could write the ‘match’ pattern like this:
('boot-parameters ('version (and version (or 0 1)))
('label label) …)
I hope that makes sense!
Ludo’.
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#40998: Guix System's initrd doesn't honor rootflags
2022-02-19 7:01 ` bug#40998: [PATCH v2 2/4] initrd: Use non-hyphenated kernel command-line parameter names Maxim Cournoyer
@ 2022-02-27 21:03 ` Ludovic Courtès
2022-02-28 20:31 ` Maxim Cournoyer
0 siblings, 1 reply; 19+ messages in thread
From: Ludovic Courtès @ 2022-02-27 21:03 UTC (permalink / raw)
To: Maxim Cournoyer; +Cc: 40998
Hi,
Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
> -(define (bootable-kernel-arguments system root-device)
> - "Return a list of kernel arguments (gexps) to boot SYSTEM from ROOT-DEVICE."
> - (list (string-append "--root="
> +(define* (bootable-kernel-arguments system root-device version)
> + "Return a list of kernel arguments (gexps) to boot SYSTEM from ROOT-DEVICE.
> +VERSION is the target version of the boot-parameters record."
> + ;; If the version is newer than 0, we use the new style initrd parameter
> + ;; names, otherwise we use the legacy ones. This is to maintain backward
> + ;; compatibility when producing bootloader configurations for older
> + ;; generations.
> + (define version>0? (> version 0))
> + (list (string-append (if version>0? "root=" "--root=")
> ;; Note: Always use the DCE format because that's what
> - ;; (gnu build linux-boot) expects for the '--root'
> + ;; (gnu build linux-boot) expects for the 'root'
> ;; kernel command-line option.
> (file-system-device->string root-device
> #:uuid-type 'dce))
> - #~(string-append "--system=" #$system)
> - #~(string-append "--load=" #$system "/boot")))
> + #~(string-append (if #$version>0? "gnu.system=" "--system=") #$system)
> + #~(string-append (if #$version>0? "gnu.load=" "--load=")
> + #$system "/boot")))
This is the logic I was suggesting to move to ‘read-boot-parameters’.
To do that, ‘read-boot-parameters’ would have to fill the
‘kernel-arguments’ field of <boot-parameters> such that it already
contains --system/gnu.system and --load/gnu.load.
And then ‘read-boot-parameters-file’ would become:
(call-with-input-file (string-append system "/parameters")
read-boot-parameters)
without the post-processing step it’s currently doing.
(And the version number doesn’t need to flow beyond
‘read-boot-parameters’.)
WDYT?
Ludo’.
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#40998: Guix System's initrd doesn't honor rootflags
2022-02-19 7:01 ` bug#40998: [PATCH v2 3/4] initrd: Honor rootfstype and rootflags command-line parameters Maxim Cournoyer
@ 2022-02-27 21:06 ` Ludovic Courtès
0 siblings, 0 replies; 19+ messages in thread
From: Ludovic Courtès @ 2022-02-27 21:06 UTC (permalink / raw)
To: Maxim Cournoyer; +Cc: 40998
Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
> * gnu/build/linux-boot.scm (boot-system): Honor rootfstype and rootflags
> arguments. Update doc. Error out in case there is insufficient information
> with regard to the root file system.
> Restore the behavior of inferring the root device from the root file system
> from the operating system in case the root argument is not provided.
> * doc/guix.texi (Initial RAM Disk): Document the new command-line parameters.
Very nice!
[...]
> - (if root-device
> - (mount-root-file-system (canonicalize-device-spec root-device)
> - (file-system-type root-fs)
> - #:volatile-root? volatile-root?
> - #:flags (mount-flags->bit-mask
> - (file-system-flags root-fs))
> - #:options (file-system-options root-fs)
> - #:check? (check? root-fs)
> - #:skip-check-if-clean?
> - (skip-check-if-clean? root-fs)
> - #:repair (repair root-fs))
> - (mount "none" "/root" "tmpfs"))
> + ;; Mount the root file system.
> + (mount-root-file-system (canonicalize-device-spec
I think the ‘if’ had (has?) a justification but I’m not sure what that
was. It’s likely nobody will notice if we remove it anyway. :-)
Ludo’.
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#40998: Guix System's initrd doesn't honor rootflags
2022-02-19 7:01 ` bug#40998: [PATCH v2 4/4] initrd: Print its command-line parameters Maxim Cournoyer
@ 2022-02-27 21:08 ` Ludovic Courtès
2022-02-28 20:36 ` Maxim Cournoyer
2022-02-28 3:45 ` bug#40998: [PATCH v2 4/4] initrd: Print its command-line parameters Tobias Geerinckx-Rice via Bug reports for GNU Guix
1 sibling, 1 reply; 19+ messages in thread
From: Ludovic Courtès @ 2022-02-27 21:08 UTC (permalink / raw)
To: Maxim Cournoyer; +Cc: 40998
Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
> * gnu/build/linux-boot.scm (boot-system): Print command-line parameters to
> standard output.
> ---
> gnu/build/linux-boot.scm | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/gnu/build/linux-boot.scm b/gnu/build/linux-boot.scm
> index 2f8b114806..30442ec8f8 100644
> --- a/gnu/build/linux-boot.scm
> +++ b/gnu/build/linux-boot.scm
> @@ -542,6 +542,8 @@ (define (device-string->file-system-device device-string)
> (root-fs* (find root-mount-point? mounts))
> (fsck.mode (find-long-option "fsck.mode" args)))
>
> + (format #t "initrd command-line parameters: ~a~%" args)
I’d happily avoid being too talkative by default, but maybe we could
honor a ‘gnu.verbosity’ parameter to get different levels of early-boot
logging? :-)
Anyway, it’s a nice patch series.
Thanks,
Ludo’.
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#40998: [PATCH v2 4/4] initrd: Print its command-line parameters.
2022-02-19 7:01 ` bug#40998: [PATCH v2 4/4] initrd: Print its command-line parameters Maxim Cournoyer
2022-02-27 21:08 ` bug#40998: Guix System's initrd doesn't honor rootflags Ludovic Courtès
@ 2022-02-28 3:45 ` Tobias Geerinckx-Rice via Bug reports for GNU Guix
2022-02-28 15:54 ` Maxim Cournoyer
1 sibling, 1 reply; 19+ messages in thread
From: Tobias Geerinckx-Rice via Bug reports for GNU Guix @ 2022-02-28 3:45 UTC (permalink / raw)
To: Maxim Cournoyer, 40998
On 19 February 2022 07:01:55 UTC, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>* gnu/build/linux-boot.scm (boot-system): Print command-line parameters to
>standard output.
>---
> gnu/build/linux-boot.scm | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/gnu/build/linux-boot.scm b/gnu/build/linux-boot.scm
>index 2f8b114806..30442ec8f8 100644
>--- a/gnu/build/linux-boot.scm
>+++ b/gnu/build/linux-boot.scm
>@@ -542,6 +542,8 @@ (define (device-string->file-system-device device-string)
> (root-fs* (find root-mount-point? mounts))
> (fsck.mode (find-long-option "fsck.mode" args)))
>
>+ (format #t "initrd command-line parameters: ~a~%" args)
>+
> (unless (or root-fs* (and root-device rootfstype))
> (error "no root file system or 'root' and 'rootfstype' parameters"))
>
I suggest dropping this patch. The kernel already does this. It's just noise.
I've installed the others here. Thanks!
Kind regards,
T G-R
Sent on the go. Excuse or enjoy my brevity.
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#40998: [PATCH v2 4/4] initrd: Print its command-line parameters.
2022-02-28 3:45 ` bug#40998: [PATCH v2 4/4] initrd: Print its command-line parameters Tobias Geerinckx-Rice via Bug reports for GNU Guix
@ 2022-02-28 15:54 ` Maxim Cournoyer
0 siblings, 0 replies; 19+ messages in thread
From: Maxim Cournoyer @ 2022-02-28 15:54 UTC (permalink / raw)
To: Tobias Geerinckx-Rice; +Cc: 40998
Hi,
Tobias Geerinckx-Rice <me@tobias.gr> writes:
> On 19 February 2022 07:01:55 UTC, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>>* gnu/build/linux-boot.scm (boot-system): Print command-line parameters to
>>standard output.
>>---
>> gnu/build/linux-boot.scm | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>>diff --git a/gnu/build/linux-boot.scm b/gnu/build/linux-boot.scm
>>index 2f8b114806..30442ec8f8 100644
>>--- a/gnu/build/linux-boot.scm
>>+++ b/gnu/build/linux-boot.scm
>>@@ -542,6 +542,8 @@ (define (device-string->file-system-device device-string)
>> (root-fs* (find root-mount-point? mounts))
>> (fsck.mode (find-long-option "fsck.mode" args)))
>>
>>+ (format #t "initrd command-line parameters: ~a~%" args)
>>+
>> (unless (or root-fs* (and root-device rootfstype))
>> (error "no root file system or 'root' and 'rootfstype' parameters"))
>>
>
> I suggest dropping this patch. The kernel already does this. It's just noise.
I thought about adding it after attempting to debug initrd/kernel
problems as can be seen here: https://elephly.net/paste/1645124517.html
(this is the Berlin boot log copied from a serial console). Not that
the initrd printing it itself would have helped here, as it appears to
be truncated from when Linux took over the initrd script, if I
understand correctly.
Anyway, I agree it's probably redundant and useless in all but more
desperate debugging cases :-), so I don't mind to drop it.
Thanks,
Maxim
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#40998: Guix System's initrd doesn't honor rootflags
2022-02-27 20:54 ` bug#40998: Guix System's initrd doesn't honor rootflags Ludovic Courtès
@ 2022-02-28 20:02 ` Maxim Cournoyer
2022-02-28 21:15 ` Ludovic Courtès
0 siblings, 1 reply; 19+ messages in thread
From: Maxim Cournoyer @ 2022-02-28 20:02 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: 40998
Hi Ludovic!
Ludovic Courtès <ludo@gnu.org> writes:
> Hi!
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> This version field exposes the (already present) version information of a boot
>> parameters file.
>>
>> * gnu/system.scm (%boot-parameters-version): New variable.
>> (<boot-parameters>)[version]: New field.
>> (read-boot-parameters): Use it.
>> (operating-system-boot-parameters-file): Likewise.
>> * tests/boot-parameters.scm (test-read-boot-parameters): Use
>> %boot-parameters-version as the default version value in the template.
>
> [...]
>
>> (define-record-type* <boot-parameters>
>> boot-parameters make-boot-parameters boot-parameters?
>> (label boot-parameters-label)
>> @@ -322,7 +326,9 @@ (define-record-type* <boot-parameters>
>> (kernel boot-parameters-kernel)
>> (kernel-arguments boot-parameters-kernel-arguments)
>> (initrd boot-parameters-initrd)
>> - (multiboot-modules boot-parameters-multiboot-modules))
>> + (multiboot-modules boot-parameters-multiboot-modules)
>> + (version boot-parameters-version ;positive integer
>> + (default %boot-parameters-version)))
>
> [...]
>
>> (match (read port)
>> - (('boot-parameters ('version 0)
>> + (('boot-parameters ('version (? version? version))
>> ('label label) ('root-device root)
>> ('kernel kernel)
>> rest ...)
>> (boot-parameters
>> + (version version)
>> (label label)
>> (root-device (device-sexp->device root))
>
> There’s no need to have a ‘version’ field in live <boot-parameters>
> records: have the ‘version’ field in the serialized format (the sexp)
> and make sure the deserializer correctly converts to the internal
> representation.
>
> Here, I think you can bump the version number in the serialized form,
> and have ‘read-boot-parameters’ automatically augment ‘kernel-arguments’
> when VERSION is 0 with “--root=XYZ”.
I initially went that route, as this was the idea you'd given me
initially. However, 'read-boot-parameters' deals with serializing the
parameters file only; to add 'root', 'gnu.load' and 'gnu.system', the
operating-system object as well as the root device are needed. The
reason 'gnu.load' and 'gnu.system' aren't written to the parameters file
to start with is because they would cause the system directory to no
longer be content-addressable; this is explained in the docstring of
'operating-system-boot-parameters-file':
When SYSTEM-KERNEL-ARGUMENTS? is true, add kernel arguments such as 'root'
and 'gnu.load' to the returned file (since the returned file is then usually
stored into the content-addressed "system" directory, it's usually not a
good idea to give it because the content hash would change by the content hash
being stored into the "parameters" file).
So, unless we were to pack 'read-boot-parameters' with unrelated duties
and a 'system-kernel-arguments?' argument, it seems unavoidable to
expose the 'version' metadata.
> (It might be that you can even do that without bumping the version
> number. Bumping is clearer but the downside is that an older Guix will
> abort when attempting to read ‘parameters’. This could happen if you
> roll back to an earlier generation and try to run ‘guix system
> reconfigure’ or similar from there.)
>
> Also, you could write the ‘match’ pattern like this:
>
> ('boot-parameters ('version (and version (or 0 1)))
> ('label label) …)
I think this patch's current form is preferable, as it means future
boot-parameters version bumps won't break older Guices (when
reconfiguring), as long as the version is an exact, non-negative integer
(e.g. when going from 1 to 2).
Let me know what you think!
Thank you,
Maxim
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#40998: Guix System's initrd doesn't honor rootflags
2022-02-27 21:03 ` bug#40998: Guix System's initrd doesn't honor rootflags Ludovic Courtès
@ 2022-02-28 20:31 ` Maxim Cournoyer
0 siblings, 0 replies; 19+ messages in thread
From: Maxim Cournoyer @ 2022-02-28 20:31 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: 40998
Hi Ludovic,
Ludovic Courtès <ludo@gnu.org> writes:
> Hi,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> -(define (bootable-kernel-arguments system root-device)
>> - "Return a list of kernel arguments (gexps) to boot SYSTEM from ROOT-DEVICE."
>> - (list (string-append "--root="
>> +(define* (bootable-kernel-arguments system root-device version)
>> + "Return a list of kernel arguments (gexps) to boot SYSTEM from ROOT-DEVICE.
>> +VERSION is the target version of the boot-parameters record."
>> + ;; If the version is newer than 0, we use the new style initrd parameter
>> + ;; names, otherwise we use the legacy ones. This is to maintain backward
>> + ;; compatibility when producing bootloader configurations for older
>> + ;; generations.
>> + (define version>0? (> version 0))
>> + (list (string-append (if version>0? "root=" "--root=")
>> ;; Note: Always use the DCE format because that's what
>> - ;; (gnu build linux-boot) expects for the '--root'
>> + ;; (gnu build linux-boot) expects for the 'root'
>> ;; kernel command-line option.
>> (file-system-device->string root-device
>> #:uuid-type 'dce))
>> - #~(string-append "--system=" #$system)
>> - #~(string-append "--load=" #$system "/boot")))
>> + #~(string-append (if #$version>0? "gnu.system=" "--system=") #$system)
>> + #~(string-append (if #$version>0? "gnu.load=" "--load=")
>> + #$system "/boot")))
>
> This is the logic I was suggesting to move to ‘read-boot-parameters’.
>
> To do that, ‘read-boot-parameters’ would have to fill the
> ‘kernel-arguments’ field of <boot-parameters> such that it already
> contains --system/gnu.system and --load/gnu.load.
As mentioned in my previous reply, that'd only be possible if we no
longer cared about having the produced system directory
content-addressable, if I understood that comment in the docstring
referred earlier correctly.
Thanks,
Maxim
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#40998: Guix System's initrd doesn't honor rootflags
2022-02-27 21:08 ` bug#40998: Guix System's initrd doesn't honor rootflags Ludovic Courtès
@ 2022-02-28 20:36 ` Maxim Cournoyer
0 siblings, 0 replies; 19+ messages in thread
From: Maxim Cournoyer @ 2022-02-28 20:36 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: 40998
Hi again,
Ludovic Courtès <ludo@gnu.org> writes:
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> * gnu/build/linux-boot.scm (boot-system): Print command-line parameters to
>> standard output.
>> ---
>> gnu/build/linux-boot.scm | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/gnu/build/linux-boot.scm b/gnu/build/linux-boot.scm
>> index 2f8b114806..30442ec8f8 100644
>> --- a/gnu/build/linux-boot.scm
>> +++ b/gnu/build/linux-boot.scm
>> @@ -542,6 +542,8 @@ (define (device-string->file-system-device device-string)
>> (root-fs* (find root-mount-point? mounts))
>> (fsck.mode (find-long-option "fsck.mode" args)))
>>
>> + (format #t "initrd command-line parameters: ~a~%" args)
>
> I’d happily avoid being too talkative by default, but maybe we could
> honor a ‘gnu.verbosity’ parameter to get different levels of early-boot
> logging? :-)
Eh, I've dropped it for now; I attribute my perceived need for it due to
the odd berlin boot output as seen from the serial console [0], which
was missing it (it appears to be truncated).
Thank you!
Maxim
[0] https://elephly.net/paste/1645131411.html
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#40998: Guix System's initrd doesn't honor rootflags
2022-02-28 20:02 ` Maxim Cournoyer
@ 2022-02-28 21:15 ` Ludovic Courtès
2022-02-28 21:45 ` Maxim Cournoyer
0 siblings, 1 reply; 19+ messages in thread
From: Ludovic Courtès @ 2022-02-28 21:15 UTC (permalink / raw)
To: Maxim Cournoyer; +Cc: 40998
Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
> Ludovic Courtès <ludo@gnu.org> writes:
[...]
>> There’s no need to have a ‘version’ field in live <boot-parameters>
>> records: have the ‘version’ field in the serialized format (the sexp)
>> and make sure the deserializer correctly converts to the internal
>> representation.
>>
>> Here, I think you can bump the version number in the serialized form,
>> and have ‘read-boot-parameters’ automatically augment ‘kernel-arguments’
>> when VERSION is 0 with “--root=XYZ”.
>
> I initially went that route, as this was the idea you'd given me
> initially. However, 'read-boot-parameters' deals with serializing the
> parameters file only; to add 'root', 'gnu.load' and 'gnu.system', the
> operating-system object as well as the root device are needed.
<boot-parameters> already has ‘root-device’, so that’s fine.
But you’re right that the system itself is a problem because that’s
self-referential—it’s the thing the “parameters” file is in. Hmm!
We could add a substitution mechanism where a literal “$SYSTEM” (say) in
the ‘kernel-arguments’ of <boot-parameters> would be substituted by the
actual system store file name by ‘read-boot-parameters’, but maybe
that’s overkill.
So long story short: keeping the ‘version’ field in <boot-parameters>
sounds reasonable after all. :-)
> The reason 'gnu.load' and 'gnu.system' aren't written to the
> parameters file to start with is because they would cause the system
> directory to no longer be content-addressable; this is explained in
> the docstring of 'operating-system-boot-parameters-file':
>
> When SYSTEM-KERNEL-ARGUMENTS? is true, add kernel arguments such as 'root'
> and 'gnu.load' to the returned file (since the returned file is then usually
> stored into the content-addressed "system" directory, it's usually not a
> good idea to give it because the content hash would change by the content hash
> being stored into the "parameters" file).
This comment originates in 40fad1c24ce60076e26f6dc8096e4716d31d90c3. I
find it a bit misleading because nothing’s “content-addressed”, but I
guess it refers to the same problem: that this is self-referential.
(There’s only one use of #:system-kernel-arguments? #t. We can remove
that keyword parameter from ‘operating-system-boot-parameters-file’
since it’s not used there.)
>> Also, you could write the ‘match’ pattern like this:
>>
>> ('boot-parameters ('version (and version (or 0 1)))
>> ('label label) …)
>
> I think this patch's current form is preferable, as it means future
> boot-parameters version bumps won't break older Guices (when
> reconfiguring), as long as the version is an exact, non-negative integer
> (e.g. when going from 1 to 2).
That’s what we want to avoid: bumping the version number means that the
new format is not backwards-compatible, and that older Guix versions
won’t be able to read it. That’s why I think ‘read-boot-parameters’
needs to be explicit about the version(s) it expects. (A more complete
example of this pattern is ‘sexp->manifest’.)
Breaking backwards compatibility should be avoided when possible, but
it’s not always possible. In ‘read-boot-parameters’, ‘bootloader-name’,
‘bootloader-menu-entries’, ‘kernel’, etc. are handled somewhat weirdly
to preserve backwards-compatibility; doing this allowed us to not bump
the file format version.
Thanks!
Ludo’.
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#40998: Guix System's initrd doesn't honor rootflags
2022-02-28 21:15 ` Ludovic Courtès
@ 2022-02-28 21:45 ` Maxim Cournoyer
0 siblings, 0 replies; 19+ messages in thread
From: Maxim Cournoyer @ 2022-02-28 21:45 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: 40998
Hi Ludovic,
Ludovic Courtès <ludo@gnu.org> writes:
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> Ludovic Courtès <ludo@gnu.org> writes:
>
> [...]
>
>>> There’s no need to have a ‘version’ field in live <boot-parameters>
>>> records: have the ‘version’ field in the serialized format (the sexp)
>>> and make sure the deserializer correctly converts to the internal
>>> representation.
>>>
>>> Here, I think you can bump the version number in the serialized form,
>>> and have ‘read-boot-parameters’ automatically augment ‘kernel-arguments’
>>> when VERSION is 0 with “--root=XYZ”.
>>
>> I initially went that route, as this was the idea you'd given me
>> initially. However, 'read-boot-parameters' deals with serializing the
>> parameters file only; to add 'root', 'gnu.load' and 'gnu.system', the
>> operating-system object as well as the root device are needed.
>
> <boot-parameters> already has ‘root-device’, so that’s fine.
>
> But you’re right that the system itself is a problem because that’s
> self-referential—it’s the thing the “parameters” file is in. Hmm!
>
> We could add a substitution mechanism where a literal “$SYSTEM” (say) in
> the ‘kernel-arguments’ of <boot-parameters> would be substituted by the
> actual system store file name by ‘read-boot-parameters’, but maybe
> that’s overkill.
>
> So long story short: keeping the ‘version’ field in <boot-parameters>
> sounds reasonable after all. :-)
OK, good, thanks for having confirmed that.
>> The reason 'gnu.load' and 'gnu.system' aren't written to the
>> parameters file to start with is because they would cause the system
>> directory to no longer be content-addressable; this is explained in
>> the docstring of 'operating-system-boot-parameters-file':
>>
>> When SYSTEM-KERNEL-ARGUMENTS? is true, add kernel arguments such as 'root'
>> and 'gnu.load' to the returned file (since the returned file is then usually
>> stored into the content-addressed "system" directory, it's usually not a
>> good idea to give it because the content hash would change by the content hash
>> being stored into the "parameters" file).
>
> This comment originates in 40fad1c24ce60076e26f6dc8096e4716d31d90c3. I
> find it a bit misleading because nothing’s “content-addressed”, but I
> guess it refers to the same problem: that this is self-referential.
>
> (There’s only one use of #:system-kernel-arguments? #t. We can remove
> that keyword parameter from ‘operating-system-boot-parameters-file’
> since it’s not used there.)
OK, I'll address this confusing comment and extraneous argument in a
separate commit.
>>> Also, you could write the ‘match’ pattern like this:
>>>
>>> ('boot-parameters ('version (and version (or 0 1)))
>>> ('label label) …)
>>
>> I think this patch's current form is preferable, as it means future
>> boot-parameters version bumps won't break older Guices (when
>> reconfiguring), as long as the version is an exact, non-negative integer
>> (e.g. when going from 1 to 2).
>
> That’s what we want to avoid: bumping the version number means that the
> new format is not backwards-compatible, and that older Guix versions
> won’t be able to read it. That’s why I think ‘read-boot-parameters’
> needs to be explicit about the version(s) it expects. (A more complete
> example of this pattern is ‘sexp->manifest’.)
>
> Breaking backwards compatibility should be avoided when possible, but
> it’s not always possible. In ‘read-boot-parameters’, ‘bootloader-name’,
> ‘bootloader-menu-entries’, ‘kernel’, etc. are handled somewhat weirdly
> to preserve backwards-compatibility; doing this allowed us to not bump
> the file format version.
Thanks for explaining! I initially thought the choice to break backward
compatibility could be left to the implementer of a new version, but now
I see this wouldn't work or be brittle.
I've modified the first commit like so:
--8<---------------cut here---------------start------------->8---
modified gnu/system.scm
@@ -365,8 +365,10 @@ (define uuid-sexp->uuid
(warning (G_ "unrecognized uuid ~a at '~a'~%") x (port-filename port))
#f)))
+ ;; New versions are not backward-compatible, so only accept past and current
+ ;; versions, not future ones.
(define (version? n)
- (and (exact-integer? n) (not (negative? n))))
+ (member n (iota (1+ %boot-parameters-version))))
(match (read port)
(('boot-parameters ('version (? version? version))
--8<---------------cut here---------------end--------------->8---
So that there's only one thing to update when we'll bump the version
again in the future (%boot-parameters-version).
I'll be sending a v3 shortly, thanks again!
Maxim
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2022-02-28 21:46 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-05-01 13:53 bug#40998: Guix System's initrd doesn't honor rootflags maxim.cournoyer
2022-02-18 15:33 ` bug#40998: [PATCH 1/3] initrd: Rename the '--root' initrd option to just 'root' Maxim Cournoyer
2022-02-18 15:33 ` bug#40998: [PATCH 2/3] initrd: Honor rootfstype and rootflags command-line parameters Maxim Cournoyer
2022-02-18 15:33 ` bug#40998: [PATCH 3/3] initrd: Print its " Maxim Cournoyer
2022-02-19 7:01 ` bug#40998: [PATCH v2 1/4] system: Add a version field to the <boot-parameters> record Maxim Cournoyer
2022-02-19 7:01 ` bug#40998: [PATCH v2 2/4] initrd: Use non-hyphenated kernel command-line parameter names Maxim Cournoyer
2022-02-27 21:03 ` bug#40998: Guix System's initrd doesn't honor rootflags Ludovic Courtès
2022-02-28 20:31 ` Maxim Cournoyer
2022-02-19 7:01 ` bug#40998: [PATCH v2 3/4] initrd: Honor rootfstype and rootflags command-line parameters Maxim Cournoyer
2022-02-27 21:06 ` bug#40998: Guix System's initrd doesn't honor rootflags Ludovic Courtès
2022-02-19 7:01 ` bug#40998: [PATCH v2 4/4] initrd: Print its command-line parameters Maxim Cournoyer
2022-02-27 21:08 ` bug#40998: Guix System's initrd doesn't honor rootflags Ludovic Courtès
2022-02-28 20:36 ` Maxim Cournoyer
2022-02-28 3:45 ` bug#40998: [PATCH v2 4/4] initrd: Print its command-line parameters Tobias Geerinckx-Rice via Bug reports for GNU Guix
2022-02-28 15:54 ` Maxim Cournoyer
2022-02-27 20:54 ` bug#40998: Guix System's initrd doesn't honor rootflags Ludovic Courtès
2022-02-28 20:02 ` Maxim Cournoyer
2022-02-28 21:15 ` Ludovic Courtès
2022-02-28 21:45 ` Maxim Cournoyer
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).