microvm: serve durable-dir volumes through the one kataShared virtiofsd

cmd/ateom-microvm/csi.go

@@ -20,12 +20,8 @@ import (
 	"context"
 	"fmt"
 	"os"
-	"os/exec"
-	"path/filepath"
-	"strings"
 
 	"github.com/agent-substrate/substrate/cmd/ateom-microvm/internal/kata"
-	"github.com/agent-substrate/substrate/cmd/ateom-microvm/internal/reaper"
 	"github.com/agent-substrate/substrate/internal/ateompath"
 	"github.com/agent-substrate/substrate/internal/proto/ateompb"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
@@ -64,19 +60,8 @@ func (s *AteomService) stageCsiVolumes(ctx context.Context, actorUID string) err
 	if _, err := os.Stat(src); err != nil {
 		return fmt.Errorf("while checking CSI volumes dir %q: %w", src, err)
 	}
-	dst := filepath.Join(kata.SharedDir(actorUID), "csi")
-	// Drop any stale mount first (lazy if busy), then ensure clean mountpoint.
-	if err := reaper.Run(exec.Command("umount", dst)); err != nil {
-		_ = reaper.Run(exec.Command("umount", "-l", dst))
-	}
-	if err := os.MkdirAll(dst, 0o755); err != nil {
-		return fmt.Errorf("creating %q: %w", dst, err)
-	}
-	cmd := exec.CommandContext(ctx, "mount", "--bind", src, dst)
-	var stderr strings.Builder
-	cmd.Stderr = &stderr
-	if err := reaper.Run(cmd); err != nil {
-		return fmt.Errorf("bind-mounting CSI volumes at %q: %w (%s)", dst, err, strings.TrimSpace(stderr.String()))
+	if err := kata.BindIntoShare(ctx, src, actorUID, "csi"); err != nil {
+		return fmt.Errorf("while binding CSI volumes into the shared tree: %w", err)
 	}
 	return nil
 }

cmd/ateom-microvm/durable.go

@@ -41,12 +41,9 @@ import (
 	"context"
 	"fmt"
 	"os"
-	"os/exec"
 	"path/filepath"
-	"strings"
 
 	"github.com/agent-substrate/substrate/cmd/ateom-microvm/internal/kata"
-	"github.com/agent-substrate/substrate/cmd/ateom-microvm/internal/reaper"
 	"github.com/agent-substrate/substrate/cmd/ateom-microvm/internal/tarutil"
 	"github.com/agent-substrate/substrate/internal/ateompath"
 	"github.com/agent-substrate/substrate/internal/proto/ateompb"
@@ -112,19 +109,8 @@ func (s *AteomService) stageDurableVolumes(ctx context.Context, actorUID string)
 	if _, err := os.Stat(src); err != nil {
 		return fmt.Errorf("while checking durable-dir volumes dir %q: %w", src, err)
 	}
-	dst := filepath.Join(kata.SharedDir(actorUID), "durable")
-	// Drop any stale mount first (lazy if busy), then ensure clean mountpoint.
-	if err := reaper.Run(exec.Command("umount", dst)); err != nil {
-		_ = reaper.Run(exec.Command("umount", "-l", dst))
-	}
-	if err := os.MkdirAll(dst, 0o755); err != nil {
-		return fmt.Errorf("creating %q: %w", dst, err)
-	}
-	cmd := exec.CommandContext(ctx, "mount", "--bind", src, dst)
-	var stderr strings.Builder
-	cmd.Stderr = &stderr
-	if err := reaper.Run(cmd); err != nil {
-		return fmt.Errorf("bind-mounting durable-dir volumes at %q: %w (%s)", dst, err, strings.TrimSpace(stderr.String()))
+	if err := kata.BindIntoShare(ctx, src, actorUID, "durable"); err != nil {
+		return fmt.Errorf("while binding durable-dir volumes into the shared tree: %w", err)
 	}
 	return nil
 }

cmd/ateom-microvm/internal/kata/cleanup_linux.go

@@ -37,13 +37,31 @@ import (
 // .../virtiofsd.sock: bind: address already in use", "Could not bind mount
 // .../shared/sandboxes/<id>/mounts", "directory not empty". Calling this
 // before each run gives a clean slate.
+//
+// Removal is gated on the unmounting: the shared tree holds bind mounts whose
+// SOURCES belong to someone else (the durable-dir and CSI volume dirs are
+// atelet's — see BindIntoShare), and a RemoveAll that walks into a live bind
+// deletes the actor's data through it. So a dir is removed only once every
+// mount beneath it is known detached; otherwise it is left in place for the
+// next sweep (the stagers tolerate a leftover dir — they unmount stale binds
+// and reuse mountpoints).
 func CleanupSandboxState(ctx context.Context, id string) {
 	dirs := []string{
 		filepath.Join("/run/kata-containers/shared/sandboxes", id),
 		filepath.Join(vcVMDir, id),
 	}
-	if b, err := os.ReadFile("/proc/self/mountinfo"); err == nil {
-		var mounts []string
+	removable := make(map[string]bool, len(dirs))
+	if b, err := os.ReadFile("/proc/self/mountinfo"); err != nil {
+		// Without the mount table there is no telling what is still mounted
+		// under the dirs, so none of them is provably safe to remove.
+		slog.WarnContext(ctx, "Cannot read mountinfo; leaving sandbox dirs in place",
+			slog.Any("err", err))
+	} else {
+		for _, d := range dirs {
+			removable[d] = true
+		}
+		type mount struct{ mp, dir string }
+		var mounts []mount
 		for _, line := range strings.Split(string(b), "\n") {
 			fields := strings.Fields(line)
 			if len(fields) < 5 {
@@ -52,21 +70,27 @@ func CleanupSandboxState(ctx context.Context, id string) {
 			mp := fields[4] // mount point
 			for _, d := range dirs {
 				if mp == d || strings.HasPrefix(mp, d+"/") {
-					mounts = append(mounts, mp)
+					mounts = append(mounts, mount{mp: mp, dir: d})
 					break
 				}
 			}
 		}
 		// Deepest paths first so child mounts unmount before their parents.
-		sort.Slice(mounts, func(i, j int) bool { return len(mounts[i]) > len(mounts[j]) })
-		for _, mp := range mounts {
-			if err := unix.Unmount(mp, unix.MNT_DETACH); err != nil {
+		sort.Slice(mounts, func(i, j int) bool { return len(mounts[i].mp) > len(mounts[j].mp) })
+		for _, m := range mounts {
+			if err := unix.Unmount(m.mp, unix.MNT_DETACH); err != nil {
 				slog.WarnContext(ctx, "Failed to unmount leftover sandbox mount",
-					slog.String("mount", mp), slog.Any("err", err))
+					slog.String("mount", m.mp), slog.Any("err", err))
+				removable[m.dir] = false
 			}
 		}
 	}
 	for _, d := range dirs {
+		if !removable[d] {
+			slog.WarnContext(ctx, "Leaving sandbox dir in place: mounts under it were not all detached",
+				slog.String("dir", d))
+			continue
+		}
 		if err := os.RemoveAll(d); err != nil {
 			slog.WarnContext(ctx, "Failed to remove leftover sandbox dir",
 				slog.String("dir", d), slog.Any("err", err))

cmd/ateom-microvm/internal/kata/overlay_linux.go

@@ -241,6 +241,45 @@ func UnmountMergedRootfs(restoreID, cid string) {
 	}
 }
 
+// BindIntoShare bind-mounts a host directory at SharedDir(id)/<name>, so the
+// ONE virtiofsd serves it to the guest as a subtree of the kataShared mount
+// (--announce-submounts presents it to the guest as its own filesystem).
+//
+// This is THE pattern for exposing another host directory to the guest — the
+// durable-dir and CSI volumes ride it today: a separate share would otherwise
+// pay for its own virtiofsd (a process, a vhost socket, an fs device in every
+// snapshot config and a restore-time revival of all three) per actor, forever.
+// A bind costs one mount, and teardown already covers it: CleanupSandboxState
+// lazily detaches every mount under the sandbox dir first, and will not remove
+// a dir whose mounts it could not all detach, so the source directory (which
+// may belong to atelet, as the volume dirs do) is never deleted through a live
+// bind. Callers must stage binds before StartVirtiofsd, both to keep
+// the served tree complete from the first request and because find-paths
+// migration re-opens a restored guest's open files by path at reconnect.
+//
+// name sits beside the per-container <cid>/... entries of the tree, so it must
+// not collide with a container id.
+func BindIntoShare(ctx context.Context, src, id, name string) error {
+	if name == "" {
+		return fmt.Errorf("BindIntoShare: empty share subdir name")
+	}
+	dst := filepath.Join(SharedDir(id), name)
+	// Drop any stale bind first (lazy if busy), then ensure a clean mountpoint.
+	if err := reaper.Run(exec.Command("umount", dst)); err != nil {
+		_ = reaper.Run(exec.Command("umount", "-l", dst))
+	}
+	if err := os.MkdirAll(dst, 0o755); err != nil {
+		return fmt.Errorf("creating share subdir %q: %w", dst, err)
+	}
+	cmd := exec.CommandContext(ctx, "mount", "--bind", src, dst)
+	var stderr strings.Builder
+	cmd.Stderr = &stderr
+	if err := reaper.Run(cmd); err != nil {
+		return fmt.Errorf("bind-mounting %q into the shared tree at %q: %w (%s)", src, dst, err, strings.TrimSpace(stderr.String()))
+	}
+	return nil
+}
+
 // ReconstructSharedDirFromImage bind-mounts a container's OCI image rootfs at
 // <cid>/rootfs under SharedDir(restoreID) so virtiofsd serves it as the read-only
 // lower. LEGACY restores only: guests from retired guest-tmpfs-upper snapshots hold

cmd/ateom-microvm/internal/kata/overlay_linux_test.go

@@ -47,6 +47,27 @@ func TestUpperWorkDirsAreSiblings(t *testing.T) {
 	}
 }
 
+// The volume subtrees ride the ONE kataShared device (BindIntoShare): the host
+// side is bind-mounted under the directory virtiofsd serves, and the guest
+// re-opens the same relative path under its kataShared mount — find-paths
+// re-opens open volume files by path on restore, so host and guest must agree.
+func TestVolumeSubtreePaths(t *testing.T) {
+	for name, guest := range map[string]string{
+		"durable": GuestDurableVolumeDir("data"),
+		"csi":     GuestCSIVolumeDir("data"),
+	} {
+		host := filepath.Join(SharedDir("uid"), name, "data")
+		hostRel, err := filepath.Rel(SharedDir("uid"), host)
+		if err != nil {
+			t.Fatalf("Rel(host): %v", err)
+		}
+		guestRel := strings.TrimPrefix(guest, guestSharedDir)
+		if hostRel != guestRel {
+			t.Errorf("%s: host-relative path %q != guest-relative path %q; find-paths would re-open the wrong file", name, hostRel, guestRel)
+		}
+	}
+}
+
 func TestVirtiofsdArgs(t *testing.T) {
 	args := virtiofsdArgs(VirtiofsdOptions{
 		SocketPath: "/run/vm/virtiofsd.sock",

cmd/ateom-microvm/internal/kata/restore.go

@@ -29,15 +29,3 @@ func VMDir(id string) string { return filepath.Join(vcVMDir, id) }
 // VsockSocketPath is the hybrid-vsock socket the CH snapshot's vsock device
 // references; CH recreates the listener here on restore.
 func VsockSocketPath(id string) string { return filepath.Join(VMDir(id), "clh.sock") }
-
-// DurableVirtiofsdSocketPath is the vhost-user-fs socket for the actor's writable
-// durable-dir share, served by a second virtiofsd alongside the rootfs share's.
-func DurableVirtiofsdSocketPath(id string) string {
-	return filepath.Join(VMDir(id), "virtiofsd-durable.sock")
-}
-
-// CsiVirtiofsdSocketPath is the vhost-user-fs socket for the actor's writable
-// CSI volumes share.
-func CsiVirtiofsdSocketPath(id string) string {
-	return filepath.Join(VMDir(id), "virtiofsd-csi.sock")
-}

cmd/ateom-microvm/restore.go

@@ -474,6 +474,12 @@ func rewriteSnapshotSocketPaths(snapshotDir, id string) error {
 	}
 	// The virtio-fs share is served by its per-VMDir virtiofsd socket; the
 	// snapshot recorded the golden actor's, so repoint it at this actor's VMDir.
+	// kataShared is the ONLY tag a restorable snapshot can carry: anything else
+	// (the retired multi-virtiofsd tags "ateDurable"/"ateCSI"/"ateUpper" among
+	// them) must fail loudly here. Repointing such a device used to pass for
+	// backward compatibility, but nothing spawns those extra virtiofsds anymore,
+	// so the "compatible" restore just failed later and worse — at CH's vhost
+	// socket connect, or as a guest wedged on a dead mount.
 	if fss, ok := cfg["fs"].([]any); ok {
 		for _, f := range fss {
 			fm, ok := f.(map[string]any)
@@ -483,14 +489,8 @@ func rewriteSnapshotSocketPaths(snapshotDir, id string) error {
 			switch tag, _ := fm["tag"].(string); tag {
 			case kata.FsTag:
 				fm["socket"] = kata.VirtiofsdSocketPath(id)
-			case "ateDurable":
-				// Legacy multi-virtiofs snapshot backward compatibility.
-				fm["socket"] = kata.DurableVirtiofsdSocketPath(id)
-			case "ateCSI":
-				// Legacy multi-virtiofs snapshot backward compatibility.
-				fm["socket"] = kata.CsiVirtiofsdSocketPath(id)
 			default:
-				return fmt.Errorf("snapshot config %q has fs device with unknown tag %q", cfgPath, tag)
+				return fmt.Errorf("snapshot config %q has fs device with unknown tag %q (multi-virtiofsd snapshots are no longer restorable)", cfgPath, tag)
 			}
 		}
 	}

cmd/ateom-microvm/restore_test.go

@@ -85,35 +85,16 @@ func TestRewriteSnapshotSocketPaths(t *testing.T) {
 		}
 	})
 
-	t.Run("legacy multi-share snapshots repoint each tag", func(t *testing.T) {
-		// Ordered with the rootfs share last to catch a rewrite that assumes it
-		// comes first, which would hand the guest the wrong filesystem.
-		dir := writeSnapshotConfig(t, []map[string]any{
-			{"tag": "ateDurable", "socket": "/run/vc/vm/golden/virtiofsd-durable.sock"},
-			{"tag": kata.FsTag, "socket": "/run/vc/vm/golden/virtiofsd.sock"},
-		})
-		if err := rewriteSnapshotSocketPaths(dir, id); err != nil {
-			t.Fatalf("rewriteSnapshotSocketPaths: %v", err)
-		}
-		got := readFsSockets(t, dir)
-		if got[kata.FsTag] != kata.VirtiofsdSocketPath(id) {
-			t.Errorf("%s socket = %q, want %q", kata.FsTag, got[kata.FsTag], kata.VirtiofsdSocketPath(id))
-		}
-		if got["ateDurable"] != kata.DurableVirtiofsdSocketPath(id) {
-			t.Errorf("ateDurable socket = %q, want %q", got["ateDurable"], kata.DurableVirtiofsdSocketPath(id))
-		}
-		if got[kata.FsTag] == got["ateDurable"] {
-			t.Error("both shares were pointed at the same socket")
-		}
-	})
-
-	t.Run("unknown tag is an error", func(t *testing.T) {
-		// "ateUpper" is the retired third share's tag: it never appears in
-		// snapshots this code produces, and one showing up must fail loudly
-		// rather than be silently repointed.
-		for _, tag := range []string{"somethingElse", "ateUpper"} {
+	t.Run("non-kataShared tags are an error", func(t *testing.T) {
+		// The retired multi-virtiofsd tags ("ateDurable"/"ateCSI" second and
+		// third shares, "ateUpper" from the retired guest-mounted-upper
+		// arrangement) must fail loudly, exactly like a tag that never existed:
+		// nothing spawns those extra virtiofsds anymore, so repointing such a
+		// device would just defer the failure to CH's vhost socket connect.
+		for _, tag := range []string{"somethingElse", "ateUpper", "ateDurable", "ateCSI"} {
 			dir := writeSnapshotConfig(t, []map[string]any{
 				{"tag": tag, "socket": "/run/vc/vm/golden/other.sock"},
+				{"tag": kata.FsTag, "socket": "/run/vc/vm/golden/virtiofsd.sock"},
 			})
 			if err := rewriteSnapshotSocketPaths(dir, id); err == nil {
 				t.Fatalf("rewriteSnapshotSocketPaths accepted fs tag %q, want an error", tag)