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

From: [email-masked] (Ludovic Courtès)
Date: Wed, 19 Apr 2017 11:59:37 +0200

Hello!

Danny Milosavljevic <[email-masked]> skribis:

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

OK, that makes a lot of sense, indeed.

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

OK, I see.  I’ll look more closely at the patch but I think it’s fine to
treat --system specially, for the reasons you gave.

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

Yeah, something like that (“that allow SYSTEM to be botted from
ROOT-DEVICE” maybe.)

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

I’ll have to check.

>> > -(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"?

The former is clearer IMO.

Thank you,
Ludo’.