Subject: Re: bug#26544: [PATCH] system: Move "--load" and other guix-specific parameters from the grub module to the generic system module.

From: Danny Milosavljevic <[email-masked]>
Date: Tue, 18 Apr 2017 16:51:59 +0200

Hi Ludo,

> Could you explain the “big picture”, the rationale behind this change?

Currenly, (gnu system grub) prepends the "--system", "--load" and "--root" on its own.  But that's something specific to the guix system and has nothing to do with bootloaders (it's not even a bootloader option - it's just passing it through to the Linux kernel and *that* is passing some through to Shepherd).

In connection with the effort to support other bootloaders we could of course copy such a prepension-block to each of the bootloader modules, but in my opinion it's better if it's moved to (bootloader-independent) boot-parameters so everyone just automatically uses the correct kernel arguments, including which guix system guix should boot.

I suspect that this wasn't done before because the boot-parameters are serialized to a file "parameters" in the "system" directory.  But then "--system=<system>" in that file would have to contain the hash value of the system, which is ... impossible (or at least very very hard to do).

Therefore, this patch makes sure the in-memory <boot-parameters> instances do contain "--system" with the correct hash value, but the stored "parameters" doesn't.

I think that that's the best of both worlds.  Clients in guix can use data read from <boot-parameters> without second-guessing everything and the stored file still doesn't have to solve a hash-functions-really-try-to-prevent-you-from-doing-that problem - the stored file doesn't contain the hash value of itself (and now doesn't need to).

I've successfully tested it by "guix system reconfigure" on my machine and also by "guix system vm" (full-boot and non-full-boot) on my machine, starting from master.

> Danny Milosavljevic <[email-masked]> skribis:
> 
> > +(define (bootable-kernel-arguments kernel-arguments system root-device)

> Please add a docstring to top-level procedures:

Something like this?

"Prepend extra arguments to KERNEL-ARGUMENTS that allow the guix system to boot SYSTEM on ROOT-DEVICE."

> So I suggest stick to this convention and thus have:
> 
>   operating-system-user-kernel-arguments     ;arguments specified by the user
>   operating-system-kernel-arguments          ;all the arguments

Hmm, I don't think it's really directly specified by the user, is it?  I'm just trying to avoid having to store the file's (more or less) own hash value into the file.

> > -(define (operating-system-parameters-file os)
> > +(define (operating-system-boot-parameters os system root-device)  
> 
> I think it would be clearer to rename in a separate patch.

I'll try.

> > +(define (read-boot-parameters-file sysgen)
> > +  "Read boot parameters from SYSGEN's (system or generation) \"parameters\"  
> 
> Please use full words in variable names, typically ‘system’ here.

Ok.  Should the docstring say "SYSTEM (system or generation)" or just "SYSTEM"?