fix issues, promote timeout in long api

Author: CMGS

cmd/vm/run.go

@@ -330,6 +330,11 @@ func (h Handler) createVM(cmd *cobra.Command, image string) (context.Context, *t
 	return ctx, info, hyper, nil
 }
 
+type nicHint struct {
+	mac, ip, gw string
+	prefix      int
+}
+
 // tapQueues returns the number of TAP queues per NIC.
 // FC only supports single-queue TAPs; CH uses one queue per vCPU.
 func tapQueues(cpu int, useFC bool) int {
@@ -433,11 +438,6 @@ func printCloudimgNetworkHints() {
 	fmt.Println("  cloud-init modules --mode=config && systemctl restart systemd-networkd")
 }
 
-type nicHint struct {
-	mac, ip, gw string
-	prefix      int
-}
-
 func printOCINetworkHints(vm *types.VM, networkConfigs []*types.NetworkConfig) {
 	fmt.Println()
 	fmt.Printf("  # Set hostname\n")

cmd/vm/status.go

@@ -87,6 +87,17 @@ func (h Handler) Status(cmd *cobra.Command, args []string) error {
 	return nil
 }
 
+type vmEvent struct {
+	Event string   `json:"event"`
+	VM    types.VM `json:"vm"`
+}
+
+type vmSnapshot struct {
+	id, name, state, ip, image string
+	cpu                        int
+	memory                     int64
+}
+
 func mergeWatchChannels(ctx context.Context, hypers []hypervisor.Hypervisor) <-chan struct{} {
 	var channels []<-chan struct{}
 	for _, h := range hypers {
@@ -199,11 +210,6 @@ func statusEventLoop(ctx context.Context, hypers []hypervisor.Hypervisor, filter
 	})
 }
 
-type vmEvent struct {
-	Event string   `json:"event"`
-	VM    types.VM `json:"vm"`
-}
-
 func statusEventLoopJSON(ctx context.Context, hypers []hypervisor.Hypervisor, filters []string, watchCh <-chan struct{}, tick <-chan time.Time) {
 	enc := json.NewEncoder(os.Stdout)
 	type snapshotWithVM struct {
@@ -237,12 +243,6 @@ func statusEventLoopJSON(ctx context.Context, hypers []hypervisor.Hypervisor, fi
 	})
 }
 
-type vmSnapshot struct {
-	id, name, state, ip, image string
-	cpu                        int
-	memory                     int64
-}
-
 func takeSnapshot(vm *types.VM) vmSnapshot {
 	return vmSnapshot{
 		id:     vm.ID,

hypervisor/backend.go

@@ -309,8 +309,8 @@ func (b *Backend) CleanStalePlaceholders(_ context.Context, ids []string) error
 }
 
 // GCCollect removes orphan VM directories and stale DB records.
-// Runs under the GC orchestrator's flock — uses lock-free DB access
-// (ReadRaw/WriteRaw) to avoid self-deadlock.
+// Kills leftover hypervisor processes before removing directories.
+// Runs under the GC orchestrator's flock — uses lock-free DB access.
 func (b *Backend) GCCollect(ctx context.Context, ids []string) error {
 	var errs []error
 	for _, id := range ids {
@@ -321,6 +321,7 @@ func (b *Backend) GCCollect(ctx context.Context, ids []string) error {
 			}
 			return nil
 		})
+		b.killOrphanProcess(ctx, runDir)
 		if err := RemoveVMDirs(runDir, logDir); err != nil {
 			errs = append(errs, err)
 		}
@@ -331,6 +332,20 @@ func (b *Backend) GCCollect(ctx context.Context, ids []string) error {
 	return errors.Join(errs...)
 }
 
+// killOrphanProcess terminates a leftover hypervisor process if the PID
+// file exists and the process matches the expected binary name.
+func (b *Backend) killOrphanProcess(ctx context.Context, runDir string) {
+	pid, err := utils.ReadPIDFile(b.PIDFilePath(runDir))
+	if err != nil {
+		return
+	}
+	sockPath := SocketPath(runDir)
+	if !utils.VerifyProcessCmdline(pid, b.Conf.BinaryName(), sockPath) {
+		return
+	}
+	_ = utils.TerminateProcess(ctx, pid, b.Conf.BinaryName(), sockPath, b.Conf.TerminateGracePeriod())
+}
+
 // PIDFilePath returns the PID file path for the backend's PID file name.
 func (b *Backend) PIDFilePath(runDir string) string {
 	return filepath.Join(runDir, b.Conf.PIDFileName())

hypervisor/cloudhypervisor/clone.go

@@ -178,7 +178,7 @@ func (ch *CloudHypervisor) restoreAndResumeClone(
 		}
 	}()
 
-	hc := utils.NewSocketHTTPClientWithTimeout(sockPath, hypervisor.VMMemTransferTimeout)
+	hc := utils.NewSocketHTTPClient(sockPath)
 
 	if err = restoreVM(ctx, hc, runDir, opts.onDemand); err != nil {
 		return fmt.Errorf("vm.restore: %w", err)
@@ -349,10 +349,11 @@ func hotSwapNets(ctx context.Context, hc *http.Client, oldNets []chNet, networkC
 		}
 		logger.Infof(ctx, "removed snapshot NIC %s (old MAC %s)", oldNet.ID, oldNet.Mac)
 	}
-	for _, nc := range networkConfigs {
+	for i, nc := range networkConfigs {
 		newNet := networkConfigToNet(nc)
 		if err := addNetVM(ctx, hc, newNet); err != nil {
-			return fmt.Errorf("add net device for %s: %w", nc.Mac, err)
+			return fmt.Errorf("add net device %d/%d (MAC %s, TAP %s): %w",
+				i+1, len(networkConfigs), nc.Mac, nc.Tap, err)
 		}
 		logger.Infof(ctx, "added NIC with MAC %s on TAP %s", nc.Mac, nc.Tap)
 	}

hypervisor/cloudhypervisor/helper.go

@@ -17,6 +17,10 @@ import (
 
 const (
 	cmdlineFileName = "cmdline"
+
+	// chMemoryRestoreOnDemand uses userfaultfd (UFFD) to lazily page in
+	// guest memory from the snapshot file, avoiding a full upfront copy.
+	chMemoryRestoreOnDemand chMemoryRestoreMode = "OnDemand"
 )
 
 var runtimeFiles = []string{hypervisor.APISocketName, "ch.pid", hypervisor.ConsoleSockName, cmdlineFileName}
@@ -68,15 +72,17 @@ func resumeVM(ctx context.Context, hc *http.Client) error {
 	return vmAPI(ctx, hc, "vm.resume", nil)
 }
 
-// snapshotVM and restoreVM bypass vmAPI's retry layer — see hypervisor.VMMemTransferTimeout.
-func snapshotVM(ctx context.Context, sockPath, destDir string) error {
+// snapshotVM and restoreVM temporarily extend the client timeout for
+// long-running memory transfers, then restore it for subsequent calls.
+func snapshotVM(ctx context.Context, hc *http.Client, destDir string) error {
+	hc.Timeout = hypervisor.VMMemTransferTimeout
+	defer func() { hc.Timeout = utils.HTTPTimeout }()
 	body, err := json.Marshal(map[string]string{
 		"destination_url": "file://" + destDir,
 	})
 	if err != nil {
 		return fmt.Errorf("marshal snapshot request: %w", err)
 	}
-	hc := utils.NewSocketHTTPClientWithTimeout(sockPath, hypervisor.VMMemTransferTimeout)
 	_, err = utils.DoAPI(ctx, hc, http.MethodPut,
 		"http://localhost/api/v1/vm.snapshot", body, http.StatusNoContent)
 	return err
@@ -85,18 +91,14 @@ func snapshotVM(ctx context.Context, sockPath, destDir string) error {
 // chMemoryRestoreMode controls how CH restores guest memory from a snapshot.
 type chMemoryRestoreMode string
 
-const (
-	// chMemoryRestoreOnDemand uses userfaultfd (UFFD) to lazily page in
-	// guest memory from the snapshot file, avoiding a full upfront copy.
-	chMemoryRestoreOnDemand chMemoryRestoreMode = "OnDemand"
-)
-
 type chRestoreConfig struct {
 	SourceURL         string              `json:"source_url"`
 	MemoryRestoreMode chMemoryRestoreMode `json:"memory_restore_mode,omitempty"`
 }
 
 func restoreVM(ctx context.Context, hc *http.Client, sourceDir string, onDemand bool) error {
+	hc.Timeout = hypervisor.VMMemTransferTimeout
+	defer func() { hc.Timeout = utils.HTTPTimeout }()
 	cfg := chRestoreConfig{
 		SourceURL: "file://" + sourceDir,
 	}

hypervisor/cloudhypervisor/restore.go

@@ -135,7 +135,7 @@ func (ch *CloudHypervisor) restoreAfterExtract(ctx context.Context, vmID string,
 		}
 	}()
 
-	hc := utils.NewSocketHTTPClientWithTimeout(sockPath, hypervisor.VMMemTransferTimeout)
+	hc := utils.NewSocketHTTPClient(sockPath)
 
 	if err = restoreVM(ctx, hc, rec.RunDir, vmCfg.OnDemand); err != nil {
 		return nil, fmt.Errorf("vm.restore: %w", err)

hypervisor/cloudhypervisor/snapshot.go

@@ -68,7 +68,7 @@ func (ch *CloudHypervisor) Snapshot(ctx context.Context, ref string) (*types.Sna
 		}
 		defer doResume()
 
-		if err := snapshotVM(ctx, sockPath, tmpDir); err != nil {
+		if err := snapshotVM(ctx, hc, tmpDir); err != nil {
 			return fmt.Errorf("snapshot: %w", err)
 		}
 

hypervisor/firecracker/clone.go

@@ -15,6 +15,10 @@ import (
 	"github.com/cocoonstack/cocoon/utils"
 )
 
+const (
+	cloneBackupSuffix = ".cocoon-clone-backup"
+)
+
 // Clone creates a new VM from a snapshot tar stream via FC snapshot/load.
 func (fc *Firecracker) Clone(ctx context.Context, vmID string, vmCfg *types.VMConfig, networkConfigs []*types.NetworkConfig, snapshotConfig *types.SnapshotConfig, snapshot io.Reader) (_ *types.VM, err error) {
 	runDir, logDir, now, cleanup, err := fc.CloneSetup(ctx, vmID, vmCfg, snapshotConfig)
@@ -190,7 +194,7 @@ func createDriveRedirects(srcConfigs, dstConfigs []*types.StorageConfig) ([]driv
 		r := driveRedirect{symlinkPath: src.Path}
 
 		if _, err := os.Stat(src.Path); err == nil {
-			backup := src.Path + ".cocoon-clone-backup"
+			backup := src.Path + cloneBackupSuffix
 			if renameErr := os.Rename(src.Path, backup); renameErr != nil {
 				cleanupDriveRedirects(redirects)
 				return nil, fmt.Errorf("backup source drive %s: %w", src.Path, renameErr)
@@ -231,15 +235,33 @@ func cleanupDriveRedirects(redirects []driveRedirect) {
 }
 
 // withSourceCOWLocked runs fn while holding the source COW lock.
+// Recovers stale symlink backups from crashed clones before proceeding.
 func withSourceCOWLocked(srcConfigs []*types.StorageConfig, fn func() error) error {
 	for _, sc := range srcConfigs {
 		if !sc.RO {
-			return withCOWPathLocked(sc.Path, fn)
+			return withCOWPathLocked(sc.Path, func() error {
+				recoverStaleBackup(sc.Path)
+				return fn()
+			})
 		}
 	}
 	return fn() // no RW disk, run unlocked
 }
 
+// recoverStaleBackup restores a backup file left by a crashed clone.
+// Caller must hold the COW lock.
+func recoverStaleBackup(cowPath string) {
+	backup := cowPath + cloneBackupSuffix
+	if _, err := os.Stat(backup); err != nil {
+		return
+	}
+	fi, err := os.Lstat(cowPath)
+	if err == nil && fi.Mode()&os.ModeSymlink != 0 {
+		_ = os.Remove(cowPath)
+	}
+	_ = os.Rename(backup, cowPath)
+}
+
 func buildNetworkOverrides(networkConfigs []*types.NetworkConfig) []fcNetworkOverride {
 	var overrides []fcNetworkOverride
 	for i, nc := range networkConfigs {