diff --git a/cmd/api/api/instances.go b/cmd/api/api/instances.go index 3e13f080c..a4174e114 100644 --- a/cmd/api/api/instances.go +++ b/cmd/api/api/instances.go @@ -343,7 +343,16 @@ func (s *ApiService) CreateInstance(ctx context.Context, request oapi.CreateInst inst, err := s.InstanceManager.CreateInstance(ctx, domainReq) if err != nil { + var vgpuPending *instances.VGPUCleanupPendingError switch { + // Checked first: it wraps the original create error, so a later + // errors.Is case would match the cause and hide the retained instance. + case errors.As(err, &vgpuPending): + log.ErrorContext(ctx, "failed to create instance", "error", err, "image", request.Body.Image) + return oapi.CreateInstance500JSONResponse{ + Code: "vgpu_cleanup_pending", + Message: fmt.Sprintf("failed to create instance; vGPU release failed during rollback and instance %s retains the assignment, delete it to retry", vgpuPending.InstanceID), + }, nil case errors.Is(err, instances.ErrImageNotReady): return oapi.CreateInstance400JSONResponse{ Code: "image_not_ready", diff --git a/cmd/api/api/instances_test.go b/cmd/api/api/instances_test.go index f37ebfbe9..4f19d6fe2 100644 --- a/cmd/api/api/instances_test.go +++ b/cmd/api/api/instances_test.go @@ -16,6 +16,7 @@ import ( "github.com/kernel/hypeman/lib/instances" "github.com/kernel/hypeman/lib/instances/phasetracking" mw "github.com/kernel/hypeman/lib/middleware" + "github.com/kernel/hypeman/lib/network" "github.com/kernel/hypeman/lib/oapi" "github.com/kernel/hypeman/lib/paths" restartpolicy "github.com/kernel/hypeman/lib/restart-policy" @@ -46,6 +47,36 @@ func TestGetInstance_NotFound(t *testing.T) { require.Error(t, err) } +type createErrorInstanceManager struct { + instances.Manager + err error +} + +func (m createErrorInstanceManager) CreateInstance(context.Context, instances.CreateInstanceRequest) (*instances.Instance, error) { + return nil, m.err +} + +// A retained-assignment error must win over the mapping of the create error +// it wraps, or the response omits the instance the caller has to delete. +func TestCreateInstance_VGPUCleanupPendingBeatsWrappedErrorMapping(t *testing.T) { + t.Parallel() + svc := newTestService(t) + svc.InstanceManager = createErrorInstanceManager{err: &instances.VGPUCleanupPendingError{ + InstanceID: "inst-1", + Err: network.ErrNameExists, + }} + + resp, err := svc.CreateInstance(ctx(), oapi.CreateInstanceRequestObject{ + Body: &oapi.CreateInstanceRequest{Image: "test-image"}, + }) + require.NoError(t, err) + + pending, ok := resp.(oapi.CreateInstance500JSONResponse) + require.True(t, ok, "expected 500 vgpu_cleanup_pending, got %T", resp) + assert.EqualValues(t, "vgpu_cleanup_pending", pending.Code) + assert.Contains(t, pending.Message, "inst-1") +} + func TestCreateInstance_AutoPullImage(t *testing.T) { t.Parallel() if _, err := os.Stat("/dev/kvm"); os.IsNotExist(err) { diff --git a/cmd/api/main.go b/cmd/api/main.go index 98d7be720..b425eaa20 100644 --- a/cmd/api/main.go +++ b/cmd/api/main.go @@ -172,6 +172,28 @@ func configureUFFDGraduationController(cfg *config.Config, instanceManager insta }, logger), nil } +func liveInstanceVGPUDevicePaths(ctx context.Context, instanceManager instances.Manager) (map[string]struct{}, error) { + allInstances, err := instanceManager.ListInstancesForReconcile(ctx) + if err != nil { + return nil, err + } + protected := make(map[string]struct{}) + for _, inst := range allInstances { + if inst.GPUDevicePath == "" { + continue + } + // A nil PID does not mean the assignment is orphaned: the PID is + // persisted only after the hypervisor starts, so a crash during boot + // leaves the device path without one. Only skip protection when the + // recorded hypervisor is known to be gone. + if inst.HypervisorPID != nil && !instances.HypervisorProcessExists(*inst.HypervisorPID, inst.SocketPath) { + continue + } + protected[inst.GPUDevicePath] = struct{}{} + } + return protected, nil +} + func run() error { // Load config early for OTel initialization // Config path can be specified via CONFIG_PATH env var or defaults to platform-specific locations @@ -362,11 +384,16 @@ func run() error { return fmt.Errorf("reconcile device state: %w", err) } - // Reconcile mdev devices (clears orphaned vGPUs from previous runs) - logger.Info("Reconciling mdev devices...") - if err := devices.ReconcileMdevs(app.Ctx, nil); err != nil { - // Log but don't fail - mdev cleanup is best-effort - logger.Warn("failed to reconcile mdev devices", "error", err) + // Reconcile vGPU devices (clears orphaned vGPUs from previous runs) + logger.Info("Reconciling vGPU devices...") + protected, err := liveInstanceVGPUDevicePaths(app.Ctx, app.InstanceManager) + if err != nil { + logger.Warn("failed to list instances for vGPU reconcile protection; skipping vendor VFIO reconciliation", "error", err) + protected = nil + } + if err := devices.ReconcileVGPUs(app.Ctx, protected); err != nil { + // Log but don't fail - vGPU cleanup is best-effort + logger.Warn("failed to reconcile vGPU devices", "error", err) } // Wire up resource validator for aggregate limit checking diff --git a/cmd/api/main_test.go b/cmd/api/main_test.go index 34dbba428..573a404c1 100644 --- a/cmd/api/main_test.go +++ b/cmd/api/main_test.go @@ -2,15 +2,18 @@ package main import ( "bytes" + "context" "net/http" "net/http/httptest" "net/url" + "os/exec" "testing" "time" "github.com/getkin/kin-openapi/openapi3filter" "github.com/go-chi/chi/v5" "github.com/golang-jwt/jwt/v5" + "github.com/kernel/hypeman/lib/instances" mw "github.com/kernel/hypeman/lib/middleware" "github.com/kernel/hypeman/lib/oapi" nethttpmiddleware "github.com/oapi-codegen/nethttp-middleware" @@ -338,3 +341,30 @@ func TestImageNameWithSlashes_URLEncoding(t *testing.T) { }) } } + +type vgpuReconcileManagerStub struct { + instances.Manager + list []instances.Instance +} + +func (s vgpuReconcileManagerStub) ListInstancesForReconcile(context.Context) ([]instances.Instance, error) { + return s.list, nil +} + +// The hypervisor PID is persisted only after boot, so an assignment without +// one may belong to a VM that is still starting and must stay protected. +func TestLiveInstanceVGPUDevicePathsProtectsAssignmentsWithoutPID(t *testing.T) { + dead := exec.Command("true") + require.NoError(t, dead.Run()) + deadPID := dead.Process.Pid + + manager := vgpuReconcileManagerStub{list: []instances.Instance{ + {StoredMetadata: instances.StoredMetadata{Id: "booting", GPUDevicePath: "/sys/bus/pci/devices/0000:82:00.4"}}, + {StoredMetadata: instances.StoredMetadata{Id: "dead", GPUDevicePath: "/sys/bus/pci/devices/0000:82:00.5", HypervisorPID: &deadPID}}, + }} + + protected, err := liveInstanceVGPUDevicePaths(context.Background(), manager) + require.NoError(t, err) + assert.Contains(t, protected, "/sys/bus/pci/devices/0000:82:00.4") + assert.NotContains(t, protected, "/sys/bus/pci/devices/0000:82:00.5") +} diff --git a/integration/vgpu_test.go b/integration/vgpu_test.go index 12861a6a7..1f1a82776 100644 --- a/integration/vgpu_test.go +++ b/integration/vgpu_test.go @@ -4,6 +4,8 @@ import ( "bytes" "context" "os" + "path/filepath" + "strings" "testing" "time" @@ -21,21 +23,23 @@ import ( "github.com/stretchr/testify/require" ) -// TestVGPU is an integration test that verifies vGPU (SR-IOV mdev) support works. +// TestVGPU is an integration test that verifies vGPU (SR-IOV) support works +// on the host's framework: mdev or NVIDIA's vendor-specific VFIO. // // This test automatically detects vGPU availability and skips if: -// - No SR-IOV VFs are found in /sys/class/mdev_bus/ +// - No vGPU framework (mdev or vendor VFIO) is discovered // - No vGPU profiles are available -// - Not running as root (required for mdev creation) +// - Not running as root (required for sysfs vGPU assignment) // - KVM is not available // // To run manually: // // sudo go test -v -run TestVGPU -timeout 5m ./integration/... // -// Note: This test verifies mdev creation and PCI device visibility inside the VM. -// It does NOT test nvidia-smi or CUDA functionality since that requires NVIDIA -// guest drivers pre-installed in the image. +// Note: This test verifies vGPU assignment, release on stop, reacquisition on +// start, and PCI device visibility inside the VM. It does NOT test nvidia-smi +// or CUDA functionality since that requires NVIDIA guest drivers pre-installed +// in the image. func TestVGPU(t *testing.T) { t.Parallel() if testing.Short() { @@ -84,9 +88,15 @@ func TestVGPU(t *testing.T) { // Cleanup any orphaned instances and mdevs t.Cleanup(func() { - if instanceID != "" { - t.Log("Cleanup: Deleting instance...") - instanceManager.DeleteInstance(ctx, instanceID) + if instanceID == "" { + return + } + if _, err := instanceManager.StopInstance(ctx, instanceID); err != nil { + t.Logf("Cleanup: stop instance: %v", err) + } + t.Log("Cleanup: Deleting instance...") + if err := instanceManager.DeleteInstance(ctx, instanceID); err != nil { + t.Errorf("cleanup: delete instance: %v", err) } }) @@ -159,9 +169,18 @@ func TestVGPU(t *testing.T) { instanceID = inst.Id t.Logf("Instance created: %s", inst.Id) - // Verify mdev UUID was assigned - require.NotEmpty(t, inst.GPUMdevUUID, "Instance should have mdev UUID assigned") - t.Logf("mdev UUID: %s", inst.GPUMdevUUID) + // Verify the assignment matches the host's framework + require.NotEmpty(t, inst.GPUDevicePath, "Instance should have a vGPU device path assigned") + switch inst.GPUFramework { + case devices.VGPUFrameworkMdev: + require.NotEmpty(t, inst.GPUMdevUUID, "mdev instance should have a UUID assigned") + t.Logf("mdev UUID: %s", inst.GPUMdevUUID) + case devices.VGPUFrameworkVendorVFIO: + require.Empty(t, inst.GPUMdevUUID, "vendor VFIO instance should not have an mdev UUID") + t.Logf("vendor VFIO VF: %s", inst.GPUDevicePath) + default: + t.Fatalf("unexpected vGPU framework %q", inst.GPUFramework) + } // Step 5: Check GPU resources AFTER creating instance t.Run("ResourcesDecrementedAfterCreation", func(t *testing.T) { @@ -180,12 +199,9 @@ func TestVGPU(t *testing.T) { assert.Less(t, availableAfter, availableBefore, "available instances should decrease after creating VM") }) - // Step 6: Verify mdev was created in sysfs - t.Run("MdevCreated", func(t *testing.T) { - mdevPath := "/sys/bus/mdev/devices/" + inst.GPUMdevUUID - _, err := os.Stat(mdevPath) - assert.NoError(t, err, "mdev device should exist at %s", mdevPath) - t.Logf("mdev exists at: %s", mdevPath) + // Step 6: Verify the assignment exists in sysfs + t.Run("VGPUAssignedInSysfs", func(t *testing.T) { + assertVGPUAssigned(t, inst.GPUFramework, inst.GPUDevicePath) }) // Step 7: Wait for guest agent to be ready @@ -225,13 +241,68 @@ func TestVGPU(t *testing.T) { require.NoError(t, err) assert.Equal(t, profile, actualInst.GPUProfile, "GPU profile should match") - assert.NotEmpty(t, actualInst.GPUMdevUUID, "mdev UUID should be set") - t.Logf("Instance GPU: profile=%s, mdev=%s", actualInst.GPUProfile, actualInst.GPUMdevUUID) + assert.Equal(t, inst.GPUFramework, actualInst.GPUFramework, "framework should match") + assert.NotEmpty(t, actualInst.GPUDevicePath, "device path should be set") + if inst.GPUFramework == devices.VGPUFrameworkMdev { + assert.NotEmpty(t, actualInst.GPUMdevUUID, "mdev UUID should be set") + } + t.Logf("Instance GPU: profile=%s, framework=%s, device=%s", actualInst.GPUProfile, actualInst.GPUFramework, actualInst.GPUDevicePath) + }) + + t.Log("Step 10: Stopping instance...") + _, err = instanceManager.StopInstance(ctx, inst.Id) + require.NoError(t, err, "stop should succeed") + + t.Run("VGPUReleasedOnStop", func(t *testing.T) { + stopped, err := instanceManager.GetInstance(ctx, inst.Id) + require.NoError(t, err) + assert.Empty(t, stopped.GPUDevicePath, "assignment metadata should be cleared on stop") + assertVGPUReleased(t, inst.GPUFramework, inst.GPUDevicePath) + }) + + t.Log("Step 11: Starting instance to reacquire a vGPU...") + started, err := instanceManager.StartInstance(ctx, inst.Id, instances.StartInstanceRequest{}) + require.NoError(t, err, "start should succeed") + + t.Run("VGPUReacquiredOnStart", func(t *testing.T) { + require.NotEmpty(t, started.GPUDevicePath, "start should assign a vGPU") + assert.Equal(t, inst.GPUFramework, started.GPUFramework, "framework should match") + assertVGPUAssigned(t, started.GPUFramework, started.GPUDevicePath) }) t.Log("✅ vGPU test PASSED!") } +func assertVGPUAssigned(t *testing.T, framework devices.VGPUFramework, devicePath string) { + t.Helper() + switch framework { + case devices.VGPUFrameworkMdev: + _, err := os.Stat(devicePath) + assert.NoError(t, err, "mdev device should exist at %s", devicePath) + case devices.VGPUFrameworkVendorVFIO: + data, err := os.ReadFile(filepath.Join(devicePath, "nvidia", "current_vgpu_type")) + require.NoError(t, err, "VF should expose current_vgpu_type") + assert.NotEqual(t, "0", strings.TrimSpace(string(data)), "VF should have a vGPU type assigned") + default: + t.Fatalf("unexpected vGPU framework %q", framework) + } +} + +func assertVGPUReleased(t *testing.T, framework devices.VGPUFramework, devicePath string) { + t.Helper() + switch framework { + case devices.VGPUFrameworkMdev: + _, err := os.Stat(devicePath) + assert.True(t, os.IsNotExist(err), "mdev device should be gone from %s", devicePath) + case devices.VGPUFrameworkVendorVFIO: + data, err := os.ReadFile(filepath.Join(devicePath, "nvidia", "current_vgpu_type")) + require.NoError(t, err, "VF should expose current_vgpu_type") + assert.Equal(t, "0", strings.TrimSpace(string(data)), "VF assignment should be released") + default: + t.Fatalf("unexpected vGPU framework %q", framework) + } +} + // checkVGPUTestPrerequisites checks if vGPU test can run. // Returns (skipReason, profileName) - skipReason is empty if all prerequisites are met. func checkVGPUTestPrerequisites() (string, string) { @@ -245,10 +316,13 @@ func checkVGPUTestPrerequisites() (string, string) { return "vGPU test requires root (sudo) for mdev creation", "" } - // Check for vGPU mode (SR-IOV VFs present) - mode := devices.DetectHostGPUMode() - if mode != devices.GPUModeVGPU { - return "vGPU test requires SR-IOV VFs in /sys/class/mdev_bus/", "" + // Check for a vGPU framework (mdev or vendor VFIO) + framework, _, err := devices.DiscoverVGPU() + if err != nil { + return "vGPU test failed to discover vGPU framework: " + err.Error(), "" + } + if framework == devices.VGPUFrameworkNone { + return "vGPU test requires SR-IOV VFs with an mdev or vendor VFIO vGPU framework", "" } // Check for available profiles diff --git a/lib/builds/manager_test.go b/lib/builds/manager_test.go index dfdc9fca3..14ccf8c2e 100644 --- a/lib/builds/manager_test.go +++ b/lib/builds/manager_test.go @@ -51,6 +51,10 @@ func (m *mockInstanceManager) ListInstances(ctx context.Context, filter *instanc return result, nil } +func (m *mockInstanceManager) ListInstancesForReconcile(ctx context.Context) ([]instances.Instance, error) { + return m.ListInstances(ctx, nil) +} + func (m *mockInstanceManager) ListSnapshots(ctx context.Context, filter *instances.ListSnapshotsFilter) ([]instances.Snapshot, error) { return nil, nil } diff --git a/lib/devices/GPU.md b/lib/devices/GPU.md index 54a19c472..2fc27448c 100644 --- a/lib/devices/GPU.md +++ b/lib/devices/GPU.md @@ -8,16 +8,17 @@ hypeman supports two GPU modes, automatically detected based on host configurati | Mode | Description | Use Case | |------|-------------|----------| -| **vGPU (SR-IOV)** | Virtual GPUs via mdev on SR-IOV VFs | Multi-tenant, shared GPU resources | +| **vGPU (SR-IOV)** | Virtual GPUs on SR-IOV VFs via mdev or vendor VFIO | Multi-tenant, shared GPU resources | | **Passthrough** | Whole GPU VFIO passthrough | Dedicated GPU per instance | The host's GPU mode is determined by the host driver configuration: -- If `/sys/class/mdev_bus/` contains VFs → vGPU mode -- If NVIDIA GPUs are available for VFIO → passthrough mode +- If `/sys/class/mdev_bus/` contains VFs → mdev vGPU mode +- If VFs expose `/sys/bus/pci/devices//nvidia/current_vgpu_type` → vendor VFIO vGPU mode +- If NVIDIA GPUs are available for whole-device VFIO → passthrough mode ## vGPU Mode (Recommended) -vGPU mode uses NVIDIA's SR-IOV technology to create Virtual Functions (VFs), each capable of hosting an mdev (mediated device) representing a vGPU. +vGPU mode uses NVIDIA's SR-IOV technology to create Virtual Functions (VFs). Hosts on older kernels represent each vGPU as an mdev. Hosts using NVIDIA's vendor VFIO framework assign the profile directly to the VF through `current_vgpu_type`. ### How It Works @@ -74,7 +75,7 @@ curl -X POST http://localhost:4973/instances \ }' ``` -The response includes the assigned mdev UUID: +On an mdev host, the response also includes the assigned mdev UUID: ```json { @@ -87,19 +88,16 @@ The response includes the assigned mdev UUID: } ``` -### Ephemeral mdev Lifecycle +### Ephemeral vGPU Lifecycle -mdev devices are **ephemeral**: created on instance start, destroyed on instance delete. +vGPU assignments are created on instance start and released on stop or delete. Hypeman creates/removes an mdev on mdev hosts and writes the profile ID/`0` to `current_vgpu_type` on vendor VFIO hosts. ``` -Instance Create → Create mdev → Attach to VM → Instance Running -Instance Delete → Stop VM → Destroy mdev → VF available again +Instance Create → Assign profile to VF → Attach VF to VM → Instance Running +Instance Stop/Delete → Release profile → VF available again ``` -This ensures: -- **Security**: No VRAM data leakage between instances -- **Clean state**: Fresh vGPU for each instance -- **Automatic cleanup**: Orphaned mdevs cleaned up on server restart +Hypeman reconciles orphaned assignments on server restart while preserving devices held open by a running VMM. ## Passthrough Mode @@ -235,13 +233,29 @@ To upgrade the NVIDIA driver version: - Run GPU passthrough E2E tests - Verify with real CUDA workloads (e.g., ollama inference) +## Rolling Back vGPU Changes + +Before downgrading Hypeman or the host to a version that does not support the active vGPU framework: + +1. Stop or delete all vGPU instances while the current Hypeman version can release their assignments. +2. Confirm `/resources` reports `used_slots: 0`. +3. Confirm no assignments remain in either framework: + ```bash + test -z "$(find /sys/bus/mdev/devices -mindepth 1 -maxdepth 1 2>/dev/null)" + find /sys/bus/pci/devices -path '*/nvidia/current_vgpu_type' -exec grep -H -v '^0$' {} + + ``` +4. Downgrade only after both checks are clean. + +If assignment cleanup fails, Hypeman retains the instance metadata so a compatible version can retry it. Do not remove that metadata manually while the assignment remains active. + ## Troubleshooting ### No GPU shown in /resources 1. Check host GPU mode detection: ```bash - ls /sys/class/mdev_bus/ # Should show VFs for vGPU mode + ls /sys/class/mdev_bus/ + find /sys/bus/pci/devices -path '*/nvidia/current_vgpu_type' ``` 2. Verify NVIDIA drivers are loaded on host: @@ -265,17 +279,18 @@ curl -s http://localhost:4973/resources | jq '.gpu.profiles' curl http://localhost:4973/instances//logs?source=app ``` -### mdev creation fails +### vGPU assignment fails -1. Check if VFs are available: - ```bash - ls /sys/class/mdev_bus/ - ``` +Check the files for the framework detected on the host: -2. Verify mdev types: - ```bash - cat /sys/class/mdev_bus/*/mdev_supported_types/*/available_instances - ``` +```bash +# mdev +cat /sys/class/mdev_bus/*/mdev_supported_types/*/available_instances + +# vendor VFIO +cat /sys/bus/pci/devices/*/nvidia/creatable_vgpu_types +cat /sys/bus/pci/devices/*/nvidia/current_vgpu_type +``` ## Performance Tuning diff --git a/lib/devices/gpu_mode.go b/lib/devices/gpu_mode.go deleted file mode 100644 index 40b3b2ba0..000000000 --- a/lib/devices/gpu_mode.go +++ /dev/null @@ -1,30 +0,0 @@ -package devices - -import ( - "os" -) - -// DetectHostGPUMode determines the host's GPU configuration mode. -// -// Returns: -// - GPUModeVGPU if /sys/class/mdev_bus has entries (SR-IOV VFs present) -// - GPUModePassthrough if NVIDIA GPUs are available for VFIO passthrough -// - GPUModeNone if no GPUs are available -// -// Note: A host is configured for either vGPU or passthrough, not both, -// because the host driver determines which mode is available. -func DetectHostGPUMode() GPUMode { - // Check for vGPU mode first (SR-IOV VFs present) - entries, err := os.ReadDir("/sys/class/mdev_bus") - if err == nil && len(entries) > 0 { - return GPUModeVGPU - } - - // Check for passthrough mode (physical GPUs available) - gpus, err := DiscoverAvailableDevices() - if err == nil && len(gpus) > 0 { - return GPUModePassthrough - } - - return GPUModeNone -} diff --git a/lib/devices/mdev_darwin.go b/lib/devices/mdev_darwin.go index 22dd3435a..4274063ed 100644 --- a/lib/devices/mdev_darwin.go +++ b/lib/devices/mdev_darwin.go @@ -2,17 +2,19 @@ package devices -import "context" +import ( + "context" + "fmt" +) // SetGPUProfileCacheTTL is a no-op on macOS. func SetGPUProfileCacheTTL(ttl string) { // No-op on macOS } -// DiscoverVFs returns an empty list on macOS. -// SR-IOV Virtual Functions are not available on macOS. -func DiscoverVFs() ([]VirtualFunction, error) { - return []VirtualFunction{}, nil +// DiscoverVGPU reports no vGPU framework on macOS. +func DiscoverVGPU() (VGPUFramework, []VirtualFunction, error) { + return VGPUFrameworkNone, nil, nil } // ListGPUProfiles returns an empty list on macOS. @@ -21,7 +23,7 @@ func ListGPUProfiles() ([]GPUProfile, error) { } // ListGPUProfilesWithVFs returns an empty list on macOS. -func ListGPUProfilesWithVFs(vfs []VirtualFunction) ([]GPUProfile, error) { +func ListGPUProfilesWithVFs(framework VGPUFramework, vfs []VirtualFunction) ([]GPUProfile, error) { return []GPUProfile{}, nil } @@ -30,6 +32,10 @@ func ListMdevDevices() ([]MdevDevice, error) { return []MdevDevice{}, nil } +func CreateVGPU(ctx context.Context, profileName, instanceID string) (*VGPUDevice, error) { + return nil, ErrVGPUNotSupportedOnMacOS +} + // CreateMdev returns an error on macOS as mdev is not supported. func CreateMdev(ctx context.Context, profileName, instanceID string) (*MdevDevice, error) { return nil, ErrVGPUNotSupportedOnMacOS @@ -45,6 +51,17 @@ func IsMdevInUse(mdevUUID string) bool { return false } +func DestroyVGPU(ctx context.Context, assignment VGPUAssignment) error { + if assignment.Framework != VGPUFrameworkNone && assignment.Framework != VGPUFrameworkMdev { + return fmt.Errorf("unknown vGPU framework %q", assignment.Framework) + } + return nil +} + +func ReconcileVGPUs(ctx context.Context, protectedDevicePaths map[string]struct{}) error { + return nil +} + // ReconcileMdevs is a no-op on macOS. func ReconcileMdevs(ctx context.Context, instanceInfos []MdevReconcileInfo) error { return nil diff --git a/lib/devices/mdev_linux.go b/lib/devices/mdev_linux.go index 21335ca23..61e55b599 100644 --- a/lib/devices/mdev_linux.go +++ b/lib/devices/mdev_linux.go @@ -89,20 +89,23 @@ func getCachedProfiles(firstVF string) []profileMetadata { return cachedProfiles } -// DiscoverVFs returns all SR-IOV Virtual Functions available for vGPU. -// These are discovered by scanning /sys/class/mdev_bus/ which contains -// VFs that can host mdev devices. -func DiscoverVFs() ([]VirtualFunction, error) { - entries, err := os.ReadDir(mdevBusPath) +// discoverMdevVFs returns all SR-IOV Virtual Functions available for vGPU, +// discovered by scanning /sys/class/mdev_bus/. +func discoverMdevVFs() ([]VirtualFunction, error) { + return discoverMdevVFsWith(mdevBusPath, pciDevicesPath, ListMdevDevices) +} + +func discoverMdevVFsWith(busPath, pciPath string, listMdevs func() ([]MdevDevice, error)) ([]VirtualFunction, error) { + entries, err := os.ReadDir(busPath) if err != nil { if os.IsNotExist(err) { - return nil, nil // No mdev_bus means no vGPU support + return nil, nil // No mdev_bus means no mdev vGPU support } return nil, fmt.Errorf("read mdev_bus: %w", err) } // List mdevs once and build a lookup map to avoid O(n*m) performance - mdevs, _ := ListMdevDevices() + mdevs, _ := listMdevs() mdevByVF := make(map[string]bool, len(mdevs)) for _, mdev := range mdevs { mdevByVF[mdev.VFAddress] = true @@ -111,10 +114,27 @@ func DiscoverVFs() ([]VirtualFunction, error) { var vfs []VirtualFunction for _, entry := range entries { vfAddr := entry.Name() + types, err := os.ReadDir(filepath.Join(busPath, vfAddr, "mdev_supported_types")) + if err != nil { + if os.IsNotExist(err) { + continue + } + return nil, fmt.Errorf("read mdev supported types for VF %s: %w", vfAddr, err) + } + usable := false + for _, typ := range types { + if typ.IsDir() { + usable = true + break + } + } + if !usable { + continue + } // Find parent GPU by checking physfn symlink // VFs have a physfn symlink pointing to their parent Physical Function - physfnPath := filepath.Join("/sys/bus/pci/devices", vfAddr, "physfn") + physfnPath := filepath.Join(pciPath, vfAddr, "physfn") parentGPU := "" if target, err := os.Readlink(physfnPath); err == nil { parentGPU = filepath.Base(target) @@ -126,27 +146,16 @@ func DiscoverVFs() ([]VirtualFunction, error) { vfs = append(vfs, VirtualFunction{ PCIAddress: vfAddr, ParentGPU: parentGPU, - HasMdev: hasMdev, + Allocated: hasMdev, }) } return vfs, nil } -// ListGPUProfiles returns available vGPU profiles with availability counts. -// Profiles are discovered from the first VF's mdev_supported_types directory. -func ListGPUProfiles() ([]GPUProfile, error) { - vfs, err := DiscoverVFs() - if err != nil { - return nil, err - } - return ListGPUProfilesWithVFs(vfs) -} - -// ListGPUProfilesWithVFs returns available vGPU profiles using pre-discovered VFs. -// This avoids redundant VF discovery when the caller already has the list. -// Uses parallel sysfs reads for fast availability counting. -func ListGPUProfilesWithVFs(vfs []VirtualFunction) ([]GPUProfile, error) { +// listMdevGPUProfilesWithVFs returns available vGPU profiles using +// pre-discovered VFs. Uses parallel sysfs reads for fast availability counting. +func listMdevGPUProfilesWithVFs(vfs []VirtualFunction) ([]GPUProfile, error) { if len(vfs) == 0 { return nil, nil } @@ -253,7 +262,7 @@ func countAvailableVFsForProfilesParallel(vfs []VirtualFunction, profiles []prof // Group free VFs by parent GPU (done once, shared by all goroutines) freeVFsByParent := make(map[string][]VirtualFunction) for _, vf := range vfs { - if vf.HasMdev { + if vf.Allocated { continue } freeVFsByParent[vf.ParentGPU] = append(freeVFsByParent[vf.ParentGPU], vf) @@ -305,7 +314,7 @@ func countAvailableForSingleProfile(freeVFsByParent map[string][]VirtualFunction // findProfileType finds the internal type name (e.g., "nvidia-556") for a profile name (e.g., "L40S-1Q") func findProfileType(profileName string) (string, error) { - vfs, err := DiscoverVFs() + vfs, err := discoverMdevVFs() if err != nil || len(vfs) == 0 { return "", fmt.Errorf("no VFs available") } @@ -453,7 +462,7 @@ func selectLeastLoadedVF(ctx context.Context, vfs []VirtualFunction, profileType allGPUs := make(map[string]bool) for _, vf := range vfs { allGPUs[vf.ParentGPU] = true - if !vf.HasMdev { + if !vf.Allocated { freeVFsByGPU[vf.ParentGPU] = append(freeVFsByGPU[vf.ParentGPU], vf) } } @@ -531,7 +540,7 @@ func CreateMdev(ctx context.Context, profileName, instanceID string) (*MdevDevic } // Discover all VFs - vfs, err := DiscoverVFs() + vfs, err := discoverMdevVFs() if err != nil { return nil, fmt.Errorf("discover VFs: %w", err) } @@ -697,7 +706,7 @@ func ReconcileMdevs(ctx context.Context, instanceInfos []MdevReconcileInfo) erro log := logger.FromContext(ctx) _ = instanceInfos - vfs, err := DiscoverVFs() + vfs, err := discoverMdevVFs() if err != nil { return fmt.Errorf("discover managed VFs: %w", err) } diff --git a/lib/devices/types.go b/lib/devices/types.go index 57d870592..c76d239ed 100644 --- a/lib/devices/types.go +++ b/lib/devices/types.go @@ -60,10 +60,16 @@ func ValidateDeviceName(name string) bool { // GPUMode represents the host's GPU configuration mode type GPUMode string +type VGPUFramework string + const ( + VGPUFrameworkNone VGPUFramework = "" + VGPUFrameworkMdev VGPUFramework = "mdev" + VGPUFrameworkVendorVFIO VGPUFramework = "vendor-vfio" + // GPUModePassthrough indicates whole GPU VFIO passthrough GPUModePassthrough GPUMode = "passthrough" - // GPUModeVGPU indicates SR-IOV + mdev based vGPU + // GPUModeVGPU indicates vGPU mode GPUModeVGPU GPUMode = "vgpu" // GPUModeNone indicates no GPU available GPUModeNone GPUMode = "none" @@ -71,9 +77,27 @@ const ( // VirtualFunction represents an SR-IOV Virtual Function for vGPU type VirtualFunction struct { - PCIAddress string `json:"pci_address"` // e.g., "0000:82:00.4" - ParentGPU string `json:"parent_gpu"` // e.g., "0000:82:00.0" - HasMdev bool `json:"has_mdev"` // true if an mdev is created on this VF + PCIAddress string `json:"pci_address"` // e.g., "0000:82:00.4" + ParentGPU string `json:"parent_gpu"` // e.g., "0000:82:00.0" + Allocated bool `json:"allocated"` // true if a vGPU is assigned to this VF + ProfileType string `json:"profile_type,omitempty"` +} + +// VGPUAssignment identifies an existing vGPU assignment to release. +type VGPUAssignment struct { + Framework VGPUFramework + DevicePath string + MdevUUID string + InstanceID string +} + +type VGPUDevice struct { + Framework VGPUFramework + VFAddress string + ProfileType string + ProfileName string + SysfsPath string + MdevUUID string } // MdevDevice represents an active mediated device (vGPU instance) diff --git a/lib/devices/vendor_vfio_linux.go b/lib/devices/vendor_vfio_linux.go new file mode 100644 index 000000000..6f59bed3d --- /dev/null +++ b/lib/devices/vendor_vfio_linux.go @@ -0,0 +1,502 @@ +//go:build linux + +package devices + +import ( + "context" + "errors" + "fmt" + "os" + "path/filepath" + "sort" + "strconv" + "strings" + "sync" + + "github.com/kernel/hypeman/lib/logger" +) + +const ( + pciDevicesPath = "/sys/bus/pci/devices" + vfioDevicesPath = "/dev/vfio/devices" +) + +type vendorVFIOSysfs struct { + pciDevicesPath string + procPath string + vfioDevicesPath string + owners map[string]string + framebufferByType map[string]int +} + +var ( + hostVendorVFIO = vendorVFIOSysfs{ + pciDevicesPath: pciDevicesPath, + procPath: procPath, + vfioDevicesPath: vfioDevicesPath, + owners: make(map[string]string), + framebufferByType: make(map[string]int), + } + vendorVFIOMu sync.Mutex +) + +func (s vendorVFIOSysfs) discoverVFs() ([]VirtualFunction, error) { + entries, err := os.ReadDir(s.pciDevicesPath) + if err != nil { + if os.IsNotExist(err) { + return nil, nil + } + return nil, fmt.Errorf("read PCI devices: %w", err) + } + + vfs := make([]VirtualFunction, 0) + for _, entry := range entries { + vfPath := filepath.Join(s.pciDevicesPath, entry.Name()) + nvidiaPath := filepath.Join(vfPath, "nvidia") + if _, err := os.Stat(filepath.Join(nvidiaPath, "creatable_vgpu_types")); err != nil { + if os.IsNotExist(err) { + continue + } + return nil, fmt.Errorf("stat creatable vGPU types for VF %s: %w", entry.Name(), err) + } + + currentType, err := readCurrentVGPUType(filepath.Join(nvidiaPath, "current_vgpu_type")) + if err != nil { + return nil, fmt.Errorf("read current vGPU type for VF %s: %w", entry.Name(), err) + } + + parentGPU := "" + if target, err := os.Readlink(filepath.Join(vfPath, "physfn")); err == nil { + parentGPU = filepath.Base(target) + } + vfs = append(vfs, VirtualFunction{ + PCIAddress: entry.Name(), + ParentGPU: parentGPU, + Allocated: currentType != "0", + ProfileType: currentType, + }) + } + + sort.Slice(vfs, func(i, j int) bool { return vfs[i].PCIAddress < vfs[j].PCIAddress }) + return vfs, nil +} + +// listProfiles aggregates creatable profiles per parent GPU. Free VFs on the +// same GPU share its framebuffer, so counting each advertising VF overreports +// availability. The driver only guarantees that a GPU still advertising a +// type can fit one more instance of it, so report that per-GPU lower bound. +func (s vendorVFIOSysfs) listProfiles(vfs []VirtualFunction) ([]GPUProfile, error) { + profilesByType := make(map[string]profileMetadata) + creatableGPUs := make(map[string]map[string]struct{}) + for _, vf := range vfs { + creatable, err := s.readCreatableProfiles(vf.PCIAddress) + if err != nil { + return nil, err + } + for _, profile := range creatable { + profilesByType[profile.TypeName] = profile + if !vf.Allocated { + gpu := vf.ParentGPU + if gpu == "" { + gpu = vf.PCIAddress + } + if creatableGPUs[profile.TypeName] == nil { + creatableGPUs[profile.TypeName] = make(map[string]struct{}) + } + creatableGPUs[profile.TypeName][gpu] = struct{}{} + } + } + } + + metadata := make([]profileMetadata, 0, len(profilesByType)) + for _, profile := range profilesByType { + metadata = append(metadata, profile) + } + sort.Slice(metadata, func(i, j int) bool { return metadata[i].Name < metadata[j].Name }) + + profiles := make([]GPUProfile, 0, len(metadata)) + for _, profile := range metadata { + profiles = append(profiles, GPUProfile{ + Name: profile.Name, + FramebufferMB: profile.FramebufferMB, + Available: len(creatableGPUs[profile.TypeName]), + }) + } + return profiles, nil +} + +func (s vendorVFIOSysfs) create(ctx context.Context, profileName, instanceID string) (*VGPUDevice, error) { + vendorVFIOMu.Lock() + defer vendorVFIOMu.Unlock() + + vfs, err := s.discoverVFs() + if err != nil { + return nil, err + } + metadata, err := s.profileMetadata(vfs) + if err != nil { + return nil, err + } + + var requested profileMetadata + found := false + for _, profile := range metadata { + if profile.Name == profileName { + requested = profile + found = true + break + } + } + if !found { + if len(metadata) == 0 && len(vfs) > 0 { + return nil, fmt.Errorf("no creatable vGPU profiles on any VF, GPUs may be at capacity: profile %q", profileName) + } + return nil, fmt.Errorf("profile %q is not creatable on any VF (unknown profile or insufficient capacity)", profileName) + } + + targetVF, err := s.selectLeastLoadedVF(vfs, requested.TypeName) + if err != nil { + return nil, err + } + if targetVF == "" { + return nil, fmt.Errorf("no available VF for profile %q", profileName) + } + + currentTypePath := filepath.Join(s.pciDevicesPath, targetVF, "nvidia", "current_vgpu_type") + if err := os.WriteFile(currentTypePath, []byte(requested.TypeName), 0200); err != nil { + return nil, fmt.Errorf("create vGPU on VF %s: %w", targetVF, err) + } + currentType, err := readCurrentVGPUType(currentTypePath) + if err != nil { + verifyErr := fmt.Errorf("verify vGPU on VF %s: %w", targetVF, err) + return nil, rollbackVendorVFIOCreate(currentTypePath, targetVF, verifyErr) + } + if currentType != requested.TypeName { + verifyErr := fmt.Errorf("verify vGPU on VF %s: type is %s, want %s", targetVF, currentType, requested.TypeName) + return nil, rollbackVendorVFIOCreate(currentTypePath, targetVF, verifyErr) + } + s.owners[targetVF] = instanceID + + logger.FromContext(ctx).InfoContext(ctx, "created vendor VFIO vGPU", + "profile", profileName, + "vf", targetVF, + "instance_id", instanceID, + ) + return &VGPUDevice{ + Framework: VGPUFrameworkVendorVFIO, + VFAddress: targetVF, + ProfileType: requested.TypeName, + ProfileName: profileName, + SysfsPath: filepath.Join(s.pciDevicesPath, targetVF), + }, nil +} + +func (s vendorVFIOSysfs) destroy(ctx context.Context, vfAddress, instanceID string) error { + return s.destroyWithOpenPaths(ctx, vfAddress, instanceID, nil) +} + +func (s vendorVFIOSysfs) destroyWithOpenPaths(ctx context.Context, vfAddress, instanceID string, openPaths map[string]struct{}) error { + vendorVFIOMu.Lock() + defer vendorVFIOMu.Unlock() + + log := logger.FromContext(ctx) + currentTypePath := filepath.Join(s.pciDevicesPath, vfAddress, "nvidia", "current_vgpu_type") + currentType, err := readCurrentVGPUType(currentTypePath) + if err != nil { + if os.IsNotExist(err) { + delete(s.owners, vfAddress) + return nil + } + return fmt.Errorf("read current vGPU type for VF %s: %w", vfAddress, err) + } + if currentType == "0" { + delete(s.owners, vfAddress) + return nil + } + + if owner, ok := s.owners[vfAddress]; ok { + if instanceID == "" { + return fmt.Errorf("cannot release vendor VFIO vGPU on VF %s without instance ID", vfAddress) + } + if owner != instanceID { + log.WarnContext(ctx, "skipping vendor VFIO vGPU release owned by another instance", + "vf", vfAddress, + "owner_instance_id", owner, + "requesting_instance_id", instanceID, + ) + return nil + } + } + + if openPaths == nil { + if openPaths, err = s.openVFIOPaths(); err != nil { + return fmt.Errorf("scan open VFIO handles: %w", err) + } + } + inUse, err := s.vfioDeviceInUse(vfAddress, openPaths) + if err != nil { + return fmt.Errorf("check vendor VFIO vGPU usage for VF %s: %w", vfAddress, err) + } + if inUse { + return fmt.Errorf("vendor VFIO vGPU on VF %s is still in use", vfAddress) + } + + if err := os.WriteFile(currentTypePath, []byte("0"), 0200); err != nil { + return fmt.Errorf("destroy vGPU on VF %s: %w", vfAddress, err) + } + delete(s.owners, vfAddress) + log.InfoContext(ctx, "destroyed vendor VFIO vGPU", "vf", vfAddress) + return nil +} + +func (s vendorVFIOSysfs) reconcile(ctx context.Context, protectedDevicePaths map[string]struct{}) error { + vfs, err := s.discoverVFs() + if err != nil { + return err + } + log := logger.FromContext(ctx) + protectedVFs := make(map[string]struct{}, len(protectedDevicePaths)) + for path := range protectedDevicePaths { + protectedVFs[filepath.Base(path)] = struct{}{} + } + var openPaths map[string]struct{} + for _, vf := range vfs { + if !vf.Allocated { + continue + } + if _, ok := protectedVFs[vf.PCIAddress]; ok { + log.DebugContext(ctx, "skipping vendor VFIO vGPU held by a live instance", "vf", vf.PCIAddress) + continue + } + if openPaths == nil { + if openPaths, err = s.openVFIOPaths(); err != nil { + return fmt.Errorf("scan open VFIO handles: %w", err) + } + } + inUse, err := s.vfioDeviceInUse(vf.PCIAddress, openPaths) + if err != nil { + log.WarnContext(ctx, "failed to check vendor VFIO vGPU usage", "vf", vf.PCIAddress, "error", err) + continue + } + if inUse { + continue + } + if err := s.destroyWithOpenPaths(ctx, vf.PCIAddress, "", openPaths); err != nil { + log.WarnContext(ctx, "failed to destroy orphaned vendor VFIO vGPU", "vf", vf.PCIAddress, "error", err) + } + } + return nil +} + +func (s vendorVFIOSysfs) selectLeastLoadedVF(vfs []VirtualFunction, profileType string) (string, error) { + usageByGPU := make(map[string]int) + unknownUsageByGPU := make(map[string]bool) + freeByGPU := make(map[string][]VirtualFunction) + for _, vf := range vfs { + if vf.Allocated { + // framebufferByType only covers currently creatable profiles, so + // after a restart an allocated type can be missing when its + // capacity is exhausted. Prefer GPUs whose load is fully known + // instead of rejecting placement outright; the kernel driver + // still enforces real capacity through creatable_vgpu_types. + framebuffer, ok := s.framebufferByType[vf.ProfileType] + if !ok { + unknownUsageByGPU[vf.ParentGPU] = true + continue + } + usageByGPU[vf.ParentGPU] += framebuffer + continue + } + profiles, err := s.readCreatableProfiles(vf.PCIAddress) + if err != nil { + return "", err + } + for _, profile := range profiles { + if profile.TypeName == profileType { + freeByGPU[vf.ParentGPU] = append(freeByGPU[vf.ParentGPU], vf) + break + } + } + } + + gpus := make([]string, 0, len(freeByGPU)) + for gpu := range freeByGPU { + gpus = append(gpus, gpu) + } + sort.Slice(gpus, func(i, j int) bool { + if unknownUsageByGPU[gpus[i]] != unknownUsageByGPU[gpus[j]] { + return !unknownUsageByGPU[gpus[i]] + } + if usageByGPU[gpus[i]] == usageByGPU[gpus[j]] { + return gpus[i] < gpus[j] + } + return usageByGPU[gpus[i]] < usageByGPU[gpus[j]] + }) + if len(gpus) == 0 { + return "", nil + } + return freeByGPU[gpus[0]][0].PCIAddress, nil +} + +func (s vendorVFIOSysfs) profileMetadata(vfs []VirtualFunction) ([]profileMetadata, error) { + profilesByType := make(map[string]profileMetadata) + for _, vf := range vfs { + profiles, err := s.readCreatableProfiles(vf.PCIAddress) + if err != nil { + return nil, err + } + for _, profile := range profiles { + profilesByType[profile.TypeName] = profile + s.framebufferByType[profile.TypeName] = profile.FramebufferMB + } + } + profiles := make([]profileMetadata, 0, len(profilesByType)) + for _, profile := range profilesByType { + profiles = append(profiles, profile) + } + sort.Slice(profiles, func(i, j int) bool { return profiles[i].Name < profiles[j].Name }) + return profiles, nil +} + +func (s vendorVFIOSysfs) readCreatableProfiles(vfAddress string) ([]profileMetadata, error) { + path := filepath.Join(s.pciDevicesPath, vfAddress, "nvidia", "creatable_vgpu_types") + data, err := os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("read creatable vGPU types for VF %s: %w", vfAddress, err) + } + return parseCreatableVGPUTypes(string(data)) +} + +func (s vendorVFIOSysfs) vfioDeviceInUse(vfAddress string, openPaths map[string]struct{}) (bool, error) { + devicePaths := make([]string, 0, 2) + probeErrs := make([]error, 0, 2) + + vfioDevices, err := os.ReadDir(filepath.Join(s.pciDevicesPath, vfAddress, "vfio-dev")) + if err != nil { + if !os.IsNotExist(err) { + probeErrs = append(probeErrs, fmt.Errorf("read VFIO devices for VF %s: %w", vfAddress, err)) + } + } else { + for _, device := range vfioDevices { + devicePaths = append(devicePaths, filepath.Join(s.vfioDevicesPath, device.Name())) + } + } + + target, err := os.Readlink(filepath.Join(s.pciDevicesPath, vfAddress, "iommu_group")) + if err != nil { + if !os.IsNotExist(err) { + probeErrs = append(probeErrs, fmt.Errorf("read IOMMU group for VF %s: %w", vfAddress, err)) + } + } else { + devicePaths = append(devicePaths, filepath.Join(filepath.Dir(s.vfioDevicesPath), filepath.Base(target))) + } + + for _, path := range devicePaths { + if _, ok := openPaths[path]; ok { + return true, nil + } + } + if len(probeErrs) > 0 { + return false, errors.Join(probeErrs...) + } + return false, nil +} + +func (s vendorVFIOSysfs) openVFIOPaths() (map[string]struct{}, error) { + processes, err := os.ReadDir(s.procPath) + if err != nil { + return nil, err + } + prefix := filepath.Dir(s.vfioDevicesPath) + string(filepath.Separator) + open := make(map[string]struct{}) + for _, process := range processes { + if _, err := strconv.Atoi(process.Name()); err != nil { + continue + } + fdPath := filepath.Join(s.procPath, process.Name(), "fd") + fds, err := os.ReadDir(fdPath) + if err != nil { + if os.IsNotExist(err) { + continue + } + return nil, fmt.Errorf("read process %s file descriptors: %w", process.Name(), err) + } + for _, fd := range fds { + target, err := os.Readlink(filepath.Join(fdPath, fd.Name())) + if err != nil { + if os.IsNotExist(err) { + continue + } + return nil, fmt.Errorf("read process %s file descriptor %s: %w", process.Name(), fd.Name(), err) + } + if strings.HasPrefix(target, prefix) { + open[target] = struct{}{} + } + } + } + return open, nil +} + +func parseCreatableVGPUTypes(value string) ([]profileMetadata, error) { + profiles := make([]profileMetadata, 0) + for lineNumber, line := range strings.Split(value, "\n") { + line = strings.TrimSpace(line) + if line == "" { + continue + } + + typeID, name, found := strings.Cut(line, ":") + typeID = strings.TrimSpace(typeID) + name = strings.TrimSpace(name) + if typeID == "ID" { + continue + } + if !found || name == "" { + return nil, fmt.Errorf("parse creatable vGPU types line %d: %q", lineNumber+1, line) + } + if _, err := strconv.Atoi(typeID); err != nil { + return nil, fmt.Errorf("parse vGPU type ID %q: %w", typeID, err) + } + profiles = append(profiles, profileMetadata{ + TypeName: typeID, + Name: name, + FramebufferMB: framebufferFromProfileName(name), + }) + } + return profiles, nil +} + +func framebufferFromProfileName(name string) int { + series := strings.LastIndexAny(name, "ABCQ") + if series <= 0 { + return 0 + } + dash := strings.LastIndex(name[:series], "-") + if dash < 0 { + return 0 + } + gb, err := strconv.Atoi(name[dash+1 : series]) + if err != nil { + return 0 + } + return gb * 1024 +} + +func rollbackVendorVFIOCreate(currentTypePath, vfAddress string, verifyErr error) error { + if err := os.WriteFile(currentTypePath, []byte("0"), 0200); err != nil { + return errors.Join(verifyErr, fmt.Errorf("roll back vGPU on VF %s: %w", vfAddress, err)) + } + return verifyErr +} + +func readCurrentVGPUType(path string) (string, error) { + data, err := os.ReadFile(path) + if err != nil { + return "", err + } + value := strings.TrimSpace(string(data)) + if _, err := strconv.Atoi(value); err != nil { + return "", fmt.Errorf("invalid current vGPU type %q", value) + } + return value, nil +} diff --git a/lib/devices/vendor_vfio_linux_test.go b/lib/devices/vendor_vfio_linux_test.go new file mode 100644 index 000000000..51668ed11 --- /dev/null +++ b/lib/devices/vendor_vfio_linux_test.go @@ -0,0 +1,501 @@ +//go:build linux + +package devices + +import ( + "context" + "errors" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const testCreatableTypes = `ID : vGPU Name +1147 : NVIDIA L40S-1Q +1148 : NVIDIA L40S-2Q +1159 : NVIDIA L40S-48Q +` + +func TestParseCreatableVGPUTypes(t *testing.T) { + t.Parallel() + + profiles, err := parseCreatableVGPUTypes(testCreatableTypes) + require.NoError(t, err) + require.Len(t, profiles, 3) + assert.Equal(t, profileMetadata{TypeName: "1147", Name: "NVIDIA L40S-1Q", FramebufferMB: 1024}, profiles[0]) + assert.Equal(t, profileMetadata{TypeName: "1159", Name: "NVIDIA L40S-48Q", FramebufferMB: 48 * 1024}, profiles[2]) +} + +func TestVendorVFIOCreateAndDestroy(t *testing.T) { + t.Parallel() + + sysfs := newTestVendorVFIOSysfs(t) + sysfs.addVF(t, "0000:82:00.0", "0000:82:00.4", "42", "0", testCreatableTypes) + + vfs, err := sysfs.discoverVFs() + require.NoError(t, err) + require.Len(t, vfs, 1) + assert.False(t, vfs[0].Allocated) + assert.Equal(t, "0000:82:00.0", vfs[0].ParentGPU) + + profiles, err := sysfs.listProfiles(vfs) + require.NoError(t, err) + assert.Equal(t, 1, profileAvailability(profiles, "NVIDIA L40S-2Q")) + + device, err := sysfs.create(context.Background(), "NVIDIA L40S-2Q", "instance-1") + require.NoError(t, err) + assert.Equal(t, VGPUFrameworkVendorVFIO, device.Framework) + assert.Equal(t, "0000:82:00.4", device.VFAddress) + assert.Equal(t, filepath.Join(sysfs.pciDevicesPath, "0000:82:00.4"), device.SysfsPath) + assertFileValue(t, filepath.Join(device.SysfsPath, "nvidia", "current_vgpu_type"), "1148") + + require.NoError(t, sysfs.destroy(context.Background(), device.VFAddress, "instance-1")) + assertFileValue(t, filepath.Join(device.SysfsPath, "nvidia", "current_vgpu_type"), "0") +} + +func TestVendorVFIODestroySkipsAssignmentOwnedByAnotherInstance(t *testing.T) { + t.Parallel() + + sysfs := newTestVendorVFIOSysfs(t) + sysfs.addVF(t, "0000:82:00.0", "0000:82:00.4", "42", "0", testCreatableTypes) + + device, err := sysfs.create(context.Background(), "NVIDIA L40S-2Q", "instance-1") + require.NoError(t, err) + + require.NoError(t, sysfs.destroy(context.Background(), device.VFAddress, "stale-instance")) + assertFileValue(t, filepath.Join(device.SysfsPath, "nvidia", "current_vgpu_type"), "1148") + + require.NoError(t, sysfs.destroy(context.Background(), device.VFAddress, "instance-1")) + assertFileValue(t, filepath.Join(device.SysfsPath, "nvidia", "current_vgpu_type"), "0") +} + +func TestVendorVFIODestroyRejectsMissingInstanceIDForOwnedVF(t *testing.T) { + t.Parallel() + + sysfs := newTestVendorVFIOSysfs(t) + sysfs.addVF(t, "0000:82:00.0", "0000:82:00.4", "42", "0", testCreatableTypes) + + device, err := sysfs.create(context.Background(), "NVIDIA L40S-2Q", "instance-1") + require.NoError(t, err) + + err = sysfs.destroy(context.Background(), device.VFAddress, "") + require.ErrorContains(t, err, "without instance ID") + assertFileValue(t, filepath.Join(device.SysfsPath, "nvidia", "current_vgpu_type"), "1148") +} + +func TestVendorVFIODestroyRetainsAssignmentInUse(t *testing.T) { + t.Parallel() + + sysfs := newTestVendorVFIOSysfs(t) + const vfAddress = "0000:82:00.4" + sysfs.addVF(t, "0000:82:00.0", vfAddress, "42", "1148", "") + + activeDevice := filepath.Join(sysfs.vfioDevicesPath, "vfio42") + fdDir := filepath.Join(sysfs.procPath, "123", "fd") + require.NoError(t, os.MkdirAll(fdDir, 0755)) + require.NoError(t, os.Symlink(activeDevice, filepath.Join(fdDir, "5"))) + + err := sysfs.destroy(context.Background(), vfAddress, "instance-1") + require.Error(t, err) + assert.ErrorContains(t, err, "still in use") + assertFileValue(t, filepath.Join(sysfs.pciDevicesPath, vfAddress, "nvidia", "current_vgpu_type"), "1148") +} + +func TestVendorVFIODestroyReleasesUnboundVF(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + unbind func(t *testing.T, sysfs testVendorVFIOSysfs, vfAddress string) + }{ + { + name: "missing vfio device directory", + unbind: func(t *testing.T, sysfs testVendorVFIOSysfs, vfAddress string) { + require.NoError(t, os.RemoveAll(filepath.Join(sysfs.pciDevicesPath, vfAddress, "vfio-dev"))) + }, + }, + { + name: "missing iommu group symlink", + unbind: func(t *testing.T, sysfs testVendorVFIOSysfs, vfAddress string) { + require.NoError(t, os.Remove(filepath.Join(sysfs.pciDevicesPath, vfAddress, "iommu_group"))) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + sysfs := newTestVendorVFIOSysfs(t) + const vfAddress = "0000:82:00.4" + sysfs.addVF(t, "0000:82:00.0", vfAddress, "42", "1148", "") + tt.unbind(t, sysfs, vfAddress) + + require.NoError(t, sysfs.destroy(context.Background(), vfAddress, "instance-1")) + assertFileValue(t, filepath.Join(sysfs.pciDevicesPath, vfAddress, "nvidia", "current_vgpu_type"), "0") + }) + } +} + +func TestVendorVFIODestroyRetainsAssignmentWhenOneVFIOPathIsMissing(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + remove func(t *testing.T, sysfs testVendorVFIOSysfs, vfAddress string) + open func(sysfs testVendorVFIOSysfs) string + }{ + { + name: "missing iommu group", + remove: func(t *testing.T, sysfs testVendorVFIOSysfs, vfAddress string) { + require.NoError(t, os.Remove(filepath.Join(sysfs.pciDevicesPath, vfAddress, "iommu_group"))) + }, + open: func(sysfs testVendorVFIOSysfs) string { + return filepath.Join(sysfs.vfioDevicesPath, "vfio42") + }, + }, + { + name: "missing vfio device directory", + remove: func(t *testing.T, sysfs testVendorVFIOSysfs, vfAddress string) { + require.NoError(t, os.RemoveAll(filepath.Join(sysfs.pciDevicesPath, vfAddress, "vfio-dev"))) + }, + open: func(sysfs testVendorVFIOSysfs) string { + return filepath.Join(filepath.Dir(sysfs.vfioDevicesPath), "42") + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + sysfs := newTestVendorVFIOSysfs(t) + const vfAddress = "0000:82:00.4" + sysfs.addVF(t, "0000:82:00.0", vfAddress, "42", "1148", "") + tt.remove(t, sysfs, vfAddress) + + fdDir := filepath.Join(sysfs.procPath, "123", "fd") + require.NoError(t, os.MkdirAll(fdDir, 0755)) + require.NoError(t, os.Symlink(tt.open(sysfs), filepath.Join(fdDir, "5"))) + + err := sysfs.destroy(context.Background(), vfAddress, "instance-1") + require.ErrorContains(t, err, "still in use") + assertFileValue(t, filepath.Join(sysfs.pciDevicesPath, vfAddress, "nvidia", "current_vgpu_type"), "1148") + }) + } +} + +func TestVendorVFIOListProfilesCountsPerGPUNotPerVF(t *testing.T) { + t.Parallel() + + sysfs := newTestVendorVFIOSysfs(t) + sysfs.addVF(t, "0000:82:00.0", "0000:82:00.4", "42", "0", testCreatableTypes) + sysfs.addVF(t, "0000:82:00.0", "0000:82:00.5", "43", "0", testCreatableTypes) + sysfs.addVF(t, "0000:e3:00.0", "0000:e3:00.4", "44", "0", testCreatableTypes) + + vfs, err := sysfs.discoverVFs() + require.NoError(t, err) + profiles, err := sysfs.listProfiles(vfs) + require.NoError(t, err) + assert.Equal(t, 2, profileAvailability(profiles, "NVIDIA L40S-48Q"), + "free VFs share their parent GPU's capacity, so availability is per GPU") +} + +func TestVendorVFIOCreateReportsCapacityWhenAllGPUsFull(t *testing.T) { + t.Parallel() + + sysfs := newTestVendorVFIOSysfs(t) + sysfs.addVF(t, "0000:82:00.0", "0000:82:00.4", "42", "1159", "") + sysfs.addVF(t, "0000:82:00.0", "0000:82:00.5", "43", "0", "ID : vGPU Name\n") + sysfs.addVF(t, "0000:e3:00.0", "0000:e3:00.4", "44", "1159", "") + + vfs, err := sysfs.discoverVFs() + require.NoError(t, err) + profiles, err := sysfs.listProfiles(vfs) + require.NoError(t, err) + assert.Empty(t, profiles) + + _, err = sysfs.create(context.Background(), "NVIDIA L40S-2Q", "instance-1") + require.Error(t, err) + assert.ErrorContains(t, err, "GPUs may be at capacity") +} + +func TestVendorVFIOCreateReportsAmbiguousMissingProfile(t *testing.T) { + t.Parallel() + + sysfs := newTestVendorVFIOSysfs(t) + sysfs.addVF(t, "0000:82:00.0", "0000:82:00.4", "42", "1148", "") + sysfs.addVF(t, "0000:82:00.0", "0000:82:00.5", "43", "0", "ID : vGPU Name\n1148 : NVIDIA L40S-2Q\n") + + _, err := sysfs.create(context.Background(), "NVIDIA L40S-48Q", "instance-1") + require.Error(t, err) + assert.ErrorContains(t, err, "not creatable on any VF") + assert.ErrorContains(t, err, "unknown profile or insufficient capacity") +} + +func TestVendorVFIOSelectsLeastLoadedGPU(t *testing.T) { + t.Parallel() + + sysfs := newTestVendorVFIOSysfs(t) + sysfs.addVF(t, "0000:82:00.0", "0000:82:00.4", "42", "1148", "") + sysfs.addVF(t, "0000:82:00.0", "0000:82:00.5", "43", "0", testCreatableTypes) + sysfs.addVF(t, "0000:e3:00.0", "0000:e3:00.4", "44", "0", testCreatableTypes) + + device, err := sysfs.create(context.Background(), "NVIDIA L40S-1Q", "instance-1") + require.NoError(t, err) + assert.Equal(t, "0000:e3:00.4", device.VFAddress) +} + +func TestVendorVFIOSelectsLeastLoadedGPUWithConsumedType(t *testing.T) { + t.Parallel() + + sysfs := newTestVendorVFIOSysfs(t) + sysfs.addVF(t, "0000:82:00.0", "0000:82:00.4", "42", "1159", "") + sysfs.addVF(t, "0000:82:00.0", "0000:82:00.5", "43", "0", testCreatableTypes) + sysfs.addVF(t, "0000:e3:00.0", "0000:e3:00.4", "44", "0", testCreatableTypes) + + vfs, err := sysfs.discoverVFs() + require.NoError(t, err) + _, err = sysfs.profileMetadata(vfs) + require.NoError(t, err) + for _, vfAddress := range []string{"0000:82:00.5", "0000:e3:00.4"} { + creatableTypesPath := filepath.Join(sysfs.pciDevicesPath, vfAddress, "nvidia", "creatable_vgpu_types") + require.NoError(t, os.Chmod(creatableTypesPath, 0644)) + require.NoError(t, os.WriteFile(creatableTypesPath, []byte("1147 : NVIDIA L40S-1Q\n"), 0444)) + } + + device, err := sysfs.create(context.Background(), "NVIDIA L40S-1Q", "instance-1") + require.NoError(t, err) + assert.Equal(t, "0000:e3:00.4", device.VFAddress) +} + +func TestVendorVFIOPlacementPrefersKnownLoadWhenAllocatedTypeIsUnknown(t *testing.T) { + t.Parallel() + + // Simulates a restart: type 1159 is allocated but no longer creatable + // anywhere, so its framebuffer size is unknown. + sysfs := newTestVendorVFIOSysfs(t) + sysfs.addVF(t, "0000:82:00.0", "0000:82:00.4", "42", "1159", "") + sysfs.addVF(t, "0000:82:00.0", "0000:82:00.5", "43", "0", "1147 : NVIDIA L40S-1Q\n") + sysfs.addVF(t, "0000:e3:00.0", "0000:e3:00.4", "44", "0", "1147 : NVIDIA L40S-1Q\n") + + device, err := sysfs.create(context.Background(), "NVIDIA L40S-1Q", "instance-1") + require.NoError(t, err) + assert.Equal(t, "0000:e3:00.4", device.VFAddress, "the GPU with unknown load should be picked last") +} + +func TestVendorVFIOPlacesOnGPUWithUnknownLoadWhenItHasTheOnlyCapacity(t *testing.T) { + t.Parallel() + + sysfs := newTestVendorVFIOSysfs(t) + sysfs.addVF(t, "0000:82:00.0", "0000:82:00.4", "42", "1159", "") + sysfs.addVF(t, "0000:82:00.0", "0000:82:00.5", "43", "0", "1147 : NVIDIA L40S-1Q\n") + + device, err := sysfs.create(context.Background(), "NVIDIA L40S-1Q", "instance-1") + require.NoError(t, err) + assert.Equal(t, "0000:82:00.5", device.VFAddress) +} + +func TestVendorVFIOReconcile(t *testing.T) { + t.Parallel() + + sysfs := newTestVendorVFIOSysfs(t) + sysfs.addVF(t, "0000:82:00.0", "0000:82:00.4", "42", "1148", "") + sysfs.addVF(t, "0000:e3:00.0", "0000:e3:00.4", "43", "1148", "") + + activeDevice := filepath.Join(sysfs.vfioDevicesPath, "vfio43") + fdDir := filepath.Join(sysfs.procPath, "123", "fd") + require.NoError(t, os.MkdirAll(fdDir, 0755)) + require.NoError(t, os.Symlink(activeDevice, filepath.Join(fdDir, "5"))) + + require.NoError(t, sysfs.reconcile(context.Background(), nil)) + assertFileValue(t, filepath.Join(sysfs.pciDevicesPath, "0000:82:00.4", "nvidia", "current_vgpu_type"), "0") + assertFileValue(t, filepath.Join(sysfs.pciDevicesPath, "0000:e3:00.4", "nvidia", "current_vgpu_type"), "1148") +} + +func TestVendorVFIOReconcileSkipsProtectedVF(t *testing.T) { + t.Parallel() + + sysfs := newTestVendorVFIOSysfs(t) + sysfs.addVF(t, "0000:82:00.0", "0000:82:00.4", "42", "1148", "") + sysfs.addVF(t, "0000:e3:00.0", "0000:e3:00.4", "43", "1148", "") + + protected := map[string]struct{}{ + filepath.Join(sysfs.pciDevicesPath, "0000:82:00.4"): {}, + } + require.NoError(t, sysfs.reconcile(context.Background(), protected)) + assertFileValue(t, filepath.Join(sysfs.pciDevicesPath, "0000:82:00.4", "nvidia", "current_vgpu_type"), "1148") + assertFileValue(t, filepath.Join(sysfs.pciDevicesPath, "0000:e3:00.4", "nvidia", "current_vgpu_type"), "0") +} + +func TestVendorVFIOReconcilePreservesLegacyGroupFD(t *testing.T) { + t.Parallel() + + sysfs := newTestVendorVFIOSysfs(t) + sysfs.addVF(t, "0000:82:00.0", "0000:82:00.4", "42", "1148", "") + sysfs.addVF(t, "0000:e3:00.0", "0000:e3:00.4", "43", "1148", "") + + legacyGroup := filepath.Join(filepath.Dir(sysfs.vfioDevicesPath), "43") + fdDir := filepath.Join(sysfs.procPath, "123", "fd") + require.NoError(t, os.MkdirAll(fdDir, 0755)) + require.NoError(t, os.Symlink(legacyGroup, filepath.Join(fdDir, "5"))) + + require.NoError(t, sysfs.reconcile(context.Background(), nil)) + assertFileValue(t, filepath.Join(sysfs.pciDevicesPath, "0000:82:00.4", "nvidia", "current_vgpu_type"), "0") + assertFileValue(t, filepath.Join(sysfs.pciDevicesPath, "0000:e3:00.4", "nvidia", "current_vgpu_type"), "1148") +} + +func TestVendorVFIOReconcilePreservesVFWhenVFIODeviceProbeFails(t *testing.T) { + t.Parallel() + + sysfs := newTestVendorVFIOSysfs(t) + const vfAddress = "0000:82:00.4" + sysfs.addVF(t, "0000:82:00.0", vfAddress, "42", "1148", "") + vfioDevPath := filepath.Join(sysfs.pciDevicesPath, vfAddress, "vfio-dev") + require.NoError(t, os.RemoveAll(vfioDevPath)) + require.NoError(t, os.WriteFile(vfioDevPath, nil, 0644)) + + require.NoError(t, sysfs.reconcile(context.Background(), nil)) + assertFileValue(t, filepath.Join(sysfs.pciDevicesPath, vfAddress, "nvidia", "current_vgpu_type"), "1148") +} + +func TestVendorVFIOReconcilePreservesVFWhenIOMMUGroupProbeFails(t *testing.T) { + t.Parallel() + + sysfs := newTestVendorVFIOSysfs(t) + const vfAddress = "0000:82:00.4" + sysfs.addVF(t, "0000:82:00.0", vfAddress, "42", "1148", "") + iommuGroupPath := filepath.Join(sysfs.pciDevicesPath, vfAddress, "iommu_group") + require.NoError(t, os.Remove(iommuGroupPath)) + require.NoError(t, os.WriteFile(iommuGroupPath, nil, 0644)) + + require.NoError(t, sysfs.reconcile(context.Background(), nil)) + assertFileValue(t, filepath.Join(sysfs.pciDevicesPath, vfAddress, "nvidia", "current_vgpu_type"), "1148") +} + +func TestVendorVFIOReconcilePreservesVFWhenProcFDDirectoryScanFails(t *testing.T) { + t.Parallel() + + sysfs := newTestVendorVFIOSysfs(t) + const vfAddress = "0000:82:00.4" + sysfs.addVF(t, "0000:82:00.0", vfAddress, "42", "1148", "") + + processPath := filepath.Join(sysfs.procPath, "123") + require.NoError(t, os.MkdirAll(processPath, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(processPath, "fd"), nil, 0644)) + + err := sysfs.reconcile(context.Background(), nil) + require.Error(t, err) + assert.ErrorContains(t, err, "read process 123 file descriptors") + assertFileValue(t, filepath.Join(sysfs.pciDevicesPath, vfAddress, "nvidia", "current_vgpu_type"), "1148") +} + +func TestVendorVFIOReconcilePreservesVFWhenProcFDLinkScanFails(t *testing.T) { + t.Parallel() + + sysfs := newTestVendorVFIOSysfs(t) + const vfAddress = "0000:82:00.4" + sysfs.addVF(t, "0000:82:00.0", vfAddress, "42", "1148", "") + + fdPath := filepath.Join(sysfs.procPath, "123", "fd") + require.NoError(t, os.MkdirAll(fdPath, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(fdPath, "5"), nil, 0644)) + + err := sysfs.reconcile(context.Background(), nil) + require.Error(t, err) + assert.ErrorContains(t, err, "read process 123 file descriptor 5") + assertFileValue(t, filepath.Join(sysfs.pciDevicesPath, vfAddress, "nvidia", "current_vgpu_type"), "1148") +} + +func TestParseCreatableVGPUTypesHeaderOnly(t *testing.T) { + t.Parallel() + + profiles, err := parseCreatableVGPUTypes("ID : vGPU Name\n") + require.NoError(t, err) + assert.Empty(t, profiles) +} + +func TestParseCreatableVGPUTypesRejectsMalformedLine(t *testing.T) { + t.Parallel() + + _, err := parseCreatableVGPUTypes("NVIDIA") + require.Error(t, err) + + _, err = parseCreatableVGPUTypes("not-an-id : NVIDIA L40S-1Q") + require.Error(t, err) +} + +func TestRollbackVendorVFIOCreate(t *testing.T) { + t.Parallel() + + verifyErr := errors.New("verification failed") + t.Run("preserves verification error", func(t *testing.T) { + currentTypePath := filepath.Join(t.TempDir(), "current_vgpu_type") + require.NoError(t, os.WriteFile(currentTypePath, []byte("1148"), 0644)) + + err := rollbackVendorVFIOCreate(currentTypePath, "0000:82:00.4", verifyErr) + require.ErrorIs(t, err, verifyErr) + assertFileValue(t, currentTypePath, "0") + }) + + t.Run("surfaces rollback error", func(t *testing.T) { + currentTypePath := filepath.Join(t.TempDir(), "missing", "current_vgpu_type") + + err := rollbackVendorVFIOCreate(currentTypePath, "0000:82:00.4", verifyErr) + require.ErrorIs(t, err, verifyErr) + assert.ErrorContains(t, err, "roll back vGPU on VF 0000:82:00.4") + }) +} + +func profileAvailability(profiles []GPUProfile, name string) int { + for _, profile := range profiles { + if profile.Name == name { + return profile.Available + } + } + return -1 +} + +type testVendorVFIOSysfs struct { + vendorVFIOSysfs +} + +func newTestVendorVFIOSysfs(t *testing.T) testVendorVFIOSysfs { + t.Helper() + root := t.TempDir() + pci := filepath.Join(root, "sys", "bus", "pci", "devices") + proc := filepath.Join(root, "proc") + vfio := filepath.Join(root, "dev", "vfio", "devices") + require.NoError(t, os.MkdirAll(pci, 0755)) + require.NoError(t, os.MkdirAll(proc, 0755)) + require.NoError(t, os.MkdirAll(vfio, 0755)) + return testVendorVFIOSysfs{vendorVFIOSysfs{ + pciDevicesPath: pci, + procPath: proc, + vfioDevicesPath: vfio, + owners: make(map[string]string), + framebufferByType: make(map[string]int), + }} +} + +func (s testVendorVFIOSysfs) addVF(t *testing.T, parent, address, vfioID, currentType, creatableTypes string) { + t.Helper() + parentPath := filepath.Join(s.pciDevicesPath, parent) + vfPath := filepath.Join(s.pciDevicesPath, address) + nvidiaPath := filepath.Join(vfPath, "nvidia") + require.NoError(t, os.MkdirAll(parentPath, 0755)) + require.NoError(t, os.MkdirAll(nvidiaPath, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(nvidiaPath, "current_vgpu_type"), []byte(currentType), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(nvidiaPath, "creatable_vgpu_types"), []byte(creatableTypes), 0444)) + require.NoError(t, os.Symlink(parentPath, filepath.Join(vfPath, "physfn"))) + vfioName := "vfio" + vfioID + require.NoError(t, os.MkdirAll(filepath.Join(vfPath, "vfio-dev", vfioName), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(s.vfioDevicesPath, vfioName), nil, 0600)) + require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "kernel", "iommu_groups", vfioID), filepath.Join(vfPath, "iommu_group"))) +} + +func assertFileValue(t *testing.T, path, expected string) { + t.Helper() + value, err := os.ReadFile(path) + require.NoError(t, err) + assert.Equal(t, expected, string(value)) +} diff --git a/lib/devices/vgpu_linux.go b/lib/devices/vgpu_linux.go new file mode 100644 index 000000000..a4ccd2db6 --- /dev/null +++ b/lib/devices/vgpu_linux.go @@ -0,0 +1,122 @@ +//go:build linux + +package devices + +import ( + "context" + "fmt" + "path/filepath" +) + +// DiscoverVGPU returns the host's active vGPU framework and virtual functions. +func DiscoverVGPU() (VGPUFramework, []VirtualFunction, error) { + return discoverVGPUWith(discoverMdevVFs, hostVendorVFIO.discoverVFs) +} + +func discoverVGPUWith(discoverMdev, discoverVendorVFIO func() ([]VirtualFunction, error)) (VGPUFramework, []VirtualFunction, error) { + vfs, err := discoverMdev() + if err != nil { + return VGPUFrameworkNone, nil, fmt.Errorf("discover mdev VFs: %w", err) + } + if len(vfs) > 0 { + return VGPUFrameworkMdev, vfs, nil + } + + vfs, err = discoverVendorVFIO() + if err != nil { + return VGPUFrameworkNone, nil, fmt.Errorf("discover vendor VFIO VFs: %w", err) + } + if len(vfs) == 0 { + return VGPUFrameworkNone, nil, nil + } + return VGPUFrameworkVendorVFIO, vfs, nil +} + +// ListGPUProfiles returns available vGPU profiles with availability counts. +func ListGPUProfiles() ([]GPUProfile, error) { + framework, vfs, err := DiscoverVGPU() + if err != nil { + return nil, err + } + return ListGPUProfilesWithVFs(framework, vfs) +} + +// ListGPUProfilesWithVFs returns available profiles for discovered VFs. +func ListGPUProfilesWithVFs(framework VGPUFramework, vfs []VirtualFunction) ([]GPUProfile, error) { + switch framework { + case VGPUFrameworkMdev: + return listMdevGPUProfilesWithVFs(vfs) + case VGPUFrameworkVendorVFIO: + return hostVendorVFIO.listProfiles(vfs) + default: + return nil, nil + } +} + +func CreateVGPU(ctx context.Context, profileName, instanceID string) (*VGPUDevice, error) { + framework, _, err := DiscoverVGPU() + if err != nil { + return nil, err + } + switch framework { + case VGPUFrameworkMdev: + mdev, err := CreateMdev(ctx, profileName, instanceID) + if err != nil { + return nil, err + } + return &VGPUDevice{ + Framework: VGPUFrameworkMdev, + VFAddress: mdev.VFAddress, + ProfileType: mdev.ProfileType, + ProfileName: mdev.ProfileName, + SysfsPath: mdev.SysfsPath, + MdevUUID: mdev.UUID, + }, nil + case VGPUFrameworkVendorVFIO: + return hostVendorVFIO.create(ctx, profileName, instanceID) + default: + return nil, fmt.Errorf("vGPU framework not available") + } +} + +func DestroyVGPU(ctx context.Context, assignment VGPUAssignment) error { + framework := assignment.Framework + if framework == VGPUFrameworkNone && assignment.MdevUUID != "" { + framework = VGPUFrameworkMdev + } + + switch framework { + case VGPUFrameworkMdev: + mdevUUID := assignment.MdevUUID + if mdevUUID == "" { + mdevUUID = filepath.Base(assignment.DevicePath) + } + return DestroyMdev(ctx, mdevUUID) + case VGPUFrameworkVendorVFIO: + return hostVendorVFIO.destroy(ctx, filepath.Base(assignment.DevicePath), assignment.InstanceID) + case VGPUFrameworkNone: + return nil + default: + return fmt.Errorf("unknown vGPU framework %q", framework) + } +} + +// ReconcileVGPUs releases orphaned vGPU assignments. +func ReconcileVGPUs(ctx context.Context, protectedDevicePaths map[string]struct{}) error { + framework, _, err := DiscoverVGPU() + if err != nil { + return err + } + + switch framework { + case VGPUFrameworkMdev: + return ReconcileMdevs(ctx, nil) + case VGPUFrameworkVendorVFIO: + if protectedDevicePaths == nil { + return nil + } + return hostVendorVFIO.reconcile(ctx, protectedDevicePaths) + default: + return nil + } +} diff --git a/lib/devices/vgpu_linux_test.go b/lib/devices/vgpu_linux_test.go new file mode 100644 index 000000000..40b987394 --- /dev/null +++ b/lib/devices/vgpu_linux_test.go @@ -0,0 +1,75 @@ +//go:build linux + +package devices + +import ( + "errors" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDiscoverVGPUWithPropagatesMdevError(t *testing.T) { + t.Parallel() + + discoveryErr := errors.New("mdev discovery failed") + vendorCalled := false + framework, vfs, err := discoverVGPUWith( + func() ([]VirtualFunction, error) { + return nil, discoveryErr + }, + func() ([]VirtualFunction, error) { + vendorCalled = true + return []VirtualFunction{{PCIAddress: "0000:82:00.4"}}, nil + }, + ) + + require.ErrorIs(t, err, discoveryErr) + assert.Equal(t, VGPUFrameworkNone, framework) + assert.Nil(t, vfs) + assert.False(t, vendorCalled) +} + +func TestDiscoverVGPUWithFallsBackFromTypelessMdevBus(t *testing.T) { + t.Parallel() + + root := t.TempDir() + busPath := filepath.Join(root, "sys", "class", "mdev_bus") + require.NoError(t, os.MkdirAll(filepath.Join(busPath, "0000:82:00.4", "mdev_supported_types"), 0755)) + + framework, vfs, err := discoverVGPUWith( + func() ([]VirtualFunction, error) { + return discoverMdevVFsWith(busPath, filepath.Join(root, "sys", "bus", "pci", "devices"), func() ([]MdevDevice, error) { + return nil, nil + }) + }, + func() ([]VirtualFunction, error) { + return []VirtualFunction{{PCIAddress: "0000:82:00.4"}}, nil + }, + ) + + require.NoError(t, err) + assert.Equal(t, VGPUFrameworkVendorVFIO, framework) + assert.Equal(t, []VirtualFunction{{PCIAddress: "0000:82:00.4"}}, vfs) +} + +func TestDiscoverVGPUWithPropagatesVendorVFIOError(t *testing.T) { + t.Parallel() + + discoveryErr := errors.New("vendor VFIO discovery failed") + framework, vfs, err := discoverVGPUWith( + func() ([]VirtualFunction, error) { + return nil, nil + }, + func() ([]VirtualFunction, error) { + return nil, discoveryErr + }, + ) + + require.ErrorIs(t, err, discoveryErr) + assert.Equal(t, VGPUFrameworkNone, framework) + assert.Nil(t, vfs) +} diff --git a/lib/hypervisor/cloudhypervisor/config.go b/lib/hypervisor/cloudhypervisor/config.go index e9f91fe4a..c5f506af9 100644 --- a/lib/hypervisor/cloudhypervisor/config.go +++ b/lib/hypervisor/cloudhypervisor/config.go @@ -125,10 +125,16 @@ func ToVMConfig(cfg hypervisor.VMConfig) vmm.VmConfig { } // Device passthrough configuration + devicePaths := make([]string, 0, len(cfg.PCIDevices)+1) + devicePaths = append(devicePaths, cfg.PCIDevices...) + if cfg.VGPUDevicePath != "" { + devicePaths = append(devicePaths, cfg.VGPUDevicePath) + } + var devices *[]vmm.DeviceConfig - if len(cfg.PCIDevices) > 0 { - deviceConfigs := make([]vmm.DeviceConfig, 0, len(cfg.PCIDevices)) - for _, path := range cfg.PCIDevices { + if len(devicePaths) > 0 { + deviceConfigs := make([]vmm.DeviceConfig, 0, len(devicePaths)) + for _, path := range devicePaths { deviceConfigs = append(deviceConfigs, vmm.DeviceConfig{ Path: path, }) diff --git a/lib/hypervisor/cloudhypervisor/config_test.go b/lib/hypervisor/cloudhypervisor/config_test.go index b5cdb96e9..235be3906 100644 --- a/lib/hypervisor/cloudhypervisor/config_test.go +++ b/lib/hypervisor/cloudhypervisor/config_test.go @@ -8,6 +8,17 @@ import ( "github.com/stretchr/testify/require" ) +func TestToVMConfig_VGPU(t *testing.T) { + t.Parallel() + + path := "/sys/bus/mdev/devices/aa618089-8b16-4d01-a136-25a0f3c73123" + vmCfg := ToVMConfig(hypervisor.VMConfig{VGPUDevicePath: path}) + + require.NotNil(t, vmCfg.Devices) + require.Len(t, *vmCfg.Devices, 1) + assert.Equal(t, path, (*vmCfg.Devices)[0].Path) +} + func TestToVMConfig_GuestMemoryBalloon(t *testing.T) { cfg := hypervisor.VMConfig{ VCPUs: 1, diff --git a/lib/hypervisor/config.go b/lib/hypervisor/config.go index 2562868da..456775868 100644 --- a/lib/hypervisor/config.go +++ b/lib/hypervisor/config.go @@ -24,7 +24,8 @@ type VMConfig struct { VsockSocket string // PCI device passthrough (GPU, etc.) - PCIDevices []string + PCIDevices []string + VGPUDevicePath string // Boot configuration KernelPath string diff --git a/lib/hypervisor/qemu/config.go b/lib/hypervisor/qemu/config.go index 7c65af299..480ef1097 100644 --- a/lib/hypervisor/qemu/config.go +++ b/lib/hypervisor/qemu/config.go @@ -82,13 +82,10 @@ func BuildArgs(cfg hypervisor.VMConfig) []string { args = append(args, "-device", fmt.Sprintf("vhost-vsock-pci,guest-cid=%d", cfg.VsockCID)) } - // PCI device passthrough (GPU, mdev vGPU, etc.) + // Whole-device PCI passthrough (vGPU attaches via VGPUDevicePath below) for _, devicePath := range cfg.PCIDevices { var deviceArg string - if strings.HasPrefix(devicePath, "/sys/bus/mdev/devices/") { - // mdev device (vGPU) - use sysfsdev parameter - deviceArg = fmt.Sprintf("vfio-pci,sysfsdev=%s", devicePath) - } else if strings.HasPrefix(devicePath, "/sys/bus/pci/devices/") { + if strings.HasPrefix(devicePath, "/sys/bus/pci/devices/") { // Full sysfs path for regular PCI device - extract the PCI address // Using filepath.Base is more robust than manual string splitting pciAddr := filepath.Base(strings.TrimSuffix(devicePath, "/")) @@ -100,6 +97,10 @@ func BuildArgs(cfg hypervisor.VMConfig) []string { args = append(args, "-device", deviceArg) } + if cfg.VGPUDevicePath != "" { + args = append(args, "-device", fmt.Sprintf("vfio-pci,sysfsdev=%s", cfg.VGPUDevicePath)) + } + // Serial console output to file. Use a chardev with append=on so QEMU // opens the file with O_APPEND. Without it, QEMU writes at its internal // fd offset; if the file is externally truncated (e.g. log rotation via diff --git a/lib/hypervisor/qemu/config_test.go b/lib/hypervisor/qemu/config_test.go index f4a3e452d..e49b65f28 100644 --- a/lib/hypervisor/qemu/config_test.go +++ b/lib/hypervisor/qemu/config_test.go @@ -122,6 +122,49 @@ func TestBuildArgs_Vsock(t *testing.T) { assert.Contains(t, args, "vhost-vsock-pci,guest-cid=123") } +func TestBuildArgs_VGPU(t *testing.T) { + t.Parallel() + + for _, path := range []string{ + "/sys/bus/mdev/devices/aa618089-8b16-4d01-a136-25a0f3c73123", + "/sys/bus/pci/devices/0000:82:00.4", + } { + path := path + t.Run(path, func(t *testing.T) { + t.Parallel() + args := BuildArgs(hypervisor.VMConfig{ + VCPUs: 1, + MemoryBytes: 512 * 1024 * 1024, + VGPUDevicePath: path, + }) + assert.Contains(t, args, "vfio-pci,sysfsdev="+path) + }) + } +} + +func TestBuildArgs_VGPUAfterPCIDevices(t *testing.T) { + args := BuildArgs(hypervisor.VMConfig{ + VCPUs: 1, + MemoryBytes: 512 * 1024 * 1024, + PCIDevices: []string{"0000:01:00.0"}, + VGPUDevicePath: "/sys/bus/mdev/devices/aa618089-8b16-4d01-a136-25a0f3c73123", + }) + + pciDeviceIndex := -1 + vgpuDeviceIndex := -1 + for i, arg := range args { + switch arg { + case "vfio-pci,host=0000:01:00.0": + pciDeviceIndex = i + case "vfio-pci,sysfsdev=/sys/bus/mdev/devices/aa618089-8b16-4d01-a136-25a0f3c73123": + vgpuDeviceIndex = i + } + } + + assert.Greater(t, pciDeviceIndex, -1) + assert.Greater(t, vgpuDeviceIndex, pciDeviceIndex) +} + func TestBuildArgs_PCIPassthrough(t *testing.T) { cfg := hypervisor.VMConfig{ VCPUs: 1, diff --git a/lib/hypervisor/socket_pid_linux.go b/lib/hypervisor/socket_pid_linux.go index 7f46ebfa3..3c0a973ec 100644 --- a/lib/hypervisor/socket_pid_linux.go +++ b/lib/hypervisor/socket_pid_linux.go @@ -11,29 +11,43 @@ import ( "strings" ) +var procDir = "/proc" + +// soAcceptcon marks a listening socket in /proc/net/unix (__SO_ACCEPTCON). +const soAcceptcon = 0x10000 + // ResolveProcessPID finds the process currently holding the listening Unix -// socket for the given hypervisor control path. -func ResolveProcessPID(socketPath string) (int, error) { - socketRef, err := socketRefForPath(socketPath) - if err == nil { - if pid, refErr := pidBySocketRef(socketRef); refErr == nil { - return pid, nil +// socket for the given hypervisor control path. confirmed reports whether the +// PID was found through socket ownership rather than its command line. +func ResolveProcessPID(socketPath string) (pid int, confirmed bool, err error) { + socketRef, socketErr := socketRefForPath(socketPath) + var refErr error + if socketErr == nil { + pid, refErr = pidBySocketRef(socketRef) + if refErr == nil { + return pid, true, nil } } if pid, cmdErr := pidByCmdline(socketPath); cmdErr == nil { - return pid, nil + return pid, false, nil } - - return 0, fmt.Errorf("resolve process pid for socket %s: no owning process found", socketPath) + if refErr != nil { + return 0, false, refErr + } + if socketErr != nil { + return 0, false, socketErr + } + return 0, false, fmt.Errorf("resolve process pid for socket %s: no owning process found", socketPath) } func pidBySocketRef(socketRef string) (int, error) { - procEntries, err := os.ReadDir("/proc") + procEntries, err := os.ReadDir(procDir) if err != nil { return 0, fmt.Errorf("read /proc: %w", err) } + var scanErr error for _, entry := range procEntries { if !entry.IsDir() { continue @@ -44,13 +58,15 @@ func pidBySocketRef(socketRef string) (int, error) { continue } - fdEntries, err := os.ReadDir(filepath.Join("/proc", entry.Name(), "fd")) + fdEntries, err := os.ReadDir(filepath.Join(procDir, entry.Name(), "fd")) if err != nil { + scanErr = err continue } for _, fdEntry := range fdEntries { - target, err := os.Readlink(filepath.Join("/proc", entry.Name(), "fd", fdEntry.Name())) + target, err := os.Readlink(filepath.Join(procDir, entry.Name(), "fd", fdEntry.Name())) if err != nil { + scanErr = err continue } if strings.TrimSpace(target) == socketRef { @@ -59,15 +75,19 @@ func pidBySocketRef(socketRef string) (int, error) { } } + if scanErr != nil { + return 0, fmt.Errorf("resolve process pid for %s: inspect process fds: %w", socketRef, scanErr) + } return 0, fmt.Errorf("resolve process pid for %s: no owning process found", socketRef) } func pidByCmdline(socketPath string) (int, error) { - procEntries, err := os.ReadDir("/proc") + procEntries, err := os.ReadDir(procDir) if err != nil { return 0, fmt.Errorf("read /proc: %w", err) } + var scanErr error for _, entry := range procEntries { if !entry.IsDir() { continue @@ -78,8 +98,12 @@ func pidByCmdline(socketPath string) (int, error) { continue } - cmdline, err := os.ReadFile(filepath.Join("/proc", entry.Name(), "cmdline")) - if err != nil || len(cmdline) == 0 { + cmdline, err := os.ReadFile(filepath.Join(procDir, entry.Name(), "cmdline")) + if err != nil { + scanErr = err + continue + } + if len(cmdline) == 0 { continue } for _, arg := range strings.Split(string(cmdline), "\x00") { @@ -89,17 +113,21 @@ func pidByCmdline(socketPath string) (int, error) { } } + if scanErr != nil { + return 0, fmt.Errorf("resolve process pid for socket %s: inspect process command lines: %w", socketPath, scanErr) + } return 0, fmt.Errorf("resolve process pid for socket %s: no matching command line found", socketPath) } func socketRefForPath(socketPath string) (string, error) { - file, err := os.Open("/proc/net/unix") + file, err := os.Open(filepath.Join(procDir, "net", "unix")) if err != nil { return "", fmt.Errorf("open /proc/net/unix: %w", err) } defer file.Close() scanner := bufio.NewScanner(file) + var socketRef string for scanner.Scan() { fields := strings.Fields(scanner.Text()) if len(fields) < 7 { @@ -112,14 +140,26 @@ func socketRefForPath(socketPath string) (string, error) { if path != socketPath { continue } + // Accepted server-side sockets list the bound path too; only the + // listener identifies the owning process. + flags, parseErr := strconv.ParseUint(fields[3], 16, 32) + if parseErr != nil || flags&soAcceptcon == 0 { + continue + } inode := fields[6] if inode == "" { break } - return fmt.Sprintf("socket:[%s]", inode), nil + if socketRef != "" { + return "", fmt.Errorf("resolve process pid for socket %s: multiple socket inodes found", socketPath) + } + socketRef = fmt.Sprintf("socket:[%s]", inode) } if err := scanner.Err(); err != nil { return "", fmt.Errorf("scan /proc/net/unix: %w", err) } + if socketRef != "" { + return socketRef, nil + } return "", fmt.Errorf("resolve process pid for socket %s: socket inode not found", socketPath) } diff --git a/lib/hypervisor/socket_pid_linux_test.go b/lib/hypervisor/socket_pid_linux_test.go index 270524532..cb6f5db6c 100644 --- a/lib/hypervisor/socket_pid_linux_test.go +++ b/lib/hypervisor/socket_pid_linux_test.go @@ -19,7 +19,65 @@ func TestResolveProcessPID(t *testing.T) { require.NoError(t, err) defer listener.Close() - pid, err := ResolveProcessPID(socketPath) + pid, confirmed, err := ResolveProcessPID(socketPath) require.NoError(t, err) + require.True(t, confirmed) require.Equal(t, os.Getpid(), pid) } + +func TestResolveProcessPIDIgnoresConnectedSocketEntries(t *testing.T) { + tmpDir := t.TempDir() + socketPath := filepath.Join(tmpDir, "test.sock") + + listener, err := net.Listen("unix", socketPath) + require.NoError(t, err) + defer listener.Close() + + // Accepted server-side sockets share the listener's path in + // /proc/net/unix; they must not make the listener's inode ambiguous. + conn, err := net.Dial("unix", socketPath) + require.NoError(t, err) + defer conn.Close() + accepted, err := listener.Accept() + require.NoError(t, err) + defer accepted.Close() + + pid, confirmed, err := ResolveProcessPID(socketPath) + require.NoError(t, err) + require.True(t, confirmed) + require.Equal(t, os.Getpid(), pid) +} + +func TestResolveProcessPIDIsUnconfirmedForDuplicateSocketPaths(t *testing.T) { + oldProcDir := procDir + procDir = t.TempDir() + t.Cleanup(func() { procDir = oldProcDir }) + + socketPath := "/tmp/test.sock" + require.NoError(t, os.MkdirAll(filepath.Join(procDir, "net"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(procDir, "net", "unix"), []byte( + "00000000: 00000002 00000000 00010000 0001 01 12345 "+socketPath+"\n"+ + "00000000: 00000002 00000000 00010000 0001 01 67890 "+socketPath+"\n"), 0o644)) + + _, confirmed, err := ResolveProcessPID(socketPath) + require.ErrorContains(t, err, "multiple socket inodes found") + require.False(t, confirmed) +} + +func TestResolveProcessPIDFailsWhenFDIsUnreadable(t *testing.T) { + oldProcDir := procDir + procDir = t.TempDir() + t.Cleanup(func() { procDir = oldProcDir }) + + socketPath := "/tmp/test.sock" + require.NoError(t, os.MkdirAll(filepath.Join(procDir, "net"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(procDir, "net", "unix"), []byte("00000000: 00000002 00000000 00010000 0001 01 12345 "+socketPath+"\n"), 0o644)) + fdDir := filepath.Join(procDir, "123", "fd") + require.NoError(t, os.MkdirAll(fdDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(fdDir, "3"), nil, 0o644)) + + _, confirmed, err := ResolveProcessPID(socketPath) + require.Error(t, err) + require.False(t, confirmed) + require.ErrorContains(t, err, "inspect process fds") +} diff --git a/lib/hypervisor/socket_pid_other.go b/lib/hypervisor/socket_pid_other.go index 75db657e6..4ee09d71b 100644 --- a/lib/hypervisor/socket_pid_other.go +++ b/lib/hypervisor/socket_pid_other.go @@ -6,6 +6,6 @@ import "fmt" // ResolveProcessPID is only implemented on Linux, where the project relies on // /proc socket metadata for runtime PID discovery. -func ResolveProcessPID(socketPath string) (int, error) { - return 0, fmt.Errorf("resolve process pid for socket %s: not supported on this platform", socketPath) +func ResolveProcessPID(socketPath string) (int, bool, error) { + return 0, false, fmt.Errorf("resolve process pid for socket %s: not supported on this platform", socketPath) } diff --git a/lib/instances/create.go b/lib/instances/create.go index fcd36aa15..731351f30 100644 --- a/lib/instances/create.go +++ b/lib/instances/create.go @@ -52,11 +52,11 @@ var systemDirectories = []string{ "/var", } -func wrapCreateMdevErr(profile string, err error) error { +func wrapCreateVGPUErr(profile string, err error) error { if errors.Is(err, devices.ErrVGPUNotSupportedOnMacOS) { return fmt.Errorf("%w: %w", ErrInvalidRequest, err) } - return fmt.Errorf("create vGPU mdev for profile %s: %w", profile, err) + return fmt.Errorf("create vGPU for profile %s: %w", profile, err) } // generateVsockCID converts first 8 chars of instance ID to a unique CID @@ -260,13 +260,27 @@ func (m *manager) createInstance( // whatever devices have been attached when cleanup runs. var attachedDeviceIDs []string var resolvedDeviceIDs []string + var gpuDevice *devices.VGPUDevice var gpuProfile string + var gpuFramework devices.VGPUFramework + var gpuDevicePath string var gpuMdevUUID string - - // Setup cleanup stack early so device attachment errors trigger cleanup + var stored *StoredMetadata + var retainedVGPU *StoredMetadata + + // Setup cleanup stack early so device attachment errors trigger cleanup. + // When rollback cannot release a vGPU assignment, report whether its + // retention record was persisted. The wrapping defer is registered first + // so it runs after cu.Clean has attempted to retain the metadata. + vgpuPersisted := false + defer func() { + if retErr != nil && retainedVGPU != nil { + retErr = &VGPUCleanupPendingError{InstanceID: id, Retained: vgpuPersisted, Err: retErr} + } + }() cu := cleanup.Make(func() { log.DebugContext(ctx, "cleaning up instance on error", "instance_id", id) - m.deleteInstanceData(id) + vgpuPersisted = m.cleanupFailedCreate(ctx, id, retainedVGPU) }) defer cu.Clean() @@ -280,23 +294,48 @@ func (m *manager) createInstance( }) } - // Handle vGPU profile request - create mdev device + // Handle vGPU profile request if req.GPU != nil && req.GPU.Profile != "" { - log.InfoContext(ctx, "creating vGPU mdev", "instance_id", id, "profile", req.GPU.Profile) - mdev, err := devices.CreateMdev(ctx, req.GPU.Profile, id) + log.InfoContext(ctx, "creating vGPU", "instance_id", id, "profile", req.GPU.Profile) + gpuDevice, err = m.createVGPUDevice(ctx, req.GPU.Profile, id) if err != nil { - log.ErrorContext(ctx, "failed to create mdev", "profile", req.GPU.Profile, "error", err) - return nil, wrapCreateMdevErr(req.GPU.Profile, err) + log.ErrorContext(ctx, "failed to create vGPU", "profile", req.GPU.Profile, "error", err) + return nil, wrapCreateVGPUErr(req.GPU.Profile, err) } - gpuProfile = req.GPU.Profile - gpuMdevUUID = mdev.UUID - log.InfoContext(ctx, "created vGPU mdev", "instance_id", id, "profile", gpuProfile, "uuid", gpuMdevUUID) + gpuProfile = gpuDevice.ProfileName + gpuFramework = gpuDevice.Framework + gpuDevicePath = gpuDevice.SysfsPath + gpuMdevUUID = gpuDevice.MdevUUID - // Add mdev cleanup to stack + // Add vGPU cleanup to stack cu.Add(func() { - log.DebugContext(ctx, "destroying mdev on cleanup", "instance_id", id, "uuid", gpuMdevUUID) - if err := devices.DestroyMdev(ctx, gpuMdevUUID); err != nil { - log.WarnContext(ctx, "failed to destroy mdev on cleanup", "instance_id", id, "uuid", gpuMdevUUID, "error", err) + assignment := devices.VGPUAssignment{ + Framework: gpuDevice.Framework, + DevicePath: gpuDevice.SysfsPath, + MdevUUID: gpuDevice.MdevUUID, + InstanceID: id, + } + if err := m.destroyVGPUAssignment(ctx, assignment); err != nil { + log.WarnContext(ctx, "failed to destroy vGPU on cleanup", "instance_id", id, "error", err) + retainedVGPU = stored + if retainedVGPU == nil { + retainedVGPU = &StoredMetadata{ + Id: id, + Name: req.Name, + Image: req.Image, + ResolvedImage: resolvedImageRef, + Platform: imageInfo.Platform, + CreatedAt: time.Now(), + HypervisorType: hvType, + HypervisorVersion: hvVersion, + SocketPath: m.paths.InstanceSocket(id, starter.SocketName()), + DataDir: m.paths.InstanceDir(id), + GPUProfile: gpuDevice.ProfileName, + GPUFramework: gpuDevice.Framework, + GPUDevicePath: gpuDevice.SysfsPath, + GPUMdevUUID: gpuDevice.MdevUUID, + } + } } }) } @@ -332,7 +371,7 @@ func (m *manager) createInstance( } // 11. Create instance metadata - stored := &StoredMetadata{ + stored = &StoredMetadata{ Id: id, Name: req.Name, Image: req.Image, @@ -364,6 +403,8 @@ func (m *manager) createInstance( VsockSocket: vsockSocket, Devices: resolvedDeviceIDs, GPUProfile: gpuProfile, + GPUFramework: gpuFramework, + GPUDevicePath: gpuDevicePath, GPUMdevUUID: gpuMdevUUID, Entrypoint: req.Entrypoint, Cmd: req.Cmd, @@ -564,6 +605,38 @@ func (m *manager) createInstance( return &finalInst, nil } +// cleanupFailedCreate reports whether the retention record for a vGPU +// assignment whose release failed during rollback was persisted. +func (m *manager) cleanupFailedCreate(ctx context.Context, id string, retainedVGPU *StoredMetadata) bool { + if retainedVGPU == nil { + m.deleteInstanceData(id) + return false + } + + log := logger.FromContext(ctx) + if err := m.ensureDirectories(id); err != nil { + log.ErrorContext(ctx, "failed to retain instance data after vGPU cleanup failure", "instance_id", id, "error", err) + if derr := m.deleteInstanceData(id); derr != nil { + log.ErrorContext(ctx, "failed to delete stale instance data after retention failure", "instance_id", id, "error", derr) + } + return false + } + retained := StoredMetadata{ + Id: id, + GPUFramework: retainedVGPU.GPUFramework, + GPUDevicePath: retainedVGPU.GPUDevicePath, + GPUMdevUUID: retainedVGPU.GPUMdevUUID, + } + if err := m.saveMetadata(&metadata{StoredMetadata: retained}); err != nil { + log.ErrorContext(ctx, "failed to retain vGPU assignment metadata after cleanup failure", "instance_id", id, "error", err) + if derr := m.deleteInstanceData(id); derr != nil { + log.ErrorContext(ctx, "failed to delete stale instance data after retention failure", "instance_id", id, "error", derr) + } + return false + } + return true +} + // validateCreateRequest validates the create instance request. // The request is mutated in-place to persist normalized egress/credential policy fields. func validateCreateRequest(req *CreateInstanceRequest) error { @@ -788,10 +861,10 @@ func (m *manager) startAndBootVM( } func resolveRuntimeHypervisorPID(log *slog.Logger, socketPath string, fallbackPID int) int { - if processExists(fallbackPID) { + if ProcessExists(fallbackPID) { return fallbackPID } - pid, err := hypervisor.ResolveProcessPID(socketPath) + pid, _, err := hypervisor.ResolveProcessPID(socketPath) if err != nil { log.Debug("using fallback hypervisor pid", "socket_path", socketPath, "pid", fallbackPID, "error", err) return fallbackPID @@ -885,12 +958,6 @@ func (m *manager) buildHypervisorConfig(ctx context.Context, inst *Instance, ima } } - // Add vGPU mdev device if configured - if inst.GPUMdevUUID != "" { - mdevPath := filepath.Join("/sys/bus/mdev/devices", inst.GPUMdevUUID) - pciDevices = append(pciDevices, mdevPath) - } - // Build topology if available var topology *hypervisor.CPUTopology if hostTopo := calculateGuestTopology(inst.Vcpus, m.hostTopology); hostTopo != nil { @@ -910,21 +977,22 @@ func (m *manager) buildHypervisorConfig(ctx context.Context, inst *Instance, ima } return hypervisor.VMConfig{ - VCPUs: inst.Vcpus, - MemoryBytes: inst.Size, - HotplugBytes: inst.HotplugSize, - Topology: topology, - GuestMemory: m.guestMemoryConfig(), - Disks: disks, - Networks: networks, - SerialLogPath: m.paths.InstanceAppLog(inst.Id), - VsockCID: inst.VsockCID, - VsockSocket: inst.VsockSocket, - PCIDevices: pciDevices, - KernelPath: kernelPath, - InitrdPath: initrdPath, - KernelArgs: m.kernelArgs(inst.HypervisorType), - EnableRosetta: inst.EnableRosetta, + VCPUs: inst.Vcpus, + MemoryBytes: inst.Size, + HotplugBytes: inst.HotplugSize, + Topology: topology, + GuestMemory: m.guestMemoryConfig(), + Disks: disks, + Networks: networks, + SerialLogPath: m.paths.InstanceAppLog(inst.Id), + VsockCID: inst.VsockCID, + VsockSocket: inst.VsockSocket, + PCIDevices: pciDevices, + VGPUDevicePath: storedVGPUDevicePath(&inst.StoredMetadata), + KernelPath: kernelPath, + InitrdPath: initrdPath, + KernelArgs: m.kernelArgs(inst.HypervisorType), + EnableRosetta: inst.EnableRosetta, }, nil } diff --git a/lib/instances/create_mdev_test.go b/lib/instances/create_mdev_test.go index db546c153..05b3e9d8c 100644 --- a/lib/instances/create_mdev_test.go +++ b/lib/instances/create_mdev_test.go @@ -45,7 +45,7 @@ func TestCreateInstanceRejectsUnsupportedVGPUBeforeResourceReservation(t *testin assert.Zero(t, validator.reserveCalls) } -func TestWrapCreateMdevErr(t *testing.T) { +func TestWrapCreateVGPUErr(t *testing.T) { t.Parallel() for _, tc := range []struct { @@ -61,15 +61,15 @@ func TestWrapCreateMdevErr(t *testing.T) { wantInvalidRequest: true, }, { - name: "other mdev error", + name: "other vGPU error", err: errors.New("boom"), - wantMessage: "create vGPU mdev for profile profile: boom", + wantMessage: "create vGPU for profile profile: boom", }, } { t.Run(tc.name, func(t *testing.T) { t.Parallel() - err := wrapCreateMdevErr("profile", tc.err) + err := wrapCreateVGPUErr("profile", tc.err) assert.ErrorIs(t, err, tc.err) if tc.wantInvalidRequest { diff --git a/lib/instances/delete.go b/lib/instances/delete.go index 07c3da1b2..0d8b069aa 100644 --- a/lib/instances/delete.go +++ b/lib/instances/delete.go @@ -7,7 +7,6 @@ import ( "syscall" "time" - "github.com/kernel/hypeman/lib/devices" "github.com/kernel/hypeman/lib/guest" "github.com/kernel/hypeman/lib/hypervisor" "github.com/kernel/hypeman/lib/logger" @@ -86,6 +85,14 @@ func (m *manager) deleteInstanceWithOptions( guest.CloseConn(dialer.Key()) } + // 3b. Block the restart policy before any teardown. If the delete fails + // partway (e.g. a failed vGPU release) the metadata is retained with the + // VMM already stopped, and without this marker the restart policy + // controller would start the instance again. + if err := m.markRestartManualStopLocked(ctx, id); err != nil { + return fmt.Errorf("block restart policy before delete: %w", err) + } + // 4. If active, try graceful guest shutdown before force kill. gracefulShutdown := false if !options.skipGracefulShutdown && (inst.State == StateRunning || inst.State == StateInitializing) { @@ -119,13 +126,24 @@ func (m *manager) deleteInstanceWithOptions( err := m.killHypervisor(killCtx, &inst) killSpanEnd(err) if err != nil { - // Log error but continue with cleanup - // Best effort to clean up even if hypervisor is unresponsive - log.WarnContext(ctx, "failed to kill hypervisor, continuing with cleanup", "instance_id", id, "error", err) + // The hypervisor may still be running, so tearing down its vGPU, + // network, and devices is unsafe. The restart policy is already + // blocked and the metadata is retained, so a retried delete is safe. + log.ErrorContext(ctx, "failed to kill hypervisor; retaining instance metadata", "instance_id", id, "error", err) + return fmt.Errorf("kill hypervisor: %w", err) } } m.closeFirecrackerUFFDSession(ctx, stored) + // 5b. Release the vGPU assignment if present, before any network, device, + // or volume teardown. A failed release retains the instance metadata; the + // VMM has already been stopped, but its attachments are intact and the + // restart policy is blocked, so a retried delete is safe. + if err := m.releaseStoredVGPU(ctx, stored); err != nil { + log.ErrorContext(ctx, "failed to destroy vGPU; retaining instance metadata", "instance_id", id, "error", err) + return fmt.Errorf("destroy vGPU: %w", err) + } + // 6. Release network allocation if inst.NetworkEnabled { m.unregisterEgressProxyInstance(ctx, id) @@ -171,15 +189,6 @@ func (m *manager) deleteInstanceWithOptions( } } - // 7c. Destroy vGPU mdev device if present - if inst.GPUMdevUUID != "" { - log.InfoContext(ctx, "destroying vGPU mdev", "instance_id", id, "uuid", inst.GPUMdevUUID) - if err := devices.DestroyMdev(ctx, inst.GPUMdevUUID); err != nil { - // Log error but continue with cleanup - log.WarnContext(ctx, "failed to destroy mdev, continuing with cleanup", "instance_id", id, "uuid", inst.GPUMdevUUID, "error", err) - } - } - // 8. Delete all instance data log.DebugContext(ctx, "deleting instance data", "instance_id", id) _, dataSpanEnd := m.startLifecycleStep(ctx, "delete_instance_data", @@ -200,43 +209,32 @@ func (m *manager) deleteInstanceWithOptions( // killHypervisor force kills the hypervisor process without graceful shutdown // Used only for delete operations where we're removing all data anyway. // For operations that need graceful shutdown (like standby), use the hypervisor API directly. +// It returns an error when the hypervisor may still be running: socket +// ownership of a live stored PID could not be confirmed, or the process did +// not exit after SIGKILL. Callers must not tear down instance resources in +// that case. func (m *manager) killHypervisor(ctx context.Context, inst *Instance) error { log := logger.FromContext(ctx) - // If we have a PID, kill the process immediately - if inst.HypervisorPID != nil { - pid := *inst.HypervisorPID - - // Check if process exists - if err := syscall.Kill(pid, 0); err == nil { - // Process exists - kill it immediately with SIGKILL - // No graceful shutdown needed since we're deleting all data - log.DebugContext(ctx, "killing hypervisor process", "instance_id", inst.Id, "pid", pid) - if err := syscall.Kill(pid, syscall.SIGKILL); err != nil { - log.WarnContext(ctx, "failed to kill hypervisor process", "instance_id", inst.Id, "pid", pid, "error", err) - } - - // Wait for process to die and reap it to prevent zombies - // SIGKILL should be instant, but give it a moment - for i := 0; i < 50; i++ { // 50 * 100ms = 5 seconds - var wstatus syscall.WaitStatus - wpid, err := syscall.Wait4(pid, &wstatus, syscall.WNOHANG, nil) - if err != nil || wpid == pid { - // Process reaped successfully or error (likely ECHILD if already reaped) - log.DebugContext(ctx, "hypervisor process killed and reaped", "instance_id", inst.Id, "pid", pid) - break - } - if i == 49 { - log.WarnContext(ctx, "hypervisor process did not exit in time", "instance_id", inst.Id, "pid", pid) - } - time.Sleep(100 * time.Millisecond) - } - } else { - log.DebugContext(ctx, "hypervisor process not running", "instance_id", inst.Id, "pid", pid) + pid, err := resolveLiveHypervisorPID(inst.HypervisorPID, inst.SocketPath) + if err != nil { + return err + } + if pid > 0 { + if inst.HypervisorPID != nil && pid != *inst.HypervisorPID { + log.WarnContext(ctx, "stored hypervisor PID does not own the instance socket, killing the socket owner", + "instance_id", inst.Id, "stored_pid", *inst.HypervisorPID, "owner_pid", pid) + } + log.DebugContext(ctx, "killing hypervisor process", "instance_id", inst.Id, "pid", pid) + if err := syscall.Kill(pid, syscall.SIGKILL); err != nil && err != syscall.ESRCH { + return fmt.Errorf("kill hypervisor process %d: %w", pid, err) + } + if !WaitForProcessExit(pid, 30*time.Second) { + return fmt.Errorf("hypervisor process %d did not exit after SIGKILL", pid) } } - // Clean up socket if it still exists + // The hypervisor is confirmed gone; remove its stale socket. os.Remove(inst.SocketPath) return nil @@ -262,12 +260,12 @@ func WaitForProcessExit(pid int, timeout time.Duration) bool { // Process still running (or wait status not yet available). case waitErr == syscall.ECHILD: // Not our child (or already reaped elsewhere). Fall back to existence check. - if err := syscall.Kill(pid, 0); err != nil { + if !ProcessExists(pid) { return true } default: // Best effort fallback on transient/unexpected wait errors. - if err := syscall.Kill(pid, 0); err != nil { + if !ProcessExists(pid) { return true } } diff --git a/lib/instances/delete_test.go b/lib/instances/delete_test.go index 0ed8efb0b..dc03b5d26 100644 --- a/lib/instances/delete_test.go +++ b/lib/instances/delete_test.go @@ -2,6 +2,7 @@ package instances import ( "os/exec" + "syscall" "testing" "time" @@ -22,6 +23,15 @@ func TestWaitForProcessExit_ReapsZombieChild(t *testing.T) { assert.Less(t, elapsed, 250*time.Millisecond, "reaping should be quick") } +func TestWaitForProcessExit_EPERMProcessIsAlive(t *testing.T) { + t.Parallel() + if syscall.Kill(1, 0) == nil { + t.Skip("running as root") + } + + assert.False(t, WaitForProcessExit(1, 100*time.Millisecond)) +} + func TestWaitForProcessExit_TimesOutForRunningProcess(t *testing.T) { t.Parallel() cmd := exec.Command("sleep", "2") diff --git a/lib/instances/fork.go b/lib/instances/fork.go index ea6d3a4c5..7354b3ed9 100644 --- a/lib/instances/fork.go +++ b/lib/instances/fork.go @@ -298,6 +298,11 @@ func (m *manager) forkInstanceFromStoppedOrStandby(ctx context.Context, id strin // phase (Standby for snapshot forks, Stopped for stopped forks) will be // recorded by the appropriate operation when the fork is acted on. forkMeta.Phases.Reset() + // A vGPU assignment is never shared with a fork: normally stop already + // released it, and an assignment retained by a failed release must stay + // with the source so only one instance retries it. The fork acquires its + // own vGPU on start from GPUProfile. + clearStoredVGPUDevice(&forkMeta) switch source.State { case StateStandby: forkMeta.Phases.Record(phasetracking.PhaseStandby, now) diff --git a/lib/instances/fork_test.go b/lib/instances/fork_test.go index 2dc632660..e88bff9f9 100644 --- a/lib/instances/fork_test.go +++ b/lib/instances/fork_test.go @@ -17,6 +17,7 @@ import ( "time" "github.com/kernel/hypeman/lib/autostandby" + "github.com/kernel/hypeman/lib/devices" "github.com/kernel/hypeman/lib/guest" "github.com/kernel/hypeman/lib/healthcheck" "github.com/kernel/hypeman/lib/hypervisor" @@ -29,6 +30,39 @@ import ( "github.com/stretchr/testify/require" ) +func TestForkInstanceClearsVGPUAssignment(t *testing.T) { + manager, _ := setupTestManager(t) + ctx := context.Background() + hvType := hypervisor.Type("fork-vgpu-test") + hypervisor.RegisterCapabilities(hvType, hypervisor.Capabilities{SupportsConcurrentForkPrepare: true}) + manager.vmStarters[hvType] = concurrentForkPrepareTestStarter{} + + sourceID := "fork-vgpu-source" + createStoppedSnapshotSourceFixture(t, manager, sourceID, sourceID, hvType) + + // A retained assignment (release failed during stop) must stay with the + // source; the fork keeps only the profile and acquires its own vGPU on + // start. + meta, err := manager.loadMetadata(sourceID) + require.NoError(t, err) + meta.GPUProfile = "NVIDIA L40S-2Q" + meta.GPUFramework = devices.VGPUFramework("future-framework") + meta.GPUDevicePath = "/sys/bus/pci/devices/0000:82:00.4" + meta.GPUMdevUUID = "retained-uuid" + require.NoError(t, manager.saveMetadata(meta)) + + forked, err := manager.ForkInstance(ctx, sourceID, ForkInstanceRequest{Name: "fork-vgpu-copy"}) + require.NoError(t, err) + assert.Equal(t, "NVIDIA L40S-2Q", forked.GPUProfile) + assert.Equal(t, devices.VGPUFrameworkNone, forked.GPUFramework) + assert.Empty(t, forked.GPUDevicePath) + assert.Empty(t, forked.GPUMdevUUID) + + source, err := manager.loadMetadata(sourceID) + require.NoError(t, err) + assert.Equal(t, "/sys/bus/pci/devices/0000:82:00.4", source.GPUDevicePath) +} + func TestForkInstance_VZStoppedSourceSupported(t *testing.T) { t.Parallel() manager, _ := setupTestManager(t) diff --git a/lib/instances/guestmemory_linux_test.go b/lib/instances/guestmemory_linux_test.go index c206dc198..92e7d6014 100644 --- a/lib/instances/guestmemory_linux_test.go +++ b/lib/instances/guestmemory_linux_test.go @@ -211,10 +211,10 @@ func requireHypervisorPID(t *testing.T, ctx context.Context, mgr *manager, insta t.Helper() inst, err := mgr.GetInstance(ctx, instanceID) require.NoError(t, err) - if inst.HypervisorPID != nil && processExists(*inst.HypervisorPID) { + if inst.HypervisorPID != nil && ProcessExists(*inst.HypervisorPID) { return *inst.HypervisorPID } - if pid, err := hypervisor.ResolveProcessPID(inst.SocketPath); err == nil { + if pid, _, err := hypervisor.ResolveProcessPID(inst.SocketPath); err == nil { return pid } require.NotNil(t, inst.HypervisorPID) diff --git a/lib/instances/lifecycle_noop_test.go b/lib/instances/lifecycle_noop_test.go index 5ca7515fa..f36aeb84f 100644 --- a/lib/instances/lifecycle_noop_test.go +++ b/lib/instances/lifecycle_noop_test.go @@ -3,14 +3,17 @@ package instances import ( "context" "errors" + "net" "os" "path/filepath" "sync" "testing" "time" + "github.com/kernel/hypeman/lib/devices" "github.com/kernel/hypeman/lib/hypervisor" "github.com/kernel/hypeman/lib/paths" + restartpolicy "github.com/kernel/hypeman/lib/restart-policy" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -147,6 +150,206 @@ func TestLifecycleNoopStandbyWithOptionsStillRejectsStandbyInstance(t *testing.T assertNoLifecycleEvent(t, events) } +func TestDeleteRetainsMetadataWhenVGPUReleaseFails(t *testing.T) { + m, id := newLifecycleNoopManagerWithInstance(t, StateStopped, time.Now().UTC()) + meta, err := m.loadMetadata(id) + require.NoError(t, err) + meta.GPUProfile = "NVIDIA L40S-2Q" + meta.GPUFramework = devices.VGPUFramework("future-framework") + meta.GPUDevicePath = "/sys/bus/pci/devices/0000:82:00.4" + require.NoError(t, m.saveMetadata(meta)) + + err = m.DeleteInstance(context.Background(), id) + require.Error(t, err) + + stored, err := m.loadMetadata(id) + require.NoError(t, err) + assert.Equal(t, devices.VGPUFramework("future-framework"), stored.GPUFramework) + assert.Equal(t, "/sys/bus/pci/devices/0000:82:00.4", stored.GPUDevicePath) +} + +func TestDeleteBlocksRestartPolicyWhenVGPUReleaseFails(t *testing.T) { + m, id := newLifecycleNoopManagerWithInstance(t, StateStopped, time.Now().UTC()) + meta, err := m.loadMetadata(id) + require.NoError(t, err) + meta.RestartPolicy = &restartpolicy.Policy{Policy: restartpolicy.PolicyAlways} + meta.GPUProfile = "NVIDIA L40S-2Q" + meta.GPUFramework = devices.VGPUFramework("future-framework") + meta.GPUDevicePath = "/sys/bus/pci/devices/0000:82:00.4" + require.NoError(t, m.saveMetadata(meta)) + + err = m.DeleteInstance(context.Background(), id) + require.Error(t, err) + + stored, err := m.loadMetadata(id) + require.NoError(t, err) + assert.Equal(t, restartpolicy.BlockedReasonManualStop, stored.RestartStatus.BlockedReason, + "a failed delete must not leave the instance restartable") +} + +func TestDeleteDropsStaleVGPUClaimedByLiveInstance(t *testing.T) { + now := time.Now().UTC() + m, id := newLifecycleNoopManagerWithInstance(t, StateStopped, now) + meta, err := m.loadMetadata(id) + require.NoError(t, err) + meta.GPUProfile = "NVIDIA L40S-2Q" + meta.GPUFramework = devices.VGPUFrameworkVendorVFIO + meta.GPUDevicePath = "/sys/bus/pci/devices/0000:82:00.4" + require.NoError(t, m.saveMetadata(meta)) + + claimantID := "inst-live-claimant" + require.NoError(t, m.ensureDirectories(claimantID)) + pid := os.Getpid() + // Bind under /tmp: a t.TempDir()-derived path exceeds the macOS AF_UNIX + // path limit. + socketDir, err := os.MkdirTemp("/tmp", "hypeman-claimant-socket-") + require.NoError(t, err) + t.Cleanup(func() { + _ = os.RemoveAll(socketDir) + }) + socketPath := filepath.Join(socketDir, "noop.sock") + listener, err := net.Listen("unix", socketPath) + require.NoError(t, err) + defer listener.Close() + require.NoError(t, m.saveMetadata(&metadata{StoredMetadata: StoredMetadata{ + Id: claimantID, + Name: claimantID, + Image: "test-image", + CreatedAt: now, + HypervisorType: lifecycleNoopHypervisorType, + HypervisorPID: &pid, + SocketPath: socketPath, + DataDir: m.paths.InstanceDir(claimantID), + GPUProfile: "NVIDIA L40S-2Q", + GPUFramework: devices.VGPUFrameworkVendorVFIO, + GPUDevicePath: "/sys/bus/pci/devices/0000:82:00.4", + }})) + + require.NoError(t, m.DeleteInstance(context.Background(), id)) + + _, err = m.loadMetadata(id) + require.Error(t, err, "deleted instance metadata should be gone") + claimant, err := m.loadMetadata(claimantID) + require.NoError(t, err) + assert.Equal(t, "/sys/bus/pci/devices/0000:82:00.4", claimant.GPUDevicePath, "live claimant keeps its assignment") +} + +func TestDeleteReleasesVGPUBeforeTeardown(t *testing.T) { + m, id := newLifecycleNoopManagerWithInstance(t, StateStopped, time.Now().UTC()) + deviceManager := &recordingDeviceManager{} + m.deviceManager = deviceManager + meta, err := m.loadMetadata(id) + require.NoError(t, err) + meta.GPUProfile = "NVIDIA L40S-2Q" + meta.GPUFramework = devices.VGPUFramework("future-framework") + meta.GPUDevicePath = "/sys/bus/pci/devices/0000:82:00.4" + meta.Devices = []string{"dev-1"} + require.NoError(t, m.saveMetadata(meta)) + + err = m.DeleteInstance(context.Background(), id) + require.Error(t, err) + assert.ErrorContains(t, err, "destroy vGPU") + assert.Empty(t, deviceManager.detached) + assert.Empty(t, deviceManager.unbound) + + stored, err := m.loadMetadata(id) + require.NoError(t, err) + assert.Equal(t, devices.VGPUFramework("future-framework"), stored.GPUFramework) +} + +// A stale release during start must be persisted immediately: if start fails +// later (here at vGPU recreation on a host without VFs), the on-disk metadata +// must no longer point at the already-released device. +func TestStartPersistsStaleVGPUReleaseImmediately(t *testing.T) { + m, id := newLifecycleNoopManagerWithInstance(t, StateStopped, time.Now().UTC()) + m.imageManager = readyFixtureImageManager{name: "test-image"} + meta, err := m.loadMetadata(id) + require.NoError(t, err) + meta.GPUProfile = "NVIDIA L40S-2Q" + meta.GPUFramework = devices.VGPUFrameworkNone + meta.GPUDevicePath = "/sys/bus/pci/devices/0000:82:00.4" + require.NoError(t, m.saveMetadata(meta)) + + _, err = m.StartInstance(context.Background(), id, StartInstanceRequest{}) + require.Error(t, err) + + stored, err := m.loadMetadata(id) + require.NoError(t, err) + assert.Empty(t, stored.GPUDevicePath, "released assignment should be persisted despite the failed start") + assert.Equal(t, "NVIDIA L40S-2Q", stored.GPUProfile, "profile is kept for the next start") +} + +func TestStopStoppedInstanceReleasesRetainedVGPU(t *testing.T) { + m, id := newLifecycleNoopManagerWithInstance(t, StateStopped, time.Now().UTC()) + meta, err := m.loadMetadata(id) + require.NoError(t, err) + meta.GPUProfile = "NVIDIA L40S-2Q" + meta.GPUFramework = devices.VGPUFrameworkNone + meta.GPUDevicePath = "/sys/bus/pci/devices/0000:82:00.4" + require.NoError(t, m.saveMetadata(meta)) + + inst, err := m.StopInstance(context.Background(), id) + require.NoError(t, err) + require.NotNil(t, inst) + assert.Equal(t, StateStopped, inst.State) + + stored, err := m.loadMetadata(id) + require.NoError(t, err) + assert.Empty(t, stored.GPUDevicePath) +} + +func TestStopStoppedInstanceVGPUReleaseFailureRemainsNoop(t *testing.T) { + m, id := newLifecycleNoopManagerWithInstance(t, StateStopped, time.Now().UTC()) + meta, err := m.loadMetadata(id) + require.NoError(t, err) + meta.GPUProfile = "NVIDIA L40S-2Q" + meta.GPUFramework = devices.VGPUFramework("future-framework") + meta.GPUDevicePath = "/sys/bus/pci/devices/0000:82:00.4" + require.NoError(t, m.saveMetadata(meta)) + + inst, err := m.StopInstance(context.Background(), id) + require.NoError(t, err) + require.NotNil(t, inst) + assert.Equal(t, StateStopped, inst.State) + + stored, err := m.loadMetadata(id) + require.NoError(t, err) + assert.Equal(t, devices.VGPUFramework("future-framework"), stored.GPUFramework) + assert.Equal(t, "/sys/bus/pci/devices/0000:82:00.4", stored.GPUDevicePath) +} + +// recordingDeviceManager is a devices.Manager stub that records passthrough +// teardown calls. Only the methods delete exercises are implemented. +type recordingDeviceManager struct { + devices.Manager + detached []string + unbound []string +} + +func (m *recordingDeviceManager) MarkDetached(ctx context.Context, deviceID string) error { + m.detached = append(m.detached, deviceID) + return nil +} + +func (m *recordingDeviceManager) UnbindFromVFIO(ctx context.Context, id string) error { + m.unbound = append(m.unbound, id) + return nil +} + +func TestLifecycleNoopStandbyRejectsVendorVFIOVGPU(t *testing.T) { + m, id := newLifecycleNoopManagerWithInstance(t, StateRunning, time.Now().UTC()) + meta, err := m.loadMetadata(id) + require.NoError(t, err) + meta.GPUProfile = "NVIDIA L40S-2Q" + meta.GPUFramework = devices.VGPUFrameworkVendorVFIO + meta.GPUDevicePath = "/sys/bus/pci/devices/0000:82:00.4" + require.NoError(t, m.saveMetadata(meta)) + + _, err = m.StandbyInstance(context.Background(), id, StandbyInstanceRequest{}) + require.ErrorIs(t, err, ErrInvalidState) + assert.ErrorContains(t, err, "standby is not supported for instances with vGPU attached") +} + func newLifecycleNoopManagerWithInstance(t *testing.T, state State, now time.Time) (*manager, string) { t.Helper() diff --git a/lib/instances/manager.go b/lib/instances/manager.go index 8e8e25f3f..ebb980810 100644 --- a/lib/instances/manager.go +++ b/lib/instances/manager.go @@ -27,6 +27,7 @@ import ( type Manager interface { ListInstances(ctx context.Context, filter *ListInstancesFilter) ([]Instance, error) + ListInstancesForReconcile(ctx context.Context) ([]Instance, error) ListSnapshots(ctx context.Context, filter *ListSnapshotsFilter) ([]Snapshot, error) GetSnapshot(ctx context.Context, snapshotID string) (*Snapshot, error) CreateInstance(ctx context.Context, req CreateInstanceRequest) (*Instance, error) @@ -179,6 +180,8 @@ type manager struct { tracer trace.Tracer now func() time.Time writeFile func(string, []byte, os.FileMode) error + createVGPU func(context.Context, string, string) (*devices.VGPUDevice, error) + destroyVGPU func(context.Context, devices.VGPUAssignment) error deleteSnapshotFn func(context.Context, string) error egressProxy *egressproxy.Service egressProxyServiceOptions egressproxy.ServiceOptions @@ -277,6 +280,8 @@ func NewManagerWithConfigE(p *paths.Paths, imageManager images.Manager, systemMa defaultHypervisor: defaultHypervisor, now: time.Now, writeFile: os.WriteFile, + createVGPU: devices.CreateVGPU, + destroyVGPU: devices.DestroyVGPU, meter: meter, tracer: tracer, guestMemoryPolicy: policy, @@ -621,6 +626,12 @@ func (m *manager) StopInstance(ctx context.Context, id string) (*Instance, error if err := m.markRestartManualStopLocked(ctx, id); err != nil { return nil, err } + // A stopped instance can retain a vGPU assignment when the release + // failed during the original stop. Retry it here so the vGPU slot is + // not held until the next start, delete, or hypeman restart. A failed + // retry only logs, keeping stop's no-op contract for already-stopped + // instances. + m.releaseRetainedVGPULocked(ctx, id) updated, err := m.currentInstanceWithoutHydration(ctx, id) if err != nil { return nil, err @@ -690,6 +701,11 @@ func (m *manager) UpdateInstance(ctx context.Context, id string, req UpdateInsta return inst, err } +// ListInstancesForReconcile returns every instance or an invalid metadata error. +func (m *manager) ListInstancesForReconcile(ctx context.Context) ([]Instance, error) { + return m.loadInstances(ctx, false) +} + // ListInstances returns instances, optionally filtered by the given criteria. // Pass nil to return all instances. func (m *manager) ListInstances(ctx context.Context, filter *ListInstancesFilter) ([]Instance, error) { diff --git a/lib/instances/process_identity_linux_test.go b/lib/instances/process_identity_linux_test.go new file mode 100644 index 000000000..bd5fecf9c --- /dev/null +++ b/lib/instances/process_identity_linux_test.go @@ -0,0 +1,271 @@ +//go:build linux + +package instances + +import ( + "bufio" + "context" + "fmt" + "net" + "os" + "os/exec" + "path/filepath" + "syscall" + "testing" + "time" + + "github.com/kernel/hypeman/lib/devices" + "github.com/kernel/hypeman/lib/paths" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestHypervisorProcessExistsTreatsUnresolvedSocketAsAlive(t *testing.T) { + t.Parallel() + + assert.True(t, HypervisorProcessExists(os.Getpid(), filepath.Join(t.TempDir(), "missing.sock"))) +} + +func TestHypervisorProcessExistsWithReboundSocketPathHelper(t *testing.T) { + if os.Getenv("HYPERVISOR_SOCKET_HELPER") != "1" { + return + } + + listener, err := net.Listen("unix", os.Getenv("HYPERVISOR_SOCKET_PATH")) + if err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + defer listener.Close() + fmt.Fprintln(os.Stdout, "ready") + _, _ = os.Stdin.Read(make([]byte, 1)) +} + +func TestHypervisorProcessExistsTreatsReboundSocketPathAsAlive(t *testing.T) { + socketPath := filepath.Join(t.TempDir(), "test.sock") + process := exec.Command(os.Args[0], "-test.run=^TestHypervisorProcessExistsWithReboundSocketPathHelper$") + process.Env = append(os.Environ(), "HYPERVISOR_SOCKET_HELPER=1", "HYPERVISOR_SOCKET_PATH="+socketPath) + stdin, err := process.StdinPipe() + require.NoError(t, err) + stdout, err := process.StdoutPipe() + require.NoError(t, err) + require.NoError(t, process.Start()) + t.Cleanup(func() { + _ = stdin.Close() + _ = process.Wait() + }) + + _, err = bufio.NewReader(stdout).ReadString('\n') + require.NoError(t, err) + require.NoError(t, os.Remove(socketPath)) + listener, err := net.Listen("unix", socketPath) + require.NoError(t, err) + defer listener.Close() + + assert.True(t, HypervisorProcessExists(os.Getpid(), socketPath)) +} + +func TestKillHypervisorSparesReusedPIDAndKillsSocketOwner(t *testing.T) { + socketPath := filepath.Join(t.TempDir(), "test.sock") + owner := exec.Command(os.Args[0], "-test.run=^TestHypervisorProcessExistsWithReboundSocketPathHelper$") + owner.Env = append(os.Environ(), "HYPERVISOR_SOCKET_HELPER=1", "HYPERVISOR_SOCKET_PATH="+socketPath) + stdin, err := owner.StdinPipe() + require.NoError(t, err) + stdout, err := owner.StdoutPipe() + require.NoError(t, err) + require.NoError(t, owner.Start()) + t.Cleanup(func() { + _ = stdin.Close() + _ = owner.Process.Kill() + _ = owner.Wait() + }) + _, err = bufio.NewReader(stdout).ReadString('\n') + require.NoError(t, err) + + stale := exec.Command("sleep", "30") + require.NoError(t, stale.Start()) + t.Cleanup(func() { + _ = stale.Process.Kill() + _ = stale.Wait() + }) + + stalePID := stale.Process.Pid + m := &manager{} + require.NoError(t, m.killHypervisor(context.Background(), &Instance{ + StoredMetadata: StoredMetadata{Id: "kill-test", HypervisorPID: &stalePID, SocketPath: socketPath}, + })) + + assert.NoError(t, syscall.Kill(stalePID, 0), "unrelated process holding the stale PID must survive delete") + assert.True(t, WaitForProcessExit(owner.Process.Pid, 5*time.Second), "socket owner should be killed") + _, statErr := os.Stat(socketPath) + assert.True(t, os.IsNotExist(statErr), "instance socket should be removed") +} + +func TestForceKillHypervisorProcessFailsOnUnconfirmedOwnership(t *testing.T) { + process := exec.Command("sleep", "30") + require.NoError(t, process.Start()) + t.Cleanup(func() { + _ = process.Process.Kill() + _ = process.Wait() + }) + + pid := process.Process.Pid + m := &manager{} + require.Error(t, m.forceKillHypervisorProcess(context.Background(), &Instance{ + StoredMetadata: StoredMetadata{Id: "kill-test", HypervisorPID: &pid, SocketPath: filepath.Join(t.TempDir(), "missing.sock")}, + })) + assert.NoError(t, syscall.Kill(pid, 0), "process with unconfirmed socket ownership must not be killed") +} + +func TestRefreshHypervisorPIDPrefersSocketOwnerOverLiveStoredPID(t *testing.T) { + socketPath := filepath.Join(t.TempDir(), "test.sock") + owner := exec.Command(os.Args[0], "-test.run=^TestHypervisorProcessExistsWithReboundSocketPathHelper$") + owner.Env = append(os.Environ(), "HYPERVISOR_SOCKET_HELPER=1", "HYPERVISOR_SOCKET_PATH="+socketPath) + stdin, err := owner.StdinPipe() + require.NoError(t, err) + stdout, err := owner.StdoutPipe() + require.NoError(t, err) + require.NoError(t, owner.Start()) + t.Cleanup(func() { + _ = stdin.Close() + _ = owner.Process.Kill() + _ = owner.Wait() + }) + _, err = bufio.NewReader(stdout).ReadString('\n') + require.NoError(t, err) + + stale := exec.Command("sleep", "30") + require.NoError(t, stale.Start()) + t.Cleanup(func() { + _ = stale.Process.Kill() + _ = stale.Wait() + }) + + stalePID := stale.Process.Pid + stored := StoredMetadata{HypervisorPID: &stalePID, SocketPath: socketPath} + refreshHypervisorPID(&stored, StateRunning) + require.NotNil(t, stored.HypervisorPID) + assert.Equal(t, owner.Process.Pid, *stored.HypervisorPID) +} + +func TestVGPUAssignmentClaimedByLiveInstanceProtectsReusedPIDClaim(t *testing.T) { + socketPath := filepath.Join(t.TempDir(), "test.sock") + owner := exec.Command(os.Args[0], "-test.run=^TestHypervisorProcessExistsWithReboundSocketPathHelper$") + owner.Env = append(os.Environ(), "HYPERVISOR_SOCKET_HELPER=1", "HYPERVISOR_SOCKET_PATH="+socketPath) + stdin, err := owner.StdinPipe() + require.NoError(t, err) + stdout, err := owner.StdoutPipe() + require.NoError(t, err) + require.NoError(t, owner.Start()) + t.Cleanup(func() { + _ = stdin.Close() + _ = owner.Process.Kill() + _ = owner.Wait() + }) + _, err = bufio.NewReader(stdout).ReadString('\n') + require.NoError(t, err) + + stale := exec.Command("sleep", "30") + require.NoError(t, stale.Start()) + t.Cleanup(func() { + _ = stale.Process.Kill() + _ = stale.Wait() + }) + + m := &manager{paths: paths.New(t.TempDir())} + const devicePath = "/sys/bus/pci/devices/0000:82:00.4" + stalePID := stale.Process.Pid + require.NoError(t, m.ensureDirectories("live-claimant")) + require.NoError(t, m.saveMetadata(&metadata{StoredMetadata: StoredMetadata{ + Id: "live-claimant", + GPUFramework: devices.VGPUFrameworkVendorVFIO, + GPUDevicePath: devicePath, + HypervisorPID: &stalePID, + SocketPath: socketPath, + }})) + + claimed, err := m.vgpuAssignmentClaimedByLiveInstance(context.Background(), "other-instance", devicePath) + require.NoError(t, err) + assert.True(t, claimed) +} + +func TestKillHypervisorSurvivesConcurrentReaper(t *testing.T) { + socketPath := filepath.Join(t.TempDir(), "test.sock") + process := exec.Command(os.Args[0], "-test.run=^TestHypervisorProcessExistsWithReboundSocketPathHelper$") + process.Env = append(os.Environ(), "HYPERVISOR_SOCKET_HELPER=1", "HYPERVISOR_SOCKET_PATH="+socketPath) + stdin, err := process.StdinPipe() + require.NoError(t, err) + stdout, err := process.StdoutPipe() + require.NoError(t, err) + require.NoError(t, process.Start()) + _, err = bufio.NewReader(stdout).ReadString('\n') + require.NoError(t, err) + + pid := process.Process.Pid + waitDone := make(chan error, 1) + go func() { waitDone <- process.Wait() }() + t.Cleanup(func() { + _ = stdin.Close() + _ = process.Process.Kill() + <-waitDone + }) + + m := &manager{} + require.NoError(t, m.killHypervisor(context.Background(), &Instance{ + StoredMetadata: StoredMetadata{Id: "kill-test", HypervisorPID: &pid, SocketPath: socketPath}, + })) +} + +func TestKillHypervisorFailsOnReusedPIDWhenSocketIsGone(t *testing.T) { + stale := exec.Command("sleep", "30") + require.NoError(t, stale.Start()) + t.Cleanup(func() { + _ = stale.Process.Kill() + _ = stale.Wait() + }) + + stalePID := stale.Process.Pid + socketPath := filepath.Join(t.TempDir(), "missing.sock") + m := &manager{} + require.Error(t, m.killHypervisor(context.Background(), &Instance{ + StoredMetadata: StoredMetadata{Id: "kill-test", HypervisorPID: &stalePID, SocketPath: socketPath}, + }), "unconfirmed ownership of a live stored PID must fail the kill") + + assert.NoError(t, syscall.Kill(stalePID, 0), "process with unconfirmed socket ownership must not be killed") +} + +func TestKillHypervisorFailsOnUnconfirmedCommandLineMatch(t *testing.T) { + socketPath := filepath.Join(t.TempDir(), "test.sock") + // A process whose command line contains the socket path but that does not + // own a listening socket: ResolveProcessPID resolves it unconfirmed. + match := exec.Command("sh", "-c", "sleep 30", "sh", socketPath) + require.NoError(t, match.Start()) + t.Cleanup(func() { + _ = match.Process.Kill() + _ = match.Wait() + }) + + matchPID := match.Process.Pid + m := &manager{} + require.Error(t, m.killHypervisor(context.Background(), &Instance{ + StoredMetadata: StoredMetadata{Id: "kill-test", HypervisorPID: &matchPID, SocketPath: socketPath}, + }), "a command-line match must not satisfy destructive ownership verification") + + assert.NoError(t, syscall.Kill(matchPID, 0), "process matched only by command line must not be killed") +} + +func TestHypervisorProcessExistsRejectsDifferentLiveSocketOwner(t *testing.T) { + socketPath := filepath.Join(t.TempDir(), "test.sock") + listener, err := net.Listen("unix", socketPath) + require.NoError(t, err) + defer listener.Close() + + process := exec.Command("sleep", "30") + require.NoError(t, process.Start()) + t.Cleanup(func() { + _ = process.Process.Kill() + _ = process.Wait() + }) + + assert.False(t, HypervisorProcessExists(process.Process.Pid, socketPath)) +} diff --git a/lib/instances/query.go b/lib/instances/query.go index 98c5359e0..533e369ea 100644 --- a/lib/instances/query.go +++ b/lib/instances/query.go @@ -575,19 +575,55 @@ func refreshHypervisorPID(stored *StoredMetadata, state State) { if !state.RequiresVMM() && state != StateUnknown { return } - if stored.HypervisorPID != nil && processExists(*stored.HypervisorPID) { - return + if pid, err := resolveLiveHypervisorPID(stored.HypervisorPID, stored.SocketPath); err == nil && pid > 0 { + stored.HypervisorPID = &pid } - if stored.SocketPath == "" { - return +} + +// resolveLiveHypervisorPID returns the PID of the live hypervisor that owns +// the instance socket, or 0 when no live hypervisor is found. It returns an +// error when a live stored PID's socket ownership cannot be confirmed. +func resolveLiveHypervisorPID(storedPID *int, socketPath string) (int, error) { + stored := 0 + if storedPID != nil && ProcessExists(*storedPID) { + stored = *storedPID } - if pid, err := hypervisor.ResolveProcessPID(stored.SocketPath); err == nil { - stored.HypervisorPID = &pid - return + if runtime.GOOS != "linux" || socketPath == "" { + return stored, nil + } + resolved, confirmed, err := hypervisor.ResolveProcessPID(socketPath) + switch { + case err == nil && confirmed && ProcessExists(resolved): + return resolved, nil + case err == nil && confirmed: + return 0, nil + } + if stored == 0 { + return 0, nil } + if err != nil { + return 0, fmt.Errorf("cannot confirm ownership of socket %s for stored hypervisor PID %d: %w", socketPath, stored, err) + } + return 0, fmt.Errorf("cannot confirm ownership of socket %s for stored hypervisor PID %d: process %d matched by command line only", socketPath, stored, resolved) } -func processExists(pid int) bool { +// HypervisorProcessExists reports whether pid owns the instance's hypervisor socket. +func HypervisorProcessExists(pid int, socketPath string) bool { + if !ProcessExists(pid) { + return false + } + if runtime.GOOS != "linux" || socketPath == "" { + return true + } + resolvedPID, confirmed, err := hypervisor.ResolveProcessPID(socketPath) + if err != nil || !confirmed || resolvedPID == pid { + return true + } + return !ProcessExists(resolvedPID) +} + +// ProcessExists reports whether pid belongs to a live, non-zombie process. +func ProcessExists(pid int) bool { if pid <= 0 { return false } @@ -839,14 +875,18 @@ func parseSentinelTimestamp(line, sentinelPrefix string) (time.Time, bool) { return time.Time{}, false } -// listInstances returns all instances +// listInstances returns all instances, skipping metadata files that cannot be loaded. func (m *manager) listInstances(ctx context.Context) ([]Instance, error) { + return m.loadInstances(ctx, true) +} + +func (m *manager) loadInstances(ctx context.Context, skipInvalid bool) ([]Instance, error) { ctx, span := m.tracerOrDefault().Start(ctx, "instances.list_metadata") defer span.End() log := logger.FromContext(ctx) log.DebugContext(ctx, "listing all instances") - files, err := m.listMetadataFiles() + files, err := m.listMetadataFilesWithStatErrors(!skipInvalid) if err != nil { log.ErrorContext(ctx, "failed to list metadata files", "error", err) return nil, err @@ -864,6 +904,11 @@ func (m *manager) listInstances(ctx context.Context) ([]Instance, error) { ) meta, err := m.loadMetadata(id) if err != nil { + if !skipInvalid { + hydrateSpan.RecordError(err) + hydrateSpan.End() + return nil, fmt.Errorf("load metadata for instance %s: %w", id, err) + } // Skip instances with invalid metadata log.WarnContext(hydrateCtx, "skipping instance with invalid metadata", "instance_id", id, "error", err) hydrateSpan.End() diff --git a/lib/instances/query_test.go b/lib/instances/query_test.go index 8bb0bb464..41aba54e8 100644 --- a/lib/instances/query_test.go +++ b/lib/instances/query_test.go @@ -14,6 +14,28 @@ import ( "github.com/stretchr/testify/require" ) +func TestListInstancesForReconcileFailsOnInvalidMetadata(t *testing.T) { + m := &manager{paths: paths.New(t.TempDir())} + + require.NoError(t, m.ensureDirectories("valid")) + require.NoError(t, m.saveMetadata(&metadata{StoredMetadata: StoredMetadata{ + Id: "valid", + Name: "valid", + CreatedAt: time.Now(), + DataDir: m.paths.InstanceDir("valid"), + }})) + require.NoError(t, m.ensureDirectories("invalid")) + require.NoError(t, os.WriteFile(m.paths.InstanceMetadata("invalid"), []byte("{"), 0644)) + + listed, err := m.ListInstances(context.Background(), nil) + require.NoError(t, err) + require.Len(t, listed, 1) + + _, err = m.ListInstancesForReconcile(context.Background()) + require.Error(t, err) + assert.ErrorContains(t, err, "load metadata for instance invalid") +} + func TestParseExitSentinelLine(t *testing.T) { t.Parallel() tests := []struct { diff --git a/lib/instances/snapshot.go b/lib/instances/snapshot.go index 97d2ca88e..0276e821e 100644 --- a/lib/instances/snapshot.go +++ b/lib/instances/snapshot.go @@ -302,6 +302,12 @@ func (m *manager) restoreSnapshot(ctx context.Context, id string, snapshotID str restored.StoppedAt = nil restored.ExitCode = nil restored.ExitMessage = "" + // vGPU assignments are live host state, not snapshot payload: keep the + // instance's current assignment (possibly retained from a failed release) + // instead of resurrecting the one embedded in the snapshot. + restored.GPUFramework = sourceMeta.GPUFramework + restored.GPUDevicePath = sourceMeta.GPUDevicePath + restored.GPUMdevUUID = sourceMeta.GPUMdevUUID restored.HypervisorType = targetHypervisor starter, err := m.getVMStarter(targetHypervisor) @@ -449,6 +455,7 @@ func (m *manager) forkSnapshot(ctx context.Context, snapshotID string, req ForkS forkMeta.ExitCode = nil forkMeta.ExitMessage = "" forkMeta.RestartStatus = restartpolicy.Status{} + clearStoredVGPUDevice(&forkMeta) forkMeta.FirecrackerUFFDSessionID = "" forkMeta.FirecrackerUFFDPagerVersion = "" forkMeta.FirecrackerUseUFFDOnNextRestore = useFirecrackerUFFDOnNextRestore(targetHypervisor, rec.Snapshot.Kind == SnapshotKindStandby, targetState) diff --git a/lib/instances/snapshot_test.go b/lib/instances/snapshot_test.go index 0fdd56665..114202026 100644 --- a/lib/instances/snapshot_test.go +++ b/lib/instances/snapshot_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + "github.com/kernel/hypeman/lib/devices" "github.com/kernel/hypeman/lib/hypervisor" "github.com/kernel/hypeman/lib/images" snapshotstore "github.com/kernel/hypeman/lib/snapshot" @@ -15,6 +16,118 @@ import ( "github.com/stretchr/testify/require" ) +func TestForkSnapshotClearsVGPUAssignment(t *testing.T) { + mgr, _ := setupTestManager(t) + ctx := context.Background() + + sourceID := "snapshot-vgpu-source" + createStoppedSnapshotSourceFixture(t, mgr, sourceID, sourceID, mgr.defaultHypervisor) + + meta, err := mgr.loadMetadata(sourceID) + require.NoError(t, err) + meta.GPUProfile = "NVIDIA L40S-2Q" + meta.GPUFramework = devices.VGPUFramework("future-framework") + meta.GPUDevicePath = "/sys/bus/pci/devices/0000:82:00.4" + meta.GPUMdevUUID = "retained-uuid" + require.NoError(t, mgr.saveMetadata(meta)) + + snapshot, err := mgr.CreateSnapshot(ctx, sourceID, CreateSnapshotRequest{ + Kind: SnapshotKindStopped, + Name: "snapshot-vgpu", + }) + require.NoError(t, err) + + forked, err := mgr.ForkSnapshot(ctx, snapshot.Id, ForkSnapshotRequest{ + Name: "snapshot-vgpu-fork", + TargetState: StateStopped, + }) + require.NoError(t, err) + assert.Equal(t, "NVIDIA L40S-2Q", forked.GPUProfile) + assert.Equal(t, devices.VGPUFrameworkNone, forked.GPUFramework) + assert.Empty(t, forked.GPUDevicePath) + assert.Empty(t, forked.GPUMdevUUID) + + source, err := mgr.loadMetadata(sourceID) + require.NoError(t, err) + assert.Equal(t, "/sys/bus/pci/devices/0000:82:00.4", source.GPUDevicePath) +} + +func TestRestoreSnapshotDoesNotResurrectStaleVGPUAssignment(t *testing.T) { + mgr, _ := setupTestManager(t) + ctx := context.Background() + + sourceID := "snapshot-vgpu-restore-stale" + createStoppedSnapshotSourceFixture(t, mgr, sourceID, sourceID, mgr.defaultHypervisor) + + meta, err := mgr.loadMetadata(sourceID) + require.NoError(t, err) + meta.GPUProfile = "NVIDIA L40S-2Q" + meta.GPUFramework = devices.VGPUFramework("future-framework") + meta.GPUDevicePath = "/sys/bus/pci/devices/0000:82:00.4" + meta.GPUMdevUUID = "retained-uuid" + require.NoError(t, mgr.saveMetadata(meta)) + + snapshot, err := mgr.CreateSnapshot(ctx, sourceID, CreateSnapshotRequest{ + Kind: SnapshotKindStopped, + Name: "snapshot-vgpu-restore-stale", + }) + require.NoError(t, err) + + // The retained assignment is released successfully after the snapshot + // was taken; a restore must not resurrect the snapshot's embedded copy. + meta, err = mgr.loadMetadata(sourceID) + require.NoError(t, err) + clearStoredVGPUDevice(&meta.StoredMetadata) + require.NoError(t, mgr.saveMetadata(meta)) + + _, err = mgr.RestoreSnapshot(ctx, sourceID, snapshot.Id, RestoreSnapshotRequest{ + TargetState: StateStopped, + TargetHypervisor: mgr.defaultHypervisor, + }) + require.NoError(t, err) + + restored, err := mgr.loadMetadata(sourceID) + require.NoError(t, err) + assert.Equal(t, devices.VGPUFrameworkNone, restored.GPUFramework) + assert.Empty(t, restored.GPUDevicePath) + assert.Empty(t, restored.GPUMdevUUID) +} + +func TestRestoreSnapshotKeepsCurrentVGPUAssignment(t *testing.T) { + mgr, _ := setupTestManager(t) + ctx := context.Background() + + sourceID := "snapshot-vgpu-restore-retained" + createStoppedSnapshotSourceFixture(t, mgr, sourceID, sourceID, mgr.defaultHypervisor) + + snapshot, err := mgr.CreateSnapshot(ctx, sourceID, CreateSnapshotRequest{ + Kind: SnapshotKindStopped, + Name: "snapshot-vgpu-restore-retained", + }) + require.NoError(t, err) + + // An assignment retained after the snapshot was taken (e.g. from a + // failed release on stop) must survive the restore for the next retry. + meta, err := mgr.loadMetadata(sourceID) + require.NoError(t, err) + meta.GPUFramework = devices.VGPUFramework("future-framework") + meta.GPUDevicePath = "/sys/bus/pci/devices/0000:82:00.4" + meta.GPUMdevUUID = "retained-uuid" + require.NoError(t, mgr.saveMetadata(meta)) + + _, err = mgr.RestoreSnapshot(ctx, sourceID, snapshot.Id, RestoreSnapshotRequest{ + TargetState: StateStopped, + TargetHypervisor: mgr.defaultHypervisor, + }) + require.NoError(t, err) + + restored, err := mgr.loadMetadata(sourceID) + require.NoError(t, err) + assert.Equal(t, devices.VGPUFramework("future-framework"), restored.GPUFramework) + assert.Equal(t, "/sys/bus/pci/devices/0000:82:00.4", restored.GPUDevicePath) + assert.Equal(t, "retained-uuid", restored.GPUMdevUUID) +} + func TestStoppedSnapshotLifecycleAndForkAfterSourceDeletion(t *testing.T) { t.Parallel() mgr, _ := setupTestManager(t) diff --git a/lib/instances/start.go b/lib/instances/start.go index 3a0879a74..abff4ef76 100644 --- a/lib/instances/start.go +++ b/lib/instances/start.go @@ -48,6 +48,20 @@ func (m *manager) startInstance( log.ErrorContext(ctx, "invalid state for start", "instance_id", id, "state", inst.State) return nil, fmt.Errorf("%w: cannot start from state %s, must be Stopped", ErrInvalidState, inst.State) } + // Release any assignment retained by an earlier failed release and + // persist the cleared fields immediately, so a failure later in start + // cannot leave on-disk metadata pointing at a device that is already + // gone (matching releaseRetainedVGPULocked). + if storedVGPUDevicePath(stored) != "" { + if err := m.releaseStoredVGPU(ctx, stored); err != nil { + log.ErrorContext(ctx, "failed to release stale vGPU before start", "instance_id", id, "error", err) + return nil, fmt.Errorf("release stale vGPU before start: %w", err) + } + if err := m.saveMetadata(meta); err != nil { + log.ErrorContext(ctx, "failed to save metadata after stale vGPU release", "instance_id", id, "error", err) + return nil, fmt.Errorf("save metadata after stale vGPU release: %w", err) + } + } // 2a. Clear stale exit info from previous run and apply command overrides stored.ExitCode = nil @@ -144,24 +158,40 @@ func (m *manager) startInstance( } } - // 4b. Recreate vGPU mdev if this instance had a GPU profile + // 4b. Recreate the vGPU if this instance had a GPU profile // Note: GPU availability was already validated in step 2b if stored.GPUProfile != "" { - log.InfoContext(ctx, "creating vGPU mdev for start", "instance_id", id, "profile", stored.GPUProfile) - mdev, err := devices.CreateMdev(ctx, stored.GPUProfile, id) + log.InfoContext(ctx, "creating vGPU for start", "instance_id", id, "profile", stored.GPUProfile) + device, err := m.createVGPUDevice(ctx, stored.GPUProfile, id) if err != nil { - log.ErrorContext(ctx, "failed to create mdev", "instance_id", id, "profile", stored.GPUProfile, "error", err) - return nil, fmt.Errorf("create vGPU mdev for profile %s: %w", stored.GPUProfile, err) + log.ErrorContext(ctx, "failed to create vGPU", "instance_id", id, "profile", stored.GPUProfile, "error", err) + return nil, fmt.Errorf("create vGPU for profile %s: %w", stored.GPUProfile, err) } - stored.GPUMdevUUID = mdev.UUID - log.InfoContext(ctx, "created vGPU mdev", "instance_id", id, "profile", stored.GPUProfile, "uuid", mdev.UUID) - // Add mdev cleanup to stack + setStoredVGPUDevice(stored, device) + // Add vGPU cleanup to stack cu.Add(func() { - log.DebugContext(ctx, "destroying mdev on cleanup", "instance_id", id, "uuid", mdev.UUID) - if err := devices.DestroyMdev(ctx, mdev.UUID); err != nil { - log.WarnContext(ctx, "failed to destroy mdev on cleanup", "instance_id", id, "uuid", mdev.UUID, "error", err) + assignment := devices.VGPUAssignment{ + Framework: device.Framework, + DevicePath: device.SysfsPath, + MdevUUID: device.MdevUUID, + InstanceID: id, + } + if err := m.destroyVGPUAssignment(ctx, assignment); err != nil { + log.WarnContext(ctx, "failed to destroy vGPU on cleanup", "instance_id", id, "error", err) + if saveErr := m.saveMetadata(meta); saveErr != nil { + log.ErrorContext(ctx, "failed to retain vGPU assignment metadata after cleanup failure", "instance_id", id, "error", saveErr) + } + } else { + clearStoredVGPUDevice(stored) + if saveErr := m.saveMetadata(meta); saveErr != nil { + log.ErrorContext(ctx, "failed to save metadata after vGPU cleanup", "instance_id", id, "error", saveErr) + } } }) + if err := m.saveMetadata(meta); err != nil { + log.ErrorContext(ctx, "failed to save metadata after vGPU creation", "instance_id", id, "error", err) + return nil, fmt.Errorf("save metadata after vGPU creation: %w", err) + } } // 5. Regenerate config disk with new network configuration diff --git a/lib/instances/stop.go b/lib/instances/stop.go index fb60c68c9..35e69822a 100644 --- a/lib/instances/stop.go +++ b/lib/instances/stop.go @@ -9,7 +9,6 @@ import ( "syscall" "time" - "github.com/kernel/hypeman/lib/devices" "github.com/kernel/hypeman/lib/guest" "github.com/kernel/hypeman/lib/hypervisor" "github.com/kernel/hypeman/lib/instances/phasetracking" @@ -96,13 +95,14 @@ func (m *manager) tryGracefulGuestShutdown(ctx context.Context, inst *Instance, func (m *manager) forceKillHypervisorProcess(ctx context.Context, inst *Instance) error { log := logger.FromContext(ctx) - if inst.HypervisorPID == nil { + if inst.HypervisorPID == nil && inst.SocketPath == "" { return nil } - - pid := *inst.HypervisorPID - if err := syscall.Kill(pid, 0); err != nil { - // Process is already gone (likely ESRCH). + pid, err := resolveLiveHypervisorPID(inst.HypervisorPID, inst.SocketPath) + if err != nil { + return err + } + if pid == 0 { return nil } @@ -110,26 +110,8 @@ func (m *manager) forceKillHypervisorProcess(ctx context.Context, inst *Instance if err := syscall.Kill(pid, syscall.SIGKILL); err != nil { return fmt.Errorf("sigkill hypervisor pid %d: %w", pid, err) } - - // Wait for process to die and reap it to avoid zombie false positives. - reaped := false - for i := 0; i < 50; i++ { // 50 * 100ms = 5s - var wstatus syscall.WaitStatus - wpid, err := syscall.Wait4(pid, &wstatus, syscall.WNOHANG, nil) - if err != nil || wpid == pid { - // Process reaped, or not our child (ECHILD) and no longer trackable here. - reaped = true - break - } - time.Sleep(100 * time.Millisecond) - } - - if !reaped { - // Timed out waiting for reap; if process still exists, treat as failure. - if err := syscall.Kill(pid, 0); err == nil { - return fmt.Errorf("hypervisor pid %d still alive after SIGKILL", pid) - } - log.WarnContext(ctx, "timeout waiting to reap hypervisor process after SIGKILL", "instance_id", inst.Id, "pid", pid) + if !WaitForProcessExit(pid, 30*time.Second) { + return fmt.Errorf("hypervisor pid %d still alive after SIGKILL", pid) } log.DebugContext(ctx, "hypervisor process force-killed", "instance_id", inst.Id, "pid", pid) @@ -263,13 +245,9 @@ func (m *manager) stopInstance( } } - // 7. Destroy vGPU mdev device if present (frees vGPU slot for other VMs) - if inst.GPUMdevUUID != "" { - log.InfoContext(ctx, "destroying vGPU mdev on stop", "instance_id", id, "uuid", inst.GPUMdevUUID) - if err := devices.DestroyMdev(ctx, inst.GPUMdevUUID); err != nil { - // Log error but continue - mdev cleanup is best-effort - log.WarnContext(ctx, "failed to destroy mdev on stop", "instance_id", id, "uuid", inst.GPUMdevUUID, "error", err) - } + // 7. Release the vGPU assignment if present. + if err := m.releaseStoredVGPU(ctx, stored); err != nil { + log.WarnContext(ctx, "failed to destroy vGPU on stop; retaining assignment metadata", "instance_id", id, "error", err) } // 8. Always remove stale runtime sockets after process exit. @@ -298,11 +276,10 @@ func (m *manager) stopInstance( } } - // 10. Update metadata (clear PID, mdev UUID, set StoppedAt) + // 10. Update metadata (clear PID, set StoppedAt) now := time.Now().UTC() stored.StoppedAt = &now stored.HypervisorPID = nil - stored.GPUMdevUUID = "" // Clear mdev UUID since we destroyed it // Boot markers are per-boot-run and must not carry across stop/restore/start. stored.ProgramStartedAt = nil stored.GuestAgentReadyAt = nil diff --git a/lib/instances/storage.go b/lib/instances/storage.go index f33a59621..d44dcbf2e 100644 --- a/lib/instances/storage.go +++ b/lib/instances/storage.go @@ -177,8 +177,12 @@ func removeAllWithRetry(path string, removeAll func(string) error, sleep func(ti } } -// listMetadataFiles returns paths to all instance metadata files +// listMetadataFiles returns paths to all instance metadata files. func (m *manager) listMetadataFiles() ([]string, error) { + return m.listMetadataFilesWithStatErrors(false) +} + +func (m *manager) listMetadataFilesWithStatErrors(failOnStatError bool) ([]string, error) { guestsDir := m.paths.GuestsDir() // Ensure guests directory exists @@ -200,6 +204,8 @@ func (m *manager) listMetadataFiles() ([]string, error) { metaPath := filepath.Join(guestsDir, entry.Name(), "metadata.json") if _, err := os.Stat(metaPath); err == nil { metaFiles = append(metaFiles, metaPath) + } else if failOnStatError && !os.IsNotExist(err) { + return nil, fmt.Errorf("stat metadata for instance %s: %w", entry.Name(), err) } } diff --git a/lib/instances/types.go b/lib/instances/types.go index df3ab7ff8..27aa492e3 100644 --- a/lib/instances/types.go +++ b/lib/instances/types.go @@ -4,6 +4,7 @@ import ( "time" "github.com/kernel/hypeman/lib/autostandby" + "github.com/kernel/hypeman/lib/devices" "github.com/kernel/hypeman/lib/healthcheck" "github.com/kernel/hypeman/lib/hypervisor" "github.com/kernel/hypeman/lib/instances/phasetracking" @@ -148,8 +149,10 @@ type StoredMetadata struct { Devices []string // Device IDs attached to this instance // GPU configuration (vGPU mode) - GPUProfile string // vGPU profile name (e.g., "L40S-1Q") - GPUMdevUUID string // mdev device UUID + GPUProfile string // vGPU profile name (e.g., "L40S-1Q") + GPUFramework devices.VGPUFramework + GPUDevicePath string + GPUMdevUUID string // populated for mdev-backed vGPUs // Command overrides (like docker run ) Entrypoint []string // Override image entrypoint (nil = use image default) diff --git a/lib/instances/vgpu.go b/lib/instances/vgpu.go new file mode 100644 index 000000000..81d032220 --- /dev/null +++ b/lib/instances/vgpu.go @@ -0,0 +1,161 @@ +package instances + +import ( + "context" + "fmt" + "path/filepath" + + "github.com/kernel/hypeman/lib/devices" + "github.com/kernel/hypeman/lib/logger" +) + +// VGPUCleanupPendingError reports a failed create whose vGPU release also +// failed during rollback. When Retained is true, deleting the retained instance +// retries the release; otherwise startup reconciliation recovers the assignment. +type VGPUCleanupPendingError struct { + InstanceID string + Retained bool + Err error +} + +func (e *VGPUCleanupPendingError) Error() string { + if e.Retained { + return fmt.Sprintf("%v; vGPU release failed during rollback, instance %s retains the assignment", e.Err, e.InstanceID) + } + return fmt.Sprintf("%v; vGPU release failed during rollback and the retention record for instance %s could not be saved; the assignment is recovered on the next startup reconcile", e.Err, e.InstanceID) +} + +func (e *VGPUCleanupPendingError) Unwrap() error { return e.Err } + +func (m *manager) createVGPUDevice(ctx context.Context, profileName, instanceID string) (*devices.VGPUDevice, error) { + create := m.createVGPU + if create == nil { + create = devices.CreateVGPU + } + return create(ctx, profileName, instanceID) +} + +func (m *manager) destroyVGPUAssignment(ctx context.Context, assignment devices.VGPUAssignment) error { + destroy := m.destroyVGPU + if destroy == nil { + destroy = devices.DestroyVGPU + } + return destroy(ctx, assignment) +} + +func setStoredVGPUDevice(stored *StoredMetadata, device *devices.VGPUDevice) { + stored.GPUFramework = device.Framework + stored.GPUDevicePath = device.SysfsPath + stored.GPUMdevUUID = device.MdevUUID +} + +func clearStoredVGPUDevice(stored *StoredMetadata) { + stored.GPUFramework = devices.VGPUFrameworkNone + stored.GPUDevicePath = "" + stored.GPUMdevUUID = "" +} + +func (m *manager) releaseStoredVGPU(ctx context.Context, stored *StoredMetadata) error { + path := storedVGPUDevicePath(stored) + if path != "" { + // Vendor VFIO VFs are reused across instances, so stale metadata can + // point at a path claimed by a live instance and the release must fail + // closed on an incomplete inventory. mdev UUIDs are unique and never + // reused, so skip the scan there — it would let one unreadable + // metadata file block every mdev release on the host. + claimed := false + if stored.GPUFramework == devices.VGPUFrameworkVendorVFIO { + var err error + claimed, err = m.vgpuAssignmentClaimedByLiveInstance(ctx, stored.Id, path) + if err != nil { + return err + } + } + if claimed { + logger.FromContext(ctx).WarnContext(ctx, "dropping stale vGPU assignment claimed by another live instance", + "instance_id", stored.Id, "device_path", path) + } else { + assignment := devices.VGPUAssignment{ + Framework: stored.GPUFramework, + DevicePath: path, + MdevUUID: stored.GPUMdevUUID, + InstanceID: stored.Id, + } + if err := m.destroyVGPUAssignment(ctx, assignment); err != nil { + return err + } + } + } + clearStoredVGPUDevice(stored) + return nil +} + +// vgpuAssignmentClaimedByLiveInstance reports whether another instance's +// stored metadata claims devicePath. It reads raw metadata instead of +// hydrating full instances: the scan runs on every vendor VFIO release, and +// deriving state would query the hypervisor of every instance on the host. +// It fails closed: unreadable metadata is an error, and a matching claim +// without a persisted PID counts as live because the PID is only persisted +// after the claimant's hypervisor starts. +func (m *manager) vgpuAssignmentClaimedByLiveInstance(ctx context.Context, excludeID, devicePath string) (bool, error) { + files, err := m.listMetadataFilesWithStatErrors(true) + if err != nil { + return false, fmt.Errorf("list instances for vGPU release check: %w", err) + } + for _, file := range files { + id := filepath.Base(filepath.Dir(file)) + if id == excludeID { + continue + } + meta, err := m.loadMetadata(id) + if err != nil { + return false, fmt.Errorf("load metadata for vGPU release check: instance %s: %w", id, err) + } + stored := &meta.StoredMetadata + if storedVGPUDevicePath(stored) != devicePath { + continue + } + if stored.HypervisorPID == nil { + return true, nil + } + pid, err := resolveLiveHypervisorPID(stored.HypervisorPID, stored.SocketPath) + if err != nil || pid > 0 { + return true, nil + } + } + return false, nil +} + +// releaseRetainedVGPULocked releases a vGPU assignment retained on a stopped +// instance after a failed release during the original stop. It is a no-op +// when no assignment is retained, and a failed retry only logs so the +// metadata stays for the next retry. The caller must hold the instance lock. +func (m *manager) releaseRetainedVGPULocked(ctx context.Context, id string) { + log := logger.FromContext(ctx) + meta, err := m.loadMetadata(id) + if err != nil { + log.WarnContext(ctx, "failed to load metadata for retained vGPU release", "instance_id", id, "error", err) + return + } + stored := &meta.StoredMetadata + if storedVGPUDevicePath(stored) == "" { + return + } + if err := m.releaseStoredVGPU(ctx, stored); err != nil { + log.WarnContext(ctx, "failed to destroy retained vGPU; retaining assignment metadata", "instance_id", id, "error", err) + return + } + if err := m.saveMetadata(meta); err != nil { + log.WarnContext(ctx, "failed to save metadata after retained vGPU release", "instance_id", id, "error", err) + } +} + +func storedVGPUDevicePath(stored *StoredMetadata) string { + if stored.GPUDevicePath != "" { + return stored.GPUDevicePath + } + if stored.GPUMdevUUID != "" { + return filepath.Join("/sys/bus/mdev/devices", stored.GPUMdevUUID) + } + return "" +} diff --git a/lib/instances/vgpu_test.go b/lib/instances/vgpu_test.go new file mode 100644 index 000000000..bc5ab9c0d --- /dev/null +++ b/lib/instances/vgpu_test.go @@ -0,0 +1,292 @@ +package instances + +import ( + "context" + "errors" + "os" + "path/filepath" + "sync" + "testing" + + "github.com/kernel/hypeman/lib/devices" + "github.com/kernel/hypeman/lib/paths" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCleanupFailedCreateRetainsVGPUAssignment(t *testing.T) { + t.Parallel() + + m := &manager{paths: paths.New(t.TempDir())} + stored := &StoredMetadata{ + Id: "failed-create", + Name: "failed-create", + GPUProfile: "NVIDIA L40S-2Q", + GPUFramework: devices.VGPUFrameworkVendorVFIO, + GPUDevicePath: "/sys/bus/pci/devices/0000:82:00.4", + GPUMdevUUID: "mdev-uuid", + NetworkEnabled: true, + IP: "192.0.2.1", + Volumes: []VolumeAttachment{{VolumeID: "volume"}}, + HypervisorType: "qemu", + DataDir: m.paths.InstanceDir("failed-create"), + } + + assert.True(t, m.cleanupFailedCreate(context.Background(), stored.Id, stored)) + + retained, err := m.loadMetadata(stored.Id) + require.NoError(t, err) + assert.Equal(t, stored.Id, retained.Id) + assert.Equal(t, stored.GPUFramework, retained.GPUFramework) + assert.Equal(t, stored.GPUDevicePath, retained.GPUDevicePath) + assert.Equal(t, stored.GPUMdevUUID, retained.GPUMdevUUID) + assert.Empty(t, retained.Name) + assert.Empty(t, retained.GPUProfile) + assert.False(t, retained.NetworkEnabled) + assert.Empty(t, retained.IP) + assert.Empty(t, retained.Volumes) + assert.Empty(t, retained.DataDir) +} + +func TestCleanupFailedCreateDeletesDataWithoutRetainedVGPU(t *testing.T) { + t.Parallel() + + m := &manager{paths: paths.New(t.TempDir())} + require.NoError(t, m.ensureDirectories("failed-create")) + + assert.False(t, m.cleanupFailedCreate(context.Background(), "failed-create", nil)) + _, err := m.loadMetadata("failed-create") + require.Error(t, err) +} + +func TestCleanupFailedCreateReportsUnpersistedRetention(t *testing.T) { + t.Parallel() + + m := &manager{paths: paths.New(t.TempDir())} + const id = "failed-create" + require.NoError(t, m.ensureDirectories(id)) + require.NoError(t, os.Mkdir(m.paths.InstanceMetadata(id), 0o755)) + + stored := &StoredMetadata{ + Id: id, + GPUFramework: devices.VGPUFrameworkVendorVFIO, + GPUDevicePath: "/sys/bus/pci/devices/0000:82:00.4", + } + assert.False(t, m.cleanupFailedCreate(context.Background(), stored.Id, stored)) + _, err := m.loadMetadata(id) + require.Error(t, err) +} + +func TestVGPUCleanupPendingErrorUnwraps(t *testing.T) { + t.Parallel() + + cause := errors.New("boot failed") + retained := &VGPUCleanupPendingError{InstanceID: "inst-1", Retained: true, Err: cause} + assert.ErrorIs(t, retained, cause) + assert.Equal(t, "boot failed; vGPU release failed during rollback, instance inst-1 retains the assignment", retained.Error()) + + unpersisted := &VGPUCleanupPendingError{InstanceID: "inst-1", Err: cause} + assert.ErrorIs(t, unpersisted, cause) + assert.Equal(t, "boot failed; vGPU release failed during rollback and the retention record for instance inst-1 could not be saved; the assignment is recovered on the next startup reconcile", unpersisted.Error()) +} + +func newStartRollbackVGPUManager(t *testing.T, destroy func(context.Context, devices.VGPUAssignment) error) (*manager, string) { + t.Helper() + m := &manager{ + paths: paths.New(t.TempDir()), + imageManager: readyFixtureImageManager{name: "test-image"}, + instanceLocks: sync.Map{}, + bootMarkerScans: sync.Map{}, + createVGPU: func(_ context.Context, profileName, _ string) (*devices.VGPUDevice, error) { + return &devices.VGPUDevice{ + Framework: devices.VGPUFrameworkVendorVFIO, + VFAddress: "0000:82:00.4", + ProfileType: "1148", + ProfileName: profileName, + SysfsPath: "/sys/bus/pci/devices/0000:82:00.4", + }, nil + }, + destroyVGPU: destroy, + } + const id = "start-rollback" + require.NoError(t, m.ensureDirectories(id)) + require.NoError(t, m.saveMetadata(&metadata{StoredMetadata: StoredMetadata{ + Id: id, + Name: id, + Image: "test-image", + GPUProfile: "NVIDIA L40S-2Q", + HypervisorType: lifecycleNoopHypervisorType, + SocketPath: m.paths.InstanceSocket(id, "noop.sock"), + DataDir: m.paths.InstanceDir(id), + }})) + return m, id +} + +func TestStartRollbackClearsVGPUAssignmentAfterSuccessfulDestroy(t *testing.T) { + var destroyed []devices.VGPUAssignment + m, id := newStartRollbackVGPUManager(t, func(_ context.Context, assignment devices.VGPUAssignment) error { + destroyed = append(destroyed, assignment) + return nil + }) + + t.Setenv("TMPDIR", filepath.Join(t.TempDir(), "missing")) + _, err := m.startInstance(context.Background(), id, StartInstanceRequest{}) + require.Error(t, err) + + require.Len(t, destroyed, 1) + assert.Equal(t, devices.VGPUAssignment{ + Framework: devices.VGPUFrameworkVendorVFIO, + DevicePath: "/sys/bus/pci/devices/0000:82:00.4", + InstanceID: id, + }, destroyed[0]) + + stored, err := m.loadMetadata(id) + require.NoError(t, err) + assert.Equal(t, "NVIDIA L40S-2Q", stored.GPUProfile) + assert.Empty(t, stored.GPUFramework) + assert.Empty(t, stored.GPUDevicePath) + assert.Empty(t, stored.GPUMdevUUID) +} + +func TestStartRollbackRetainsVGPUAssignmentAfterFailedDestroy(t *testing.T) { + m, id := newStartRollbackVGPUManager(t, func(context.Context, devices.VGPUAssignment) error { + return errors.New("destroy failed") + }) + + t.Setenv("TMPDIR", filepath.Join(t.TempDir(), "missing")) + _, err := m.startInstance(context.Background(), id, StartInstanceRequest{}) + require.Error(t, err) + + stored, err := m.loadMetadata(id) + require.NoError(t, err) + assert.Equal(t, devices.VGPUFrameworkVendorVFIO, stored.GPUFramework) + assert.Equal(t, "/sys/bus/pci/devices/0000:82:00.4", stored.GPUDevicePath) +} + +func TestVGPUAssignmentClaimedByLiveInstanceFailsOnInvalidMetadata(t *testing.T) { + t.Parallel() + + m := &manager{paths: paths.New(t.TempDir())} + require.NoError(t, m.ensureDirectories("invalid-instance")) + require.NoError(t, os.WriteFile(m.paths.InstanceMetadata("invalid-instance"), []byte("{"), 0o644)) + + _, err := m.vgpuAssignmentClaimedByLiveInstance(context.Background(), "other-instance", "/sys/bus/pci/devices/0000:82:00.4") + require.Error(t, err) +} + +func TestVGPUAssignmentClaimedByLiveInstanceNormalizesLegacyMdevPath(t *testing.T) { + t.Parallel() + + m := &manager{paths: paths.New(t.TempDir())} + require.NoError(t, m.ensureDirectories("legacy-claimant")) + pid := os.Getpid() + require.NoError(t, m.saveMetadata(&metadata{StoredMetadata: StoredMetadata{ + Id: "legacy-claimant", + Name: "legacy-claimant", + GPUMdevUUID: "legacy-uuid", + HypervisorPID: &pid, + }})) + + claimed, err := m.vgpuAssignmentClaimedByLiveInstance(context.Background(), "other-instance", "/sys/bus/mdev/devices/legacy-uuid") + require.NoError(t, err) + assert.True(t, claimed) +} + +func TestVGPUAssignmentClaimedByLiveInstanceProtectsNilPIDClaim(t *testing.T) { + t.Parallel() + + m := &manager{paths: paths.New(t.TempDir())} + require.NoError(t, m.ensureDirectories("booting-claimant")) + require.NoError(t, m.saveMetadata(&metadata{StoredMetadata: StoredMetadata{ + Id: "booting-claimant", + Name: "booting-claimant", + GPUDevicePath: "/sys/bus/pci/devices/0000:82:00.4", + }})) + + claimed, err := m.vgpuAssignmentClaimedByLiveInstance(context.Background(), "other-instance", "/sys/bus/pci/devices/0000:82:00.4") + require.NoError(t, err) + assert.True(t, claimed, "a matching claim without a persisted PID must be treated as live: the PID is only persisted after the claimant boots") +} + +func TestVGPUAssignmentClaimedByLiveInstanceIgnoresDeadClaim(t *testing.T) { + t.Parallel() + + m := &manager{paths: paths.New(t.TempDir())} + require.NoError(t, m.ensureDirectories("dead-claimant")) + deadPID := 1 << 30 + require.NoError(t, m.saveMetadata(&metadata{StoredMetadata: StoredMetadata{ + Id: "dead-claimant", + Name: "dead-claimant", + GPUDevicePath: "/sys/bus/pci/devices/0000:82:00.4", + HypervisorPID: &deadPID, + }})) + + claimed, err := m.vgpuAssignmentClaimedByLiveInstance(context.Background(), "other-instance", "/sys/bus/pci/devices/0000:82:00.4") + require.NoError(t, err) + assert.False(t, claimed, "a claim whose hypervisor is gone must not block the release") +} + +func TestReleaseStoredVGPUSkipsClaimScanForMdev(t *testing.T) { + t.Parallel() + + m := &manager{ + paths: paths.New(t.TempDir()), + destroyVGPU: func(context.Context, devices.VGPUAssignment) error { return nil }, + } + require.NoError(t, m.ensureDirectories("invalid-instance")) + require.NoError(t, os.WriteFile(m.paths.InstanceMetadata("invalid-instance"), []byte("{"), 0o644)) + + stored := &StoredMetadata{ + Id: "mdev-instance", + GPUFramework: devices.VGPUFrameworkMdev, + GPUMdevUUID: "uuid-1", + GPUDevicePath: "/sys/bus/mdev/devices/uuid-1", + } + require.NoError(t, m.releaseStoredVGPU(context.Background(), stored), + "an unreadable metadata file must not block mdev releases") + assert.Empty(t, stored.GPUDevicePath) +} + +func TestStoredVGPUDevicePath(t *testing.T) { + t.Parallel() + + assert.Equal(t, "/sys/bus/pci/devices/0000:82:00.4", storedVGPUDevicePath(&StoredMetadata{ + GPUDevicePath: "/sys/bus/pci/devices/0000:82:00.4", + GPUMdevUUID: "legacy-uuid", + })) + assert.Equal(t, "/sys/bus/mdev/devices/legacy-uuid", storedVGPUDevicePath(&StoredMetadata{ + GPUMdevUUID: "legacy-uuid", + })) + assert.Empty(t, storedVGPUDevicePath(&StoredMetadata{})) +} + +func TestReleaseStoredVGPURetainsMetadataOnFailure(t *testing.T) { + t.Parallel() + + m := &manager{paths: paths.New(t.TempDir())} + stored := &StoredMetadata{ + GPUFramework: devices.VGPUFramework("future-framework"), + GPUDevicePath: "/sys/bus/pci/devices/0000:82:00.4", + } + err := m.releaseStoredVGPU(context.Background(), stored) + assert.Error(t, err) + assert.Equal(t, devices.VGPUFramework("future-framework"), stored.GPUFramework) + assert.Equal(t, "/sys/bus/pci/devices/0000:82:00.4", stored.GPUDevicePath) +} + +func TestSetAndClearStoredVGPUDevice(t *testing.T) { + t.Parallel() + + stored := &StoredMetadata{} + setStoredVGPUDevice(stored, &devices.VGPUDevice{ + Framework: devices.VGPUFrameworkVendorVFIO, + SysfsPath: "/sys/bus/pci/devices/0000:82:00.4", + }) + assert.Equal(t, devices.VGPUFrameworkVendorVFIO, stored.GPUFramework) + assert.Equal(t, "/sys/bus/pci/devices/0000:82:00.4", stored.GPUDevicePath) + + clearStoredVGPUDevice(stored) + assert.Empty(t, stored.GPUFramework) + assert.Empty(t, stored.GPUDevicePath) + assert.Empty(t, stored.GPUMdevUUID) +} diff --git a/lib/instances/wait_test.go b/lib/instances/wait_test.go index bab6f06d7..a42464795 100644 --- a/lib/instances/wait_test.go +++ b/lib/instances/wait_test.go @@ -32,6 +32,9 @@ func (s *stubManager) GetInstance(ctx context.Context, id string) (*Instance, er func (s *stubManager) ListInstances(context.Context, *ListInstancesFilter) ([]Instance, error) { return nil, nil } +func (s *stubManager) ListInstancesForReconcile(context.Context) ([]Instance, error) { + return nil, nil +} func (s *stubManager) ListSnapshots(context.Context, *ListSnapshotsFilter) ([]Snapshot, error) { return nil, nil } diff --git a/lib/resources/gpu.go b/lib/resources/gpu.go index dc2437bbf..054e3744e 100644 --- a/lib/resources/gpu.go +++ b/lib/resources/gpu.go @@ -1,7 +1,10 @@ package resources import ( + "context" + "github.com/kernel/hypeman/lib/devices" + "github.com/kernel/hypeman/lib/logger" ) // GPUResourceStatus represents the GPU resource status for the API response. @@ -16,40 +19,36 @@ type GPUResourceStatus struct { // GetGPUStatus returns the current GPU resource status. // Returns nil if no GPU is available or the mode is "none". -func GetGPUStatus() *GPUResourceStatus { - mode := devices.DetectHostGPUMode() - if mode == devices.GPUModeNone { +func GetGPUStatus(ctx context.Context) *GPUResourceStatus { + framework, vfs, err := devices.DiscoverVGPU() + if err != nil { + // Only report passthrough once vGPU discovery confirms no vGPU + // framework. On a vGPU host a transient probe failure would otherwise + // expose the PFs/VFs as available passthrough slots while active vGPU + // assignments exist. + logger.FromContext(ctx).WarnContext(ctx, "failed to discover vGPU state", "error", err) return nil } - - switch mode { - case devices.GPUModeVGPU: - return getVGPUStatus() - case devices.GPUModePassthrough: - return getPassthroughStatus() - default: - return nil + if framework != devices.VGPUFrameworkNone { + return getVGPUStatus(ctx, framework, vfs) } + return getPassthroughStatus() } -// getVGPUStatus returns GPU status for vGPU mode (SR-IOV + mdev). -func getVGPUStatus() *GPUResourceStatus { - vfs, err := devices.DiscoverVFs() - if err != nil || len(vfs) == 0 { - return nil - } - - // Count used VFs (those with mdevs) +// getVGPUStatus returns GPU status for vGPU mode (SR-IOV). +func getVGPUStatus(ctx context.Context, framework devices.VGPUFramework, vfs []devices.VirtualFunction) *GPUResourceStatus { usedSlots := 0 + // Count used VFs (those with a vGPU assigned) for _, vf := range vfs { - if vf.HasMdev { + if vf.Allocated { usedSlots++ } } // Get available profiles (reuse VFs to avoid redundant discovery) - profiles, err := devices.ListGPUProfilesWithVFs(vfs) + profiles, err := devices.ListGPUProfilesWithVFs(framework, vfs) if err != nil { + logger.FromContext(ctx).WarnContext(ctx, "failed to list vGPU profiles; reporting none", "framework", framework, "error", err) profiles = nil } diff --git a/lib/resources/monitoring_test.go b/lib/resources/monitoring_test.go index bd092a3f0..e6df0d483 100644 --- a/lib/resources/monitoring_test.go +++ b/lib/resources/monitoring_test.go @@ -188,7 +188,7 @@ func TestStartMonitoringPublishesGPUMetrics(t *testing.T) { mgr, _, _ := monitoringTestManager(t) originalProvider := currentGPUStatusProvider() - setGPUStatusProvider(func() *GPUResourceStatus { + setGPUStatusProvider(func(context.Context) *GPUResourceStatus { return &GPUResourceStatus{ Mode: "vgpu", TotalSlots: 8, diff --git a/lib/resources/resource.go b/lib/resources/resource.go index caaf5ba50..86f644bda 100644 --- a/lib/resources/resource.go +++ b/lib/resources/resource.go @@ -37,13 +37,13 @@ var ( gpuStatusProvider = GetGPUStatus ) -func currentGPUStatusProvider() func() *GPUResourceStatus { +func currentGPUStatusProvider() func(context.Context) *GPUResourceStatus { gpuStatusProviderMu.RLock() defer gpuStatusProviderMu.RUnlock() return gpuStatusProvider } -func setGPUStatusProvider(fn func() *GPUResourceStatus) { +func setGPUStatusProvider(fn func(context.Context) *GPUResourceStatus) { if fn == nil { fn = GetGPUStatus } @@ -427,7 +427,7 @@ func (m *Manager) GetFullStatus(ctx context.Context) (*FullResourceStatus, error } // Get GPU status - gpuStatus := currentGPUStatusProvider()() + gpuStatus := currentGPUStatusProvider()(ctx) return &FullResourceStatus{ CPU: *cpuStatus, @@ -691,7 +691,7 @@ func (m *Manager) validateAllocationLocked(ctx context.Context, excludeID string // Check GPU if needed if req.GPUSlots > 0 { - gpuStatus := currentGPUStatusProvider()() + gpuStatus := currentGPUStatusProvider()(ctx) if gpuStatus == nil { return fmt.Errorf("insufficient GPU: no GPU available on this host") }