Dependency related additions to the systemd generator

[InsanePrawn]

Needs some detail work, e.g. default values for the config vars.

As discovered in #9611, there is still some potentially desirable behaviours that the systemd generator does or does not show:

• Don't blindly symlink all generated mount units into local-fs.target.wants/
• create .mount units for datasets with canmount=noauto

Motivation and Context

• Don't blindly symlink all generated mount units into local-fs.target.wants/
  Admins might not want the default behaviour of systemd mounting every canmount=on dataset with a valid mount point during boot (e.g. to utilize .automount units). Added code to disable the symlinking based on a variable, leaving it enabled by default (same as old behaviour)
• create .mount units for datasets with canmount=noauto
  Added code to conditionally create .mount units for datasets with canmount=noauto, disabled by default (same as old behaviour)
  Caveats: Currently no default values for those config strings. Might want to discuss whether to check for 'yes' or 'no' for every variable to achieve the best possible default behaviour.

Description

• read the config from /etc/default/zfs (first commit):
  Looking for feedback! Is this the right file to store this config? Currently just sourcing it as a shell script, anything nicer? Config variable names - I think at least ZFS_SYSTEMD_NOAUTO_UNITS needs a rename. Suggestions?
• Don't blindly symlink all generated mount units into local-fs.target.wants/
  Skip symlinking altogether if config var is set. Currently always skips symlink creation for canmount=noauto datasets. Seems sane to me.
• create .mount units for datasets with canmount=noauto
  I've left the creation of these units disabled by default, as they would get pulled in by mount units of descending paths (which can come from outside ZFS!).
  This seems like the right thing to do in most situations, but I'd rather have a switch to control this rather than having to change pool configs or layouts to accommodate the generator, especially since this would be a breaking change.

from man systemd.mount:

   Implicit Dependencies
       The following dependencies are implicitly added:

       •   If a mount unit is beneath another mount unit in the file system hierarchy, both a requirement dependency and an ordering dependency between both units are created
           automatically.

How Has This Been Tested?

Very manually on my Debian 9 server. The variables seem to function correctly.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the ZFS on Linux code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• All new and existing tests passed.
• All commit messages are properly formatted and contain Signed-off-by.

[behlendorf]

cc: @aerusso @rlaager

[rlaager]

I made a commit that tweaks the if statement whitespace. It seemed easier to do that than to explain what I was thinking. That can be squashed when it comes time to merge or if you're respinning the PR anyway.

I am concerned that this will break if you have multiple datasets with the same mountpoint. As I said before: #9611 (comment)

However, if we have two datasets with the same mountpoint and one is noauto and one is “on”, we would have a problem. The unit name comes from the mountpoint (and AFAIK this is required). We would need to give precedence to the auto one. The current code handles this by skipping all noauto mounts. But once we lift that, it will be a problem.

The code currently refuses to overwrite existing mount files. If we changed that to only apply for noauto, then the auto one would overwrite if it came later, but would not be overwritten if it came first. This would achieve the right result without making the generator take two passes.

Can you change it to behave like that? Having multiple datasets with the same mountpoint will be very common with Ubuntu's zsys utility for root-on-ZFS setups, and not just for mountpoint=/.

Is there a particular reason we need an option to filter out the noauto mountpoints, as opposed to just always creating them? If nothing Wants= them, they're not going to have any effect anyway, right? Or is there an issue in the following scenario: if I have a canmount=noauto, mountpoint=/foo and a canmount=yes, mountpoint=/foo/bar, will the foo-bar.mount Wants foo.mount and mount /foo against the expressed noauto? Can you test that scenario? If that does not happen, then I think we can just always create the canmount=noauto units and eliminate that option.

Why is it that you want the option to NOT mount all the canmount=on datasets? I'm still not understanding your use case there. Can you try explaining that one more time?

[InsanePrawn]

That can be squashed when it comes time to merge or if you're respinning the PR anyway.

Ah, I was wondering what the preferred formatting there was. Apparently I also managed to be inconsistent about it 🙃
ACK, will squash on the next opportunity.

I am concerned that this will break if you have multiple datasets with the same mountpoint. As I said before: #9611 (comment)

However, if we have two datasets with the same mountpoint and one is noauto and one is “on”, we would have a problem. The unit name comes from the mountpoint (and AFAIK this is required). We would need to give precedence to the auto one. The current code handles this by skipping all noauto mounts. But once we lift that, it will be a problem.
The code currently refuses to overwrite existing mount files. If we changed that to only apply for noauto, then the auto one would overwrite if it came later, but would not be overwritten if it came first. This would achieve the right result without making the generator take two passes.

I will veto the overwrite feature - noauto or not - as this could also overwrite existing units created by non-zfs generators.
In order to achieve this, we would need some more intricate 'is the mount unit to be overwritten zfs AND noauto' type of logic. I'd be open to adding a sort -t \t -k 3,3 kind of logic to sort the FS list before parsing its lines, but I'm not sure how that affects the rest of the mount order.

Can you change it to behave like that? Having multiple datasets with the same mountpoint will be very common with Ubuntu's zsys utility for root-on-ZFS setups, and not just for mountpoint=/.

Or is there an issue in the following scenario: if I have a canmount=noauto, mountpoint=/foo and a canmount=yes, mountpoint=/foo/bar, will the foo-bar.mount Wants foo.mount and mount /foo against the expressed noauto?

Exactly. Please see the initial post, including a systemd manpage quote...

Why is it that you want the option to NOT mount all the canmount=on datasets? I'm still not understanding your use case there. Can you try explaining that one more time?

Enabling this behaviour basically makes systemd aware of the zfs mount points but causes minimal change in behaviour otherwise. The point is to make systemd aware of these possible mount points but only make systemd mount things that are directly needed by something, i.e. for daemons like libvirt. Leaving the mount handling for datasets not directly required by other things has many possible use cases, such as zfs mount -a, systemd automount units, and whatever weird stuff the admin might be doing. Making them all wants for local-fs.target is just one of many ways to have all the datasets mounted, I don't see why we should take the choice how to do so from the administrator, especially when the default stays 'mount all the things through systemd'.

I still have open questions around the config variables, such as whether we should have some error handling around missing or especially invalid values, renaming, etc.

[rlaager]

I will veto the overwrite feature - noauto or not - as this could also overwrite existing units created by non-zfs generators.

Good point, a ZFS noauto overwriting an auto from another generator (e.g. the fstab generator) would be bad.

In order to achieve this, we would need some more intricate 'is the mount unit to be overwritten zfs AND noauto' type of logic. I'd be open to adding a sort -t \t -k 3,3 kind of logic to sort the FS list before parsing its lines, but I'm not sure how that affects the rest of the mount order.

Another great point! That concept would work. As long as all the auto datasets are processed before the noauto datasets, we don't need overwriting. The ordering is otherwise irrelevant anyway, as systemd just sees a bunch of files on disk. The original order they were written doesn't matter.

Can you change it to behave like that? Having multiple datasets with the same mountpoint will be very common with Ubuntu's zsys utility for root-on-ZFS setups, and not just for mountpoint=/.
Or is there an issue in the following scenario: if I have a canmount=noauto, mountpoint=/foo and a canmount=yes, mountpoint=/foo/bar, will the foo-bar.mount Wants foo.mount and mount /foo against the expressed noauto?

Exactly. Please see the initial post, including a systemd manpage quote...

I tested, with ext4 in /etc/fstab. It does behave like this. systemd has changed the classic behavior of /etc/fstab mounting. I suppose that does make some amount of sense, as what is the point of having a /foo if you're not going to mount it and are going to mount /foo/bar? With /foo/bar mounted, mounting /foo will hide /foo/bar. Allowing that behavior in something newly designed (i.e. systemd) probably doesn't make much sense, so systemd is probably in the right here.

Given that /etc/fstab mounts behave this way, I don't see why it'd be a problem for ZFS either. Absent a good reason, I think we should behave the same way the fstab generator does, which would mean we should unconditionally create the units for noauto datasets.

Why is it that you want the option to NOT mount all the canmount=on datasets? I'm still not understanding your use case there. Can you try explaining that one more time?

Enabling this behaviour basically makes systemd aware of the zfs mount points but causes minimal change in behaviour otherwise. The point is to make systemd aware of these possible mount points but only make systemd mount things that are directly needed by something, i.e. for daemons like libvirt. Leaving the mount handling for datasets not directly required by other things has many possible use cases, such as zfs mount -a, systemd automount units, and whatever weird stuff the admin might be doing. Making them all wants for local-fs.target is just one of many ways to have all the datasets mounted, I don't see why we should take the choice how to do so from the administrator, especially when the default stays 'mount all the things through systemd'.

Do you personally have a use case for not mounting all the datasets with canmount=on? If so, what is it, and are you also disabling the zfs-mount.service that calls zfs mount -a?

[aerusso]

Sorry about the delayed response.

However, if we have two datasets with the same mountpoint and one is noauto and one is “on”, we would have a problem.

This is in regards to having multiple datasets with the same mountpoint, but the comment is true more generally: systemd doesn't "understand" the idea that a mountpoint might be mounted by different filesystems in any way (at least that I know of). I don't think we should try to shoehorn this into ZFS either.

I will veto the overwrite feature - noauto or not - as this could also overwrite existing units created by non-zfs generators.
In order to achieve this, we would need some more intricate 'is the mount unit to be overwritten zfs AND noauto' type of logic. I'd be open to adding a sort -t \t -k 3,3 kind of logic to sort the FS list before parsing its lines, but I'm not sure how that affects the rest of the mount order.

I agree---a mountpoint should unambiguously specify a dataset/filesystem from the systemd perspective, unless we have some better understanding of the mechanism systemd handles this.

In particular, I think we should detect if:

1. We are creating multiple canmount=noauto mount units
2. We are overwriting an existing .mount file (or, ideally, one that will be created by another mount generator)

and abort making any canmount=noauto units in that contingency. There may be some mechanism to run zfs-mount-generator after other ones (I think late-dir in man systemd.generator may be what we want got 2.).

I tested, with ext4 in /etc/fstab. It does behave like this. systemd has changed the classic behavior of /etc/fstab mounting. I suppose that does make some amount of sense, as what is the point of having a /foo if you're not going to mount it and are going to mount /foo/bar? With /foo/bar mounted, mounting /foo will hide /foo/bar. Allowing that behavior in something newly designed (i.e. systemd) probably doesn't make much sense, so systemd is probably in the right here.

Given that /etc/fstab mounts behave this way, I don't see why it'd be a problem for ZFS either. Absent a good reason, I think we should behave the same way the fstab generator does, which would mean we should unconditionally create the units for noauto datasets.

I'm not thrilled about this. There is going to be someone who complains about this. But I also don't think it's worth a fight.

Why is it that you want the option to NOT mount all the canmount=on datasets? I'm still not understanding your use case there. Can you try explaining that one more time?

Enabling this behaviour basically makes systemd aware of the zfs mount points but causes minimal change in behaviour otherwise. The point is to make systemd aware of these possible mount points but only make systemd mount things that are directly needed by something, i.e. for daemons like libvirt. Leaving the mount handling for datasets not directly required by other things has many possible use cases, such as zfs mount -a, systemd automount units, and whatever weird stuff the admin might be doing. Making them all wants for local-fs.target is just one of many ways to have all the datasets mounted, I don't see why we should take the choice how to do so from the administrator, especially when the default stays 'mount all the things through systemd'.

Do you personally have a use case for not mounting all the datasets with canmount=on? If so, what is it, and are you also disabling the zfs-mount.service that calls zfs mount -a?

Again, I don't see why we want to hack this in for zfs in particular. @InsanePrawn is there a reason you cannot just mask the mount files if you don't want them mounted?

As an aside, the "right" way to do this would be filesystem agnostic. One candidate would be having putting some property like systemd.target=late that could be specified on a filesystem in, e.g., /etc/fstab and then have some target late-fs.target that Wants= those mounts files (and local-fs.target doesn't). If such a feature were offered by systemd, I'd advocate using a user dataset property to support it.

[rlaager]

I'd advocate using a user dataset property to support it.

The following is getting somewhat far afield from this particular PR, but it may be worthwhile to support the equivalents of following at some point:

• x-systemd.requires=
• x-systemd.before=, x-systemd.after=
• x-systemd.requires-mounts-for=
• x-systemd.automount, x-systemd.idle-timeout=
• nofail

The other options don't seem to make sense for ZFS: https://www.freedesktop.org/software/systemd/man/systemd.mount.html#fstab

nofail actually somewhat fits into this discussion. We are currently treating everything with nofail (i.e. making local-fs.target Wants them rather than Requires them). @aerusso was that intentional?

Again, I'm coming at this from the perspective that we should match the "stock" (i.e. fstab generator) behavior as much as possible.

[aerusso]

I came from a perspective of balancing "stock" behavior of systemd-fstab-generator with not causing trouble (if zfs-mount.service fails, the system probably still boots).

I also think adding support for the x-systemd.* options is a good idea, but maybe out of scope (unless @InsanePrawn wants to do the legwork in this patch). As for nofail, that would ideally also be supported by zfs mount. I'm coming from the perspective that we want these settings to make sense independent of init system---which is particularly important because of the whole OpenZFS merge going on.

As for x-systemd.automount, I think it would be great to support it! If that's the main issue around the "late mounting" behavior desired by @InsanePrawn, then maybe that's the proximate solution.

As for the late-mounting behavior, the more I think about it, the more sure I am that systemd should implement a x-systemd.target=local-fs default, and allow it to be overridden, e.g. to zfs. And then we support that switch. If can come to some micro-consensus about this, I think we should file a feature request at systemd.

[rlaager]

I came from a perspective of balancing "stock" behavior of systemd-fstab-generator with not causing trouble (if zfs-mount.service fails, the system probably still boots).

Upon further reading, nofail is the default for the fstab generator. From systemd.mount again, "Specifically systemd-fstab-generator acts as though ... "fg,nofail" was appended [to the option list]." So we're doing the right thing.

I also think adding support for the x-systemd.* options is a good idea, but maybe out of scope (unless @InsanePrawn wants to do the legwork in this patch).

Agreed.

As for nofail, that would ideally also be supported by zfs mount. I'm coming from the perspective that we want these settings to make sense independent of init system---which is particularly important because of the whole OpenZFS merge going on.

Agreed, mostly. I think it's okay to have systemd-specific properties used only for systemd, and likewise for anything else. Perhaps they should be user properties. I'm not sure what we should name them. Using e.g. org.freedesktop.systemd:automount would follow the convention for user properties, but in theory we'd need sign-off from the systemd folks, since it's their domain. Using x-systemd.automount to match fstab might not be a terrible idea, but then they'd have to be "real" properties? That's probably the way I'd lean, but we'll have to see what Brian thinks.

As for x-systemd.automount, I think it would be great to support it! If that's the main issue around the "late mounting" behavior desired by @InsanePrawn, then maybe that's the proximate solution.

Indeed, but either way, I still want to hear a use case articulated by @InsanePrawn or whoever has such a use case.

As for the late-mounting behavior, the more I think about it, the more sure I am that systemd should implement a x-systemd.target=local-fs default, and allow it to be overridden, e.g. to zfs. And then we support that switch. If can come to some micro-consensus about this, I think we should file a feature request at systemd.

I'm still struggling to understand why someone wants to mount a local filesystem at boot but does not want it to be wanted by local-fs.target. This is not ZFS-specific behavior.

To recap my understanding of the current status... I'd like to see the following changes:

1. Sort auto before noauto in the processing loop, as suggested by @InsanePrawn.
2. Eliminate the ZFS_SYSTEMD_NOAUTO_UNITS option, always writing them. This is change from previous generator behavior. We will be writing noauto mountpoints but not Wantsing them, so they will not mount by default. This is consistent with the fstab generator. I (or someone) will need to do some careful testing with root-on-ZFS dataset layouts to be 100% sure this is okay there.
3. Eliminate the ZFS_SYSTEMD_REQUIRE_MOUNTS option, keeping the current behavior. Alternatively, provide a compelling use case that is yours personally (i.e. not hypothetical). If this is not something where ZFS is inherently different than /etc/fstab, we probably need to take that case to upstream systemd too, so that we can match whatever solution they choose for /etc/fstab.
4. From @aerusso, "If there are multiple canmount=noauto datasets, do not write units for any of them."

[aerusso]

@rlaager I think we're basically on the same page, but I'd like to add one more behavior to this list of requests:

4. If there are multiple canmount=noauto datasets, do not write units for any of them.

[rlaager]

If there are multiple canmount=noauto datasets, do not write units for any of them.

I have no objection to that. Is there a way to easily implement that in sh without making this two-pass? It's undoubtedly going to involve the sort being first by canmount, then mountpoint.

[aerusso]

We're starting to push the limits of shell script, but you could do something like

if [ -e "${mountfile}" ] ; then
   if [ "$(head -n1 "$mountfile")" = "# Automatically generated by zfs-mount-generator" ] ; then
       if [ "${p_canmount}" = "on"  ; then
         #overwrite
       elif # mountfile is a link to /dev/null or is not WantedBy=local-fs.target
         continue # to next dataset
       elif
         ln -sf /dev/null "${mountfile}"
       fi
   else
      continue #to next dataset
   fi
fi

Of course this isn't even remotely tested. We might also want to clean up all the /dev/null symlinks afterwards. We should also probably put canmount=noauto stuff in generator.late, so that anything else that provides those mounts doesn't get confused.

Also, this is starting to get to the point that it should be compiled. It's fine to add this, and I actually think we should get a good script implementation together before rewriting it in C so we can iterate faster. This is definitely out of scope for this PR, though.

[rlaager]

It might make sense to rewrite the generator in Python, either as a permanent solution or as a middle step on the way to C.

[aerusso]

Given how early in the boot systemd generators are run (i.e., first thing after PID1 is called), python may not be available. I don't think porting this to Python is a good idea.

[InsanePrawn]

Hello again, sorry I went MIA for a little, RL caught up with me.

Now, where were we?

I also think adding support for the x-systemd.* options is a good idea, but maybe out of scope (unless @InsanePrawn wants to do the legwork in this patch).

Agreed.

It looks like one could just pour the contents of those properties' values into corresponding lines in the generated units, right?
I might look into doing that in a followup PR.

As for nofail, that would ideally also be supported by zfs mount. I'm coming from the perspective that we want these settings to make sense independent of init system---which is particularly important because of the whole OpenZFS merge going on.

Agreed, mostly. I think it's okay to have systemd-specific properties used only for systemd, and likewise for anything else. Perhaps they should be user properties. I'm not sure what we should name them. Using e.g. org.freedesktop.systemd:automount would follow the convention for user properties, but in theory we'd need sign-off from the systemd folks, since it's their domain. Using x-systemd.automount to match fstab might not be a terrible idea, but then they'd have to be "real" properties? That's probably the way I'd lean, but we'll have to see what Brian thinks.

What about org.openzfs.systemd.generator:{before,after,...}?

As for x-systemd.automount, I think it would be great to support it! If that's the main issue around the "late mounting" behavior desired by @InsanePrawn, then maybe that's the proximate solution.

Indeed, but either way, I still want to hear a use case articulated by @InsanePrawn or whoever has such a use case.

As for the late-mounting behavior, the more I think about it, the more sure I am that systemd should implement a x-systemd.target=local-fs default, and allow it to be overridden, e.g. to zfs. And then we support that switch. If can come to some micro-consensus about this, I think we should file a feature request at systemd.

I'm still struggling to understand why someone wants to mount a local filesystem at boot but does not want it to be wanted by local-fs.target. This is not ZFS-specific behavior.

Alright, to summarize, one would want that because zfs mount -a fails somewhat gracefully, systemd mounting stuff does not. It waits around and puts red statuses and 'degraded' all over your system 🙃 For example, zfs mount -a will just skip datasets that don't have their crypto keys loaded. Systemd will halt boot and ask for passphrases if that's the key type used.

Let's get into an example, my workstation at work.
I have a rootfs on a pool with an encrypted, unmountable root dataset. The key is stored in a file, as specified in my keysource property.
I have /boot on LUKS, cause GRUB can ask me for the passphrase and decrypt that. My (LUKS encrypted...) initramfs contains a keyfile to unlock my OS datasets, including the rootfs, basically everything except some stuff that goes below my home directory.
Now, I do want systemd to automatically try to mount the rootfs and pull in the keyload unit for that.
In the future, I'd also like it to mount /var/lib/libvirt before libvirt.service starts.
Afterwards, zfs-mount.service executes zfs mount -a, mounting what it can.
As hinted at before, my pool contains a second encryptionroot, an unmountable dataset called 'cryptstore' below my home directory, that has a couple mountable children. This has a passphrase and is unlocked manually by me, usually from within my graphical desktop session. Basically I run zfs load-key and then zfs mount -a. Since I'm bad at keeping a work/life balance and separating the two, I use the 'cryptstore' to store (semi-)private stuff, like my spotify credentials.
Now, what I don't want is for systemd to halt boot or try to mount anything cryptstore related. However, without ZFS_SYSTEMD_REQUIRE_MOUNTS='no', it would do just that.
Now we could spend all day arguing that I should just mask home-prawn-cryptstore-keyload.service and get on with it. While that may be true for a single second encryption root, imagine having a server automatically house backups (raw sends!) of multiple servers. Now we're talking about extending the backup system to detect encryption roots and mask their keyload units or manual administrator intervention, otherwise your boot process will wait several minutes for those passphrases.

Okay, fine, then just don't use the generator on such a server, right?
Ah, remember good old libvirt.service? I still want systemd to mount whatever it needs to start a service beforehand. And no, I will not manually go around and create mount units for those datasets either. I want a working generator that does just enough to get a race-free systemd boot.

Some might argue the cryptstore datasets should be noauto, but I really like the fact that running mount -a any time after I unlocked the cryptstore dataset will make sure it and its children are mounted. Something I couldn't have with canmount=noauto. Plus, there's no zfs mount -r so far...

I am aware all of this is based on abusing the difference between zfs mount -a and zfs mount -al, but since this mechanic exists today and works, I don't see why we should refuse people this flexibility in the generator with a very simple config option I already implemented.

To recap my understanding of the current status... I'd like to see the following changes:

1. Sort `auto` before `noauto` in the processing loop, as suggested by @InsanePrawn.

2. Eliminate the `ZFS_SYSTEMD_NOAUTO_UNITS` option, always writing them. This is change from previous generator behavior. We will be writing `noauto` mountpoints but not `Wants`ing them, so they will not mount by default. This is consistent with the fstab generator. I (or someone) will need to do some careful testing with root-on-ZFS dataset layouts to be 100% sure this is okay there.

Sigh. They will mount if any other (ZFS or not) mount unit below their mount point ever gets activated. That might be the right thing to do in a lot of cases, and I'd be okay with making it the new default, but please give me the option to toggle this, even if it's as a user property.

3. Eliminate the `ZFS_SYSTEMD_REQUIRE_MOUNTS` option, keeping the current behavior. Alternatively, provide a compelling use case that is yours personally (i.e. not hypothetical). If this is not something where ZFS is inherently different than `/etc/fstab`, we probably need to take that case to upstream systemd too, so that we can match whatever solution they choose for `/etc/fstab`.

See two paragraphs above. :)

4. From @aerusso, "If there are multiple canmount=noauto datasets, do not write units for any of them."

What about keeping a list of mount units (file names, as they're the escaped version of the mount path?) of noauto units already generated by us this run.

if the unit is noauto:
    If our to be generated file name is in the list:
        [keep it in the list,] remove the file if it exists.
    If it's not && the file doesn't exist:
        put the name in the list, write the file.

By only tracking noauto datasets that we wrote, we avoid a whole bunch of problems like overwriting/deleting foreign mount units.

The first problem I see with this is the lack of arrays in POSIX shell. Sigh.
On second thought I can see that this fails to remove existing noauto units when you rerun the generator after creating a conflicting noauto dataset. Maybe checking for our header is indeed the way to go?

[rlaager]

I also think adding support for the x-systemd.* options is a good idea, but maybe out of scope (unless @InsanePrawn wants to do the legwork in this patch).

It looks like one could just pour the contents of those properties' values into corresponding lines in the generated units, right?

Yes.

What about org.openzfs.systemd.generator:{before,after,...}?

That's not bad. I'd probably drop the "generator" for less typing, so org.openzfs.systemd:{before,after,...}.

Some might argue the cryptstore datasets should be noauto, but I really like the fact that running mount -a any time after I unlocked the cryptstore dataset will make sure it and its children are mounted.

I have a couple of alternatives I'd like to explore.

A) Is it possible/easy to create additional target units? Could you create a cryptstore.target that Wants= the mount units for cryptstore and its children? Then they could be noauto and you could unlock/mount them with something like systemctl start cryptstore.target? I'm not sure how target units work, though.

B) Your proposed solution uses a global (system-wide) setting to fix a local problem with one dataset hierarchy. What if, instead, we had a user property which meant "don't wants this in local-fs.target"? User properties are inherited, so you would only need to set this on the cryptstore dataset and its children would automatically inherit it. They would get mount units and cryptstore would get the keyload unit, but they would not be automatically pulled in at boot. This seems like it would address both of your scenarios with more local granularity. It wouldn't interfere with stock (Ubuntu installer or HOWTO) root-on-ZFS datasets.

They will mount if any other (ZFS or not) mount unit below their mount point ever gets activated. That might be the right thing to do in a lot of cases, and I'd be okay with making it the new default, but please give me the option to toggle this, even if it's as a user property.

What's your use case here (for not wanting noauto units to be created at all)?

[InsanePrawn]

Some might argue the cryptstore datasets should be noauto, but I really like the fact that running mount -a any time after I unlocked the cryptstore dataset will make sure it and its children are mounted.

I have a couple of alternatives I'd like to explore.

A) Is it possible/easy to create additional target units? Could you create a cryptstore.target that Wants= the mount units for cryptstore and its children? Then they could be noauto and you could unlock/mount them with something like systemctl start cryptstore.target? I'm not sure how target units work, though.

I'm not exactly sure what you're trying to accomplish here, my goal is to have the cryptstore not handled through systemd.
Anyway, that's a lot of manual intervention to jump through just to not have that toggle...

B) Your proposed solution uses a global (system-wide) setting to fix a local problem with one dataset hierarchy. What if, instead, we had a user property which meant "don't wants this in local-fs.target"? User properties are inherited, so you would only need to set this on the cryptstore dataset and its children would automatically inherit it. They would get mount units and cryptstore would get the keyload unit, but they would not be automatically pulled in at boot. This seems like it would address both of your scenarios with more local granularity.

