Guard vGPU releases with live-instance claims and branch the integration test by framework

A vGPU assignment goes stale when its release succeeds but the metadata
save does not (or start fails between the release and its first save).
The in-process owner map only covers assignments created since the last
restart and the VFIO handle scan only covers VMs that have opened the
device, so after a restart a stale release could still write 0 to a VF
during another live instance's pre-open boot window. Consult live
instance metadata on every release: when another instance with a live
hypervisor process claims the same device path, drop the stale metadata
without touching the device.

The integration test asserted mdev specifics (UUID, /sys/bus/mdev path)
and failed before exercising the lifecycle on a vendor VFIO host.
Branch the assertions by framework and extend the test to cover release
on stop and reacquisition on start.

Also report an absent-but-possibly-valid profile as ambiguous instead
of 'not found': the vendor VFIO creatable catalog is capacity-dependent,
so a valid larger profile disappears while smaller ones remain.

Author: yummybomb

integration/vgpu_test.go

@@ -4,6 +4,8 @@ import (
 	"bytes"
 	"context"
 	"os"
+	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 
@@ -21,19 +23,21 @@ 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.
+// 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) {
@@ -159,9 +163,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 +193,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 +235,74 @@ 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)
+	})
+
+	// Step 10: Stop releases the assignment
+	t.Log("Step 10: Stopping instance to release the vGPU...")
+	_, 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)
+	})
+
+	// Step 11: Start reacquires an assignment
+	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!")
 }
 
