microvm: serve durable-dir volumes through the one kataShared virtiofsd - #1034
microvm: serve durable-dir volumes through the one kataShared virtiofsd#1034Lucky Abolorunke (Oneimu) wants to merge 1 commit into
Conversation
255fa83 to
7bf72d1
Compare
|
/assign Benjamin Elder (@BenTheElder) |
| // three — per actor, forever. A bind costs one mount, and teardown already | ||
| // covers it: CleanupSandboxState lazily detaches every mount under the sandbox | ||
| // dir before removing it, so the source directory (which may belong to atelet, | ||
| // as the durable-dir volumes do) is never touched. Callers must stage binds |
There was a problem hiding this comment.
🤖 should-fix 🟡 – CleanupSandboxState doesn't enforce this, so a failed detach deletes the actor's durable data.
It logs each unix.Unmount failure and continues, then runs os.RemoveAll(d) unconditionally. Worse, the whole unmount loop sits inside if b, err := os.ReadFile("/proc/self/mountinfo"); err == nil with no else — if that read fails, nothing is unmounted and the RemoveAll still runs, silently.
Either way RemoveAll walks into a live _durable bind and deletes through it, and what it deletes is ateompath.DurableDirVolumeMountsDir — the data durable volumes exist to preserve. Nothing under SharedDir had that property before: the merged rootfs overlays would take their whiteouts to an ateom-owned upper that gets wiped anyway, and their lower is reconstructible from the image.
Both triggers are unlikely, which is why I'd not block on it — but the guarantee is asserted here and in durable.go ("nothing of atelet's is ever deleted through it") rather than enforced. Skipping the RemoveAll for a directory whose unmounts didn't all succeed turns silent data loss into a leaked directory, which is the right trade.
There was a problem hiding this comment.
Fixed — removal is now gated on the unmounts: an unmount failure (or unreadable mountinfo) leaves the dir in place instead of RemoveAll through a live bind. The stagers tolerate leftover dirs, so the degraded case is a retry, not data loss.
| // kataShared tree — so teardown has nothing extra to unmount. Unlike the RO | ||
| // lower's virtiofsd this one runs with cache=auto: the host contents change | ||
| // underneath the guest whenever a snapshot is restored into them. | ||
| // stageLegacyDurableShare starts the second virtiofsd that snapshots from the |
There was a problem hiding this comment.
🤖 question 🟢 – Is this worth carrying? We don't support in-place upgrade, and the position on snapshot compatibility elsewhere this week has been that we don't care about it yet.
Unlike a stub, this one genuinely works — you've kept the device detection, the second daemon, its teardown, and the tests, and you verified a legacy restore end to end. That's the cost: a branch through restoreFullScope, a legacyDurable return threaded out of rewriteSnapshotSocketPaths, DurableFsTag/DurableVirtiofsdSocketPath/durableVirtiofsdLogPath kept alive, and a lineage that stays permanently two-share once restored.
If pre-change snapshots can just be discarded, dropping all of it lets rewriteSnapshotSocketPaths keep its single return value and lets default: reject ateDurable the way it already rejects the retired ateUpper tag.
There was a problem hiding this comment.
Human: Yes, don't try to maintain compatibility yet. Just clean up the legacy code.
There was a problem hiding this comment.
Done: dropped entirely, and the PR is re-scoped after #840 landed (which already folded durable/CSI into the single share).
Hoist the durable-dir and CSI volume staging (two inlined copies of the same umount/mkdir/bind sequence) into kata.BindIntoShare, the one pattern for exposing a host directory through the single kataShared share. Drop the dead restore compat for the retired ateDurable/ateCSI fs devices — nothing spawns those virtiofsds anymore, so repointing their sockets only deferred the failure to cloud-hypervisor's vhost connect — and gate CleanupSandboxState's dir removal on every mount beneath being detached, so RemoveAll can never delete the actor's volume data through a live bind.
7bf72d1 to
ba12245
Compare
2 participants
What Changed and Why
Durable-dir volumes were previously served to the guest by a second per-actor
virtiofsd. That arrangement predates the writablekataSharedshare: when the rootfs share was a read-only lower, a writable durable share had to be its own device. Since #846, thekataSharedtree is writable and served with--announce-submounts, making the second daemon redundant—it cost an extra process andvhostsocket per actor, an extrafsdevice in every snapshot config, and a restore-time revival of all three.This PR folds the durable-dir volumes into the single existing share:
_durable):kata.BindIntoSharebind-mounts theatelet-owned volumes directory into the served tree as its_durablesubtree. The leading underscore keeps it out of the container-ID namespace (container names are RFC 1123 labels and cannot begin with an underscore). The guest sees it as a submount of thekataSharedmount, and containers bind their volumes from<shared>/_durable/<volume>exactly as they previously did from the second share.Cold boot no longer spawns the durable
virtiofsd, and the VM configuration carries exactly onevirtio-fsdevice.ateletstill owns the directory (creates it before boot, wipes it on actor reset). The bind isateom-owned mount state, detached byCleanupSandboxStatebefore any removal, ensuringatelet's data is never touched through it.Checkpoints tar the host directory directly under every scope, exactly as before.
Note
The bind uses the same mechanism the per-container merged rootfs mounts already use through this
virtiofsd(submounts inside the served tree, re-opened byfind-pathson restore), introducing no new guest-side behavior.BindIntoShare's doc comment establishes this pattern for future per-actor shares: mount a reserved subtree rather than adding a device (relevant to in-flight work such as #803; #923 already follows this subtree approach).Compatibility with Existing Snapshots
Restore is self-describing in both directions:
A snapshot whose
config.jsoncarries anateDurablefs device originates from the two-share era. The resumed guest still expects that device, so restore revives the secondvirtiofsdexactly as before (stageLegacyDurableShare). Such a lineage remains two-share across its own re-checkpoints sincecloud-hypervisorre-emits the device.A snapshot without the device gets the volumes re-bound into the shared tree before
virtiofsdstarts, allowingfind-pathsto re-open the guest's open durable files at their_durable/...paths, which the restored tar reproduces exactly.Detection logic lives in
rewriteSnapshotSocketPaths, which already inspects the configuration'sfsdevices. The configuration acts as the authority because the device is whatcloud-hypervisorre-opens, regardless of the actor's spec.How This Was Tested
rewriteSnapshotSocketPathsclassifies single-device and two-device snapshot configs correctly (routing restore to the bind vs. the legacy share).kata-packagetest pinning_durablesubtree invariants: the underscore namespace reservation, the guest path residing inside the singlekataSharedmount, and host/guest agreement on the relative path re-opened byfind-paths.mainbefore this change to exercise the legacy two-share fallback path.