gador/nixpkgs
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

switch-to-configuration-ng: honour X-* directives in user-unit migration pass

Browse Source 
The post-activation pass added in 5cc82c4922 to handle units migrating
from a per-user manager (home-manager) to NixOS unconditionally restarts
or starts any candidate. dbus-broker.service explicitly opts out of
restarts via reloadIfChanged because restarting the session bus kills
running clients; the second pass ignored that and restarted it anyway.

Apply the same X-ReloadIfChanged / X-RestartIfChanged / RefuseManualStop /
RefuseManualStart / X-OnlyManualStart checks that handle_modified_unit
performs, so a migrated unit is reloaded, skipped, restarted or started
as its directives require.

Covered by new switch-test specialisations for reloadIfChanged and
restartIfChanged = false.
This commit is contained in:
r-vdp
2026-05-22 12:42:19 +02:00
parent d68e4aadd6
commit 76c8d45099
2 changed files with 238 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files
nixos/tests/switch-test.nix
+39
View File
@@ -739,6 +739,22 @@ in
'';
};
# As above, but with reloadIfChanged: pass 2 must reload, not
# restart.
userServiceMigratedToNixosReloadOnly.configuration = {
imports = [ userServiceMigratedToNixosNoStop.configuration ];
systemd.user.services.migrated = {
reloadIfChanged = true;
serviceConfig.ExecReload = "${pkgs.coreutils}/bin/true";
};
};
# As above, but with restartIfChanged = false: pass 2 must skip it.
userServiceMigratedToNixosNoRestart.configuration = {
imports = [ userServiceMigratedToNixosNoStop.configuration ];
systemd.user.services.migrated.restartIfChanged = false;
};
no_inhibitors.configuration.system.switch.inhibitors = lib.mkForce { };
inhibitors.configuration.system.switch.inhibitors = lib.mkForce {
@@ -1818,6 +1834,29 @@ in
out = machine.succeed(f"sudo -u usertest {user_env} cat /run/user/1001/migrated-owner")
assert_contains(out, "nixos")
# Pass 2 must honour reloadIfChanged.
switch_to_specialisation("${machine}", "")
machine.fail(f"sudo -u usertest {user_env} systemctl --user is-active migrated.service")
seed_home_unit()
out = switch_to_specialisation("${machine}", "userServiceMigratedToNixosReloadOnly")
assert_lacks(out, "restarting (post-activation) the following user units: migrated.service")
assert_contains(out, "reloading (post-activation) the following user units: migrated.service")
user_systemctl("is-active migrated.service")
# Reloaded only, so the home ExecStart never re-ran.
out = machine.succeed(f"sudo -u usertest {user_env} cat /run/user/1001/migrated-owner")
assert_contains(out, "home")
# Pass 2 must honour restartIfChanged = false.
switch_to_specialisation("${machine}", "")
machine.fail(f"sudo -u usertest {user_env} systemctl --user is-active migrated.service")
seed_home_unit()
out = switch_to_specialisation("${machine}", "userServiceMigratedToNixosNoRestart")
assert_lacks(out, "\nrestarting (post-activation) the following user units: migrated.service")
assert_contains(out, "NOT restarting (post-activation) the following user units: migrated.service")
user_systemctl("is-active migrated.service")
out = machine.succeed(f"sudo -u usertest {user_env} cat /run/user/1001/migrated-owner")
assert_contains(out, "home")
# Units that remain shadowed by ~/.config must be left alone in both
# passes even though /etc now also defines them.
switch_to_specialisation("${machine}", "")
pkgs/by-name/sw/switch-to-configuration-ng/src/main.rs
+199 -17
View File
@@ -688,12 +688,7 @@ fn handle_modified_unit(
let reload_list = scope.reload_list_file();
let use_restart_as_stop_and_start = new_unit_info.is_none();
if matches!(
unit,
"sysinit.target" | "basic.target" | "multi-user.target" | "graphical.target"
) || unit.ends_with(".unit")
|| unit.ends_with(".slice")
{
if cannot_be_restarted_directly(unit) {
// Do nothing. These cannot be restarted directly.
// Slices and Paths don't have to be restarted since properties (resource limits and
@@ -940,6 +935,17 @@ fn parse_fstab(fstab: impl BufRead) -> (HashMap<String, Filesystem>, HashMap<Str
(filesystems, swaps)
}
/// Whether a unit cannot be (re)started directly. Special targets are pulled
/// in by their dependents; slices and paths get their properties applied on
/// daemon-reload.
fn cannot_be_restarted_directly(unit: &str) -> bool {
matches!(
unit,
"sysinit.target" | "basic.target" | "multi-user.target" | "graphical.target"
) || unit.ends_with(".path")
|| unit.ends_with(".slice")
}
// Returns a HashMap containing the same contents as the passed in `units`, minus the units in
// `units_to_filter`.
fn filter_units(
@@ -957,6 +963,50 @@ fn filter_units(
res
}
/// Action to take on a unit that migrated to NixOS ownership during the
/// post-activation pass. Honours the same X-* directives as
/// `handle_modified_unit`.
#[derive(Debug, PartialEq)]
enum MigrationAction {
Skip,
Reload,
Restart,
Start,
}
impl MigrationAction {
/// Action to take on a migrated unit that is still active.
fn for_active_unit(unit: &str, new_unit_info: &UnitInfo) -> Self {
if cannot_be_restarted_directly(unit) {
return Self::Skip;
}
if parse_systemd_bool(Some(new_unit_info), "Service", "X-ReloadIfChanged", false) {
return Self::Reload;
}
if !parse_systemd_bool(Some(new_unit_info), "Service", "X-RestartIfChanged", true)
|| parse_systemd_bool(Some(new_unit_info), "Unit", "RefuseManualStop", false)
|| parse_systemd_bool(Some(new_unit_info), "Unit", "X-OnlyManualStart", false)
{
return Self::Skip;
}
Self::Restart
}
/// Action to take on a migrated unit that the previous owner stopped.
fn for_stopped_unit(new_unit_info: &UnitInfo) -> Self {
if parse_systemd_bool(Some(new_unit_info), "Unit", "RefuseManualStart", false)
|| parse_systemd_bool(Some(new_unit_info), "Unit", "X-OnlyManualStart", false)
{
return Self::Skip;
}
Self::Start
}
}
fn unit_is_active(conn: &LocalConnection, unit: &str) -> Result<bool> {
let unit_object_path = conn
.with_proxy(
@@ -1510,29 +1560,40 @@ fn do_user_switch(parent_exe: String) -> anyhow::Result<()> {
let active_after = get_active_units(&systemd)?;
let mut to_reload = HashMap::new();
let mut to_restart = HashMap::new();
let mut to_start = HashMap::new();
let mut to_skip = HashMap::new();
for unit in &migration_candidates {
match active_after.get(unit) {
// Honour X-* directives so reloadIfChanged/restartIfChanged hold.
let new_unit_file = new_unit_dir.join(unit);
let new_unit_info = parse_unit(&new_unit_file, &new_unit_file)?;
let action = match active_after.get(unit) {
Some(unit_state) => {
// Only act if /etc now wins (i.e. the higher-priority
// copy is gone). Read errors are treated as "leave alone".
let now_etc = unit_state
.proxy
.get("org.freedesktop.systemd1.Unit", "FragmentPath")
.map(|p: String| p.starts_with(fragment_prefix))
.unwrap_or(false);
if now_etc {
// Still running with the previous manager's binary;
// restart so the /etc definition takes effect.
to_restart.insert(unit.clone(), ());
if !now_etc {
// Still shadowed (or read error); leave it alone.
continue;
}
// else: still shadowed by ~/.config, leave it alone.
MigrationAction::for_active_unit(unit, &new_unit_info)
}
None => {
// Stopped by the previous manager; start the /etc copy.
to_start.insert(unit.clone(), ());
}
}
// Stopped by the previous manager; start the /etc copy.
None => MigrationAction::for_stopped_unit(&new_unit_info),
};
match action {
MigrationAction::Skip => to_skip.insert(unit.clone(), ()),
MigrationAction::Reload => to_reload.insert(unit.clone(), ()),
MigrationAction::Restart => to_restart.insert(unit.clone(), ()),
MigrationAction::Start => to_start.insert(unit.clone(), ()),
};
}
// Re-start active targets so any other newly-unmasked dependencies are
@@ -1543,6 +1604,24 @@ fn do_user_switch(parent_exe: String) -> anyhow::Result<()> {
}
}
if !to_skip.is_empty() {
print_units("NOT restarting (post-activation)", &to_skip);
}
print_units("reloading (post-activation)", &to_reload);
for unit in to_reload.keys() {
match systemd.reload_unit(unit, "replace") {
Ok(job_path) => {
submitted_jobs.borrow_mut().insert(job_path, Job::Reload);
}
Err(err) => {
eprintln!("Failed to reload user unit {unit}: {err}");
exit_code = 4;
}
}
}
block_on_jobs(&dbus_conn, &submitted_jobs);
print_units("restarting (post-activation)", &to_restart);
for unit in to_restart.keys() {
match systemd.restart_unit(unit, "replace") {
@@ -2863,4 +2942,107 @@ After=dev-disk-by\x2dlabel-root.device
);
}
}
fn unit_info(
sections: &[(&str, &[(&str, &str)])],
) -> HashMap<String, HashMap<String, Vec<String>>> {
sections
.iter()
.map(|(section, kvs)| {
(
section.to_string(),
kvs.iter()
.map(|(k, v)| (k.to_string(), vec![v.to_string()]))
.collect(),
)
})
.collect()
}
#[test]
fn migration_action_for_active_unit() {
use super::MigrationAction;
// Plain service: restart.
assert_eq!(
MigrationAction::for_active_unit("foo.service", &unit_info(&[])),
MigrationAction::Restart
);
// reloadIfChanged must reload, not restart.
assert_eq!(
MigrationAction::for_active_unit(
"foo.service",
&unit_info(&[("Service", &[("X-ReloadIfChanged", "true")])])
),
MigrationAction::Reload
);
// X-RestartIfChanged=false (restartIfChanged = false) must skip.
assert_eq!(
MigrationAction::for_active_unit(
"foo.service",
&unit_info(&[("Service", &[("X-RestartIfChanged", "false")])])
),
MigrationAction::Skip
);
// RefuseManualStop must skip.
assert_eq!(
MigrationAction::for_active_unit(
"foo.service",
&unit_info(&[("Unit", &[("RefuseManualStop", "yes")])])
),
MigrationAction::Skip
);
// X-OnlyManualStart must skip.
assert_eq!(
MigrationAction::for_active_unit(
"foo.service",
&unit_info(&[("Unit", &[("X-OnlyManualStart", "yes")])])
),
MigrationAction::Skip
);
// Units that cannot be restarted directly must skip.
for unit in [
"sysinit.target",
"basic.target",
"multi-user.target",
"graphical.target",
"foo.path",
"bar.slice",
] {
assert_eq!(
MigrationAction::for_active_unit(unit, &unit_info(&[])),
MigrationAction::Skip,
"{unit}"
);
}
}
#[test]
fn migration_action_for_stopped_unit() {
use super::MigrationAction;
assert_eq!(
MigrationAction::for_stopped_unit(&unit_info(&[])),
MigrationAction::Start
);
assert_eq!(
MigrationAction::for_stopped_unit(&unit_info(&[(
"Unit",
&[("RefuseManualStart", "true")]
)])),
MigrationAction::Skip
);
assert_eq!(
MigrationAction::for_stopped_unit(&unit_info(&[(
"Unit",
&[("X-OnlyManualStart", "true")]
)])),
MigrationAction::Skip
);
}
}