address code reviews

Author: sjmiller609

lib/hypervisor/cloudhypervisor/cloudhypervisor.go

@@ -5,15 +5,15 @@ package cloudhypervisor
 import (
 	"context"
 	"fmt"
+	"time"
 
 	"github.com/onkernel/hypeman/lib/hypervisor"
 	"github.com/onkernel/hypeman/lib/vmm"
 )
 
 // CloudHypervisor implements hypervisor.Hypervisor for Cloud Hypervisor VMM.
 type CloudHypervisor struct {
-	client     *vmm.VMM
-	socketPath string
+	client *vmm.VMM
 }
 
 // New creates a new Cloud Hypervisor client for an existing VMM socket.
@@ -23,8 +23,7 @@ func New(socketPath string) (*CloudHypervisor, error) {
 		return nil, fmt.Errorf("create vmm client: %w", err)
 	}
 	return &CloudHypervisor{
-		client:     client,
-		socketPath: socketPath,
+		client: client,
 	}, nil
 }
 
@@ -114,7 +113,10 @@ func (c *CloudHypervisor) GetVMInfo(ctx context.Context) (*hypervisor.VMInfo, er
 		return nil, fmt.Errorf("unknown vm state: %s", resp.JSON200.State)
 	}
 
-	return &hypervisor.VMInfo{State: state}, nil
+	return &hypervisor.VMInfo{
+		State:            state,
+		MemoryActualSize: resp.JSON200.MemoryActualSize,
+	}, nil
 }
 
 // Pause suspends VM execution.
@@ -185,6 +187,59 @@ func (c *CloudHypervisor) ResizeMemory(ctx context.Context, bytes int64) error {
 	return nil
 }
 
+// ResizeMemoryAndWait changes the VM's memory allocation and waits for it to stabilize.
+// It polls until the actual memory size stabilizes (stops changing) or timeout is reached.
+func (c *CloudHypervisor) ResizeMemoryAndWait(ctx context.Context, bytes int64, timeout time.Duration) error {
+	// First, request the resize
+	if err := c.ResizeMemory(ctx, bytes); err != nil {
+		return err
+	}
+
+	// Poll until memory stabilizes
+	const pollInterval = 20 * time.Millisecond
+	deadline := time.Now().Add(timeout)
+
+	var lastSize int64 = -1
+	stableCount := 0
+	const requiredStableChecks = 3 // Require 3 consecutive stable readings
+
+	for time.Now().Before(deadline) {
+		select {
+		case <-ctx.Done():
+			return ctx.Err()
+		default:
+		}
+
+		info, err := c.GetVMInfo(ctx)
+		if err != nil {
+			return fmt.Errorf("poll memory size: %w", err)
+		}
+
+		if info.MemoryActualSize == nil {
+			// No memory info available, just return after resize
+			return nil
+		}
+
+		currentSize := *info.MemoryActualSize
+
+		if currentSize == lastSize {
+			stableCount++
+			if stableCount >= requiredStableChecks {
+				// Memory has stabilized
+				return nil
+			}
+		} else {
+			stableCount = 0
+			lastSize = currentSize
+		}
+
+		time.Sleep(pollInterval)
+	}
+
+	// Timeout reached, but resize was requested successfully
+	return nil
+}
+
 func ptr[T any](v T) *T {
 	return &v
 }

lib/hypervisor/cloudhypervisor/config.go

@@ -40,7 +40,7 @@ func ToVMConfig(cfg hypervisor.VMConfig) vmm.VmConfig {
 	}
 
 	// Disk configuration
