Repository navigation
nvx-rs: skip journal loading for GPT lower layers - #807
Merged
Merged
Conversation
Mount GPT lower layers with noload as well as errors=remount-ro. Without noload, ext4 can scan a nonempty journal even when its needs_recovery bit is clear; a cyclic journal can hold assembly there indefinitely. Keep rejecting the recovery bit before any mount, rather than allowing noload to admit a recovery-required layer. Add adapter VM scenarios with a cyclic internal journal, with and without the recovery bit. Check that the former assembles, serves reads and copy-up, and tears down, while the latter fails before mounting. Assert journal suppression in every GPT scenario and document the deliberate change from #776's journal-loading policy. EROFS lowers and writable scratches retain their existing mount options. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 30bab0d7-7c3e-4120-9add-6c9cc1dc9e7e
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The focused mount-option change preserves recovery rejection and has targeted unit and VM regression coverage, with no identified blocking issues.
No open issues 🎉
What changed in this PR
This PR updates the guest library’s GPT lower-layer mount policy to prevent hangs while scanning corrupt journals. It preserves rejection of layers that require recovery.
Changes:
- Adds
noloadwhile retainingerrors=remount-roand read-only mount protections. - Adds VM regression scenarios for cyclic journals with and without required recovery.
- Updates unit expectations and documentation; EROFS layers and writable scratches remain unchanged.
| File | Description |
|---|---|
tests/runtime-adapter/tests/vm_layered.rs |
Adds cyclic-journal scenarios and asserts journal suppression. |
tests/runtime-adapter/README.md |
Documents the updated mount policy and VM scenarios. |
nvx-rs/src/guest/root/tests.rs |
Updates expected mount options and recovery-check commentary. |
nvx-rs/src/guest/root.rs |
Suppresses GPT lower-layer journal loading and documents recovery safeguards. |
nvx-rs/README.md |
Explains the journal policy and its scope. |
Review effort: Balanced
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.
Part of #621. Refs #622 and #535. Follow-up to #776.
Mount GPT ext4 lower layers with
noload,errors=remount-ro, retainingro,nosuid,nodev. A read-only mount can load and scan an internal journal even when the file system'sneeds_recoverybit is clear. Suppressing journal loading avoids that path instead of adding a second journal parser to the guest library.This deliberately changes #776's policy of mounting without
noload. The assembly still rejectsneeds_recoverybefore any mount, so journal suppression does not admit a layer that explicitly requires recovery. EROFS layers, writable scratches, overlay modes, and teardown order are unchanged.The adapter's VM scenarios now assert journal suppression for every GPT lower. Two new cases cover an internal cyclic journal: with
needs_recoveryclear, assembly, reads, copy-up, and teardown complete; with it set, the assembly reports its typed recovery failure before mounting. The same cyclic-journal scenario against the exact base's guest agent timed out during assembly after 60 seconds on WHP; the fixed agent passes.Validation
python scripts/nvx.py verify;nvx-rsClippy (all targets/features and lib/no-default-features), tests (all features, defaults, and lib/no-default-features), no-default-features build, rustdoc with warnings denied, and Rust 1.89 checks. Linux totals: 1,249 all-features tests, 597 default tests, and 184 no-default-features tests. The focused root suite passes all 59 tests.cargo fmt --all --checkfornvx-rs,aci_edge_sandboxes, andtests/runtime-adapter;python scripts/check_markdown_links.py. The adapter's protocol-only and host configurations pass Clippy, build, tests, and Rust 1.89 checks on Windows and Linux; its static musl guest configuration passes the same checks on Linux.check-nvx-rsgates (bundled-artifact fixture, macOS cross-check,check-guest-binaries,build-guest-binaries, andtest-guest-binaries, 34 passed), and allcheck-aci-edge-sandboxesnon-hypervisor gates, including agent bundling, docs, MSRV, and macOS.cargo test --locked --features host --test vm_layered --test vm_restart -- --ignoredpasses all 21 layered scenarios and 2 restart tests, including the cyclic journal and recovery-required cases.python scripts/nvx.py test-microvm --backend <whp|mshv> --processors 1, selectingmanaged-exec-config,sandbox-lifecycle,sandbox-blocks,scratch-snapshot,snapshot-tiers, andmanaged-lifecycle, with the branch's freshly built Alpine initramfs.The runtime artifacts came from
v0.1.0-dev.843c091bb835, with the pinned OpenVMM79a501b63fcd2f150e94928d0ceba79cd2fe96c4; guest agents were rebuilt from the tested sources. MSHV ran as root with the development-only--cpu-profile hostoverride, since its host cannot present the built-in Genoa profile. KVM and the complete backend matrix are left to CI.