[bug#29932] [PATCH 0/2] Clean up operating-system-kernel-arguments.

[bug#29932] [PATCH 0/2] Clean up operating-system-kernel-arguments.

[Danny Milosavljevic]

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(-)

[bug#29932] [PATCH 1/2] system: Inline operating-system-kernel-arguments.

[Danny Milosavljevic]

* 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)))

[bug#29932] [PATCH 2/2] system: Rename operating-system-user-kernel-arguments to operating-system-kernel-arguments.

[Danny Milosavljevic]

* 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

[bug#29932] [PATCH 0/2] Clean up operating-system-kernel-arguments.

[Ludovic Courtès]

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’.

[bug#29932] [PATCH 0/2] Clean up operating-system-kernel-arguments.

[Danny Milosavljevic]

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")))
...

[bug#29932] [PATCH 0/2] Clean up operating-system-kernel-arguments.

[Ludovic Courtès]

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’.

[bug#29932] [PATCH 0/2] Clean up operating-system-kernel-arguments.

[Danny Milosavljevic]

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)))

[bug#29932] [PATCH 0/2] Clean up operating-system-kernel-arguments.

[Danny Milosavljevic]

> 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).

[bug#29932] [PATCH 0/2] Clean up operating-system-kernel-arguments.

[Ludovic Courtès]

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’.

[bug#29932] [PATCH 0/2] Clean up operating-system-kernel-arguments.

[Danny Milosavljevic]

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?

[bug#29932] [PATCH 0/2] Clean up operating-system-kernel-arguments.

[Ludovic Courtès]

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’.

[bug#29932] [PATCH v2 1/2] system: Split up operating-system-kernel-arguments into operating-system-boot-kernel-arguments and operating-system-user-kernel-arguments.

[Danny Milosavljevic]

* 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)))

[bug#29932] [PATCH v2 2/2] system: Rename operating-system-user-kernel-arguments to operating-system-kernel-arguments.

[Danny Milosavljevic]

* 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

[bug#29932] [PATCH v2 2/2] system: Rename operating-system-user-kernel-arguments to operating-system-kernel-arguments.

[Ludovic Courtès]

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’.

[bug#29932] [PATCH v2 2/2] system: Rename operating-system-user-kernel-arguments to operating-system-kernel-arguments.

[Danny Milosavljevic]

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.

[bug#29932] [PATCH v2 2/2] system: Rename operating-system-user-kernel-arguments to operating-system-kernel-arguments.

[Maxim Cournoyer]

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

bug#29932: [PATCH 0/2] Clean up operating-system-kernel-arguments.

[Maxim Cournoyer]

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