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

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 58086 in the body.
You can then email your comments to 58086 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

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

From: iyzsong <at> envs.net
To: guix-patches <at> gnu.org
Cc: 宋文武 <iyzsong <at> member.fsf.org>
Subject: [PATCH] gnu: Add fstrim-service-type.
Date: Mon, 26 Sep 2022 15:28:25 +0800

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.
---
 gnu/services/admin.scm | 56 +++++++++++++++++++++++++++++++++++++++++-
 gnu/services/mcron.scm |  8 ++++++
 2 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/gnu/services/admin.scm b/gnu/services/admin.scm
index 252bedb0bd..2b22fc5b33 100644
--- a/gnu/services/admin.scm
+++ b/gnu/services/admin.scm
@@ -21,6 +21,7 @@
 (define-module (gnu services admin)
   #:use-module (gnu packages admin)
   #:use-module (gnu packages certs)
+  #:use-module (gnu packages linux)
   #:use-module (gnu packages package-management)
   #:use-module (gnu services)
   #:use-module (gnu services mcron)
@@ -30,6 +31,7 @@ (define-module (gnu services admin)
   #:use-module (guix packages)
   #:use-module (guix records)
   #:use-module (srfi srfi-1)
+  #:use-module (ice-9 match)
   #:use-module (ice-9 vlist)
   #:export (%default-rotations
             %rotated-files
@@ -63,7 +65,11 @@ (define-module (gnu services admin)
             unattended-upgrade-configuration-services-to-restart
             unattended-upgrade-configuration-system-expiration
             unattended-upgrade-configuration-maximum-duration
-            unattended-upgrade-configuration-log-file))
+            unattended-upgrade-configuration-log-file
+
+            fstrim-service-type
+            fstrim-configuration
+            fstrim-configuration?))
 
 ;;; Commentary:
 ;;;
@@ -376,4 +382,52 @@ (define unattended-upgrade-service-type
     "Periodically upgrade the system from the current configuration.")
    (default-value (unattended-upgrade-configuration))))
 
+
+;;;
+;;; fstrim.
+;;;
+
+(define-record-type* <fstrim-configuration>
+  fstrim-configuration make-fstrim-configuration fstrim-configuration?
+  (command  fstrim-configuration-command
+            (default
+              (list (file-append util-linux "/sbin/fstrim")
+                    "--verbose" "--quiet-unsupported"
+                    "--listed-in" "/etc/fstab")))
+  (interval fstrim-configuration-interval (default (* 60 60 24 7)))) ; weekly
+
+;;; By storing the time of job's last run in a file, we can catch up on missed
+;;; runs when the system was powered down.
+(define fstrim-mcron-stamp "/var/lib/mcron/fstrim.stamp")
+
+(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))))))))))
+
+(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.")
+   (default-value (fstrim-configuration))))
+
 ;;; admin.scm ends here