This might be an acceptable compromise. Given the fact that using inheritance, I could supposedly set it on my root dataset and therefore get the desired effect on the entire pool, I would be open to this, although it's not as nice for the backup host use case, received (user) properties and all.

It wouldn't interfere with stock (Ubuntu installer or HOWTO) root-on-ZFS datasets.

Side note: Nothing I proposed so far breaks or interferes with anything either, as I've kept the current behaviour the default.

They will mount if any other (ZFS or not) mount unit below their mount point ever gets activated. That might be the right thing to do in a lot of cases, and I'd be okay with making it the new default, but please give me the option to toggle this, even if it's as a user property.

What's your use case here (for not wanting noauto units to be created at all)?

As I said: If they're created and something else below them is activated, so will they. The only way to avoid this is to not create them.

Imagine dataset zroot/backups/remotemachine/a with canmount=noauto and mountpoint=/home/a. Your proposed behaviour for the generator is to just generate a mount unit A for it.
Now if anything else creates a mount unit B for /home/a/b, the activation of B will also pull in our A and relevant keyload units.
Sounds like it's the right thing to do 99% of the time, but what when it's not? You can try masking A, but this might entrail other problems, such as blocking other generators from creating that mount (what if you had a non-zfs entry for /home/a in your fstab? What about other weird generators we don't even know about?) and also leads back to manual intervention.

My overall use case here is to use the systemd generator only for satisfying a couple of service dependencies and keeping its nose out of my zfs for the rest of the day. I think that's the use case for most of the few people who activate it in the first place anyway.

I'm willing to take some initial setup, like setting a config value or a property, but I won't accept solutions where I'm expected to go around masking units or something else whenever a new (encryption root?) dataset shows up.

[rlaager]

I'm not exactly sure what you're trying to accomplish here, my goal is to have the cryptstore not handled through systemd.

Your previously stated goal was that you didn't want to mount cryptstore automatically on boot. That's reasonable. I don't see a huge difference between zfs load-key ... ; zfs mount -a or systemctl start cryptstore.target.

Anyway, that's a lot of manual intervention to jump through just to not have that toggle...

Creating one target unit is roughly the same amount of work as editing a toggle in /etc/default/zfs and then also manually creating Wants symlinks for particular system datasets as necessary. One might be less or more work, depending on what you count, but they're in the same ballpark for sure.

I'm not sure if the target idea even works anyway, but it sounds like we're converging on a different solution, so this is moot.

This might be an acceptable compromise. Given the fact that using inheritance, I could supposedly set it on my root dataset and therefore get the desired effect on the entire pool, I would be open to this, although it's not as nice for the backup host use case, received (user) properties and all.

In the backup use case, you'd just set it on the dataset that contains all the backups (in my case, e.g. the dataset for /srv/backup).

It wouldn't interfere with stock (Ubuntu installer or HOWTO) root-on-ZFS datasets.

Side note: Nothing I proposed so far breaks or interferes with anything either, as I've kept the current behaviour the default.

Sure, but that's not what's at issue. I've proposed a solution to your use case that doesn't break those scenarios. Your proposed solution to your use case does break those scenarios, or requires a bunch more manual work to create all the relevant Wants symlinks.

Imagine dataset zroot/backups/remotemachine/a with canmount=noauto and mountpoint=/home/a. Your proposed behaviour for the generator is to just generate a mount unit A for it.
Now if anything else creates a mount unit B for /home/a/b, the activation of B will also pull in our A and relevant keyload units.
Sounds like it's the right thing to do 99% of the time, but what when it's not?

Right, that's the obviously correct thing to do most of the time and matches the stock behavior for /etc/fstab. It doesn't sound like you have a personal use case here, so we should hold off on this bit until we have an actual use case in front of us. Having a use case helps ensure our solution is actually useful for someone. Otherwise, we may make the wrong design choices. And once a feature is added, it's very hard to remove, as you never know if someone (or how many) are using it.

By the way, I think my reading of systemd.mount's documentation about nofail was wrong. nofail is only be the default for NFS filesystems.

In light of all of the above, here is my current proposal:

1. Sort canmount auto before noauto in the processing loop, as suggested by @InsanePrawn.
2. Eliminate the ZFS_SYSTEMD_NOAUTO_UNITS option from this PR. Create mount units for noauto filesystems, which will not be Wanted, Required, or Before local-fs.target. This has the highest priority over NAME_TBD and nofail below. From @aerusso, "If there are multiple canmount=noauto datasets [for the same mountpoint], do not write units for any of them."
3. Eliminate the ZFS_SYSTEMD_REQUIRE_MOUNTS option in this PR. Instead, have an org.openzfs.systemd:NAME_TBD that if set to VALUE_TBD, means the unit will not be Wanted, Required, or Before local-fs.target. In other words, it will behave like noauto in that particular regard, but otherwise behaves normally. This is only relevant if it is canmount=auto, of course. This option has priority over nofail below.
4. Add org.openzfs.systemd:nofail. If set to on, the mount will be Wanted by local-fs.target. If set to off, the mount will be Required and Before local-fs.target. If unset (or set to any other value), the mount will be Wanted and Before local-fs.target, which is the current behavior. We may want to change that at some point (e.g. OpenZFS 2.0) to match either off (to match systemd-fstab-generator) or on (under the argument that this generator uses a cache, unlike /etc/fstab which is authoritative, and we should fix the inconsistency in the current default).
5. Add: org.openzfs.systemd:{requires,before,after,requires-mounts-for} that map to the relevant systemd properties in the mount unit. These systemd properties are additive in a unit, so this should be just literally writing out those lines, with no effect on anything else.
6. Revisit automount and idle-timeout properties in the future (or now, if someone wants to test)?

Thoughts?

Thoughts on a name for item 2? What about org.openzfs.systemd:noauto as the name, and on as the value that triggers the behavior? This would be explained as: the "systemd" noauto overrides canmount=noauto. Alternatively, we could flip the logic, doing org.openzfs.systemd:auto, with off as the value that triggers the behavior.

@InsanePrawn would then do something like:

zfs set org.openzfs.systemd:noauto=on rpool/home/insaneprawn/cryptstore

This would be inherited by its children, so all of those would get mount units, but would not be Wanted, Required, or Before local-fs.target. Since no other mountpoint (from e.g. /etc/fstab) would be pulling them in, they would not get mounted on boot, and thus would not hold up the boot or fail the boot. Later, zfs load-key and zfs mount -a could be used to mount cryptstore and its children. But everything else works as per usual stock rules, especially "system" datasets.

[aerusso]

I basically agree with @rlaager. I'll add a couple things:

@InsanePrawn :

Some might argue the cryptstore datasets should be noauto, but I really like the fact that running mount -a any time after I unlocked the cryptstore dataset will make sure it and its children are mounted. Something I couldn't have with canmount=noauto. Plus, there's no zfs mount -r so far...

I wanted zfs mount -r just a couple weeks ago, and I think that would be a fantastic idea.

@rlaager :

I'm not sure if the target idea even works anyway, but it sounds like we're converging on a different solution, so this is moot.

I still think the target is a good idea. It expresses the goal of mounting things in terms of "when". In fact, your point 3 could be called org.openzfs.systemd:target and have an implied default of local-fs, but accept none to have the described behavior.

I think that, with that one additional behavior, we'd really be at an ideal solution.

[rlaager]

I wanted zfs mount -r just a couple weeks ago, and I think that would be a fantastic idea.

There's a feature request for that, which has been open for some time: #2901

I still think the target is a good idea. It expresses the goal of mounting things in terms of "when". In fact, your point 3 could be called org.openzfs.systemd:target and have an implied default of local-fs, but accept none to have the described behavior.

It would still be necessary to create the target unit by hand, so I'm not sure that the ZFS user property is saving you much. Another concern that I have is that this creates a new "target" option in the systemd namespace that has no analog in mount units, mount options, or the fstab generator. (Worst case, if systemd later adds a "target" option, we have a mess.) I'm not completely against it, though. There is a certain elegance to this proposal, since it addresses point 3.

[InsanePrawn]

Hey guys, I've rebased, squashed and modified my commits a little. Here's how far I currently got.

Current Status

DONE

1. Sort canmount auto before noauto in the processing loop, as suggested by @InsanePrawn.

Done, though only tested outside the generator script.

2. Eliminate the ZFS_SYSTEMD_NOAUTO_UNITS option from this PR. Create mount units for noauto filesystems, which will not be Wanted, Required, or Before local-fs.target. This has the highest priority over NAME_TBD and nofail below.

Done!

3. Eliminate the ZFS_SYSTEMD_REQUIRE_MOUNTS option in this PR. Instead, have an org.openzfs.systemd:NAME_TBD that if set to VALUE_TBD, means the unit will not be Wanted, Required, or Before local-fs.target. In other words, it will behave like noauto in that particular regard, but otherwise behaves normally. This is only relevant if it is canmount=auto, of course. This option has priority over nofail below.

Thoughts on a name for item [3]? What about org.openzfs.systemd:noauto as the name, and on as the value that triggers the behavior? This would be explained as: the "systemd" noauto overrides canmount=noauto. Alternatively, we could flip the logic, doing org.openzfs.systemd:auto, with off as the value that triggers the behavior.

Done. The only things that change de-facto are the Before= line and the symlinking into local-fs.target.wants/. I've gone ahead and used 'org.openzfs.systemd:noauto'; I'd be okay with keeping it that way, but it's still open for discussion.

In fact, your point 3 could be called org.openzfs.systemd:target and have an implied default of local-fs, but accept none to have the described behavior.

I think that even if systemd upstream agrees that being able to override the target is a useful thing to do and possibly even implements it for /etc/fstab, noauto is still an orthogonal feature to this, for inheritance reasons alone. I could see myself implementing target as (aside from the .mount contents) more or less replacing local-fs.target.wants with ${target}.target.wants in mkdir and ln -s, the rest of the .target setup procedure being up to the user, with a special case for none.

How long do we wait for an answer from systemd upstream before we just take a vote on org.openzfs.systemd:target?

TODO:

From @aerusso, "If there are multiple canmount=noauto datasets [for the same mountpoint], do not write units for any of them."

I haven't started working on this yet. Shell variable arrays would help with doing this single pass, but we'd be making this generator depend on bash. Anyone got opinions on this? the printf substitution in the sort line could also be replaced with a simple $'\t' if we had the luxury of bashisms.

4. Add org.openzfs.systemd:nofail. If set to on, the mount will be Wanted by local-fs.target. If set to off, the mount will be Required and Before local-fs.target. If unset (or set to any other value), the mount will be Wanted and Before local-fs.target, which is the current behavior. We may want to change that at some point (e.g. OpenZFS 2.0) to match either off (to match systemd-fstab-generator) or on (under the argument that this generator uses a cache, unlike /etc/fstab which is authoritative, and we should fix the inconsistency in the current default).

Haven't worked on this yet but I have renamed req_dir to wants_dir in preparation for this. I'd prefer if the behaviour for the unset property matched one of the settings, but I can understand your wish not to change the existing default behaviour.
(Although it puzzles me that at the same time you're ready to start creating noauto units with no way to turn it off, but okay...)

5. Add: org.openzfs.systemd:{requires,before,after,requires-mounts-for} that map to the relevant systemd properties in the mount unit. These systemd properties are additive in a unit, so this should be just literally writing out those lines, with no effect on anything else.

I'll add this in the coming days.

6. Revisit automount and idle-timeout properties in the future (or now, if someone wants to test)?

Future work as in future PRs as far as I'm concerned :)

Thoughts?

Same question.

[rlaager]

How long do we wait for an answer from systemd upstream before we just take a vote on org.openzfs.systemd:target?

Has anyone proposed this to upstream systemd? I don't think we need to implement this in ZFS now. But if someone wants it, a good first step would be to propose the idea to upstream systemd as x-systemd.target, or similar, for /etc/fstab.

From @aerusso, "If there are multiple canmount=noauto datasets [for the same mountpoint], do not write units for any of them."

I haven't started working on this yet. Shell variable arrays would help with doing this single pass, but we'd be making this generator depend on bash. Anyone got opinions on this? the printf substitution in the sort line could also be replaced with a simple $'\t' if we had the luxury of bashisms.

/bin/sh is definitely not always bash (e.g. on Debian), so if we require bash, we'll have to be explicit about it with /bin/bash. If we can't do it without bashisms, maybe we should just skip it for the first round of changes and try to implement it in a C rewrite?

Haven't worked on this yet but I have renamed req_dir to wants_dir in preparation for this. I'd prefer if the behaviour for the unset property matched one of the settings

Me too, but this might need some more testing. I'm concerned that if the cache gets out of date (e.g. a filesystem is deleted but not from the cache), it seems like it would drop the boot into emergency mode. (I'm not sure, though.) If we make it match the behavior of "on", then we lose the Before=local-fs.target, which might be bad in a different way (e.g. services will start expecting local filesystems to be mounted and they're not).

Revisit automount and idle-timeout properties in the future (or now, if someone wants to test)?

Future work as in future PRs as far as I'm concerned :)

Seems reasonable.

[aerusso]

@rlaager

How long do we wait for an answer from systemd upstream before we just take a vote on org.openzfs.systemd:target?

Has anyone proposed this to upstream systemd?

I did here. @poettering has tagged it, but there has been no discussion. I agree we can move forward with a partial solution, but I would really like to get at least some cross-project buy in.

EDIT: He's responded in the positive. I think the ideal way forward is for someone (and I guess that will be me) to implement the functionality in systemd, and get it merged. Then double back and implement the feature here. Don't let that block progress on orthogonal features here, though.

We should decide on what namespace the systemd bits should live in. Do we want x-systemd.*, org.freedesktop.systemd.*, or org.open-zfs.systemd.*?

[InsanePrawn]

Rebased to include #9970 and

what we'd like to do is reserve the namespace org.openzfs.*:* for user properties.

Can you please update the property names in this PR by dropping the dash. Thanks!

org.openzfs.systemd:*

done. But I do feel obligated to point out:
a) http://openzfs.org redirects to http://open-zfs.org (https doesn't and cert errors anyway, but that's another story), this is easy to turn around, though.
b) I know that's not user properties, but:

