On Wed, Jan 31, 2018 at 02:30:22PM -0600, Eric Sandeen wrote:

On 1/31/18 12:41 AM, Darrick J. Wong wrote:

quoted

From: Darrick J. Wong <redacted>

Create a systemd service unit so that we can run the online scrubber
under systemd with (somewhat) appropriate containment.

Signed-off-by: Darrick J. Wong <redacted>
---
v2: fix some of the debian packaging weirdness, kudos to Nathan Scott!

Er, hang on:

checking for SYSTEMD... configure: error: Package requirements (systemd) were not met:

now systemd is a /build/ requirement for xfsprogs?  nope nope. :)

This is only used for unit file install targets, right?  If systemd
is missing, surely those install targets should just be skipped.

Looks like the Makefile DTRT, but the stuff in m4/ should be non-fatal?

(I can't tell what makes this fatal ... I think its' fatal?

Yeah, I forgot to hoist the pkg-config bits into the if-true and
neutralize the if-not parts of the PKG_CHECK_MODULES call.

--D

quoted

checking for SYSTEMD... configure: error: Package requirements (systemd) were not met:

No package 'systemd' found

Consider adjusting the PKG_CONFIG_PATH environment variable if you
installed software in a non-standard prefix.

Alternatively, you may set the environment variables SYSTEMD_CFLAGS
and SYSTEMD_LIBS to avoid the need to call pkg-config.
See the pkg-config man page for more details.

# echo $?
1

looks fatal.)

...

quoted

diff --git a/scrub/Makefile b/scrub/Makefile
index ca6dab0..0632794 100644
--- a/scrub/Makefile
+++ b/scrub/Makefile

@@ -15,6 +15,19 @@ LTCOMMAND = xfs_scrub
 INSTALL_SCRUB = install-scrub
 XFS_SCRUB_ALL_PROG = xfs_scrub_all
 XFS_SCRUB_ARGS = -b -n
+ifeq ($(HAVE_SYSTEMD),yes)
+INSTALL_SCRUB += install-systemd
+SYSTEMD_SERVICES = xfs_scrub@.service xfs_scrub_all.service xfs_scrub_all.timer xfs_scrub_fail@.service
+OPTIONAL_TARGETS += $(SYSTEMD_SERVICES)
+endif
+ifeq ($(HAVE_CROND),yes)
+INSTALL_SCRUB += install-crond
+CRONTABS = xfs_scrub_all.cron
+OPTIONAL_TARGETS += $(CRONTABS)
+# Don't enable the crontab by default for now
+CROND_DIR = $(PKG_LIB_DIR)/$(PKG_NAME)
+endif

Wouldn't an else if arrangement make more sense?  Why would we install both?
I'd have expected "Install systemd if available, else install
crond if available, else don't install anything." 
... and then the cron job can stop checking for systemd too:

quoted

diff --git a/scrub/xfs_scrub_all.cron.in b/scrub/xfs_scrub_all.cron.in
new file mode 100644
index 0000000..3dea929
--- /dev/null
+++ b/scrub/xfs_scrub_all.cron.in

@@ -0,0 +1 @@
+10 3 * * 0 root test -e /run/systemd/system || @sbindir@/xfs_scrub_all

diff --git a/scrub/xfs_scrub_all.in b/scrub/xfs_scrub_all.in

unless I'm missing something?


Thanks,
-Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html