From: ludo@gnu.org (Ludovic Courtès)
To: David Craven <david@craven.ch>
Cc: guix-devel@gnu.org
Subject: Re: [PATCH 2/2] system: Add btrfs file system support.
Date: Sat, 03 Dec 2016 16:31:19 +0100 [thread overview]
Message-ID: <871sxpav20.fsf@gnu.org> (raw)
In-Reply-To: <20161130183635.6513-2-david@craven.ch> (David Craven's message of "Wed, 30 Nov 2016 19:36:35 +0100")
David Craven <david@craven.ch> skribis:
> * gnu/system/linux-initrd.scm (linux-modules, helper-packages): Add
> btrfs modules when a btrfs file-system is used.
> * gnu/build/file-systems.scm (check-file-system-irrecoverable-error,
> check-file-system-ext): New variables.
> (check-file-system): Support non ext file systems gracefully.
[...]
> -(define (check-file-system device type)
> - "Run a file system check of TYPE on DEVICE."
> - (define fsck
> - (string-append "fsck." type))
> -
> - (let ((status (system* fsck "-v" "-p" "-C" "0" device)))
> +(define (check-file-system-irrecoverable-error prog code device)
> + (format (current-error-port)
> + "'~a' exited with code ~a on ~a; spawning Bourne-like REPL~%"
> + prog code device)
> + (start-repl %bournish-language))
This is confusing because this procedure doesn’t check anything, despite
the name. :-)
> +(define (check-file-system-ext device type)
> + (let* ((fsck (string-append "fsck." type))
> + (status (system* fsck "-v" "-p" "-C" "0" device)))
> (match (status:exit-val status)
> (0
> #t)
I think I made a different suggestion to Marius, so I may well be
contradicting myself now, but what comes to mind now is to define a list
of “checkers” corresponding to each file system type:
(define-record-type <file-system-checker>
(file-system-checker predicate checker)
file-system-checker?
(predicate file-system-checker-predicate)
(checker file-system-checker-procedure))
Then we can have an alist:
(define %ext2-checker
(file-system-checker (cut string-prefix? "ext" <>)
(lambda (device type)
…)))
(define %file-system-checkers
(list %ext2-checker %vfat-checker %btrfs-checker))
Each checker procedure would return an enum (either using (rnrs enums)
or returning a symbol like 'pass, 'errors-corrected, or 'reboot-needed).
The code to start the REPL would be outside the checkers themselves, in
the generic ‘check-file-system’.
WDYT?
> --- a/gnu/system/linux-initrd.scm
> +++ b/gnu/system/linux-initrd.scm
> @@ -193,6 +193,9 @@ loaded at boot time in the order in which they appear."
> ,@(if (find (file-system-type-predicate "9p") file-systems)
> virtio-9p-modules
> '())
> + ,@(if (find (file-system-type-predicate "btrfs") file-systems)
> + '("btrfs")
> + '())
> ,@(if volatile-root?
> '("fuse")
> '())
> @@ -200,11 +203,12 @@ loaded at boot time in the order in which they appear."
>
> (define helper-packages
> ;; Packages to be copied on the initrd.
> - `(,@(if (find (lambda (fs)
> - (string-prefix? "ext" (file-system-type fs)))
> - file-systems)
> + `(,@(if (find (file-system-type-predicate "ext4") file-systems)
> (list e2fsck/static)
> '())
> + ,@(if (find (file-system-type-predicate "btrfs") file-systems)
> + (list btrfs-progs/static)
> + '())
> ,@(if volatile-root?
> (list unionfs-fuse/static)
> '())))
This part LGTM.
Thanks!
Ludo’.
next prev parent reply other threads:[~2016-12-03 15:31 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-30 18:36 [PATCH 1/2] gnu: Add btrfs-progs/static David Craven
2016-11-30 18:36 ` [PATCH 2/2] system: Add btrfs file system support David Craven
2016-12-01 19:18 ` Marius Bakke
2016-12-02 10:50 ` David Craven
2016-12-02 11:12 ` Chris Marusich
2016-12-02 16:27 ` David Craven
2016-12-03 15:21 ` Ludovic Courtès
2016-12-03 15:18 ` Ludovic Courtès
2016-12-03 15:31 ` Ludovic Courtès [this message]
2016-12-03 16:21 ` David Craven
2016-12-05 20:44 ` Ludovic Courtès
2016-12-03 15:15 ` [PATCH 1/2] gnu: Add btrfs-progs/static Ludovic Courtès
2016-12-03 21:41 ` David Craven
2016-12-05 20:51 ` 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=871sxpav20.fsf@gnu.org \
--to=ludo@gnu.org \
--cc=david@craven.ch \
--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).