public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH storage v2] api: disks: zfs: enable monthly scrub when creating a pool
@ 2024-08-27 14:24 Maximiliano Sandoval
  2024-08-28  8:14 ` Stoiko Ivanov
  0 siblings, 1 reply; 2+ messages in thread
From: Maximiliano Sandoval @ 2024-08-27 14:24 UTC (permalink / raw)
  To: pve-devel

And disable it when destroying the pool.

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


 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;
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [pve-devel] [PATCH storage v2] api: disks: zfs: enable monthly scrub when creating a pool
  2024-08-27 14:24 [pve-devel] [PATCH storage v2] api: disks: zfs: enable monthly scrub when creating a pool Maximiliano Sandoval
@ 2024-08-28  8:14 ` Stoiko Ivanov
  0 siblings, 0 replies; 2+ messages in thread
From: Stoiko Ivanov @ 2024-08-28  8:14 UTC (permalink / raw)
  To: Maximiliano Sandoval; +Cc: Proxmox VE development discussion

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


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2024-08-28  8:14 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-08-27 14:24 [pve-devel] [PATCH storage v2] api: disks: zfs: enable monthly scrub when creating a pool Maximiliano Sandoval
2024-08-28  8:14 ` Stoiko Ivanov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal