nixos/switch-to-configuration: reload user units #44990

Ma27 · 2018-08-13T17:53:15Z

Motivation for this change

When rebuilding you have to manually run systemctl --user daemon-reload. It gathers all authenticated users using
loginctl list-user and runs daemon-reload for each of them.

This is a first step towards a nixos-rebuild which is able to reload
user units from systemd. The entire task is fairly hard, however I
consider this patch usable as it allows to restart units without running
daemon-reload for each authenticated user.

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- other Linux distributions
Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
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/)
Determined the impact on package closure size (by running nix path-info -S before and after)
Fits CONTRIBUTING.md.

edolstra · 2018-08-13T20:40:45Z

nixos/modules/system/activation/switch-to-configuration.pl

+    my $name = getpwuid(int($f));
+    print STDERR "reloading user units for $name...\n";
+
+    system("su -l $name -c \$'XDG_RUNTIME_DIR=/run/user/$f @systemd@/bin/systemctl --user daemon-reload'");


Running programs under arbitrary user accounts from a root script frightens me. In any case, it should be

system("su", "-l", $name, "-c" "command...");

to prevent shell interpolation etc.

thx, totally missed that!

edolstra · 2018-08-13T20:41:47Z

nixos/modules/system/activation/switch-to-configuration.pl

@@ -412,6 +412,21 @@ sub filterUnits {
 # Make systemd reload its units.
 system("@systemd@/bin/systemctl", "daemon-reload") == 0 or $res = 3;

+# Reload user units
+opendir (DIR, "/run/user") or $res = 3;


Shouldn't this use loginctl to get active sessions?

I'll look into this. I thought of reading /run/user first as easiest approach to get active sessions, but if there's an easier way to implement this in perl with loginctl I'll change this!

Ma27 · 2018-08-13T22:52:45Z

@edolstra would you mind having another look at this? I switched to a loginctl-based approach now, but I'm not a Perl expert, so I'm not sure if there's still a better solution 😅

edolstra · 2018-08-14T08:46:10Z

nixos/modules/system/activation/switch-to-configuration.pl

@@ -412,6 +412,19 @@ sub filterUnits {
 # Make systemd reload its units.
 system("@systemd@/bin/systemctl", "daemon-reload") == 0 or $res = 3;

+# Reload user units
+open my $listActiveUsers, '-|', '@systemd@/bin/loginctl list-users | @coreutils@/bin/tail -n +2 | @coreutils@/bin/head -n -2';


=> loginctl list-users --no-legend.

Ma27 · 2018-08-14T09:31:00Z

@edolstra done!

Mic92 · 2018-08-14T10:51:34Z

nixos/modules/system/activation/switch-to-configuration.pl

@@ -412,6 +412,19 @@ sub filterUnits {
 # Make systemd reload its units.
 system("@systemd@/bin/systemctl", "daemon-reload") == 0 or $res = 3;

+# Reload user units
+open my $listActiveUsers, '-|', '@systemd@/bin/loginctl list-users --no-legend';


It does not make a difference here, but for consistency splitting arguments is better.

open my $listActiveUsers, '-|', '@systemd@/bin/loginctl', 'list-users', '--no-legend';

Mic92 · 2018-08-14T10:58:34Z

nixos/modules/system/activation/switch-to-configuration.pl

+while (my $f = <$listActiveUsers>) {
+    $f =~ s/^\s+//;
+    $f =~ s/\s+\n$//;
+    my ($uid, $name) = split / /, $f, 2;


You can use a single regex instead of three.

next unless $f =~ /^\s*(?<uid>\d+)\s+(?<user>\S+)/; my ($uid, $name) = ($+{uid}, $+{user});

This code also skips if it does not match.

Mic92 · 2018-08-14T11:18:47Z

nixos/modules/system/activation/switch-to-configuration.pl

+    my ($uid, $name) = split / /, $f, 2;
+    print STDERR "reloading user units for $name...\n";
+
+    system("su", "-l", $name, "-c", "XDG_RUNTIME_DIR=/run/user/$uid @systemd@/bin/systemctl --user daemon-reload");


Can daemon-reload start services? If so it might be better of after systemd-tmpfiles.

no, it simply reloads the configuration files in /etc/systemd/user, you have to restart them manually ATM. Some lines above the same will be done for system-wide services.

The patch simply reloads the services, but doesn't restart them. I started working on this locally, but this task is way harder and will take some time before I can file a usable patch. However I thought that this change might be worth to publish for now :)

When rebuilding you have to manually run `systemctl --user daemon-reload`. It gathers all authenticated users using `loginctl list-user` and runs `daemon-reload` for each of them. This is a first step towards a `nixos-rebuild` which is able to reload user units from systemd. The entire task is fairly hard, however I consider this patch usable as it allows to restart units without running `daemon-reload` for each authenticated user.

Ma27 · 2018-08-14T11:39:56Z

@Mic92 done!

Ma27 · 2018-08-27T08:40:44Z

@edolstra @Mic92 any chance to get this merged before the 18.09 branchoff? :)

GrahamcOfBorg added 6.topic: nixos 8.has: changelog 8.has: documentation 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels Aug 13, 2018

edolstra reviewed Aug 13, 2018

View reviewed changes

Ma27 force-pushed the reload-user-units-during-activation branch from 85bf866 to 8e7b141 Compare August 13, 2018 22:51

edolstra reviewed Aug 14, 2018

View reviewed changes

Ma27 force-pushed the reload-user-units-during-activation branch from 8e7b141 to 96d6cc0 Compare August 14, 2018 09:30

Mic92 reviewed Aug 14, 2018

View reviewed changes

Ma27 force-pushed the reload-user-units-during-activation branch from 96d6cc0 to fc2bde6 Compare August 14, 2018 11:38

jtojnar mentioned this pull request Aug 16, 2018

systemd: Idempotency problem #45113

Closed

Mic92 merged commit a6ced42 into NixOS:master Aug 27, 2018

Ma27 deleted the reload-user-units-during-activation branch August 27, 2018 10:15

Ma27 mentioned this pull request Sep 15, 2018

modifying a service in systemd.user.services does not cause daemon-reload #7326

Closed

refnil mentioned this pull request Feb 9, 2019

Systemd user services are not reloaded on nix-rebuild #29146

Closed

Gerschtli mentioned this pull request Aug 18, 2019

Systemd user services are not stopped automatically #65462

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nixos/switch-to-configuration: reload user units #44990

nixos/switch-to-configuration: reload user units #44990

Ma27 commented Aug 13, 2018 •

edited

edolstra Aug 13, 2018

Ma27 Aug 13, 2018

edolstra Aug 13, 2018

Ma27 Aug 13, 2018

Ma27 commented Aug 13, 2018

edolstra Aug 14, 2018

Ma27 commented Aug 14, 2018

Mic92 Aug 14, 2018

Mic92 Aug 14, 2018

Mic92 Aug 14, 2018

Ma27 Aug 14, 2018

Ma27 commented Aug 14, 2018

Ma27 commented Aug 27, 2018

nixos/switch-to-configuration: reload user units #44990

nixos/switch-to-configuration: reload user units #44990

Conversation

Ma27 commented Aug 13, 2018 • edited

Motivation for this change

Things done

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ma27 commented Aug 13, 2018

Choose a reason for hiding this comment

Ma27 commented Aug 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ma27 commented Aug 14, 2018

Ma27 commented Aug 27, 2018

Ma27 commented Aug 13, 2018 •

edited