Tidy VF health store boundaries and admission errors

- Name the unavailable VF health store in the GPU admission error
  instead of reporting slot exhaustion with used < total.
- Load the VF health store in devices.NewManager so a manager built
  outside wire cannot run placement against an empty, non-persisting
  quarantine set.
- Inline the single-caller placement-lock wrapper and the locked
  checked-addresses variant; AllocatableVFs no longer reaches into
  store internals.
- Deduplicate the sentinel controller's probe block; distinguish the
  stat-failure and load-failure warn messages.
- Restore the fail-closed rationale comment on listMetadataFilesStrict
  and document the archive-before-assign detection window in GPU.md.

Author: yummybomb

lib/devices/GPU.md

@@ -302,8 +302,12 @@ one gets through.
 
 Detection requires the hypeman guest agent: an image that skips the agent
 never reports, so a wedge hit exclusively by such images stays undetected in
-v1. The scanner requires the complete standalone guest-agent log envelope, so
-customer commands and ordinary output containing the marker do not match. The
+v1. Start archives the previous boot's app log before persisting a new
+assignment, so a marker written within one scan interval of a stop/start is
+archived unscanned; that VF returns to the pool unconvicted until the next
+victim boot re-emits the marker. The scanner requires the complete standalone
+guest-agent log envelope, so customer commands and ordinary output containing
+the marker do not match. The
 serial console remains guest-writable: a root guest can deliberately imitate
 the entire line and quarantine the VF its own instance holds; the quarantine
 only ever removes capacity, never touches the instance.

lib/devices/manager.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"log/slog"
 	"os"
 	"runtime"
 	"strings"
