Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tasks/btrfs: add services.btrfs.autoScrub #32496

Merged
merged 1 commit into from Dec 17, 2017

Conversation

florianjacob
Copy link
Contributor

@florianjacob florianjacob commented Dec 9, 2017

for automatic regular scrubbing of mounted btrfs filesystems,
similar to what's already there for zfs.

Motivation for this change

Regular scrubs make sure bitrot gets detected early, and gets corrected when there's a copy available. 🔧

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

}
];

# This will yield duplicated units when If the user mounts a filesystem multiple times
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment seems a bit strange here: "… units when If the user …"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the hint!

@@ -31,5 +68,56 @@ in
''
btrfs device scan
'';
};
}
// mkIf cfgScrub.enable {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work as expected, I though you needed to have mkMerge or something? In particular, I would assume the // will remove the (any (fs: fs == "btrfs") config.boot.supportedFilesystems) condition of the above code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This indeed broke my initrd, as I discovered on booting today, argh. 🤦‍ You're totally right.

I updated it to use mkMerge in a similar way to what the zfs module does, now I see why they're doing it that way.

@florianjacob florianjacob force-pushed the btrfs-autoScrub branch 2 times, most recently from 27fdcc8 to a080e74 Compare December 9, 2017 14:58
@florianjacob
Copy link
Contributor Author

I additionally added a paragraph about this option to the 1803 changelog.

services.btrfs.autoScrub.fileSystems = mkDefault (mapAttrsToList (name: fs: fs.mountPoint)
(filterAttrs (name: fs: fs.fsType == "btrfs") config.fileSystems));

# did not manage to do it via the usual btrfs-scrub@.timer/.service
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious, what issues did you encounter using the template units?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Not a blocker, just to be clear)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joachifm the full story:

What I initially tried to achieve was this, which seems to be the usual solution on other distros (copied from Arch Linux):

btrfs-scrub@.timer:

[Unit]
Description=Monthly Btrfs scrub on %f

[Timer]
OnCalendar=monthly
AccuracySec=1d
Persistent=true

[Install]
WantedBy=multi-user.target

btrfs-scrub@.service:

[Unit]
Description=Btrfs scrub on %f

[Service]
Nice=19
IOSchedulingClass=idle
ExecStart=/usr/bin/btrfs scrub start -B %f

enabling specific units for scrubbing:

> systemctl enable btrfs-scrub@$(systemd-escape /).timer
Created symlink /etc/systemd/system/multi-user.target.wants/btrfs-scrub@-.timer → /usr/lib/systemd/system/btrfs-scrub@.timer.
> systemctl enable btrfs-scrub@$(systemd-escape /home).timer
Created symlink /etc/systemd/system/multi-user.target.wants/btrfs-scrub@-home.timer → /usr/lib/systemd/system/btrfs-scrub@.timer.

each parameterized timer unit will execute a corresponding parameterized service unit.

My issues:

Packing the units (without [Install], of course) works fine using systemd.services/timers."btrfs-scrub@". But enabling the parameterized timers was problematic: I came up with a few variants (should have kept the code to show it to you…), trying to use wants / wantedBy and aliases to create the required symlinks. But as soon as something like that is added to the timer, the timer file suddenly is empty like this:

[Unit]

[Timer]

without any error message. The only thing I noted in the logs was that sometimes it tried to start btrfs-scrub@timers.timer, but I have no idea where the timers parameterization came from - from timers.target? But how?

(I found no exapmle in NixOS where something similar is done.)

So I tried to have only a single timer, activating a list of services, but a timer can only start a single services. Which lead me to the non-templated version you see here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, alright, well I guess this can be rectified later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I was not sure whether that is actually wanted or preffered vs letting nix do all the work.
I extended the comment a little describing the issue, and marked it as a TODO.

@florianjacob florianjacob force-pushed the btrfs-autoScrub branch 3 times, most recently from a9f55e6 to be2d9bc Compare December 12, 2017 21:53
for automatic regular scrubbing of mounted btrfs filesystems,
similar to what's already there for zfs.
@joachifm joachifm merged commit 4fb4d2f into NixOS:master Dec 17, 2017
@florianjacob
Copy link
Contributor Author

@rycee and @joachifm, thank you for your reviews! 🍻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants