| Message ID | 20250122020025.GL1611770@frogsfrogsfrogs (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | xfs_scrub_all.timer: don't run if /var/lib/xfsprogs is readonly | expand |
On Tue, Jan 21, 2025 at 06:00:25PM -0800, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > The xfs_scrub_all program wants to write a state file into the package > state dir to keep track of how recently it performed a media scan. > Don't allow the systemd timer to run if that path isn't writable. Why would the path not be writable? Do we need a different place for it that is guaranteed to be writable even for setups they try to keep much of the system read-only?
On Wed, Jan 22, 2025 at 07:02:30AM +0100, Christoph Hellwig wrote: > On Tue, Jan 21, 2025 at 06:00:25PM -0800, Darrick J. Wong wrote: > > From: Darrick J. Wong <djwong@kernel.org> > > > > The xfs_scrub_all program wants to write a state file into the package > > state dir to keep track of how recently it performed a media scan. > > Don't allow the systemd timer to run if that path isn't writable. > > Why would the path not be writable? Do we need a different place for > it that is guaranteed to be writable even for setups they try to keep > much of the system read-only? Eh, it's mostly to shut up systems where even /var/lib is readonly. IIRC systemd's volatile mode isn't quite that silly, but it'd be nice to avoid xfs_scrub_all repeatedly failing every time the timer fires. OTOH maybe a better solution is just to run scrub in media scan mode if the media scan stamp file can't be opened for writing? --D
On Tue, Jan 21, 2025 at 11:18:29PM -0800, Darrick J. Wong wrote: > Eh, it's mostly to shut up systems where even /var/lib is readonly. What are those systems? They must still provide some place writable for semi-persistant data, so we should look for that? > IIRC systemd's volatile mode isn't quite that silly, but it'd be nice > to avoid xfs_scrub_all repeatedly failing every time the timer fires. > > OTOH maybe a better solution is just to run scrub in media scan mode if > the media scan stamp file can't be opened for writing? Or not run it at all? Either way I'd like to understand what causes this.
diff --git a/scrub/Makefile b/scrub/Makefile index 1e1109048c2a83..934b9062651bf1 100644 --- a/scrub/Makefile +++ b/scrub/Makefile @@ -108,10 +108,14 @@ endif # Automatically trigger a media scan once per month XFS_SCRUB_ALL_AUTO_MEDIA_SCAN_INTERVAL=1mo -LDIRT = $(XFS_SCRUB_ALL_PROG) $(XFS_SCRUB_FAIL_PROG) *.service *.cron +LDIRT = $(XFS_SCRUB_ALL_PROG) $(XFS_SCRUB_FAIL_PROG) *.service *.cron xfs_scrub_all.timer default: depend $(LTCOMMAND) $(XFS_SCRUB_ALL_PROG) $(XFS_SCRUB_FAIL_PROG) $(OPTIONAL_TARGETS) +xfs_scrub_all.timer: xfs_scrub_all.timer.in $(builddefs) + @echo " [SED] $@" + $(Q)$(SED) -e "s|@pkg_state_dir@|$(PKG_STATE_DIR)|g" < $< > $@ + xfs_scrub_all: xfs_scrub_all.in $(builddefs) @echo " [SED] $@" $(Q)$(SED) -e "s|@sbindir@|$(PKG_SBIN_DIR)|g" \ diff --git a/scrub/xfs_scrub_all.timer b/scrub/xfs_scrub_all.timer.in similarity index 77% rename from scrub/xfs_scrub_all.timer rename to scrub/xfs_scrub_all.timer.in index f0c557fc380391..9008f036d496c0 100644 --- a/scrub/xfs_scrub_all.timer +++ b/scrub/xfs_scrub_all.timer.in @@ -1,10 +1,11 @@ # SPDX-License-Identifier: GPL-2.0-or-later # -# Copyright (C) 2018-2024 Oracle. All Rights Reserved. +# Copyright (C) 2018-2025 Oracle. All Rights Reserved. # Author: Darrick J. Wong <djwong@kernel.org> [Unit] Description=Periodic XFS Online Metadata Check for All Filesystems +ConditionPathIsReadWrite=@pkg_state_dir@ [Timer] # Run on Sunday at 3:10am, to avoid running afoul of DST changes