@@ -85,6 +86,12 @@ type manager struct {
 // NewManager creates a new device manager.
 // Use SetLivenessChecker after construction to enable accurate orphan detection.
 func NewManager(p *paths.Paths) Manager {
+	// The VF health store lives with the device manager: constructing one
+	// without loading the store would run placement against an empty,
+	// non-persisting quarantine set.
+	if err := InitVFHealth(p.VFHealthState()); err != nil {
+		slog.Default().Error("failed to load VF health state; vGPU placement is disabled until the state file is repaired or removed", "error", err)
+	}
 	return &manager{
 		paths:      p,
 		vfioBinder: NewVFIOBinder(),

lib/devices/vf_health.go

@@ -45,13 +45,6 @@ var (
 	vendorVFIOMu sync.Mutex
 )
 
-// withVGPUPlacementLock serializes vendor VFIO placement with quarantine updates.
-func withVGPUPlacementLock(f func()) {
-	vendorVFIOMu.Lock()
-	defer vendorVFIOMu.Unlock()
-	f()
-}
-
 // InitVFHealth points the store at its state file and loads any persisted quarantines.
 func InitVFHealth(path string) error {
 	vfHealth.mu.Lock()
@@ -115,10 +108,6 @@ func (s *vfHealthStore) ensureLoadedLocked() error {
 func (s *vfHealthStore) checkedAddresses() (map[string]struct{}, error) {
 	s.mu.Lock()
 	defer s.mu.Unlock()
-	return s.checkedAddressesLocked()
-}
-
-func (s *vfHealthStore) checkedAddressesLocked() (map[string]struct{}, error) {
 	if err := s.ensureLoadedLocked(); err != nil {
 		return nil, fmt.Errorf("VF health state unavailable: %w", err)
 	}
@@ -131,13 +120,10 @@ func (s *vfHealthStore) checkedAddressesLocked() (map[string]struct{}, error) {
 
 // AllocatableVFs returns the number of free VFs eligible for placement.
 func AllocatableVFs(framework VGPUFramework, vfs []VirtualFunction) (int, error) {
-	vfHealth.mu.Lock()
-	defer vfHealth.mu.Unlock()
-
 	if framework != VGPUFrameworkVendorVFIO {
 		return countFreeVFs(vfs, nil), nil
 	}
-	quarantined, err := vfHealth.checkedAddressesLocked()
+	quarantined, err := vfHealth.checkedAddresses()
 	if err != nil {
 		return 0, err
 	}
@@ -157,14 +143,13 @@ func countFreeVFs(vfs []VirtualFunction, quarantined map[string]struct{}) int {
 	return available
 }
 
-// QuarantineVF records a wedge conviction and persists it, under the vGPU
-// placement lock so a convicted VF is never concurrently selected. A repeat
-// conviction leaves the record unchanged and reports existed=true.
+// QuarantineVF records a wedge conviction and persists it, under the vendor
+// VFIO placement lock so a convicted VF is never concurrently selected. A
+// repeat conviction leaves the record unchanged and reports existed=true.
 func QuarantineVF(q VFQuarantine) (existed bool, err error) {
-	withVGPUPlacementLock(func() {
-		existed, err = vfHealth.quarantine(q)
-	})
-	return existed, err
+	vendorVFIOMu.Lock()
+	defer vendorVFIOMu.Unlock()
+	return vfHealth.quarantine(q)
 }
 
 // VFHealthStoreUnavailable reports whether the persisted VF health state

lib/instances/storage.go

@@ -195,7 +195,9 @@ func (m *manager) listMetadataFiles() ([]string, error) {
 }
 
 // listMetadataFilesStrict returns readable metadata paths and joins any stat
-// errors other than absence.
+// errors other than absence. Fail-closed callers (the vGPU release claim scan
+// and startup reconcile protection) use it so an unreadable instance is an
+// error instead of silently missing.
 func (m *manager) listMetadataFilesStrict() ([]string, error) {
 	files, statErr, err := m.walkMetadataFiles()
 	return files, errors.Join(statErr, err)

lib/instances/vgpu_sentinel.go

@@ -140,29 +140,24 @@ func (c *VGPUSentinelController) Run(ctx context.Context) error {
 	// The framework is fixed for the process lifetime, so one successful
 	// probe settles the gate. A failed probe fails open (a transient
 	// discovery error must not disable detection) and is retried each tick.
-	vendor, known := c.probeVendorVFIO()
-	if known && !vendor {
-		return nil
-	}
-	if known {
-		c.log.Info("vGPU sentinel controller started")
-	}
 	ticker := time.NewTicker(c.interval)
 	defer ticker.Stop()
+	var known bool
 	for {
+		if !known {
+			var vendor bool
+			vendor, known = c.probeVendorVFIO()
+			if known && !vendor {
+				return nil
+			}
+			if known {
+				c.log.Info("vGPU sentinel controller started")
+			}
+		}
 		select {
 		case <-ctx.Done():
 			return nil
 		case <-ticker.C:
-			if !known {
-				vendor, known = c.probeVendorVFIO()
-				if known && !vendor {
-					return nil
-				}
-				if known {
-					c.log.Info("vGPU sentinel controller started")
-				}
-			}
 			c.scanOnce(ctx)
 		}
 	}
@@ -336,7 +331,7 @@ func (m *manager) listVGPUSentinelTargets(ctx context.Context) ([]vgpuSentinelTa
 		return nil, err
 	}
 	if statErr != nil {
-		logger.FromContext(ctx).WarnContext(ctx, "vGPU sentinel skipping unreadable instance metadata", "error", statErr)
+		logger.FromContext(ctx).WarnContext(ctx, "vGPU sentinel cannot stat some instance metadata; their VFs are not scanned", "error", statErr)
 	}
 	targets := make([]vgpuSentinelTarget, 0, len(files))
 	for _, file := range files {

lib/instances/vgpu_sentinel_test.go

@@ -353,7 +353,7 @@ func TestListVGPUSentinelTargetsSkipsUnstattableMetadata(t *testing.T) {
 	require.NoError(t, err)
 	require.Len(t, targets, 1)
 	assert.Equal(t, "readable", targets[0].instanceID)
-	assert.Contains(t, logs.String(), "vGPU sentinel skipping unreadable instance metadata")
+	assert.Contains(t, logs.String(), "vGPU sentinel cannot stat some instance metadata; their VFs are not scanned")
 	assert.Contains(t, logs.String(), "unreadable-a")
 	assert.Contains(t, logs.String(), "unreadable-b")
 }

lib/providers/providers.go

@@ -96,9 +96,6 @@ func ProvideNetworkManager(p *paths.Paths, cfg *config.Config) network.Manager {
 
 // ProvideDeviceManager provides the device manager
 func ProvideDeviceManager(p *paths.Paths) devices.Manager {
-	if err := devices.InitVFHealth(p.VFHealthState()); err != nil {
-		slog.Default().Error("failed to load VF health state; vGPU placement is disabled until the state file is repaired or removed", "error", err)
-	}
 	return devices.NewManager(p)
 }
 

lib/resources/gpu.go

@@ -10,12 +10,16 @@ import (
 // GPUResourceStatus represents the GPU resource status for the API response.
 // Returns nil if no GPU is available on the host.
 type GPUResourceStatus struct {
-	Mode             string                      `json:"mode"`               // "vgpu" or "passthrough"
-	TotalSlots       int                         `json:"total_slots"`        // VFs for vGPU, physical GPUs for passthrough
-	UsedSlots        int                         `json:"used_slots"`         // Slots currently in use
-	AllocatableSlots int                         `json:"-"`                  // Healthy free slots used by admission control
-	Profiles         []devices.GPUProfile        `json:"profiles,omitempty"` // vGPU mode only
-	Devices          []devices.PassthroughDevice `json:"devices,omitempty"`  // passthrough mode only
+	Mode             string `json:"mode"`        // "vgpu" or "passthrough"
+	TotalSlots       int    `json:"total_slots"` // VFs for vGPU, physical GPUs for passthrough
+	UsedSlots        int    `json:"used_slots"`  // Slots currently in use
+	AllocatableSlots int    `json:"-"`           // Healthy free slots used by admission control
+	// AllocatableSlotsErr explains a zeroed AllocatableSlots caused by an
+	// unreadable VF health store, so admission failures name the real cause
+	// instead of reading as exhaustion.
+	AllocatableSlotsErr string                      `json:"-"`
+	Profiles            []devices.GPUProfile        `json:"profiles,omitempty"` // vGPU mode only
+	Devices             []devices.PassthroughDevice `json:"devices,omitempty"`  // passthrough mode only
 }
 
 // GetGPUStatus returns the current GPU resource status.
@@ -53,18 +57,19 @@ func getVGPUStatus(ctx context.Context, framework devices.VGPUFramework, vfs []d
 		profiles = nil
 	}
 	allocatableSlots, err := devices.AllocatableVFs(framework, vfs)
-	if err != nil {
-		logger.FromContext(ctx).WarnContext(ctx, "failed to count allocatable vGPU slots; reporting none", "framework", framework, "error", err)
-		allocatableSlots = 0
-	}
-
-	return &GPUResourceStatus{
+	status := &GPUResourceStatus{
 		Mode:             string(devices.GPUModeVGPU),
 		TotalSlots:       len(vfs),
 		UsedSlots:        usedSlots,
 		AllocatableSlots: allocatableSlots,
 		Profiles:         profiles,
 	}
+	if err != nil {
+		logger.FromContext(ctx).WarnContext(ctx, "failed to count allocatable vGPU slots; reporting none", "framework", framework, "error", err)
+		status.AllocatableSlots = 0
+		status.AllocatableSlotsErr = err.Error()
+	}
+	return status
 }
 
 // getPassthroughStatus returns GPU status for whole-GPU passthrough mode.

lib/resources/gpu_test.go

@@ -24,6 +24,8 @@ func TestGetVGPUStatusFailsClosedWhenVFHealthIsUnavailable(t *testing.T) {
 
 	status := getVGPUStatus(context.Background(), devices.VGPUFrameworkVendorVFIO, []devices.VirtualFunction{{PCIAddress: "0000:82:00.4"}})
 	assert.Zero(t, status.AllocatableSlots)
+	assert.Contains(t, status.AllocatableSlotsErr, "VF health state unavailable",
+		"admission must be able to name the real cause instead of reporting exhaustion")
 }
 
 func TestReserveAllocationUsesAllocatableGPUSlots(t *testing.T) {
@@ -42,6 +44,11 @@ func TestReserveAllocationUsesAllocatableGPUSlots(t *testing.T) {
 	err := mgr.ValidateAllocation(ctx, 0, 0, 0, 0, 0, 0, true)
 	require.ErrorContains(t, err, "no allocatable vgpu slots")
 
+	status.AllocatableSlotsErr = "VF health state unavailable: read failed"
+	err = mgr.ValidateAllocation(ctx, 0, 0, 0, 0, 0, 0, true)
+	require.ErrorContains(t, err, "vGPU placement is disabled: VF health state unavailable")
+	status.AllocatableSlotsErr = ""
+
 	status.AllocatableSlots = 1
 	require.NoError(t, mgr.ReserveAllocation(ctx, "pending-a", 0, 0, 0, 0, 0, 0, true))
 	err = mgr.ReserveAllocation(ctx, "pending-b", 0, 0, 0, 0, 0, 0, true)

lib/resources/resource.go

@@ -697,6 +697,9 @@ func (m *Manager) validateAllocationLocked(ctx context.Context, excludeID string
 		}
 		availableSlots := gpuStatus.AllocatableSlots - pending.GPUSlots
 		if availableSlots < req.GPUSlots {
+			if gpuStatus.AllocatableSlotsErr != "" {
+				return fmt.Errorf("insufficient GPU: vGPU placement is disabled: %s", gpuStatus.AllocatableSlotsErr)
+			}
 			if availableSlots <= 0 {
 				return fmt.Errorf("insufficient GPU: no allocatable %s slots available (%d total, %d in use)",
 					gpuStatus.Mode, gpuStatus.TotalSlots, gpuStatus.UsedSlots)