gc

Author: CMGS

gc/gc.go

@@ -40,3 +40,23 @@ func (m Module[S]) resolveTargets(snap any, others map[string]any) []string {
 func (m Module[S]) collect(ctx context.Context, ids []string) error {
 	return m.Collect(ctx, ids)
 }
+
+// UsedBlobIDs is optionally implemented by GC snapshots that reference image blobs.
+// Image GC modules use CollectUsedBlobIDs to skip dangling blobs still needed by VMs.
+type UsedBlobIDs interface {
+	UsedBlobIDs() map[string]struct{}
+}
+
+// CollectUsedBlobIDs aggregates all blob hex IDs referenced by UsedBlobIDs
+// snapshots in others.
+func CollectUsedBlobIDs(others map[string]any) map[string]struct{} {
+	used := make(map[string]struct{})
+	for _, snap := range others {
+		if u, ok := snap.(UsedBlobIDs); ok {
+			for id := range u.UsedBlobIDs() {
+				used[id] = struct{}{}
+			}
+		}
+	}
+	return used
+}

gc/orchestrator.go

@@ -25,41 +25,50 @@ func Register[S any](o *Orchestrator, m Module[S]) {
 
 // Run executes one GC cycle:
 //
-//  1. For each module: TryLock → ReadDB → Unlock (skip if busy).
-//  2. Each module's Resolve analyses its typed snapshot, with other modules'
-//     snapshots available as map[string]any for cross-module analysis.
-//  3. For each snapshotted module: TryLock → Collect → Unlock (skip if busy).
-//     Collect is called even with empty targets so modules can run housekeeping
-//     (e.g. stale temp cleanup).
+//  1. TryLock all modules; skip those whose lock is busy.
+//  2. ReadDB each locked module to build a snapshot.
+//  3. Resolve deletion targets per module. Each module's Resolve receives
+//     all other snapshots (typed as any) for cross-module analysis — e.g.,
+//     image GC checks UsedBlobIDs from the VM snapshot to protect active blobs.
+//  4. Collect targets for each snapshotted module.
+//  5. Unlock all (deferred).
 //
-// Step 3 re-acquires the lock rather than holding it from step 1 to keep
-// contention minimal. commitAndRecord validates blob existence under lock
-// before writing the index, so a deletion racing with a commit is caught
-// there and the pull retries.
+// All locks are held for the entire cycle so that the snapshot, resolve, and
+// collect phases see a consistent view. GC runs infrequently and executes
+// fast, so the extended lock hold is acceptable.
 func (o *Orchestrator) Run(ctx context.Context) error {
 	logger := log.WithFunc("gc.Run")
 
-	// Phase 1: collect each module's snapshot under lock.
-	snapshots := make(map[string]any, len(o.modules))
+	// Acquire all locks up front; hold until GC finishes.
+	var locked []runner
 	for _, m := range o.modules {
 		ok, err := m.getLocker().TryLock(ctx)
 		if err != nil || !ok {
 			logger.Warnf(ctx, "skip %s: lock busy", m.getName())
 			continue
 		}
-		snap, readErr := m.readSnapshot(ctx)
-		m.getLocker().Unlock(ctx) //nolint:errcheck
-		if readErr != nil {
-			logger.Warnf(ctx, "snapshot %s: %v", m.getName(), readErr)
+		locked = append(locked, m)
+	}
+	defer func() {
+		for _, m := range locked {
+			m.getLocker().Unlock(ctx) //nolint:errcheck
+		}
+	}()
+
+	// Phase 1: snapshot all locked modules.
+	snapshots := make(map[string]any, len(locked))
+	for _, m := range locked {
+		snap, err := m.readSnapshot(ctx)
+		if err != nil {
+			logger.Warnf(ctx, "snapshot %s: %v", m.getName(), err)
 			continue
 		}
 		snapshots[m.getName()] = snap
 	}
 
-	// Phase 2: resolve deletion targets per module (no locks held).
-	// Each module sees its own snapshot typed, others as any.
+	// Phase 2: resolve deletion targets (cross-module via snapshots).
 	targets := make(map[string][]string)
-	for _, m := range o.modules {
+	for _, m := range locked {
 		snap, ok := snapshots[m.getName()]
 		if !ok {
 			continue
@@ -69,23 +78,15 @@ func (o *Orchestrator) Run(ctx context.Context) error {
 		}
 	}
 
-	// Phase 3: collect under lock for every snapshotted module.
-	// Collect is called even with empty targets so modules can do housekeeping.
+	// Phase 3: collect.
 	var errs []string
-	for _, m := range o.modules {
+	for _, m := range locked {
 		if _, snapshotted := snapshots[m.getName()]; !snapshotted {
 			continue
 		}
-		ids := targets[m.getName()] // nil if no targets — Collect handles this
-		ok, err := m.getLocker().TryLock(ctx)
-		if err != nil || !ok {
-			logger.Warnf(ctx, "skip collect %s: lock busy", m.getName())
-			continue
-		}
-		collectErr := m.collect(ctx, ids)
-		m.getLocker().Unlock(ctx) //nolint:errcheck
-		if collectErr != nil {
-			errs = append(errs, fmt.Sprintf("%s: %v", m.getName(), collectErr))
+		ids := targets[m.getName()]
+		if err := m.collect(ctx, ids); err != nil {
+			errs = append(errs, fmt.Sprintf("%s: %v", m.getName(), err))
 		}
 	}
 	if len(errs) > 0 {

hypervisor/cloudhypervisor/cloudhypervisor.go

@@ -84,12 +84,16 @@ func (ch *CloudHypervisor) Delete(ctx context.Context, ids []string, force bool)
 				return fmt.Errorf("stop before delete: %w", err)
 			}
 		}
-		return ch.store.Update(ctx, func(idx *hypervisor.VMIndex) error {
+		if err := ch.store.Update(ctx, func(idx *hypervisor.VMIndex) error {
 			if _, ok := idx.VMs[id]; !ok {
 				return hypervisor.ErrNotFound
 			}
 			delete(idx.VMs, id)
 			return nil
-		})
+		}); err != nil {
+			return err
+		}
+		ch.cleanupRuntimeFiles(id)
+		return nil
 	})
 }

hypervisor/cloudhypervisor/create.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"os/exec"
+	"path/filepath"
 	"slices"
 	"strings"
 	"time"
@@ -31,6 +32,9 @@ func (ch *CloudHypervisor) Create(ctx context.Context, vmCfg *types.VMConfig, st
 		boot = *bootCfg
 	}
 
+	// Extract blob IDs before prepare transforms the StorageConfigs.
+	blobIDs := extractBlobIDs(storageConfigs, bootCfg)
+
 	var (
 		sc  []*types.StorageConfig
 		err error
@@ -56,6 +60,7 @@ func (ch *CloudHypervisor) Create(ctx context.Context, vmCfg *types.VMConfig, st
 		VMInfo:         info,
 		StorageConfigs: sc,
 		BootConfig:     &boot,
+		ImageBlobIDs:   blobIDs,
 	}
 
 	if err := ch.store.Update(ctx, func(idx *hypervisor.VMIndex) error {
@@ -150,3 +155,33 @@ func ReverseLayerSerials(sc []*types.StorageConfig) []string {
 	slices.Reverse(serials)
 	return serials
 }
+
+// extractBlobIDs extracts digest hexes from the original image StorageConfigs
+// and BootConfig paths. Must be called before prepare transforms them.
+func extractBlobIDs(sc []*types.StorageConfig, boot *types.BootConfig) map[string]struct{} {
+	ids := make(map[string]struct{})
+	if boot != nil && boot.KernelPath != "" {
+		// OCI: erofs layer blobs + boot dir hexes.
+		for _, s := range sc {
+			if s.RO {
+				ids[blobHexFromPath(s.Path)] = struct{}{}
+			}
+		}
+		// boot/{hex}/vmlinuz → hex
+		ids[filepath.Base(filepath.Dir(boot.KernelPath))] = struct{}{}
+		if boot.InitrdPath != "" {
+			ids[filepath.Base(filepath.Dir(boot.InitrdPath))] = struct{}{}
+		}
+	} else if len(sc) > 0 {
+		// Cloudimg: base qcow2 blob hex (before overlay replaces it).
+		ids[blobHexFromPath(sc[0].Path)] = struct{}{}
+	}
+	return ids
+}
+
+// blobHexFromPath extracts the digest hex from a blob file path.
+// e.g., "/var/lib/cocoon/oci/blobs/abc123.erofs" → "abc123"
+func blobHexFromPath(path string) string {
+	base := filepath.Base(path)
+	return strings.TrimSuffix(base, filepath.Ext(base))
+}

hypervisor/cloudhypervisor/gc.go

@@ -2,78 +2,53 @@ package cloudhypervisor
 
 import (
 	"context"
-	"errors"
-	"os"
-	"path/filepath"
 
 	"github.com/projecteru2/cocoon/gc"
 	"github.com/projecteru2/cocoon/hypervisor"
-	"github.com/projecteru2/cocoon/utils"
 )
 
 // compile-time interface check.
 var _ hypervisor.Hypervisor = (*CloudHypervisor)(nil)
 
 // chSnapshot is the typed GC snapshot for the Cloud Hypervisor backend.
+// Its sole purpose is to expose the union of all VMs' ImageBlobIDs so that
+// image GC modules can skip blobs still needed by active VMs.
 type chSnapshot struct {
-	vmIDs   map[string]struct{} // VM IDs present in the DB index
-	runDirs []string            // VM IDs that have a runtime dir on disk
-	logDirs []string            // VM IDs that have a log dir on disk
+	blobIDs map[string]struct{} // union of all VMs' ImageBlobIDs
 }
 
+// UsedBlobIDs implements gc.UsedBlobIDs.
+func (s chSnapshot) UsedBlobIDs() map[string]struct{} { return s.blobIDs }
+
 // GCModule returns a typed gc.Module[chSnapshot] for the Cloud Hypervisor backend.
 //
-// ReadDB (under lock): reads the VM index and scans the runtime/log base dirs
-// on disk, returning a snapshot of what exists vs what is recorded.
-//
-// Resolve: returns VM IDs whose runtime or log dirs are orphaned (present on
-// disk but no longer in the DB).
-//
-// Collect (under lock): removes the orphaned dirs for each VM ID.
+// ReadDB (under lock): collects all image blob IDs referenced by VMs.
+// Resolve/Collect are no-ops — VM residual cleanup is done by Delete directly.
+// The module exists purely as a data provider for cross-module GC.
 func (ch *CloudHypervisor) GCModule() gc.Module[chSnapshot] {
 	return gc.Module[chSnapshot]{
 		Name:   typ,
 		Locker: ch.locker,
 		ReadDB: func(ctx context.Context) (chSnapshot, error) {
 			var snap chSnapshot
 			if err := ch.store.Read(func(idx *hypervisor.VMIndex) error {
-				snap.vmIDs = make(map[string]struct{}, len(idx.VMs))
-				for id := range idx.VMs {
-					snap.vmIDs[id] = struct{}{}
+				snap.blobIDs = make(map[string]struct{})
+				for _, rec := range idx.VMs {
+					if rec == nil {
+						continue
+					}
+					for hex := range rec.ImageBlobIDs {
+						snap.blobIDs[hex] = struct{}{}
+					}
 				}
 				return nil
 			}); err != nil {
 				return snap, err
 			}
-			snap.runDirs = utils.ScanSubdirs(filepath.Join(ch.conf.RunDir, "cloudhypervisor"))
-			snap.logDirs = utils.ScanSubdirs(filepath.Join(ch.conf.LogDir, "cloudhypervisor"))
 			return snap, nil
 		},
-		Resolve: func(snap chSnapshot, _ map[string]any) []string {
-			unreferenced := utils.FilterUnreferenced(snap.runDirs, snap.vmIDs)
-			seen := make(map[string]struct{}, len(unreferenced))
-			for _, id := range unreferenced {
-				seen[id] = struct{}{}
-			}
-			for _, id := range utils.FilterUnreferenced(snap.logDirs, snap.vmIDs) {
-				if _, already := seen[id]; !already {
-					unreferenced = append(unreferenced, id)
-				}
-			}
-			return unreferenced
-		},
-		Collect: func(ctx context.Context, ids []string) error {
-			var errs []error
-			for _, vmID := range ids {
-				if err := os.RemoveAll(ch.conf.CHVMRunDir(vmID)); err != nil && !os.IsNotExist(err) {
-					errs = append(errs, err)
-				}
-				if err := os.RemoveAll(ch.conf.CHVMLogDir(vmID)); err != nil && !os.IsNotExist(err) {
-					errs = append(errs, err)
-				}
-			}
-			return errors.Join(errs...)
-		},
+		Resolve: func(_ chSnapshot, _ map[string]any) []string { return nil },
+		Collect: func(_ context.Context, _ []string) error { return nil },
 	}
 }
 

hypervisor/db.go

@@ -21,6 +21,11 @@ type VMRecord struct {
 	// BootConfig holds the kernel and initrd paths for direct-boot VMs.
 	// Nil for UEFI-boot VMs (cloud images).
 	BootConfig *types.BootConfig `json:"boot_config,omitempty"`
+
+	// ImageBlobIDs is the set of digest hex strings of image blobs this VM
+	// depends on. Populated at create time from the original StorageConfigs
+	// (before COW prepare). Image GC skips dangling blobs that appear here.
+	ImageBlobIDs map[string]struct{} `json:"image_blob_ids,omitempty"`
 }
 
 // VMIndex is the top-level DB structure shared by all hypervisor backends.

images/cloudimg/gc.go

@@ -31,8 +31,17 @@ func (c *CloudImg) GCModule() gc.Module[cloudimgSnapshot] {
 			snap.blobs = images.ScanBlobHexes(c.conf.CloudimgBlobsDir(), ".qcow2")
 			return snap, nil
 		},
-		Resolve: func(snap cloudimgSnapshot, _ map[string]any) []string {
-			return images.FilterUnreferenced(snap.blobs, snap.refs)
+		Resolve: func(snap cloudimgSnapshot, others map[string]any) []string {
+			used := gc.CollectUsedBlobIDs(others)
+
+			var result []string
+			for _, hex := range images.FilterUnreferenced(snap.blobs, snap.refs) {
+				if _, inUse := used[hex]; inUse {
+					continue
+				}
+				result = append(result, hex)
+			}
+			return result
 		},
 		Collect: func(ctx context.Context, ids []string) error {
 			var errs []error

images/oci/gc.go

@@ -39,19 +39,28 @@ func (o *OCI) GCModule() gc.Module[ociSnapshot] {
 			}
 			return snap, nil
 		},
-		Resolve: func(snap ociSnapshot, _ map[string]any) []string {
-			// Collect unreferenced blobs, then any boot dirs not already included.
-			unreferenced := images.FilterUnreferenced(snap.blobs, snap.refs)
-			seen := make(map[string]struct{}, len(unreferenced))
-			for _, h := range unreferenced {
-				seen[h] = struct{}{}
+		Resolve: func(snap ociSnapshot, others map[string]any) []string {
+			used := gc.CollectUsedBlobIDs(others)
+
+			dangling := images.FilterUnreferenced(snap.blobs, snap.refs)
+			seen := make(map[string]struct{}, len(dangling))
+			var result []string
+			for _, hex := range dangling {
+				if _, inUse := used[hex]; inUse {
+					continue
+				}
+				result = append(result, hex)
+				seen[hex] = struct{}{}
 			}
 			for _, hex := range images.FilterUnreferenced(snap.bootDirs, snap.refs) {
+				if _, inUse := used[hex]; inUse {
+					continue
+				}
 				if _, already := seen[hex]; !already {
-					unreferenced = append(unreferenced, hex)
+					result = append(result, hex)
 				}
 			}
-			return unreferenced
+			return result
 		},
 		Collect: func(ctx context.Context, ids []string) error {
 			var errs []error