Repository navigation
Conversation
Activation touches file bind mount targets even when they already exist in the new lower layer. This copies them into the persistent upperdir with origin handles that refer to that generation's metadata image. Only create missing targets. Extend the mutable overlay VM test to check that a bind mount over a declared regular file survives two generation switches without copying its target into the upperdir. Assisted-by: Codex (OpenAI GPT-6.1-Sol)
merrkry
marked this pull request as ready for review
October 20, 2026 10:13
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-ready-for-review/3032/7838 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
During a mutable
/etcoverlay switch, activation callstouchon file bind mount targets even when they already exist in the new lower layer (code).The
touchresults in a timestamp update, which triggers overlayfs copy-up. The copied-up file persists in the upperdir with an origin handle that refers to that generation's lower inode.We should create the target only when it is missing to avoid this unnecessary copy-up (updated code).
The regression test declares an empty regular file in the base configuration, so the lowerdir provides it in all three generations. It bind-mounts a regular file containing
preservedover/etc/declared-filemount.On switch,
-f "$mountPoint"sees the regular bind-mounted source and returns true. Before transferring that mount, the old code touches$tmpMountPoint, which still exposes the empty file from the new lowerdir. This creates the unwanted upperdir copy.After each of two generation switches, the test checks:
/etc/declared-filemountreturnspreserved, confirming that the bind mount was transferred./.rw-etc/upper/declared-filemountdoes not exist, confirming that activation did not copy up the lowerdir placeholder.The first switch is enough to reproduce the bug; the second checks the same properties along the existing test's next switch. The original file-mount case binds
/dev/null, a character device. Its-fcheck returns false, so it skips thetouchbranch and does not cover this bug.Related to #533343 (mount-point handling) and #437424. This fix is a small change and can be applied independently of the overlay switch rewrite.
Validation on x86_64-linux:
nixfmt --checkon both changed files andgit diff --checkpass.nix-build nixos/release.nix -A tests.activation-etc-overlay-mutable.x86_64-linux --no-out-link --max-jobs 2 --cores 2AI disclosure: Codex with OpenAI GPT-6.1-Sol assisted with the code, regression test, commit message, and PR description. Validation used the NixOS VM test framework and nixfmt.
Things done
./result/bin/.nixpkgs-reviewon this PR. See nixpkgs-review usage.passthru.tests.