diff --git a/gnu/services/mcron.scm b/gnu/services/mcron.scm
index 23760ebda4..833d979ab4 100644
--- a/gnu/services/mcron.scm
+++ 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"))))
+
 (define mcron-service-type
   (service-type (name 'mcron)
                 (description
@@ -161,6 +167,8 @@ (define mcron-service-type
                 (extensions
                  (list (service-extension shepherd-root-service-type
                                           mcron-shepherd-services)
+                       (service-extension activation-service-type
+                                          (const %mcron-activation))
                        (service-extension profile-service-type
                                           (compose list
                                                    mcron-configuration-mcron))))
-- 
2.37.3

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’.

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

From: Bruno Victal <mirai <at> makinata.eu>
To: iyzsong <at> envs.net
Cc: Ludovic Courtès <ludo <at> gnu.org>, 58086 <at> debbugs.gnu.org,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: Re: [bug#58086] [PATCH] gnu: Add fstrim-service-type.
Date: Wed, 22 Mar 2023 12:03:44 +0000

Hi 宋文武,

I didn't notice this patch and ended up reimplementing another fstrim-service-type at [1].
Where we differ in the implementations:
* I didn't attempt to implement anacron capabilities, since I made the scheduling configurable.
* Uses define-configuration which can embed documentation and generate it for the manual.
* Slightly more “guix-y” style of configuration.


[1]: <https://issues.guix.gnu.org/61964>


Apologies for the duplicated effort!


Cheers,
Bruno

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Bruno Victal <mirai <at> makinata.eu>
Cc: 61964-done <at> debbugs.gnu.org, 58086-done <at> debbugs.gnu.org
Subject: Re: bug#61964: [PATCH] services: Add fstrim-service-type.
Date: Wed, 22 Mar 2023 10:13:03 -0400

Hi,

Bruno Victal <mirai <at> makinata.eu> writes:

> * gnu/services/linux.scm (fstrim-service-type): New variable.
> (fstrim-mcron-job, serialize-fstrim-configuration)
> (fstrim-serialize-list-of-strings, fstrim-serialize-boolean): New procedure.
> (mcron-time?): New predicate.
> (fstrim-configuration): New record.
> * doc/guix.texi (Linux Services): Document new fstrim-service-type.

I've installed the change, with the following mostly cosmetic adjustments:

--8<---------------cut here---------------start------------->8---
modified   doc/guix.texi
@@ -37493,7 +37493,7 @@ notifications.
 The command @command{fstrim} can be used to discard (or @dfn{trim})
 unused blocks on a mounted file system.
 
-@c This was copied from the fstrim manpage, with some texinfo touch-ups.
+@c This was copied from the fstrim manpage, with some Texinfo touch-ups.
 @quotation Warning
 Running @command{fstrim} frequently, or even using
 @command{mount -o discard}, might negatively affect the lifetime of
@@ -37540,8 +37540,8 @@ Verbose execution.
 Suppress error messages if trim operation (ioctl) is unsupported.
 
 @item @code{extra-arguments} (type: maybe-list-of-strings)
-Extra options to append to @command{fstrim} command.@footnote{Run
-@samp{man fstrim} for more information.}
+Extra options to append to @command{fstrim} (run @samp{man fstrim} for
+more information).
 
 @end table
 @end deftp
modified   gnu/services/linux.scm
@@ -185,10 +185,9 @@ (define (fstrim-serialize-list-of-strings field-name value)
 
 (define-configuration fstrim-configuration
   (package
-   (file-like util-linux)
-   "The package providing the @command{fstrim} command."
-   empty-serializer)
-
+    (file-like util-linux)
+    "The package providing the @command{fstrim} command."
+    empty-serializer)
   (schedule
    (mcron-time "0 0 * * 0")
    "Schedule for launching @command{fstrim}.  This can be a procedure, a list
@@ -196,8 +195,7 @@ (define-configuration fstrim-configuration
 Job specification, mcron, the mcron manual}.  By default this is set to run
 weekly on Sunday at 00:00."
    empty-serializer)
-
-  ;; fstrim options
+  ;; The following are fstrim-related options.
   (listed-in
    (maybe-list-of-strings '("/etc/fstab" "/proc/self/mountinfo"))
    ;; Note: documentation sourced from the fstrim manpage.
@@ -205,27 +203,19 @@ (define-configuration fstrim-configuration
 empty files are silently ignored.  The evaluation of the list @emph{stops}
 after the first non-empty file.  File systems with @code{X-fstrim.notrim} mount
 option in fstab are skipped.")
-
   (verbose?
    (boolean #t)
    "Verbose execution.")
-
   (quiet-unsupported?
    (boolean #t)
    "Suppress error messages if trim operation (ioctl) is unsupported.")
-
   (extra-arguments
    maybe-list-of-strings
-   ;; Tracked at: <https://issues.guix.gnu.org/62374>.
-   ;; FIXME <at> GUILE(TEXINFO): @footnote causes errors when calling
-   ;;                       configuration->documentation.
-   ;; > Throw to key `parser-error' with args `(#f "Unknown command" footnote)'
-   "Extra options to append to @command{fstrim} command.@footnote{Run
-@samp{man fstrim} for more information.}"
+   "Extra options to append to @command{fstrim} (run @samp{man fstrim} for
+more information)."
    (lambda (_ value)
      (if (maybe-value-set? value)
          value '())))
-
   (prefix fstrim-))
 
 (define (serialize-fstrim-configuration config)
--8<---------------cut here---------------end--------------->8---

Thank you!

-- 
Maxim

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

From: 宋文武 <iyzsong <at> envs.net>
To: Bruno Victal <mirai <at> makinata.eu>
Cc: Ludovic Courtès <ludo <at> gnu.org>, 58086 <at> debbugs.gnu.org,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: Re: [bug#58086] [PATCH] gnu: Add fstrim-service-type.
Date: Fri, 24 Mar 2023 18:28:21 +0800

Bruno Victal <mirai <at> makinata.eu> writes:

> I didn't notice this patch and ended up reimplementing another fstrim-service-type at [1].
> Where we differ in the implementations:
> * I didn't attempt to implement anacron capabilities, since I made the scheduling configurable.
> * Uses define-configuration which can embed documentation and generate it for the manual.
> * Slightly more “guix-y” style of configuration.

Yes, that's indeed better, thank you!

I had forgot my patch...

Previous Next

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