[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Patches to implement system roll-back and switch-generation
From: |
Chris Marusich |
Subject: |
Re: Patches to implement system roll-back and switch-generation |
Date: |
Sat, 15 Oct 2016 23:28:03 -0700 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Hi Ludo,
address@hidden (Ludovic Courtès) writes:
> Hello,
>
> Chris Marusich <address@hidden> skribis:
>
>> address@hidden (Ludovic Courtès) writes:
>
> [...]
>
>>> Sorry about that! Hopefully we can work around the conflicts.
>>
>> I think we can. But I think it will require backwards incompatible
>> changes to the boot parameters file. Here's why:
>>
>> Many of the existing procedures in (gnu system grub) take a "file
>> system" object as input (e.g. the 'grub-configuration-file' procedure).
>> However, the boot parameters file does not currently contain all the
>> information that a "file system" object contains.
>
> Good point. This ‘store-fs’ argument was added in response to
> <http://bugs.gnu.org/22281>.
>
>> Here's an example of what it contains today:
>>
>> (boot-parameters
>> (version 0)
>> (label "GNU with Linux-Libre 4.1.20 (beta)")
>> (root-device "root")
>> (kernel
>> "/gnu/store/zygby8db0adcyj3m6rjflr80jarfy9b5-linux-libre-4.1.20")
>> (kernel-arguments ())
>> (initrd
>> (string-append
>> "/gnu/store/hlra3a0g3a14bjvdn3vbagwfvy4nmhn8-base-initrd"
>> "/initrd")))
>>
>> To avoid backwards-incompatible changes to the structure of the boot
>> parameters file, I had originally planned to refactor the procedures in
>> (gnu system grub) so that I could use them with the limited information
>> that is contained in the version 0 boot parameters file. However,
>> commit 0f65f54e has modified these procedures in a way that makes it
>> very awkward to refactor the "file system" object out of them. Now, to
>> re-use the existing procedures, I believe I will need to add this
>> missing information (i.e., the information contained in a file system
>> object) to the boot parameters file, so that I can construct a "file
>> system" object to pass to those procedures. Does that sound right to
>> you?
>
> Yes, I think so.
>
> More precisely, I think we need to add a ‘device’ field to <menu-entry>,
> which could be the UUID or label of the device where the kernel and
> initrd are to be found, or #f, in which case grub.cfg would contain a
> “search --file” command (instead of “search --label” or “search
> --fs-uuid”).
>
> Correspondingly, we’d add a ‘device’ (or ‘boot-device’?) field to
> <boot-parameters>. The key is that ‘device’ can be different from
> ‘root-device’ because the store and root devices can be different from
> one another.
>
> That way we could remove the ‘store-fs’ parameter of
> ‘grub-configuration-file’ since that information would now be contained
> in each <menu-entry>.
>
That sounds promising! I'll try that approach.
>
>> If I do that, then it will probably be a backwards-incompatible change,
>> so I will do it in the following way. I will simply store an entire
>> "file system" object in the boot parameters file. I will bump the
>> version of the boot parameters file from 0 to 1. To ensure that all new
>> system generations use version 1, I will update commands like
>> "reconfigure" to always create a version 1 boot parameters file. I will
>> make the new commands (roll-back and switch-generation) refuse to switch
>> to any system generation which uses version 0 (because it isn't possible
>> to construct a complete "file system" object from a version 0 boot
>> parameters file). I will also update existing commands like
>> 'list-generations' so that they will gracefully handle both versions.
>>
>> Does this sound like the right approach to you?
>
> I think we don’t need to bump the version number: ‘read-boot-parameters’
> can simply do what it currently does for ‘initrd’ and
> ‘kernel-arguments’, which is to provide a default value when they’re
> missing. Here the default value could be ‘root-device’.
I think you're probably right about this, too. I'll try it that way.
>
>> I've tried using 'git send-email' on GuixSD before, and it didn't work
>> for me (because a mail transfer agent is not running on my GuixSD
>> system). When the new patches are ready, I'll try once more to get it
>> working.
>
> AFAICT an MTA is not needed.
>
I'll let you know if it works!
>
>>>> - "Return the GRUB configuration file corresponding to CONFIG, a
>>>> -<grub-configuration> object, and where the store is available at
>>>> STORE-FS, a
>>>> -<file-system> object. OLD-ENTRIES is taken to be a list of menu entries
>>>> -corresponding to old generations of the system."
>>>> + "Return a derivation which builds the GRUB configuration file
>>>> corresponding
>>>> +to CONFIG, a <grub-configuration> object, and where the store is
>>>> available at
>>>> +STORE-FS, a <file-system> object. OLD-ENTRIES is taken to be a list of
>>>> menu
>>>> +entries corresponding to old generations of the system."
>>>
>>> OK, although I often write “Return something” when that really means
>>> “Return a derivation that builds something”.
>>
>> Upon closer inspection, it looks like this procedure,
>> 'grub-configuration-file', actually returns a monadic value (in the
>> store monad), which "contains" a derivation, which in turn builds the
>> grub configuration file. Even in a case like this, where there is so
>> much indirection, is it appropriate to elide all those details?
>>
>> If this is the style we should consistently use in our documentation,
>> then that's fine, and I will happily follow suit in the name of
>> consistency. However, as a newcomer to this code base, to gexps, to
>> derivations, and to monads, in the beginning I was very confused about
>> how to use this procedure's return value.
>>
>> If I can think of a good way to make stuff like this more obvious for
>> newcomers, I'll let you know. For now, though, I think the best thing
>> to do is to change my patches to conform to the existing style.
>
> I think so. :-)
>
> That said, I can understand that the indirections can be confusing,
> esp. since these parts are not properly documented. That “return a
> file” really means “return a derivation as a monadic value” is non
> obvious.
>
> We can now avoid monadic procedures by using the declarative counterpart
> of the monadic API. That is, we could write:
>
> (define (grub-configuration-file …) ;normal proc
> (computed-file "grub.cfg" builder))
>
> instead of:
>
> (define (grub-configuration-file …) ;monadic proc
> (gexp->derivation "grub.cfg" builder))
>
> I would welcome such changes.
>
That's an interesting idea. However, in this case, I think we need to
pass the build options (from the parsed command-line arguments) along
somehow. How should we set the build options when using the declarative
'computed-file' procedure? It seems like the most obvious way would be
to pass the build options in as arguments to the 'computed-file'
procedure, but is there a better way?
--
Chris
signature.asc
Description: PGP signature