From 7a56ddaf9d42e9aee8ddbf2d95f9ea82502d094c Mon Sep 17 00:00:00 2001 From: r-vdp Date: Mon, 17 Feb 2025 12:06:27 +0100 Subject: [PATCH 1/3] switch-to-configuration-ng: don't block when the lockfile is already locked When the lockfile is already locked, stc-ng currently blocks without any message indicating what is going on. This is very confusing to users, especially because most will not know about the lockfile. Since the current call blocks, we actually never end up in the else branch. Switching to a non-blocking call, makes sure that we print out an error and exit in case the lockfile is already locked by another process, which makes it a lot clearer what's going on and gives the user a clue as to what to do to unblock the situation. --- pkgs/by-name/sw/switch-to-configuration-ng/src/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkgs/by-name/sw/switch-to-configuration-ng/src/src/main.rs b/pkgs/by-name/sw/switch-to-configuration-ng/src/src/main.rs index a64d8566225b..64fc71e85aed 100644 --- a/pkgs/by-name/sw/switch-to-configuration-ng/src/src/main.rs +++ b/pkgs/by-name/sw/switch-to-configuration-ng/src/src/main.rs @@ -1037,7 +1037,7 @@ fn do_system_switch(action: Action) -> anyhow::Result<()> { die(); }; - let Ok(_lock) = Flock::lock(lock, FlockArg::LockExclusive) else { + let Ok(_lock) = Flock::lock(lock, FlockArg::LockExclusiveNonblock) else { eprintln!("Could not acquire lock"); die(); }; From 125edf84247b22c830776cc0cf21c182c2179712 Mon Sep 17 00:00:00 2001 From: r-vdp Date: Mon, 17 Feb 2025 13:14:25 +0100 Subject: [PATCH 2/3] switch-to-configuration: don't block on flock, see 7a56ddaf9d42e9aee8ddbf2d95f9ea82502d094c --- nixos/modules/system/activation/switch-to-configuration.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nixos/modules/system/activation/switch-to-configuration.pl b/nixos/modules/system/activation/switch-to-configuration.pl index 672291541d03..8eee52bfa332 100755 --- a/nixos/modules/system/activation/switch-to-configuration.pl +++ b/nixos/modules/system/activation/switch-to-configuration.pl @@ -95,7 +95,7 @@ if (!-f "/etc/NIXOS" && (read_file("/etc/os-release", err_mode => "quiet") // "" make_path("/run/nixos", { mode => oct(755) }); open(my $stc_lock, '>>', '/run/nixos/switch-to-configuration.lock') or die "Could not open lock - $!"; -flock($stc_lock, LOCK_EX) or die "Could not acquire lock - $!"; +flock($stc_lock, LOCK_EX|LOCK_NB) or die "Could not acquire lock - $!"; openlog("nixos", "", LOG_USER); # run pre-switch checks From 1aa7e7a131e88cec8bdce48d1a291947e6379593 Mon Sep 17 00:00:00 2001 From: r-vdp Date: Mon, 17 Feb 2025 13:15:01 +0100 Subject: [PATCH 3/3] switch-to-configuration: add a test to make sure that we don't block on the lockfile We should exit with a clear error message instead. --- nixos/tests/switch-test.nix | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/nixos/tests/switch-test.nix b/nixos/tests/switch-test.nix index 4ad08b726bf5..ba1a6f329cd3 100644 --- a/nixos/tests/switch-test.nix +++ b/nixos/tests/switch-test.nix @@ -732,6 +732,23 @@ in { out = switch_to_specialisation("${machine}", "modifiedSystemConf") assert_contains(out, "starting the following units: ${dbusService}\n") + with subtest("aborts on already locked lock file"): + (exitcode, _) = machine.execute( + 'flock -x --nb /run/nixos/switch-to-configuration.lock -c "${otherSystem}/bin/switch-to-configuration test"', + timeout=5 + ) + # See man timeout, exit codes above 124 come from the timeout command + # We want to make sure that stc actually exited with an error code, + # if instead we hit the timeout, then it means that stc hangs, which is + # what we don't want + # TODO: We cannot match on the exact exit code since it's not consistent between + # stc and stc-ng, since errno/last_os_error is not a very stable interface, + # we should probably get rid of that in stc-ng once we got rid of the + # perl implementation + assert exitcode < 124, \ + "switch-to-configuration did not abort as expected, " + \ + f"probably it timed out instead (exit code: {exitcode}), 124 means timeout" + with subtest("fstab mounts"): switch_to_specialisation("${machine}", "") # add a mountpoint