[PATCH] gnu: Add fstrim-service-type.

  • Done
  • quality assurance status badge
Details
5 participants
  • iyzsong
  • Ludovic Courtès
  • Christopher Baines
  • Maxim Cournoyer
  • Bruno Victal
Owner
unassigned
Submitted by
iyzsong
Severity
normal
Merged with

Debbugs page

I
I
iyzsong wrote on 26 Sep 2022 00:28
(address . guix-patches@gnu.org)(name . 宋文武)(address . iyzsong@member.fsf.org)
20220926072825.4689-1-iyzsong@envs.net
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.
---
gnu/services/admin.scm | 56 +++++++++++++++++++++++++++++++++++++++++-
gnu/services/mcron.scm | 8 ++++++
2 files changed, 63 insertions(+), 1 deletion(-)

Toggle diff (114 lines)
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
L
L
Ludovic Courtès wrote on 6 Oct 2022 14:00
(address . iyzsong@envs.net)
87a668hdlz.fsf@gnu.org
Hi,

iyzsong@envs.net skribis:

Toggle quote (11 lines)
> 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. :-)


[...]

Toggle quote (20 lines)
> +(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?

Toggle quote (9 lines)
> +(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?

Toggle quote (11 lines)
> +++ 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’.
C
C
Christopher Baines wrote on 3 Nov 2022 08:44
tag 58086 moreinfo
(address . control@debbugs.gnu.org)
87a658qbz3.fsf@cbaines.net
tags 58086 + moreinfo
quit
B
B
Bruno Victal wrote on 22 Mar 2023 05:03
Re: [bug#58086] [PATCH] gnu: Add fstrim-service-type.
(address . iyzsong@envs.net)
1a1991d3-afc0-4e2b-50a4-dfad763907f7@makinata.eu
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.




Apologies for the duplicated effort!


Cheers,
Bruno
M
M
Maxim Cournoyer wrote on 22 Mar 2023 07:08
control message for bug #58086
(address . control@debbugs.gnu.org)
875yasj2tp.fsf@gmail.com
merge 58086 61964
quit
M
M
Maxim Cournoyer wrote on 22 Mar 2023 07:13
Re: bug#61964: [PATCH] services: Add fstrim-service-type.
(name . Bruno Victal)(address . mirai@makinata.eu)
874jqcj2mo.fsf_-_@gmail.com
Hi,

Bruno Victal <mirai@makinata.eu> writes:

Toggle quote (7 lines)
> * 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:

Toggle snippet (78 lines)
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@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)

Thank you!

--
Maxim
Closed
宋
宋
宋文武 wrote on 24 Mar 2023 03:28
Re: [bug#58086] [PATCH] gnu: Add fstrim-service-type.
(name . Bruno Victal)(address . mirai@makinata.eu)
87o7oijvei.fsf@envs.net
Bruno Victal <mirai@makinata.eu> writes:

Toggle quote (6 lines)
> 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...
?
Your comment

This issue is archived.

To comment on this conversation send an email to 58086@patchwise.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 58086
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch