From: ludo@gnu.org (Ludovic Courtès)
To: Chris Marusich <cmmarusich@gmail.com>
Cc: guix-devel@gnu.org
Subject: Re: Patches to implement system roll-back and switch-generation
Date: Tue, 11 Oct 2016 23:05:10 +0200 [thread overview]
Message-ID: <87pon6sik9.fsf@gnu.org> (raw)
In-Reply-To: <87lgxy11py.fsf@gmail.com> (Chris Marusich's message of "Sat, 08 Oct 2016 23:22:01 -0700")
Hello,
Chris Marusich <cmmarusich@gmail.com> skribis:
> ludo@gnu.org (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>.
> 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'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.
>>> - "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.
> Thank you for your patience. I'll send the updated patches when they're
> ready.
Awesome, thanks!
Ludo’.
next prev parent reply other threads:[~2016-10-11 21:05 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-30 6:21 Patches to implement system roll-back and switch-generation Chris Marusich
2016-10-04 10:01 ` Ludovic Courtès
2016-10-09 6:22 ` Chris Marusich
2016-10-11 21:05 ` Ludovic Courtès [this message]
2016-10-16 6:28 ` Chris Marusich
2016-10-19 21:07 ` Ludovic Courtès
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://guix.gnu.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87pon6sik9.fsf@gnu.org \
--to=ludo@gnu.org \
--cc=cmmarusich@gmail.com \
--cc=guix-devel@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).