Skip to content

fix(apple): use explicit cached disk modes for Linux guests - #144

Merged
kerdofficial merged 1 commit into
mainfrom
fix/linux-disk-caching
Nov 4, 2026
Merged

kerdofficial merged 1 commit into
mainfrom
fix/linux-disk-caching

Conversation

@kerdofficial

@kerdofficial kerdofficial commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Linux guests sometimes die during long write-heavy jobs with guest filesystem Input/output error (runner log writes fail, exit 134) or stop answering SSH. The Swift helper attached Linux disks with VZDiskImageStorageDeviceAttachment(url:readOnly:), leaving caching and synchronization modes implicit, which is the configuration other Virtualization.framework projects saw corrupt Linux filesystems (lima #1957, Tart #675, UTM's filesystem workaround). They fixed it by choosing the modes explicitly.

Linux boot disks and repository cache disks now use cachingMode: .cached and synchronizationMode: .full. macOS guest disks and the read-only setup seed are unchanged.

Proof: every Linux VM I booted overnight in isolated dev homes used this helper (image preparations, tools/serial/headroom/sleep/memory checks, stress rounds) without a disk error. In a controlled A/B under real load (2 VMs per arm, 20 minutes of repeated pnpm install of this repository plus an fsync file writer, while the host wrote 8 GiB files to the same USB SSD, guest memory well below host RAM), both arms had 0 kernel I/O errors and 0 hung tasks; the new mode completed 24 install cycles per VM versus 21 with implicit modes, so no throughput regression. A second round ran the full CI loop of this repository in each guest (pnpm install + pnpm check) under the same host write pressure: again 0 I/O errors and 0 hung tasks in both arms, with equal throughput (2 CI cycles and 4 writer rounds per VM). Neither arm reproduced the field EIO, so this PR is backed by the documented Virtualization.framework behavior rather than an observed before/after failure. swift build --package-path native/apple passes.

Summary by CodeRabbit

  • Improvements
    • Linux boot disks and optional cache disks now use cached I/O with full synchronization.
    • macOS boot disks retain their existing attachment settings, without the Linux-specific I/O setting.

@github-actions github-actions Bot added the size:S 10-29 effective changed lines, test files excluded in mixed PRs. label Nov 3, 2026
@coderabbitai

coderabbitai Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Walkthrough

Linux boot disks and optional cache disks now use cached I/O with full synchronization. macOS boot disks retain the default attachment settings.

Changes: Disk attachment settings

Layer / File(s) Summary
Configure boot and cache disk I/O
native/apple/Sources/VectisVM/main.swift
Linux boot disks and optional cache disks use cached I/O with full synchronization. macOS boot disks retain the default settings.
Merge Risk: Minimal · up to 0024f

The disk attachment changes match the intended Linux and cache-disk behavior without an established merge-blocking risk.

Security architecture risk: Low · up to 0024f

The change is contained to disk-attachment settings and does not expand guest access or change disk ownership. It affects Linux boot disks and optional cache disks for both Linux and macOS. Crash and power-loss durability have not been independently demonstrated.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The affected data scope is the existing writable Linux boot images and optional Linux or macOS cache images attached through this helper. The diff does not add disk targets, callers, or writable authority.

Security Findings and Attack Paths

  • inferred — No introduced or worsened attack path was established in the changed flow. Existing caller-supplied disk paths reach the same attachment operations; the modification changes I/O modes rather than input reachability or privilege.

Trust Boundaries and Controls

  • observed — The helper retains its existing OS and argument checks, absolute cache-path requirement, read-only seed attachment, and boot/cache URL inequality check. These are preserved controls, not new guarantees of path containment or underlying file identity.

Resilience and Maintainability Implications

  • observed — The base/head comparison preserves validation, VM start, signal handling, normal stop, delegate error callbacks, and terminal error exits. No ownership, retry, cleanup, or recovery transition is added by this change; abrupt-failure durability remains unverified.
Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title names the main change: Linux guest disks switch to explicit cached disk modes. The title follows conventional commit form and matches the raw summary and PR objectives. It is concise and spe…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description check Passed The description gives a clear problem statement, the specific change (explicit cachingMode .cached and synchronizationMode .full for Linux boot and cache disks, macOS disks unchanged), supporting evid…
Full details: Title check

Explanation

The title names the main change: Linux guest disks switch to explicit cached disk modes. The title follows conventional commit form and matches the raw summary and PR objectives. It is concise and specific.

Full details: Description check

Explanation

The description gives a clear problem statement, the specific change (explicit cachingMode .cached and synchronizationMode .full for Linux boot and cache disks, macOS disks unchanged), supporting evidence from A/B tests, and a build check. It does not follow the repository template: the required 'What changed' and 'Why' headings are not used as headings, the 'UI changes' section is not addressed (it is not applicable here, but it is not marked as deleted), and the Checklist is missing entirely, including the conventional-commit title check, 'pnpm check' status, and surface coverage. The author reports only 'swift build --package-path native/apple' passing, not 'pnpm check'. The description is mostly complete on substance, so this is a pass with minor template gaps.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches
Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.
Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 05c10ec9-849d-4ae5-bb60-213cbdd31e0c
Commits

Reviewing files that changed from the base of the PR and between 80362c0 and 0024f7e.

Files selected for processing: 1
  • native/apple/Sources/VectisVM/main.swift

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

@kerdofficial
kerdofficial merged commit 19521c0 into main Nov 4, 2026
7 checks passed
@kerdofficial
kerdofficial deleted the fix/linux-disk-caching branch November 4, 2026 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 effective changed lines, test files excluded in mixed PRs.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant