Skip to content

feat: kernel-dependent BTRFS UUID collision resolution (temp_fsid on >=6.7) - #674

Open
bfjelds wants to merge 10 commits into
mainfrom
user/bfjelds/mjolnir/acl-cosi-temp-fsuid
Open

feat: kernel-dependent BTRFS UUID collision resolution (temp_fsid on >=6.7)#674
bfjelds wants to merge 10 commits into
mainfrom
user/bfjelds/mjolnir/acl-cosi-temp-fsuid

Conversation

@bfjelds

@bfjelds bfjelds commented Jun 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Summary

Adds kernel-version-dependent mount strategy for ACL BTRFS UUID collisions during A/B updates:

  • Kernel >=6.7: Mount the staging device with -o temp_fsid, which assigns a temporary in-memory UUID and bypasses the BTRFS global UUID registry. This is the preferred solution as it mounts real staging content without needing verity hash verification.
  • Kernel <6.7 (e.g. 6.6.x): Fall back to the existing bind-mount from active /usr, which requires verity hash matching to prove content is identical.

Note: The temp_fsid codepath is aspirational. We believe it will work, but until trident A/B update and ACL run on a kernel >6.6, it is untested in production.

Changes

crates/osutils/src/uname.rs

  • Added KernelVersion struct with parse() and running(); kernel capability is checked inline via kv >= BTRFS_TEMP_FSID_MIN_KERNEL at the single call site.
  • 6 unit tests covering Azure Linux format, 6.7+, garbage input

crates/trident/src/engine/newroot.rs

  • Split monolithic detect_acl_btrfs_uuid_collision into three focused functions:
    • detect_acl_btrfs_uuid_collision - pure UUID collision detection
    • verify_acl_bind_mount_safety - verity hash check (bind-mount path only)
    • resolve_acl_btrfs_uuid_collision - orchestrator that picks strategy based on kernel version
  • New AclBtrfsCollisionResolution enum: TempFsid vs BindMountActiveUsr
  • Unknown/unparseable kernel version falls back to bind-mount (safe default)

Testing

  • All 6 KernelVersion unit tests pass
  • All 7 ACL duplicate UUID validation tests pass
  • cargo build and cargo fmt --check clean on Linux

@bfjelds
bfjelds force-pushed the user/bfjelds/mjolnir/acl-cosi-temp-fsuid branch from eed9f51 to e4eaf3f Compare June 5, 2026 19:36
@bfjelds

bfjelds commented Jun 5, 2026

Copy link
Copy Markdown
Member Author

/azp run [GITHUB]-trident-pr

@azure-pipelines

azure-pipelines Bot commented Jun 5, 2026

Copy link
Copy Markdown
Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@bfjelds
bfjelds marked this pull request as ready for review June 8, 2026 18:16
@bfjelds
bfjelds requested a review from a team as a code owner June 8, 2026 18:16
@bfjelds
bfjelds force-pushed the user/bfjelds/mjolnir/acl-cosi-combined branch from b5f1ff1 to 1522587 Compare June 11, 2026 19:06
bfjelds and others added 6 commits July 14, 2026 09:14
@bfjelds
feat: kernel-dependent BTRFS UUID collision resolution
798f9a5 
On kernel >=6.7, use mount -o temp_fsuid to mount the staging device
directly, bypassing the BTRFS global UUID registry. This is the
preferred solution as it mounts real staging content without needing
verity hash verification.

On kernel <6.7 (e.g. 6.6.x), fall back to the existing bind-mount
strategy which requires verity hash matching to prove the active and
staging content are identical.

Changes:
- Add KernelVersion parser to osutils/uname.rs with unit tests
- Split detect_acl_btrfs_uuid_collision into collision detection and
  resolution strategy (AclBtrfsCollisionResolution enum)
- Add verify_acl_bind_mount_safety for the bind-mount path
- Mount handler selects strategy based on kernel version

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@bfjelds
style: rustfmt fix in uname tests
8a3ed81 
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@bfjelds
docs: note temp_fsuid codepath is aspirational/untested
0635702 
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@bfjelds
feat: gate BTRFS temp_fsuid behind enableAzl4 internal param
ce38e10 
The temp_fsuid mount path (kernel >=6.7) is aspirational and untested in
production. Gate it behind the enableAzl4 internal parameter so it only
activates when explicitly opted in. When the flag is absent, the
bind-mount fallback is used. No special warning or fallback from
temp_fsuid failure — mount errors propagate as-is to surface issues.

The enableAzl4 flag is intentionally broad: it will gate additional
Azure Linux 4 behaviors as they are added.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@bfjelds
refactor: address deep review findings DR-002 through DR-005
62427f6 
DR-002: Move BTRFS temp_fsuid domain knowledge out of osutils. Remove
supports_btrfs_temp_fsuid() from KernelVersion (generic layer) and
define BTRFS_TEMP_FSUID_MIN_KERNEL constant in the consumer (newroot.rs).
KernelVersion now relies on derived Ord for version comparisons.

DR-003: Distinguish uname execution failure from parse failure. The
match on KernelVersion::running() now logs different warnings for Err
(uname command failed) vs Ok(None) (output not parseable).

DR-004: Add doc comment explaining why verity hash verification is
intentionally skipped for the temp_fsuid path (it mounts real staging
content, not a bind-mount of active, so no identity assumption to verify).

DR-005: Eliminate double pattern match on AclBtrfsCollisionResolution in
the mount loop. Add collision_uuid() accessor method so the UUID is
extracted once, then dispatch on the resolution variant in a single match.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@bfjelds
style: rustfmt fix for BTRFS_TEMP_FSUID_MIN_KERNEL
66af464 
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@bfjelds
bfjelds force-pushed the user/bfjelds/mjolnir/acl-cosi-temp-fsuid branch from e4eaf3f to 66af464 Compare July 14, 2026 16:22
@bfjelds
bfjelds changed the base branch from user/bfjelds/mjolnir/acl-cosi-combined to main July 14, 2026 16:23
@bfjelds
bfjelds requested a review from Copilot July 14, 2026 16:51
Copilot AI reviewed Jul 14, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a kernel-version-dependent strategy for handling ACL BTRFS filesystem UUID collisions during A/B updates, preferring temp_fsuid mounts on kernels that support it (>= 6.7) and otherwise falling back to the existing verity-validated bind-mount approach.

Changes:

  • Introduces KernelVersion parsing/comparison utilities in osutils::uname to gate behavior on the running kernel’s major/minor.
  • Refactors ACL BTRFS UUID collision handling in newroot to select between temp_fsuid mounting vs bind-mounting active /usr.
  • Adds a new internal param constant (enableAzl4) to explicitly gate AZL4/kernel-capability-dependent behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
crates/trident/src/engine/newroot.rs Adds strategy selection for ACL BTRFS UUID collision resolution (temp_fsuid vs bind-mount) and refactors detection/verification helpers.
crates/trident_api/src/constants.rs Adds internal_params::ENABLE_AZL4 to gate AZL4-specific behaviors.
crates/osutils/src/uname.rs Adds KernelVersion parsing + ordering and unit tests for common uname formats.

Comment thread crates/trident/src/engine/newroot.rs Outdated
Comment thread crates/trident/src/engine/newroot.rs
Comment thread crates/trident/src/engine/newroot.rs Outdated
@bfjelds
fix: address PR review - fail closed on ACL BTRFS collision edge cases
83309fe 
- verify_acl_bind_mount_safety: refuse bind-mount when no staging verity hash is available instead of allowing it, matching the BindMountActiveUsr verity-proof contract.

- detect_acl_btrfs_uuid_collision: warn when lsblk errors instead of silently swallowing the error.

- resolve_acl_btrfs_uuid_collision: correct a misleading comment about enableAzl4-absent behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 14, 2026 17:03
Copilot AI reviewed Jul 14, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

Comment thread crates/trident/src/engine/newroot.rs Outdated
Comment thread crates/trident/src/engine/newroot.rs Outdated
Comment thread crates/trident_api/src/constants.rs
Comment thread crates/osutils/src/uname.rs
Comment thread crates/trident/src/engine/newroot.rs Outdated
@bfjelds
fix: correct BTRFS mount option to temp_fsid; clarify uname failure l…
69ea2d9 
…ogging

The Linux 6.7 BTRFS option is temp_fsid, not temp_fsuid; the old string would fail at mount time with an unknown-option error. Rename the runtime mount string, enum variant, constant, local var, and all comments/docstrings.

Restructure the kernel-version gating so uname execution failure and parse failure emit distinct warnings instead of both logging 'could not parse'.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 14, 2026 17:18
@bfjelds bfjelds changed the title feat: kernel-dependent BTRFS UUID collision resolution (temp_fsuid on >=6.7) feat: kernel-dependent BTRFS UUID collision resolution (temp_fsid on >=6.7) Jul 14, 2026
Copilot AI reviewed Jul 14, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

Comment thread crates/osutils/src/uname.rs
Comment thread crates/trident/src/engine/newroot.rs
Comment thread crates/trident/src/engine/newroot.rs
Comment thread crates/trident/src/engine/newroot.rs Outdated
@bfjelds
fix: harden ACL BTRFS collision handling per review
9971ecf 
- KernelVersion::parse: trim uname output so a trailing newline on a bare major.minor release (e.g. '5.15\n') no longer fails to parse.

- detect_acl_btrfs_uuid_collision: warn explicitly when lsblk returns Ok(None) instead of swallowing it, matching the Err treatment.

- resolve_acl_btrfs_uuid_collision: return Result and fail with a structured AclBtrfsUuidCollisionUnresolved error (carrying the verity reason) when a collision exists but cannot be safely resolved, instead of returning None and deferring to an opaque mount failure.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 14, 2026 17:36
Copilot AI reviewed Jul 14, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

Comment thread crates/trident/src/engine/newroot.rs Outdated
Comment thread crates/trident/src/engine/newroot.rs Outdated
Comment thread crates/trident/src/engine/newroot.rs
Comment thread crates/trident/src/engine/newroot.rs
@bfjelds
refactor: address review - testable strategy selection and message cl…
d2d9c4c 
…eanup

- Extract pure select_acl_collision_strategy() and add unit tests for strategy selection (enableAzl4+6.7 => TempFsid; +6.6 / undetermined kernel / flag-off => BindMount) plus verify_acl_bind_mount_safety missing/empty-hash cases.

- temp_fsid mount arm: use with_context to avoid eagerly allocating the error string on the success path.

- bind-mount arm: use USR_MOUNT_POINT_PATH instead of a hard-coded /usr literal.

- Reword the collision warnings to describe temp_fsid availability rather than asserting a specific kernel version, since BindMount is also chosen when the flag is unset or the kernel version is undetermined.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 14, 2026 18:06
Copilot AI reviewed Jul 14, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

@bfjelds

bfjelds commented Jul 14, 2026

Copy link
Copy Markdown
Member Author

/azp run [GITHUB]-trident-pr-e2e

@azure-pipelines

azure-pipelines Bot commented Jul 14, 2026

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@bfjelds

bfjelds commented Jul 15, 2026

Copy link
Copy Markdown
Member Author

/azp run [GITHUB]-trident-pr-e2e

@azure-pipelines

azure-pipelines Bot commented Jul 15, 2026

Copy link
Copy Markdown
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@bfjelds