% grep org.open-zfs. -r --exclude-dir=.git 2>/dev/null

include/sys/fs/zfs.h: * (e.g. "org.open-zfs:") to avoid conflicting names being developed
man/man5/zpool-features.5:GUID	org.open-zfs:large_blocks
module/zcommon/zfeature_common.c:	    "org.open-zfs:large_blocks", "large_blocks",
tests/zfs-tests/tests/functional/cli_root/zfs_sysfs/zfs_sysfs_live.ksh:pool_feature_attr="/sys/module/zfs/features.pool/org.open-zfs:large_blocks/guid"

EDIT:

Then again

tests/zfs-tests/cmd/libzfs_input_check/libzfs_input_check.c:	fnvlist_add_string(props, "org.openzfs:launch", "September 17th, 2013");
tests/zfs-tests/cmd/libzfs_input_check/libzfs_input_check.c:	fnvlist_add_string(props, "org.openzfs:launch", "September 17th, 2013");
tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.list_user_props.ksh:TESTPROP="org.openzfs:test_property"

[behlendorf]

@InsanePrawn thanks for updating this. Yes, the domains and needed certs are something we're working to sort out, although I expect it will take us a little bit. Luckily org.open-zfs:large_blocks is the only place we're using the dash.

[rlaager]

@aerusso If it looks good to you, can you change your review to approved? Also, FWIW, I tested this on a root-on-ZFS setup. I'm not saying you shouldn't test it too, of course. :)

