review fixes

Author: sjmiller609

cmd/api/api/instances.go

@@ -116,6 +116,38 @@ func (s *ApiService) CreateInstance(ctx context.Context, request oapi.CreateInst
 		networkEnabled = *request.Body.Network.Enabled
 	}
 
+	// Parse network bandwidth limits (0 = auto)
+	var networkBandwidthDownload int64
+	var networkBandwidthUpload int64
+	if request.Body.Network != nil {
+		if request.Body.Network.BandwidthDownload != nil && *request.Body.Network.BandwidthDownload != "" {
+			var bwBytes datasize.ByteSize
+			bwStr := *request.Body.Network.BandwidthDownload
+			bwStr = strings.TrimSuffix(bwStr, "/s")
+			bwStr = strings.TrimSuffix(bwStr, "ps")
+			if err := bwBytes.UnmarshalText([]byte(bwStr)); err != nil {
+				return oapi.CreateInstance400JSONResponse{
+					Code:    "invalid_bandwidth_download",
+					Message: fmt.Sprintf("invalid bandwidth_download format: %v", err),
+				}, nil
+			}
+			networkBandwidthDownload = int64(bwBytes)
+		}
+		if request.Body.Network.BandwidthUpload != nil && *request.Body.Network.BandwidthUpload != "" {
+			var bwBytes datasize.ByteSize
+			bwStr := *request.Body.Network.BandwidthUpload
+			bwStr = strings.TrimSuffix(bwStr, "/s")
+			bwStr = strings.TrimSuffix(bwStr, "ps")
+			if err := bwBytes.UnmarshalText([]byte(bwStr)); err != nil {
+				return oapi.CreateInstance400JSONResponse{
+					Code:    "invalid_bandwidth_upload",
+					Message: fmt.Sprintf("invalid bandwidth_upload format: %v", err),
+				}, nil
+			}
+			networkBandwidthUpload = int64(bwBytes)
+		}
+	}
+
 	// Parse devices (GPU passthrough)
 	var deviceRefs []string
 	if request.Body.Devices != nil {
@@ -163,18 +195,20 @@ func (s *ApiService) CreateInstance(ctx context.Context, request oapi.CreateInst
 	}
 
 	domainReq := instances.CreateInstanceRequest{
-		Name:           request.Body.Name,
-		Image:          request.Body.Image,
-		Size:           size,
-		HotplugSize:    hotplugSize,
-		OverlaySize:    overlaySize,
-		Vcpus:          vcpus,
-		DiskIOBps:      diskIOBps,
-		Env:            env,
-		NetworkEnabled: networkEnabled,
-		Devices:        deviceRefs,
-		Volumes:        volumes,
-		Hypervisor:     hvType,
+		Name:                     request.Body.Name,
+		Image:                    request.Body.Image,
+		Size:                     size,
+		HotplugSize:              hotplugSize,
+		OverlaySize:              overlaySize,
+		Vcpus:                    vcpus,
+		DiskIOBps:                diskIOBps,
+		NetworkBandwidthDownload: networkBandwidthDownload,
+		NetworkBandwidthUpload:   networkBandwidthUpload,
+		Env:                      env,
+		NetworkEnabled:           networkEnabled,
+		Devices:                  deviceRefs,
+		Volumes:                  volumes,
+		Hypervisor:               hvType,
 	}
 
 	inst, err := s.InstanceManager.CreateInstance(ctx, domainReq)

cmd/api/config/config.go

@@ -1,6 +1,7 @@
 package config
 
 import (
+	"fmt"
 	"os"
 	"runtime/debug"
 	"strconv"
@@ -112,6 +113,9 @@ type Config struct {
 	OversubNetwork float64 // Network oversubscription ratio
 	OversubDiskIO  float64 // Disk I/O oversubscription ratio
 
+	// Network rate limiting
+	UploadBurstMultiplier int // Multiplier for upload burst ceiling vs guaranteed rate (default: 4)
+
 	// Resource capacity limits (empty = auto-detect from host)
 	DiskLimit       string  // Hard disk limit for DataDir, e.g. "500GB"
 	NetworkLimit    string  // Hard network limit, e.g. "10Gbps" (empty = detect from uplink speed)
@@ -190,6 +194,9 @@ func Load() *Config {
 		OversubNetwork: getEnvFloat("OVERSUB_NETWORK", 2.0),
 		OversubDiskIO:  getEnvFloat("OVERSUB_DISK_IO", 2.0),
 
+		// Network rate limiting
+		UploadBurstMultiplier: getEnvInt("UPLOAD_BURST_MULTIPLIER", 4),
+
 		// Resource capacity limits (empty = auto-detect)
 		DiskLimit:       getEnv("DISK_LIMIT", ""),
 		NetworkLimit:    getEnv("NETWORK_LIMIT", ""),
@@ -233,3 +240,28 @@ func getEnvFloat(key string, defaultValue float64) float64 {
 	}
 	return defaultValue
 }
+
+// Validate checks configuration values for correctness.
+// Returns an error if any configuration value is invalid.
+func (c *Config) Validate() error {
+	// Validate oversubscription ratios are positive
+	if c.OversubCPU <= 0 {
+		return fmt.Errorf("OVERSUB_CPU must be positive, got %v", c.OversubCPU)
+	}
+	if c.OversubMemory <= 0 {
+		return fmt.Errorf("OVERSUB_MEMORY must be positive, got %v", c.OversubMemory)
+	}
+	if c.OversubDisk <= 0 {
+		return fmt.Errorf("OVERSUB_DISK must be positive, got %v", c.OversubDisk)
+	}
+	if c.OversubNetwork <= 0 {
+		return fmt.Errorf("OVERSUB_NETWORK must be positive, got %v", c.OversubNetwork)
+	}
+	if c.OversubDiskIO <= 0 {
+		return fmt.Errorf("OVERSUB_DISK_IO must be positive, got %v", c.OversubDiskIO)
+	}
+	if c.UploadBurstMultiplier < 1 {
+		return fmt.Errorf("UPLOAD_BURST_MULTIPLIER must be >= 1, got %v", c.UploadBurstMultiplier)
+	}
+	return nil
+}

cmd/api/main.go

@@ -182,7 +182,6 @@ func run() error {
 	if err := app.NetworkManager.SetupHTB(app.Ctx, networkCapacity); err != nil {
 		logger.Warn("failed to setup HTB on bridge (network rate limiting disabled)", "error", err)
 	}
-	logger.Info("Set up HTB qdisc on bridge for network fair sharing")
 
 	// Reconcile device state (clears orphaned attachments from crashed VMs)
 	// Set up liveness checker so device reconciliation can accurately detect orphaned attachments

lib/instances/create.go

@@ -336,7 +336,7 @@ func (m *manager) createInstance(
 			InstanceName:  req.Name,
 			DownloadBps:   stored.NetworkBandwidthDownload,
 			UploadBps:     stored.NetworkBandwidthUpload,
-			UploadCeilBps: stored.NetworkBandwidthUpload * 2, // Allow burst to 2x guaranteed rate
+			UploadCeilBps: stored.NetworkBandwidthUpload * int64(m.networkManager.GetUploadBurstMultiplier()),
 		})
 		if err != nil {
 			log.ErrorContext(ctx, "failed to allocate network", "instance_id", id, "network", networkName, "error", err)

lib/network/README.md

@@ -252,7 +252,7 @@ Network bandwidth is limited separately for download and upload directions:
 When not specified in the create request:
 - Both download and upload = `(vcpus / cpu_capacity) * network_capacity`
 - Symmetric by default
-- Upload ceiling = 2x guaranteed rate (allows bursting)
+- Upload ceiling = 4x guaranteed rate (configurable via `UPLOAD_BURST_MULTIPLIER`)
 
 Note: In case of unexpected scenarios like power loss, straggler TAP devices may persist until manual cleanup or host reboot.
 

lib/network/allocate.go

@@ -111,8 +111,8 @@ func (m *manager) RecreateAllocation(ctx context.Context, instanceID string, dow
 	}
 
 	// 3. Recreate TAP device with same name and rate limits from instance metadata
-	// Use uploadBps as ceiling too (same as guaranteed rate on restore)
-	if err := m.createTAPDevice(alloc.TAPDevice, network.Bridge, network.Isolated, downloadBps, uploadBps, uploadBps); err != nil {
+	uploadCeilBps := uploadBps * int64(m.GetUploadBurstMultiplier())
+	if err := m.createTAPDevice(alloc.TAPDevice, network.Bridge, network.Isolated, downloadBps, uploadBps, uploadCeilBps); err != nil {
 		return fmt.Errorf("create TAP device: %w", err)
 	}
 	m.recordTAPOperation(ctx, "create")

lib/network/manager.go

@@ -30,6 +30,9 @@ type Manager interface {
 	GetAllocation(ctx context.Context, instanceID string) (*Allocation, error)
 	ListAllocations(ctx context.Context) ([]Allocation, error)
 	NameExists(ctx context.Context, name string) (bool, error)
+
+	// GetUploadBurstMultiplier returns the configured multiplier for upload burst ceiling.
+	GetUploadBurstMultiplier() int
 }
 
 // manager implements the Manager interface
@@ -123,3 +126,12 @@ func (m *manager) getDefaultNetwork(ctx context.Context) (*Network, error) {
 func (m *manager) SetupHTB(ctx context.Context, capacityBps int64) error {
 	return m.setupBridgeHTB(ctx, m.config.BridgeName, capacityBps)
 }
+
+// GetUploadBurstMultiplier returns the configured multiplier for upload burst ceiling.
+// Defaults to 4 if not configured.
+func (m *manager) GetUploadBurstMultiplier() int {
+	if m.config.UploadBurstMultiplier < 1 {
+		return DefaultUploadBurstMultiplier
+	}
+	return m.config.UploadBurstMultiplier
+}

lib/network/types.go

@@ -2,6 +2,12 @@ package network
 
 import "time"
 
+// DefaultUploadBurstMultiplier is the default multiplier applied to guaranteed upload rate
+// to calculate the burst ceiling. This allows VMs to burst above their guaranteed rate
+// when other VMs are not using their full bandwidth allocation.
+// Configurable via UPLOAD_BURST_MULTIPLIER environment variable.
+const DefaultUploadBurstMultiplier = 4
+
 // Network represents a virtual network for instances
 type Network struct {
 	Name      string // "default", "internal"

lib/resources/README.md

@@ -57,7 +57,7 @@ Bidirectional rate limiting with separate download and upload controls:
 **Default limits:**
 - Proportional to CPU: `(vcpus / cpu_capacity) * network_capacity`
 - Symmetric download/upload by default
-- Upload ceiling = 2x guaranteed rate (allows bursting when bandwidth available)
+- Upload ceiling = 4x guaranteed rate by default (configurable via `UPLOAD_BURST_MULTIPLIER`)
 
 **Capacity tracking:**
 - Uses max(download, upload) per instance since they share physical link

lib/resources/cpu.go

@@ -3,6 +3,7 @@ package resources
 import (
 	"bufio"
 	"context"
+	"fmt"
 	"os"
 	"strconv"
 	"strings"
@@ -63,7 +64,7 @@ func (c *CPUResource) Allocated(ctx context.Context) (int64, error) {
 func detectCPUCapacity() (int64, error) {
 	file, err := os.Open("/proc/cpuinfo")
 	if err != nil {
-		return 0, err
+		return 0, fmt.Errorf("open /proc/cpuinfo: %w", err)
 	}
 	defer file.Close()