-
-
Notifications
You must be signed in to change notification settings - Fork 15.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
nixos/gdm: start on tty7 possibly fixing reload switch #70357
Conversation
Hmm, I actually wrote a nixos test to play around with rebuild switch and didn't have any issues. I will try with |
Ah, right, might've been something else then. my testing was pretty ad-hoc. |
Ok, so I've tested again and when the activation script tries to restart |
Ah, so no idea why, but that does seem to be fixed by using vt7:
|
@GrahamcOfBorg build nixosTests.gnome3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guess it's fine. IIRC most other display managers have their initial vt as 7 anyways.
Actually, I think there's some things missing in our unit that are present in the upstream one diff --git a/nixos/modules/services/x11/display-managers/gdm.nix b/nixos/modules/services/x11/display-managers/gdm.nix
index 6c566f8752f..5f0ceb6fe1f 100644
--- a/nixos/modules/services/x11/display-managers/gdm.nix
+++ b/nixos/modules/services/x11/display-managers/gdm.nix
@@ -165,9 +165,16 @@ in
"systemd-machined.service"
"systemd-user-sessions.service"
"getty@tty${gdm.initialVT}.service"
+ "plymouth-quit.service"
+ "plymouth-start.service"
];
systemd.services.display-manager.conflicts = [
"getty@tty${gdm.initialVT}.service"
+ "plymouth-quit.service"
+ ];
+
+ systemd.services.display-manager.onFailure = [
+ "plymouth-quit.service"
];
systemd.services.display-manager.serviceConfig = {
@@ -177,6 +184,9 @@ in
BusName = "org.gnome.DisplayManager";
StandardOutput = "syslog";
StandardError = "inherit";
+ ExecReload = "${pkgs.coreutils}/bin/kill -SIGHUP $MAINPID";
+ KeyringMode = "shared";
+ EnvironmentFile = "-/etc/locale.conf";
};
systemd.services.display-manager.path = [ pkgs.gnome3.gnome-session ];
This one was probably important now |
Tried testing plymouth in a VM, but it's pretty buggy in nixos so it never shows with or without the extra stuff in gdm. Shouldn't hurt to include though. I also added |
I think I might need to test this patch a little more if it fixes reloading on switch.
Right that's actually very important 👍 |
Right, it's quite likely the problem is unrelated and I just happened to hit the problems when being on vt1 and didn't see them on vt7. It sure doesn't make any sense that this should fix it (at least when we have specify the getty conflict). I'll move the service/gnome-initial stuff out of this PR. |
b4dcf84
to
c9856b7
Compare
Thanks, that unblocks those changes since we haven't determined this actually fixes things 👍 |
So I'm able to reproduce session crashing by going from a66adfc to a66adfc reliably (plain rebuild switch without any changes). With hedning@e84811d plain rebuilds works, then going back to a66adfc also works, and going from a66adfc to hedning@e84811d also works. Now in #70954 @Ninlives seemed to get crashes with this fix so I'm not entirely sure what's up (I'm doing this with gdm/gnome-session 3.34.1 so that might be the difference) :/ |
Not entirely sure how this works, but this does seem to fix reload switch restarting gdm and killing the current gnome-session.
c9856b7
to
204ed39
Compare
@hedning I can confirm, at least in my testing, this change fixes switching plainly and I believe by changing anything else. I've been testing with
which is just copied from the switch test, and running It's really weird that just running the switch script with no changes before this causes an issue. |
Even tested with your original issue on |
Nice :) (should've looked into testing in a VM instead of killing my session a bunch of times though 😆) I did try out just removing the |
Does not seem to work for me. I cherry picked this onto 07d4df5, rebuild and after a reboot added |
OK, that sucks. There's a small possibility that it requires 3.34.1 though looking at gdm and gnome-session commits there's nothing obvious that we didn't already pull into 3.34.0. So this seems to just fix a subset of the rebuild switch issues, in particular rebuilds that don't touch the graphical session stuff in some way (in the extreme case "empty" rebuilds). But there's still rebuilds which can crash the session by touching the wrong bits (that was most likely the case in 3.32 too). |
Right @hedning, it's possible there's multiple independent issues with the switch script here (there's reports that predate systemd user changes). |
Hmm, for what's its worth, tried to reproduce @jtojnar failure with dconf-editor without the session crashing:
|
I can reproduce it 100% of times on latest unstable with Shell 3.34.1 (https://github.com/NixOS/nixpkgs/commits/3b33c6370a9f55b72356409bc5025af4aa7d4b91) as well. I tried switching to Wayland session. No matter how small (swapping |
Jesus that's strange (though the minimal changes in 3.34.1 fixing things would be weird too) 😕 |
I do not even need to change configuration. Just running
The last line is cleanup not getting run due to the abrupt hanging. |
This is what display-manager.service logs around the end:
|
Nothing looks out of the ordinary, gnome-initial-setup stuff appears to be correct
|
The assertion is suspect, but that appears long after the decision was already made. |
Yeah, I'm pretty perplexed 😆 . I can reproduce the problem 100% in a VM with a minimal gnome install. In particular I built two VMs, one with gdm starting on tty1 (at b943338) and one starting on tty7 (b943338 with the fix picked). Inside the VMs I symlinked Going from tty So here's a sample of how that looked: 1 -> 7 -> 1 -> 1
Here's a log from one of the crashes journal log of a rebuild with crash
Edit: I patched nixos-rebuild so it would tag vms with the correct git tag (nixos-version consistently reporting the wrong version inside a VM made me doubt my sanity somewhat) |
Right, looks there's an issue with the interaction between Tried some stuff (eg. updated to plymouth git master, which fedora is on), but to no avail. We might need to drop the conflict. Don't think we actually make use of it properly. It's probably there to handle fading plymouth reliably (eg. it would look bad if plymouth dies in the middle of fading). But we don't support any fading, and plymouth is pretty buggy all over. |
@hedning Sounds good. It seems that being tty1 does cause issues, but there's the issue with the conflict also. |
Yep, looks like two fairly unrelated problems. |
Motivation for this change
Not entirely sure how this works, but this does seem to fix reload switch
restarting gdm and killing the current gnome-session.
In particular I enabled
services.postgresql
with switch and that killed my session (even though that shouldn't touch the display-manager service). Have done a few switch rebuilds when starting on VT7 and haven't seen any issues so far.Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)Notify maintainers
cc @