Insaneprawn:

a) http://openzfs.org redirects to http://open-zfs.org (https doesn't and cert errors anyway, but that's another story), this is easy to turn around, though.

I have proposed flipping that around. The openzfs.org domain was not available initially, which is why the dashed version was used first. We'll see what happens there.

b) I know that's not user properties, but:

I have submitted PR #10003 for that.

[aerusso]

It boots on two of my machines, one zfs-on-root, both having some encrypted datasets. LGTM

[codecov]

Codecov Report

Merging #9649 into master will decrease coverage by 13%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #9649      +/-   ##
==========================================
- Coverage      79%      67%     -13%     
==========================================
  Files         385      303      -82     
  Lines      122384   105074   -17310     
==========================================
- Hits        96936    69969   -26967     
- Misses      25448    35105    +9657

Flag	Coverage Δ
#kernel	?
#user	67% <ø> (ø)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4f4ddf9...16c076a. Read the comment docs.

[rlaager]

@InsanePrawn Thanks for all your work on this! This has been a long process, but you've made something really nice here.

[behlendorf]

Merged as:

ecbbdac Systemd mount generator: Generate noauto units; add control properties
9d2f3b7 Systemd mount generator: Silence shellcheck warnings

@InsanePrawn @rlaager @aerusso thank you for all your work on this.