WIP: Refactor systemd and use /etc/default/zfs if available

[FransUrbo]

Rearrangement of some source directories.

• Move the systemd files into the contrib directory. This isn't really part of the ZFS/ZoL code, so it's more appropriate to put it in the contrib directory.

Use the /etc/{default,sysconfig}/zfs file in systemd service files (1of3).

• Instead of looing for the hardcoded @sysconfdir@/zfs/zpool.cache file, use the $ZFS_CACHE if/when set.
• Instead of using hardcoded /dev/disk/by-id dir in import, use the ZPOOL_IMPORT_PATH if/when set.
• Use ZPOOL_IMPORT_OPTS if/when applicable.

Use the /etc/{default,sysconfig}/zfs file in systemd service files (2of3).

• Use VERBOSE_MOUNT, DO_OVERLAY_MOUNTS and/or MOUNT_EXTRA_OPTIONS to zfs mount if/when applicable.

Use the /etc/{default,sysconfig}/zfs file in systemd service files (3of3).

• Use the ZED_ARGS variable when starting zed if/when applicable.

This whole PR is untested, so I'm tagging this WIP.

[FransUrbo]

The first commit isn't really necessary, but I've been increasingly ... "annoyed" that there's stuff that isn't really part of ZoL in the tree. I think it makes more sense to have all that in the contrib directory (which we created it for). So this PR includes a move of the systemd directory into contrib/.

[FransUrbo]

@behlendorf Do we have an (approximate) ETA on this?

Tag this 0.7.0?

[behlendorf]

I don't think this belongs in contrib. This is code we support and maintain as part of the project so I'd like to leave where it is.

[behlendorf]

My only concern with this is that VERBOSE_MOUNT="no" and DO_OVERLAY_MOUNT="no". That means both these options will be enabled when they should be disabled. How about limiting this to just MOUNT_EXTRA_OPTIONS which will work correctly.

[behlendorf]

@FransUrbo sorry about completely neglecting this. The gist of these changes look entirely reasonable to me. If you can address the issues I mentioned and rebase this against master without the "Rearrangement of some source directories" patch I don't see why we can't get it tested and added as part of the other proposed systemd changes.

[FransUrbo]

@FransUrbo sorry about completely neglecting this.

That's ok I guess, I've grown used to it :)
If you can address the issues I mentioned and rebase this against master without the "Rearrangement of some source directories" patch I don't see why we can't get it tested and added as part of the other proposed systemd changes.

In my opinion, everything that isn't part of the core ZFS code, should
go into contrib. It's technically the work of the downstream maintainers
to deal with this, not us.

But since there haven't been any, it made sense for us to do it. And
we can keep doing it for everyone that prefer to install from source.

But I think contrib is correct in this case. However, we can leave that
discussion to later if you're adamant about this.

[behlendorf]

Let's leave the contrib argument for latter and move forward on the rest of these changes. :)

[FransUrbo]

@behlendorf Ok, I've updated the PR with your remarks fixed. I updated the comments to, maybe you should merge them into one commit when/if accepting it.

[FransUrbo]

Looking at this and your #4699 again, I think it's possible to merge the two import files into one and just update the documentation for /etc/{sysconfig,default}/zfs to indicate that one could use the ZPOOL_IMPORT_OPTS with systemd to choose between importing with cache or scan.

[FransUrbo]

I'm pretty sure we can write the one line equivalence of

Pseudo code:

if cache exists; then
use cache file
else if ZPOOL_IMPORT_PATH is set; then
use ZPOOL_IMPORT_PATH
fi

[behlendorf]

Let's leave these two files separate for now. That allows for each to enabled/disabled independently.

[behlendorf]

When testing this under CentOS 7 I ran in to numerous problems.

• The environment variable substitutions aren't working, this may be because systemd does its own parsing of the unit files so normal shell syntax isn't supported.
• initconfdir replacement isn't defined and needs to be added to the Makefile.am: -e 's,@initconfdir\@,$(DEFAULT_INITCONF_DIR),g'.
• Variables appear not to be allowed in ConditionPathExists.

Adding this kind of support I think is a good idea but it needs more testing.

[behlendorf]

Closing. While I think this is a good idea the proposed patches just don't work. I ran in to numerous problems as described in the comment above. If someone would like to add this functionality they should open a new PR.