Skip to content

feat(vmm): give every restored child its own rootfs backing - #321

Draft
jrimmer wants to merge 11 commits into
deeplethe:devfrom
jrimmer:feat/per-child-rootfs-backing
Draft

jrimmer wants to merge 11 commits into
deeplethe:devfrom
jrimmer:feat/per-child-rootfs-backing

Conversation

@jrimmer

@jrimmer jrimmer commented Sep 12, 2026 •

Copy link
Copy Markdown
Contributor

Fixes the defect in #317 — concurrent children of one tag sharing a writable rootfs — and carries the version policy that comes with it.

The mechanism, and why it is this one

Neither released Firecracker has drive_overrides: I checked the v1.16.2 and v1.17.0 binaries, not the source, and neither contains the string. So the load-time override from #5774 is not available in a released build.

What a released Firecracker does have is PATCH /drives/{drive_id} on a restored VM, and it moves the device's storage rather than just its config. Measured: after re-pointing a child's rootfs, a write made inside the guest landed in the new backing and not in the original file.

So each child gets its own reflink clone of the tag's rootfs and is re-pointed at it before it runs:

1. cp --reflink=always <tag>/rootfs.ext4 <child>/rootfs.ext4     # free where the fs supports it
2. PUT   /snapshot/load {…, resume_vm:false}                     -> 204
3. PATCH /drives/rootfs {drive_id, path_on_host:<child backing>} -> 204
4. PATCH /vm {"state":"Resumed"}                                 -> 204

The ordering is load-bearing, and step 2 is why. Loading paused leaves the guest un-executed, so nothing it does at boot — journal replay, /var/log, agent startup — can reach the shared base before the drive is re-pointed. Measured: the base stays byte-identical through boot while the backing changes.

Fail closed. A child whose rootfs cannot be re-pointed is not resumed. It would run against the shared base, which is the defect, so the failure surfaces and the child is dropped (killing its Firecracker) rather than run unsafely.

Two API details worth having: PATCH /drives requires drive_id in the body (without it a 400 reads as a body error, not a capability error), and PUT /drives to add a drive post-restore fails with PCI is not enabled since forkd boots pci=off — irrelevant here, relevant if anyone wants per-child extra devices later.

Evidence

Claim How it was measured, on a KVM host running v1.17.0
children are isolated two children wrote the same guest path; each read back its own bytes (child-1: AAA=1 BBB=0, child-2: AAA=0 BBB=1)
the shared base is never written tag rootfs sha256 identical before and after both children wrote
backings are reclaimed on kill 2 backings → 0 after DELETE
a crash strands nothing SIGKILL the controller, let systemd restart it: 2 backings → 0
daemon-baked tags are covered POST /v1/snapshots now writes rootfs into snapshot.json; daemon branches inherit it
the predicate 21 assertions, each confirmed by running it

The crash case is interesting and shaped the sweep. On a restart, systemd kills the unit's cgroup, so the children die with the controller; the reap finds dead processes and correctly prunes rather than kills, so nothing reaches Vm::drop and nothing used to remove the backing — while the watchdog skips the directory because its socket file is still present. A sweep now runs before the reap, when those directories are unowned.

Its gate is process ownership, not file presence: a directory is skipped while any live Firecracker's --api-sock points inside it, and if any live Firecracker cannot be read, the sweep removes nothing. Leaking a backing costs disk; deleting a running VM's backing destroys its disk mid-job, so every ambiguity resolves toward keeping it.

The per-kill reclaim is kept alongside the sweep rather than replaced: it covers a process verified ours and confirmed dead at kill time, which the pre-reap sweep cannot see.

Consequences to plan for

  • No Firecracker upgrade is required. An earlier revision required v1.15 on the claim that older builds accept PATCH /drives without moving storage. That was secondhand and wrong: measured on stock v1.5.1, v1.7.0, v1.10.1, v1.12.1, v1.14.1 and v1.17.0, a restored VM's writes move to the new file and /vm/config reports it. Existing snapshots keep loading; nothing needs re-baking. Every re-point is still verified by reading /vm/config back, and a child is not resumed unless it reports its own backing.
  • The vendored fork is needed only for --live. It carries MemBackendConfig::shared, and forkd-vmm sends shared in exactly one place — the memfd path live-fork uses. Everything else runs on stock Firecracker.
  • Reflink. The FICLONE constant fix landed separately as fix(vmm): use the real FICLONE ioctl number #322; this branch is rebased on it. On a filesystem without cloning a backing is a full copy per child (now made in the children's restore threads, in parallel), so pool size is a storage decision there. Warning, free-space gating and an ext4 policy are proposed as a follow-up.
  • Branches and workspace suspends copy the rootfs inside the source's pause. The copy has to match the vmstate's instant. Near-free with reflink; a full copy without it — which matters most for live BRANCH, whose point is a short pause.
  • Backings live under the work dir, which is std::env::temp_dir()-derived (now from one helper shared with the sweep). On a host where /tmp is tmpfs that charges a child's rootfs copy to RAM; TMPDIR relocates it.
  • Writable volumes are still shared and are the same defect on /dev/vdb: VolumeSpec.read_only defaults to false and the path is frozen, so children of a tag carrying one all open the same file. Out of scope here; worth its own issue.

Not covered by the evidence

Both crash runs exercised the cgroup-kill flavour, because the children were spawned without a memory limit and so died with the controller. The other flavour — Firecracker outliving the controller, e.g. children in their own per-VM cgroup — exercises the kill path, which is verified through a plain DELETE but not yet through a crash.

Snapshots with no recorded rootfs keep the previous behaviour. Daemon-baked snapshots, branches and workspace suspends all record one now.

@jrimmer

jrimmer commented Sep 12, 2026 •

Copy link
Copy Markdown
Contributor Author

Defect found while checking what a consumer needs: branching a restored child is no longer durable

Branch used to be safe because a branch recorded the snapshot's rootfs, which persists. With per-child backings it records the child's own backing — inside a work dir that Drop and the sweep reclaim — so the clone dies with its source.

Measured on a v1.17 host with this branch deployed:

source child:            sb-6aa4f2a3-…
branch it -> tag:        br-clone created
kill the source child
spawn from br-clone:     400, "firecracker API PUT /snapshot/load returned 400:
                               Failed to restore from snapshot …"

Two things make it worse than a stale path. The clone's snapshot.json records no rootfs at all, so snapshot_restore_problem cannot see the missing file and forkd reports the FC load failure rather than "this tag's rootfs is gone" — an opaque 400. And the window is not just "source child exits": any restart whose sweep collects the backing breaks every tag derived from that child.

This matters beyond a corner case: Branch taken from a running sandbox is an ordinary way to clone or snapshot one, so clones would break intermittently depending on when their source was reaped.

The fix belongs here: on branch, materialise the child's rootfs into the new tag — a reflink clone of the backing, recorded as the new tag's rootfs — instead of leaving the vmstate pointing into an ephemeral work dir. That also restores the property that a tag's rootfs is its own, which pack/sidecar and snapshot_restore_problem both assume.

I would not merge this PR without it. I have not written it yet, and I am marking it here rather than leaving it for whoever reads the diff.

@jrimmer

jrimmer commented Sep 12, 2026

Copy link
Copy Markdown
Contributor Author

The clone path was dead: wrong FICLONE ioctl number

While checking what a consumer needed I measured the per-child cost on a ZFS pool and got a full copy — 12291 MiB of pool growth for one 12 GiB child, when both the source and the destination were on the same pool and that pool demonstrably clones (cp --reflink=always on the same files costs nothing).

chain::reflink_copy passes 0x40209409 for FICLONE. The kernel dispatches on the whole ioctl value including its size field, and that value encodes a 32-byte payload — FICLONERANGE's size — so it is answered with ENOTTY:

kernel:  #define FICLONE  _IOW(0x94, 9, int)      -> 0x40049409
on ZFS, same source file and directory:
  ioctl 0x40209409 -> ENOTTY        ioctl 0x40049409 -> clone

ENOTTY is in the list this function treats as "this filesystem does not support reflink", so it falls through to a full streamed copy — on every filesystem, silently, including the ones where cloning works. That is why the number being wrong was invisible: the failure path is the same as the legitimate one.

Two consequences beyond this PR: a restored child cost a full copy of the rootfs (measured above), and the bake path's "reflink preferred" baseline clone was a full copy too. Corrected in this PR; docs/VENDORED-FIRECRACKER.md-adjacent notes are not affected, but the cost model in the PR body is now accurate.

@WaylandYang WaylandYang 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.

Thanks. Pausing on a baked-in drive and re-pointing it with PATCH /drives before resume is the right mechanism, and the evidence table is exactly what this needed. Not merging yet. Beyond the branch-durability blocker you already flagged, here is what I found.

Already landed separately: the FICLONE fix you described (0x40209409 → 0x40049409) was never pushed to this branch; the diff doesn't touch chain.rs. It is in dev now as #322, credited to you and verified on an XFS reflink=1 loopback (old number → ENOTTY, new → shared extents). Please rebase onto dev.

Blocking

  1. Branch durability (your comment): a branch taken from a restored child has to materialise the child's backing into the new tag and record it as that tag's rootfs.
  2. No runtime Firecracker version gate. The CHANGELOG says PATCH /drives is accepted but doesn't move storage on FC < 1.15, so on an older binary children silently share the base again. That is the exact corruption this PR fixes, now with no signal. A documented minimum isn't enough; this needs to fail closed. Options: check firecracker --version when the controller/CLI starts, or GET /vm/config after the PATCH and refuse to resume unless the drive's path_on_host is the backing.
  3. Read-only rootfs snapshots pay a full copy per child for nothing. The CLI read-only (squashfs) bake records snap.rootfs = cfg.rootfs, and restore clones whenever self.rootfs.is_some(). Every child of a read-only tag now copies the squashfs and PATCHes a drive that can never be written. Snapshot doesn't persist the drive's read-only flag (your ask #2 on #317), so the fix probably starts there: record it at bake and skip backings for read-only drives.
  4. Cost on filesystems without reflink. On ext4 (the default on most hosts, including our test box) each child is a full streamed copy of the rootfs. The copies run sequentially in the loop before any load, so fork -n N costs N × rootfs in both disk and spawn latency, with no warning. At minimum: say once per restore that the fallback happened, gate on free space (the available_bytes helper from #316), and run the copies in the per-child threads. Whether ext4 hosts should get an explicit opt-in/refusal instead of silent full copies is a product call worth making in this PR.

Non-blocking

  • sweep_stranded_backings scans std::env::temp_dir() for forkd-daemon-*. It matches where the controller creates work dirs only while both use the same TMPDIR, so please derive both from one helper.
  • The upgrade note (re-bake every tag when moving to FC ≥ 1.15) is a release-level decision. We'll schedule it with the release rather than let it land silently in dev.

I can't exercise the KVM path on my side (the test host has no KVM), so your measured evidence plus a rerun after these changes will be what we rely on.

yermakoffivan pushed a commit to yermakoffivan/forkd that referenced this pull request Sep 14, 2026
0x40209409 encodes a 32-byte payload; FICLONE is _IOW(0x94, 9, int) =
0x40049409. The kernel answered ENOTTY, which the fallback reads as no
reflink, so every reflink_copy streamed a full copy on every
filesystem. Reported by @jrimmer in deeplethe#321.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@jrimmer

jrimmer commented Sep 28, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review, and for landing the FICLONE fix as #322.

A correction first: my earlier comment said the FICLONE constant was "corrected in this PR". It wasn't — that commit never reached this branch. #322 is the fix; I'll rebase onto dev and drop any overlap.

Plan for this PR:

  1. Branch durability — on branch from a restored child, reflink the child's backing into the new tag and record it as the tag's rootfs.
  2. Fail closed on Firecracker < 1.15 — after the PATCH /drives, read back GET /vm/config and refuse to resume unless the rootfs path_on_host is the child's backing. That checks the behaviour itself rather than a version string, so it also covers patched or vendored builds.
  3. Read-only rootfs — record the drive's read-only flag in the snapshot at bake, and skip the backing for read-only drives.

I'd like to split 4 (cost without reflink) into a follow-up PR, since it carries the product call you raised (silent full copy vs. opt-in vs. refusal on ext4). My proposal there: warn once per restore on fallback, gate on available_bytes, move the copies into the per-child threads, and refuse by default on a non-cloning filesystem unless an explicit opt-in is set. Happy to fold it in here instead if you'd rather review it all at once.

The non-blocking items (one helper for the work-dir root; the upgrade note belonging to the release) I'll take here.

I'll rerun the evidence table on KVM after the changes and post the results.

A snapshot's rootfs is one ext4, and Firecracker reopens that path verbatim for
every child, so children of one tag share a filesystem. With the drive opened
read-write — how every tag here was baked — two concurrent children are two
guest kernels writing one filesystem with no coordinator: package files pick up
other files' bytes, `/var/lib/dpkg` directory entries return EBADMSG, and the
damage reads as a random build failure rather than a sandbox error.

Firecracker cannot re-point a drive at load time (there is no `drive_overrides`
in any release — checked in the v1.16.2 and v1.17.0 binaries), but it *can*
re-point one on a restored VM via `PATCH /drives/{drive_id}`, and that call
genuinely moves the device's storage: a write made in the guest afterwards lands
in the new backing and not in the old file. So each child now gets its own
reflink clone of the tag's rootfs and is re-pointed at it.

The ordering matters and is the reason this is a three-call sequence rather than
one. Loading with `resume_vm: false` leaves the guest un-executed, so nothing it
does at boot — journal replay, `/var/log`, agent startup — can reach the shared
base before the drive is re-pointed; measured, the base stays byte-identical
through boot while the backing changes. Resuming after the re-point then hands
the guest a disk that was always its own.

**Fail closed.** A child whose rootfs could not be re-pointed is not resumed: it
would run against the shared base, which is the defect being fixed. The failure
propagates and the child is dropped (killing its Firecracker) rather than run
unsafely.

The backing lives in the child's work dir, so removing the dir reclaims it —
a crash that skips that leaves the same class of straggler as the sockets
already do. Snapshots with no recorded rootfs (daemon-side branches inherit the
source's) keep the previous single-call behaviour.

Requires Firecracker >= 1.15 for PATCH-on-restore, and a vmstate version match,
so it is deployed together with an upgrade and a re-bake.

Signed-off-by: jrimmer <jason@rimmer.net>
…uire

The vendored fork reads as mandatory today, which it is not: it carries the
opt-in `MemBackendConfig::shared` field and `forkd-vmm` sends `shared` in exactly
one place — the memfd path live-fork uses. Everything else, including the whole
bake/restore/branch/exec surface, runs on stock Firecracker. Correcting that also
gives somewhere to record the two version facts that now matter:

- the tested baseline is v1.17.0, and per-child rootfs backings need **v1.15+**,
  because `PATCH /drives` on a restored VM is accepted by older builds but does
  not move the device's storage — a hard minimum, not a warning, since the
  failure mode is silent sharing rather than an error;
- a vmstate is version-pinned, so upgrading invalidates every snapshot and means
  re-baking every tag.

Adds the matching upgrade note and the feature entry to CHANGELOG.

Signed-off-by: jrimmer <jason@rimmer.net>
Verifying the per-child backings on a live host showed the leak immediately:
after killing two children, both 1.6 GiB copies were still on disk and the work
dir was still there. Work dirs are not reclaimed on their own — the host had
seven leftover ones holding only sockets and consoles, which was harmless until
a backing moved in.

The child owns its backing now, and `Drop` removes it alongside the socket and
cgroup it already cleans up, so a kill reclaims the copy without depending on
the caller's work-dir hygiene. Set from the backing pass rather than at
construction, because the child has to exist before anything can own it.

A controller crash can still strand a backing; that is the same class as the
stale staging dirs and `.prev-*` backups, which are swept by their next use
rather than by Drop.

Signed-off-by: jrimmer <jason@rimmer.net>
Restore clones each child's writable backing from the snapshot's recorded
rootfs, so a snapshot with no recorded path has nothing to clone and its
children fall back to sharing one file. The CLI has always written the field;
the daemon never did, which left daemon-created tags — `cp`/clone and preview
branches — as the one place children still shared a rootfs.

The path is the one the parent actually booted from, canonicalized, matching
what the CLI records. Daemon-created branches already inherit `rootfs` from
their head snapshot, so those are covered by this single site.

Signed-off-by: jrimmer <jason@rimmer.net>
A child killed while the controller is up has its backing removed by Vm::drop.
The other order — controller crash, then startup reap — had nothing collecting
them: the watchdog's stale-dir sweep keys on child-1.sock being absent, and it
is present because the orphaned child held it until the reap killed it.

The reap now reads each orphan's work dir from its own --api-sock while /proc
still has it, and removes the child-* backings in that dir once the process is
confirmed dead. Only the backings are removed, never the dir (the watchdog owns
that), and only for a directory whose process this reap just reaped, so a live
VM cannot be caught.

Signed-off-by: jrimmer <jason@rimmer.net>
…shapes

The predicate selects what a startup reap deletes, and the work dir holds
`child-<n>.sock` and `child-<n>.console` under the same prefix, while the
stem is the tag's rootfs filename and so not always `rootfs.ext4`. Each
assertion names the shape it is defending.

Signed-off-by: jrimmer <jason@rimmer.net>
The per-kill reclaim could not cover a row retired *without* a kill. The crash
simulation showed the ordinary restart shape: systemd kills the unit's cgroup,
so the children die with the controller, the reap finds dead processes, and it
prunes rather than kills — which is correct, but nothing then reaches `Vm::drop`,
so nothing removes the backing. The watchdog leaves that directory alone because
the socket file is still present.

The sweep runs before the reap, when those children are already gone and their
directories are unowned. Its gate is process ownership rather than file
presence: a directory is skipped while any live Firecracker's `--api-sock`
points inside it, and if any live Firecracker cannot be read the sweep removes
nothing, because ownership can no longer be established. Leaking a backing costs
disk; deleting a live VM's backing destroys that VM's disk mid-job.

Signed-off-by: jrimmer <jason@rimmer.net>
… happy path

The predicate already had the cases its callers rely on. These are the ones a
reader would guess the other way, and two of them guard against a plausible
future edit:

- `child-1.sock.bak` and `child-1.SOCK` match, because the comparison is against
  everything after the first dot rather than a suffix. Nothing creates such a
  file; the assertions exist so the approximation is visible rather than assumed
  exact.
- a multi-dot or non-ext4 stem (`child-1.tar.gz`, `child-1.rootfs.squashfs`)
  matches, because the stem is the tag's rootfs filename. Tightening this to
  `.ext4` would look like a hardening and would silently stop collecting
  backings for any tag whose rootfs is not an ext4.
- the index is never parsed, so leading zeros and absurd values pass.
- ASCII digits only: generalising to `is_numeric()` would accept unicode digits
  and start deleting names we never created.

Every assertion was confirmed by running it, not by reading the predicate.

Signed-off-by: jrimmer <jason@rimmer.net>
…their own rootfs

Three defects in per-child backings, found in review and on a KVM host.

Branches did not own their rootfs. A snapshot of a restored child recorded the
child's backing, a file in a work dir that dies with the child, so the new tag
stopped loading once its source was gone ("Failed to restore from snapshot ...
No such file"), and until then every child restored from it wrote the source's
live disk. BRANCH (full, diff and live) and workspace suspend now point the
paused source's drive at a clone the new tag owns for the length of the
snapshot, record it as the tag's rootfs, and point the drive home before the
source resumes. A source whose drive cannot be pointed home is killed rather
than resumed onto the tag's disk.

A PATCH was trusted on its 204. Every re-point now reads GET /vm/config back
and fails unless Firecracker reports the new path, so a child is never resumed
on the shared base on the strength of an accepted request.

Read-only rootfs was copied per child for nothing. Whether the drive is
writable is now read from the restored vmstate itself, so read-only tags,
including ones baked before this, share the file without a copy. The copies
move into the per-child restore threads, so they run in parallel.

The restore work-dir root and its prefix come from one helper, shared by the
spawn path and the startup sweep.
…ease measured

The floor rested on a claim that PATCH /drives on a restored VM is accepted but
does not move storage before v1.15. Measured on stock v1.5.1, v1.7.0, v1.10.1,
v1.12.1, v1.14.1 and v1.17.0: every one moves the guest's writes to the new
file and reports the new path in /vm/config. So no upgrade, and no re-bake, is
needed for per-child backings, and the upgrade note goes with it.
@jrimmer
jrimmer force-pushed the feat/per-child-rootfs-backing branch from 60d0b8c to 3f98a20 Compare September 28, 2026 21:18
@jrimmer

jrimmer commented Sep 28, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto dev (with #322) and pushed. Blockers 1–3 are addressed; 4 is proposed as a follow-up as described above.

A correction that removes blocker 2's premise

The "Firecracker ≥ 1.15, else PATCH /drives is accepted but storage doesn't move" claim was secondhand and never measured. I measured it: restore a snapshot paused, PATCH /drives/<id> to a copy, resume, and watch which host file the guest's writes land in.

stock release (aarch64) PATCH guest writes land in /vm/config reports
v1.5.1 204 new file new path
v1.7.0 204 new file new path
v1.10.1 204 new file new path
v1.12.1 204 new file new path
v1.14.1 204 new file new path
v1.17.0 204 new file new path

So there is no version floor and no re-bake: the upgrade note is gone from CHANGELOG and VENDORED-FIRECRACKER.md. The fail-closed check is still there, in the form you suggested: every re-point is followed by GET /vm/config, and the child is resumed only if Firecracker reports its own backing as the rootfs. Unit-tested against a fake API socket that answers 204 and keeps the old path: the child is never sent Resumed.

What changed

  1. Branch durability. A snapshot of a restored child now owns its rootfs. While the source is paused, its drive is pointed at a clone in the new tag's dir, the snapshot is taken (so the vmstate records a durable path), and the drive is pointed home before resume. The clone is recorded as the tag's rootfs. If the drive can't be verified home, the source is killed rather than resumed onto the tag's disk; a drop guard covers every early return. This covers full, diff and live BRANCH, and workspace suspend, which had the same defect. Note that branches previously wrote rootfs: None, so children of a branch tag also shared the source's live backing, not only lost it when the source died.
  2. Fail closed on the re-point — above.
  3. Read-only rootfs. Rather than a new snapshot.json field, the restore thread reads is_read_only from /vm/config after the paused load — i.e. from the vmstate itself — and skips the backing. That also covers read-only tags baked before this change.
  4. (partial) The per-child copies moved into the per-child restore threads, so they run in parallel. Warning, free-space gate and the ext4 policy are for the follow-up.
  • Non-blocking: work-dir root and prefix now come from one helper used by both the spawn path and the sweep.

Evidence: forkd-vmm against stock Firecracker v1.17.0 on KVM

Guest /init writes a counter to /dev/vda offset 0 every 100 ms, so a file's first bytes show which host file the guest is writing.

check result
two concurrent children of one writable tag each write their own backing TICK…24 → …34 in each backing
the tag's rootfs through that byte-identical
branch a child with the detour; source after resume back on its own backing (…36 → …46)
the branch's rootfs after the source resumes unchanged, holds the snapshot instant (…35)
kill the source its backing is reclaimed
restore the branch after its source is gone loads; the child continues from …35 on its own backing; branch rootfs unwritten
control: same branch without the detour, kill source, restore PUT /snapshot/load returned 400 … Failed to restore devices … No such file — the defect reproduced
read-only tag, two children both restored, no backing made, nothing to detour

Not covered yet

  • This ran on aarch64 KVM through forkd-vmm directly, without netns, so multi-child batches ran as two single-child batches of the same tag. The controller's BRANCH/suspend handlers are covered by build, clippy and unit tests, not yet by a KVM run through the daemon. I'll rerun the full table through the controller on our x86 host (v1.17.0) and post it here before this leaves draft.
  • The two forkd-vmm tests that fail on this machine (apply_diff_copies_only_data_regions, assemble_chain_memory_produces_correct_bytes) fail identically on dev: they assume 4 KiB pages and this host has 16 KiB.

The .prev rootfs sweep leaves a backup alone while the pid in its name is a
running process, since it belongs to a concurrent bake. The tests named their
"dead" backups prev-1000 and prev-2000, which are often live pids on a CI
runner, so the sweep skipped them and four tests failed depending on what the
runner happened to be running. Pointing the constant at pid 1 reproduces the
exact failures. Use pids above PID_MAX_LIMIT (2^22), which the kernel never
assigns.
@jrimmer

jrimmer commented Sep 28, 2026

Copy link
Copy Markdown
Contributor Author

CI note: bd01919 fixes a flaky test from #314, not part of this change. The .prev sweep tests used made-up "dead" pids 1000/2000, which can be live on a runner, and the sweep rightly leaves a live pid's backup alone. Setting the pid to 1 reproduces the exact failures; they now use pids above PID_MAX_LIMIT. Happy to split it into its own PR if you prefer.

keeganmccallum added a commit to urun-sh/forkd that referenced this pull request Oct 15, 2026
…nk probe, REST smoke spawn (#3)

* feat(cli): doctor readiness gate — --json, FC version pin, netns floor, reflink workdir probe, REST smoke spawn

forkd doctor today is a human diagnostic (18 checks, glyph report). A
serving pool needs a GATE: a cpu-sbx node is ready only when every
blocking check is green AND the node can actually fork.

- --json: machine-readable {ready, checks[]} output; exit code is
  the gate (0 = no FAIL). WARN never blocks, same as the human path.
- --expected-fc-version: exact pin compare (snapshot-family rule:
  restore only on the exact FC version x CPU family x guest kernel;
  '>=1.10' is too loose for a homogeneous pool).
- --min-netns N: fan-out >1 needs one forkd-child-* netns per
  concurrent child; fewer = FAIL.
- reflink workdir check (new, always on): per-child rootfs backing
  (PR deeplethe#321) is FICLONE on XFS (~6 ms/fork) but a full stream copy on
  ext4 (~265 s/fork for a 24G base). Probes ioctl(FICLONE) in TMPDIR.
- --smoke-tag TAG [--smoke-port P]: REST-spawn one sandbox through
  the live daemon (per_child_netns), optionally TCP-probe envd
  (:49983), always delete. Proves restore+fork e2e.

tempfile moves dev->runtime deps (doctor uses it on the probe path).

* fix(cli): doctor smoke spawn probes envd via the child netns

The guest of a per-child-netns sandbox lives at 10.42.0.2 INSIDE its
netns; from the root netns the probe got 'No route to host'. Probe
now setns()es via 'ip netns exec' in a helper child (never in-process)
and retries for 10s to cover envd/socat bind lag — same contract as
lane-02's init-loop.sh. Non-Linux gets a plain-connect stub.
Also derive Default for DoctorOpts (clippy derivable_impls).

* feat(cli): doctor warns on ARP gc_thresh defaults that starve big netns pools

Each per-child netns carries its own ARP neighbor table. The Linux
defaults (gc_thresh1/2/3 = 128/512/1024) overflow once a pool does
n~100 REST fan-out spawns: every subsequent spawn then stalls in
guest-net bring-up with 'neighbour: arp_cache: neighbor table
overflow!' in dmesg (observed live on the Phase 0 C3 node, forkd-p0-1:
POST /v1/sandboxes >55s stall while GETs stayed instant).

WARN, not FAIL: a fresh node passes until it fans out hard, and the
fix is one sysctl — but a serving pool must raise it at bootstrap.

* fix(cli): doctor tests — Check::warn/fail take a hint argument

* fix(cli): doctor review fixes — serde_json spawn body, timeout on netns probe, assert serialized readiness

CodeRabbit PR#3:
- smoke spawn body built with serde_json::json! (a --smoke-tag with
  quotes/backslashes previously produced malformed JSON and could
  inject extra fields like n/memory_limit_mib).
- the in-netns TCP probe is argv-wrapped in `timeout 3` (the kernel's
  SYN retry window is ~2 min; without the cap a dropped-SYN guest
  blocked the readiness gate for minutes past the 10 s deadline) and
  the address is passed as a positional parameter ($1), never
  interpolated into the shell text.
- json gate test asserts the serialized {"ready":…} value (via the
  extracted json_output helper) instead of re-testing the predicate.

* fix(cli): smoke spawn FAILS when cleanup DELETE fails (CodeRabbit PR#3)

A leaked probe sandbox on a 'passing' node defeats the gate: it holds
a netns and a firecracker slot. The cleanup closure now checks the
DELETE result and downgrades the Check to fail on transport error or
non-2xx.

* fix(cli): netns probe used the colon /dev/tcp form — bash requires /dev/tcp/HOST/PORT

'exec 3<>/dev/tcp/10.42.0.2:49983' is not valid bash /dev/tcp syntax;
it fails with 'No such file or directory' even when the guest answers.
Live diagnosis on forkd-p0-1: colon form always fails, slash form
(/dev/tcp/10.42.0.2/49983) connects in the same child. The address is
a parse-validated SocketAddr, so building the probe text from it
carries no injection surface.
keeganmccallum added a commit to urun-sh/forkd that referenced this pull request Oct 15, 2026
…n on the Ubuntu 22.04 sandbox hosts (#6)

* fix(vmm): best-effort guest clock sync after snapshot restore (Option B)

After snapshot restore, the guest's KVM clock reflects the snapshot's
saved time, not the host's current wall time. For snapshots created
minutes or hours ago, the guest clock starts in the past, causing TLS
certificate validation failures, incorrect timestamps, and token
expiry issues inside the guest.

This adds Phase 2.5 to restore_many_with: after the parallel restore
PUT completes, poll each child's guest agent (forkd-agent.py on port
8888) until it responds, then exec `date -s @<host_unix_ts>` to set
the guest clock to the host's current wall time. All children in a
batch are synced in parallel.

This is the cross-platform, cross-kernel-version alternative to the
`clocksource=kvm-clock` boot arg (PR deeplethe#289), which is x86-only and
forward-only (requires re-baking existing snapshots). This approach
works on any architecture, any Linux kernel, and any existing
snapshot without re-baking.

Best-effort and non-fatal: if the guest agent doesn't come up within
10 seconds, or `date` is unavailable, the sandbox remains usable with
a stale clock. Failures are logged as warnings.

References deeplethe#288 (stale sandbox VM clock problem).

Signed-off-by: jrimmer <jrimmer@users.noreply.github.com>
Signed-off-by: jrimmer <jason@rimmer.net>

* fix: clippy doc_lazy_continuation in sync_guest_clocks doc comment

Signed-off-by: jrimmer <jason@rimmer.net>

* test(vmm): add integration test for guest clock sync + fix exec args

Fix a bug in sync_guest_clocks: the exec args were ["-c", cmd] which
causes subprocess.run to try executing a program named "-c" (fails with
FileNotFoundError). Changed to ["sh", "-c", cmd] so the date command runs
through the shell as intended. The bug was silently masked because clock
sync is best-effort (failures logged as warnings, not propagated) and
the clocksource=kvm-clock boot arg (deeplethe#289) was correcting the clock on
modern kernels.

Add sync_guest_clocks_corrects_wrong_clock integration test:
- Boots a fresh VM via Vm::boot
- Deliberately sets the guest clock to 2001-09-09 (old_ts=1000000000)
- Calls sync_guest_clocks on the single VM
- Verifies the guest clock is corrected to host wall time (within 5s)
- Verifies the corrected time is NOT the mock old time

This isolates the scripting solution: even if KVM_CLOCK_REALTIME already
corrected the clock, the test forces a wrong clock and verifies the
sync function fixes it. The mock clock underneath is fine — we're
testing the mechanism (date -s via guest agent exec), not the
environment (kernel clock behavior).

The test is #[ignore] (requires Linux + KVM + firecracker + rootfs)
and #[cfg(target_os = "linux")].

Signed-off-by: jrimmer <jason@rimmer.net>

* fix(vmm): per-child timestamp, opt-in clock sync, reject n>1 shared-IP

Three findings addressed:

1. Host timestamp captured before polling agent — a child that became
   ready near the 10-second deadline was set ~10 seconds in the past.
   Fix: capture SystemTime::now() per child AFTER ping succeeds,
   immediately before the date -s exec. This minimizes the gap between
   timestamp capture and clock correction.

2. restore_many_with unconditionally performed the agent protocol —
   custom/no-agent snapshots added up to 10 seconds to every restore
   even though failure is only a warning.
   Fix: clock sync is now opt-in via ForkOpts::sync_guest_clocks
   (default false). The daemon's spawn paths (create_sandbox,
   spawn_one_for_workspace) set it to true because all forkd images
   contain forkd-agent. CLI paths keep it false — the CLI doesn't
   know whether the image has an agent.

3. For n>1 with per_child_netns=false, every worker contacted the
   same 10.42.0.2:8888 — the shared IP/MAC topology is fundamentally
   conflicting.
   Fix: restore_many_with now rejects n>1 with per_child_netns=false
   with a clear error message directing callers to use
   per_child_netns=true for multi-child spawns.

Signed-off-by: jrimmer <jason@rimmer.net>

* fix(vmm): expose per-child clock sync outcomes for fail-closed callers

sync_guest_clocks now returns (u128, Vec<ClockSyncOutcome>) instead
of just u128. ClockSyncOutcome is an enum with Synced{child_index}
and Failed{child_index, error} variants, exposed via a new
ForkResult::clock_sync_outcomes field.

Callers (e.g. the controller's create_sandbox) can inspect the
per-child outcomes to decide whether to fail-closed or degrade when
some children have unsynchronized clocks. The controller now logs
clock_sync_failures alongside the existing timing fields.

Also fixes the unused child_index warning from the thread closure
(the index is used in the collection loop, not inside the thread).

Tests:
- clock_sync_outcome_construction_and_matching: verifies enum
  discriminants, field access, and the filter-for-failures pattern
  callers use to decide fail-closed behavior.
- sync_guest_clocks_corrects_wrong_clock (#[ignore] integration):
  now also asserts the outcome vec contains exactly one Synced
  entry for the single child.
- fork_result_has_clock_sync_ms_field: updated to include the new
  clock_sync_outcomes field.

Signed-off-by: jrimmer <jason@rimmer.net>

* fix(vmm): clippy unused-enumerate-index + behavioral clock sync tests

Two CI/coverage gaps from the review:

1. Clippy (rust CI was red): the sync_guest_clocks child loop used
   .enumerate() and discarded the index (clippy::unused_enumerate_index).
   Drop the .enumerate() and iterate children directly.

2. Behavioral coverage for the three review asks:
   - multi-child address/mode: restore_many_with_rejects_multi_child_shared_ip
     unit test proves n>1 with per_child_netns=false is rejected before any
     Firecracker spawn or filesystem side effect.
   - no-agent/disabled-sync latency: fork_opts_sync_guest_clocks_defaults_disabled
     unit test proves the opt-in default keeps sync off (no added latency for
     custom/no-agent images).
   - delayed agent readiness (timestamp freshness):
     sync_guest_clocks_delayed_agent_readiness (#[ignore] integration) boots a
     VM, calls sync_guest_clocks immediately (no pre-wait) so it must poll for
     the not-yet-ready agent, then asserts the guest clock matches host wall
     time within 3s — a stale pre-poll timestamp would drift by the poll
     duration.

Signed-off-by: jrimmer <jason@rimmer.net>

* fix(vmm): deterministic delayed-agent-readiness test for clock sync

WaylandYang's final review round (round 5, 2026-08-14) flagged the
delayed-agent-readiness integration test as non-deterministic: it
called sync_guest_clocks immediately and only checked the final drift
plus an upper elapsed bound, so it never guaranteed that agent
readiness was actually delayed beyond the accepted 3-second drift. If
the agent came up quickly, the old pre-poll-timestamp implementation
still passed the test.

Extract the timestamp-placement logic from sync_guest_clocks into
sync_one_child_clock(ping, now, set_clock), which accepts injectable
readiness-probe / timestamp-source / clock-setter closures. The
production call site wires these to ping_at/ping_in_netns,
SystemTime::now, and the date -s exec; the test wires them to a
deterministic probe that fails for 4 seconds (> the 3s accepted drift)
then succeeds.

The new unit test sync_one_child_clock_timestamp_fresh_after_delayed_ping:
  1. asserts the induced poll delay (4s) exceeds the accepted drift (3s),
     so the freshness assertion is meaningful,
  2. asserts the timestamp passed to set_clock was captured AFTER ping
     succeeded (now_called_at >= probe_ready_at, freshness_gap < 3s) --
     a pre-poll-timestamp implementation would fail by ~4s,
  3. asserts set_clock was actually invoked with that fresh timestamp.

No VM, guest agent, or KVM is required -- it runs in the unit-test suite
on every platform. The end-to-end mechanism (real agent + date -s) is
still covered by the #[ignore] integration test
sync_guest_clocks_corrects_wrong_clock.

Verified on Linux (rust 1.83, x86_64): cargo clippy clean for
forkd-vmm/controller/cli/uffd; cargo test -p forkd-vmm --lib passes
48 passed, 0 failed, 3 ignored.

Signed-off-by: jrimmer <jason@rimmer.net>

* fix(vmm): address ce-code-review on clock sync refactor

Apply the three cross-reviewer findings from the ce-code-review pass
(5 reviewers: correctness, testing, maintainability, reliability,
adversarial):

1. Timeout error dropped the agent address (P2, safe_auto) — flagged by
   correctness, testing, and adversarial. In multi-child restores the
   operator could not tell which child's agent failed to come up. Now
   wrapped at the call site: `.map_err(|e| e.context(format!(
   "guest agent at {addr} did not come up")))`.

2. No unit test for the timeout path (P2, manual) — flagged by testing,
   reliability, and adversarial. Only the #[ignore] integration tests
   exercised it, and the 10s production timeout made a dedicated unit
   test impractical. The timeout is now an injected parameter on
   sync_one_child_clock, and sync_one_child_clock_times_out_when_probe_
   never_succeeds uses a 500ms injected timeout to verify the bail
   message and that now()/set_clock are never called on the timeout
   path.

3. No unit test for set_clock failure propagation (P2, manual) —
   flagged by testing, reliability, and adversarial.
   sync_one_child_clock_propagates_set_clock_failure verifies a
   set_clock error propagates verbatim and that now() was still called
   (timestamp captured before the failed set).

Acknowledged (P3 advisory, no action): the 4s induced delay makes the
freshness test the slowest non-ignored unit test; wall-clock second
granularity in the freshness assertions is robust given the 4s>3s
headroom and the compile-time invariant guard.

Verified on Linux (rust 1.83, x86_64): cargo clippy clean for
forkd-vmm/controller/cli/uffd; cargo test -p forkd-vmm --lib passes
50 passed, 0 failed, 3 ignored.

Signed-off-by: jrimmer <jason@rimmer.net>

* style: cargo fmt on clock sync refactor

Signed-off-by: jrimmer <jason@rimmer.net>

* fix(controller): run Registry::reconcile under a single lock

reconcile() took K+4 separate registry-mutex acquisitions for a pass
that prunes K stale sandboxes: one lock per removed id, plus one each
for the stale scan, the live-id snapshot, and the workspace walk. Two
handler calls interleaved between the scan and the per-id removal
could make the reported prune count diverge from what was actually
removed.

Collect the stale ids, remove them, and walk the workspaces for the
Running -> Stale transition all under one `self.inner.lock()`
acquisition. `pid_alive` is a `/proc` stat and takes no lock we care
about, so holding the registry mutex across it is fine. flush()
behavior is unchanged: it still runs once, after the lock is dropped,
only when something changed.

Adds coverage for reconcile()'s prune-count and workspace-transition
behavior (previously untested): a dead sandbox is pruned and its
Running workspace flips to Stale with live_sandbox_id cleared; a
sandbox with no recorded pid is left alone and its workspace stays
Running; a Suspended workspace is never touched; and a no-op pass
does not rewrite state.json.

Fixes deeplethe#310

Signed-off-by: Amir Fathi <amirfathi.me@gmail.com>

* fix(vmm): restore-path resilience for issue deeplethe#301 (squashed)

Squash of 10 commits on fix/restore-path-resilience for rebasing onto
current dev (which now contains deeplethe#281's shared-TAP lifecycle).

Original commits:
- 858f63b fix(vmm): restore-path resilience for issue deeplethe#301
- ce3d800 fix(vmm): apply review fixes for deeplethe#301
- 0d73362 fix(vmm): make DEFAULT_SOCKET_WAIT_SECS pub for cross-crate use
- e820848 fix(controller): use Vm::pid() accessor, not private field
- 1ddf40c fix(controller): drop needless borrow in workspace orphan scan call
- ca52820 fix(vmm): partial-success contract + concurrent socket wait for restore_many_with
- 9e863de fix(controller): expose orphan-firecracker detection as a metric
- 2aca840 test(vmm): integration test for partial-success restore
- 9342e7a test(vmm): accept Restore or SocketWait phase in partial-success test
- f2751b9 fix(vmm): review fixes for deeplethe#301 — indexed success, valid test topology, docs

Signed-off-by: jrimmer <jason@rimmer.net>

* style: cargo fmt on rebase resolution

Signed-off-by: jrimmer <jason@rimmer.net>

* fix(vmm): preserve original child slot in clock-sync outcomes

Review deeplethe#302: restore_many_with flattening children via
children::flatten() renumbered ClockSyncOutcome.child_index, so a
survivor past a failed hole (spawn/socket-wait) was reported as
0..n instead of its original 1-based within-batch slot, breaking
the resource correlation that ForkChild/RestoreFailure promise.

- sync_guest_clocks now takes &[(usize, &Vm)] where the usize is the
  original 1-based within-batch slot; outcomes echo that slot in both
  ClockSyncOutcome::Synced/Failed and the tracing child spans.
- Add live_children_with_indices(): the single index-preserving
  mapping (enumerate + filter_map, i+1) used by restore_many_with,
  extracted so the invariant is unit-testable without KVM.
- Regression: live_children_with_indices_preserves_survivor_slot_  across_hole proves [None,Some,None,Some] -> slots [2,4], not [0,1].
- Update the single-child clock-sync integration test to pass slot 1
  and assert child_index == 1.

Signed-off-by: jrimmer <jason@rimmer.net>

* style: fix clippy doc-lazy-continuation in sync docs

Map a line directly following the #Cons list was parsed as an
unindented list continuation (-D warnings / clippy::doc-lazy-continuation).
Separate it with a blank doc line.

Signed-off-by: jrimmer <jason@rimmer.net>

* fix(controller): kill orphaned Firecracker processes on startup

Issue deeplethe#298: when the controller crashes or restarts while Firecracker
processes are still alive, the restarted controller cannot reconstruct
resource ownership for orphaned VMs. Both the NetnsAllocator (active
set) and shared_tap_owner are initialized empty, so a new spawn can
reserve an orphaned VM's netns index or claim the shared tap even
though the orphan still owns it.

Fix: add `Registry::kill_orphans()` called after `reconcile()` on
startup. After reconcile() prunes dead-PID entries, any remaining
sandbox entries have alive PIDs but no live_vms handle — they are
unmanageable orphans. kill_orphans() kills each orphan (after
verifying the PID belongs to a Firecracker process via /proc/<pid>/comm
to guard against PID reuse), prunes the registry entry, and marks
workspaces whose sandbox was killed as Stale.

After this, the NetnsAllocator (empty active set) and shared_tap_owner
(None) are safe: no orphaned VM holds a netns index or the shared tap.

Closes deeplethe#298

Signed-off-by: jrimmer <jason@rimmer.net>

* fix(controller): address ce review findings for orphan recovery

Address all P0 and P1 findings from the 7-reviewer ce-* panel:

P0: kill_pid failure no longer prunes the registry entry. ESRCH (process
already dead from TOCTOU race) is distinguished from real errors (EPERM
etc.). On ESRCH, the entry is safely pruned. On real errors, the entry
is kept in the registry to prevent the exact resource collision issue

P1 fixes:
- Wait for process death after SIGKILL (5s bounded timeout polling
  /proc/<pid> disappearance). A D-state process stuck on I/O can hold
  netns/tap resources past the kill return; if it doesn't exit within
  the timeout, the registry entry is kept.
- pid_is_firecracker uses exact match (== "firecracker") instead of
  substring match to avoid false-positive kills of unrelated processes.
- Separate KillOrphansResult struct (killed, pruned_stale, kill_failed)
  replaces the misleading single usize counter. The startup log now
  reports all three counts.
- Extract mark_stale_workspaces() helper to eliminate the ~22-line
  duplicated workspace stale-marking block between reconcile() and
  kill_orphans().
- kill_orphans is pub(crate) with a doc invariant: "MUST only be called
  before any Vm handle is held" — prevents future post-startup misuse.
- Defensive pid <= 1 guard in pid_is_firecracker (defense-in-depth
  against corrupted state.json).
- ESRCH downgraded from error! to debug! (benign TOCTOU race).
- SAFETY comment added on unsafe libc::kill.

New tests: pid:None entries skipped, multiple workspaces same sandbox,
persistence to disk after kill_orphans, reconcile+kill_orphans integration.

Signed-off-by: jrimmer <jason@rimmer.net>

* fix(controller): abort startup on kill_failed, document PID-reuse limit

WaylandYang round 4 found that kill_orphans correctly keeps registry
entries on EPERM/timeout (kill_failed > 0), but run_daemon only logs
the count and continues. The NetnsAllocator active set and
shared_tap_owner start empty, so the next spawn can reuse the failed
orphan's netns index or tap lease — the exact collision deeplethe#298 prevents.

Fix: run_daemon now aborts startup when kill_failed > 0, forcing
operator intervention rather than silently admitting conflicting
spawns. The error message names the count and tells the operator to
kill the orphans manually and restart.

Also updated the pid_is_firecracker doc comment to be honest about the
PID-reuse limitation: the comm check is a defense-in-depth heuristic,
not complete PID-reuse protection. A robust fix would use pidfd
signaling plus durable identity validation (process start time recorded
with the registry entry at spawn time, compared on startup). That is
tracked as a follow-up.

Added test: kill_orphans_result_kill_failed_contract verifies the
KillOrphansResult contract (kill_failed == 0 on empty registry, so
baseline startup does NOT abort).

Signed-off-by: jrimmer <jason@rimmer.net>

* fix(controller): extract check_orphan_kill_result for testable abort decision

The startup abort logic was inline in run_daemon, making it
impossible to unit-test the kill_failed > 0 → bail decision path.
Extracted into pub(crate) check_orphan_kill_result(&KillOrphansResult)
-> Result<()> so the abort contract is directly testable.

Tests:
- check_orphan_kill_result_aborts_on_kill_failure: verifies that
  kill_failed=1 returns Err with "aborting startup" message, and
  kill_failed=0 returns Ok even with killed/pruned entries.
- check_orphan_kill_result_error_contains_count: verifies the error
  message contains the exact kill_failed count so operators know
  how many orphans need manual intervention.

The existing kill_orphans_result_kill_failed_contract test
(zero-baseline) is retained as documentation of the baseline
contract. The new tests exercise the actual abort decision logic.

Signed-off-by: jrimmer <jason@rimmer.net>

* fix(controller): remove orphaned last_err assignment after rebase

The dead-last_err cleanup dropped the declaration and the unreachable
expect, but the `last_err = Some(e)` assignment (guarded by the
tap/cgroup-busy warn block on main) survived the rebase and referenced a
now-missing binding. Remove it — the loop already returns on the final
attempt, so the assignment is dead.

Signed-off-by: jrimmer <jason@rimmer.net>

* fix(controller): durable process identity for orphan recovery

Replace the comm=="firecracker" check with durable process identity
verification and pidfd signaling, addressing review r6 (WaylandYang)
and the ce-code-review security findings.

The comm check was spoofable (prctl PR_SET_NAME) and could not
distinguish two Firecracker processes sharing a PID over time — a
same-name PID-reuse attack would kill a legitimate new Firecracker,
and on a multi-tenant host a cross-tenant DoS was possible. The
TOCTOU window between the check and kill(2) was also acknowledged.

Changes:

1. SandboxInfo::proc_starttime (serde default) — persists the process
   start time (field 22 of /proc/<pid>/stat, clock ticks since boot)
   captured at VM registration. Backward-compatible: old state.json
   files without this field deserialize to None.

2. read_proc_starttime — robust parse of /proc/<pid>/stat that handles
   the parenthesized comm field (field 2, can contain spaces/parens) by
   splitting at the last ')' and indexing field 22 in the remainder.

3. process_identity_matches — compares the live start time against the
   recorded one. Returns Match / PidReuse / Dead / Unknown. PidReuse
   means the original process exited and the PID was recycled — prune
   the stale entry without killing. Unknown (no recorded start time,
   /proc unreadable, or off-Linux) fails closed.

4. pidfd_open + pidfd_send_kill — signal through a pidfd opened AFTER
   the identity check, closing the TOCTOU window: the fd is pinned to
   the verified process, so a PID reuse between check and signal cannot
   redirect the kill. Uses raw libc::syscall with SYS_pidfd_open /
   SYS_pidfd_send_signal (Linux 5.3+; libc 0.2.x exposes the syscall
   numbers but not named fns).

5. comm_is_firecracker — retained as a secondary confirmation after
   the primary start-time check passes, to catch corrupted state.json.
   Not a security boundary.

6. kill_orphans reworked: Match → pidfd kill + wait_for_death; PidReuse
   → prune stale (no kill); Dead → prune stale; Unknown → fail closed
   (keep entry, increment kill_failed so run_daemon aborts startup).

Tests updated to reflect the new fail-closed semantics:
- kill_orphans_prunes_alive_pid_entries now uses a mismatched start
  time (u64::MAX) to trigger PidReuse → prune, not the old None path.
- kill_orphans_fails_closed_on_unknown_identity (new): alive PID with
  proc_starttime: None → fail closed, entry kept, kill_failed=1.
- kill_orphans_prunes_same_name_pid_reuse_without_killing (new
  regression): the same-name Firecracker PID-reuse attack the reviewer
  flagged — a mismatched start time must prune without killing, even
  if comm were "firecracker".
- All other kill_orphans tests updated to use mismatched start times
  where they previously relied on the comm-check prune path.

Verified on Linux (rust 1.83, x86_64) via Docker: clippy clean for
forkd-controller; cargo test --all-features passes 104 unit + 8
integration tests, 0 failed.

Signed-off-by: jrimmer <jason@rimmer.net>

* fix(controller): close pidfd TOCTOU + ce-code-review r6 follow-ups

Address ce-code-review findings (security P1, testing P1, adversarial
P2/P3, plus the cross-reviewer coverage gaps) on the durable-identity
rework from 2e275fe.

1. Residual TOCTOU window (security P1, adversarial ADV-1): pidfd
   closes only the open→signal window, NOT the check→open window. The
   original could exit and the PID could be reused by another
   Firecracker between process_identity_matches (which reads
   /proc/<pid>/stat) and pidfd_open. Fix: after pidfd_open succeeds,
   re-read the live start time and compare to the recorded one. A
   mismatch means the PID was reused between the check and the open →
   prune without killing (PidReuse). The pidfd pins the process at
   open time, so /proc/<pid> reflects the pinned process. This closes
   the check→open window; the comm check is now a pure sanity check,
   not a load-bearing defense. Design comment corrected to not
   overstate the guarantee.

2. Off-Linux comment/behavior mismatch (adversarial ADV-2): the
   module comment claimed off-Linux "prunes without killing" but
   Unknown actually fails closed (keeps entry, increments kill_failed,
   run_daemon aborts). Fixed the comment to state the real behavior.

3. Cross-platform test guards (adversarial ADV-3): the PidReuse-
   asserting tests (mismatched starttime → pruned_stale) now carry
   #[cfg(target_os = "linux")] because off-Linux returns Unknown
   (kill_failed), not PidReuse. The cross-platform fail-closed test
   (proc_starttime: None → Unknown) stays ungated.

4. Match-arm coverage (testing P1, +reliability/security/adversarial):
   new kill_orphans_match_arm_kills_verified_child_via_pidfd spawns a
   real child, records its true start time (Match), and asserts the
   secondary comm_is_firecracker check fails closed for a
   non-firecracker process — proving the Match arm is entered and the
   comm gate works as defense-in-depth. (Killing a real Firecracker
   would require a Firecracker binary; the comm gate is the
   testable boundary here.)

5. start-time parser unit test (testing P2): read_proc_starttime now
   has a direct test (parses our own /proc/<pid>/stat, rejects pid<=1,
   returns None for nonexistent PID).

6. Backward-compat serde test (testing safe_auto): new
   proc_starttime_defaults_to_none_when_absent_in_old_state_json
   deserializes a hand-written pre-r6 state.json (no proc_starttime
   key) and asserts #[serde(default)] yields None — the contract that
   lets existing deployments upgrade without wiping state.

Verified on Linux (rust 1.83, x86_64) via Docker: clippy clean for
forkd-controller; cargo test --all-features passes 107 unit + 8
integration tests, 0 failed (3 new tests pass).

Signed-off-by: jrimmer <jason@rimmer.net>

* fix(controller): add SIGKILL success-path regression + correct misleading comments

Review r8 (2026-08-20) found that
kill_orphans_match_arm_kills_verified_child_via_pidfd did not do what
its name claimed: it spawned 'sleep' (comm='sleep'), so the secondary
comm_is_firecracker check failed closed and the irreversible success
path (pidfd_send_kill -> wait_for_death -> registry removal) was never
executed.

- Rename the existing test to
  kill_orphans_match_arm_comm_mismatch_fails_closed so its name and
  doc accurately describe the comm-gate fail-closed branch it covers.
- Add kill_orphans_match_arm_kills_verified_firecracker_via_pidfd,
  which spawns a disposable process whose /proc/<pid>/comm is literally
  'firecracker' (a copy of 'sleep' renamed to 'firecracker'), records
  its true start time, and asserts killed == 1, kill_failed == 0,
  pruned_stale == 0, and the registry entry is removed — exercising the
  full irreversible pidfd kill path.

Also correct two misleading comments flagged in the same review:
- SandboxInfo::proc_starttime claimed legacy None state may use
  comm-only verification; the implementation fails closed (keeps the
  entry, increments kill_failed) and never does comm-only kill.
- IdentityCheck::Unknown claimed the caller chooses between a
  comm-only kill or prune; kill_orphans keeps the entry and aborts
  startup via kill_failed.

Signed-off-by: jrimmer <jason@rimmer.net>

* style: cargo fmt on new firecracker-named kill test

Signed-off-by: jrimmer <jason@rimmer.net>

* fix(controller): persist boot id + fail closed on pid-less entries

Addresses review deeplethe#299 (both blockers) and rebases onto dev with deeplethe#312's
single-lock reconcile preserved.

Blocker 1 — cross-reboot false Match (PID+starttick collision):
  proc_starttime is ticks since boot, only unique within one host boot.
  The registry persists across reboots, so after a reboot an unrelated
  Firecracker can share both the numeric PID and the boot-relative start
  tick of a recorded entry, and a bare starttime check would SIGKILL the
  wrong process. Persist /proc/sys/kernel/random/boot_id at registration
  (SandboxInfo.boot_id) and gate the identity check on it:
    - different boot id -> PidReuse (prune WITHOUT signaling; the old
      process cannot still exist across a reboot)
    - missing recorded boot id (legacy state.json) -> Unknown (fail closed)
    - live boot id unreadable -> Unknown (fail closed)
  New read_boot_id() helper; recorded at all three production VM
  registration sites in http.rs.

Blocker 2 — pid: None entries recreated the deeplethe#298 collision risk:
  Such rows were skipped by both reconcile and kill_orphans, so startup
  succeeded with an empty allocator/shared-tap ownership while a live VM
  may still hold those resources. kill_orphans now counts retained rows
  without a PID into KillOrphansResult.unresolved, and
  check_orphan_kill_result aborts startup on unresolved > 0 (fail closed).

Rebase: preserved deeplethe#312's single-lock reconcile (whole in-memory pass under
one registry lock, including inline workspace-stale marking); kept
mark_stale_workspaces only for kill_orphans' use after pruning.

Tests:
  - kill_orphans_does_not_kill_across_boot_id_mismatch: matching PID +
    real start time but a DIFFERENT recorded boot id -> pruned without
    signaling (killed==0, pruned_stale==1).
  - kill_orphans_counts_pid_none_entries_as_unresolved (replaces
    kill_orphans_skips_pid_none_entries): pid:None rows surface as
    unresolved and block startup via check_orphan_kill_result.
  - check_orphan_kill_result_aborts_on_unresolved: startup-decision
    regression.
  - Existing kill-path tests set boot_id to the current boot id where
    they exercise the Match/PidReuse path; the legacy fail-closed test
    keeps boot_id: None.

Signed-off-by: jrimmer <jason@rimmer.net>

* fix(controller): resolve use-after-move in busy-retry warn (E0382)

The rebase onto post-deeplethe#302 dev merged both sides of the busy-retry block
textually but not semantically: in the Err(e) arm, last_err = Some(e)
(and the final_attempt variant Some(e.context(...))) both move e, then the
trailing tracing::warn!(error = %e, ...) borrowed the moved value. deeplethe#302 had
moved that warn into the else branch ahead of the assignment.

The arm already computes let msg = format!("{e:#}") for busy-classification,
so use error = %msg in the warn to avoid re-borrowing e after the moves.

* test(controller): make orphan-kill SIGKILL regression robust to CI orphan-reaping

kill_orphans_match_arm_kills_verified_firecracker_via_pidfd spawned its
firecracker-named 'sleep' via sh -c '<bin> 30 & echo $!' and relied on the
backgrounded process being reparented to init, so wait_for_death would see
/proc/<pid> disappear right after SIGKILL. On the GitHub Actions runner that
detached child is reaped almost immediately (diagnostic showed state=Z ppid=1
at t=0ms, dead by 5ms), so process_identity_matches returned Dead,
kill_orphans pruned it, and the test failed with pruned_stale=1 instead of
killed=1.

Spawn the process as a DIRECT child the test owns (the runner cannot reap an
owned, non-orphaned child), and reap it via a small reaper thread that calls
Child::wait(). wait_for_death (polling /proc/<pid>) then observes the SIGKILL
reaping and the Match arm reports killed=1 as intended.

* fix(controller): distinct legacy-row diagnostics + off-Linux gate skip (review deeplethe#299 r9)

Legacy state.json rows (no recorded boot identity via serde defaults)
took the generic Unknown path on first restart after upgrade and aborted
startup with an EPERM-style message describing neither cause nor fix.

- IdentityCheck::LegacyUnverifiable: missing recorded starttime OR boot
  id is reported distinctly from a genuinely unverifiable environment.
- KillOrphansResult.legacy_unverified counts legacy rows separately
  from kill_failed; run_daemon logs include the new counter.
- check_orphan_kill_result emits a dedicated upgrade diagnostic (kill
  the recorded PID manually or remove the entry once verified dead),
  distinct from the kill-failure and pid-less-row messages.
- Off-Linux, identity can never be verified, so the gate skipped any
  registry row and aborted dev-box startup; the abort decision is now
  extracted into a pure cross-platform orphan_abort_reason(), the Linux
  gate fails closed on it, and the off-Linux gate logs the counts and
  allows startup (review deeplethe#299 non-blocking).
- Corrected the wrong pidfd comment: /proc/<pid> reflects whoever
  currently holds the PID; the post-open starttime re-read is what
  makes the reuse check safe.
- kill_orphans folds its two entry lock acquisitions into one guard,
  matching the deeplethe#312 single-lock invariant.

Tests: legacy rows count as legacy_unverified (not kill_failed); abort
distinctness (legacy vs kill-failure diagnostics); Linux-gated gate
wiring test; existing abort-contract tests retargeted to the pure
decision function so they run on every platform.

* fix(controller): fail spawn loudly when boot id is unreadable at registration

read_boot_id() returning None at registration was recorded silently,
producing a sandbox row that can never pass startup identity
verification — bricking every subsequent controller start until
state.json was hand-edited (review deeplethe#299).

registration_boot_id() treats an unreadable boot id as a spawn-blocking
error. Both production registration paths call it BEFORE any VM work:
the /v1/fork handler returns a 500 up front, and
spawn_one_for_workspace bails before snapshot lookup. Off-Linux it
returns Ok(None) so dev-box spawns keep working.

* fix(controller): type alias for orphan row to satisfy clippy type_complexity

* fix(vmm): immutable baseline rootfs — reflink clone before boot (closes deeplethe#296)

The rootfs corruption issue (deeplethe#296) was caused by vm.kill() SIGKILLing
firecracker without a clean ext4 unmount, leaving the rootfs dirty.
The previous approach (PR deeplethe#295) ran e2fsck -fy before each boot to
repair the dirty journal — but the reviewer flagged a TOCTOU race:
the /proc/*/fd scan is point-in-time, so another VM could open the
rootfs during e2fsck.

The immutable-baseline approach eliminates the race entirely:

1. The original rootfs is the IMMUTABLE BASELINE — never mounted RW.
2. Before each boot, forkd creates a reflink copy (FICLONE ioctl,
   instant on btrfs/xfs/overlayfs; falls back to full copy on ext4/tmpfs)
   in the snapshot directory.
3. The VM boots from the clone and writes to it; the baseline stays clean.
4. After vm.kill(), the clone persists as the snapshot's rootfs (needed
   for restores — Firecracker re-opens the rootfs from the path in the
   vmstate).
5. The next forkd snapshot --rootfs <baseline> boots from a fresh clone
   of the still-clean baseline — no e2fsck needed, no TOCTOU race.

The reflink copy is exposed as pub fn reflink_copy in chain.rs (wraps
the existing copy_base_memory which already has FICLONE + stream
fallback). The snapshot_cmd function in forkd-cli creates the clone at
<snapshot_dir>/rootfs.ext4 and records its path in snap.rootfs.

Signed-off-by: jrimmer <jason@rimmer.net>

* fix(snapshot): cache versioning, atomic staging, portable rootfs transport

Review deeplethe#295 r6 (WaylandYang 2026-08-14): three correctness/portability
blockers on the immutable-baseline clone design. All three closed, plus
the four requested regression tests.

Blocker 1 — cache versioning (crates/forkd-cli/src/main.rs):
A cached rootfs produced or dirtied by an older forkd version was
reused solely because the path exists, then cloned as the supposedly
immutable baseline. Now each built rootfs gets a `.cache-meta.json`
sidecar recording schema_version + sha256 + image + size + forkd
version. `validate_cached_rootfs` trusts a cache entry only when the
meta exists, the schema version matches ROOTFS_CACHE_SCHEMA_VERSION
(=1), and the live sha256 still matches. Legacy entries (no meta),
schema mismatches, or sha mismatches (truncation/mutation) force a
rebuild. Wired into both from_image_cmd and run_cmd cache-hit paths;
write_rootfs_cache_meta is called after every build.

Blocker 2 — atomic snapshot staging (snapshot_cmd + publish_snapshot):
snapshot_cmd removed snap_dir/rootfs.ext4 BEFORE clone/boot/snapshot
succeeded, so re-running a tag destroyed the last usable snapshot and
(src==dst) could unlink its own source. The entire new snapshot
(rootfs clone + vmstate + memory.bin + snapshot.json) is now built
under a distinct staging dir (<snap_dir>.staging-<pid>) and only
published via publish_snapshot() after boot + warmup + snapshot +
metadata write all succeed. publish_snapshot does a safe two-step
shuffle: move the old snap_dir aside, rename staging into place (the
commit point), then drop the old. On commit-point failure the old
snapshot is restored from the aside, so a crash at any point leaves
either the new OR the old snapshot, never neither. A src==dst guard
rejects cloning the baseline into the snapshot's own rootfs.ext4 path.

Blocker 3 — portable rootfs transport (crates/forkd-cli/src/hub.rs):
The rootfs was shipped TWICE — tarred into the pack via SNAPSHOT_FILES
AND emitted as a content-addressed .rootfs.zst sidecar — duplicating a
potentially huge image. pack() now records rootfs.ext4 in the manifest
files list (for integrity accounting + list_local) but skips appending
it to the tar body when a portable sidecar is emitted, so there is ONE
rootfs transport. RootfsRef.target_path is now a PORTABLE relative
filename (e.g. "rootfs.ext4") instead of the packing host's absolute
path; satisfy_rootfs resolves it against the destination snapshot dir
(absolute paths from legacy packs still work). unpack_into now
returns the dest snapshot dir so the relative target_path can be
resolved; unpack_chain_into returns the head link's dest.

Tests (crates/forkd-cli/src/main.rs):
- validate_cached_rootfs_rejects_legacy_entry_without_meta (upgrade/dirty-cache)
- validate_cached_rootfs_rejects_wrong_schema_version (upgrade/dirty-cache)
- validate_cached_rootfs_rejects_sha_mismatch_after_mutation (dirty-cache)
- validate_cached_rootfs_accepts_fresh_valid_entry
- validate_cached_rootfs_misses_on_missing_file
- publish_snapshot_atomically_replaces_existing (same-tag failure)
- publish_snapshot_into_nonexistent_snap_dir
- publish_snapshot_preserves_existing_when_staging_missing (same-tag failure recovery)

Signed-off-by: jrimmer <jason@rimmer.net>

* style: cargo fmt + fix stray brace in run_cmd cache validation

Rebase resolution left a stray closing brace after the
validate_cached_rootfs match block in run_cmd (the original
if/else's trailing brace was not removed). cargo fmt the rebased
diff.

Signed-off-by: jrimmer <jason@rimmer.net>

* fix(cli): qualify sha256_file calls with hub:: prefix

write_rootfs_cache_meta and validate_cached_rootfs called sha256_file
without the hub:: prefix; the function lives in forkd-cli::hub, not
main.rs. This was a compile error (E0425 cannot find function
sha256_file) on Linux clippy CI.

Signed-off-by: jrimmer <jason@rimmer.net>

* fix(cli): clippy redundant closure in destination fallback

unwrap_or_else(|| std::path::PathBuf::new()) -> unwrap_or_else(std::path::PathBuf::new)

Signed-off-by: jrimmer <jason@rimmer.net>

* fix(snapshot): 4 blockers from review deeplethe#295 (2026-08-22)

Rebased onto dev (d2d238a, incl. deeplethe#312). Addresses all four review
blocks on the staging/transport flow while keeping cache-versioning and
failure-rollback.

Blocker 1 — FC-visible rootfs path unstable across publish:
Firecracker serializes the drive path_on_host INTO the binary vmstate
and REOPENS it on restore (no PUT /drives override is accepted before
/snapshot/load). Previously snapshot_cmd cloned rootfs to
staging_dir/rootfs.ext4 and booted the VM from it, then publish_snapshot
renamed the whole dir to snap_dir — leaving the vmstate's recorded path
nonexistent, so the first restore after publish failed. Fix: clone+boot
from the STABLE final path snap_dir/rootfs.ext4 from the start; only
vmstate/memory/snapshot.json are staged. publish_snapshot is replaced by
publish_snapshot_metadata, which renames the 3 metadata files into
snap_dir (snapshot.json LAST = commit marker) and never moves the rootfs.
Tests: metadata-replace-keeps-rootfs, into-nonexistent-snap_dir,
preserves-existing-on-missing-metadata.

Blocker 2 — pack/unpack sidecar never placable:
pack listed rootfs.ext4 in manifest.files but omitted it from the tar
body; unpack verified EVERY declared file before satisfy_rootfs, hashing
the missing extracted rootfs and failing. Fix: when a portable sidecar
is emitted, retain rootfs.ext4 out of manifest.files (sidecar carries
sha integrity). Test: pack_unpack_roundtrip_with_sidecar_rootfs.

Blocker 3 — src==dst guard compared staging path, not final:
extracted to rootfs_clone_into_self(src, snap_dir) comparing canonical
against the FINAL snap_dir/rootfs.ext4, so re-snapshotting a tag whose
baseline is its own rootfs.ext4 is rejected. Test: same-tag regression.

Blocker 4 — path traversal via RootfsRef.target_path:
satisfy_rootfs now REQUIRES a safe single-component relative filename
(no absolute / no separators / no ..), rejecting malicious ../../../ or
legacy absolute paths (fail-closed; writes would otherwise land outside
snap_dir under sudo). Test: satisfy_rootfs_rejects_unsafe_target_paths.

Signed-off-by: jrimmer <jason@rimmer.net>

* fix(snapshot): re-bake rollback + validated absolute rootfs placement (deeplethe#295 r9)

Addresses the 2026-08-22T20:28 review round on PR deeplethe#295.

Blocker 1 — a failed re-bake destroyed the previously published tag:
the old rootfs was remove_file'd before the VM booted, so any later
failure (boot timeout, snapshot error, publish error, interrupt) left
old metadata frozen against a replaced, dirtied rootfs (deeplethe#296 EBADMSG).
The previous rootfs is now renamed aside to <tag>.rootfs.ext4.prev-<pid>
and a RootfsRollback guard restores it on every failure path (Drop =
rollback, covers ?-propagation and panic unwinding); the backup is
deleted only after publish_snapshot_metadata commits the new snapshot.

Blocker 2 — snap_dir-relative target_path broke cross-host restore:
Firecracker freezes the drive path into the vmstate and reopens it
verbatim at /snapshot/load, so placement must not depend on the
destination snap_dir. manifest.rootfs.target_path records the absolute
path again (un-breaking published packs whose absolute target_path the
r6 code rejected outright), and satisfy_rootfs validates it against
forkd-managed roots — the forkd data dir, its snapshots/ tree, and the
rootfs cache (, default /var/cache/forkd) — with
traversal/backslash rejection and existing-prefix canonicalization
(symlink escape hardening) before any privileged write. A missing or
unfetchable sidecar is now a hard error: a warn-and-continue produced a
snapshot that could never restore.

Nits: duplicated src==dst comment line removed; unpack_chain_into tail
bails instead of returning an empty PathBuf that would resolve
target_path against the CWD under sudo.

Tests: RootfsRollback armed/disarmed unit test; satisfy_rootfs
rejection matrix updated for the absolute-path contract plus placement
and sha-dedup assertions; cross-snap-dir verbatim placement round-trip
(reviewer's blocker-2 regression ask); validate_rootfs_target_path unit
contract; the #[ignore] KVM regression extended with a failed-re-bake
rollback and a cross-tag sidecar placement scenario (FORKD_TEST_*).

Signed-off-by: jrimmer <jason@rimmer.net>

* fix(snapshot): clippy needless-question-mark + restore opts rebuild in KVM test

Signed-off-by: jrimmer <jason@rimmer.net>

* test(snapshot): rollback restores content at the published path, not absence

Signed-off-by: jrimmer <jason@rimmer.net>

* test(snapshot): surface per-child restore failures instead of swallowing them

Signed-off-by: jrimmer <jason@rimmer.net>

* fix(snapshot): repoint staged snapshot.json at final paths on publish

The staging-flow snapshot.json was serialized with staging-dir vmstate/
memory paths, and publish_snapshot_metadata renamed the files without
fixing up the paths inside — so every staged snapshot published since
the atomic-staging change recorded a vmstate path in a directory that
is deleted right after publish. Restore then fails with 'Failed to
open snapshot file' (caught by the first real-KVM run of the
snapshot_stable_rootfs_publishes_and_restores regression on a KVM
host; CI could never see it because the test is #[ignore]-gated).

publish_snapshot_metadata now rewrites the staged snapshot.json's
vmstate/memory keys to the snap_dir paths BEFORE installing it (still
snapshot.json-last, so the commit marker is never a half-fixed file).
This is safe precisely because vmstate/mem are /snapshot/load request
arguments, unlike the drive path frozen into the binary vmstate.

Publish unit tests now stage real JSON (as snapshot_cmd does) and
assert the repoint; the KVM regression asserts per-child restore
failures are empty so a dead child can no longer hide behind an Ok.

Signed-off-by: jrimmer <jason@rimmer.net>

* fix(cli): sweep every rootfs .prev backup on re-bake

A re-bake parks the previous published rootfs as
`<tag>.rootfs.ext4.prev-<pid>`. The pid belongs to the run that parked
it, so the same-pid check could only ever clean up after a run whose
pid happened to match. A `kill -9` between the preserve-rename and the
publish stranded the file under a pid no later run would look for —
several GB that nothing collected — while the tag was left with no
`rootfs.ext4` until someone renamed the backup back by hand.

Sweep every `<tag>.rootfs.ext4.prev-*` sibling instead. When the
published rootfs is missing, the newest backup is renamed back into
place; the remaining ones are scratch from runs that never published
and are dropped best-effort — a stale file that cannot be removed no
longer aborts the bake, since the preserve-rename would overwrite it
anyway.

Also corrects the `satisfy_rootfs` doc comment, which still described
the missing-sidecar case as a warning when it is now a `bail!`.

Both are follow-ups from the deeplethe#295 approval.

Signed-off-by: jrimmer <jason@rimmer.net>

* fix(vmm): reap the Firecracker child on every failed boot

`Vm::boot` spawned Firecracker and then drove `/boot-source`,
`/drives/rootfs`, `/entropy` and `/actions` with a bare
`std::process::Child` in hand. `Child` has no kill-on-drop, so any `?`
in that window — `wait_for_sock` timing out, or an API call failing
after the process was up — returned an error and abandoned a *live*
Firecracker.

The orphan keeps the sandbox's tap device and rootfs fd open, and the
tap name is frozen into the vmstate, so every later spawn of that
snapshot dies inside Firecracker with `Open tap device failed` before
it can create its API socket. Callers see
`socket .../child-N.sock never appeared` and the snapshot stays
unusable until the orphan is killed by hand — the permanent-failure
mode in deeplethe#301.

Own the child in a `PendingFirecracker` guard that reaps it on every
early return and hands it to the `Vm` only on success. `restore_many_with`
already wrapped its children immediately, so `boot` was the only window
in the crate that could orphan a process with no handle left.

Two unit tests cover the drop path (the pid must be gone, not just
untracked) and the take path (the handed-over child must still be live).

Signed-off-by: jrimmer <jason@rimmer.net>

* fix(cli): gate bakes on free space, and always clean up the staging dir

Both halves of one failure mode: a bake that runs out of space does not
stop, it produces a *corrupt* artifact — a rootfs whose package files
carry other files' bytes, or a truncated `memory.bin`. Inside a guest
that surfaces much later as `uname: option requires an argument`,
`Exec format error`, or `EBADMSG` on a `/var/lib/dpkg` entry, so it
reads like a broken build rather than a broken image.

- `forkd parent build` (conversion) and `forkd snapshot` (bake) check
  free space on the target filesystem before the first write and refuse
  below a 5 GiB reserve (`FORKD_MIN_FREE_GIB` overrides). Conversion is
  the first multi-GB write of the pipeline, and the snapshot dir holds
  the clone plus a fresh `memory.bin`. The probe stays advisory: if
  `statvfs` cannot run, warn and continue rather than block work on a
  broken measurement. `doctor::available_bytes(path)` is extracted from
  the check's hardcoded-path statvfs so both callers share one probe.

- `forkd snapshot` removes its staging dir on every exit. Only the
  success path did, so any `?` after the volatile artifacts were written
  — boot timeout, snapshot error, publish error, interrupt — left a
  fully written `memory.bin` (GBs) beside the snapshot dir that nothing
  ever collected.

Tests cover the guard (removal, and tolerance of an already-removed dir —
the success path renames files out and a stale sweep may beat it) and the
probe's ancestor walk, which is what makes it usable before the snapshot
dir exists.

Signed-off-by: jrimmer <jason@rimmer.net>

* test(cli): tolerate free-space drift between the two statvfs probes

Parallel tests write to the same filesystem between the two calls, so
exact equality flakes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(cli): restore the .prev backup unless the tag was republished

A rootfs.ext4 at the published path is not evidence the tag is whole:
the interrupted run clones its fresh rootfs there right after parking
the backup and then boots and writes to it for the whole warmup. The
sweep discarded the backup in that case, leaving the old metadata
frozen against a dirtied, unpublished clone (deeplethe#296) with the last good
rootfs deleted.

Decide by the publish commit marker instead: restore the newest backup
unless snapshot.json is newer than the backup's park ctime. Backups
whose pid is still running a bake are left alone.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(vmm): use the real FICLONE ioctl number

0x40209409 encodes a 32-byte payload; FICLONE is _IOW(0x94, 9, int) =
0x40049409. The kernel answered ENOTTY, which the fallback reads as no
reflink, so every reflink_copy streamed a full copy on every
filesystem. Reported by @jrimmer in deeplethe#321.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* style: rustfmt the FICLONE derivation test

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* style(cli): name the backup tuple for clippy::type_complexity

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(vmm): compare FICLONE without a cast

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(release): build the release binaries against jammy (glibc 2.35) — ubuntu-latest's glibc 2.39 binary dies on the sandbox host VMs (#5)

LIVE refusal (urun dev-usw2 2026-10-15, cpu-sbx node boot, stage
forkd-daemon): the freshly-installed forkd-controller crash-looped with
  /usr/local/forkd/forkd-controller: /lib/x86_64-linux-gnu/libc.so.6:
  version GLIBC_2.38 not found (required by forkd-controller)
The release workflow builds on ubuntu-latest (24.04, glibc 2.39) while
the sandbox host VM image is Ubuntu 22.04 (glibc 2.35) — a binary built
on newer glibc can never run there.

The binaries job now builds inside an ubuntu:22.04 container (all-TLS
via rustls; no openssl native deps), keeping every other step identical.
The docker image job is untouched (it runs the binary inside its own
container image).

---------

Signed-off-by: jrimmer <jrimmer@users.noreply.github.com>
Signed-off-by: jrimmer <jason@rimmer.net>
Signed-off-by: Amir Fathi <amirfathi.me@gmail.com>
Co-authored-by: jrimmer <jason@rimmer.net>
Co-authored-by: WaylandYang <145302500+WaylandYang@users.noreply.github.com>
Co-authored-by: Amir Fathi <amirfathi.me@gmail.com>
Co-authored-by: WaylandYang <wayland0916@gmail.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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.

2 participants