Merged
Conversation
Clearly it would be the best if we'd directly generate mount units instead of converting /etc/fstab. But in order to do that we need to test it throughly so this approach is for the next stable release. This fix however is intended for inclusion into release-14.12 and release-15.09. Using a simple regular expression unfortunately isn't sufficient for proper mount unit name quoting/escaping and there is a utility in systemd called systemd-escape which does nothing less than that. Of course, using an external program to escape the unit name is way more expensive and causes us to fork for each mount point. But given that we already do quite a lot of forks just for unit starting and stopping, I think it doesn't matter that much. Well, except if you have a whole bunch of mount points. However, if the latter is the case and you have thousands of mount points, you probably have stumbled over this already if your mount point contains a dash. As for my motivation to fix this: I've stumbled on this while trying to fix the "none" backend test for NixOps (see NixOS/nixops#350), where the target machines use /nix/.ro-store and /nix/.rw-store as mount points. The implementation we had so far did improperly escape it so those mount points got the following unit files: * nix-.ro-store.mount * nix-.rw-store.mount The correct names for these units are however: * nix-.ro\x2dstore.mount * nix-.rw\x2dstore.mount So using systemd-escape now properly generates these names. Signed-off-by: aszlig <aszlig@redmoonstudios.org>
Also related to NixOS/nixops#350, because while switching to the new configuration, depending on /nix/store also propagates to the mount points for /nix/.ro-store and /nix/.rw-store and we don't get an error while trying to unmount them (because nix-daemon needs to be stopped for unmounting these paths). While Nix does have the option to set a different store path, I've found only hardcoded references in nix-daemon.nix, so I'm using a hardcoded reference here as well, because after all customizing the store path will probably only make sense on non-NixOS systems. Signed-off-by: aszlig <aszlig@redmoonstudios.org>
aszlig
added
0.kind: bug
Member
|
cc @edolstra |
aszlig
added a commit
to aszlig/nixops
that referenced
this pull request
Oct 23, 2015
We have virtualisation.writableStore active for the targets that are not yet deployed by NixOps but not within the NixOps network specification. This has worked regardless in the past, because using a writable store was implemented solely using boot.initrd.postMountCommands, so switching to a new configuration didn't change anything and left the store untouched. But after NixOS/nixpkgs@e68b0c7, the mounts for the writable store are now using the fileSystems configuration attribute, which causes the mount points to be changed on switch. Another problem is that the mount units weren't escaped properly. But that isn't the problem here anymore (because we don't change the mounts between activation) and has been (or is going to be) dealt with in NixOS/nixpkgs#10546. Signed-off-by: aszlig <aszlig@redmoonstudios.org>
Member
|
cc @rbvermaa |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
These fixes are not only related to NixOS/nixops#350 of course but also fix two other things:
fileSystems.Cc: @edolstra