nixos/stc: Improve mount unit handling

We should sometimes restart the units rather than reloading them so the
changes are actually applied. / and /nix are explicitly excluded because
there was some very old issue where these were unmounted. I don't think
this will affect many people since most people use fstab mounts instead
but I plan to adapt this behavior for fstab mounts as well in the future
(once I wrote a test for the fstab thingies).
This commit is contained in:
Janne Heß 2023-08-20 11:05:46 +02:00
parent 37b8244412
commit eb831f759b
No known key found for this signature in database
3 changed files with 51 additions and 9 deletions

View file

@ -25,8 +25,11 @@ checks:
since changes in their values are applied by systemd when systemd is
reloaded.
- `.mount` units are **reload**ed. These mostly come from the `/etc/fstab`
parser.
- `.mount` units are **reload**ed if only their `Options` changed. If anything
else changed (like `What`), they are **restart**ed unless they are the mount
unit for `/` or `/nix` in which case they are reloaded to prevent the system
from crashing. Note that this is the case for `.mount` units and not for
mounts from `/etc/fstab`. These are explained in [](#sec-switching-systems).
- `.socket` units are currently ignored. This is to be fixed at a later
point.

View file

@ -313,7 +313,8 @@ sub unrecord_unit {
# needs to be restarted or reloaded. If the units differ, the service
# is restarted unless the only difference is `X-Reload-Triggers` in the
# `Unit` section. If this is the only modification, the unit is reloaded
# instead of restarted.
# instead of restarted. If the only difference is `Options` in the
# `[Mount]` section, the unit is reloaded rather than restarted.
# Returns:
# - 0 if the units are equal
# - 1 if the units are different and a restart action is required
@ -390,6 +391,11 @@ sub compare_units { ## no critic(Subroutines::ProhibitExcessComplexity)
next;
}
}
# If this is a mount unit, check if it was only `Options`
if ($section_name eq "Mount" and $ini_key eq "Options") {
$ret = 2;
next;
}
return 1;
}
}
@ -440,10 +446,18 @@ sub handle_modified_unit { ## no critic(Subroutines::ProhibitManyArgs, Subroutin
# properties (resource limits and inotify watches)
# seem to get applied on daemon-reload.
} elsif ($unit =~ /\.mount$/msx) {
# Reload the changed mount unit to force a remount.
# FIXME: only reload when Options= changed, restart otherwise
$units_to_reload->{$unit} = 1;
record_unit($reload_list_file, $unit);
# Just restart the unit. We wouldn't have gotten into this subroutine
# if only `Options` was changed, in which case the unit would be reloaded.
# The only exception is / and /nix because it's very unlikely we can safely
# unmount them so we reload them instead. This means that we may not get
# all changes into the running system but it's better than crashing it.
if ($unit eq "-.mount" or $unit eq "nix.mount") {
$units_to_reload->{$unit} = 1;
record_unit($reload_list_file, $unit);
} else {
$units_to_restart->{$unit} = 1;
record_unit($restart_list_file, $unit);
}
} elsif ($unit =~ /\.socket$/msx) {
# FIXME: do something?
# Attempt to fix this: https://github.com/NixOS/nixpkgs/pull/141192

View file

@ -450,7 +450,7 @@ in {
];
};
mountModified.configuration = {
mountOptionsModified.configuration = {
systemd.mounts = [
{
description = "Testmount";
@ -463,6 +463,19 @@ in {
];
};
mountModified.configuration = {
systemd.mounts = [
{
description = "Testmount";
what = "ramfs";
type = "ramfs";
where = "/testmount";
options = "size=10M";
wantedBy = [ "local-fs.target" ];
}
];
};
timer.configuration = {
systemd.timers.test-timer = {
wantedBy = [ "timers.target" ];
@ -1137,7 +1150,8 @@ in {
switch_to_specialisation("${machine}", "mount")
out = machine.succeed("mount | grep 'on /testmount'")
assert_contains(out, "size=1024k")
out = switch_to_specialisation("${machine}", "mountModified")
# Changing options reloads the unit
out = switch_to_specialisation("${machine}", "mountOptionsModified")
assert_lacks(out, "stopping the following units:")
assert_lacks(out, "NOT restarting the following changed units:")
assert_contains(out, "reloading the following units: testmount.mount\n")
@ -1147,6 +1161,17 @@ in {
# It changed
out = machine.succeed("mount | grep 'on /testmount'")
assert_contains(out, "size=10240k")
# Changing anything but `Options=` restarts the unit
out = switch_to_specialisation("${machine}", "mountModified")
assert_lacks(out, "stopping the following units:")
assert_lacks(out, "NOT restarting the following changed units:")
assert_lacks(out, "reloading the following units:")
assert_contains(out, "\nrestarting the following units: testmount.mount\n")
assert_lacks(out, "\nstarting the following units:")
assert_lacks(out, "the following new units were started:")
# It changed
out = machine.succeed("mount | grep 'on /testmount'")
assert_contains(out, "ramfs")
with subtest("timers"):
switch_to_specialisation("${machine}", "timer")