review: whole-repo round — unify backend capability gates, extend fsync-skip to streamed paths

vm debug shared none of createVM's capability gate list and had drifted
(missing fc+windows and windows-requires-cloudimg); both now call the same
validators. ExtractTar's per-file fsync goes away: clone/restore streams
follow the #160 no-sync contract and snapshot Create/Import rely on the
SyncTree barrier — Import previously never made dir entries durable before
the record committed, so it gains that barrier. Multiprocess storm tests
share one spawn/wait harness in contracttest instead of four copies.

Author: CMGS

cmd/vm/debug.go

@@ -44,11 +44,8 @@ func (h Handler) Debug(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
-	if conf.UseFirecracker && vmCfg.SharedMemory {
-		return fmt.Errorf("--fc and --shared-memory are mutually exclusive: Firecracker does not support vhost-user-fs hot-plug")
-	}
-	if conf.UseFirecracker && vmCfg.HugePages {
-		return fmt.Errorf("--fc and --hugepages are mutually exclusive: Firecracker cannot restore hugetlbfs-backed snapshots")
+	if err = validateBackendFlags(conf, vmCfg); err != nil {
+		return err
 	}
 	if len(vmCfg.DataDisks) > 0 {
 		fmt.Fprintln(os.Stderr, "warning: --data-disk is ignored in debug mode (debug only prints the hypervisor launch command; data disks need PrepareDataDisks to materialize)")
@@ -58,11 +55,11 @@ func (h Handler) Debug(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
+	if err = validateBootCompat(conf, vmCfg, boot); err != nil {
+		return err
+	}
 
 	if conf.UseFirecracker {
-		if boot.KernelPath == "" {
-			return fmt.Errorf("--fc requires OCI images (direct kernel boot)")
-		}
 		// FC requires uncompressed ELF kernel — resolve vmlinux path for debug output.
 		if err := firecracker.EnsureVmlinuxBoot(boot); err != nil {
 			return err

cmd/vm/run.go

@@ -419,14 +419,8 @@ func (h Handler) createVM(cmd *cobra.Command, image string) (context.Context, *t
 		return nil, nil, nil, err
 	}
 
-	if conf.UseFirecracker && vmCfg.Windows {
-		return nil, nil, nil, fmt.Errorf("--fc and --windows are mutually exclusive: Firecracker does not support Windows guests")
-	}
-	if conf.UseFirecracker && vmCfg.SharedMemory {
-		return nil, nil, nil, fmt.Errorf("--fc and --shared-memory are mutually exclusive: Firecracker does not support vhost-user-fs hot-plug")
-	}
-	if conf.UseFirecracker && vmCfg.HugePages {
-		return nil, nil, nil, fmt.Errorf("--fc and --hugepages are mutually exclusive: Firecracker cannot restore hugetlbfs-backed snapshots")
+	if err = validateBackendFlags(conf, vmCfg); err != nil {
+		return nil, nil, nil, err
 	}
 	bridgeDev, _ := cmd.Flags().GetString("bridge")
 	if bridgeDev != "" && vmCfg.Network != "" {
@@ -446,11 +440,8 @@ func (h Handler) createVM(cmd *cobra.Command, image string) (context.Context, *t
 	if err != nil {
 		return nil, nil, nil, err
 	}
-	if vmCfg.Windows && bootCfg.KernelPath != "" {
-		return nil, nil, nil, fmt.Errorf("--windows requires cloudimg (UEFI boot), got OCI direct boot image")
-	}
-	if conf.UseFirecracker && bootCfg.KernelPath == "" {
-		return nil, nil, nil, fmt.Errorf("--fc requires OCI images (direct kernel boot): Firecracker does not support UEFI/cloudimg boot")
+	if err = validateBootCompat(conf, vmCfg, bootCfg); err != nil {
+		return nil, nil, nil, err
 	}
 	cmdcore.EnsureFirmwarePath(conf, bootCfg)
 
@@ -485,6 +476,33 @@ func (h Handler) createVM(cmd *cobra.Command, image string) (context.Context, *t
 	return ctx, info, hyper, nil
 }
 
+// validateBackendFlags fast-fails flag combinations the selected backend can never launch; boot-mode-dependent checks live in validateBootCompat. Shared by create and debug so the capability gate list cannot drift.
+func validateBackendFlags(conf *config.Config, vmCfg *types.VMConfig) error {
+	if !conf.UseFirecracker {
+		return nil
+	}
+	switch {
+	case vmCfg.Windows:
+		return fmt.Errorf("--fc and --windows are mutually exclusive: Firecracker does not support Windows guests")
+	case vmCfg.SharedMemory:
+		return fmt.Errorf("--fc and --shared-memory are mutually exclusive: Firecracker does not support vhost-user-fs hot-plug")
+	case vmCfg.HugePages:
+		return fmt.Errorf("--fc and --hugepages are mutually exclusive: Firecracker cannot restore hugetlbfs-backed snapshots")
+	}
+	return nil
+}
+
+func validateBootCompat(conf *config.Config, vmCfg *types.VMConfig, bootCfg *types.BootConfig) error {
+	directBoot := hypervisor.IsDirectBoot(bootCfg)
+	if vmCfg.Windows && directBoot {
+		return fmt.Errorf("--windows requires cloudimg (UEFI boot), got OCI direct boot image")
+	}
+	if conf.UseFirecracker && !directBoot {
+		return fmt.Errorf("--fc requires OCI images (direct kernel boot): Firecracker does not support UEFI/cloudimg boot")
+	}
+	return nil
+}
+
 // pinResolvedBlobs holds the resolved image's digest locks until the reserve
 // commits; the empty set (bridge/dataless) pins nothing.
 func pinResolvedBlobs(ctx context.Context, backends []imagebackend.Images, ref string, blobIDs map[string]struct{}) (func(), error) {

hypervisor/cloudhypervisor/utils.go

@@ -181,8 +181,7 @@ func restoreVM(ctx context.Context, hc *http.Client, sourceDir, restoreMode stri
 	case restoreModeMmap:
 		cfg.MemoryRestoreMode = chMemoryRestoreMmap
 	default:
-		// Fail loud rather than silently falling back to eager copy — a
-		// misspelled or wrong-case mode is a ~6x restore-latency regression.
+		// Fail loud: a misspelled mode silently falling back to eager copy is a ~6x restore-latency regression.
 		return fmt.Errorf("unknown restore mode %q", restoreMode)
 	}
 	body, err := json.Marshal(cfg)

meta/contracttest/workers.go

@@ -0,0 +1,35 @@
+package contracttest
+
+import (
+	"bytes"
+	"os"
+	"os/exec"
+	"testing"
+)
+
+// SpawnWorkers re-execs the test binary n times against the helper test testName; env yields each worker's extra environment.
+func SpawnWorkers(t *testing.T, n int, testName string, env func(w int) []string) []*exec.Cmd {
+	t.Helper()
+	cmds := make([]*exec.Cmd, 0, n)
+	for w := range n {
+		cmd := exec.Command(os.Args[0], "-test.run="+testName+"$", "-test.count=1") //nolint:gosec
+		cmd.Env = append(os.Environ(), env(w)...)
+		out := &bytes.Buffer{}
+		cmd.Stdout, cmd.Stderr = out, out
+		if err := cmd.Start(); err != nil {
+			t.Fatalf("start worker %d: %v", w, err)
+		}
+		cmds = append(cmds, cmd)
+	}
+	return cmds
+}
+
+// WaitWorkers fails on the first worker that exits non-zero, echoing its combined output; what labels the failure.
+func WaitWorkers(t *testing.T, cmds []*exec.Cmd, what string) {
+	t.Helper()
+	for w, cmd := range cmds {
+		if err := cmd.Wait(); err != nil {
+			t.Fatalf("worker %d %s: %v\n%s", w, what, err, cmd.Stdout)
+		}
+	}
+}

meta/json/multiprocess_test.go

@@ -2,7 +2,6 @@ package json
 
 import (
 	"bufio"
-	"bytes"
 	"fmt"
 	"os"
 	"os/exec"
@@ -12,6 +11,7 @@ import (
 	"time"
 
 	"github.com/cocoonstack/cocoon/meta"
+	"github.com/cocoonstack/cocoon/meta/contracttest"
 )
 
 const (
@@ -39,27 +39,10 @@ func TestMultiProcessCorrectness(t *testing.T) {
 	}
 	dir := t.TempDir()
 	workers := stormWorkers()
-	cmds := make([]*exec.Cmd, 0, workers)
-	for w := range workers {
-		cmd := exec.Command(os.Args[0], "-test.run=TestMultiProcessWorker$", "-test.count=1") //nolint:gosec
-		cmd.Env = append(
-			os.Environ(),
-			"META_MP_DIR="+dir,
-			"META_MP_WORKER="+strconv.Itoa(w),
-			fmt.Sprintf("META_MP_OPS=%d", mpOps),
-		)
-		out := &bytes.Buffer{}
-		cmd.Stdout, cmd.Stderr = out, out
-		if err := cmd.Start(); err != nil {
-			t.Fatalf("start worker %d: %v", w, err)
-		}
-		cmds = append(cmds, cmd)
-	}
-	for w, cmd := range cmds {
-		if err := cmd.Wait(); err != nil {
-			t.Fatalf("worker %d failed: %v\n%s", w, err, cmd.Stdout)
-		}
-	}
+	cmds := contracttest.SpawnWorkers(t, workers, "TestMultiProcessWorker", func(w int) []string {
+		return []string{"META_MP_DIR=" + dir, "META_MP_WORKER=" + strconv.Itoa(w), fmt.Sprintf("META_MP_OPS=%d", mpOps)}
+	})
+	contracttest.WaitWorkers(t, cmds, "failed")
 
 	s := newStore(t, dir, "alpha")
 	c := meta.NewCollection[map[string]int](s, "alpha", "records")
@@ -115,27 +98,10 @@ func TestInverseScopeNoDeadlock(t *testing.T) {
 	}
 	dir := t.TempDir()
 	scopes := []string{"alpha:beta", "beta:alpha"}
-	cmds := make([]*exec.Cmd, 0, 2*inverseWorkers)
-	for w := range 2 * inverseWorkers {
-		cmd := exec.Command(os.Args[0], "-test.run=TestInverseScopeWorker$", "-test.count=1") //nolint:gosec
-		cmd.Env = append(
-			os.Environ(),
-			"META_MP_DIR="+dir,
-			"META_MP_WORKER="+strconv.Itoa(w),
-			"META_MP_SCOPE="+scopes[w%2],
-		)
-		out := &bytes.Buffer{}
-		cmd.Stdout, cmd.Stderr = out, out
-		if err := cmd.Start(); err != nil {
-			t.Fatalf("start worker %d: %v", w, err)
-		}
-		cmds = append(cmds, cmd)
-	}
-	for w, cmd := range cmds {
-		if err := cmd.Wait(); err != nil {
-			t.Fatalf("worker %d failed (deadlock or error): %v\n%s", w, err, cmd.Stdout)
-		}
-	}
+	cmds := contracttest.SpawnWorkers(t, 2*inverseWorkers, "TestInverseScopeWorker", func(w int) []string {
+		return []string{"META_MP_DIR=" + dir, "META_MP_WORKER=" + strconv.Itoa(w), "META_MP_SCOPE=" + scopes[w%2]}
+	})
+	contracttest.WaitWorkers(t, cmds, "failed (deadlock or error)")
 	s := newStore(t, dir, "alpha", "beta")
 	for _, ns := range []string{"alpha", "beta"} {
 		total := 0

meta/sqlite/multiprocess_test.go

@@ -2,7 +2,6 @@ package sqlite
 
 import (
 	"bufio"
-	"bytes"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -14,6 +13,7 @@ import (
 	"testing"
 
 	"github.com/cocoonstack/cocoon/meta"
+	"github.com/cocoonstack/cocoon/meta/contracttest"
 )
 
 const (
@@ -70,27 +70,10 @@ func TestMultiProcessCorrectness(t *testing.T) {
 	dir := t.TempDir()
 	_ = newStore(t, dir, "alpha") // parent initializes once; workers only open
 	workers := stormWorkers()
-	cmds := make([]*exec.Cmd, 0, workers)
-	for w := range workers {
-		cmd := exec.Command(os.Args[0], "-test.run=TestMultiProcessWorker$", "-test.count=1") //nolint:gosec
-		cmd.Env = append(
-			os.Environ(),
-			"META_MP_DIR="+dir,
-			"META_MP_WORKER="+strconv.Itoa(w),
-			fmt.Sprintf("META_MP_OPS=%d", mpOps),
-		)
-		out := &bytes.Buffer{}
-		cmd.Stdout, cmd.Stderr = out, out
-		if err := cmd.Start(); err != nil {
-			t.Fatalf("start worker %d: %v", w, err)
-		}
-		cmds = append(cmds, cmd)
-	}
-	for w, cmd := range cmds {
-		if err := cmd.Wait(); err != nil {
-			t.Fatalf("worker %d failed: %v\n%s", w, err, cmd.Stdout)
-		}
-	}
+	cmds := contracttest.SpawnWorkers(t, workers, "TestMultiProcessWorker", func(w int) []string {
+		return []string{"META_MP_DIR=" + dir, "META_MP_WORKER=" + strconv.Itoa(w), fmt.Sprintf("META_MP_OPS=%d", mpOps)}
+	})
+	contracttest.WaitWorkers(t, cmds, "failed")
 
 	s := newStore(t, dir, "alpha")
 	records := map[string]struct{}{}
@@ -157,27 +140,10 @@ func TestInverseScopeNoDeadlock(t *testing.T) {
 	dir := t.TempDir()
 	_ = newStore(t, dir, "alpha", "beta")
 	scopes := []string{"alpha:beta", "beta:alpha"}
-	cmds := make([]*exec.Cmd, 0, 2*inverseWorkers)
-	for w := range 2 * inverseWorkers {
-		cmd := exec.Command(os.Args[0], "-test.run=TestInverseScopeWorker$", "-test.count=1") //nolint:gosec
-		cmd.Env = append(
-			os.Environ(),
-			"META_MP_DIR="+dir,
-			"META_MP_WORKER="+strconv.Itoa(w),
-			"META_MP_SCOPE="+scopes[w%2],
-		)
-		out := &bytes.Buffer{}
-		cmd.Stdout, cmd.Stderr = out, out
-		if err := cmd.Start(); err != nil {
-			t.Fatalf("start worker %d: %v", w, err)
-		}
-		cmds = append(cmds, cmd)
-	}
-	for w, cmd := range cmds {
-		if err := cmd.Wait(); err != nil {
-			t.Fatalf("worker %d failed (deadlock or error): %v\n%s", w, err, cmd.Stdout)
-		}
-	}
+	cmds := contracttest.SpawnWorkers(t, 2*inverseWorkers, "TestInverseScopeWorker", func(w int) []string {
+		return []string{"META_MP_DIR=" + dir, "META_MP_WORKER=" + strconv.Itoa(w), "META_MP_SCOPE=" + scopes[w%2]}
+	})
+	contracttest.WaitWorkers(t, cmds, "failed (deadlock or error)")
 	s := newStore(t, dir, "alpha", "beta")
 	for _, ns := range []string{"alpha", "beta"} {
 		total := 0

snapshot/localfile/import.go

@@ -70,6 +70,11 @@ func (lf *LocalFile) Import(ctx context.Context, r io.Reader, name, description
 		return "", err
 	}
 
+	// Files and dir entries durable before the record exists (ExtractTar never fsyncs).
+	if err = utils.SyncTree(dataDir); err != nil {
+		return "", fmt.Errorf("sync snapshot to disk: %w", err)
+	}
+
 	size, sizeErr := utils.DirSize(dataDir)
 	if sizeErr != nil {
 		return "", fmt.Errorf("compute data dir size: %w", sizeErr)

utils/tar.go

@@ -54,7 +54,7 @@ func TarDir(tw *tar.Writer, dir string) error {
 	return nil
 }
 
-// ExtractTar extracts flat tar entries into dir; entries matching any skip predicate are dropped.
+// ExtractTar extracts flat tar entries into dir; entries matching any skip predicate are dropped. It never fsyncs — callers needing durability follow with SyncTree.
 func ExtractTar(dir string, r io.Reader, skip ...func(name string) bool) error {
 	tr := tar.NewReader(r)
 	for {
@@ -112,17 +112,17 @@ func tarFileFrom(tw *tar.Writer, f *os.File, fi os.FileInfo, nameInTar string) e
 }
 
 // extractFileSparse restores a sparse file from its segment map.
-func extractFileSparse(path string, r io.Reader, perm os.FileMode, realSize int64, mapJSON string) error {
+func extractFileSparse(path string, r io.Reader, perm os.FileMode, realSize int64, mapJSON string) (err error) {
 	var segments []sparseSegment
-	if err := json.Unmarshal([]byte(mapJSON), &segments); err != nil {
+	if err = json.Unmarshal([]byte(mapJSON), &segments); err != nil {
 		return fmt.Errorf("decode sparse map: %w", err)
 	}
 
 	f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, perm) //nolint:gosec
 	if err != nil {
 		return err
 	}
-	defer f.Close() //nolint:errcheck
+	defer func() { err = errors.Join(err, f.Close()) }()
 
 	if err := f.Truncate(realSize); err != nil {
 		return err
@@ -137,15 +137,15 @@ func extractFileSparse(path string, r io.Reader, perm os.FileMode, realSize int6
 		}
 	}
 
-	return f.Sync()
+	return nil
 }
 
-func extractFile(path string, r io.Reader, perm os.FileMode) error {
+func extractFile(path string, r io.Reader, perm os.FileMode) (err error) {
 	f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, perm) //nolint:gosec
 	if err != nil {
 		return err
 	}
-	defer f.Close() //nolint:errcheck
+	defer func() { err = errors.Join(err, f.Close()) }()
 
 	bp := sparseBlockPool.Get().(*[]byte)
 	buf := *bp
@@ -178,7 +178,7 @@ func extractFile(path string, r io.Reader, perm os.FileMode) error {
 		}
 	}
 
-	return f.Sync()
+	return nil
 }
 
 // writeBlockSparse seeks over all-zero chunks instead of writing them.