stage2 - replace tmp.mount with tmpfs handling on bootup/activation #14778

sheenobu · 2016-04-17T17:19:08Z

Things done

Tested using sandboxing (nix-build --option build-use-chroot true or nix.useChroot on NixOS)
Built on platform(s)
- NixOS
- OS X
- Linux
Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Fits CONTRIBUTING.md.

~~Haven't tested yet. would love some help on how to do that =)~~

Built an ova. testing now~

mention-bot · 2016-04-17T17:19:10Z

By analyzing the blame information on this pull request, we identified @ehmry, @edolstra and @peti to be potential reviewers

lucabrunox · 2016-04-17T17:27:57Z

I wonder why /tmp can't be a reulgar filesystem under fileSystems ? It's like we're reinventing all the options for it that are already available with fileSystems.

sheenobu · 2016-04-17T17:44:20Z

@lethalman We do not want /tmp to be subject to changes to configuration unless it is during stage-2. There is an open issue for this, #10670, but it doesn't explain why. Changing how /tmp is mounted without a reboot will hose programs which have written to /tmp.

You can do it yourself, via tmpOnTmpfs = false and a custom fileSystem entry, but doing it by default would violate the principal of least surprise. Especially since current documentation on /tmp suggests changes to how /tmp is mounted only happens at boot.

ehmry · 2016-04-17T18:43:38Z

It seems I was the one who originally created the tmpOnTmpfs option. I just wanted a convient boolean rather than having to consult the mount man page for tmpfs options and define a mount outside of hardware-configuration.nix.

I am totally ok with taking this out of systemd's hands.

ehmry · 2016-04-17T18:54:23Z

nixos/modules/system/activation/activation-script.nix

@@ -158,8 +158,11 @@ in
        ${pkgs.utillinux}/bin/mount -o "remount,size=${config.boot.devSize}" none /dev
        ${pkgs.utillinux}/bin/mount -o "remount,size=${config.boot.devShmSize}" none /dev/shm
        ${pkgs.utillinux}/bin/mount -o "remount,size=${config.boot.runSize}" none /run
-      '';


A + optionalString config.boot.tmpOnTmpfs "blah blah"; would be better.

Taken care of, thanks.

peti · 2016-04-18T08:15:05Z

nixos/modules/system/activation/activation-script.nix

@@ -158,8 +158,11 @@ in
        ${pkgs.utillinux}/bin/mount -o "remount,size=${config.boot.devSize}" none /dev
        ${pkgs.utillinux}/bin/mount -o "remount,size=${config.boot.devShmSize}" none /dev/shm
        ${pkgs.utillinux}/bin/mount -o "remount,size=${config.boot.runSize}" none /run
+      '' + optionalString config.boot.tmpOnTmpfs ''
+        if [ mountpoint -q /tmp ]; then


So what happens if a user says boot.tmpOnTmpfs = true but /tmp is not a valid mount point? The current solution is to fail silently, but that doesn't seem ideal?

boot.tmpOnTmpfs is only going to apply /on boot/. The activation-script shouldn't attempt to remount if you set boot.tmpOnTmpfs to true without rebooting.

That said... the stage-2-init.sh script doesn't seem to do much error handling of mount points fail during boot?

edolstra · 2016-04-18T11:11:47Z

I agree with @lethalman that it would be nicer to handle this via fileSystems. The neededForBoot flag could be set automatically for /tmp to cause it to be mounted early (just as we do for /var, /nix etc.). Preventing /tmp from being mounted in a running system could probably be accomplished by dropping the dependency on tmp.mount in basic.target (I think that should be enough to stop switch-to-configuration starting it).

sheenobu · 2016-04-18T17:27:01Z

I agree with @lethalman that it would be nicer to handle this via fileSystems

Still on the fence as fileSystem changes are applied during switch-to-configuration (correct me if I'm wrong, please) where /tmp is a place we don't want that to happen.

Preventing /tmp from being mounted in a running system could probably be accomplished by dropping the dependency on tmp.mount in basic.target (I think that should be enough to stop switch-to-configuration starting it).

Good point, though i'm wary about messing with basic.target (since it's core systemd and not a nixos provided unit). basic.target has a whole special case for tmp.mount

NotaseCretagen · 2016-04-29T14:41:11Z

FYI, this is what happens if using systemd to mount /tmp as tmpfs in this way

sheenobu · 2016-06-25T23:28:02Z

Any new thoughts on this? Possible candidate for inclusion in 16.09?

joachifm · 2016-07-28T18:06:01Z

I've added this to the 16.09 milestone in the hopes that the issue gets resolved one way or another.

cmfwyp · 2016-08-04T04:44:54Z

@sheenobu