-	var disks []vmm.DiskConfig
+	disks := make([]vmm.DiskConfig, 0, len(cfg.Disks))
 	for _, d := range cfg.Disks {
 		disk := vmm.DiskConfig{
 			Path: ptr(d.Path),

lib/hypervisor/config.go

@@ -55,7 +55,8 @@ type NetworkConfig struct {
 
 // VMInfo contains current VM state information
 type VMInfo struct {
-	State VMState
+	State            VMState
+	MemoryActualSize *int64 // Current actual memory size in bytes (if available)
 }
 
 // VMState represents the VM execution state

lib/hypervisor/hypervisor.go

@@ -5,6 +5,7 @@ package hypervisor
 
 import (
 	"context"
+	"time"
 
 	"github.com/onkernel/hypeman/lib/paths"
 )
@@ -59,6 +60,11 @@ type Hypervisor interface {
 	// Check Capabilities().SupportsHotplugMemory before calling.
 	ResizeMemory(ctx context.Context, bytes int64) error
 
+	// ResizeMemoryAndWait changes the VM's memory allocation and waits for it to stabilize.
+	// This polls until the actual memory size matches the target or stabilizes.
+	// Check Capabilities().SupportsHotplugMemory before calling.
+	ResizeMemoryAndWait(ctx context.Context, bytes int64, timeout time.Duration) error
+
 	// Capabilities returns what features this hypervisor supports.
 	Capabilities() Capabilities
 }

lib/instances/README.md

@@ -40,10 +40,11 @@ Manages VM instance lifecycle using Cloud Hypervisor.
       metadata.json             # State, versions, timestamps
       overlay.raw               # 50GB sparse writable overlay
       config.erofs              # Compressed config disk
-      ch.sock                   # Cloud Hypervisor API socket
-      ch-stdout.log             # CH process output
+      cloud-hypervisor.sock     # Hypervisor API socket (named after hypervisor type)
       logs/
-        console.log             # Serial console (VM output)
+        app.log                 # Guest application log (serial console output)
+        vmm.log                 # Hypervisor log (stdout+stderr)
+        hypeman.log             # Hypeman operations log
       snapshots/
         snapshot-latest/        # Snapshot directory
           config.json           # VM configuration

lib/instances/create.go

@@ -273,7 +273,7 @@ func (m *manager) createInstance(
 		KernelVersion:     string(kernelVer),
 		HypervisorType:    hypervisor.TypeCloudHypervisor,
 		HypervisorVersion: string(vmm.V49_0), // Use latest
-		SocketPath:        m.paths.InstanceSocket(id),
+		SocketPath:        m.paths.InstanceSocket(id, string(hypervisor.TypeCloudHypervisor)),
 		DataDir:           m.paths.InstanceDir(id),
 		VsockCID:          vsockCID,
 		VsockSocket:       vsockSocket,

lib/instances/standby.go

@@ -72,9 +72,10 @@ func (m *manager) standbyInstance(
 	}
 
 	// 6. Reduce memory to base size (virtio-mem hotplug) if supported
+	// Wait for memory to stabilize so the snapshot is as small as possible
 	if hv.Capabilities().SupportsHotplugMemory {
 		log.DebugContext(ctx, "reducing VM memory before snapshot", "instance_id", id, "base_size", inst.Size)
-		if err := hv.ResizeMemory(ctx, inst.Size); err != nil {
+		if err := hv.ResizeMemoryAndWait(ctx, inst.Size, 5*time.Second); err != nil {
 			// Log warning but continue - snapshot will just be larger
 			log.WarnContext(ctx, "failed to reduce memory, snapshot will be larger", "instance_id", id, "error", err)
 		}

lib/instances/storage.go

@@ -11,16 +11,16 @@ import (
 
 // Filesystem structure:
 // {dataDir}/guests/{instance-id}/
-//   metadata.json      # Instance metadata
-//   overlay.raw        # Configurable sparse overlay disk (default 10GB)
-//   config.ext4        # Read-only config disk (generated)
-//   ch.sock            # Cloud Hypervisor API socket
+//   metadata.json           # Instance metadata
+//   overlay.raw             # Configurable sparse overlay disk (default 10GB)
+//   config.ext4             # Read-only config disk (generated)
+//   cloud-hypervisor.sock   # Hypervisor API socket (named after hypervisor type)
 //   logs/
-//     app.log          # Guest application log (serial console output)
-//     vmm.log          # Cloud Hypervisor VMM log (stdout+stderr combined)
-//     hypeman.log      # Hypeman operations log (actions taken on this instance)
+//     app.log               # Guest application log (serial console output)
+//     vmm.log               # Hypervisor log (stdout+stderr combined)
+//     hypeman.log           # Hypeman operations log (actions taken on this instance)
 //   snapshots/
-//     snapshot-latest/ # Snapshot directory
+//     snapshot-latest/      # Snapshot directory
 //       config.json
 //       memory-ranges
 

lib/network/derive.go

@@ -16,6 +16,7 @@ import (
 type instanceMetadata struct {
 	Name           string
 	NetworkEnabled bool
+	HypervisorType string
 }
 
 // deriveAllocation derives network allocation from CH or snapshot
@@ -48,7 +49,7 @@ func (m *manager) deriveAllocation(ctx context.Context, instanceID string) (*All
 	netmask := fmt.Sprintf("%d.%d.%d.%d", ipNet.Mask[0], ipNet.Mask[1], ipNet.Mask[2], ipNet.Mask[3])
 
 	// 4. Try to derive from running VM first
-	socketPath := m.paths.InstanceSocket(instanceID)
+	socketPath := m.paths.InstanceSocket(instanceID, meta.HypervisorType)
 	if fileExists(socketPath) {
 		client, err := vmm.NewVMM(socketPath)
 		if err == nil {

lib/paths/paths.go

@@ -144,9 +144,9 @@ func (p *Paths) InstanceVolumeOverlaysDir(instanceID string) string {
 	return filepath.Join(p.InstanceDir(instanceID), "vol-overlays")
 }
 
-// InstanceSocket returns the path to instance API socket.
-func (p *Paths) InstanceSocket(id string) string {
-	return filepath.Join(p.InstanceDir(id), "ch.sock")
+// InstanceSocket returns the path to instance API socket for the given hypervisor type.
+func (p *Paths) InstanceSocket(id string, hypervisorType string) string {
+	return filepath.Join(p.InstanceDir(id), hypervisorType+".sock")
 }
 
 // InstanceVsockSocket returns the path to instance vsock socket.