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

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

(bootable-kernel-arguments): Export.

(read-boot-parameters-file): Use bootable-kernel-arguments.

bootable-kernel-arguments.

---

gnu/system.scm | 15 ++++++---------

gnu/system/vm.scm | 4 +++-

2 files changed, 9 insertions(+), 10 deletions(-)

(operating-system-kernel-arguments): ... this.

operating-system-kernel-arguments.

---

gnu/system.scm | 6 +++---

gnu/system/vm.scm | 2 +-

2 files changed, 4 insertions(+), 4 deletions(-)

Hello,

Danny Milosavljevic <dannym@scratchpost.org> skribis:

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:

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:

[...]

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,

Ah! Good to know.

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:

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:

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

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:

Hello,

Danny Milosavljevic <dannym@scratchpost.org> skribis:

This should be:

(boot-kernel-arguments os root-device)

That’s why you were getting:

… which tells you the first argument is a gexp, whereas ‘append’ expects

a list.

HTH!

Ludo’.

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

(operating-system-kernel-arguments): ... this.

<operating-system>: Adapt to new name.

(operating-system-boot-parameters): Adapt to new name.

---

gnu/system.scm | 6 +++---

gnu/system/vm.scm | 2 +-

gnu/tests.scm | 2 +-

3 files changed, 5 insertions(+), 5 deletions(-)

Hi,

Danny Milosavljevic <dannym@scratchpost.org> skribis:

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,

It's added by PATCH v2 1/2 from the series. Didn't the second mail get through?

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:

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:

There haven't been any further comments.

Closing.

Maxim