diff mbox series

[bug#45692,2/4] gnu: Make file-systems target extensible by services.

Message ID yrjG3wstz9GpyH32kV3JqsEiG-KdxmP9_JHe6ghdMoizrBWpIZhd3hl2VPjFuWagt1WLPXMqSGdMZZWTcvE6J7qxVjdcvq3zTK-Wccd3wEs=@protonmail.com
State Accepted
Headers show
Series None | expand

Commit Message

dziltener--- via Guix-patches via Jan. 25, 2021, 12:18 a.m. UTC
Hello as well,

That is certainly another possibility, would this be more palatable to Guix?

Thanks
raid5atemyhomework

> Hello!
>
> raid5atemyhomework raid5atemyhomework@protonmail.com writes:
>
> > > From 792a8f8efc95e4fe9a94d42f839ddcfb034b8540 Mon Sep 17 00:00:00 2001
> > > From: raid5atemyhomework raid5atemyhomework@protonmail.com
> > > Date: Wed, 6 Jan 2021 08:15:54 +0800
> > > Subject: [PATCH 2/4] gnu: Make file-systems target extensible by services.
>
> It’s not clear to me what “file-systems target” is, and why we’re
> extending ‘file-systems-target-service-type’… I think what we want is
> to extend the ‘file-system-service-type’ with shepherd services’ names,
> which means some shepherd services that will handle file systems
> mounting themself instead of the usual <file-system> objects, fstab
> entries, mounted by kernel.
>
> So I write with this patch to extend file-system-service-type directly
> instead of introducing a new ‘file-systems-target-service-type’:
>
> What do you think? Thank you!

Comments

Ludovic Courtès Feb. 10, 2021, 2:17 p.m. UTC | #1
Hi!

I agree with 宋文武.

Still…

raid5atemyhomework <raid5atemyhomework@protonmail.com> skribis:

>    (service-type (name 'file-systems)
>                  (extensions
>                   (list (service-extension shepherd-root-service-type
> -                                          file-system-shepherd-services)
> +                                          (lambda (value)
> +                                            (file-system-shepherd-services
> +                                             (filter file-system? value)
> +                                             (filter symbol? value))))
>                         (service-extension fstab-service-type
> -                                          file-system-fstab-entries)
> +                                          (lambda (value)
> +                                            (file-system-fstab-entries
> +                                             (filter file-system? value))))
>  
>                         ;; Have 'user-processes' depend on 'file-systems'.
>                         (service-extension user-processes-service-type
>                                            (const '(file-systems)))))
> +
> +                ;; Extensions consist of lists of <file-system> objects or
> +                ;; shepherd services’ names (symbols).  In the latter case,
> +                ;; the provided shepherd services supposed to mount and
> +                ;; unmount some file systems themself.

Why do we need to extend with symbols?

In general it’s much clearer if extensions receive only one type of
object (<file-system> records in this case).  It’s also best to avoid
passing around symbolic names (that’s why we extend with <file-system>
records rather than with Shepherd service names or whatever.)

Ludo’.
raid5atemyhomework Feb. 10, 2021, 2:46 p.m. UTC | #2
> Why do we need to extend with symbols?
>
> In general it’s much clearer if extensions receive only one type of
> object (<file-system> records in this case). It’s also best to avoid
> passing around symbolic names (that’s why we extend with <file-system>
> records rather than with Shepherd service names or whatever.)

For this case, how would it be done?

ZFS file system, on other operating systems and distributions, is documented as automatically mounting filesystems, without management in an `fstab` or similar file, because the intent is that you would make lots of filesystems for various uses and managing an `fstab` would be too onerous.  Thus, ZFS file system expects to mount multiple file systems with a single `zfs mount -a` command at startup.

Would the below sketch be acceptable?

```scheme
; gnu/system/file-systems.scm
(define-record-type* file-system #;...
  #;...
  (has-fstab-entry?   file-system-has-fstab-entry?  (default #t)))

;...
; gnu/services/base,scm

(define file-system-service-type
  (service-type
    #;...
    (extensions
      (list #;...
            (service-extension fstab-service-type
                               (lambda (file-systems)
                                 (filter file-system-has-fstab-entry?
                                         file-systems)))
            #;...))
    #;...))

;...
; gnu/services/file-systems.scm

(define zfs-service-type
  (service-type
     #;...
     (extensions (list #;...
                       (service-extension file-system-service-type
                                          (const (list (file-system
                                                         (device "dummy")
                                                         (mount-point "zfs/*")
                                                         (has-fstab-entry? #f)))))))
    #;...))
```

Then there will be a Shepherd service providing `file-system-zfs/*` which would perform `zfs mount -a -l` on `start` and `zfs unmount -a -f` on `stop`.

Would that be acceptable?  I am wary of this since it creates a dummy file-system and needs an additional field on every `file-system` record, one which is *only* used by ZFS.  I feel the `file-system-target-service-type` is more generic and does not use trickery.

Thanks
raid5atemyhomework
diff mbox series

Patch

From 44ee1e470a2f9d4985af4d51654d9f943caa0f24 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E5=AE=8B=E6=96=87=E6=AD=A6?= <iyzsong@member.fsf.org>
Date: Sat, 23 Jan 2021 20:39:06 +0800
Subject: [PATCH] services: Allow 'file-system-service-type' extensible by
 service name.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* gnu/services/base.scm (file-system-shepherd-services): Add
'extra-services-names' paramater.
(file-system-service-type): Handle services’ names from extensions.
---
 gnu/services/base.scm | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index f6a490f712..7bddef5034 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -364,15 +364,16 @@  FILE-SYSTEM."
                        (gnu system file-systems)
                        ,@%default-modules)))))))
 
-(define (file-system-shepherd-services file-systems)
+(define (file-system-shepherd-services file-systems extra-services-names)
   "Return the list of Shepherd services for FILE-SYSTEMS."
   (let* ((file-systems (filter file-system-mount? file-systems)))
     (define sink
       (shepherd-service
        (provision '(file-systems))
-       (requirement (cons* 'root-file-system 'user-file-systems
-                           (map file-system->shepherd-service-name
-                                file-systems)))
+       (requirement (append '(root-file-system user-file-systems)
+                            (map file-system->shepherd-service-name
+                                 file-systems)
+                            extra-services-names))
        (documentation "Target for all the initially-mounted file systems")
        (start #~(const #t))
        (stop #~(const #f))))
@@ -429,13 +430,23 @@  FILE-SYSTEM."
   (service-type (name 'file-systems)
                 (extensions
                  (list (service-extension shepherd-root-service-type
-                                          file-system-shepherd-services)
+                                          (lambda (value)
+                                            (file-system-shepherd-services
+                                             (filter file-system? value)
+                                             (filter symbol? value))))
                        (service-extension fstab-service-type
-                                          file-system-fstab-entries)
+                                          (lambda (value)
+                                            (file-system-fstab-entries
+                                             (filter file-system? value))))
 
                        ;; Have 'user-processes' depend on 'file-systems'.
                        (service-extension user-processes-service-type
                                           (const '(file-systems)))))
+
+                ;; Extensions consist of lists of <file-system> objects or
+                ;; shepherd services’ names (symbols).  In the latter case,
+                ;; the provided shepherd services supposed to mount and
+                ;; unmount some file systems themself.
                 (compose concatenate)
                 (extend append)
                 (description
-- 
2.29.2