Re: [RFC proxmox{,-backup} 0/6] add scheduled fstrim job for datastore's backing filesystems

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Am 19.03.26 um 15:37 schrieb Christian Ebner:
> As reported in the community forum [0], the default systemd service
> to run fstrim does not cover datastores mounted via systemd mount
> unit, since the fstrim command is invoked via:
> ```
> fstrim --listed-in /etc/fstab:/proc/self/mountinfo ...
> ```
> which however only considers the list up to the first non-empty
> file according to the man page [1].

But basically that means systemd would already be totally fine with
fstrim'ing all mounted file-systems, not sure if its really worth to
reimplement this ourselves, especially given that a single datastore
doesn't exactly map to one filesystem. Also, ZFS, a popular choice for
datastores, comes with it's own trim handling triggering every first
Sunday of a month (see /etc/cron.d/zfsutils-linux).

I'd rather prefer leveraging what's existing compared to carry our own
implementation here, not just for the (probably not *that* big)
maintenance burden, but also because it increases complexity in general
by re-implementing parts of systemd ourselves (can be fine, if there's
a good reason, I'm not yet convinced that this is one).

I'd rather hook into fstrim.service/timer (e.g. through an override that
can add another ExecStart). If it turns out that this is so frequently
in need for per-datastore configuration with good use cases where such
an approach really falls short, we can still provide actual integration
into api/ui/... and OTOH, we cannot easily drop such an integration like
here again.

> To allow for easy configuration of scheduled fstrims also on
> filesystems backing datastores in PBS, implement a scheduled job
> with per-datastore schedule configuration. Enable and default to
> executing the fstrim job for new datastores (except crated via ZFS
> dialog).
> 
> Open question remaining:
> How to best handle datastores located on ZFS? Should the command default
> to zpool trim? Should it set `autotrim=on` on datastore creation instead
> and silently ignore as it is now?

see above, it's already handled.