From mboxrd@z Thu Jan 1 00:00:00 1970 From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=) Subject: Re: [PATCH 2/2] system: Add btrfs file system support. Date: Sat, 03 Dec 2016 16:31:19 +0100 Message-ID: <871sxpav20.fsf@gnu.org> References: <20161130183635.6513-1-david@craven.ch> <20161130183635.6513-2-david@craven.ch> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:36741) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cDCHe-00023l-S3 for guix-devel@gnu.org; Sat, 03 Dec 2016 10:31:27 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cDCHZ-0004vK-SN for guix-devel@gnu.org; Sat, 03 Dec 2016 10:31:26 -0500 In-Reply-To: <20161130183635.6513-2-david@craven.ch> (David Craven's message of "Wed, 30 Nov 2016 19:36:35 +0100") List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: David Craven Cc: guix-devel@gnu.org David Craven 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=E2=80=99t check anything, de= spite 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 =E2=80=9Ccheckers=E2=80=9D corresponding to each file system type: (define-record-type (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) =E2=80=A6))) (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 =E2=80=98check-file-system=E2=80=99. 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 appe= ar." >=20=20 > (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=E2=80=99.