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

Previous Next

Full log

Message #8 received at 58086 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: iyzsong <at> envs.net
Cc: 宋文武 <iyzsong <at> member.fsf.org>, 58086 <at> debbugs.gnu.org
Subject: Re: bug#58086: [PATCH] gnu: Add fstrim-service-type.
Date: Thu, 06 Oct 2022 23:00:08 +0200

Hi,

iyzsong <at> envs.net skribis:

> From: 宋文武 <iyzsong <at> 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’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.