Re: [PATCH 2/2] system: Add btrfs file system support.

[Ludovic Courtès]

David Craven <address@hidden> 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’.