[bug#58086] [PATCH] gnu: Add fstrim-service-type.

["Ludovic Courtès" <ludo@gnu.org>]

Hi,

iyzsong@envs.net skribis:

> From: 宋文武 <iyzsong@member.fsf.org>
>
> A timestamp file "/var/lib/mcron/fstrim.stamp" is used to ensure we will
> catch up on missed job runs when the system was powered down.
>
> * gnu/services/mcron.scm (%mcron-activation): New extension to create
> '/var/lib/mcron'.
> * gnu/services/admin.scm (fstrim-configuration): New record type.
> (fstrim-mcron-jobs): New procedure.
> (fstrim-service-type): New service type.

Please add documentation in doc/guix.texi.  :-)


[...]

> +(define fstrim-mcron-jobs
> +  (match-lambda
> +    (($ <fstrim-configuration> command interval)
> +     (list
> +      #~(job
> +         (let ((last-time
> +                (catch #t
> +                  (lambda ()
> +                    (with-input-from-file #$fstrim-mcron-stamp read))
> +                  ;; We schedule a first run immediately.
> +                  (const 0))))
> +           (lambda (current-time)
> +             (let ((next-time (max current-time (+ #$interval last-time))))
> +               (set! last-time next-time)
> +               next-time)))
> +         (lambda ()
> +           (apply system* '#$command)
> +           (with-output-to-file #$fstrim-mcron-stamp
> +             (lambda () (write (current-time))))))))))

That seems a little bit complicated, no?  That’s because you want to
make sure it runs immediately at boot if it never ran before, right?  Is
that important?

> +(define fstrim-service-type
> +  (service-type
> +   (name 'fstrim)
> +   (extensions
> +    (list (service-extension mcron-service-type
> +                             fstrim-mcron-jobs)))
> +   (description
> +    "Periodically discard unused blocks on filesystems.")

“file systems”, two words.  Perhaps add a few more words mentioning the
fstrim package?

> +++ b/gnu/services/mcron.scm
> @@ -154,6 +154,12 @@ (define mcron-shepherd-services
>                (actions
>                 (list (shepherd-schedule-action mcron files)))))))))
>  
> +(define %mcron-activation
> +  (with-imported-modules '((guix build utils))
> +    #~(begin
> +        (use-modules (guix build utils))
> +        (mkdir-p "/var/lib/mcron"))))

I’m not sure the fstrim timestamp should leave in a directory that looks
as if it was “owned” by mcron.

Could you send an updated patch?

Thanks,
Ludo’.