Previously, the accessor for the field "kernel-arguments" in the structure <operating-system> was called "operating-system-user-kernel-arguments". The procedure "operating-system-kernel-arguments" made sure to add arguments that made the system boot from a given device. After some reflection I think I was mistaken in that. It's nicer if the accessor is called "operating-system-kernel-argmuents" and if the users just use "bootable-kernel-arguments" on their own in order to amend them. That's what this patch does. Danny Milosavljevic (2): system: Inline operating-system-kernel-arguments. system: Rename operating-system-user-kernel-arguments to operating-system-kernel-arguments. gnu/system.scm | 19 ++++++++----------- gnu/system/vm.scm | 4 +++- 2 files changed, 11 insertions(+), 12 deletions(-)
* gnu/system.scm (operating-system-kernel-arguments): Remove. (bootable-kernel-arguments): Export. (read-boot-parameters-file): Use bootable-kernel-arguments. * gnu/system/vm.scm (system-qemu-image/shared-store-script): Use bootable-kernel-arguments. --- gnu/system.scm | 15 ++++++--------- gnu/system/vm.scm | 4 +++- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/gnu/system.scm b/gnu/system.scm index df89ca06d..42e0c37c1 100644 --- a/gnu/system.scm +++ b/gnu/system.scm @@ -117,7 +117,9 @@ local-host-aliases %setuid-programs %base-packages - %base-firmware)) + %base-firmware + + bootable-kernel-arguments)) ;;; Commentary: ;;; @@ -200,13 +202,6 @@ booted from ROOT-DEVICE" (sudoers-file operating-system-sudoers-file ; file-like (default %sudoers-specification))) -(define (operating-system-kernel-arguments os system.drv root-device) - "Return all the kernel arguments, including the ones not specified -directly by the user." - (bootable-kernel-arguments (operating-system-user-kernel-arguments os) - system.drv - root-device)) - \f ;;; ;;; Boot parameters @@ -940,7 +935,9 @@ kernel arguments for that derivation to <boot-parameters>." (kernel (operating-system-kernel-file os)) (kernel-arguments (if system.drv - (operating-system-kernel-arguments os system.drv root-device) + (bootable-kernel-arguments + (operating-system-user-kernel-arguments os) + system.drv root-device) (operating-system-user-kernel-arguments os))) (initrd initrd) (bootloader-name bootloader-name) diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm index 53629daa9..323eceac5 100644 --- a/gnu/system/vm.scm +++ b/gnu/system/vm.scm @@ -704,7 +704,9 @@ it is mostly useful when FULL-BOOT? is true." #:disk-image-size disk-image-size))) (define kernel-arguments #~(list #$@(if graphic? #~() #~("console=ttyS0")) - #+@(operating-system-kernel-arguments os os-drv "/dev/vda1"))) + #+@(bootable-kernel-arguments + (operating-system-user-kernel-arguments os) + os-drv "/dev/vda1"))) (define qemu-exec #~(list (string-append #$qemu "/bin/" #$(qemu-command (%current-system)))
* gnu/system.scm (operating-system-user-kernel-arguments): Rename to ... (operating-system-kernel-arguments): ... this. * gnu/system/vm.scm (system-qemu-image/shared-store-script): Use operating-system-kernel-arguments. --- gnu/system.scm | 6 +++--- gnu/system/vm.scm | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/gnu/system.scm b/gnu/system.scm index 42e0c37c1..79e3facee 100644 --- a/gnu/system.scm +++ b/gnu/system.scm @@ -149,7 +149,7 @@ booted from ROOT-DEVICE" operating-system? (kernel operating-system-kernel ; package (default linux-libre)) - (kernel-arguments operating-system-user-kernel-arguments + (kernel-arguments operating-system-kernel-arguments (default '())) ; list of gexps/strings (bootloader operating-system-bootloader) ; <bootloader-configuration> @@ -936,9 +936,9 @@ kernel arguments for that derivation to <boot-parameters>." (kernel-arguments (if system.drv (bootable-kernel-arguments - (operating-system-user-kernel-arguments os) + (operating-system-kernel-arguments os) system.drv root-device) - (operating-system-user-kernel-arguments os))) + (operating-system-kernel-arguments os))) (initrd initrd) (bootloader-name bootloader-name) (store-device (ensure-not-/dev (fs->boot-device store))) diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm index 323eceac5..13c1557df 100644 --- a/gnu/system/vm.scm +++ b/gnu/system/vm.scm @@ -705,7 +705,7 @@ it is mostly useful when FULL-BOOT? is true." (define kernel-arguments #~(list #$@(if graphic? #~() #~("console=ttyS0")) #+@(bootable-kernel-arguments - (operating-system-user-kernel-arguments os) + (operating-system-kernel-arguments os) os-drv "/dev/vda1"))) (define qemu-exec
Hello,
Danny Milosavljevic <dannym@scratchpost.org> skribis:
> Previously, the accessor for the field "kernel-arguments" in the structure
> <operating-system> was called "operating-system-user-kernel-arguments".
>
> The procedure "operating-system-kernel-arguments" made sure to add arguments
> that made the system boot from a given device.
>
> After some reflection I think I was mistaken in that.
>
> It's nicer if the accessor is called "operating-system-kernel-argmuents"
> and if the users just use "bootable-kernel-arguments" on their own in order to
> amend them.
>
> That's what this patch does.
I find ‘bootable-kernel-arguments’ to be quite unusual for a public
interface.
It’d feel more idiomatic to me if, instead, we had an
‘operating-system-boot-kernel-arguments’ procedure that takes an OS and
returns (list --root --system …). Then it’d be up to the caller to
append that to what ‘operating-system-kernel-arguments’ returns.
WDYT?
Thanks,
Ludo’.
Hi Ludo,
On Mon, 08 Jan 2018 10:26:54 +0100
ludo@gnu.org (Ludovic Courtès) wrote:
> Danny Milosavljevic <dannym@scratchpost.org> skribis:
>
> > Previously, the accessor for the field "kernel-arguments" in the structure
> > <operating-system> was called "operating-system-user-kernel-arguments".
> >
> > The procedure "operating-system-kernel-arguments" made sure to add arguments
> > that made the system boot from a given device.
> >
> > After some reflection I think I was mistaken in that.
> >
> > It's nicer if the accessor is called "operating-system-kernel-argmuents"
> > and if the users just use "bootable-kernel-arguments" on their own in order to
> > amend them.
> >
> > That's what this patch does.
>
> I find ‘bootable-kernel-arguments’ to be quite unusual for a public
> interface.
>
> It’d feel more idiomatic to me if, instead, we had an
> ‘operating-system-boot-kernel-arguments’ procedure that takes an OS and
> returns (list --root --system …). Then it’d be up to the caller to
> append that to what ‘operating-system-kernel-arguments’ returns.
Yeah, but looking at it some more, it doesn't really need an OS. It needs the system derivation (and root device).
Do we still call it "operating-system-..." when it won't get an OS (or anything from it) as parameter?
What it does now is
(define (bootable-kernel-arguments kernel-arguments system.drv root-device)
"Prepend extra arguments to KERNEL-ARGUMENTS that allow SYSTEM.DRV to be
booted from ROOT-DEVICE"
(cons* (string-append "--root="
(if (uuid? root-device)
;; Note: Always use the DCE format because that's
;; what (gnu build linux-boot) expects for the
;; '--root' kernel command-line option.
(uuid->string (uuid-bytevector root-device) 'dce)
root-device))
#~(string-append "--system=" #$system.drv)
#~(string-append "--load=" #$system.drv "/boot")
kernel-arguments))
We could make it do
(define (bootable-kernel-arguments* system.drv root-device)
"Return extra boot arguments that allow SYSTEM.DRV to be
booted from ROOT-DEVICE"
(list (string-append "--root="
(if (uuid? root-device)
;; Note: Always use the DCE format because that's
;; what (gnu build linux-boot) expects for the
;; '--root' kernel command-line option.
(uuid->string (uuid-bytevector root-device) 'dce)
root-device))
#~(string-append "--system=" #$system.drv)
#~(string-append "--load=" #$system.drv "/boot")))
But then it doesn't take anything from <operating-system>.
The current users are:
(define (operating-system-kernel-arguments os system.drv root-device)
"Return all the kernel arguments, including the ones not specified
directly by the user."
(bootable-kernel-arguments (operating-system-user-kernel-arguments os)
system.drv
root-device))
Of that, the current users are:
(define (operating-system-boot-parameters os system.drv root-device)
"Return a monadic <boot-parameters> record that describes the boot parameters
of OS. SYSTEM.DRV is either a derivation or #f. If it's a derivation, adds
kernel arguments for that derivation to <boot-parameters>."
(mlet* %store-monad
((initrd (operating-system-initrd-file os))
(store -> (operating-system-store-file-system os))
(bootloader -> (bootloader-configuration-bootloader
(operating-system-bootloader os)))
(bootloader-name -> (bootloader-name bootloader))
(label -> (kernel->boot-label (operating-system-kernel os))))
(return (boot-parameters
(label label)
(root-device root-device)
(kernel (operating-system-kernel-file os))
(kernel-arguments
(if system.drv
(operating-system-kernel-arguments os system.drv root-device)
(operating-system-user-kernel-arguments os)))
(initrd initrd)
(bootloader-name bootloader-name)
(store-device (ensure-not-/dev (fs->boot-device store)))
(store-mount-point (file-system-mount-point store))))))
(define* (system-qemu-image/shared-store-script os
#:key
(qemu qemu)
(graphic? #t)
(memory-size 256)
(mappings '())
full-boot?
(disk-image-size
(* (if full-boot? 500 70)
(expt 2 20)))
(options '()))
...
(define kernel-arguments
#~(list #$@(if graphic? #~() #~("console=ttyS0"))
#+@(operating-system-kernel-arguments os os-drv "/dev/vda1")))
...
Hi Danny, Danny Milosavljevic <dannym@scratchpost.org> skribis: > On Mon, 08 Jan 2018 10:26:54 +0100 > ludo@gnu.org (Ludovic Courtès) wrote: [...] >> It’d feel more idiomatic to me if, instead, we had an >> ‘operating-system-boot-kernel-arguments’ procedure that takes an OS and >> returns (list --root --system …). Then it’d be up to the caller to >> append that to what ‘operating-system-kernel-arguments’ returns. > > Yeah, but looking at it some more, it doesn't really need an OS. It needs the system derivation (and root device). Since <operating-system> has a “gexp compiler”, you can write: #~(string-append "--system=" #$os) where ‘os’ is an <operating-system>. It automatically computes its derivation. Thus, no need to explicitly call ‘operating-system-derivation’ and pass “system.drv” arguments around. So we’d just need a slight adjustment to ‘bootable-kernel-arguments’ (so that it takes the root device from the given OS object) and then rename it to ‘operating-system-kernel-arguments’. How does that sound? Ludo’.
Hi Ludo, > #~(string-append "--system=" #$os) > > where ‘os’ is an <operating-system>. It automatically computes its > derivation. Thus, no need to explicitly call > ‘operating-system-derivation’ and pass “system.drv” arguments around. Ah! Good to know. > So we’d just need a slight adjustment to ‘bootable-kernel-arguments’ (so > that it takes the root device from the given OS object) and then rename > it to ‘operating-system-kernel-arguments’. bootable-kernel-arguments is also used by the "parameters" file serializer. Also, the user that is modifying a <operating-system> instance (for example marionette-operating-system adding "panic=1") would erronously use operating-system-kernel-arguments in order to get the previous instance's arguments, resulting in the "--root", "--load" etc being prepended twice, no? The user might want to pass some kernel arguments which have nothing to do with Guix (which <operating-system>'s "kernel-arguments" is for) and then GuixSD needs some extra arguments to be able to boot the actual system (which can be found entirely automatically - nice!). Example: diff --git a/gnu/tests.scm b/gnu/tests.scm index 0caa922fd..3e4c3d4e3 100644 --- a/gnu/tests.scm +++ b/gnu/tests.scm @@ -172,6 +172,14 @@ marionette service in the guest is started after the Shepherd services listed in REQUIREMENTS." (operating-system (inherit os) + ;; Make sure the guest dies on error. + (kernel-arguments (cons "panic=1" + (operating-system-user-kernel-arguments os))) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + ;; Make sure the guest doesn't hang in the REPL on error. + (initrd (lambda (fs . rest) + (apply (operating-system-initrd os) fs + #:on-error 'backtrace + rest))) (services (cons (service marionette-service-type (marionette-configuration (requirements requirements) I'd suggest this: diff --git a/gnu/system.scm b/gnu/system.scm index df89ca06d..6466c7c48 100644 --- a/gnu/system.scm +++ b/gnu/system.scm @@ -73,7 +73,8 @@ operating-system-hosts-file operating-system-kernel operating-system-kernel-file - operating-system-kernel-arguments + operating-system-bootable-kernel-arguments + operating-system-user-kernel-arguments operating-system-initrd operating-system-users operating-system-groups @@ -200,12 +201,13 @@ booted from ROOT-DEVICE" (sudoers-file operating-system-sudoers-file ; file-like (default %sudoers-specification))) -(define (operating-system-kernel-arguments os system.drv root-device) - "Return all the kernel arguments, including the ones not specified -directly by the user." - (bootable-kernel-arguments (operating-system-user-kernel-arguments os) - system.drv - root-device)) +(define* (operating-system-bootable-kernel-arguments os) + "Prepend extra arguments to OS's kernel-arguments that allow OS to be booted." + (let* ((root-file-system (operating-system-root-file-system os)) + (root-device (file-system-device root-file-system))) + #~(bootable-kernel-arguments (operating-system-user-kernel-arguments os) + #$os + root-device))) ^L ;;; @@ -940,7 +942,7 @@ kernel arguments for that derivation to <boot-parameters>." (kernel (operating-system-kernel-file os)) (kernel-arguments (if system.drv - (operating-system-kernel-arguments os system.drv root-device) + (operating-system-bootable-kernel-arguments os) (operating-system-user-kernel-arguments os))) (initrd initrd) (bootloader-name bootloader-name) diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm index 496f2ac4e..117e333a7 100644 --- a/gnu/system/vm.scm +++ b/gnu/system/vm.scm @@ -716,7 +716,7 @@ it is mostly useful when FULL-BOOT? is true." #:disk-image-size disk-image-size))) (define kernel-arguments #~(list #$@(if graphic? #~() #~("console=ttyS0")) - #+@(operating-system-kernel-arguments os os-drv "/dev/vda1"))) + #+@(operating-system-bootable-kernel-arguments os))) (define qemu-exec #~(list (string-append #$qemu "/bin/" #$(qemu-command (%current-system)))
> It’d feel more idiomatic to me if, instead, we had an
> ‘operating-system-boot-kernel-arguments’ procedure that takes an OS and
> returns (list --root --system …). Then it’d be up to the caller to
> append that to what ‘operating-system-kernel-arguments’ returns.
We could also do that...
Note that bootable-kernel-arguments is also used by this:
(define (read-boot-parameters-file system)
"Read boot parameters from SYSTEM's (system or generation) \"parameters\"
file and returns the corresponding <boot-parameters> object or #f if the
format is unrecognized.
The object has its kernel-arguments extended in order to make it bootable."
(let* ((file (string-append system "/parameters"))
(params (call-with-input-file file read-boot-parameters))
(root (boot-parameters-root-device params))
(kernel-arguments (boot-parameters-kernel-arguments params)))
(if params
(boot-parameters
(inherit params)
(kernel-arguments (bootable-kernel-arguments kernel-arguments
system root)))
#f)))
Which is used by the ./guix/scripts/system.scm generation lister etc (that is, they have the derivations already).
Danny Milosavljevic <dannym@scratchpost.org> skribis: > Hi Ludo, > >> #~(string-append "--system=" #$os) >> >> where ‘os’ is an <operating-system>. It automatically computes its >> derivation. Thus, no need to explicitly call >> ‘operating-system-derivation’ and pass “system.drv” arguments around. > > Ah! Good to know. > >> So we’d just need a slight adjustment to ‘bootable-kernel-arguments’ (so >> that it takes the root device from the given OS object) and then rename >> it to ‘operating-system-kernel-arguments’. > > bootable-kernel-arguments is also used by the "parameters" file serializer. Good point, so probably we need to keep it as-is internally. For user consumption though, we can expose ‘operating-system-kernel-arguments’ (or whatever we call it.) > Also, the user that is modifying a <operating-system> instance (for example marionette-operating-system adding "panic=1") would erronously use operating-system-kernel-arguments in order to get the previous instance's arguments, resulting in the "--root", "--load" etc being prepended twice, no? > > The user might want to pass some kernel arguments which have nothing to do with Guix (which <operating-system>'s "kernel-arguments" is for) and then GuixSD needs some extra arguments to be able to boot the actual system (which can be found entirely automatically - nice!). Oh wait, now I realize that ‘operating-system-kernel-arguments’ is already taken. :-) So another name suggestion would be: ‘operating-system-essential-kernel-arguments’. Thoughts? Sorry for the confusion! Ludo’.
Newest attempt: diff --git a/gnu/system.scm b/gnu/system.scm index 40e259f43..37f0e76ec 100644 --- a/gnu/system.scm +++ b/gnu/system.scm @@ -73,7 +73,8 @@ operating-system-hosts-file operating-system-kernel operating-system-kernel-file - operating-system-kernel-arguments + operating-system-boot-kernel-arguments + operating-system-user-kernel-arguments operating-system-initrd operating-system-users operating-system-groups @@ -90,7 +91,6 @@ operating-system-activation-script operating-system-user-accounts operating-system-shepherd-service-names - operating-system-user-kernel-arguments operating-system-derivation operating-system-profile @@ -126,10 +126,9 @@ ;;; ;;; Code: -(define (bootable-kernel-arguments kernel-arguments system.drv root-device) - "Prepend extra arguments to KERNEL-ARGUMENTS that allow SYSTEM.DRV to be -booted from ROOT-DEVICE" - (cons* (string-append "--root=" +(define (boot-kernel-arguments system.drv root-device) + "Kernel-arguments that allow SYSTEM.DRV to be booted from ROOT-DEVICE" + (list (string-append "--root=" (if (uuid? root-device) ;; Note: Always use the DCE format because that's @@ -138,8 +137,7 @@ booted from ROOT-DEVICE" (uuid->string (uuid-bytevector root-device) 'dce) root-device)) #~(string-append "--system=" #$system.drv) - #~(string-append "--load=" #$system.drv "/boot") - kernel-arguments)) + #~(string-append "--load=" #$system.drv "/boot"))) ;; System-wide configuration. ;; TODO: Add per-field docstrings/stexi. @@ -201,12 +199,11 @@ booted from ROOT-DEVICE" (sudoers-file operating-system-sudoers-file ; file-like (default %sudoers-specification))) -(define (operating-system-kernel-arguments os system.drv root-device) - "Return all the kernel arguments, including the ones not specified -directly by the user." - (bootable-kernel-arguments (operating-system-user-kernel-arguments os) - system.drv - root-device)) +(define* (operating-system-boot-kernel-arguments os) + "Kernel arguments that allow OS (only) to be booted." + (let* ((root-file-system (operating-system-root-file-system os)) + (root-device (file-system-device root-file-system))) + #~(boot-kernel-arguments #$os root-device))) ^L ;;; @@ -319,8 +316,7 @@ The object has its kernel-arguments extended in order to make it bootable." (if params (boot-parameters (inherit params) - (kernel-arguments (bootable-kernel-arguments kernel-arguments - system root))) + (kernel-arguments (append (boot-kernel-arguments system root) kernel-arguments))) #f))) (define (boot-parameters->menu-entry conf) @@ -940,9 +936,10 @@ kernel arguments for that derivation to <boot-parameters>." (root-device root-device) (kernel (operating-system-kernel-file os)) (kernel-arguments - (if system.drv - (operating-system-kernel-arguments os system.drv root-device) - (operating-system-user-kernel-arguments os))) + (append (if system.drv + (operating-system-boot-kernel-arguments os) + '()) + (operating-system-user-kernel-arguments os))) (initrd initrd) (bootloader-name bootloader-name) (store-device (ensure-not-/dev (fs->boot-device store))) diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm index 496f2ac4e..6ba76142b 100644 --- a/gnu/system/vm.scm +++ b/gnu/system/vm.scm @@ -716,7 +716,8 @@ it is mostly useful when FULL-BOOT? is true." #:disk-image-size disk-image-size))) (define kernel-arguments #~(list #$@(if graphic? #~() #~("console=ttyS0")) - #+@(operating-system-kernel-arguments os os-drv "/dev/vda1"))) + #+@(append (operating-system-boot-kernel-arguments os) + (operating-system-user-kernel-arguments os)))) (define qemu-exec #~(list (string-append #$qemu "/bin/" #$(qemu-command (%current-system))) I get this error message: In gnu/system.scm: 905:2 2 (_ _) 939:14 1 (_ _) In unknown file: 0 (append #<gexp (boot-kernel-arguments #<gexp-input #<<?> ?) ERROR: In procedure append: Wrong type argument in position 1 (expecting empty list): #<gexp (boot-kernel-arguments #<gexp-input #<<operating-system> kern... gnu/system.scm:939 is the "append" in the middle of "operating-system-boot-parameters". What now?
Hello, Danny Milosavljevic <dannym@scratchpost.org> skribis: > +(define* (operating-system-boot-kernel-arguments os) > + "Kernel arguments that allow OS (only) to be booted." > + (let* ((root-file-system (operating-system-root-file-system os)) > + (root-device (file-system-device root-file-system))) > + #~(boot-kernel-arguments #$os root-device))) This should be: (boot-kernel-arguments os root-device) That’s why you were getting: > In gnu/system.scm: > 905:2 2 (_ _) > 939:14 1 (_ _) > In unknown file: > 0 (append #<gexp (boot-kernel-arguments #<gexp-input #<<?> ?) > ERROR: In procedure append: Wrong type argument in position 1 (expecting empty list): #<gexp (boot-kernel-arguments #<gexp-input #<<operating-system> kern... … which tells you the first argument is a gexp, whereas ‘append’ expects a list. HTH! Ludo’.
* gnu/system.scm (operating-system-kernel-arguments): Remove. (operating-system-boot-kernel-arguments): New variable. Export it. (bootable-kernel-arguments): Remove. (boot-kernel-arguments): Remove. (operating-system-boot-parameters): Adapt to the above. --- gnu/system.scm | 35 ++++++++++++++++------------------- gnu/system/vm.scm | 3 ++- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/gnu/system.scm b/gnu/system.scm index 40e259f43..51f45f6ac 100644 --- a/gnu/system.scm +++ b/gnu/system.scm @@ -73,7 +73,8 @@ operating-system-hosts-file operating-system-kernel operating-system-kernel-file - operating-system-kernel-arguments + operating-system-boot-kernel-arguments + operating-system-user-kernel-arguments operating-system-initrd operating-system-users operating-system-groups @@ -90,7 +91,6 @@ operating-system-activation-script operating-system-user-accounts operating-system-shepherd-service-names - operating-system-user-kernel-arguments operating-system-derivation operating-system-profile @@ -126,10 +126,9 @@ ;;; ;;; Code: -(define (bootable-kernel-arguments kernel-arguments system.drv root-device) - "Prepend extra arguments to KERNEL-ARGUMENTS that allow SYSTEM.DRV to be -booted from ROOT-DEVICE" - (cons* (string-append "--root=" +(define (boot-kernel-arguments system.drv root-device) + "Kernel-arguments that allow SYSTEM.DRV to be booted from ROOT-DEVICE" + (list (string-append "--root=" (if (uuid? root-device) ;; Note: Always use the DCE format because that's @@ -138,8 +137,7 @@ booted from ROOT-DEVICE" (uuid->string (uuid-bytevector root-device) 'dce) root-device)) #~(string-append "--system=" #$system.drv) - #~(string-append "--load=" #$system.drv "/boot") - kernel-arguments)) + #~(string-append "--load=" #$system.drv "/boot"))) ;; System-wide configuration. ;; TODO: Add per-field docstrings/stexi. @@ -201,12 +199,11 @@ booted from ROOT-DEVICE" (sudoers-file operating-system-sudoers-file ; file-like (default %sudoers-specification))) -(define (operating-system-kernel-arguments os system.drv root-device) - "Return all the kernel arguments, including the ones not specified -directly by the user." - (bootable-kernel-arguments (operating-system-user-kernel-arguments os) - system.drv - root-device)) +(define* (operating-system-boot-kernel-arguments os) + "Kernel arguments that allow OS (only) to be booted." + (let* ((root-file-system (operating-system-root-file-system os)) + (root-device (file-system-device root-file-system))) + (boot-kernel-arguments os root-device))) \f ;;; @@ -319,8 +316,7 @@ The object has its kernel-arguments extended in order to make it bootable." (if params (boot-parameters (inherit params) - (kernel-arguments (bootable-kernel-arguments kernel-arguments - system root))) + (kernel-arguments (append (boot-kernel-arguments system root) kernel-arguments))) #f))) (define (boot-parameters->menu-entry conf) @@ -940,9 +936,10 @@ kernel arguments for that derivation to <boot-parameters>." (root-device root-device) (kernel (operating-system-kernel-file os)) (kernel-arguments - (if system.drv - (operating-system-kernel-arguments os system.drv root-device) - (operating-system-user-kernel-arguments os))) + (append (if system.drv + (operating-system-boot-kernel-arguments os) + '()) + (operating-system-user-kernel-arguments os))) (initrd initrd) (bootloader-name bootloader-name) (store-device (ensure-not-/dev (fs->boot-device store))) diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm index 496f2ac4e..6ba76142b 100644 --- a/gnu/system/vm.scm +++ b/gnu/system/vm.scm @@ -716,7 +716,8 @@ it is mostly useful when FULL-BOOT? is true." #:disk-image-size disk-image-size))) (define kernel-arguments #~(list #$@(if graphic? #~() #~("console=ttyS0")) - #+@(operating-system-kernel-arguments os os-drv "/dev/vda1"))) + #+@(append (operating-system-boot-kernel-arguments os) + (operating-system-user-kernel-arguments os)))) (define qemu-exec #~(list (string-append #$qemu "/bin/" #$(qemu-command (%current-system)))
* gnu/system.scm (operating-system-user-kernel-arguments): Rename to... (operating-system-kernel-arguments): ... this. <operating-system>: Adapt to new name. (operating-system-boot-parameters): Adapt to new name. * gnu/system/vm.scm (system-qemu-image/shared-store-script): Adapt to new name. * gnu/tests.scm (marionette-operating-system): Adapt to new name. --- gnu/system.scm | 6 +++--- gnu/system/vm.scm | 2 +- gnu/tests.scm | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/gnu/system.scm b/gnu/system.scm index 51f45f6ac..92b3e2959 100644 --- a/gnu/system.scm +++ b/gnu/system.scm @@ -74,7 +74,7 @@ operating-system-kernel operating-system-kernel-file operating-system-boot-kernel-arguments - operating-system-user-kernel-arguments + operating-system-kernel-arguments operating-system-initrd operating-system-users operating-system-groups @@ -146,7 +146,7 @@ operating-system? (kernel operating-system-kernel ; package (default linux-libre)) - (kernel-arguments operating-system-user-kernel-arguments + (kernel-arguments operating-system-kernel-arguments (default '())) ; list of gexps/strings (bootloader operating-system-bootloader) ; <bootloader-configuration> @@ -939,7 +939,7 @@ kernel arguments for that derivation to <boot-parameters>." (append (if system.drv (operating-system-boot-kernel-arguments os) '()) - (operating-system-user-kernel-arguments os))) + (operating-system-kernel-arguments os))) (initrd initrd) (bootloader-name bootloader-name) (store-device (ensure-not-/dev (fs->boot-device store))) diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm index 6ba76142b..ec2f5b2f7 100644 --- a/gnu/system/vm.scm +++ b/gnu/system/vm.scm @@ -717,7 +717,7 @@ it is mostly useful when FULL-BOOT? is true." (define kernel-arguments #~(list #$@(if graphic? #~() #~("console=ttyS0")) #+@(append (operating-system-boot-kernel-arguments os) - (operating-system-user-kernel-arguments os)))) + (operating-system-kernel-arguments os)))) (define qemu-exec #~(list (string-append #$qemu "/bin/" #$(qemu-command (%current-system))) diff --git a/gnu/tests.scm b/gnu/tests.scm index 3e4c3d4e3..a4f70a5a4 100644 --- a/gnu/tests.scm +++ b/gnu/tests.scm @@ -174,7 +174,7 @@ in REQUIREMENTS." (inherit os) ;; Make sure the guest dies on error. (kernel-arguments (cons "panic=1" - (operating-system-user-kernel-arguments os))) + (operating-system-kernel-arguments os))) ;; Make sure the guest doesn't hang in the REPL on error. (initrd (lambda (fs . rest) (apply (operating-system-initrd os) fs
Hi,
Danny Milosavljevic <dannym@scratchpost.org> skribis:
> * gnu/system.scm (operating-system-user-kernel-arguments): Rename to...
> (operating-system-kernel-arguments): ... this.
> <operating-system>: Adapt to new name.
> (operating-system-boot-parameters): Adapt to new name.
> * gnu/system/vm.scm (system-qemu-image/shared-store-script): Adapt to new name.
> * gnu/tests.scm (marionette-operating-system): Adapt to new name.
> ---
> gnu/system.scm | 6 +++---
> gnu/system/vm.scm | 2 +-
> gnu/tests.scm | 2 +-
> 3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/gnu/system.scm b/gnu/system.scm
> index 51f45f6ac..92b3e2959 100644
> --- a/gnu/system.scm
> +++ b/gnu/system.scm
> @@ -74,7 +74,7 @@
> operating-system-kernel
> operating-system-kernel-file
> operating-system-boot-kernel-arguments
> - operating-system-user-kernel-arguments
> + operating-system-kernel-arguments
I’m a bit lost: in my tree I don’t have
‘operating-system-boot-kernel-arguments’. Is it still pending?
Otherwise my only question is whether it’s a good idea to move away from
the ‘user-’ convention. On one hand, it’s the convention we also have
for services (‘-user-services’ vs. ‘-services’), so it would be a good
thing to remain consistent. OTOH, what you propose is maybe clearer.
Thoughts?
Thanks,
Ludo’.
Hi Ludo, > I’m a bit lost: in my tree I don’t have > ‘operating-system-boot-kernel-arguments’. Is it still pending? It's added by PATCH v2 1/2 from the series. Didn't the second mail get through? > Otherwise my only question is whether it’s a good idea to move away from > the ‘user-’ convention. On one hand, it’s the convention we also have > for services (‘-user-services’ vs. ‘-services’), so it would be a good > thing to remain consistent. OTOH, what you propose is maybe clearer. > > Thoughts? Yeah, I've split it into two patches because I actually got used to operating-system-user-kernel-arguments by now (only a few days in). We could only apply PATCH v2 1/2 and not apply PATCH v2 2/2 if we wanted. In the end it comes down to whether we deem the existence operating-system-boot-kernel-arguments an implementation detail or not (whether the user would ever need to be aware of operating-system-boot-kernel-arguments). We have to export operating-system-boot-kernel-arguments because one thing in gnu/system/vm.scm needs it - otherwise it would be very much an implementation detail. Let's see what the others say.
Hello,
Danny Milosavljevic <dannym@scratchpost.org> writes:
> Hi Ludo,
>
>> I’m a bit lost: in my tree I don’t have
>> ‘operating-system-boot-kernel-arguments’. Is it still pending?
>
> It's added by PATCH v2 1/2 from the series. Didn't the second mail get through?
>
>> Otherwise my only question is whether it’s a good idea to move away from
>> the ‘user-’ convention. On one hand, it’s the convention we also have
>> for services (‘-user-services’ vs. ‘-services’), so it would be a good
>> thing to remain consistent. OTOH, what you propose is maybe clearer.
>>
>> Thoughts?
>
> Yeah, I've split it into two patches because I actually got used to
> operating-system-user-kernel-arguments by now (only a few days in).
> We could only apply PATCH v2 1/2 and not apply PATCH v2 2/2 if we
> wanted.
>
> In the end it comes down to whether we deem the existence
> operating-system-boot-kernel-arguments an implementation detail or not
> (whether the user would ever need to be aware of
> operating-system-boot-kernel-arguments). We have to export
> operating-system-boot-kernel-arguments because one thing in
> gnu/system/vm.scm needs it - otherwise it would be very much an
> implementation detail.
>
> Let's see what the others say.
Two years later, here's what I have to say :-)
I think it's nice, as a user, to be able to inspect the dynamically
computed kernel arguments that Guix would use, as that can be used for
debugging and gaining a better understanding (e.g., when passing an
argument option that overrides one computed by Guix).
If I followed this discussion correctly, currently we have:
1. operating-system-kernel-arguments which is a combination of
dynamically computed arguments by Guix + the users arguments and
2. operating-system-user-arguments which are the users arguments
themselves.
It is proposed here to split this into:
1. operating-system-boot-kernel-arguments for the Guix-computed ones
2. operating-system-user-kernel-arguments remains unchanged
Thus if the user wants to know what boot arguments their system will
use, they'd have to append these two together.
I think that two years have elapsed without touching this is perhaps an
indication that it doesn't address any real problem :-). While it's
good to attempt to clarify things, I'm afraid that changing this would
confuse more that it'd help. As Ludovic pointed out, it'd also clash
with the convention currently in use for services.
What you do think?
Maxim
Hello,
Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
> Hello,
>
> Danny Milosavljevic <dannym@scratchpost.org> writes:
>
>> Hi Ludo,
>>
>>> I’m a bit lost: in my tree I don’t have
>>> ‘operating-system-boot-kernel-arguments’. Is it still pending?
>>
>> It's added by PATCH v2 1/2 from the series. Didn't the second mail get through?
>>
>>> Otherwise my only question is whether it’s a good idea to move away from
>>> the ‘user-’ convention. On one hand, it’s the convention we also have
>>> for services (‘-user-services’ vs. ‘-services’), so it would be a good
>>> thing to remain consistent. OTOH, what you propose is maybe clearer.
>>>
>>> Thoughts?
>>
>> Yeah, I've split it into two patches because I actually got used to
>> operating-system-user-kernel-arguments by now (only a few days in).
>> We could only apply PATCH v2 1/2 and not apply PATCH v2 2/2 if we
>> wanted.
>>
>> In the end it comes down to whether we deem the existence
>> operating-system-boot-kernel-arguments an implementation detail or not
>> (whether the user would ever need to be aware of
>> operating-system-boot-kernel-arguments). We have to export
>> operating-system-boot-kernel-arguments because one thing in
>> gnu/system/vm.scm needs it - otherwise it would be very much an
>> implementation detail.
>>
>> Let's see what the others say.
>
> Two years later, here's what I have to say :-)
>
> I think it's nice, as a user, to be able to inspect the dynamically
> computed kernel arguments that Guix would use, as that can be used for
> debugging and gaining a better understanding (e.g., when passing an
> argument option that overrides one computed by Guix).
>
> If I followed this discussion correctly, currently we have:
>
> 1. operating-system-kernel-arguments which is a combination of
> dynamically computed arguments by Guix + the users arguments and
> 2. operating-system-user-arguments which are the users arguments
> themselves.
>
> It is proposed here to split this into:
>
> 1. operating-system-boot-kernel-arguments for the Guix-computed ones
> 2. operating-system-user-kernel-arguments remains unchanged
>
> Thus if the user wants to know what boot arguments their system will
> use, they'd have to append these two together.
>
> I think that two years have elapsed without touching this is perhaps an
> indication that it doesn't address any real problem :-). While it's
> good to attempt to clarify things, I'm afraid that changing this would
> confuse more that it'd help. As Ludovic pointed out, it'd also clash
> with the convention currently in use for services.
>
> What you do think?
There haven't been any further comments.
Closing.
Maxim