switch-to-configuration-ng: fix with nix-upgrade #343709
Conversation
|
This was working previously, so I assume the perl implementation didn't had this issue. |
|
cc @jmbaur |
The mount can be busy and the update fails.
| @@ -1293,7 +1293,8 @@ won't take effect until you reboot the system. | |||
| units_to_reload.insert(unit.to_string(), ()); | |||
| record_unit(RELOAD_LIST_FILE, &unit) | |||
| } | |||
| } else { | |||
| // systemd creates a transient mount unit for /nix/store, that we shouldn't try to unmount | |||
| } else if !matches!(mountpoint.as_str(), "/nix/store") { | |||
There was a problem hiding this comment.
Why is this needed here and not in the perl one?
There was a problem hiding this comment.
I don't know. It wasn't needed when we introduced the test. Was the perl version taking transient mount units into account?
There was a problem hiding this comment.
I think stc shouldn't touch any transient units. Same reason why transient units have IgnoreOnIsolate=yes. Otherwise you'll kill things like people's graphical applications in GNOME and kill their docker containers
There was a problem hiding this comment.
Probably get_active_units should skip those units entirely?
There was a problem hiding this comment.
The dbus interface doesn't seem to return IgnoreOnIsolate directly for the dbus call used, but it may be accessible through some other command.
There was a problem hiding this comment.
There should be a Transient=yes property exposed in dbus. Maybe we can use that?
There was a problem hiding this comment.
Same as IgnoreOnIsolate, it's GetUnit() in https://www.freedesktop.org/software/systemd/man/latest/org.freedesktop.systemd1.html
so additional dbus call is needed.
It's just not clear to me how to do this with the generated dbus interface:
https://gist.github.com/Mic92/37f1789f9bfc91fa7376845442fa6d80
There was a problem hiding this comment.
I don't feel familiar enough with this library. Closing this pull request instead. Probably a release blocker if it indeed can break automounts and docker container.
There was a problem hiding this comment.
Otherwise you'll kill things like people's graphical applications in GNOME
@arianvp Won't those be user units, and thus unaffected?
|
This is a NixOS / systemd thing that doesn't require review from the Nix team. It seems pretty harmless. I suppose the kernel is clever enough to "close" the file system when the only mount is read-only or something? Or maybe we're just relying on the kernel to finish up nicely during its final procedure before shutting down the all the remaining hardware. I'm not an expert on this, but as far as Nix is concerned, sounds good to me. |
|
The nix-team only got requested here, because I had a nix-related patch in the pull request before. |
|
Opened an issue instead: #343975 |
Description of changes
See https://hydra.nixos.org/build/271655940/nixlog/29 for the failure
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.