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

[Danny Milosavljevic <dannym@scratchpost.org>]

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