search

kdave / btrfsmaintenance

Scripts for btrfs maintenance tasks like periodic scrub, balance, trim or defrag on selected mountpoints or directories.
GNU General Public License v2.0
900 stars 79 forks source link

[RFC] Add ConditionPathIsReadWrite= to service files #56

Open super7ramp opened 6 years ago

super7ramp commented 6 years ago

@sysrich proposed to drop btrfsmaintenance from openSUSE's patterns-base because of an issue with read-only filesystems:

periodic trim, scrub, etc, are also all broken on Kubic and the Transactional System Role on Tumbleweed and Leap (how do you scrub a readonly root filesystem?)

@DimStar77 suggested to use ConditionPathIsReadWrite=… in service file instead in order to just skip the task if the concerned mountpoint is read-only.

Here's a proposal to make btrfsmaintenance-refresh script able to add this condition. Open to any comments.

sysrich commented 6 years ago

While I like this idea, I think we should reach out to the maintainers of btrfsprogs about fixing the handling of scrub/trim/etc on read-only root filesystems

All of those functions should work on devices even when the filesystem is read-only - this can be proven by doing a scrub on a read-write subvolume on a read-only root filesystem - the entire filesystem, not just the read-write subvolume is scrubbed

So, the fact that btrfsprogs are failing on a read-only rootfs is a bug, which this PR worksaround, but I'm not sure we really want it to workaround that incorrect behaviour

thkukuk commented 6 years ago

fstrim seems to work:

fstrim -av /var: 8.4 GiB (8988311552 bytes) trimmed /: 18.8 GiB (20178690048 bytes) trimmed

Defrag doesn't seem to work at all.

For balance and scrub: with this patch, if you have one read-only filesystem in your list, NO filesystem will be balanced or scrubed! Never. That's why I don't like this solution: it only hides the problem, but does not solve it.

As the paths are configure options in /etc/sysconfig/btrfsmaintenance, it would be better to find more clever solutions. If the path is "/", change it automatically to "/.snapshots" as we do already in a lot of other tools. If something is read-only, print a warning and skip the entry, so that the admin knows that something is wrongly configured and can fix it, but never ignore all entries and do nothing.