Skip to content

feat(vmm): Phase 6.1 — Vm::snapshot_vmstate_only + VmstateOnly FC patch#190

Merged
WaylandYang merged 3 commits into
mainfrom
feat/v0.4-phase6.1-vmstate-only
May 29, 2026
Merged

feat(vmm): Phase 6.1 — Vm::snapshot_vmstate_only + VmstateOnly FC patch#190
WaylandYang merged 3 commits into
mainfrom
feat/v0.4-phase6.1-vmstate-only

Conversation

@WaylandYang
Copy link
Copy Markdown
Contributor

Summary

First PR of Phase 6 (live-fork BRANCH path). Establishes the FC half of the integration:

  • Vendor patch: SnapshotType::VmstateOnly added to deeplethe/firecracker:forkd-v0.4-mem-backend-shared-v1.12 (commit c5dff4bb1). Touches 5 files, +34/-3 LOC. The FC create_snapshot skips the memory-dump call when snapshot_type == VmstateOnly; the mem_file_path request field is still required by the schema but is never opened or touched.
  • Forkd wrapper: Vm::snapshot_vmstate_only(vmstate: PathBuf) in crates/forkd-vmm/src/lib.rs. Issues PUT /snapshot/create with the new type; passes /tmp/forkd-vmstate-only-mem-ignored as the unused mem_file_path. Documents the FC-fork dependency.

Both halves verified end-to-end on the dev box against a coding-agent snapshot:

=== boot FC ===
  FC pid = 2192729
  /snapshot/load -> HTTP 204
=== pause + VmstateOnly snapshot ===
  /vm pause -> HTTP 204
  /snapshot/create (VmstateOnly) -> HTTP 204
=== verify ===
  OK vmstate written: 21941 bytes
  OK mem_file_path placeholder untouched
  /vm resume -> HTTP 204
  OK FC pid 2192729 still alive after resume
PATCH VERIFIED (VmstateOnly)

Unblocks Phase 6.2 (memfd handle getter) and 6.3 (the live BRANCH path itself). See DESIGN-v0.4-PHASE6.md in #189.

Test plan

  • Patched FC builds clean (tools/devtool build --release, ~1.8 min).
  • test-vmstate-only.sh smoke test passes against the rebuilt binary.
  • vmstate JSON written, mem_file_path placeholder untouched, FC survives resume.
  • CI green (this PR adds no runtime code in the default path — snapshot_vmstate_only is unused until Phase 6.3 wires it in).
  • docs/VENDORED-FIRECRACKER.md will be updated to list the 4th vendored commit in a follow-up after chore(vendor): refresh FC patch to v1.12 with full build-clean diff #188 merges (avoiding a conflict with that PR's edits to the same file).

🤖 Generated with Claude Code

Wraps the new `snapshot_type: "VmstateOnly"` variant landed in the
vendored FC fork (deeplethe/firecracker commit c5dff4bb1, on
forkd-v0.4-mem-backend-shared-v1.12). The patched FC writes vmstate
JSON but skips memory.bin entirely — forkd-controller will own that
write via the WpBranch async-copy path in PR 6.3.

Verified end-to-end on the dev box with the rebuilt patched binary:

  === pause + VmstateOnly snapshot ===
    /vm pause -> HTTP 204
    /snapshot/create (VmstateOnly) -> HTTP 204
  === verify ===
    OK vmstate written: 21941 bytes
    OK mem_file_path placeholder untouched
    /vm resume -> HTTP 204
    OK FC pid 2192729 still alive after resume

Signed-off-by: Phase 6.1 of DESIGN-v0.4-PHASE6.md.
1. Broken doc link `../../DESIGN-v0.4-PHASE6.md` — from
   `crates/forkd-vmm/src/lib.rs` repo root is 3 levels up, not 2.
   Use `../../../`.

