cleanup

Author: rgarcia

cmd/api/api/devices.go

@@ -46,8 +46,12 @@ func (s *ApiService) ListAvailableDevices(ctx context.Context, request oapi.List
 
 // CreateDevice registers a new device for passthrough
 func (s *ApiService) CreateDevice(ctx context.Context, request oapi.CreateDeviceRequestObject) (oapi.CreateDeviceResponseObject, error) {
+	var name string
+	if request.Body.Name != nil {
+		name = *request.Body.Name
+	}
 	req := devices.CreateDeviceRequest{
-		Name:       request.Body.Name,
+		Name:       name,
 		PCIAddress: request.Body.PciAddress,
 	}
 
@@ -136,7 +140,7 @@ func deviceToOAPI(d devices.Device) oapi.Device {
 	deviceType := oapi.DeviceType(d.Type)
 	return oapi.Device{
 		Id:          d.Id,
-		Name:        d.Name,
+		Name:        &d.Name,
 		Type:        deviceType,
 		PciAddress:  d.PCIAddress,
 		VendorId:    d.VendorID,

lib/devices/README.md

@@ -37,7 +37,7 @@ lib/devices/
 ├── vfio.go         # VFIO bind/unbind operations
 ├── manager.go      # Manager interface and implementation
 ├── manager_test.go # Unit tests
-├── gpu_e2e_test.go # End-to-end GPU passthrough test
+├── gpu_e2e_test.go # End-to-end GPU passthrough test (auto-skips if no GPU)
 └── scripts/
     └── gpu-reset.sh  # GPU recovery script (see Troubleshooting)
 ```
@@ -285,29 +285,36 @@ Then reboot.
 
 ### Running the E2E Test
 
-The GPU passthrough E2E test requires:
-- Root permissions (for driver bind/unbind)
+The GPU passthrough E2E test **automatically detects** GPU availability and skips if prerequisites aren't met.
+
+**Why GPU tests require root**: Unlike network tests which can use Linux capabilities (`CAP_NET_ADMIN`), GPU passthrough requires writing to sysfs files (`/sys/bus/pci/drivers/*/unbind`, etc.) which are protected by standard Unix file permissions (owned by root, mode 0200). Capabilities don't bypass DAC (discretionary access control) for file writes.
+
+Prerequisites for the test to run (not skip):
+- **Root permissions** (sudo) - required for sysfs driver operations
 - NVIDIA GPU on host
-- IOMMU enabled
+- IOMMU enabled (`intel_iommu=on` or `amd_iommu=on`)
 - `vfio_pci` and `vfio_iommu_type1` modules loaded
 - `/sbin` in PATH (for `mkfs.ext4`)
 
 ```bash
-# Load VFIO modules
+# Prepare the environment
 sudo modprobe vfio_pci vfio_iommu_type1
 
-# Run the test
+# Run via make - test auto-skips if not root or no GPU
+make test
+
+# Or run directly with sudo
 sudo env PATH=$PATH:/sbin:/usr/sbin \
-  go test -v -tags=integration,gpu -run TestGPUPassthrough \
-  -timeout 5m ./lib/devices/...
+  go test -v -run TestGPUPassthrough -timeout 5m ./lib/devices/...
 ```
 
 The test will:
-1. Discover available NVIDIA GPUs
-2. Register the first GPU found
-3. Create a VM with GPU passthrough
-4. Verify the GPU is visible inside the VM
-5. Clean up (delete VM, unbind from VFIO, restore nvidia driver)
+1. Check prerequisites and skip if not met (not root, no GPU, no IOMMU, etc.)
+2. Discover available NVIDIA GPUs
+3. Register the first GPU found
+4. Create a VM with GPU passthrough
+5. Verify the GPU is visible inside the VM
+6. Clean up (delete VM, unbind from VFIO, restore nvidia driver)
 
 ## Future Plans: GPU Sharing Across Multiple VMs
 

lib/devices/gpu_e2e_test.go

@@ -1,5 +1,3 @@
-//go:build gpu
-
 package devices_test
 
 import (
@@ -25,31 +23,32 @@ import (
 
 // TestGPUPassthrough is an E2E test that verifies GPU passthrough works.
 //
-// Run with:
+// This test automatically detects GPU availability and skips if:
+//   - No NVIDIA GPU is found
+//   - IOMMU is not enabled
+//   - VFIO modules are not loaded
+//   - Not running as root
 //
-//	sudo env PATH=$PATH:/sbin:/usr/sbin go test -tags "gpu containers_image_openpgp" -v -run TestGPUPassthrough ./lib/devices/...
+// To run manually:
 //
-// Prerequisites:
-//   - NVIDIA GPU available on host (bound to nvidia driver)
-//   - IOMMU enabled (intel_iommu=on or amd_iommu=on in kernel cmdline)
-//   - vfio-pci kernel module loaded (sudo modprobe vfio_pci vfio_iommu_type1)
-//   - Root permissions (sudo) for driver bind/unbind operations
-//   - /sbin in PATH for mkfs.ext4
+//	sudo env PATH=$PATH:/sbin:/usr/sbin go test -v -run TestGPUPassthrough ./lib/devices/...
 //
 // WARNING: This test will unbind the GPU from the nvidia driver, which may
 // disrupt other processes using the GPU. The test attempts to restore the
 // nvidia driver binding on cleanup.
 func TestGPUPassthrough(t *testing.T) {
 	ctx := context.Background()
 
-	// Check prerequisites
-	checkPrerequisites(t)
-
-	// Check if running as root (needed for driver bind/unbind)
-	if os.Geteuid() != 0 {
-		t.Skip("Skipping GPU test: must run as root (sudo) to bind/unbind drivers")
+	// Auto-detect GPU availability - skip if prerequisites not met
+	skipReason := checkGPUTestPrerequisites()
+	if skipReason != "" {
+		t.Skip(skipReason)
 	}
 
+	// Log that prerequisites passed
+	groups, _ := os.ReadDir("/sys/kernel/iommu_groups")
+	t.Logf("GPU test prerequisites met: %d IOMMU groups found", len(groups))
+
 	// Setup test infrastructure
 	tmpDir := t.TempDir()
 	p := paths.New(tmpDir)
@@ -267,25 +266,51 @@ func TestGPUPassthrough(t *testing.T) {
 	t.Log("✅ GPU passthrough test PASSED!")
 }
 
-func checkPrerequisites(t *testing.T) {
-	t.Helper()
-
+// checkGPUTestPrerequisites checks if GPU passthrough test can run.
+// Returns empty string if all prerequisites are met, otherwise returns skip reason.
+func checkGPUTestPrerequisites() string {
 	// Check KVM
 	if _, err := os.Stat("/dev/kvm"); os.IsNotExist(err) {
-		t.Fatal("/dev/kvm not available - ensure KVM is enabled and user is in 'kvm' group")
+		return "GPU passthrough test requires /dev/kvm"
 	}
 
-	// Check VFIO
+	// Check VFIO modules
 	if _, err := os.Stat("/dev/vfio/vfio"); os.IsNotExist(err) {
-		t.Fatal("/dev/vfio/vfio not available - ensure vfio-pci module is loaded (modprobe vfio-pci)")
+		return "GPU passthrough test requires VFIO (modprobe vfio_pci vfio_iommu_type1)"
 	}
 
 	// Check IOMMU is enabled by looking for IOMMU groups
 	groups, err := os.ReadDir("/sys/kernel/iommu_groups")
 	if err != nil || len(groups) == 0 {
-		t.Fatal("No IOMMU groups found - ensure IOMMU is enabled (intel_iommu=on or amd_iommu=on in kernel cmdline)")
+		return "GPU passthrough test requires IOMMU (intel_iommu=on or amd_iommu=on)"
+	}
+
+	// Check for NVIDIA GPU
+	available, err := devices.DiscoverAvailableDevices()
+	if err != nil {
+		return "GPU passthrough test failed to discover devices: " + err.Error()
+	}
+
+	hasNvidiaGPU := false
+	for _, d := range available {
+		if strings.Contains(strings.ToLower(d.VendorName), "nvidia") {
+			hasNvidiaGPU = true
+			break
+		}
+	}
+	if !hasNvidiaGPU {
+		return "GPU passthrough test requires an NVIDIA GPU"
 	}
-	t.Logf("Found %d IOMMU groups", len(groups))
+
+	// GPU passthrough requires root (euid=0) for sysfs driver bind/unbind operations.
+	// Unlike network operations which can use CAP_NET_ADMIN, sysfs file writes are
+	// protected by standard Unix DAC (file permissions), not just capabilities.
+	// The files in /sys/bus/pci/drivers/ are owned by root with mode 0200.
+	if os.Geteuid() != 0 {
+		return "GPU passthrough test requires root (sudo) for sysfs driver operations"
+	}
+
+	return "" // All prerequisites met
 }
 
 func waitForInstanceReady(ctx context.Context, t *testing.T, mgr instances.Manager, id string, timeout time.Duration) error {

lib/devices/manager.go

@@ -5,7 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"os"
-	"path/filepath"
+	"strings"
 	"sync"
 	"time"
 
@@ -62,8 +62,7 @@ func (m *manager) ListDevices(ctx context.Context) ([]Device, error) {
 	m.mu.RLock()
 	defer m.mu.RUnlock()
 
-	devicesDir := m.devicesDir()
-	entries, err := os.ReadDir(devicesDir)
+	entries, err := os.ReadDir(m.paths.DevicesDir())
 	if err != nil {
 		if os.IsNotExist(err) {
 			return []Device{}, nil
@@ -98,12 +97,7 @@ func (m *manager) ListAvailableDevices(ctx context.Context) ([]AvailableDevice,
 func (m *manager) CreateDevice(ctx context.Context, req CreateDeviceRequest) (*Device, error) {
 	log := logger.FromContext(ctx)
 
-	// Validate name
-	if !ValidateDeviceName(req.Name) {
-		return nil, ErrInvalidName
-	}
-
-	// Validate PCI address format
+	// Validate PCI address format (required)
 	if !ValidatePCIAddress(req.PCIAddress) {
 		return nil, ErrInvalidPCIAddress
 	}
@@ -114,11 +108,26 @@ func (m *manager) CreateDevice(ctx context.Context, req CreateDeviceRequest) (*D
 		return nil, fmt.Errorf("get device info: %w", err)
 	}
 
+	// Generate ID
+	id := cuid2.Generate()
+
+	// Handle optional name: if not provided, generate one from PCI address
+	name := req.Name
+	if name == "" {
+		// Generate name from PCI address: 0000:a2:00.0 -> pci-0000-a2-00-0
+		name = "pci-" + strings.ReplaceAll(strings.ReplaceAll(req.PCIAddress, ":", "-"), ".", "-")
+	}
+
+	// Validate name format
+	if !ValidateDeviceName(name) {
+		return nil, ErrInvalidName
+	}
+
 	m.mu.Lock()
 	defer m.mu.Unlock()
 
 	// Check if name already exists
-	if _, err := m.findByName(req.Name); err == nil {
+	if _, err := m.findByName(name); err == nil {
 		return nil, ErrNameExists
 	}
 
@@ -127,13 +136,10 @@ func (m *manager) CreateDevice(ctx context.Context, req CreateDeviceRequest) (*D
 		return nil, ErrAlreadyExists
 	}
 
-	// Generate ID
-	id := cuid2.Generate()
-
 	// Create device
 	device := &Device{
 		Id:          id,
-		Name:        req.Name,
+		Name:        name,
 		Type:        DetermineDeviceType(deviceInfo),
 		PCIAddress:  req.PCIAddress,
 		VendorID:    deviceInfo.VendorID,
@@ -145,19 +151,19 @@ func (m *manager) CreateDevice(ctx context.Context, req CreateDeviceRequest) (*D
 	}
 
 	// Ensure directories exist
-	if err := os.MkdirAll(m.deviceDir(id), 0755); err != nil {
+	if err := os.MkdirAll(m.paths.DeviceDir(id), 0755); err != nil {
 		return nil, fmt.Errorf("create device dir: %w", err)
 	}
 
 	// Save device metadata
 	if err := m.saveDevice(device); err != nil {
-		os.RemoveAll(m.deviceDir(id))
+		os.RemoveAll(m.paths.DeviceDir(id))
 		return nil, fmt.Errorf("save device: %w", err)
 	}
 
 	log.InfoContext(ctx, "registered device",
 		"id", id,
-		"name", req.Name,
+		"name", name,
 		"pci_address", req.PCIAddress,
 		"type", device.Type,
 	)
@@ -208,7 +214,7 @@ func (m *manager) DeleteDevice(ctx context.Context, id string) error {
 	}
 
 	// Remove device directory
-	if err := os.RemoveAll(m.deviceDir(id)); err != nil {
+	if err := os.RemoveAll(m.paths.DeviceDir(id)); err != nil {
 		return fmt.Errorf("remove device dir: %w", err)
 	}
 
@@ -339,20 +345,8 @@ func (m *manager) MarkDetached(ctx context.Context, deviceID string) error {
 
 // Helper methods
 
-func (m *manager) devicesDir() string {
-	return filepath.Join(m.paths.GuestsDir(), "..", "devices")
-}
-
-func (m *manager) deviceDir(id string) string {
-	return filepath.Join(m.devicesDir(), id)
-}
-
-func (m *manager) deviceMetadataPath(id string) string {
-	return filepath.Join(m.deviceDir(id), "metadata.json")
-}
-
 func (m *manager) loadDevice(id string) (*Device, error) {
-	data, err := os.ReadFile(m.deviceMetadataPath(id))
+	data, err := os.ReadFile(m.paths.DeviceMetadata(id))
 	if err != nil {
 		if os.IsNotExist(err) {
 			return nil, ErrNotFound
@@ -374,11 +368,11 @@ func (m *manager) saveDevice(device *Device) error {
 		return err
 	}
 
-	return os.WriteFile(m.deviceMetadataPath(device.Id), data, 0644)
+	return os.WriteFile(m.paths.DeviceMetadata(device.Id), data, 0644)
 }
 
 func (m *manager) findByName(name string) (*Device, error) {
-	entries, err := os.ReadDir(m.devicesDir())
+	entries, err := os.ReadDir(m.paths.DevicesDir())
 	if err != nil {
 		return nil, ErrNotFound
 	}
@@ -402,7 +396,7 @@ func (m *manager) findByName(name string) (*Device, error) {
 }
 
 func (m *manager) findByPCIAddress(pciAddress string) (*Device, error) {
-	entries, err := os.ReadDir(m.devicesDir())
+	entries, err := os.ReadDir(m.paths.DevicesDir())
 	if err != nil {
 		return nil, ErrNotFound
 	}

lib/devices/types.go

@@ -29,8 +29,8 @@ type Device struct {
 
 // CreateDeviceRequest is the request to register a new device
 type CreateDeviceRequest struct {
-	Name       string `json:"name"`        // required: globally unique name
-	PCIAddress string `json:"pci_address"` // required: PCI address (e.g., "0000:a2:00.0")
+	Name       string `json:"name,omitempty"` // optional: globally unique name (auto-generated if not provided)
+	PCIAddress string `json:"pci_address"`    // required: PCI address (e.g., "0000:a2:00.0")
 }
 
 // AvailableDevice represents a PCI device discovered on the host