From 96ba0ca283607977620d31809d52396232074806 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Sat, 5 Jan 2013 01:05:25 +0100 Subject: [PATCH] For some units, use "systemctl restart" rather than "systemctl stop/start" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit During a configuration switch, changed units are stopped in the old configuration, then started in the new configuration (i.e. after running the activation script and running "systemctl daemon-reload"). This ensures that services are stopped using the ExecStop/ExecStopPost commands from the old configuration. However, for some services it's undesirable to stop them; in particular dhcpcd, which deconfigures its network interfaces when it stops. This is dangerous when doing remote upgrades - usually things go right (especially because the switch script ignores SIGHUP), but not always (see 9aa69885f04969e5d31dcb8265c327adc908954e). Likewise, sshd should be kept running for as long as possible to prevent a lock-out if the switch fails. So the new option ‘stopIfChanged = false’ causes "systemctl restart" to be used instead of "systemctl stop" followed by "systemctl start". This is only proper for services that don't have stop commands. (And it might not handle dependencies properly in some cases, but I'm not sure.) --- modules/services/networking/dhcpcd.nix | 5 ++ modules/services/networking/ssh/sshd.nix | 2 + .../activation/switch-to-configuration.pl | 46 +++++++++++++------ modules/system/boot/systemd-unit-options.nix | 15 ++++++ modules/system/boot/systemd.nix | 1 + 5 files changed, 56 insertions(+), 13 deletions(-) diff --git a/modules/services/networking/dhcpcd.nix b/modules/services/networking/dhcpcd.nix index 2a0d73f6004..cf7f621a85d 100644 --- a/modules/services/networking/dhcpcd.nix +++ b/modules/services/networking/dhcpcd.nix @@ -97,6 +97,11 @@ in wantedBy = [ "network.target" ]; + # Stopping dhcpcd during a reconfiguration is undesirable + # because it brings down the network interfaces configured by + # dhcpcd. So do a "systemctl restart" instead. + stopIfChanged = false; + path = [ dhcpcd pkgs.nettools pkgs.openresolv ]; serviceConfig = diff --git a/modules/services/networking/ssh/sshd.nix b/modules/services/networking/ssh/sshd.nix index 21f81152fa5..8f898ce06a1 100644 --- a/modules/services/networking/ssh/sshd.nix +++ b/modules/services/networking/ssh/sshd.nix @@ -267,6 +267,8 @@ in wantedBy = [ "multi-user.target" ]; + stopIfChanged = false; + path = [ pkgs.openssh ]; environment.LD_LIBRARY_PATH = nssModulesPath; diff --git a/modules/system/activation/switch-to-configuration.pl b/modules/system/activation/switch-to-configuration.pl index b085778f099..28ccc158f72 100644 --- a/modules/system/activation/switch-to-configuration.pl +++ b/modules/system/activation/switch-to-configuration.pl @@ -6,6 +6,7 @@ use File::Basename; use File::Slurp; use Cwd 'abs_path'; +my $startListFile = "/run/systemd/start-list"; my $restartListFile = "/run/systemd/restart-list"; my $reloadListFile = "/run/systemd/reload-list"; @@ -125,7 +126,7 @@ while (my ($unit, $state) = each %{$activePrev}) { if ($unit ne "suspend.target" && $unit ne "hibernate.target") { my $unitInfo = parseUnit($newUnitFile); unless (boolIsTrue($unitInfo->{'RefuseManualStart'} // "false")) { - write_file($restartListFile, { append => 1 }, "$unit\n"); + write_file($startListFile, { append => 1 }, "$unit\n"); } } } @@ -158,21 +159,31 @@ while (my ($unit, $state) = each %{$activePrev}) { foreach my $socket (@sockets) { if (defined $activePrev->{$socket}) { push @unitsToStop, $socket; - write_file($restartListFile, { append => 1 }, "$socket\n"); + write_file($startListFile, { append => 1 }, "$socket\n"); $socketActivated = 1; } } } - # Otherwise, record that this unit needs to be - # started below. We write this to a file to - # ensure that the service gets restarted if we're - # interrupted. - if (!$socketActivated) { - write_file($restartListFile, { append => 1 }, "$unit\n"); - } + if (!boolIsTrue($unitInfo->{'X-StopIfChanged'} // "true")) { - push @unitsToStop, $unit; + # This unit should be restarted instead of + # stopped and started. + write_file($restartListFile, { append => 1 }, "$unit\n"); + + } else { + + # If the unit is not socket-activated, record + # that this unit needs to be started below. + # We write this to a file to ensure that the + # service gets restarted if we're interrupted. + if (!$socketActivated) { + write_file($startListFile, { append => 1 }, "$unit\n"); + } + + push @unitsToStop, $unit; + + } } } } @@ -216,7 +227,7 @@ foreach my $mountPoint (keys %$prevFss) { push @unitsToStop, $unit; } elsif ($prev->{fsType} ne $new->{fsType} || $prev->{device} ne $new->{device}) { # Filesystem type or device changed, so unmount and mount it. - write_file($restartListFile, { append => 1 }, "$unit\n"); + write_file($startListFile, { append => 1 }, "$unit\n"); push @unitsToStop, $unit; } elsif ($prev->{options} ne $new->{options}) { # Mount options changes, so remount it. @@ -266,16 +277,25 @@ system("@systemd@/bin/systemctl", "reset-failed"); # Make systemd reload its units. system("@systemd@/bin/systemctl", "daemon-reload") == 0 or $res = 3; +# Restart changed services (those that have to be restarted rather +# than stopped and started). +my @restart = unique(split('\n', read_file($restartListFile, err_mode => 'quiet') // "")); +if (scalar @restart > 0) { + print STDERR "restarting the following units: ", join(", ", sort(@restart)), "\n"; + system("@systemd@/bin/systemctl", "restart", "--", @restart) == 0 or $res = 4; + unlink($restartListFile); +} + # Start all active targets, as well as changed units we stopped above. # The latter is necessary because some may not be dependencies of the # targets (i.e., they were manually started). FIXME: detect units # that are symlinks to other units. We shouldn't start both at the # same time because we'll get a "Failed to add path to set" error from # systemd. -my @start = unique("default.target", split('\n', read_file($restartListFile, err_mode => 'quiet') // "")); +my @start = unique("default.target", split('\n', read_file($startListFile, err_mode => 'quiet') // "")); print STDERR "starting the following units: ", join(", ", sort(@start)), "\n"; system("@systemd@/bin/systemctl", "start", "--", @start) == 0 or $res = 4; -unlink($restartListFile); +unlink($startListFile); # Reload units that need it. This includes remounting changed mount # units. diff --git a/modules/system/boot/systemd-unit-options.nix b/modules/system/boot/systemd-unit-options.nix index ad9b5da2316..1f8097ada1c 100644 --- a/modules/system/boot/systemd-unit-options.nix +++ b/modules/system/boot/systemd-unit-options.nix @@ -183,6 +183,21 @@ rec { ''; }; + stopIfChanged = mkOption { + type = types.bool; + default = true; + description = '' + If set, a changed unit is restarted by calling + systemctl stop in the old configuration, + then systemctl start in the new one. + Otherwise, it is restarted in a single step using + systemctl restart in the new configuration. + The latter is less correct because it runs the + ExecStop commands from the new + configuration. + ''; + }; + }; diff --git a/modules/system/boot/systemd.nix b/modules/system/boot/systemd.nix index f4d0655118e..a0462951d00 100644 --- a/modules/system/boot/systemd.nix +++ b/modules/system/boot/systemd.nix @@ -246,6 +246,7 @@ let ${let env = cfg.globalEnvironment // def.environment; in concatMapStrings (n: "Environment=${n}=${getAttr n env}\n") (attrNames env)} ${optionalString (!def.restartIfChanged) "X-RestartIfChanged=false"} + ${optionalString (!def.stopIfChanged) "X-StopIfChanged=false"} ${optionalString (def.preStart != "") '' ExecStartPre=${makeJobScript "${name}-pre-start" ''