Activity

1. danneu commented on May 23, 2026

   

   danneu

   on May 23, 2026

   OwnerAuthor

   More actions

   Add Explicit Flash Storage Profile

   Summary

   Add opt-in SSD/NVMe support through a pool-wide storage profile. The default
   stays HDD-compatible. Setting braid.storageProfile = "flash" makes braid open
   LUKS mappings with discard passthrough and mount btrfs with async discard. The
   HDD profile gets an explicit nodiscard mount option so that no device in the
   pool can ever trigger the kernel's auto-async-discard heuristic. The cryptsetup
   security tradeoff is documented: discard can leak filesystem usage patterns
   through encryption.

   Research facts to preserve in the docs: btrfs supports discard=async since
   kernel 5.6 and auto-enables it since 6.2 whenever any device in the pool
   advertises discard support; an explicit nodiscard mount option is the only
   way to suppress this for HDD pools that might contain a flash member or sit on
   a thinly-provisioned backing device. dm-crypt advertises discard only when
   opened with --allow-discards; cryptsetup status reports this as
   flags: discards.

   Key Changes

   • Add public config surface:
     • Nix option braid.storageProfile = "hdd" | "flash", default "hdd".
     • Generated config JSON field storage_profile.
     • Rust StorageProfile enum plus Config::storage_profile().
     • No CLI override and no auto-detection; the operator chooses the profile.
   • Update command construction:
     • Extend CryptsetupLuksOpen and CryptsetupLuksOpenKeyFile requests with
       allow_discards: bool.
     • In flash profile, render cryptsetup open ... --allow-discards ....
     • Keep existing --perf-no_read_workqueue and --perf-no_write_workqueue.
     • Do not add --persistent; the Nix config owns this policy, not LUKS
       metadata.
     • Do not add discard flags to luksFormat, passphrase tests, key enrollment,
       or header backups.
   • Update mount behavior:
     • HDD profile mounts as noatime,skip_balance,subvolid=5,nodiscard. The
       explicit nodiscard is required: btrfs auto-enables async discard since
       kernel 6.2 if any device in the pool advertises support, which would
       otherwise leak through HDD pools that contain a flash member or sit on
       thinly-provisioned storage.
     • Flash profile mounts with noatime,skip_balance,subvolid=5,discard=async.
     • Thread the profile through a single rendering chokepoint:
       base_mount_options(profile) in cli/src/cmd.rs. The Mount and
       MountWithOptions to_argv branches consume the profile-derived options
       so every caller (unlock, auto-unlock, add/bootstrap, recover, degraded
       mounts, replace/recovery paths) inherits the right policy without
       per-callsite logic.
     • Make the chokepoint structural: CmdRequest::Mount and
       CmdRequest::MountWithOptions carry a profile: StorageProfile field so
       omitting the profile is a compile error.
     • Keep noatime for flash too; even without HDD spindown, it avoids extra
       metadata writes.
   • Handle already-open mappers (structural at the LUKS boundary):
     • Extend parse_cryptsetup_status to parse optional flags: and expose
       discards: bool.
     • Promote MapperOwnership::Owned to carry the discards bit, e.g.
       Owned { discards: bool }, populated by classify_mapper_ownership from
       the parsed status.
     • Thread StorageProfile (or a MapperReusePolicy derived from it) through
       ensure_luks_open, ensure_luks_open_with_key_file, and
       probe_mapper_open so the reuse rule is enforced exactly once, at the
       helper that owns the unsafe reuse decision. Omitting the profile is a
       compile error.
     • Flash profile: Owned { discards: false } returns a hard error from
       ensure_luks_open* with a message telling the operator to run braid lock and retry. Owned { discards: true } resolves to
       OpenOutcome::AlreadyOwned. probe_mapper_open fails closed in the same
       case with the same braid lock remediation -- not "needs reopen" -- so
       the planner does not try to dry-run an impossible cryptsetup open
       against a mapper that is still active. Current planning (plan_open_pool
       in cli/src/mount.rs:303 and the mapper_open: bool on
       PresentLuks in cli/src/types.rs:511) treats false as "schedule an
       open", so probe-level failure is the only safe shape.
     • HDD profile: Owned { discards: _ } always resolves to
       OpenOutcome::AlreadyOwned; the discard bit is ignored. No behavior
       change for HDD.
     • All current callsites of these helpers (in cli/src/mount.rs,
       cli/src/add.rs, cli/src/replace.rs, cli/src/recover.rs) thread the
       profile from the already-loaded Config. Because the policy lives in the
       helper, no callsite-level check is required, and a future callsite
       cannot bypass the rule without making the type system unhappy.
   • Update docs:
     • Revise ADR-015 and Principle 11 from "HDD-only defaults" to "HDD default,
       flash opt-in".
     • Document the flash profile in README and docs/guides/nixos-configuration.md.
     • Update unlock/mount docs to include the explicit profile-dependent mount
       options: HDD = noatime,skip_balance,subvolid=5,nodiscard, flash =
       noatime,skip_balance,subvolid=5,discard=async.
     • Explicitly state why HDD uses nodiscard: it neutralizes the kernel's
       auto-async-discard heuristic for pools that might contain flash members or
       sit on thin-provisioned storage.
     • Explicitly state the security tradeoff from cryptsetup: discard can reveal
       filesystem type, used space, and related usage patterns.

   Test Plan

   • Rust unit tests:
     • Config parses default "hdd" and explicit "flash", rejects unknown
       profiles.
     • cryptsetup status parser handles no flags: line, flags: discards, and
       mixed flags.
     • HDD CryptsetupLuksOpen* argv has no --allow-discards.
     • Flash CryptsetupLuksOpen* argv includes --allow-discards.
     • Pin mount argv at the central rendering chokepoint (to_argv for
       CmdRequest::Mount and CmdRequest::MountWithOptions) across the full
       matrix:
       • HDD plain mount argv contains nodiscard and not discard=async.
       • HDD degraded mount argv contains nodiscard,degraded and not
         discard=async.
       • Flash plain mount argv contains discard=async and not nodiscard.
       • Flash degraded mount argv contains discard=async,degraded and not
         nodiscard.
     • Pin the LUKS reuse rule at the helper boundary, not only at planning:
       • classify_mapper_ownership populates Owned { discards } from the
         parsed cryptsetup status for both flags: discards and a missing
         flags: line.
       • Under flash profile, ensure_luks_open returns a hard error for
         Owned { discards: false } and the error message names braid lock as
         the remediation.
       • Under flash profile, ensure_luks_open returns AlreadyOwned for
         Owned { discards: true }.
       • Under flash profile, ensure_luks_open_with_key_file matches the above
         two cases. (Same boundary, separate entry point, separate test.)
       • Under HDD profile, both ensure_luks_open and
         ensure_luks_open_with_key_file return AlreadyOwned regardless of
         discards.
       • Under flash profile, probe_mapper_open returns a hard error for
         Owned { discards: false } with the braid lock remediation, not a
         "needs reopen" classification.
       • plan_open_pool (unlock dry-run path in cli/src/mount.rs) surfaces
         that probe error verbatim when run with the flash profile against a
         mapper that is active but lacks discards. Pin both the dry-run
         preview and the executed unlock at this seam so the remediation
         message reaches the operator before any cryptsetup open is
         planned.
   • Nix/module tests:
     • Default generated /etc/braid/config.json contains or implies
       storage_profile = "hdd".
     • braid.storageProfile = "flash" emits "storage_profile": "flash".
     • Invalid profile fails Nix evaluation through the enum type.
   • Focused VM tests (each registered in flake.nix checks):
     • flash-profile-unlock: flash profile, create a small pool, unlock it, then
       assert cryptsetup status braid-disk1 includes flags: with discards,
       findmnt -o OPTIONS -n /mnt/storage includes discard=async and not
       nodiscard, and existing skip_balance and subvolid=5 assertions still
       pass.
     • flash-profile-add-bootstrap: flash profile, run braid add on a fresh
       pool (the non-unlock mount-producing seam), then assert the mounted btrfs
       options include discard=async. This exercises a different mount path
       from the unlock test and catches regressions where add/bootstrap or
       recover/remount fail to thread the profile.
     • hdd-profile-mount-options: default (HDD) profile pool, unlock it, then
       assert findmnt -o OPTIONS -n /mnt/storage includes nodiscard and not
       discard=async. Confirms the explicit nodiscard reaches mount argv and
       the kernel does not auto-enable discard on the VM's backing devices.
   • Verification:
     • just test-rust
     • just test-vm flash-profile-unlock flash-profile-add-bootstrap hdd-profile-mount-options
     • nix flake check --no-build to confirm the three new checks are
       registered and evaluate.
     • mdbook build docs

   Assumptions

   • The feature is explicit opt-in, not drive auto-detection.
   • Flash profile enables LUKS discard passthrough by default.
   • Scrub scheduling stays monthly by default.
   • Mixed HDD/flash pools are not blocked; the profile is an operator policy
     choice, and unsupported lower layers simply will not perform useful discard.
   • TUI/NVMe SMART parsing is already present, so this commit does not need a TUI
     feature unless docs mention existing monitoring support.