+// assertVGPUAssigned verifies in sysfs that the device at path carries a live
+// vGPU assignment for the given framework.
+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)
+	}
+}
+
+// assertVGPUReleased verifies in sysfs that the device at path no longer
+// carries a vGPU assignment.
+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) {

lib/devices/vendor_vfio_linux.go

@@ -141,7 +141,10 @@ func (s vendorVFIOSysfs) create(ctx context.Context, profileName, instanceID str
 		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 not found", profileName)
+		// The creatable catalog is capacity-dependent: a valid larger profile
+		// disappears once no GPU can fit it while smaller ones remain, so an
+		// absent profile is indistinguishable from an unknown one.
+		return nil, fmt.Errorf("profile %q is not creatable on any VF (unknown profile or insufficient capacity)", profileName)
 	}
 
 	targetVF, err := s.selectLeastLoadedVF(vfs, metadata, requested.TypeName)

lib/devices/vendor_vfio_linux_test.go

@@ -111,6 +111,21 @@ func TestVendorVFIOCreateReportsCapacityWhenAllGPUsFull(t *testing.T) {
 	assert.ErrorContains(t, err, "GPUs may be at capacity")
 }
 
+// A partially loaded host drops larger profiles from the creatable lists
+// while smaller ones remain, so a missing profile cannot be proven invalid.
+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()
 

lib/instances/delete.go

@@ -129,7 +129,7 @@ func (m *manager) deleteInstanceWithOptions(
 	// or volume teardown. A failed release retains the instance metadata, and
 	// nothing destructive has happened to its attachments yet, so a retried
 	// delete is safe.
-	if err := releaseStoredVGPU(ctx, stored); err != nil {
+	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)
 	}

lib/instances/lifecycle_noop_test.go

@@ -166,6 +166,46 @@ func TestDeleteRetainsMetadataWhenVGPUReleaseFails(t *testing.T) {
 	assert.Equal(t, "/sys/bus/pci/devices/0000:82:00.4", stored.GPUDevicePath)
 }
 
+// A stale assignment (release succeeded but the save did not) can reference a
+// device that has since been reassigned. Releasing it must not touch the
+// device out from under the live instance that owns it now; the stale
+// metadata is dropped instead, so the delete completes.
+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.VGPUFramework("future-framework")
+	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()
+	require.NoError(t, m.saveMetadata(&metadata{StoredMetadata: StoredMetadata{
+		Id:             claimantID,
+		Name:           claimantID,
+		Image:          "test-image",
+		CreatedAt:      now,
+		HypervisorType: lifecycleNoopHypervisorType,
+		HypervisorPID:  &pid,
+		SocketPath:     m.paths.InstanceSocket(claimantID, "noop.sock"),
+		DataDir:        m.paths.InstanceDir(claimantID),
+		GPUProfile:     "NVIDIA L40S-2Q",
+		GPUFramework:   devices.VGPUFramework("future-framework"),
+		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{}

lib/instances/start.go

@@ -48,7 +48,7 @@ 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)
 	}
-	if err := releaseStoredVGPU(ctx, stored); err != nil {
+	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)
 	}

lib/instances/stop.go

@@ -263,7 +263,7 @@ func (m *manager) stopInstance(
 	}
 
 	// 7. Release the vGPU assignment if present.
-	if err := releaseStoredVGPU(ctx, stored); err != nil {
+	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)
 	}
 

lib/instances/vgpu.go

@@ -2,7 +2,9 @@ package instances
 
 import (
 	"context"
+	"fmt"
 	"path/filepath"
+	"syscall"
 
 	"github.com/kernel/hypeman/lib/devices"
 	"github.com/kernel/hypeman/lib/logger"
@@ -20,23 +22,59 @@ func clearStoredVGPUDevice(stored *StoredMetadata) {
 	stored.GPUMdevUUID = ""
 }
 
-func releaseStoredVGPU(ctx context.Context, stored *StoredMetadata) error {
+func (m *manager) releaseStoredVGPU(ctx context.Context, stored *StoredMetadata) error {
 	path := storedVGPUDevicePath(stored)
 	if path != "" {
-		assignment := devices.VGPUAssignment{
-			Framework:  stored.GPUFramework,
-			DevicePath: path,
-			MdevUUID:   stored.GPUMdevUUID,
-			InstanceID: stored.Id,
-		}
-		if err := devices.DestroyVGPU(ctx, assignment); err != nil {
+		claimed, err := m.vgpuAssignmentClaimedByLiveInstance(ctx, stored.Id, path)
+		if err != nil {
 			return err
 		}
+		if claimed {
+			// The stored claim is stale: the device now belongs to a live
+			// instance, so drop the metadata without touching the device.
+			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 := devices.DestroyVGPU(ctx, assignment); err != nil {
+				return err
+			}
+		}
 	}
 	clearStoredVGPUDevice(stored)
 	return nil
 }
 
+// vgpuAssignmentClaimedByLiveInstance reports whether an instance other than
+// excludeID claims devicePath with a live hypervisor process. An assignment
+// goes stale when its release succeeds but the metadata save does not (or
+// start fails between the release and its first save); the device can then be
+// reassigned, and honoring the stale claim would release it out from under
+// the live instance. The in-process owner map only covers assignments created
+// since the last restart and the VFIO handle scan only covers VMs that have
+// opened the device, so live metadata is the durable source consulted here.
+func (m *manager) vgpuAssignmentClaimedByLiveInstance(ctx context.Context, excludeID, devicePath string) (bool, error) {
+	instances, err := m.listInstances(ctx)
+	if err != nil {
+		return false, fmt.Errorf("list instances for vGPU release check: %w", err)
+	}
+	for i := range instances {
+		inst := &instances[i]
+		if inst.Id == excludeID || inst.GPUDevicePath != devicePath || inst.HypervisorPID == nil {
+			continue
+		}
+		if syscall.Kill(*inst.HypervisorPID, 0) == nil {
+			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
@@ -52,7 +90,7 @@ func (m *manager) releaseRetainedVGPULocked(ctx context.Context, id string) {
 	if storedVGPUDevicePath(stored) == "" {
 		return
 	}
-	if err := releaseStoredVGPU(ctx, stored); err != nil {
+	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
 	}

lib/instances/vgpu_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/kernel/hypeman/lib/devices"
+	"github.com/kernel/hypeman/lib/paths"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -24,11 +25,12 @@ func TestStoredVGPUDevicePath(t *testing.T) {
 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 := releaseStoredVGPU(context.Background(), stored)
+	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)