Still on the fence as fileSystem changes are applied during switch-to-configuration (correct me if I'm wrong, please) where /tmp is a place we don't want that to happen.

Applying the configuration directly is what nixos-rebuild switch is supposed to do, though. The nixos-rebuild boot command can be used to set it up now but only activate the configuration on next boot. I think that if configuration changes can break the system, the system administrator should simply not use nixos-rebuild switch and should instead set the configuration to be activated on next boot. One rationale for this is that there are already plenty of ways to break things on a running system with nixos-rebuild switch.

Removing tmp.mount from basic.target isn't an elegant solution, but it's still more elegant than duplicating the options specifically for tmp, and without removing tmp.mount from basic.target I would still prefer the fileSystems approach, which will work just fine as long as nixos-rebuild boot is used instead of nixos-rebuild switch.

domenkozar · 2016-08-28T12:04:25Z

Note that this PR needs to be addressed in next days if it wants to reach 16.09

domenkozar · 2016-10-10T11:27:29Z

@sheenobu why was it closed?

sheenobu · 2016-10-10T18:28:37Z

@domenkozar it was an accident due to branch cleanup.

mmahut · 2019-08-03T14:37:53Z

What is the status of this pull request?

mmahut · 2019-08-19T11:39:54Z

Closing due to lack of activity, feel free to reopen this if needed.

sheenobu force-pushed the bugfix/tmpfs_on_boot branch from 629beaa to 631c36d Compare April 17, 2016 17:22

ehmry reviewed Apr 17, 2016
View reviewed changes

stage2 - replace tmp.mount with tmpfs handling on bootup/activation

5bad72d

sheenobu force-pushed the bugfix/tmpfs_on_boot branch from 631c36d to 5bad72d Compare April 17, 2016 19:11

peti reviewed Apr 18, 2016
View reviewed changes

domenkozar added the 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS label Apr 18, 2016

NotaseCretagen mentioned this pull request Apr 29, 2016

"update-locatedb.service: Failed at step NAMESPACE spawning" #15083

Closed

joachifm added this to the 16.09 milestone Jul 28, 2016

cmfwyp mentioned this pull request Aug 3, 2016

nixos: set default for boot.tmpOnTmpfs to true #17494

Closed

2 tasks

domenkozar removed this from the 16.09 milestone Aug 31, 2016

sheenobu closed this Oct 10, 2016

sheenobu deleted the bugfix/tmpfs_on_boot branch October 10, 2016 03:37

sheenobu restored the bugfix/tmpfs_on_boot branch October 10, 2016 03:37

sheenobu reopened this Oct 10, 2016

Mic92 added the 2.status: merge conflict This PR has merge conflicts with the target branch label Oct 22, 2016

mmahut closed this Aug 19, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

stage2 - replace tmp.mount with tmpfs handling on bootup/activation #14778

stage2 - replace tmp.mount with tmpfs handling on bootup/activation #14778

sheenobu commented Apr 17, 2016 •

edited

Loading

mention-bot commented Apr 17, 2016

lucabrunox commented Apr 17, 2016

sheenobu commented Apr 17, 2016 •

edited

Loading

ehmry commented Apr 17, 2016

ehmry Apr 17, 2016

sheenobu Apr 17, 2016

peti Apr 18, 2016

sheenobu Apr 18, 2016

sheenobu Apr 18, 2016

edolstra commented Apr 18, 2016

sheenobu commented Apr 18, 2016

NotaseCretagen commented Apr 29, 2016

sheenobu commented Jun 25, 2016

joachifm commented Jul 28, 2016

cmfwyp commented Aug 4, 2016

domenkozar commented Aug 28, 2016

domenkozar commented Oct 10, 2016

sheenobu commented Oct 10, 2016

mmahut commented Aug 3, 2019

mmahut commented Aug 19, 2019

stage2 - replace tmp.mount with tmpfs handling on bootup/activation #14778

stage2 - replace tmp.mount with tmpfs handling on bootup/activation #14778

Conversation

sheenobu commented Apr 17, 2016 • edited Loading

Things done

mention-bot commented Apr 17, 2016

lucabrunox commented Apr 17, 2016

sheenobu commented Apr 17, 2016 • edited Loading

ehmry commented Apr 17, 2016

ehmry Apr 17, 2016

Choose a reason for hiding this comment

sheenobu Apr 17, 2016

Choose a reason for hiding this comment

peti Apr 18, 2016

Choose a reason for hiding this comment

sheenobu Apr 18, 2016

Choose a reason for hiding this comment

sheenobu Apr 18, 2016

Choose a reason for hiding this comment

edolstra commented Apr 18, 2016

sheenobu commented Apr 18, 2016

NotaseCretagen commented Apr 29, 2016

sheenobu commented Jun 25, 2016

joachifm commented Jul 28, 2016

cmfwyp commented Aug 4, 2016

domenkozar commented Aug 28, 2016

domenkozar commented Oct 10, 2016

sheenobu commented Oct 10, 2016

mmahut commented Aug 3, 2019

mmahut commented Aug 19, 2019

sheenobu commented Apr 17, 2016 •

edited

Loading

sheenobu commented Apr 17, 2016 •

edited

Loading