From: Stoiko Ivanov <s.ivanov@proxmox.com>
To: Maximiliano Sandoval <m.sandoval@proxmox.com>
Cc: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH storage v2] api: disks: zfs: enable monthly scrub when creating a pool
Date: Wed, 28 Aug 2024 10:14:58 +0200 [thread overview]
Message-ID: <20240828101458.3ed1c6fe@rosa.proxmox.com> (raw)
In-Reply-To: <20240827142433.357779-1-m.sandoval@proxmox.com>
On Tue, 27 Aug 2024 16:24:33 +0200
Maximiliano Sandoval <m.sandoval@proxmox.com> wrote:
> And disable it when destroying the pool.
>
can be fine in general - but why?
our packages ship:
/etc/cron.d/zfsutils-linux (and /usr/lib/zfs-linux/scrub)
which does (afair) run a monthly scrub (and trim) under certain conditions
(including by default on healthy pools)
what do we gain by explicitly enabling a unit for scrubbing (which at
least from experiences with the per-pool import unit, will cause odd
warnings in the journal, resulting in support requests)?
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
>
> Differences from v1:
> - Add --now flag to commands so they are `started` on top of `enabled`.
a short explanation on why this is/was necessary would help (maybe even
with a link to the relevant documentation e.g.:
https://documentation.suse.com/smart/systems-management/html/systemd-working-with-timers/index.html#systemd-timer-manage
>
>
> src/PVE/API2/Disks/ZFS.pm | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/src/PVE/API2/Disks/ZFS.pm b/src/PVE/API2/Disks/ZFS.pm
> index 6fb6bd6b..6d9ace5b 100644
> --- a/src/PVE/API2/Disks/ZFS.pm
> +++ b/src/PVE/API2/Disks/ZFS.pm
> @@ -490,6 +490,13 @@ __PACKAGE__->register_method ({
> run_command($cmd);
> }
>
> + if (-e '/lib/systemd/system/zfs-scrub-monthly@.timer') {
> + my $scrubtimer = 'zfs-scrub-monthly@'. PVE::Systemd::escape_unit($name, undef) . '.timer';
> + $cmd = ['systemctl', 'enable', $scrubtimer, '--now'];
> + print "# ", join(' ', @$cmd), "\n";
> + run_command($cmd);
> + }
> +
> PVE::Diskmanage::udevadm_trigger($devs->@*);
>
> if ($param->{add_storage}) {
> @@ -583,6 +590,11 @@ __PACKAGE__->register_method ({
> run_command(['systemctl', 'disable', $importunit]);
> }
>
> + if (-e '/lib/systemd/system/zfs-scrub-monthly@.timer') {
> + my $scrubtimer = 'zfs-scrub-monthly@' . PVE::Systemd::escape_unit($name) . '.timer';
> + run_command(['systemctl', 'disable', $scrubtimer, '--now']);
> + }
> +
> run_command(['zpool', 'destroy', $name]);
>
> my $config_err;
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
prev parent reply other threads:[~2024-08-28 8:14 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-27 14:24 Maximiliano Sandoval
2024-08-28 8:14 ` Stoiko Ivanov [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240828101458.3ed1c6fe@rosa.proxmox.com \
--to=s.ivanov@proxmox.com \
--cc=m.sandoval@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox