On Wed, Jan 31, 2018 at 02:30:22PM -0600, Eric Sandeen wrote: > On 1/31/18 12:41 AM, Darrick J. Wong wrote: > > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx> > > > > 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 <darrick.wong@xxxxxxxxxx> > > --- > > 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 > > > 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.) > > ... > > > > 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: > > > 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@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
