Remove verbose vGPU comments

Author: yummybomb

cmd/api/main.go

@@ -159,11 +159,6 @@ func configureUFFDGraduationController(cfg *config.Config, instanceManager insta
 	}, logger), nil
 }
 
-// liveInstanceVGPUDevicePaths returns the stored vGPU device paths of
-// instances whose hypervisor process is still alive. Vendor VFIO
-// reconciliation skips these VFs so it cannot clear an assignment out from
-// under a VM that survived a hypeman restart but has not opened the VFIO
-// device yet.
 func liveInstanceVGPUDevicePaths(ctx context.Context, instanceManager instances.Manager) (map[string]struct{}, error) {
 	allInstances, err := instanceManager.ListInstancesForReconcile(ctx)
 	if err != nil {
@@ -372,17 +367,13 @@ func run() error {
 		return fmt.Errorf("reconcile device state: %w", 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 {
-		// Without the instance inventory we cannot tell live vendor VFIO
-		// assignments from orphans, so skip that framework's reconciliation.
 		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)
 	}
 

integration/vgpu_test.go

@@ -23,23 +23,7 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-// 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 vGPU framework (mdev or vendor VFIO) is discovered
-//   - No vGPU profiles are available
-//   - 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 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.
+// TestVGPU verifies vGPU support through mdev or vendor VFIO.
 func TestVGPU(t *testing.T) {
 	t.Parallel()
 	if testing.Short() {
@@ -163,7 +147,6 @@ func TestVGPU(t *testing.T) {
 	instanceID = inst.Id
 	t.Logf("Instance created: %s", inst.Id)
 
-	// 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:
@@ -193,7 +176,6 @@ func TestVGPU(t *testing.T) {
 		assert.Less(t, availableAfter, availableBefore, "available instances should decrease after creating VM")
 	})
 
-	// Step 6: Verify the assignment exists in sysfs
 	t.Run("VGPUAssignedInSysfs", func(t *testing.T) {
 		assertVGPUAssigned(t, inst.GPUFramework, inst.GPUDevicePath)
 	})
@@ -243,7 +225,6 @@ func TestVGPU(t *testing.T) {
 		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")
@@ -255,7 +236,6 @@ func TestVGPU(t *testing.T) {
 		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")
@@ -269,8 +249,6 @@ func TestVGPU(t *testing.T) {
 	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 {
@@ -286,8 +264,6 @@ func assertVGPUAssigned(t *testing.T, framework devices.VGPUFramework, devicePat
 	}
 }
 
-// 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 {
@@ -316,7 +292,6 @@ func checkVGPUTestPrerequisites() (string, string) {
 		return "vGPU test requires root (sudo) for mdev creation", ""
 	}
 
-	// Check for a vGPU framework (SR-IOV VFs present)
 	framework, _, err := devices.DiscoverVGPU()
 	if err != nil {
 		return "vGPU test failed to discover vGPU framework: " + err.Error(), ""

lib/devices/mdev_linux.go

@@ -89,9 +89,6 @@ func getCachedProfiles(firstVF string) []profileMetadata {
 	return cachedProfiles
 }
 
-// discoverMdevVFs returns all SR-IOV Virtual Functions available for mdev vGPU.
-// These are discovered by scanning /sys/class/mdev_bus/ which contains
-// VFs that can host mdev devices.
 func discoverMdevVFs() ([]VirtualFunction, error) {
 	entries, err := os.ReadDir(mdevBusPath)
 	if err != nil {
@@ -133,8 +130,6 @@ func discoverMdevVFs() ([]VirtualFunction, error) {
 	return vfs, nil
 }
 
-// listMdevGPUProfilesWithVFs returns available vGPU profiles with availability
-// counts, discovered from the first VF's mdev_supported_types directory.
 func listMdevGPUProfilesWithVFs(vfs []VirtualFunction) ([]GPUProfile, error) {
 	if len(vfs) == 0 {
 		return nil, nil

lib/devices/types.go

@@ -69,7 +69,7 @@ const (
 
 	// GPUModePassthrough indicates whole GPU VFIO passthrough
 	GPUModePassthrough GPUMode = "passthrough"
-	// GPUModeVGPU indicates an mdev or vendor VFIO vGPU host
+	// GPUModeVGPU indicates vGPU mode
 	GPUModeVGPU GPUMode = "vgpu"
 	// GPUModeNone indicates no GPU available
 	GPUModeNone GPUMode = "none"
@@ -88,8 +88,6 @@ type VGPUAssignment struct {
 	Framework  VGPUFramework
 	DevicePath string
 	MdevUUID   string
-	// InstanceID guards vendor VFIO releases: a release is skipped when the
-	// VF's in-process owner is a different instance.
 	InstanceID string
 }
 

lib/devices/vendor_vfio_linux.go

@@ -25,7 +25,7 @@ type vendorVFIOSysfs struct {
 	pciDevicesPath  string
 	procPath        string
 	vfioDevicesPath string
-	owners          map[string]string // assignments created by this process
+	owners          map[string]string
 }
 
 var (
@@ -135,15 +135,9 @@ func (s vendorVFIOSysfs) create(ctx context.Context, profileName, instanceID str
 		}
 	}
 	if !found {
-		// A VF with an assigned vGPU reports nothing creatable, and a VF on a
-		// framebuffer-exhausted GPU reports only the header, so a host at full
-		// capacity has an empty catalog. Report that as capacity, not config.
 		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)
 		}
-		// 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)
 	}
 
@@ -184,17 +178,10 @@ func (s vendorVFIOSysfs) create(ctx context.Context, profileName, instanceID str
 	}, nil
 }
 
-// destroy releases the vGPU assignment on vfAddress. In-process ownership
-// prevents stale instance metadata from releasing a VF before its new QEMU
-// process opens the device. Open VFIO handles protect assignments recovered
-// after a hypeman restart.
 func (s vendorVFIOSysfs) destroy(ctx context.Context, vfAddress, instanceID string) error {
 	return s.destroyWithOpenPaths(ctx, vfAddress, instanceID, nil)
 }
 
-// destroyWithOpenPaths is destroy with an optional pre-scanned set of open
-// VFIO handles, so reconcile can share one /proc scan across all releases.
-// A nil openPaths triggers a fresh scan.
 func (s vendorVFIOSysfs) destroyWithOpenPaths(ctx context.Context, vfAddress, instanceID string, openPaths map[string]struct{}) error {
 	vendorVFIOMu.Lock()
 	defer vendorVFIOMu.Unlock()
@@ -268,9 +255,6 @@ func (s vendorVFIOSysfs) reconcile(ctx context.Context, protectedDevicePaths map
 				return fmt.Errorf("scan open VFIO handles: %w", err)
 			}
 		}
-		// destroyWithOpenPaths repeats this probe under the lock; the
-		// pre-check only keeps live VMs from being logged below as failed
-		// destroys.
 		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)
@@ -287,9 +271,6 @@ func (s vendorVFIOSysfs) reconcile(ctx context.Context, protectedDevicePaths map
 }
 
 func (s vendorVFIOSysfs) selectLeastLoadedVF(vfs []VirtualFunction, metadata []profileMetadata, profileType string) (string, error) {
-	// metadata only covers currently creatable types, so an allocated type no
-	// longer creatable anywhere weighs 0 and its GPU can look less loaded than
-	// it is. The driver's creatable lists still bound placement correctness.
 	framebufferByType := make(map[string]int, len(metadata))
 	for _, profile := range metadata {
 		framebufferByType[profile.TypeName] = profile.FramebufferMB
@@ -374,8 +355,6 @@ func (s vendorVFIOSysfs) vfioDeviceInUse(vfAddress string, openPaths map[string]
 		}
 	}
 
-	// QEMU holds the legacy /dev/vfio/<group> node unless iommufd is
-	// configured, so check the VF's iommu group alongside the iommufd cdevs.
 	target, err := os.Readlink(filepath.Join(s.pciDevicesPath, vfAddress, "iommu_group"))
 	if os.IsNotExist(err) {
 		return false, nil
@@ -397,8 +376,6 @@ func (s vendorVFIOSysfs) vfioDeviceInUse(vfAddress string, openPaths map[string]
 	return false, nil
 }
 
-// openVFIOPaths returns every path under the VFIO device root that any
-// process holds open, from a single /proc scan shared by all VF checks.
 func (s vendorVFIOSysfs) openVFIOPaths() (map[string]struct{}, error) {
 	processes, err := os.ReadDir(s.procPath)
 	if err != nil {
@@ -434,11 +411,6 @@ func (s vendorVFIOSysfs) openVFIOPaths() (map[string]struct{}, error) {
 	return open, nil
 }
 
-// parseCreatableVGPUTypes reads the table the NVIDIA driver emits, with a
-// header row and the type ID first:
-//
-//	ID    : vGPU Name
-//	1147  : NVIDIA L40S-1Q
 func parseCreatableVGPUTypes(value string) ([]profileMetadata, error) {
 	profiles := make([]profileMetadata, 0)
 	for lineNumber, line := range strings.Split(value, "\n") {

lib/devices/vendor_vfio_linux_test.go

@@ -124,8 +124,6 @@ func TestVendorVFIODestroyReleasesUnboundVF(t *testing.T) {
 	}
 }
 
-// At full capacity every VF reports an empty or header-only creatable list,
-// so the catalog is empty and create must report capacity, not a bad profile.
 func TestVendorVFIOCreateReportsCapacityWhenAllGPUsFull(t *testing.T) {
 	t.Parallel()
 
@@ -145,8 +143,6 @@ 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()
 
@@ -284,7 +280,6 @@ func TestVendorVFIOReconcilePreservesVFWhenProcFDLinkScanFails(t *testing.T) {
 	assertFileValue(t, filepath.Join(sysfs.pciDevicesPath, vfAddress, "nvidia", "current_vgpu_type"), "1148")
 }
 
-// A GPU with no framebuffer left prints the header and nothing else.
 func TestParseCreatableVGPUTypesHeaderOnly(t *testing.T) {
 	t.Parallel()
 

lib/devices/vgpu_linux.go

@@ -8,9 +8,7 @@ import (
 	"path/filepath"
 )
 
-// DiscoverVGPU returns the host's active vGPU framework and its virtual
-// functions from a single sysfs scan. Detection is not cached because VFs
-// appear only after SR-IOV enablement, which can happen after hypeman starts.
+// DiscoverVGPU returns the host's active vGPU framework and virtual functions.
 func DiscoverVGPU() (VGPUFramework, []VirtualFunction, error) {
 	return discoverVGPUWith(discoverMdevVFs, hostVendorVFIO.discoverVFs)
 }
@@ -42,8 +40,7 @@ func ListGPUProfiles() ([]GPUProfile, error) {
 	return ListGPUProfilesWithVFs(framework, vfs)
 }
 
-// ListGPUProfilesWithVFs returns available profiles for VFs already
-// discovered under the given framework, avoiding a re-scan.
+// ListGPUProfilesWithVFs returns available profiles for discovered VFs.
 func ListGPUProfilesWithVFs(framework VGPUFramework, vfs []VirtualFunction) ([]GPUProfile, error) {
 	switch framework {
 	case VGPUFrameworkMdev:
@@ -83,8 +80,6 @@ func CreateVGPU(ctx context.Context, profileName, instanceID string) (*VGPUDevic
 
 func DestroyVGPU(ctx context.Context, assignment VGPUAssignment) error {
 	framework := assignment.Framework
-	// Metadata written before GPUFramework existed carries only the mdev
-	// UUID, so treat it as an mdev assignment.
 	if framework == VGPUFrameworkNone && assignment.MdevUUID != "" {
 		framework = VGPUFrameworkMdev
 	}
@@ -105,12 +100,7 @@ func DestroyVGPU(ctx context.Context, assignment VGPUAssignment) error {
 	}
 }
 
-// ReconcileVGPUs releases orphaned vGPU assignments. Vendor VFIO VFs whose
-// stored device path is in protectedDevicePaths are skipped because a live
-// instance holds the assignment, even if no process has opened the VFIO
-// device yet (e.g. a VM still booting after a hypeman restart). A nil map
-// means the protected paths are unavailable, so vendor VFIO reconciliation
-// is skipped; mdev reconciliation does not need them and still runs.
+// ReconcileVGPUs releases orphaned vGPU assignments.
 func ReconcileVGPUs(ctx context.Context, protectedDevicePaths map[string]struct{}) error {
 	framework, _, err := DiscoverVGPU()
 	if err != nil {

lib/instances/delete.go

@@ -225,8 +225,6 @@ func (m *manager) killHypervisor(ctx context.Context, inst *Instance) error {
 					break
 				}
 				if err != nil {
-					// After a hypeman restart the hypervisor is not our child, so
-					// Wait4 returns ECHILD. Wait until it has actually exited.
 					if killErr := syscall.Kill(pid, 0); killErr == syscall.ESRCH {
 						log.DebugContext(ctx, "hypervisor process killed", "instance_id", inst.Id, "pid", pid)
 						break

lib/instances/lifecycle_noop_test.go

@@ -167,10 +167,6 @@ 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)

lib/instances/manager.go

@@ -697,8 +697,7 @@ func (m *manager) UpdateInstance(ctx context.Context, id string, req UpdateInsta
 	return inst, err
 }
 
-// ListInstancesForReconcile returns every instance and fails if any metadata
-// file cannot be loaded, so callers never reconcile against a partial inventory.
+// ListInstancesForReconcile returns every instance or an invalid metadata error.
 func (m *manager) ListInstancesForReconcile(ctx context.Context) ([]Instance, error) {
 	return m.loadInstances(ctx, false)
 }