2. Move `mem_file_path` placeholder from `/tmp/...` to
   `/dev/null/forkd-vmstate-only-mem-ignored`. The vendored FC
   skips opening the file for VmstateOnly, but if a future
   regression ever did open it, `/tmp` is attacker-pre-creatable
   (symlink to a real file -> truncated by FC's `set_len`).
   Routing through `/dev/null/...` makes `open(2)` return ENOTDIR
   loudly — defense in depth, no behavior change today. Promoted
   to a `VMSTATE_ONLY_MEM_PLACEHOLDER` const so the choice is
   documented in one place.

3. Signature now takes `memory: PathBuf` + `volumes: Vec<VolumeSpec>`
   and returns `Snapshot` — symmetric with `snapshot_to` /
   `snapshot_diff_to` so Phase 6.3's branch_sandbox call doesn't
   have to manually reconstruct the Snapshot record (memory.bin
   path + volumes are needed for the post-pause copy pipeline and
   restore-time volume re-attach).

4. Add 2 unit tests:
   - `vmstate_only_placeholder_is_unwriteable` — confirms the
     placeholder really does return ENOTDIR if opened, so the
     defense in #2 doesn't silently break on a future Linux
     kernel.
   - `vmstate_only_request_body_shape` — asserts the JSON body
     has `snapshot_type: "VmstateOnly"` and `mem_file_path`
     starts with `/dev/null/`. Catches typos to either string
     (stock + patched FC reject unknown variants with 400) at
     compile-time-adjacent speed instead of waiting for the
     dev-box smoke test.

5. TODO comment about `SNAPSHOT_TIMEOUT_SECS` being sized for
   multi-GB snapshots while VmstateOnly takes ~22 ms. Inside
   Phase 6.3's <10 ms pause window a hung FC would extend
   source-paused for the full 60 s. Tighten when wiring into
   branch_sandbox.

Verified on the dev box: 23/23 forkd-vmm tests pass (including
the 2 new ones); smoke test against the patched FC (which still
runs through the rest of the snapshot pipeline) still succeeds.
clippy::suspicious-open-options requires an explicit truncate() call
on OpenOptions when combining write(true) + create(true). The
existing test doesn't actually expect the open to succeed (the path
routes through /dev/null/ and returns ENOTDIR), so truncate(false)
is the semantically-honest choice.
@WaylandYang WaylandYang merged commit 63bca01 into main May 29, 2026
2 checks passed
@WaylandYang WaylandYang deleted the feat/v0.4-phase6.1-vmstate-only branch May 29, 2026 08:36
WaylandYang added a commit that referenced this pull request May 29, 2026
Full-workspace audit (cargo doc --document-private-items + RUSTDOCFLAGS=-D warnings)
turned up 16+ unresolved-link / unclosed-HTML-tag errors. None are
runtime bugs but they break the published rustdoc:

  - 2 fresh errors from PR #190's snapshot_vmstate_only doc
    comment: `[snapshot_to]` / `[snapshot_diff_to]` intra-doc links
    didn't resolve because rustdoc needs the `Self::` prefix when
    pointing at sibling methods. Use `Self::snapshot_to`.

  - 11+ pre-existing errors in crates/forkd-cli/src/main.rs:
    angle-bracketed CLI placeholders (`<name>`, `<path>`, `<file>`,
    `<sanitized-tag>`, `<image-slug>`, `<tag>`) were treated as
    unclosed HTML tags. Wrap the affected subcommand-listing block
    in a `text` code fence; individually backtick the remaining
    placeholders in arg docs (`/tmp/forkd-parent-<tag>/`,
    `~/.local/share/forkd/snapshots/<tag>/`, etc.).

  - 1 error: bare `[:ro]` after `HOST_FILE:GUEST_PATH` parsed as
    an intra-doc link with empty target. Wrap the whole format
    string in backticks.

  - 1 error: bare `https://forkd-hub.deeplethe.com.` got flagged
    "this URL is not a hyperlink". Wrap in angle brackets so
    rustdoc treats it as an autolink.

  - 1 error: `[`DESIGN-v0.4.md`]` in the kvm-uffd-wp PoC was a
    dangling intra-doc link. Switch to plain prose reference.

  - 1 error: angle brackets in hub.rs MetaJson doc comment for
    parent_tag.

Verified: `cargo doc -D warnings` exit 0, fmt/clippy/test all
still pass (85 tests, 0 failures).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant