From 3097daf36a8c22bf2871e9a212b85e00dbc688f0 Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Thu, 20 Aug 2026 15:39:50 +0000 Subject: [PATCH 01/11] Quarantine wedged vGPU VFs convicted by the guest sentinel --- cmd/api/main.go | 6 + cmd/api/wire.go | 42 ++-- cmd/api/wire_gen.go | 86 ++++---- lib/devices/GPU.md | 32 ++- lib/devices/manager.go | 4 + lib/devices/vendor_vfio_linux.go | 40 +++- lib/devices/vendor_vfio_linux_test.go | 72 +++++++ lib/devices/vf_health.go | 166 ++++++++++++++++ lib/devices/vf_health_darwin.go | 7 + lib/devices/vf_health_test.go | 67 +++++++ lib/instances/vgpu_sentinel.go | 274 ++++++++++++++++++++++++++ lib/instances/vgpu_sentinel_test.go | 184 +++++++++++++++++ lib/paths/paths.go | 5 + lib/providers/vgpu_sentinel.go | 13 ++ 14 files changed, 929 insertions(+), 69 deletions(-) create mode 100644 lib/devices/vf_health.go create mode 100644 lib/devices/vf_health_darwin.go create mode 100644 lib/devices/vf_health_test.go create mode 100644 lib/instances/vgpu_sentinel.go create mode 100644 lib/instances/vgpu_sentinel_test.go create mode 100644 lib/providers/vgpu_sentinel.go diff --git a/cmd/api/main.go b/cmd/api/main.go index f73a0ae53..0e78758c0 100644 --- a/cmd/api/main.go +++ b/cmd/api/main.go @@ -709,6 +709,12 @@ func run() error { return app.HealthCheckController.Run(gctx) }) } + if app.VGPUSentinelController != nil { + grp.Go(func() error { + logger.Info("starting vGPU sentinel controller") + return app.VGPUSentinelController.Run(gctx) + }) + } if restartController, ok := app.InstanceManager.(interface { StartRestartPolicyController(context.Context) error }); ok { diff --git a/cmd/api/wire.go b/cmd/api/wire.go index 133bf41f6..123d59bf5 100644 --- a/cmd/api/wire.go +++ b/cmd/api/wire.go @@ -29,26 +29,27 @@ import ( // application struct to hold initialized components type application struct { - Ctx context.Context - Logger *slog.Logger - Config *config.Config - ImageManager images.Manager - SystemManager system.Manager - NetworkManager network.Manager - DeviceManager devices.Manager - InstanceManager instances.Manager - VolumeManager volumes.Manager - BuilderManager builders.Manager - IngressManager ingress.Manager - BuildManager builds.Manager - PushManager imagepush.Manager - ResourceManager *resources.Manager - GuestMemoryController guestmemory.Controller - AutoStandbyController *autostandby.Controller - HealthCheckController *instances.HealthCheckController - VMMetricsManager *vm_metrics.Manager - Registry *registry.Registry - ApiService *api.ApiService + Ctx context.Context + Logger *slog.Logger + Config *config.Config + ImageManager images.Manager + SystemManager system.Manager + NetworkManager network.Manager + DeviceManager devices.Manager + InstanceManager instances.Manager + VolumeManager volumes.Manager + BuilderManager builders.Manager + IngressManager ingress.Manager + BuildManager builds.Manager + PushManager imagepush.Manager + ResourceManager *resources.Manager + GuestMemoryController guestmemory.Controller + AutoStandbyController *autostandby.Controller + HealthCheckController *instances.HealthCheckController + VGPUSentinelController *instances.VGPUSentinelController + VMMetricsManager *vm_metrics.Manager + Registry *registry.Registry + ApiService *api.ApiService } // initializeApp is the injector function @@ -72,6 +73,7 @@ func initializeApp() (*application, func(), error) { providers.ProvideGuestMemoryController, providers.ProvideAutoStandbyController, providers.ProvideHealthCheckController, + providers.ProvideVGPUSentinelController, providers.ProvideVMMetricsManager, providers.ProvideRegistry, api.New, diff --git a/cmd/api/wire_gen.go b/cmd/api/wire_gen.go index 9eb13c6ea..57034551a 100644 --- a/cmd/api/wire_gen.go +++ b/cmd/api/wire_gen.go @@ -82,6 +82,10 @@ func initializeApp() (*application, func(), error) { } autostandbyController := providers.ProvideAutoStandbyController(instancesManager, config, logger) healthCheckController := providers.ProvideHealthCheckController(instancesManager, logger) + vgpuSentinelController, err := providers.ProvideVGPUSentinelController(instancesManager, logger) + if err != nil { + return nil, nil, err + } vm_metricsManager, err := providers.ProvideVMMetricsManager(instancesManager, config, logger) if err != nil { return nil, nil, err @@ -92,26 +96,27 @@ func initializeApp() (*application, func(), error) { } apiService := api.New(config, manager, instancesManager, volumesManager, buildersManager, networkManager, devicesManager, ingressManager, buildsManager, imagepushManager, resourcesManager, controller, autostandbyController, vm_metricsManager) mainApplication := &application{ - Ctx: context, - Logger: logger, - Config: config, - ImageManager: manager, - SystemManager: systemManager, - NetworkManager: networkManager, - DeviceManager: devicesManager, - InstanceManager: instancesManager, - VolumeManager: volumesManager, - BuilderManager: buildersManager, - IngressManager: ingressManager, - BuildManager: buildsManager, - PushManager: imagepushManager, - ResourceManager: resourcesManager, - GuestMemoryController: controller, - AutoStandbyController: autostandbyController, - HealthCheckController: healthCheckController, - VMMetricsManager: vm_metricsManager, - Registry: registry, - ApiService: apiService, + Ctx: context, + Logger: logger, + Config: config, + ImageManager: manager, + SystemManager: systemManager, + NetworkManager: networkManager, + DeviceManager: devicesManager, + InstanceManager: instancesManager, + VolumeManager: volumesManager, + BuilderManager: buildersManager, + IngressManager: ingressManager, + BuildManager: buildsManager, + PushManager: imagepushManager, + ResourceManager: resourcesManager, + GuestMemoryController: controller, + AutoStandbyController: autostandbyController, + HealthCheckController: healthCheckController, + VGPUSentinelController: vgpuSentinelController, + VMMetricsManager: vm_metricsManager, + Registry: registry, + ApiService: apiService, } return mainApplication, func() { }, nil @@ -121,24 +126,25 @@ func initializeApp() (*application, func(), error) { // application struct to hold initialized components type application struct { - Ctx context.Context - Logger *slog.Logger - Config *config.Config - ImageManager images.Manager - SystemManager system.Manager - NetworkManager network.Manager - DeviceManager devices.Manager - InstanceManager instances.Manager - VolumeManager volumes.Manager - BuilderManager builders.Manager - IngressManager ingress.Manager - BuildManager builds.Manager - PushManager imagepush.Manager - ResourceManager *resources.Manager - GuestMemoryController guestmemory.Controller - AutoStandbyController *autostandby.Controller - HealthCheckController *instances.HealthCheckController - VMMetricsManager *vm_metrics.Manager - Registry *registry.Registry - ApiService *api.ApiService + Ctx context.Context + Logger *slog.Logger + Config *config.Config + ImageManager images.Manager + SystemManager system.Manager + NetworkManager network.Manager + DeviceManager devices.Manager + InstanceManager instances.Manager + VolumeManager volumes.Manager + BuilderManager builders.Manager + IngressManager ingress.Manager + BuildManager builds.Manager + PushManager imagepush.Manager + ResourceManager *resources.Manager + GuestMemoryController guestmemory.Controller + AutoStandbyController *autostandby.Controller + HealthCheckController *instances.HealthCheckController + VGPUSentinelController *instances.VGPUSentinelController + VMMetricsManager *vm_metrics.Manager + Registry *registry.Registry + ApiService *api.ApiService } diff --git a/lib/devices/GPU.md b/lib/devices/GPU.md index ff11c5cde..c6e264b2f 100644 --- a/lib/devices/GPU.md +++ b/lib/devices/GPU.md @@ -282,13 +282,26 @@ NVRM: GPU 0000:00:03.0: RmInitAdapter failed! (0x22:0x65:884) ``` (0x65 = timeout; the guest's init requests are never answered, and -`/proc/interrupts` shows the GPU's MSI-X vectors allocated but idle). Because -placement is deterministic least-loaded, an idle host re-picks the same VF for -every request, so one wedged VF presents as all vGPU instances failing while -`/resources` reports full capacity. - -The wedge itself leaves no host-side log: no kernel error, no XID, no plugin -crash. The trigger is a SIGKILL delivered to QEMU while the vGPU plugin is +`/proc/interrupts` shows the GPU's MSI-X vectors allocated but idle). + +Hypeman detects this automatically: the guest kernel writes that line to the +serial console, which lands in the instance's `logs/app.log`, and the vGPU +sentinel controller scans that file for every vendor VFIO instance. A match +quarantines the VF in `/gpu/vf-health.json` (it survives restarts): +the VF is excluded from placement and from advertised profile availability, +and its parent GPU becomes overflow-only so it drains toward the SR-IOV +cycle. The conviction is logged at error level (`quarantined wedged vGPU VF`) +and counted in `hypeman_instances_vgpu_sentinel_convictions_total`; +`hypeman_instances_vgpu_quarantined_vfs` gauges the current quarantine count. +A burst of convictions (more than 3 in 15 minutes) pauses auto-conviction, so +a systemic non-wedge init failure — e.g. a guest/host driver mismatch rolling +out — cannot quarantine the fleet. + +The wedge-creating kill itself leaves no host-side log: no kernel error, no +XID, no plugin crash. Detection therefore happens on the next boot that lands +on the VF, whose guest emits the sentinel ~27s after spawn. + +The trigger is a SIGKILL delivered to QEMU while the vGPU plugin is still initializing the VF (roughly the first seconds after process start): a single hard kill in that window wedges the VF near-deterministically, while QEMU processes that exit voluntarily — error exits, QMP quit, SIGTERM — @@ -310,6 +323,11 @@ requires no vGPU assignments on that GPU): /usr/lib/nvidia/sriov-manage -e ``` +After the cycle, boot a verification instance on the recovered VF and confirm +its guest reaches the driver (no sentinel in its app log), then clear the +quarantine by removing the VF's entry from `/gpu/vf-health.json` +and restarting hypeman. + Do not unbind/rebind the VF from the nvidia driver — it breaks the nvidia-vgpu-vfio core-device registration (`vfio_pci_core_device not found`) and the VF stops accepting assignments entirely until the SR-IOV cycle. diff --git a/lib/devices/manager.go b/lib/devices/manager.go index 30763c04d..50e615e76 100644 --- a/lib/devices/manager.go +++ b/lib/devices/manager.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "log/slog" "os" "runtime" "strings" @@ -85,6 +86,9 @@ type manager struct { // NewManager creates a new device manager. // Use SetLivenessChecker after construction to enable accurate orphan detection. func NewManager(p *paths.Paths) Manager { + if err := initVFHealthStore(p.VFHealthState()); err != nil { + slog.Default().Error("failed to load VF health state; persisted quarantines are not in effect", "error", err) + } return &manager{ paths: p, vfioBinder: NewVFIOBinder(), diff --git a/lib/devices/vendor_vfio_linux.go b/lib/devices/vendor_vfio_linux.go index a6d37e80d..2201ecaf5 100644 --- a/lib/devices/vendor_vfio_linux.go +++ b/lib/devices/vendor_vfio_linux.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "log/slog" + "math/rand/v2" "os" "path/filepath" "sort" @@ -29,6 +30,15 @@ type vendorVFIOSysfs struct { vfioDevicesPath string owners map[string]string framebufferByType map[string]int + pickVFIndex func(n int) int // overridden in tests; nil means random +} + +// withVGPUPlacementLock runs f under the lock that serializes vendor VFIO +// vGPU placement, so quarantine updates and VF selection cannot interleave. +func withVGPUPlacementLock(f func()) { + vendorVFIOMu.Lock() + defer vendorVFIOMu.Unlock() + f() } var ( @@ -102,6 +112,7 @@ func (s vendorVFIOSysfs) discoverVFs() ([]VirtualFunction, error) { // available_instances. This is a best-effort snapshot because creating on one // VF may revoke the type from siblings that share its GPU framebuffer. func (s vendorVFIOSysfs) listProfiles(vfs []VirtualFunction) ([]GPUProfile, error) { + quarantined := vfHealth.snapshotAddresses() profilesByType := make(map[string]profileMetadata) creatableVFs := make(map[string]int) for _, vf := range vfs { @@ -113,9 +124,10 @@ func (s vendorVFIOSysfs) listProfiles(vfs []VirtualFunction) ([]GPUProfile, erro slog.Default().Warn("skipping unreadable creatable vGPU types", "vf", vf.PCIAddress, "error", err) continue } + _, bad := quarantined[vf.PCIAddress] for _, profile := range creatable { profilesByType[profile.TypeName] = profile - if !vf.Allocated { + if !vf.Allocated && !bad { creatableVFs[profile.TypeName]++ } } @@ -304,10 +316,22 @@ func (s vendorVFIOSysfs) reconcile(ctx context.Context, protectedDevicePaths map } func (s vendorVFIOSysfs) selectLeastLoadedVF(vfs []VirtualFunction, profileType string) (string, error) { + quarantined := vfHealth.snapshotAddresses() usageByGPU := make(map[string]int) unknownUsageByGPU := make(map[string]bool) + quarantinedByGPU := make(map[string]int) freeByGPU := make(map[string][]VirtualFunction) for _, vf := range vfs { + // A quarantined VF is never a placement candidate, but its parent GPU + // stays usable: the count only deprioritizes the card so it drains + // toward the SR-IOV cycle instead of staying warm. + _, bad := quarantined[vf.PCIAddress] + if bad { + quarantinedByGPU[vf.ParentGPU]++ + if !vf.Allocated { + continue + } + } if vf.Allocated { // framebufferByType only covers currently creatable profiles, so // after a restart an allocated type can be missing when its @@ -341,6 +365,9 @@ func (s vendorVFIOSysfs) selectLeastLoadedVF(vfs []VirtualFunction, profileType gpus = append(gpus, gpu) } sort.Slice(gpus, func(i, j int) bool { + if quarantinedByGPU[gpus[i]] != quarantinedByGPU[gpus[j]] { + return quarantinedByGPU[gpus[i]] < quarantinedByGPU[gpus[j]] + } if unknownUsageByGPU[gpus[i]] != unknownUsageByGPU[gpus[j]] { return !unknownUsageByGPU[gpus[i]] } @@ -352,7 +379,16 @@ func (s vendorVFIOSysfs) selectLeastLoadedVF(vfs []VirtualFunction, profileType if len(gpus) == 0 { return "", nil } - return freeByGPU[gpus[0]][0].PCIAddress, nil + // Randomize among the chosen GPU's free VFs. A deterministic + // lowest-address pick would route every first create on an idle host to + // the same VF, so a single undetected wedged VF presents as every GPU + // create failing. + candidates := freeByGPU[gpus[0]] + pick := s.pickVFIndex + if pick == nil { + pick = rand.IntN + } + return candidates[pick(len(candidates))].PCIAddress, nil } func (s vendorVFIOSysfs) profileMetadata(vfs []VirtualFunction) ([]profileMetadata, error) { diff --git a/lib/devices/vendor_vfio_linux_test.go b/lib/devices/vendor_vfio_linux_test.go index 4f2e40844..490348502 100644 --- a/lib/devices/vendor_vfio_linux_test.go +++ b/lib/devices/vendor_vfio_linux_test.go @@ -571,3 +571,75 @@ func assertFileValue(t *testing.T, path, expected string) { require.NoError(t, err) assert.Equal(t, expected, string(value)) } + +func TestVendorVFIOSkipsQuarantinedVF(t *testing.T) { + resetVFHealthStore(t) + _, err := QuarantineVF(VFQuarantine{VFAddress: "0000:82:00.4"}) + require.NoError(t, err) + + sysfs := newTestVendorVFIOSysfs(t) + sysfs.pickVFIndex = func(int) int { return 0 } + 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) + + 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 TestVendorVFIONoVFWhenAllQuarantined(t *testing.T) { + resetVFHealthStore(t) + _, err := QuarantineVF(VFQuarantine{VFAddress: "0000:82:00.4"}) + require.NoError(t, err) + + sysfs := newTestVendorVFIOSysfs(t) + sysfs.addVF(t, "0000:82:00.0", "0000:82:00.4", "42", "0", testCreatableTypes) + + _, err = sysfs.create(context.Background(), "NVIDIA L40S-1Q", "instance-1") + require.ErrorContains(t, err, "no available VF") +} + +func TestVendorVFIOCardBiasAvoidsGPUWithQuarantinedVF(t *testing.T) { + resetVFHealthStore(t) + _, err := QuarantineVF(VFQuarantine{VFAddress: "0000:82:00.4"}) + require.NoError(t, err) + + // GPU 82 sorts first by address and has a free healthy VF, but its + // quarantined sibling must demote the whole card to overflow-only. + sysfs := newTestVendorVFIOSysfs(t) + sysfs.pickVFIndex = func(int) int { return 0 } + 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) + + 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 TestVendorVFIOSelectUsesTiebreakAmongFreeVFs(t *testing.T) { + sysfs := newTestVendorVFIOSysfs(t) + sysfs.pickVFIndex = func(n int) int { return n - 1 } + 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) + + 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 TestVendorVFIOListProfilesExcludesQuarantinedFromAvailability(t *testing.T) { + resetVFHealthStore(t) + _, err := QuarantineVF(VFQuarantine{VFAddress: "0000:82:00.4"}) + require.NoError(t, err) + + 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) + + vfs, err := sysfs.discoverVFs() + require.NoError(t, err) + profiles, err := sysfs.listProfiles(vfs) + require.NoError(t, err) + assert.Equal(t, 1, profileAvailability(profiles, "NVIDIA L40S-1Q")) +} diff --git a/lib/devices/vf_health.go b/lib/devices/vf_health.go new file mode 100644 index 000000000..1560ab0eb --- /dev/null +++ b/lib/devices/vf_health.go @@ -0,0 +1,166 @@ +package devices + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "sort" + "sync" + "time" +) + +// VFHealthRecord tracks a virtual function quarantined after a wedge +// conviction. A quarantined VF is excluded from vGPU placement until the +// parent GPU is SR-IOV cycled and the record cleared. +type VFHealthRecord struct { + VFAddress string `json:"vf_address"` + InstanceID string `json:"instance_id,omitempty"` + SentinelLine string `json:"sentinel_line,omitempty"` + WedgeCount int `json:"wedge_count"` + QuarantinedAt time.Time `json:"quarantined_at"` +} + +// VFQuarantine describes a wedge conviction to record. +type VFQuarantine struct { + VFAddress string + InstanceID string + SentinelLine string +} + +type vfHealthStore struct { + mu sync.Mutex + path string + records map[string]VFHealthRecord +} + +var vfHealth = &vfHealthStore{records: make(map[string]VFHealthRecord)} + +// initVFHealthStore points the store at its state file and loads any +// persisted quarantines. Called from NewManager during startup wiring. +func initVFHealthStore(path string) error { + vfHealth.mu.Lock() + defer vfHealth.mu.Unlock() + vfHealth.path = path + vfHealth.records = make(map[string]VFHealthRecord) + + data, err := os.ReadFile(path) + if err != nil { + if os.IsNotExist(err) { + return nil + } + return fmt.Errorf("read VF health state: %w", err) + } + var records []VFHealthRecord + if err := json.Unmarshal(data, &records); err != nil { + return fmt.Errorf("unmarshal VF health state: %w", err) + } + for _, record := range records { + vfHealth.records[record.VFAddress] = record + } + return nil +} + +// QuarantineVF records a wedge conviction for a VF and persists it. It takes +// the vGPU placement lock so a convicted VF is never concurrently selected +// for a new assignment. Repeat convictions on the same VF increment its +// wedge count and keep the original quarantine time. +func QuarantineVF(q VFQuarantine) (VFHealthRecord, error) { + var record VFHealthRecord + var err error + withVGPUPlacementLock(func() { + record, err = vfHealth.quarantine(q) + }) + return record, err +} + +// ClearVFQuarantine removes a VF's quarantine record, returning whether one +// existed. Callers clear a VF only after the parent GPU has been SR-IOV +// cycled and a verification boot came back clean. +func ClearVFQuarantine(vfAddress string) (bool, error) { + var cleared bool + var err error + withVGPUPlacementLock(func() { + cleared, err = vfHealth.clear(vfAddress) + }) + return cleared, err +} + +// QuarantinedVFs returns all quarantine records, ordered by VF address. +func QuarantinedVFs() []VFHealthRecord { + vfHealth.mu.Lock() + defer vfHealth.mu.Unlock() + records := make([]VFHealthRecord, 0, len(vfHealth.records)) + for _, record := range vfHealth.records { + records = append(records, record) + } + sort.Slice(records, func(i, j int) bool { return records[i].VFAddress < records[j].VFAddress }) + return records +} + +func (s *vfHealthStore) quarantine(q VFQuarantine) (VFHealthRecord, error) { + s.mu.Lock() + defer s.mu.Unlock() + record, ok := s.records[q.VFAddress] + if !ok { + record = VFHealthRecord{ + VFAddress: q.VFAddress, + QuarantinedAt: time.Now().UTC(), + } + } + record.WedgeCount++ + record.InstanceID = q.InstanceID + record.SentinelLine = q.SentinelLine + s.records[q.VFAddress] = record + if err := s.persistLocked(); err != nil { + return record, err + } + return record, nil +} + +func (s *vfHealthStore) clear(vfAddress string) (bool, error) { + s.mu.Lock() + defer s.mu.Unlock() + if _, ok := s.records[vfAddress]; !ok { + return false, nil + } + delete(s.records, vfAddress) + return true, s.persistLocked() +} + +// snapshotAddresses returns the set of quarantined VF addresses. +func (s *vfHealthStore) snapshotAddresses() map[string]struct{} { + s.mu.Lock() + defer s.mu.Unlock() + addresses := make(map[string]struct{}, len(s.records)) + for address := range s.records { + addresses[address] = struct{}{} + } + return addresses +} + +func (s *vfHealthStore) persistLocked() error { + if s.path == "" { + return nil + } + records := make([]VFHealthRecord, 0, len(s.records)) + for _, record := range s.records { + records = append(records, record) + } + sort.Slice(records, func(i, j int) bool { return records[i].VFAddress < records[j].VFAddress }) + data, err := json.MarshalIndent(records, "", " ") + if err != nil { + return fmt.Errorf("marshal VF health state: %w", err) + } + if err := os.MkdirAll(filepath.Dir(s.path), 0755); err != nil { + return fmt.Errorf("create VF health state dir: %w", err) + } + tmp := s.path + ".tmp" + if err := os.WriteFile(tmp, data, 0644); err != nil { + return fmt.Errorf("write VF health state: %w", err) + } + if err := os.Rename(tmp, s.path); err != nil { + return fmt.Errorf("rename VF health state: %w", err) + } + return nil +} diff --git a/lib/devices/vf_health_darwin.go b/lib/devices/vf_health_darwin.go new file mode 100644 index 000000000..3d5db72a1 --- /dev/null +++ b/lib/devices/vf_health_darwin.go @@ -0,0 +1,7 @@ +package devices + +// withVGPUPlacementLock runs f. Vendor VFIO placement does not exist on +// darwin, so there is no placement lock to hold. +func withVGPUPlacementLock(f func()) { + f() +} diff --git a/lib/devices/vf_health_test.go b/lib/devices/vf_health_test.go new file mode 100644 index 000000000..c5a11b17a --- /dev/null +++ b/lib/devices/vf_health_test.go @@ -0,0 +1,67 @@ +package devices + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// resetVFHealthStore points the package-level store at a fresh temp file and +// restores an empty, unpersisted store when the test finishes. +func resetVFHealthStore(t *testing.T) string { + t.Helper() + path := filepath.Join(t.TempDir(), "vf-health.json") + require.NoError(t, initVFHealthStore(path)) + t.Cleanup(func() { + vfHealth.mu.Lock() + defer vfHealth.mu.Unlock() + vfHealth.path = "" + vfHealth.records = make(map[string]VFHealthRecord) + }) + return path +} + +func TestQuarantineVFPersistsAcrossReload(t *testing.T) { + path := resetVFHealthStore(t) + + record, err := QuarantineVF(VFQuarantine{ + VFAddress: "0000:e3:00.4", + InstanceID: "instance-1", + SentinelLine: "NVRM: GPU 0000:e3:00.4: RmInitAdapter failed! (0x22:0x65:884)", + }) + require.NoError(t, err) + assert.Equal(t, 1, record.WedgeCount) + assert.False(t, record.QuarantinedAt.IsZero()) + + // A repeat conviction increments the count and keeps the original time. + again, err := QuarantineVF(VFQuarantine{VFAddress: "0000:e3:00.4", InstanceID: "instance-2"}) + require.NoError(t, err) + assert.Equal(t, 2, again.WedgeCount) + assert.Equal(t, record.QuarantinedAt, again.QuarantinedAt) + + // Reload from disk, as a hypeman restart would. + require.NoError(t, initVFHealthStore(path)) + records := QuarantinedVFs() + require.Len(t, records, 1) + assert.Equal(t, "0000:e3:00.4", records[0].VFAddress) + assert.Equal(t, 2, records[0].WedgeCount) + assert.Equal(t, "instance-2", records[0].InstanceID) +} + +func TestClearVFQuarantine(t *testing.T) { + resetVFHealthStore(t) + + _, err := QuarantineVF(VFQuarantine{VFAddress: "0000:e3:00.4"}) + require.NoError(t, err) + + cleared, err := ClearVFQuarantine("0000:e3:00.4") + require.NoError(t, err) + assert.True(t, cleared) + assert.Empty(t, QuarantinedVFs()) + + cleared, err = ClearVFQuarantine("0000:e3:00.4") + require.NoError(t, err) + assert.False(t, cleared) +} diff --git a/lib/instances/vgpu_sentinel.go b/lib/instances/vgpu_sentinel.go new file mode 100644 index 000000000..1340d582f --- /dev/null +++ b/lib/instances/vgpu_sentinel.go @@ -0,0 +1,274 @@ +package instances + +import ( + "bufio" + "context" + "errors" + "io" + "log/slog" + "os" + "path/filepath" + "regexp" + "strings" + "time" + + "github.com/kernel/hypeman/lib/devices" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/metric" +) + +const ( + vgpuSentinelScanInterval = 5 * time.Second + + // A burst of convictions is more likely a systemic, non-wedge init + // failure (e.g. a guest/host driver mismatch rolling out fleet-wide) + // than several independent wedges; quarantining every VF would degrade + // the host harder than the failure itself, so auto-conviction pauses. + vgpuSentinelBrakeWindow = 15 * time.Minute + vgpuSentinelBrakeLimit = 3 +) + +// vgpuSentinelPattern matches the guest NVIDIA driver's init-failure line as +// it arrives on the serial console. The full "NVRM: ... RmInitAdapter +// failed!" shape is required: the bare token also appears in echoed exec +// command lines, and the trailing (stage:status:line) tuple is +// driver-build-specific, so neither is safe to match. +var vgpuSentinelPattern = regexp.MustCompile(`NVRM: .*RmInitAdapter failed!`) + +type vgpuSentinelTarget struct { + instanceID string + vfAddress string + appLogPath string +} + +type vgpuSentinelStore interface { + listVGPUSentinelTargets(ctx context.Context) ([]vgpuSentinelTarget, error) +} + +type vgpuSentinelTail struct { + offset int64 + done bool +} + +// VGPUSentinelController scans the serial console log of vendor VFIO vGPU +// instances for the guest driver's RmInitAdapter failure line. That line is a +// positive fingerprint of a wedged VF — the driver is present, trying, and +// failing — so a match quarantines the VF, removing it from placement until +// an operator cycles the parent GPU. +type VGPUSentinelController struct { + store vgpuSentinelStore + log *slog.Logger + interval time.Duration + now func() time.Time + quarantine func(devices.VFQuarantine) (devices.VFHealthRecord, error) + convictions metric.Int64Counter + tails map[string]*vgpuSentinelTail + recent []time.Time +} + +func NewVGPUSentinelController(manager Manager, meter metric.Meter, log *slog.Logger) (*VGPUSentinelController, error) { + if manager == nil || log == nil { + return nil, nil + } + store, ok := manager.(vgpuSentinelStore) + if !ok { + return nil, nil + } + + convictions, err := meter.Int64Counter( + "hypeman_instances_vgpu_sentinel_convictions_total", + metric.WithDescription("Total wedged-VF sentinel matches by result"), + ) + if err != nil { + return nil, err + } + _, err = meter.Int64ObservableGauge( + "hypeman_instances_vgpu_quarantined_vfs", + metric.WithDescription("Number of vGPU virtual functions currently quarantined"), + metric.WithInt64Callback(func(_ context.Context, o metric.Int64Observer) error { + o.Observe(int64(len(devices.QuarantinedVFs()))) + return nil + }), + ) + if err != nil { + return nil, err + } + + return &VGPUSentinelController{ + store: store, + log: log.With("controller", "vgpu_sentinel"), + interval: vgpuSentinelScanInterval, + now: time.Now, + quarantine: devices.QuarantineVF, + convictions: convictions, + tails: make(map[string]*vgpuSentinelTail), + }, nil +} + +func (c *VGPUSentinelController) Run(ctx context.Context) error { + c.log.Info("vGPU sentinel controller started") + ticker := time.NewTicker(c.interval) + defer ticker.Stop() + for { + select { + case <-ctx.Done(): + return nil + case <-ticker.C: + c.scanOnce(ctx) + } + } +} + +func (c *VGPUSentinelController) scanOnce(ctx context.Context) { + targets, err := c.store.listVGPUSentinelTargets(ctx) + if err != nil { + c.log.WarnContext(ctx, "vGPU sentinel scan failed to list instances", "error", err) + return + } + alive := make(map[string]struct{}, len(targets)) + for _, target := range targets { + alive[target.instanceID] = struct{}{} + c.scanTarget(ctx, target) + } + for id := range c.tails { + if _, ok := alive[id]; !ok { + delete(c.tails, id) + } + } +} + +func (c *VGPUSentinelController) scanTarget(ctx context.Context, target vgpuSentinelTarget) { + tail := c.tails[target.instanceID] + if tail == nil { + tail = &vgpuSentinelTail{} + c.tails[target.instanceID] = tail + } + if tail.done { + return + } + line, found, err := scanForSentinel(target.appLogPath, &tail.offset) + if err != nil { + c.log.WarnContext(ctx, "vGPU sentinel scan failed to read app log", + "instance_id", target.instanceID, "error", err) + return + } + if !found { + return + } + tail.done = c.convict(ctx, target, line) +} + +// convict quarantines the target's VF, subject to the conviction brake. +// It reports whether scanning for this instance is finished; a failed +// quarantine leaves the tail open so the recurring sentinel retries it. +func (c *VGPUSentinelController) convict(ctx context.Context, target vgpuSentinelTarget, line string) bool { + now := c.now() + recent := c.recent[:0] + for _, t := range c.recent { + if now.Sub(t) < vgpuSentinelBrakeWindow { + recent = append(recent, t) + } + } + c.recent = recent + + if len(c.recent) >= vgpuSentinelBrakeLimit { + c.log.ErrorContext(ctx, "vGPU sentinel conviction brake engaged; VF not quarantined", + "vf", target.vfAddress, + "instance_id", target.instanceID, + "sentinel_line", line, + "convictions_in_window", len(c.recent), + "window", vgpuSentinelBrakeWindow.String(), + ) + c.convictions.Add(ctx, 1, metric.WithAttributes(attribute.String("result", "suppressed"))) + return true + } + + record, err := c.quarantine(devices.VFQuarantine{ + VFAddress: target.vfAddress, + InstanceID: target.instanceID, + SentinelLine: line, + }) + if err != nil { + c.log.ErrorContext(ctx, "failed to quarantine wedged vGPU VF", + "vf", target.vfAddress, "instance_id", target.instanceID, "error", err) + return false + } + c.recent = append(c.recent, now) + c.log.ErrorContext(ctx, "quarantined wedged vGPU VF", + "vf", target.vfAddress, + "instance_id", target.instanceID, + "sentinel_line", line, + "wedge_count", record.WedgeCount, + ) + c.convictions.Add(ctx, 1, metric.WithAttributes(attribute.String("result", "convicted"))) + return true +} + +// scanForSentinel reads complete lines from offset onward, advancing offset +// and returning the first sentinel match. A partial trailing line is left +// unconsumed for the next scan. An offset past the file size means the log +// was archived for a new boot, so the scan restarts from the top. +func scanForSentinel(path string, offset *int64) (string, bool, error) { + f, err := os.Open(path) + if err != nil { + if os.IsNotExist(err) { + return "", false, nil + } + return "", false, err + } + defer f.Close() + + info, err := f.Stat() + if err != nil { + return "", false, err + } + if info.Size() < *offset { + *offset = 0 + } + if _, err := f.Seek(*offset, io.SeekStart); err != nil { + return "", false, err + } + + reader := bufio.NewReader(f) + for { + line, err := reader.ReadString('\n') + if err != nil { + if errors.Is(err, io.EOF) { + return "", false, nil + } + return "", false, err + } + *offset += int64(len(line)) + if vgpuSentinelPattern.MatchString(line) { + return strings.TrimSpace(line), true, nil + } + } +} + +// listVGPUSentinelTargets returns instances holding a vendor VFIO vGPU +// assignment. It reads raw metadata rather than hydrating full instances: +// the scan runs continuously and deriving state would query every +// hypervisor on the host. +func (m *manager) listVGPUSentinelTargets(ctx context.Context) ([]vgpuSentinelTarget, error) { + files, err := m.listMetadataFiles() + if err != nil { + return nil, err + } + targets := make([]vgpuSentinelTarget, 0, len(files)) + for _, file := range files { + id := filepath.Base(filepath.Dir(file)) + meta, err := m.loadMetadata(id) + if err != nil { + continue + } + if meta.GPUFramework != devices.VGPUFrameworkVendorVFIO || meta.GPUDevicePath == "" { + continue + } + targets = append(targets, vgpuSentinelTarget{ + instanceID: id, + vfAddress: filepath.Base(meta.GPUDevicePath), + appLogPath: m.paths.InstanceAppLog(id), + }) + } + return targets, nil +} diff --git a/lib/instances/vgpu_sentinel_test.go b/lib/instances/vgpu_sentinel_test.go new file mode 100644 index 000000000..b1b2f7140 --- /dev/null +++ b/lib/instances/vgpu_sentinel_test.go @@ -0,0 +1,184 @@ +package instances + +import ( + "context" + "errors" + "log/slog" + "os" + "path/filepath" + "testing" + "time" + + "github.com/kernel/hypeman/lib/devices" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/metric/noop" +) + +const testSentinelLine = "[ 27.031415] NVRM: GPU 0000:e3:00.4: RmInitAdapter failed! (0x22:0x65:884)\n" + +func TestVGPUSentinelPattern(t *testing.T) { + t.Parallel() + + assert.True(t, vgpuSentinelPattern.MatchString(testSentinelLine)) + // Tuple values are driver-build-specific; the match must not depend on them. + assert.True(t, vgpuSentinelPattern.MatchString("[ 47.2] NVRM: GPU 0000:e3:00.4: RmInitAdapter failed! (0x26:0xffff:1482)")) + + // Echoed exec command lines carry the bare token on the same console. + assert.False(t, vgpuSentinelPattern.MatchString("$ dmesg | grep -c RmInitAdapter")) + assert.False(t, vgpuSentinelPattern.MatchString("[ 7.1] NVRM: loading NVIDIA UNIX Open Kernel Module for x86_64")) +} + +type fakeSentinelStore struct { + targets []vgpuSentinelTarget +} + +func (s *fakeSentinelStore) listVGPUSentinelTargets(context.Context) ([]vgpuSentinelTarget, error) { + return s.targets, nil +} + +func newTestSentinelController(t *testing.T, store vgpuSentinelStore) (*VGPUSentinelController, *[]devices.VFQuarantine) { + t.Helper() + counter, err := noop.NewMeterProvider().Meter("test").Int64Counter("test") + require.NoError(t, err) + var quarantined []devices.VFQuarantine + c := &VGPUSentinelController{ + store: store, + log: slog.New(slog.DiscardHandler), + interval: time.Hour, + now: time.Now, + quarantine: func(q devices.VFQuarantine) (devices.VFHealthRecord, error) { + quarantined = append(quarantined, q) + return devices.VFHealthRecord{VFAddress: q.VFAddress, WedgeCount: 1}, nil + }, + convictions: counter, + tails: make(map[string]*vgpuSentinelTail), + } + return c, &quarantined +} + +func TestVGPUSentinelControllerConvictsOnce(t *testing.T) { + t.Parallel() + + logPath := filepath.Join(t.TempDir(), "app.log") + store := &fakeSentinelStore{targets: []vgpuSentinelTarget{{ + instanceID: "instance-1", + vfAddress: "0000:e3:00.4", + appLogPath: logPath, + }}} + c, quarantined := newTestSentinelController(t, store) + ctx := context.Background() + + // Log missing (boot not started) and then healthy output: no conviction. + c.scanOnce(ctx) + require.NoError(t, os.WriteFile(logPath, []byte("booting\nnvidia driver loaded\n"), 0644)) + c.scanOnce(ctx) + assert.Empty(t, *quarantined) + + f, err := os.OpenFile(logPath, os.O_APPEND|os.O_WRONLY, 0644) + require.NoError(t, err) + // A partial line without its newline must not convict yet. + _, err = f.WriteString("[ 27.03] NVRM: GPU 0000:e3:00.4: RmInitAdapter failed") + require.NoError(t, err) + c.scanOnce(ctx) + assert.Empty(t, *quarantined) + + _, err = f.WriteString("! (0x22:0x65:884)\n") + require.NoError(t, err) + require.NoError(t, f.Close()) + c.scanOnce(ctx) + require.Len(t, *quarantined, 1) + assert.Equal(t, "0000:e3:00.4", (*quarantined)[0].VFAddress) + assert.Equal(t, "instance-1", (*quarantined)[0].InstanceID) + assert.Contains(t, (*quarantined)[0].SentinelLine, "RmInitAdapter failed!") + + // The sentinel recurs every ~20s; a convicted instance is not re-convicted. + appendSentinelLine(t, logPath) + c.scanOnce(ctx) + assert.Len(t, *quarantined, 1) +} + +func TestVGPUSentinelControllerRetriesFailedQuarantine(t *testing.T) { + t.Parallel() + + logPath := filepath.Join(t.TempDir(), "app.log") + require.NoError(t, os.WriteFile(logPath, []byte(testSentinelLine), 0644)) + store := &fakeSentinelStore{targets: []vgpuSentinelTarget{{ + instanceID: "instance-1", + vfAddress: "0000:e3:00.4", + appLogPath: logPath, + }}} + c, quarantined := newTestSentinelController(t, store) + quarantineErr := errors.New("persist failed") + realQuarantine := c.quarantine + c.quarantine = func(devices.VFQuarantine) (devices.VFHealthRecord, error) { + return devices.VFHealthRecord{}, quarantineErr + } + ctx := context.Background() + + c.scanOnce(ctx) + assert.Empty(t, *quarantined) + assert.False(t, c.tails["instance-1"].done) + + // The next recurrence of the sentinel retries the quarantine. + c.quarantine = realQuarantine + appendSentinelLine(t, logPath) + c.scanOnce(ctx) + assert.Len(t, *quarantined, 1) + assert.True(t, c.tails["instance-1"].done) +} + +func TestVGPUSentinelControllerBrakeSuppressesConvictionBursts(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + targets := make([]vgpuSentinelTarget, 0, vgpuSentinelBrakeLimit+1) + for i := 0; i < vgpuSentinelBrakeLimit+1; i++ { + logPath := filepath.Join(dir, string(rune('a'+i))+".log") + require.NoError(t, os.WriteFile(logPath, []byte(testSentinelLine), 0644)) + targets = append(targets, vgpuSentinelTarget{ + instanceID: "instance-" + string(rune('a'+i)), + vfAddress: "0000:e3:00." + string(rune('4'+i)), + appLogPath: logPath, + }) + } + c, quarantined := newTestSentinelController(t, &fakeSentinelStore{targets: targets}) + + c.scanOnce(context.Background()) + // The burst converts to convictions up to the limit; the rest are + // suppressed rather than quarantining the whole host. + assert.Len(t, *quarantined, vgpuSentinelBrakeLimit) + for _, target := range targets { + assert.True(t, c.tails[target.instanceID].done) + } +} + +func TestVGPUSentinelControllerDropsStaleTails(t *testing.T) { + t.Parallel() + + logPath := filepath.Join(t.TempDir(), "app.log") + require.NoError(t, os.WriteFile(logPath, []byte("booting\n"), 0644)) + store := &fakeSentinelStore{targets: []vgpuSentinelTarget{{ + instanceID: "instance-1", + vfAddress: "0000:e3:00.4", + appLogPath: logPath, + }}} + c, _ := newTestSentinelController(t, store) + ctx := context.Background() + + c.scanOnce(ctx) + require.Contains(t, c.tails, "instance-1") + + store.targets = nil + c.scanOnce(ctx) + assert.NotContains(t, c.tails, "instance-1") +} + +func appendSentinelLine(t *testing.T, path string) { + t.Helper() + f, err := os.OpenFile(path, os.O_APPEND|os.O_WRONLY, 0644) + require.NoError(t, err) + _, err = f.WriteString(testSentinelLine) + require.NoError(t, err) + require.NoError(t, f.Close()) +} diff --git a/lib/paths/paths.go b/lib/paths/paths.go index add23ff52..aff107dca 100644 --- a/lib/paths/paths.go +++ b/lib/paths/paths.go @@ -314,6 +314,11 @@ func (p *Paths) DeviceMetadata(id string) string { return filepath.Join(p.DeviceDir(id), "metadata.json") } +// VFHealthState returns the path to the persisted vGPU VF health file. +func (p *Paths) VFHealthState() string { + return filepath.Join(p.dataDir, "gpu", "vf-health.json") +} + // Volume path methods // VolumesDir returns the root volumes directory. diff --git a/lib/providers/vgpu_sentinel.go b/lib/providers/vgpu_sentinel.go new file mode 100644 index 000000000..0931f1fe6 --- /dev/null +++ b/lib/providers/vgpu_sentinel.go @@ -0,0 +1,13 @@ +package providers + +import ( + "log/slog" + + "github.com/kernel/hypeman/lib/instances" + "go.opentelemetry.io/otel" +) + +func ProvideVGPUSentinelController(instanceManager instances.Manager, log *slog.Logger) (*instances.VGPUSentinelController, error) { + meter := otel.GetMeterProvider().Meter("hypeman") + return instances.NewVGPUSentinelController(instanceManager, meter, log) +} From c21b617193567508851abcc2e134d2297f675dc9 Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Thu, 20 Aug 2026 19:10:07 +0000 Subject: [PATCH 02/11] Report GPU init failure from the guest agent and harden quarantine Detection moves from host-side matching of the raw NVRM kernel line to an explicit guest-to-host report: the guest agent watches /dev/kmsg for the driver's RmInitAdapter failure and emits a HYPEMAN-GPU-INIT-FAILED marker over the established sentinel-marker channel, which the controller convicts on. The raw kernel line is no longer matched, so detection now requires the guest agent; images without it do not report. Quarantine hardening in the same pass: - The conviction brake pauses instead of drops: a suppressed conviction leaves the tail open and the agent's re-emission retries it once the window clears. - A match on an already-quarantined VF is not a new conviction: no brake accounting, no metric, no wedge-count inflation on controller restarts. - Tails reset when the instance acquires a new assignment, so a finished tail from a previous boot or VF cannot suppress scanning the next one. - A vf-health state file that fails to load refuses mutations (and retries the load) instead of letting the next conviction clobber every previously persisted quarantine. - Oversized unterminated log lines are skipped instead of re-buffered on every scan. - The controller idles on hosts without the vendor VFIO framework. GPU.md: DCGM quiesce is now an ordered step of the recovery sequence, and clearing vf-health.json documents the immediate-restart requirement. --- lib/devices/GPU.md | 55 +++++++---- lib/devices/vendor_vfio_linux_test.go | 8 +- lib/devices/vf_health.go | 81 +++++++++++----- lib/devices/vf_health_test.go | 53 +++++++++-- lib/instances/logs.go | 1 + lib/instances/vgpu_sentinel.go | 115 ++++++++++++++++------ lib/instances/vgpu_sentinel_test.go | 116 +++++++++++++++++++---- lib/system/guest_agent/gpu_watch.go | 106 +++++++++++++++++++++ lib/system/guest_agent/gpu_watch_test.go | 41 ++++++++ lib/system/guest_agent/main.go | 4 + 10 files changed, 481 insertions(+), 99 deletions(-) create mode 100644 lib/system/guest_agent/gpu_watch.go create mode 100644 lib/system/guest_agent/gpu_watch_test.go diff --git a/lib/devices/GPU.md b/lib/devices/GPU.md index c6e264b2f..1e4d38c47 100644 --- a/lib/devices/GPU.md +++ b/lib/devices/GPU.md @@ -284,22 +284,32 @@ NVRM: GPU 0000:00:03.0: RmInitAdapter failed! (0x22:0x65:884) (0x65 = timeout; the guest's init requests are never answered, and `/proc/interrupts` shows the GPU's MSI-X vectors allocated but idle). -Hypeman detects this automatically: the guest kernel writes that line to the -serial console, which lands in the instance's `logs/app.log`, and the vGPU -sentinel controller scans that file for every vendor VFIO instance. A match -quarantines the VF in `/gpu/vf-health.json` (it survives restarts): -the VF is excluded from placement and from advertised profile availability, -and its parent GPU becomes overflow-only so it drains toward the SR-IOV -cycle. The conviction is logged at error level (`quarantined wedged vGPU VF`) -and counted in `hypeman_instances_vgpu_sentinel_convictions_total`; +Hypeman detects this automatically: the guest agent watches the guest kernel +log (`/dev/kmsg`) for that line and reports it as a `HYPEMAN-GPU-INIT-FAILED` +marker — the same guest-to-host channel as the other `HYPEMAN-*` markers, +landing in the instance's `logs/app.log` — and the vGPU sentinel controller +scans that file for every vendor VFIO instance. A match quarantines the VF in +`/gpu/vf-health.json` (it survives restarts): the VF is excluded +from placement and from advertised profile availability, and its parent GPU +becomes overflow-only so it drains toward the SR-IOV cycle. The conviction is +logged at error level (`quarantined wedged vGPU VF`) and counted in +`hypeman_instances_vgpu_sentinel_convictions_total`; `hypeman_instances_vgpu_quarantined_vfs` gauges the current quarantine count. -A burst of convictions (more than 3 in 15 minutes) pauses auto-conviction, so -a systemic non-wedge init failure — e.g. a guest/host driver mismatch rolling -out — cannot quarantine the fleet. +A burst of convictions (more than 3 in 15 minutes) pauses auto-conviction — +the guest agent keeps re-emitting the marker while the failure persists, so a +paused conviction lands once the window clears — so a systemic non-wedge init +failure (e.g. a guest/host driver mismatch rolling out) cannot quarantine the +fleet before an operator sees it. + +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 marker also rides a guest-writable channel — a root guest could forge +it and quarantine its own VF; the conviction brake bounds the blast radius, +and the quarantine only ever removes capacity, never touches the instance. The wedge-creating kill itself leaves no host-side log: no kernel error, no XID, no plugin crash. Detection therefore happens on the next boot that lands -on the VF, whose guest emits the sentinel ~27s after spawn. +on the VF, whose guest driver starts failing ~27s after spawn. The trigger is a SIGKILL delivered to QEMU while the vGPU plugin is still initializing the VF (roughly the first seconds after process start): @@ -316,23 +326,32 @@ External SIGKILLs (OOM killer, manual `kill -9`) can still trigger it. Confirm by assigning the same profile on a different VF: if that guest initializes, the VF is wedged, not the driver stack. Remediate by cycling SR-IOV on the parent GPU (this destroys and recreates all of its VFs, so it -requires no vGPU assignments on that GPU): +requires no vGPU assignments on that GPU). The DCGM quiesce is not optional: +with `nv-hostengine`/`dcgm-exporter` holding the GPUs open, `sriov-manage -d` +fails with `Cannot obtain unbindLock` on first contact. ```bash +# 1. Quiesce the services holding the GPU (required for the unbind lock). +systemctl stop nvidia-dcgm-exporter nvidia-dcgm + +# 2. Cycle SR-IOV on the parent GPU. /usr/lib/nvidia/sriov-manage -d /usr/lib/nvidia/sriov-manage -e + +# 3. Restart the quiesced services. +systemctl start nvidia-dcgm nvidia-dcgm-exporter ``` After the cycle, boot a verification instance on the recovered VF and confirm -its guest reaches the driver (no sentinel in its app log), then clear the -quarantine by removing the VF's entry from `/gpu/vf-health.json` -and restarting hypeman. +its guest reaches the driver (no `HYPEMAN-GPU-INIT-FAILED` in its app log), +then clear the quarantine by removing the VF's entry from +`/gpu/vf-health.json` and restarting hypeman immediately — the +running process keeps the quarantine in memory, and a conviction landing +before the restart re-persists it over your edit. Do not unbind/rebind the VF from the nvidia driver — it breaks the nvidia-vgpu-vfio core-device registration (`vfio_pci_core_device not found`) and the VF stops accepting assignments entirely until the SR-IOV cycle. -Services holding the GPU (DCGM, persistenced) must be stopped for the cycle -to obtain the unbind lock. ### vGPU assignment fails diff --git a/lib/devices/vendor_vfio_linux_test.go b/lib/devices/vendor_vfio_linux_test.go index 490348502..dae633ed7 100644 --- a/lib/devices/vendor_vfio_linux_test.go +++ b/lib/devices/vendor_vfio_linux_test.go @@ -574,7 +574,7 @@ func assertFileValue(t *testing.T, path, expected string) { func TestVendorVFIOSkipsQuarantinedVF(t *testing.T) { resetVFHealthStore(t) - _, err := QuarantineVF(VFQuarantine{VFAddress: "0000:82:00.4"}) + _, _, err := QuarantineVF(VFQuarantine{VFAddress: "0000:82:00.4"}) require.NoError(t, err) sysfs := newTestVendorVFIOSysfs(t) @@ -589,7 +589,7 @@ func TestVendorVFIOSkipsQuarantinedVF(t *testing.T) { func TestVendorVFIONoVFWhenAllQuarantined(t *testing.T) { resetVFHealthStore(t) - _, err := QuarantineVF(VFQuarantine{VFAddress: "0000:82:00.4"}) + _, _, err := QuarantineVF(VFQuarantine{VFAddress: "0000:82:00.4"}) require.NoError(t, err) sysfs := newTestVendorVFIOSysfs(t) @@ -601,7 +601,7 @@ func TestVendorVFIONoVFWhenAllQuarantined(t *testing.T) { func TestVendorVFIOCardBiasAvoidsGPUWithQuarantinedVF(t *testing.T) { resetVFHealthStore(t) - _, err := QuarantineVF(VFQuarantine{VFAddress: "0000:82:00.4"}) + _, _, err := QuarantineVF(VFQuarantine{VFAddress: "0000:82:00.4"}) require.NoError(t, err) // GPU 82 sorts first by address and has a free healthy VF, but its @@ -630,7 +630,7 @@ func TestVendorVFIOSelectUsesTiebreakAmongFreeVFs(t *testing.T) { func TestVendorVFIOListProfilesExcludesQuarantinedFromAvailability(t *testing.T) { resetVFHealthStore(t) - _, err := QuarantineVF(VFQuarantine{VFAddress: "0000:82:00.4"}) + _, _, err := QuarantineVF(VFQuarantine{VFAddress: "0000:82:00.4"}) require.NoError(t, err) sysfs := newTestVendorVFIOSysfs(t) diff --git a/lib/devices/vf_health.go b/lib/devices/vf_health.go index 1560ab0eb..e6e8e7fcb 100644 --- a/lib/devices/vf_health.go +++ b/lib/devices/vf_health.go @@ -32,6 +32,11 @@ type vfHealthStore struct { mu sync.Mutex path string records map[string]VFHealthRecord + // loadErr remembers a failed load of an existing state file. While set, + // mutations are refused: persisting the empty in-memory store would + // permanently clobber every previously persisted quarantine. Mutating + // calls retry the load first so a transient read error self-heals. + loadErr error } var vfHealth = &vfHealthStore{records: make(map[string]VFHealthRecord)} @@ -42,36 +47,62 @@ func initVFHealthStore(path string) error { vfHealth.mu.Lock() defer vfHealth.mu.Unlock() vfHealth.path = path - vfHealth.records = make(map[string]VFHealthRecord) + return vfHealth.loadLocked() +} + +func (s *vfHealthStore) loadLocked() error { + s.records = make(map[string]VFHealthRecord) + s.loadErr = nil - data, err := os.ReadFile(path) + data, err := os.ReadFile(s.path) if err != nil { if os.IsNotExist(err) { return nil } - return fmt.Errorf("read VF health state: %w", err) + s.loadErr = fmt.Errorf("read VF health state: %w", err) + return s.loadErr } var records []VFHealthRecord if err := json.Unmarshal(data, &records); err != nil { - return fmt.Errorf("unmarshal VF health state: %w", err) + s.loadErr = fmt.Errorf("unmarshal VF health state: %w", err) + return s.loadErr } for _, record := range records { - vfHealth.records[record.VFAddress] = record + s.records[record.VFAddress] = record } return nil } +// ensureLoadedLocked retries a previously failed load. Mutations must not +// proceed on a store that failed to load its existing state file. +func (s *vfHealthStore) ensureLoadedLocked() error { + if s.loadErr == nil { + return nil + } + return s.loadLocked() +} + // QuarantineVF records a wedge conviction for a VF and persists it. It takes // the vGPU placement lock so a convicted VF is never concurrently selected -// for a new assignment. Repeat convictions on the same VF increment its -// wedge count and keep the original quarantine time. -func QuarantineVF(q VFQuarantine) (VFHealthRecord, error) { +// for a new assignment. A conviction of an already-quarantined VF returns +// the existing record unchanged with existed=true — one wedge produces one +// record no matter how many victim boots report it. +func QuarantineVF(q VFQuarantine) (VFHealthRecord, bool, error) { var record VFHealthRecord + var existed bool var err error withVGPUPlacementLock(func() { - record, err = vfHealth.quarantine(q) + record, existed, err = vfHealth.quarantine(q) }) - return record, err + return record, existed, err +} + +// IsVFQuarantined reports whether a quarantine record exists for the VF. +func IsVFQuarantined(vfAddress string) bool { + vfHealth.mu.Lock() + defer vfHealth.mu.Unlock() + _, ok := vfHealth.records[vfAddress] + return ok } // ClearVFQuarantine removes a VF's quarantine record, returning whether one @@ -98,29 +129,35 @@ func QuarantinedVFs() []VFHealthRecord { return records } -func (s *vfHealthStore) quarantine(q VFQuarantine) (VFHealthRecord, error) { +func (s *vfHealthStore) quarantine(q VFQuarantine) (VFHealthRecord, bool, error) { s.mu.Lock() defer s.mu.Unlock() - record, ok := s.records[q.VFAddress] - if !ok { - record = VFHealthRecord{ - VFAddress: q.VFAddress, - QuarantinedAt: time.Now().UTC(), - } + if err := s.ensureLoadedLocked(); err != nil { + return VFHealthRecord{}, false, err + } + if record, ok := s.records[q.VFAddress]; ok { + return record, true, nil + } + record := VFHealthRecord{ + VFAddress: q.VFAddress, + InstanceID: q.InstanceID, + SentinelLine: q.SentinelLine, + WedgeCount: 1, + QuarantinedAt: time.Now().UTC(), } - record.WedgeCount++ - record.InstanceID = q.InstanceID - record.SentinelLine = q.SentinelLine s.records[q.VFAddress] = record if err := s.persistLocked(); err != nil { - return record, err + return record, false, err } - return record, nil + return record, false, nil } func (s *vfHealthStore) clear(vfAddress string) (bool, error) { s.mu.Lock() defer s.mu.Unlock() + if err := s.ensureLoadedLocked(); err != nil { + return false, err + } if _, ok := s.records[vfAddress]; !ok { return false, nil } diff --git a/lib/devices/vf_health_test.go b/lib/devices/vf_health_test.go index c5a11b17a..9e759906a 100644 --- a/lib/devices/vf_health_test.go +++ b/lib/devices/vf_health_test.go @@ -1,6 +1,7 @@ package devices import ( + "os" "path/filepath" "testing" @@ -19,6 +20,7 @@ func resetVFHealthStore(t *testing.T) string { defer vfHealth.mu.Unlock() vfHealth.path = "" vfHealth.records = make(map[string]VFHealthRecord) + vfHealth.loadErr = nil }) return path } @@ -26,42 +28,75 @@ func resetVFHealthStore(t *testing.T) string { func TestQuarantineVFPersistsAcrossReload(t *testing.T) { path := resetVFHealthStore(t) - record, err := QuarantineVF(VFQuarantine{ + record, existed, err := QuarantineVF(VFQuarantine{ VFAddress: "0000:e3:00.4", InstanceID: "instance-1", - SentinelLine: "NVRM: GPU 0000:e3:00.4: RmInitAdapter failed! (0x22:0x65:884)", + SentinelLine: "HYPEMAN-GPU-INIT-FAILED ts=2026-08-20T15:04:05Z nvrm=\"NVRM: GPU 0000:e3:00.4: RmInitAdapter failed! (0x22:0x65:884)\"", }) require.NoError(t, err) + assert.False(t, existed) assert.Equal(t, 1, record.WedgeCount) assert.False(t, record.QuarantinedAt.IsZero()) + assert.True(t, IsVFQuarantined("0000:e3:00.4")) - // A repeat conviction increments the count and keeps the original time. - again, err := QuarantineVF(VFQuarantine{VFAddress: "0000:e3:00.4", InstanceID: "instance-2"}) + // A repeat conviction (another victim boot, or a rescan after restart) + // returns the original record unchanged: one wedge, one record. + again, existed, err := QuarantineVF(VFQuarantine{VFAddress: "0000:e3:00.4", InstanceID: "instance-2"}) require.NoError(t, err) - assert.Equal(t, 2, again.WedgeCount) - assert.Equal(t, record.QuarantinedAt, again.QuarantinedAt) + assert.True(t, existed) + assert.Equal(t, record, again) // Reload from disk, as a hypeman restart would. require.NoError(t, initVFHealthStore(path)) records := QuarantinedVFs() require.Len(t, records, 1) assert.Equal(t, "0000:e3:00.4", records[0].VFAddress) - assert.Equal(t, 2, records[0].WedgeCount) - assert.Equal(t, "instance-2", records[0].InstanceID) + assert.Equal(t, 1, records[0].WedgeCount) + assert.Equal(t, "instance-1", records[0].InstanceID) } func TestClearVFQuarantine(t *testing.T) { resetVFHealthStore(t) - _, err := QuarantineVF(VFQuarantine{VFAddress: "0000:e3:00.4"}) + _, _, err := QuarantineVF(VFQuarantine{VFAddress: "0000:e3:00.4"}) require.NoError(t, err) cleared, err := ClearVFQuarantine("0000:e3:00.4") require.NoError(t, err) assert.True(t, cleared) assert.Empty(t, QuarantinedVFs()) + assert.False(t, IsVFQuarantined("0000:e3:00.4")) cleared, err = ClearVFQuarantine("0000:e3:00.4") require.NoError(t, err) assert.False(t, cleared) } + +func TestQuarantineVFRefusesToClobberUnloadedState(t *testing.T) { + path := resetVFHealthStore(t) + + _, _, err := QuarantineVF(VFQuarantine{VFAddress: "0000:e3:00.4"}) + require.NoError(t, err) + + // Corrupt the state file and reload, as a hypeman restart over a bad + // file would. The load fails and mutations must not persist the empty + // in-memory store over the previous quarantines. + require.NoError(t, os.WriteFile(path, []byte("not json"), 0644)) + require.Error(t, initVFHealthStore(path)) + + _, _, err = QuarantineVF(VFQuarantine{VFAddress: "0000:e3:00.5"}) + require.Error(t, err) + data, err := os.ReadFile(path) + require.NoError(t, err) + assert.Equal(t, "not json", string(data), "a failed load must not be overwritten by later convictions") + + // Once the file is readable again the next conviction self-heals: it + // reloads the persisted records and appends to them. + restored := `[{"vf_address":"0000:e3:00.4","wedge_count":1,"quarantined_at":"2026-08-20T00:00:00Z"}]` + require.NoError(t, os.WriteFile(path, []byte(restored), 0644)) + _, existed, err := QuarantineVF(VFQuarantine{VFAddress: "0000:e3:00.5"}) + require.NoError(t, err) + assert.False(t, existed) + assert.Len(t, QuarantinedVFs(), 2) + assert.True(t, IsVFQuarantined("0000:e3:00.4"), "reload must recover the previously persisted quarantine") +} diff --git a/lib/instances/logs.go b/lib/instances/logs.go index f10e8537d..208f21cc3 100644 --- a/lib/instances/logs.go +++ b/lib/instances/logs.go @@ -35,6 +35,7 @@ var ErrLogNotFound = fmt.Errorf("log file not found") var appLogNoiseMarkers = []string{ "HYPEMAN-PROGRAM-START", "HYPEMAN-AGENT-READY", + "HYPEMAN-GPU-INIT-FAILED", "HYPEMAN-HEADERS-START", "HYPEMAN-HEADERS-READY", "HYPEMAN-HEADERS-FAILED", diff --git a/lib/instances/vgpu_sentinel.go b/lib/instances/vgpu_sentinel.go index 1340d582f..3b45997c2 100644 --- a/lib/instances/vgpu_sentinel.go +++ b/lib/instances/vgpu_sentinel.go @@ -26,19 +26,30 @@ const ( // the host harder than the failure itself, so auto-conviction pauses. vgpuSentinelBrakeWindow = 15 * time.Minute vgpuSentinelBrakeLimit = 3 + + // vgpuSentinelMaxLineBytes bounds how much of an unterminated trailing + // line a scan will buffer and re-read. Marker lines are short; anything + // this long is console spam and is skipped once complete reading it + // becomes unreasonable. + vgpuSentinelMaxLineBytes = 64 * 1024 ) -// vgpuSentinelPattern matches the guest NVIDIA driver's init-failure line as -// it arrives on the serial console. The full "NVRM: ... RmInitAdapter -// failed!" shape is required: the bare token also appears in echoed exec -// command lines, and the trailing (stage:status:line) tuple is -// driver-build-specific, so neither is safe to match. -var vgpuSentinelPattern = regexp.MustCompile(`NVRM: .*RmInitAdapter failed!`) +// vgpuSentinelPattern matches the guest agent's HYPEMAN-GPU-INIT-FAILED +// report as it arrives on the serial console (the agent observes the NVIDIA +// driver's RmInitAdapter failure in the guest kernel log and emits this +// marker — the same guest-to-host channel as the other HYPEMAN-* markers). +// The full marker shape including the nvrm field is required: a bare token +// could appear in echoed exec command lines. +var vgpuSentinelPattern = regexp.MustCompile(`HYPEMAN-GPU-INIT-FAILED ts=\S+ nvrm="`) type vgpuSentinelTarget struct { instanceID string vfAddress string appLogPath string + // assignedAt identifies one assignment epoch: it changes whenever the + // instance acquires a vGPU, so a tail from a previous boot or a previous + // VF is never carried into the next one. + assignedAt string } type vgpuSentinelStore interface { @@ -46,8 +57,10 @@ type vgpuSentinelStore interface { } type vgpuSentinelTail struct { - offset int64 - done bool + vfAddress string + assignedAt string + offset int64 + done bool } // VGPUSentinelController scans the serial console log of vendor VFIO vGPU @@ -56,14 +69,16 @@ type vgpuSentinelTail struct { // failing — so a match quarantines the VF, removing it from placement until // an operator cycles the parent GPU. type VGPUSentinelController struct { - store vgpuSentinelStore - log *slog.Logger - interval time.Duration - now func() time.Time - quarantine func(devices.VFQuarantine) (devices.VFHealthRecord, error) - convictions metric.Int64Counter - tails map[string]*vgpuSentinelTail - recent []time.Time + store vgpuSentinelStore + log *slog.Logger + interval time.Duration + now func() time.Time + quarantine func(devices.VFQuarantine) (devices.VFHealthRecord, bool, error) + isQuarantined func(string) bool + hostFramework func() devices.VGPUFramework + convictions metric.Int64Counter + tails map[string]*vgpuSentinelTail + recent []time.Time } func NewVGPUSentinelController(manager Manager, meter metric.Meter, log *slog.Logger) (*VGPUSentinelController, error) { @@ -95,17 +110,35 @@ func NewVGPUSentinelController(manager Manager, meter metric.Meter, log *slog.Lo } return &VGPUSentinelController{ - store: store, - log: log.With("controller", "vgpu_sentinel"), - interval: vgpuSentinelScanInterval, - now: time.Now, - quarantine: devices.QuarantineVF, + store: store, + log: log.With("controller", "vgpu_sentinel"), + interval: vgpuSentinelScanInterval, + now: time.Now, + quarantine: devices.QuarantineVF, + isQuarantined: devices.IsVFQuarantined, + hostFramework: func() devices.VGPUFramework { + framework, _, err := devices.DiscoverVGPU() + if err != nil { + // Fail open: a transient discovery error must not disable + // detection on a vendor VFIO host. + return devices.VGPUFrameworkVendorVFIO + } + return framework + }, convictions: convictions, tails: make(map[string]*vgpuSentinelTail), }, nil } func (c *VGPUSentinelController) Run(ctx context.Context) error { + // Wedges only exist on vendor VFIO hosts; scanning instance metadata + // every interval on mdev and CPU-only hosts would be pure overhead. The + // framework is fixed for the lifetime of the process (SR-IOV provisioning + // precedes hypeman startup), so this is a one-time gate. + if framework := c.hostFramework(); framework != devices.VGPUFrameworkVendorVFIO { + c.log.Info("vGPU sentinel controller idle: host has no vendor VFIO framework", "framework", string(framework)) + return nil + } c.log.Info("vGPU sentinel controller started") ticker := time.NewTicker(c.interval) defer ticker.Stop() @@ -139,8 +172,11 @@ func (c *VGPUSentinelController) scanOnce(ctx context.Context) { func (c *VGPUSentinelController) scanTarget(ctx context.Context, target vgpuSentinelTarget) { tail := c.tails[target.instanceID] - if tail == nil { - tail = &vgpuSentinelTail{} + if tail == nil || tail.vfAddress != target.vfAddress || tail.assignedAt != target.assignedAt { + // A new assignment (stop/start, possibly on a different VF) writes a + // fresh log; a tail finished for the previous assignment must not + // suppress scanning the new one. + tail = &vgpuSentinelTail{vfAddress: target.vfAddress, assignedAt: target.assignedAt} c.tails[target.instanceID] = tail } if tail.done { @@ -159,9 +195,20 @@ func (c *VGPUSentinelController) scanTarget(ctx context.Context, target vgpuSent } // convict quarantines the target's VF, subject to the conviction brake. -// It reports whether scanning for this instance is finished; a failed -// quarantine leaves the tail open so the recurring sentinel retries it. +// It reports whether scanning for this instance is finished; a failed or +// brake-suppressed quarantine leaves the tail open so the recurring report +// retries it — the brake pauses auto-conviction, it must not permanently +// drop a conviction. func (c *VGPUSentinelController) convict(ctx context.Context, target vgpuSentinelTarget, line string) bool { + if c.isQuarantined(target.vfAddress) { + // Already out of placement — typically a rescan of a standing + // victim's log after a controller restart. Not a new wedge: no + // metric, no brake accounting. + c.log.InfoContext(ctx, "vGPU sentinel matched an already-quarantined VF", + "vf", target.vfAddress, "instance_id", target.instanceID) + return true + } + now := c.now() recent := c.recent[:0] for _, t := range c.recent { @@ -180,10 +227,10 @@ func (c *VGPUSentinelController) convict(ctx context.Context, target vgpuSentine "window", vgpuSentinelBrakeWindow.String(), ) c.convictions.Add(ctx, 1, metric.WithAttributes(attribute.String("result", "suppressed"))) - return true + return false } - record, err := c.quarantine(devices.VFQuarantine{ + record, existed, err := c.quarantine(devices.VFQuarantine{ VFAddress: target.vfAddress, InstanceID: target.instanceID, SentinelLine: line, @@ -193,6 +240,10 @@ func (c *VGPUSentinelController) convict(ctx context.Context, target vgpuSentine "vf", target.vfAddress, "instance_id", target.instanceID, "error", err) return false } + if existed { + // Lost a conviction race; the VF is already quarantined. + return true + } c.recent = append(c.recent, now) c.log.ErrorContext(ctx, "quarantined wedged vGPU VF", "vf", target.vfAddress, @@ -234,6 +285,11 @@ func scanForSentinel(path string, offset *int64) (string, bool, error) { line, err := reader.ReadString('\n') if err != nil { if errors.Is(err, io.EOF) { + // An oversized unterminated line would otherwise be buffered + // again on every scan; skip it once it exceeds the bound. + if len(line) > vgpuSentinelMaxLineBytes { + *offset += int64(len(line)) + } return "", false, nil } return "", false, err @@ -264,10 +320,15 @@ func (m *manager) listVGPUSentinelTargets(ctx context.Context) ([]vgpuSentinelTa if meta.GPUFramework != devices.VGPUFrameworkVendorVFIO || meta.GPUDevicePath == "" { continue } + assignedAt := "" + if meta.GPUAssignedAt != nil { + assignedAt = meta.GPUAssignedAt.UTC().Format(time.RFC3339Nano) + } targets = append(targets, vgpuSentinelTarget{ instanceID: id, vfAddress: filepath.Base(meta.GPUDevicePath), appLogPath: m.paths.InstanceAppLog(id), + assignedAt: assignedAt, }) } return targets, nil diff --git a/lib/instances/vgpu_sentinel_test.go b/lib/instances/vgpu_sentinel_test.go index b1b2f7140..d552345c9 100644 --- a/lib/instances/vgpu_sentinel_test.go +++ b/lib/instances/vgpu_sentinel_test.go @@ -15,18 +15,20 @@ import ( "go.opentelemetry.io/otel/metric/noop" ) -const testSentinelLine = "[ 27.031415] NVRM: GPU 0000:e3:00.4: RmInitAdapter failed! (0x22:0x65:884)\n" +const testSentinelLine = "2026/08/20 15:04:05 [guest-agent] HYPEMAN-GPU-INIT-FAILED ts=2026-08-20T15:04:05.123456789Z nvrm=\"NVRM: GPU 0000:e3:00.4: RmInitAdapter failed! (0x22:0x65:884)\"\n" func TestVGPUSentinelPattern(t *testing.T) { t.Parallel() assert.True(t, vgpuSentinelPattern.MatchString(testSentinelLine)) - // Tuple values are driver-build-specific; the match must not depend on them. - assert.True(t, vgpuSentinelPattern.MatchString("[ 47.2] NVRM: GPU 0000:e3:00.4: RmInitAdapter failed! (0x26:0xffff:1482)")) - // Echoed exec command lines carry the bare token on the same console. - assert.False(t, vgpuSentinelPattern.MatchString("$ dmesg | grep -c RmInitAdapter")) - assert.False(t, vgpuSentinelPattern.MatchString("[ 7.1] NVRM: loading NVIDIA UNIX Open Kernel Module for x86_64")) + // The raw guest kernel line is not the conviction signal: the guest + // agent observes it in the guest and reports the marker instead. + assert.False(t, vgpuSentinelPattern.MatchString("[ 27.031415] NVRM: GPU 0000:e3:00.4: RmInitAdapter failed! (0x22:0x65:884)")) + + // Echoed exec command lines can carry the bare token on the same console. + assert.False(t, vgpuSentinelPattern.MatchString("$ dmesg | grep -c HYPEMAN-GPU-INIT-FAILED")) + assert.False(t, vgpuSentinelPattern.MatchString("2026/08/20 15:04:05 [guest-agent] HYPEMAN-AGENT-READY ts=2026-08-20T15:04:05Z")) } type fakeSentinelStore struct { @@ -47,12 +49,13 @@ func newTestSentinelController(t *testing.T, store vgpuSentinelStore) (*VGPUSent log: slog.New(slog.DiscardHandler), interval: time.Hour, now: time.Now, - quarantine: func(q devices.VFQuarantine) (devices.VFHealthRecord, error) { + quarantine: func(q devices.VFQuarantine) (devices.VFHealthRecord, bool, error) { quarantined = append(quarantined, q) - return devices.VFHealthRecord{VFAddress: q.VFAddress, WedgeCount: 1}, nil + return devices.VFHealthRecord{VFAddress: q.VFAddress, WedgeCount: 1}, false, nil }, - convictions: counter, - tails: make(map[string]*vgpuSentinelTail), + isQuarantined: func(string) bool { return false }, + convictions: counter, + tails: make(map[string]*vgpuSentinelTail), } return c, &quarantined } @@ -78,12 +81,12 @@ func TestVGPUSentinelControllerConvictsOnce(t *testing.T) { f, err := os.OpenFile(logPath, os.O_APPEND|os.O_WRONLY, 0644) require.NoError(t, err) // A partial line without its newline must not convict yet. - _, err = f.WriteString("[ 27.03] NVRM: GPU 0000:e3:00.4: RmInitAdapter failed") + _, err = f.WriteString("2026/08/20 15:04:05 [guest-agent] HYPEMAN-GPU-INIT-FAILED ts=2026-08-20T15:04:05Z nvrm=\"NVRM: GPU 0000:e3:00.4: RmInitAdapter fail") require.NoError(t, err) c.scanOnce(ctx) assert.Empty(t, *quarantined) - _, err = f.WriteString("! (0x22:0x65:884)\n") + _, err = f.WriteString("ed! (0x22:0x65:884)\"\n") require.NoError(t, err) require.NoError(t, f.Close()) c.scanOnce(ctx) @@ -92,7 +95,8 @@ func TestVGPUSentinelControllerConvictsOnce(t *testing.T) { assert.Equal(t, "instance-1", (*quarantined)[0].InstanceID) assert.Contains(t, (*quarantined)[0].SentinelLine, "RmInitAdapter failed!") - // The sentinel recurs every ~20s; a convicted instance is not re-convicted. + // The guest agent re-emits the marker while the failure persists; a + // convicted instance is not re-convicted. appendSentinelLine(t, logPath) c.scanOnce(ctx) assert.Len(t, *quarantined, 1) @@ -111,8 +115,8 @@ func TestVGPUSentinelControllerRetriesFailedQuarantine(t *testing.T) { c, quarantined := newTestSentinelController(t, store) quarantineErr := errors.New("persist failed") realQuarantine := c.quarantine - c.quarantine = func(devices.VFQuarantine) (devices.VFHealthRecord, error) { - return devices.VFHealthRecord{}, quarantineErr + c.quarantine = func(devices.VFQuarantine) (devices.VFHealthRecord, bool, error) { + return devices.VFHealthRecord{}, false, quarantineErr } ctx := context.Background() @@ -120,7 +124,7 @@ func TestVGPUSentinelControllerRetriesFailedQuarantine(t *testing.T) { assert.Empty(t, *quarantined) assert.False(t, c.tails["instance-1"].done) - // The next recurrence of the sentinel retries the quarantine. + // The next recurrence of the marker retries the quarantine. c.quarantine = realQuarantine appendSentinelLine(t, logPath) c.scanOnce(ctx) @@ -128,7 +132,7 @@ func TestVGPUSentinelControllerRetriesFailedQuarantine(t *testing.T) { assert.True(t, c.tails["instance-1"].done) } -func TestVGPUSentinelControllerBrakeSuppressesConvictionBursts(t *testing.T) { +func TestVGPUSentinelControllerBrakePausesConvictionBursts(t *testing.T) { t.Parallel() dir := t.TempDir() @@ -143,14 +147,88 @@ func TestVGPUSentinelControllerBrakeSuppressesConvictionBursts(t *testing.T) { }) } c, quarantined := newTestSentinelController(t, &fakeSentinelStore{targets: targets}) + base := time.Now() + c.now = func() time.Time { return base } c.scanOnce(context.Background()) // The burst converts to convictions up to the limit; the rest are - // suppressed rather than quarantining the whole host. + // suppressed rather than quarantining the whole host, and their tails + // stay open — the brake pauses conviction, it does not drop it. assert.Len(t, *quarantined, vgpuSentinelBrakeLimit) + suppressed := 0 for _, target := range targets { - assert.True(t, c.tails[target.instanceID].done) + if !c.tails[target.instanceID].done { + suppressed++ + // The guest agent re-emits the marker while the failure persists. + appendSentinelLine(t, target.appLogPath) + } } + assert.Equal(t, 1, suppressed) + + // Within the window the suppression holds. + c.scanOnce(context.Background()) + assert.Len(t, *quarantined, vgpuSentinelBrakeLimit) + + // Once the window clears, the next re-emission convicts. + c.now = func() time.Time { return base.Add(vgpuSentinelBrakeWindow + time.Second) } + for _, target := range targets { + if !c.tails[target.instanceID].done { + appendSentinelLine(t, target.appLogPath) + } + } + c.scanOnce(context.Background()) + assert.Len(t, *quarantined, vgpuSentinelBrakeLimit+1) +} + +func TestVGPUSentinelControllerSkipsQuarantinedVFs(t *testing.T) { + t.Parallel() + + logPath := filepath.Join(t.TempDir(), "app.log") + require.NoError(t, os.WriteFile(logPath, []byte(testSentinelLine), 0644)) + store := &fakeSentinelStore{targets: []vgpuSentinelTarget{{ + instanceID: "instance-1", + vfAddress: "0000:e3:00.4", + appLogPath: logPath, + }}} + c, quarantined := newTestSentinelController(t, store) + c.isQuarantined = func(vf string) bool { return vf == "0000:e3:00.4" } + + // A rescan of a standing victim's log after a controller restart must + // not re-convict a persisted quarantine: no brake accounting, no metric, + // and the tail closes. + c.scanOnce(context.Background()) + assert.Empty(t, *quarantined) + assert.Empty(t, c.recent) + assert.True(t, c.tails["instance-1"].done) +} + +func TestVGPUSentinelControllerRescansNewAssignment(t *testing.T) { + t.Parallel() + + logPath := filepath.Join(t.TempDir(), "app.log") + require.NoError(t, os.WriteFile(logPath, []byte(testSentinelLine), 0644)) + store := &fakeSentinelStore{targets: []vgpuSentinelTarget{{ + instanceID: "instance-1", + vfAddress: "0000:e3:00.4", + appLogPath: logPath, + assignedAt: "2026-08-20T15:00:00Z", + }}} + c, quarantined := newTestSentinelController(t, store) + ctx := context.Background() + + c.scanOnce(ctx) + require.Len(t, *quarantined, 1) + require.True(t, c.tails["instance-1"].done) + + // A stop/start acquires a new assignment (possibly a different VF) and + // archives the log; the finished tail from the previous assignment must + // not suppress scanning the new boot. + require.NoError(t, os.WriteFile(logPath, []byte(testSentinelLine), 0644)) + store.targets[0].vfAddress = "0000:e3:00.5" + store.targets[0].assignedAt = "2026-08-20T16:00:00Z" + c.scanOnce(ctx) + require.Len(t, *quarantined, 2) + assert.Equal(t, "0000:e3:00.5", (*quarantined)[1].VFAddress) } func TestVGPUSentinelControllerDropsStaleTails(t *testing.T) { diff --git a/lib/system/guest_agent/gpu_watch.go b/lib/system/guest_agent/gpu_watch.go new file mode 100644 index 000000000..2e08df7af --- /dev/null +++ b/lib/system/guest_agent/gpu_watch.go @@ -0,0 +1,106 @@ +package main + +import ( + "bufio" + "io" + "log" + "os" + "path/filepath" + "strings" + "time" +) + +const ( + // gpuInitFailedSentinelPrefix is the guest-to-host report of a failed + // NVIDIA driver init. It rides the same channel as the other HYPEMAN-* + // markers: agent log output reaches the serial console, which the host + // persists per instance and scans. The host quarantines the vGPU VF on + // this marker, so it is only ever emitted for a kernel-log line the + // driver itself produced. + gpuInitFailedSentinelPrefix = "HYPEMAN-GPU-INIT-FAILED" + + kmsgPath = "/dev/kmsg" + nvidiaPCIVendorID = "0x10de" + + // gpuReportThrottle bounds marker emission. The driver retries init + // every ~20s on a wedged VF, and reopening /dev/kmsg replays the ring + // buffer, so without a floor a long-lived broken guest would spam the + // console. + gpuReportThrottle = 30 * time.Second + + kmsgReopenDelay = 5 * time.Second +) + +// hasNVIDIADevice reports whether any PCI function belongs to NVIDIA. A vGPU +// guest always enumerates its VF, even on a wedged slot, so a guest with no +// NVIDIA function never needs the watcher. +func hasNVIDIADevice() bool { + vendors, _ := filepath.Glob("/sys/bus/pci/devices/*/vendor") + for _, path := range vendors { + data, err := os.ReadFile(path) + if err != nil { + continue + } + if strings.TrimSpace(string(data)) == nvidiaPCIVendorID { + return true + } + } + return false +} + +// watchGPUInitFailure tails the guest kernel log for the NVIDIA driver's +// RmInitAdapter failure and reports each occurrence with a +// HYPEMAN-GPU-INIT-FAILED marker. Opening /dev/kmsg replays the ring buffer +// from the start, so failures that predate the agent are reported too. Reads +// error with EPIPE when the ring overwrites the read position; reopen and +// resume. +func watchGPUInitFailure() { + var lastReport time.Time + for { + f, err := os.Open(kmsgPath) + if err != nil { + log.Printf("[guest-agent] cannot open %s for GPU init watch: %v", kmsgPath, err) + return + } + scanKmsg(f, func(msg string) { + if time.Since(lastReport) < gpuReportThrottle { + return + } + lastReport = time.Now() + log.Printf("[guest-agent] %s ts=%s nvrm=%q", gpuInitFailedSentinelPrefix, time.Now().UTC().Format(time.RFC3339Nano), msg) + }) + _ = f.Close() + time.Sleep(kmsgReopenDelay) + } +} + +// scanKmsg reads /dev/kmsg records from r and calls report for each GPU +// init-failure message. +func scanKmsg(r io.Reader, report func(msg string)) { + reader := bufio.NewReader(r) + for { + record, err := reader.ReadString('\n') + if msg, ok := gpuInitFailureMessage(record); ok { + report(msg) + } + if err != nil { + return + } + } +} + +// gpuInitFailureMessage extracts the message from a /dev/kmsg record +// (";") and reports whether it is the NVIDIA driver's +// init-failure line. The full line shape is required — the trailing +// (stage:status:line) tuple is driver-build-specific and unsafe to match. +func gpuInitFailureMessage(record string) (string, bool) { + _, msg, found := strings.Cut(record, ";") + if !found { + return "", false + } + msg = strings.TrimSpace(msg) + if !strings.HasPrefix(msg, "NVRM:") || !strings.Contains(msg, "RmInitAdapter failed!") { + return "", false + } + return msg, true +} diff --git a/lib/system/guest_agent/gpu_watch_test.go b/lib/system/guest_agent/gpu_watch_test.go new file mode 100644 index 000000000..1481f995e --- /dev/null +++ b/lib/system/guest_agent/gpu_watch_test.go @@ -0,0 +1,41 @@ +package main + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestGPUInitFailureMessage(t *testing.T) { + msg, ok := gpuInitFailureMessage("3,1042,8462102,-;NVRM: GPU 0000:00:03.0: RmInitAdapter failed! (0x22:0x65:884)\n") + assert.True(t, ok) + assert.Equal(t, "NVRM: GPU 0000:00:03.0: RmInitAdapter failed! (0x22:0x65:884)", msg) + + // Tuple values are driver-build-specific; the match must not depend on them. + _, ok = gpuInitFailureMessage("3,1042,8462102,-;NVRM: GPU 0000:00:03.0: RmInitAdapter failed! (0x26:0xffff:1482)\n") + assert.True(t, ok) + + for _, record := range []string{ + "6,1041,8462100,-;NVRM: loading NVIDIA UNIX Open Kernel Module for x86_64\n", + "6,1043,8462110,-;nvidia-gridd: RmInitAdapter failed mentioned in userspace\n", + "no separator RmInitAdapter failed!\n", + " continuation line of a multi-line record\n", + } { + _, ok := gpuInitFailureMessage(record) + assert.False(t, ok, "record %q must not match", record) + } +} + +func TestScanKmsgReportsEachFailureRecord(t *testing.T) { + records := strings.Join([]string{ + "6,1,100,-;booting", + "3,2,200,-;NVRM: GPU 0000:00:03.0: RmInitAdapter failed! (0x22:0x65:884)", + "6,3,300,-;unrelated", + "3,4,400,-;NVRM: GPU 0000:00:03.0: RmInitAdapter failed! (0x22:0x65:884)", + }, "\n") + "\n" + + var got []string + scanKmsg(strings.NewReader(records), func(msg string) { got = append(got, msg) }) + assert.Len(t, got, 2) +} diff --git a/lib/system/guest_agent/main.go b/lib/system/guest_agent/main.go index 84fd2a5da..f726dd286 100644 --- a/lib/system/guest_agent/main.go +++ b/lib/system/guest_agent/main.go @@ -54,6 +54,10 @@ func main() { startClockKeeper() + if hasNVIDIADevice() { + go watchGPUInitFailure() + } + // Create gRPC server grpcServer := grpc.NewServer() pb.RegisterGuestServiceServer(grpcServer, &guestServer{}) From 38249dcff22d5fea3854fce2115660a62e21afb1 Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Thu, 20 Aug 2026 20:15:38 +0000 Subject: [PATCH 03/11] Close sentinel replay, fail-open, and persist-retry holes Review fixes on the quarantine layer: - start now archives the previous boot's serial log before persisting the new vGPU assignment. The sentinel keys its tail on the assignment epoch, so the old ordering let a scan replay the previous boot's wedge report against the freshly assigned VF. A failed archive is fatal for GPU instances instead of a warning. - placement and profile availability refuse to run when the VF health state file exists but cannot be loaded, instead of treating the empty in-memory set as healthy and returning every quarantined VF to rotation. The load is retried on each attempt, so a repaired file self-heals. - a conviction whose persist fails is rolled back from memory: keeping it made the next report look like a repeat conviction, ending retries with nothing on disk. - the marker pattern requires the full shape through the quoted NVRM payload; a truncated or payload-less marker echoed by an exec command no longer matches. - unreadable instance metadata in the sentinel target listing logs a warning instead of silently shrinking detection coverage. - GPU.md recovery runbook unwound a circularity: placement excludes quarantined VFs and there is no VF-pin API, so the entry is cleared before the verification boot; the sentinel re-quarantines automatically if the cycle did not cure the VF. --- lib/devices/GPU.md | 10 +++++--- lib/devices/manager.go | 2 +- lib/devices/vendor_vfio_linux.go | 10 ++++++-- lib/devices/vf_health.go | 34 +++++++++++++++--------- lib/devices/vf_health_test.go | 40 +++++++++++++++++++++++++++++ lib/instances/start.go | 17 +++++++++--- lib/instances/vgpu_sentinel.go | 12 ++++++--- lib/instances/vgpu_sentinel_test.go | 5 +++- 8 files changed, 103 insertions(+), 27 deletions(-) diff --git a/lib/devices/GPU.md b/lib/devices/GPU.md index 1e4d38c47..8d78b7057 100644 --- a/lib/devices/GPU.md +++ b/lib/devices/GPU.md @@ -342,12 +342,14 @@ systemctl stop nvidia-dcgm-exporter nvidia-dcgm systemctl start nvidia-dcgm nvidia-dcgm-exporter ``` -After the cycle, boot a verification instance on the recovered VF and confirm -its guest reaches the driver (no `HYPEMAN-GPU-INIT-FAILED` in its app log), -then clear the quarantine by removing the VF's entry from +After the cycle, clear the quarantine by removing the VF's entry from `/gpu/vf-health.json` and restarting hypeman immediately — the running process keeps the quarantine in memory, and a conviction landing -before the restart re-persists it over your edit. +before the restart re-persists it over your edit. Then boot a GPU instance as +verification: placement excludes quarantined VFs, so the recovered VF cannot +be targeted while its entry exists, and there is no VF-pin API — clearing +first is safe because the sentinel automatically re-quarantines the VF if the +cycle did not cure it (every cycle in hardware validation did). Do not unbind/rebind the VF from the nvidia driver — it breaks the nvidia-vgpu-vfio core-device registration (`vfio_pci_core_device not found`) diff --git a/lib/devices/manager.go b/lib/devices/manager.go index 50e615e76..7d44b4e92 100644 --- a/lib/devices/manager.go +++ b/lib/devices/manager.go @@ -87,7 +87,7 @@ type manager struct { // Use SetLivenessChecker after construction to enable accurate orphan detection. func NewManager(p *paths.Paths) Manager { if err := initVFHealthStore(p.VFHealthState()); err != nil { - slog.Default().Error("failed to load VF health state; persisted quarantines are not in effect", "error", err) + 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, diff --git a/lib/devices/vendor_vfio_linux.go b/lib/devices/vendor_vfio_linux.go index 2201ecaf5..a263dd59f 100644 --- a/lib/devices/vendor_vfio_linux.go +++ b/lib/devices/vendor_vfio_linux.go @@ -112,7 +112,10 @@ func (s vendorVFIOSysfs) discoverVFs() ([]VirtualFunction, error) { // available_instances. This is a best-effort snapshot because creating on one // VF may revoke the type from siblings that share its GPU framebuffer. func (s vendorVFIOSysfs) listProfiles(vfs []VirtualFunction) ([]GPUProfile, error) { - quarantined := vfHealth.snapshotAddresses() + quarantined, err := vfHealth.checkedAddresses() + if err != nil { + return nil, err + } profilesByType := make(map[string]profileMetadata) creatableVFs := make(map[string]int) for _, vf := range vfs { @@ -316,7 +319,10 @@ func (s vendorVFIOSysfs) reconcile(ctx context.Context, protectedDevicePaths map } func (s vendorVFIOSysfs) selectLeastLoadedVF(vfs []VirtualFunction, profileType string) (string, error) { - quarantined := vfHealth.snapshotAddresses() + quarantined, err := vfHealth.checkedAddresses() + if err != nil { + return "", err + } usageByGPU := make(map[string]int) unknownUsageByGPU := make(map[string]bool) quarantinedByGPU := make(map[string]int) diff --git a/lib/devices/vf_health.go b/lib/devices/vf_health.go index e6e8e7fcb..b3e88d736 100644 --- a/lib/devices/vf_health.go +++ b/lib/devices/vf_health.go @@ -82,6 +82,23 @@ func (s *vfHealthStore) ensureLoadedLocked() error { return s.loadLocked() } +// checkedAddresses returns the quarantined VF addresses, retrying a failed +// load first. Placement must not run against an empty record set that only +// exists because the state file was unreadable: that would put every +// previously quarantined VF back into rotation. +func (s *vfHealthStore) checkedAddresses() (map[string]struct{}, error) { + s.mu.Lock() + defer s.mu.Unlock() + if err := s.ensureLoadedLocked(); err != nil { + return nil, fmt.Errorf("VF health state unavailable: %w", err) + } + addresses := make(map[string]struct{}, len(s.records)) + for address := range s.records { + addresses[address] = struct{}{} + } + return addresses, nil +} + // QuarantineVF records a wedge conviction for a VF and persists it. It takes // the vGPU placement lock so a convicted VF is never concurrently selected // for a new assignment. A conviction of an already-quarantined VF returns @@ -147,7 +164,11 @@ func (s *vfHealthStore) quarantine(q VFQuarantine) (VFHealthRecord, bool, error) } s.records[q.VFAddress] = record if err := s.persistLocked(); err != nil { - return record, false, err + // A quarantine is only real once it is on disk. Keeping the record in + // memory would make the next report look like a repeat conviction and + // end retries, leaving nothing persisted for the next restart. + delete(s.records, q.VFAddress) + return VFHealthRecord{}, false, err } return record, false, nil } @@ -165,17 +186,6 @@ func (s *vfHealthStore) clear(vfAddress string) (bool, error) { return true, s.persistLocked() } -// snapshotAddresses returns the set of quarantined VF addresses. -func (s *vfHealthStore) snapshotAddresses() map[string]struct{} { - s.mu.Lock() - defer s.mu.Unlock() - addresses := make(map[string]struct{}, len(s.records)) - for address := range s.records { - addresses[address] = struct{}{} - } - return addresses -} - func (s *vfHealthStore) persistLocked() error { if s.path == "" { return nil diff --git a/lib/devices/vf_health_test.go b/lib/devices/vf_health_test.go index 9e759906a..e7a06e736 100644 --- a/lib/devices/vf_health_test.go +++ b/lib/devices/vf_health_test.go @@ -72,6 +72,46 @@ func TestClearVFQuarantine(t *testing.T) { assert.False(t, cleared) } +func TestQuarantineVFRollsBackOnPersistFailure(t *testing.T) { + resetVFHealthStore(t) + // Point the store below a path component that is a file, so persisting + // fails at MkdirAll. + blocker := filepath.Join(t.TempDir(), "blocker") + require.NoError(t, os.WriteFile(blocker, nil, 0644)) + vfHealth.path = filepath.Join(blocker, "vf-health.json") + + _, _, err := QuarantineVF(VFQuarantine{VFAddress: "0000:e3:00.4"}) + require.Error(t, err) + + // A quarantine is only real once persisted: the failed record must not + // linger in memory, or the retried conviction would be treated as a + // repeat and never reach disk. + assert.False(t, IsVFQuarantined("0000:e3:00.4")) + assert.Empty(t, QuarantinedVFs()) +} + +func TestCheckedAddressesFailsClosedOnUnloadedState(t *testing.T) { + path := resetVFHealthStore(t) + + _, _, err := QuarantineVF(VFQuarantine{VFAddress: "0000:e3:00.4"}) + require.NoError(t, err) + + require.NoError(t, os.WriteFile(path, []byte("not json"), 0644)) + require.Error(t, initVFHealthStore(path)) + + // Placement must not run against a record set that is only empty because + // the state file was unreadable. + _, err = vfHealth.checkedAddresses() + require.Error(t, err) + + // A repaired file self-heals on the next placement attempt. + restored := `[{"vf_address":"0000:e3:00.4","wedge_count":1,"quarantined_at":"2026-08-20T00:00:00Z"}]` + require.NoError(t, os.WriteFile(path, []byte(restored), 0644)) + addresses, err := vfHealth.checkedAddresses() + require.NoError(t, err) + assert.Contains(t, addresses, "0000:e3:00.4") +} + func TestQuarantineVFRefusesToClobberUnloadedState(t *testing.T) { path := resetVFHealthStore(t) diff --git a/lib/instances/start.go b/lib/instances/start.go index 44dc50216..51a786b60 100644 --- a/lib/instances/start.go +++ b/lib/instances/start.go @@ -182,6 +182,19 @@ func (m *manager) startInstance( } } + // Archive the previous boot's serial log before any new vGPU assignment + // is persisted: the sentinel controller keys its tail on the assignment + // epoch, so an old log left in place would be rescanned from the top as + // the new boot's output and could replay a wedge report against the + // fresh VF. For GPU instances a failed archive is therefore fatal. + if err := m.archiveAppLogForBoot(id); err != nil { + if stored.GPUProfile != "" { + log.ErrorContext(ctx, "failed to archive app log before start", "instance_id", id, "error", err) + return nil, fmt.Errorf("archive app log before start: %w", err) + } + log.WarnContext(ctx, "failed to archive app log before start", "instance_id", id, "error", err) + } + // 4b. Recreate the vGPU if this instance had a GPU profile // Note: GPU availability was already validated in step 2b if stored.GPUProfile != "" { @@ -230,10 +243,6 @@ func (m *manager) startInstance( } configDiskSpanEnd(nil) - if err := m.archiveAppLogForBoot(id); err != nil { - log.WarnContext(ctx, "failed to archive app log before start", "instance_id", id, "error", err) - } - // 6. Start hypervisor and boot VM (reuses logic from create) bootStart := time.Now().UTC() stored.StartedAt = &bootStart diff --git a/lib/instances/vgpu_sentinel.go b/lib/instances/vgpu_sentinel.go index 3b45997c2..c3bb62f9f 100644 --- a/lib/instances/vgpu_sentinel.go +++ b/lib/instances/vgpu_sentinel.go @@ -13,6 +13,7 @@ import ( "time" "github.com/kernel/hypeman/lib/devices" + "github.com/kernel/hypeman/lib/logger" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric" ) @@ -38,9 +39,9 @@ const ( // report as it arrives on the serial console (the agent observes the NVIDIA // driver's RmInitAdapter failure in the guest kernel log and emits this // marker — the same guest-to-host channel as the other HYPEMAN-* markers). -// The full marker shape including the nvrm field is required: a bare token -// could appear in echoed exec command lines. -var vgpuSentinelPattern = regexp.MustCompile(`HYPEMAN-GPU-INIT-FAILED ts=\S+ nvrm="`) +// The full marker shape through the quoted NVRM payload is required: a bare +// token or a truncated marker could appear in echoed exec command lines. +var vgpuSentinelPattern = regexp.MustCompile(`HYPEMAN-GPU-INIT-FAILED ts=\S+ nvrm="NVRM: [^"]*RmInitAdapter failed![^"]*"`) type vgpuSentinelTarget struct { instanceID string @@ -315,6 +316,11 @@ func (m *manager) listVGPUSentinelTargets(ctx context.Context) ([]vgpuSentinelTa id := filepath.Base(filepath.Dir(file)) meta, err := m.loadMetadata(id) if err != nil { + if !errors.Is(err, ErrNotFound) { + // An unreadable record removes its VF from detection; say so + // rather than silently shrinking coverage. + logger.FromContext(ctx).WarnContext(ctx, "vGPU sentinel skipping unreadable instance metadata", "instance_id", id, "error", err) + } continue } if meta.GPUFramework != devices.VGPUFrameworkVendorVFIO || meta.GPUDevicePath == "" { diff --git a/lib/instances/vgpu_sentinel_test.go b/lib/instances/vgpu_sentinel_test.go index d552345c9..8e5ba7fb5 100644 --- a/lib/instances/vgpu_sentinel_test.go +++ b/lib/instances/vgpu_sentinel_test.go @@ -26,8 +26,11 @@ func TestVGPUSentinelPattern(t *testing.T) { // agent observes it in the guest and reports the marker instead. assert.False(t, vgpuSentinelPattern.MatchString("[ 27.031415] NVRM: GPU 0000:e3:00.4: RmInitAdapter failed! (0x22:0x65:884)")) - // Echoed exec command lines can carry the bare token on the same console. + // Echoed exec command lines can carry the bare token, a truncated marker, + // or a marker-shaped line without the NVRM payload on the same console. assert.False(t, vgpuSentinelPattern.MatchString("$ dmesg | grep -c HYPEMAN-GPU-INIT-FAILED")) + assert.False(t, vgpuSentinelPattern.MatchString("$ echo 'HYPEMAN-GPU-INIT-FAILED ts=x nvrm=\"'")) + assert.False(t, vgpuSentinelPattern.MatchString("HYPEMAN-GPU-INIT-FAILED ts=2026-08-20T15:04:05Z nvrm=\"something else\"")) assert.False(t, vgpuSentinelPattern.MatchString("2026/08/20 15:04:05 [guest-agent] HYPEMAN-AGENT-READY ts=2026-08-20T15:04:05Z")) } From 96953bef5fa9a67a55cc241a19530f20fc9a76a8 Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Fri, 21 Aug 2026 15:11:35 +0000 Subject: [PATCH 04/11] Match only kernel-facility kmsg records in the GPU init watch The /dev/kmsg record priority encodes facility*8+level. Kernel printk is always facility 0 and the kernel assigns userspace writers LOG_USER or higher (a facility-0 prefix is coerced to LOG_USER, verified on a live 6.12 kernel), so requiring facility 0 makes in-guest forgery of the report impossible, matching the intended kernel-records-only semantics. Also retry a failed /dev/kmsg open instead of permanently disabling the watcher for the guest's lifetime. --- lib/system/guest_agent/gpu_watch.go | 32 +++++++++++++++++++----- lib/system/guest_agent/gpu_watch_test.go | 13 ++++++++++ 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/lib/system/guest_agent/gpu_watch.go b/lib/system/guest_agent/gpu_watch.go index 2e08df7af..7a4376526 100644 --- a/lib/system/guest_agent/gpu_watch.go +++ b/lib/system/guest_agent/gpu_watch.go @@ -6,6 +6,7 @@ import ( "log" "os" "path/filepath" + "strconv" "strings" "time" ) @@ -29,6 +30,11 @@ const ( gpuReportThrottle = 30 * time.Second kmsgReopenDelay = 5 * time.Second + + // kmsgOpenRetryDelay paces reopen attempts after a failed /dev/kmsg + // open, so a guest where the open fails does not silently lose wedge + // detection for its whole lifetime. + kmsgOpenRetryDelay = time.Minute ) // hasNVIDIADevice reports whether any PCI function belongs to NVIDIA. A vGPU @@ -59,8 +65,9 @@ func watchGPUInitFailure() { for { f, err := os.Open(kmsgPath) if err != nil { - log.Printf("[guest-agent] cannot open %s for GPU init watch: %v", kmsgPath, err) - return + log.Printf("[guest-agent] cannot open %s for GPU init watch (retrying): %v", kmsgPath, err) + time.Sleep(kmsgOpenRetryDelay) + continue } scanKmsg(f, func(msg string) { if time.Since(lastReport) < gpuReportThrottle { @@ -90,14 +97,27 @@ func scanKmsg(r io.Reader, report func(msg string)) { } // gpuInitFailureMessage extracts the message from a /dev/kmsg record -// (";") and reports whether it is the NVIDIA driver's -// init-failure line. The full line shape is required — the trailing -// (stage:status:line) tuple is driver-build-specific and unsafe to match. +// (",,,;") and reports whether it is the +// NVIDIA driver's init-failure line. Only kernel records match: the priority +// field encodes facility*8+level, kernel printk is always facility 0, and +// the kernel assigns userspace /dev/kmsg writers LOG_USER or higher (a +// facility-0 prefix from userspace is coerced to LOG_USER), so a process +// inside the guest cannot forge a matching record. The full line shape is +// required — the trailing (stage:status:line) tuple is driver-build-specific +// and unsafe to match. func gpuInitFailureMessage(record string) (string, bool) { - _, msg, found := strings.Cut(record, ";") + prefix, msg, found := strings.Cut(record, ";") + if !found { + return "", false + } + priority, _, found := strings.Cut(prefix, ",") if !found { return "", false } + value, err := strconv.ParseUint(priority, 10, 32) + if err != nil || value>>3 != 0 { + return "", false + } msg = strings.TrimSpace(msg) if !strings.HasPrefix(msg, "NVRM:") || !strings.Contains(msg, "RmInitAdapter failed!") { return "", false diff --git a/lib/system/guest_agent/gpu_watch_test.go b/lib/system/guest_agent/gpu_watch_test.go index 1481f995e..3c46149d2 100644 --- a/lib/system/guest_agent/gpu_watch_test.go +++ b/lib/system/guest_agent/gpu_watch_test.go @@ -16,11 +16,24 @@ func TestGPUInitFailureMessage(t *testing.T) { _, ok = gpuInitFailureMessage("3,1042,8462102,-;NVRM: GPU 0000:00:03.0: RmInitAdapter failed! (0x26:0xffff:1482)\n") assert.True(t, ok) + // Any kernel log level matches; only the facility is load-bearing. + _, ok = gpuInitFailureMessage("4,1044,8462120,-;NVRM: GPU 0000:00:03.0: RmInitAdapter failed! (0x22:0x65:884)\n") + assert.True(t, ok) + for _, record := range []string{ "6,1041,8462100,-;NVRM: loading NVIDIA UNIX Open Kernel Module for x86_64\n", "6,1043,8462110,-;nvidia-gridd: RmInitAdapter failed mentioned in userspace\n", "no separator RmInitAdapter failed!\n", " continuation line of a multi-line record\n", + // Userspace /dev/kmsg writes carry facility LOG_USER or higher — the + // kernel coerces a facility-0 prefix to LOG_USER — so these records, + // captured from a live 6.12 kernel by writing the driver's line into + // /dev/kmsg from a root shell, must never convict: + "12,307,4250363151,-;NVRM: GPU 0000:00:03.0: RmInitAdapter failed! (0x22:0x65:884)\n", // plain write + "8,308,4250380620,-;NVRM: GPU 0000:00:03.0: RmInitAdapter failed! (0x22:0x65:884)\n", // "<0>" prefix + "9,310,5898419120,-;NVRM: GPU 0000:00:03.0: RmInitAdapter failed! (0x22:0x65:884)\n", // "<1>" prefix + "24,309,5898400480,-;NVRM: GPU 0000:00:03.0: RmInitAdapter failed! (0x22:0x65:884)\n", // "<24>" prefix (facility 3) + "x,1,100,-;NVRM: GPU 0000:00:03.0: RmInitAdapter failed! (0x22:0x65:884)\n", // malformed priority } { _, ok := gpuInitFailureMessage(record) assert.False(t, ok, "record %q must not match", record) From 078052e68f8e0b7d8de944fa491442f8a9cccb5e Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Fri, 21 Aug 2026 15:11:57 +0000 Subject: [PATCH 05/11] Bound vGPU sentinel scans to the line cap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The scan buffered each complete line whole, so guest console output could make the controller allocate line-sized buffers every pass; the 64KB cap only kept an unterminated tail from being re-read, and once such a line was skipped its late-arriving tail was parsed as a fresh line. Read through a fixed-size buffer instead: a line that overflows it cannot be a marker, so it is discarded — across scans if its newline has not arrived — without ever being held in memory, and its tail can no longer replay a marker. Rotation resets the skip state with the offset. --- lib/instances/vgpu_sentinel.go | 73 ++++++++++++++++----------- lib/instances/vgpu_sentinel_test.go | 77 +++++++++++++++++++++++++++++ 2 files changed, 122 insertions(+), 28 deletions(-) diff --git a/lib/instances/vgpu_sentinel.go b/lib/instances/vgpu_sentinel.go index c3bb62f9f..b8483a6e1 100644 --- a/lib/instances/vgpu_sentinel.go +++ b/lib/instances/vgpu_sentinel.go @@ -28,10 +28,11 @@ const ( vgpuSentinelBrakeWindow = 15 * time.Minute vgpuSentinelBrakeLimit = 3 - // vgpuSentinelMaxLineBytes bounds how much of an unterminated trailing - // line a scan will buffer and re-read. Marker lines are short; anything - // this long is console spam and is skipped once complete reading it - // becomes unreasonable. + // vgpuSentinelMaxLineBytes bounds how much of any single log line a scan + // holds in memory. The log is guest-controlled console output; marker + // lines are a few hundred bytes, so anything longer is console spam and + // is discarded — including its later-arriving tail — without ever being + // buffered whole. vgpuSentinelMaxLineBytes = 64 * 1024 ) @@ -61,7 +62,11 @@ type vgpuSentinelTail struct { vfAddress string assignedAt string offset int64 - done bool + // skippingLongLine marks that offset sits inside a line that exceeded + // vgpuSentinelMaxLineBytes; content is discarded until its newline so an + // oversized line's tail is never parsed as a fresh line. + skippingLongLine bool + done bool } // VGPUSentinelController scans the serial console log of vendor VFIO vGPU @@ -183,7 +188,7 @@ func (c *VGPUSentinelController) scanTarget(ctx context.Context, target vgpuSent if tail.done { return } - line, found, err := scanForSentinel(target.appLogPath, &tail.offset) + line, found, err := scanForSentinel(target.appLogPath, tail) if err != nil { c.log.WarnContext(ctx, "vGPU sentinel scan failed to read app log", "instance_id", target.instanceID, "error", err) @@ -256,11 +261,14 @@ func (c *VGPUSentinelController) convict(ctx context.Context, target vgpuSentine return true } -// scanForSentinel reads complete lines from offset onward, advancing offset -// and returning the first sentinel match. A partial trailing line is left -// unconsumed for the next scan. An offset past the file size means the log -// was archived for a new boot, so the scan restarts from the top. -func scanForSentinel(path string, offset *int64) (string, bool, error) { +// scanForSentinel reads complete lines from the tail's offset onward, +// advancing the offset and returning the first sentinel match. A partial +// trailing line is left unconsumed for the next scan. An offset past the +// file size means the log was archived for a new boot, so the scan restarts +// from the top. Memory is bounded by the line cap: a line that overflows the +// read buffer cannot be a marker, so it is consumed — across scans if its +// newline has not arrived yet — without ever being held whole. +func scanForSentinel(path string, tail *vgpuSentinelTail) (string, bool, error) { f, err := os.Open(path) if err != nil { if os.IsNotExist(err) { @@ -274,31 +282,40 @@ func scanForSentinel(path string, offset *int64) (string, bool, error) { if err != nil { return "", false, err } - if info.Size() < *offset { - *offset = 0 + if info.Size() < tail.offset { + tail.offset = 0 + tail.skippingLongLine = false } - if _, err := f.Seek(*offset, io.SeekStart); err != nil { + if _, err := f.Seek(tail.offset, io.SeekStart); err != nil { return "", false, err } - reader := bufio.NewReader(f) + reader := bufio.NewReaderSize(f, vgpuSentinelMaxLineBytes) for { - line, err := reader.ReadString('\n') - if err != nil { - if errors.Is(err, io.EOF) { - // An oversized unterminated line would otherwise be buffered - // again on every scan; skip it once it exceeds the bound. - if len(line) > vgpuSentinelMaxLineBytes { - *offset += int64(len(line)) - } - return "", false, nil + line, err := reader.ReadSlice('\n') + switch { + case err == nil: + tail.offset += int64(len(line)) + if tail.skippingLongLine { + tail.skippingLongLine = false + continue + } + if vgpuSentinelPattern.Match(line) { + return strings.TrimSpace(string(line)), true, nil + } + case errors.Is(err, bufio.ErrBufferFull): + tail.offset += int64(len(line)) + tail.skippingLongLine = true + case errors.Is(err, io.EOF): + // A partial trailing line stays unconsumed for the next scan, + // unless it is the tail of an oversized line being discarded. + if tail.skippingLongLine { + tail.offset += int64(len(line)) } + return "", false, nil + default: return "", false, err } - *offset += int64(len(line)) - if vgpuSentinelPattern.MatchString(line) { - return strings.TrimSpace(line), true, nil - } } } diff --git a/lib/instances/vgpu_sentinel_test.go b/lib/instances/vgpu_sentinel_test.go index 8e5ba7fb5..c9a7b37f5 100644 --- a/lib/instances/vgpu_sentinel_test.go +++ b/lib/instances/vgpu_sentinel_test.go @@ -6,6 +6,7 @@ import ( "log/slog" "os" "path/filepath" + "strings" "testing" "time" @@ -255,6 +256,82 @@ func TestVGPUSentinelControllerDropsStaleTails(t *testing.T) { assert.NotContains(t, c.tails, "instance-1") } +// The marker is a few hundred bytes, so a line that overflows the read +// buffer is guest console spam by definition: it must not convict even when +// it embeds a marker, must never be buffered whole, and its tail — arriving +// on a later scan — must not be parsed as a fresh line. +func TestScanForSentinelDiscardsOversizedLines(t *testing.T) { + t.Parallel() + + logPath := filepath.Join(t.TempDir(), "app.log") + tail := &vgpuSentinelTail{} + + // An oversized terminated line with an embedded marker: skipped whole. + huge := strings.Repeat("x", vgpuSentinelMaxLineBytes) + testSentinelLine + require.NoError(t, os.WriteFile(logPath, []byte(huge), 0644)) + line, found, err := scanForSentinel(logPath, tail) + require.NoError(t, err) + assert.False(t, found, "a marker inside an oversized line must not convict") + assert.Empty(t, line) + + // A short marker line after the oversized one still convicts. + appendSentinelLine(t, logPath) + line, found, err = scanForSentinel(logPath, tail) + require.NoError(t, err) + require.True(t, found) + assert.Contains(t, line, "RmInitAdapter failed!") +} + +func TestScanForSentinelDiscardsOversizedLineTailAcrossScans(t *testing.T) { + t.Parallel() + + logPath := filepath.Join(t.TempDir(), "app.log") + tail := &vgpuSentinelTail{} + + // An oversized line still missing its newline: the scan enters skip mode. + require.NoError(t, os.WriteFile(logPath, []byte(strings.Repeat("x", vgpuSentinelMaxLineBytes+10)), 0644)) + _, found, err := scanForSentinel(logPath, tail) + require.NoError(t, err) + assert.False(t, found) + assert.True(t, tail.skippingLongLine) + + // The line's tail arrives later carrying a marker shape; it is still the + // same oversized line, so it must be discarded, not parsed as fresh. + appendSentinelLine(t, logPath) + _, found, err = scanForSentinel(logPath, tail) + require.NoError(t, err) + assert.False(t, found, "the tail of an oversized line must not convict") + assert.False(t, tail.skippingLongLine) + + // The next genuine marker line convicts. + appendSentinelLine(t, logPath) + line, found, err := scanForSentinel(logPath, tail) + require.NoError(t, err) + require.True(t, found) + assert.Contains(t, line, "RmInitAdapter failed!") +} + +func TestScanForSentinelResetsSkipStateOnTruncatedLog(t *testing.T) { + t.Parallel() + + logPath := filepath.Join(t.TempDir(), "app.log") + tail := &vgpuSentinelTail{} + + require.NoError(t, os.WriteFile(logPath, []byte(strings.Repeat("x", vgpuSentinelMaxLineBytes+10)), 0644)) + _, _, err := scanForSentinel(logPath, tail) + require.NoError(t, err) + require.True(t, tail.skippingLongLine) + + // Rotation truncates the file under the tail; the restart from the top + // must clear the skip state or a marker in the fresh log would be lost. + require.NoError(t, os.WriteFile(logPath, []byte(testSentinelLine), 0644)) + line, found, err := scanForSentinel(logPath, tail) + require.NoError(t, err) + require.True(t, found) + assert.Contains(t, line, "RmInitAdapter failed!") + assert.False(t, tail.skippingLongLine) +} + func appendSentinelLine(t *testing.T, path string) { t.Helper() f, err := os.OpenFile(path, os.O_APPEND|os.O_WRONLY, 0644) From b35501a9c00710b1442f24b81b710d2bcc370ee1 Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Fri, 21 Aug 2026 15:12:09 +0000 Subject: [PATCH 06/11] Signal an unavailable VF health store MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the persisted state file fails to load, quarantine mutations are refused and vGPU placement fails closed, but the quarantined-VFs gauge reads zero from the empty in-memory set — exactly when quarantines exist and are unreadable. Export the load-failure state as its own gauge so the condition is alertable. --- lib/devices/vf_health.go | 11 +++++++++++ lib/instances/vgpu_sentinel.go | 18 ++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/lib/devices/vf_health.go b/lib/devices/vf_health.go index b3e88d736..8fe36e049 100644 --- a/lib/devices/vf_health.go +++ b/lib/devices/vf_health.go @@ -134,6 +134,17 @@ func ClearVFQuarantine(vfAddress string) (bool, error) { return cleared, err } +// VFHealthStoreUnavailable reports whether the persisted VF health state +// failed to load. While true, quarantine mutations are refused and vGPU +// placement fails closed — and the in-memory record set is empty, so the +// quarantine gauge would otherwise read zero exactly when quarantines exist +// but are unreadable. Surface this state instead of hiding it. +func VFHealthStoreUnavailable() bool { + vfHealth.mu.Lock() + defer vfHealth.mu.Unlock() + return vfHealth.loadErr != nil +} + // QuarantinedVFs returns all quarantine records, ordered by VF address. func QuarantinedVFs() []VFHealthRecord { vfHealth.mu.Lock() diff --git a/lib/instances/vgpu_sentinel.go b/lib/instances/vgpu_sentinel.go index b8483a6e1..c8d325ff3 100644 --- a/lib/instances/vgpu_sentinel.go +++ b/lib/instances/vgpu_sentinel.go @@ -114,6 +114,24 @@ func NewVGPUSentinelController(manager Manager, meter metric.Meter, log *slog.Lo if err != nil { return nil, err } + // The quarantined-VFs gauge reads zero when the state file is unreadable + // — exactly the state where quarantines exist but are unloadable and vGPU + // placement is failing closed — so that condition gets its own signal. + _, err = meter.Int64ObservableGauge( + "hypeman_instances_vgpu_vf_health_store_unavailable", + metric.WithDescription("1 when the persisted VF health state failed to load; quarantine mutations are refused and vGPU placement is disabled until it is repaired"), + metric.WithInt64Callback(func(_ context.Context, o metric.Int64Observer) error { + if devices.VFHealthStoreUnavailable() { + o.Observe(1) + } else { + o.Observe(0) + } + return nil + }), + ) + if err != nil { + return nil, err + } return &VGPUSentinelController{ store: store, From fc4433839647a4aefaf6f14414a59eacdce524ef Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Fri, 21 Aug 2026 15:41:25 +0000 Subject: [PATCH 07/11] Repeat the GPU init-failure marker so printk splits cannot lose a report The serial console is a shared byte stream: kernel printk bypasses the tty buffer and can land mid-marker, and on a wedged VF the kernel is emitting NVRM errors exactly when the agent reports. A corrupted copy does not match the host's full-shape scan (deliberately, so echoed commands cannot convict), which delayed the report to the next 30s re-emission. Emit each report as three identical lines sharing one ts; the host convicts on the first intact copy and ignores the rest. --- lib/instances/vgpu_sentinel_test.go | 26 ++++++++++++++++++++++ lib/system/guest_agent/gpu_watch.go | 20 ++++++++++++++++- lib/system/guest_agent/gpu_watch_test.go | 28 ++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 1 deletion(-) diff --git a/lib/instances/vgpu_sentinel_test.go b/lib/instances/vgpu_sentinel_test.go index c9a7b37f5..4a2175906 100644 --- a/lib/instances/vgpu_sentinel_test.go +++ b/lib/instances/vgpu_sentinel_test.go @@ -256,6 +256,32 @@ func TestVGPUSentinelControllerDropsStaleTails(t *testing.T) { assert.NotContains(t, c.tails, "instance-1") } +// Kernel printk shares the serial console with the agent and can split a +// marker mid-write — which is why the agent emits each report as several +// identical lines. A corrupted copy must not convict (the strict shape is +// what keeps echoed commands from convicting), and the intact repeat on the +// next line must. +func TestScanForSentinelConvictsOnIntactRepeatAfterSplitMarker(t *testing.T) { + t.Parallel() + + logPath := filepath.Join(t.TempDir(), "app.log") + split := "2026/08/20 15:04:05 [guest-agent] HYPEMAN-GPU-INIT-FAILED ts=2026-08-20T15:04:05Z nvrm=\"NVRM: GPU 0000:e3:0\n" + + "[ 27.031415] NVRM: GPU 0000:00:03.0: RmInitAdapter failed! (0x22:0x65:884)\n" + + "0.4: RmInitAdapter failed! (0x22:0x65:884)\"\n" + require.NoError(t, os.WriteFile(logPath, []byte(split), 0644)) + + tail := &vgpuSentinelTail{} + _, found, err := scanForSentinel(logPath, tail) + require.NoError(t, err) + assert.False(t, found, "neither a split marker nor the raw kernel line may convict") + + appendSentinelLine(t, logPath) + line, found, err := scanForSentinel(logPath, tail) + require.NoError(t, err) + require.True(t, found, "the intact repeat must convict") + assert.Contains(t, line, "HYPEMAN-GPU-INIT-FAILED") +} + // The marker is a few hundred bytes, so a line that overflows the read // buffer is guest console spam by definition: it must not convict even when // it embeds a marker, must never be buffered whole, and its tail — arriving diff --git a/lib/system/guest_agent/gpu_watch.go b/lib/system/guest_agent/gpu_watch.go index 7a4376526..297b0c095 100644 --- a/lib/system/guest_agent/gpu_watch.go +++ b/lib/system/guest_agent/gpu_watch.go @@ -29,6 +29,15 @@ const ( // console. gpuReportThrottle = 30 * time.Second + // gpuReportRepeats is how many identical marker lines one report emits. + // The serial console is a shared byte stream: kernel printk bypasses the + // tty buffer and can split a userspace write mid-marker, and on a wedged + // VF the kernel is emitting NVRM errors exactly when reports are sent. A + // corrupted copy does not match the host's full-shape scan, so each + // report is several identical lines — one write each — and the host + // convicts on the first intact copy, ignoring the rest. + gpuReportRepeats = 3 + kmsgReopenDelay = 5 * time.Second // kmsgOpenRetryDelay paces reopen attempts after a failed /dev/kmsg @@ -74,13 +83,22 @@ func watchGPUInitFailure() { return } lastReport = time.Now() - log.Printf("[guest-agent] %s ts=%s nvrm=%q", gpuInitFailedSentinelPrefix, time.Now().UTC().Format(time.RFC3339Nano), msg) + emitGPUInitFailureReport(msg) }) _ = f.Close() time.Sleep(kmsgReopenDelay) } } +// emitGPUInitFailureReport writes one report as gpuReportRepeats identical +// marker lines, all carrying the same ts so the host sees one report. +func emitGPUInitFailureReport(msg string) { + ts := time.Now().UTC().Format(time.RFC3339Nano) + for range gpuReportRepeats { + log.Printf("[guest-agent] %s ts=%s nvrm=%q", gpuInitFailedSentinelPrefix, ts, msg) + } +} + // scanKmsg reads /dev/kmsg records from r and calls report for each GPU // init-failure message. func scanKmsg(r io.Reader, report func(msg string)) { diff --git a/lib/system/guest_agent/gpu_watch_test.go b/lib/system/guest_agent/gpu_watch_test.go index 3c46149d2..cca106278 100644 --- a/lib/system/guest_agent/gpu_watch_test.go +++ b/lib/system/guest_agent/gpu_watch_test.go @@ -1,10 +1,13 @@ package main import ( + "bytes" + "log" "strings" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestGPUInitFailureMessage(t *testing.T) { @@ -40,6 +43,31 @@ func TestGPUInitFailureMessage(t *testing.T) { } } +// One report is emitted as several identical marker lines because kernel +// printk shares the serial console and can split a single write mid-marker +// — and a wedged VF guarantees printk traffic at report time. Any one +// intact copy convicts; the copies share one ts so they read as one report. +func TestEmitGPUInitFailureReportRepeatsMarkerLines(t *testing.T) { + var buf bytes.Buffer + prev := log.Writer() + log.SetOutput(&buf) + defer log.SetOutput(prev) + + emitGPUInitFailureReport("NVRM: GPU 0000:00:03.0: RmInitAdapter failed! (0x22:0x65:884)") + + lines := strings.Split(strings.TrimSpace(buf.String()), "\n") + require.Len(t, lines, gpuReportRepeats) + for i, line := range lines { + assert.Contains(t, line, "HYPEMAN-GPU-INIT-FAILED ts=") + assert.Contains(t, line, `nvrm="NVRM: GPU 0000:00:03.0: RmInitAdapter failed! (0x22:0x65:884)"`) + // Identical from the marker onward: same ts, one report. + assert.Equal(t, + lines[0][strings.Index(lines[0], "HYPEMAN"):], + line[strings.Index(line, "HYPEMAN"):], + "copy %d must be identical to the first", i) + } +} + func TestScanKmsgReportsEachFailureRecord(t *testing.T) { records := strings.Join([]string{ "6,1,100,-;booting", From 8ba72343d0bfc4e3cf3effeee9b72db24e49ffb1 Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Fri, 21 Aug 2026 15:52:31 +0000 Subject: [PATCH 08/11] Remove the conviction brake Auto-conviction no longer pauses on a burst. Systemic non-wedge init failures (e.g. a driver-mismatch image rollout) are expected to be caught on a test host before reaching production, and the convictions counter remains the alerting signal if one gets through; the brake was extra state and logic guarding against a case the rollout process already covers. Quarantine still only removes capacity and never touches instances, and the store's fail-closed load handling is unchanged. --- lib/devices/GPU.md | 14 ++++----- lib/instances/vgpu_sentinel.go | 47 ++++------------------------- lib/instances/vgpu_sentinel_test.go | 44 ++++++--------------------- 3 files changed, 23 insertions(+), 82 deletions(-) diff --git a/lib/devices/GPU.md b/lib/devices/GPU.md index 8d78b7057..800c4b5a0 100644 --- a/lib/devices/GPU.md +++ b/lib/devices/GPU.md @@ -295,17 +295,17 @@ becomes overflow-only so it drains toward the SR-IOV cycle. The conviction is logged at error level (`quarantined wedged vGPU VF`) and counted in `hypeman_instances_vgpu_sentinel_convictions_total`; `hypeman_instances_vgpu_quarantined_vfs` gauges the current quarantine count. -A burst of convictions (more than 3 in 15 minutes) pauses auto-conviction — -the guest agent keeps re-emitting the marker while the failure persists, so a -paused conviction lands once the window clears — so a systemic non-wedge init -failure (e.g. a guest/host driver mismatch rolling out) cannot quarantine the -fleet before an operator sees it. +There is no rate limit on convictions: a systemic non-wedge init failure +(e.g. a guest/host driver mismatch) emits the same line on every VF and +would quarantine the whole host, so such changes must be validated on a +test host first, and the convictions counter is the signal to alert on if +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 marker also rides a guest-writable channel — a root guest could forge -it and quarantine its own VF; the conviction brake bounds the blast radius, -and the quarantine only ever removes capacity, never touches the instance. +it and quarantine the VF its own instance holds; the quarantine only ever +removes capacity, never touches the instance. The wedge-creating kill itself leaves no host-side log: no kernel error, no XID, no plugin crash. Detection therefore happens on the next boot that lands diff --git a/lib/instances/vgpu_sentinel.go b/lib/instances/vgpu_sentinel.go index c8d325ff3..e97a611fa 100644 --- a/lib/instances/vgpu_sentinel.go +++ b/lib/instances/vgpu_sentinel.go @@ -14,20 +14,12 @@ import ( "github.com/kernel/hypeman/lib/devices" "github.com/kernel/hypeman/lib/logger" - "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric" ) const ( vgpuSentinelScanInterval = 5 * time.Second - // A burst of convictions is more likely a systemic, non-wedge init - // failure (e.g. a guest/host driver mismatch rolling out fleet-wide) - // than several independent wedges; quarantining every VF would degrade - // the host harder than the failure itself, so auto-conviction pauses. - vgpuSentinelBrakeWindow = 15 * time.Minute - vgpuSentinelBrakeLimit = 3 - // vgpuSentinelMaxLineBytes bounds how much of any single log line a scan // holds in memory. The log is guest-controlled console output; marker // lines are a few hundred bytes, so anything longer is console spam and @@ -78,13 +70,11 @@ type VGPUSentinelController struct { store vgpuSentinelStore log *slog.Logger interval time.Duration - now func() time.Time quarantine func(devices.VFQuarantine) (devices.VFHealthRecord, bool, error) isQuarantined func(string) bool hostFramework func() devices.VGPUFramework convictions metric.Int64Counter tails map[string]*vgpuSentinelTail - recent []time.Time } func NewVGPUSentinelController(manager Manager, meter metric.Meter, log *slog.Logger) (*VGPUSentinelController, error) { @@ -98,7 +88,7 @@ func NewVGPUSentinelController(manager Manager, meter metric.Meter, log *slog.Lo convictions, err := meter.Int64Counter( "hypeman_instances_vgpu_sentinel_convictions_total", - metric.WithDescription("Total wedged-VF sentinel matches by result"), + metric.WithDescription("Total wedged-VF sentinel convictions"), ) if err != nil { return nil, err @@ -137,7 +127,6 @@ func NewVGPUSentinelController(manager Manager, meter metric.Meter, log *slog.Lo store: store, log: log.With("controller", "vgpu_sentinel"), interval: vgpuSentinelScanInterval, - now: time.Now, quarantine: devices.QuarantineVF, isQuarantined: devices.IsVFQuarantined, hostFramework: func() devices.VGPUFramework { @@ -218,42 +207,19 @@ func (c *VGPUSentinelController) scanTarget(ctx context.Context, target vgpuSent tail.done = c.convict(ctx, target, line) } -// convict quarantines the target's VF, subject to the conviction brake. -// It reports whether scanning for this instance is finished; a failed or -// brake-suppressed quarantine leaves the tail open so the recurring report -// retries it — the brake pauses auto-conviction, it must not permanently -// drop a conviction. +// convict quarantines the target's VF. It reports whether scanning for this +// instance is finished; a failed quarantine leaves the tail open so the +// recurring report retries it. func (c *VGPUSentinelController) convict(ctx context.Context, target vgpuSentinelTarget, line string) bool { if c.isQuarantined(target.vfAddress) { // Already out of placement — typically a rescan of a standing // victim's log after a controller restart. Not a new wedge: no - // metric, no brake accounting. + // metric. c.log.InfoContext(ctx, "vGPU sentinel matched an already-quarantined VF", "vf", target.vfAddress, "instance_id", target.instanceID) return true } - now := c.now() - recent := c.recent[:0] - for _, t := range c.recent { - if now.Sub(t) < vgpuSentinelBrakeWindow { - recent = append(recent, t) - } - } - c.recent = recent - - if len(c.recent) >= vgpuSentinelBrakeLimit { - c.log.ErrorContext(ctx, "vGPU sentinel conviction brake engaged; VF not quarantined", - "vf", target.vfAddress, - "instance_id", target.instanceID, - "sentinel_line", line, - "convictions_in_window", len(c.recent), - "window", vgpuSentinelBrakeWindow.String(), - ) - c.convictions.Add(ctx, 1, metric.WithAttributes(attribute.String("result", "suppressed"))) - return false - } - record, existed, err := c.quarantine(devices.VFQuarantine{ VFAddress: target.vfAddress, InstanceID: target.instanceID, @@ -268,14 +234,13 @@ func (c *VGPUSentinelController) convict(ctx context.Context, target vgpuSentine // Lost a conviction race; the VF is already quarantined. return true } - c.recent = append(c.recent, now) c.log.ErrorContext(ctx, "quarantined wedged vGPU VF", "vf", target.vfAddress, "instance_id", target.instanceID, "sentinel_line", line, "wedge_count", record.WedgeCount, ) - c.convictions.Add(ctx, 1, metric.WithAttributes(attribute.String("result", "convicted"))) + c.convictions.Add(ctx, 1) return true } diff --git a/lib/instances/vgpu_sentinel_test.go b/lib/instances/vgpu_sentinel_test.go index 4a2175906..449e441d9 100644 --- a/lib/instances/vgpu_sentinel_test.go +++ b/lib/instances/vgpu_sentinel_test.go @@ -52,7 +52,6 @@ func newTestSentinelController(t *testing.T, store vgpuSentinelStore) (*VGPUSent store: store, log: slog.New(slog.DiscardHandler), interval: time.Hour, - now: time.Now, quarantine: func(q devices.VFQuarantine) (devices.VFHealthRecord, bool, error) { quarantined = append(quarantined, q) return devices.VFHealthRecord{VFAddress: q.VFAddress, WedgeCount: 1}, false, nil @@ -136,12 +135,16 @@ func TestVGPUSentinelControllerRetriesFailedQuarantine(t *testing.T) { assert.True(t, c.tails["instance-1"].done) } -func TestVGPUSentinelControllerBrakePausesConvictionBursts(t *testing.T) { +// A burst of convictions across many instances is quarantined without any +// rate limit: systemic non-wedge failures (e.g. a driver-mismatch rollout) +// are expected to be caught on a test host before reaching production, and +// the convictions counter is the alerting signal if one gets through. +func TestVGPUSentinelControllerConvictsBursts(t *testing.T) { t.Parallel() dir := t.TempDir() - targets := make([]vgpuSentinelTarget, 0, vgpuSentinelBrakeLimit+1) - for i := 0; i < vgpuSentinelBrakeLimit+1; i++ { + targets := make([]vgpuSentinelTarget, 0, 5) + for i := 0; i < 5; i++ { logPath := filepath.Join(dir, string(rune('a'+i))+".log") require.NoError(t, os.WriteFile(logPath, []byte(testSentinelLine), 0644)) targets = append(targets, vgpuSentinelTarget{ @@ -151,37 +154,12 @@ func TestVGPUSentinelControllerBrakePausesConvictionBursts(t *testing.T) { }) } c, quarantined := newTestSentinelController(t, &fakeSentinelStore{targets: targets}) - base := time.Now() - c.now = func() time.Time { return base } c.scanOnce(context.Background()) - // The burst converts to convictions up to the limit; the rest are - // suppressed rather than quarantining the whole host, and their tails - // stay open — the brake pauses conviction, it does not drop it. - assert.Len(t, *quarantined, vgpuSentinelBrakeLimit) - suppressed := 0 + assert.Len(t, *quarantined, len(targets)) for _, target := range targets { - if !c.tails[target.instanceID].done { - suppressed++ - // The guest agent re-emits the marker while the failure persists. - appendSentinelLine(t, target.appLogPath) - } + assert.True(t, c.tails[target.instanceID].done) } - assert.Equal(t, 1, suppressed) - - // Within the window the suppression holds. - c.scanOnce(context.Background()) - assert.Len(t, *quarantined, vgpuSentinelBrakeLimit) - - // Once the window clears, the next re-emission convicts. - c.now = func() time.Time { return base.Add(vgpuSentinelBrakeWindow + time.Second) } - for _, target := range targets { - if !c.tails[target.instanceID].done { - appendSentinelLine(t, target.appLogPath) - } - } - c.scanOnce(context.Background()) - assert.Len(t, *quarantined, vgpuSentinelBrakeLimit+1) } func TestVGPUSentinelControllerSkipsQuarantinedVFs(t *testing.T) { @@ -198,11 +176,9 @@ func TestVGPUSentinelControllerSkipsQuarantinedVFs(t *testing.T) { c.isQuarantined = func(vf string) bool { return vf == "0000:e3:00.4" } // A rescan of a standing victim's log after a controller restart must - // not re-convict a persisted quarantine: no brake accounting, no metric, - // and the tail closes. + // not re-convict a persisted quarantine: no metric, and the tail closes. c.scanOnce(context.Background()) assert.Empty(t, *quarantined) - assert.Empty(t, c.recent) assert.True(t, c.tails["instance-1"].done) } From 5dd965c84d3c80ca93921ffb66f0926228316a77 Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Fri, 21 Aug 2026 16:32:03 +0000 Subject: [PATCH 09/11] Read /dev/kmsg with a record-sized buffer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each /dev/kmsg read returns exactly one record and fails with EINVAL — without consuming the record — when the buffer is smaller. Records run up to CONSOLE_EXT_LOG_MAX (8 KiB), so bufio's default 4 KiB buffer wedged the watcher on the first oversized record: every reopen replayed the ring into the same record, silently losing all detection behind it. Size the buffer to the kernel's record bound and log non-EPIPE scan errors so a wedge is visible instead of silent. --- lib/system/guest_agent/gpu_watch.go | 27 ++++++++++++--- lib/system/guest_agent/gpu_watch_test.go | 44 +++++++++++++++++++++++- 2 files changed, 65 insertions(+), 6 deletions(-) diff --git a/lib/system/guest_agent/gpu_watch.go b/lib/system/guest_agent/gpu_watch.go index 297b0c095..daa2ae666 100644 --- a/lib/system/guest_agent/gpu_watch.go +++ b/lib/system/guest_agent/gpu_watch.go @@ -2,12 +2,14 @@ package main import ( "bufio" + "errors" "io" "log" "os" "path/filepath" "strconv" "strings" + "syscall" "time" ) @@ -40,6 +42,14 @@ const ( kmsgReopenDelay = 5 * time.Second + // kmsgRecordBufferBytes must be at least the kernel's maximum /dev/kmsg + // record size (CONSOLE_EXT_LOG_MAX, 8 KiB): each read(2) returns exactly + // one record and fails with EINVAL — without consuming the record — when + // the buffer is smaller. bufio's default 4 KiB buffer would wedge the + // watcher on the first oversized record, with every reopen replaying the + // ring back into it, silently losing all detection behind it. + kmsgRecordBufferBytes = 8192 + // kmsgOpenRetryDelay paces reopen attempts after a failed /dev/kmsg // open, so a guest where the open fails does not silently lose wedge // detection for its whole lifetime. @@ -78,7 +88,7 @@ func watchGPUInitFailure() { time.Sleep(kmsgOpenRetryDelay) continue } - scanKmsg(f, func(msg string) { + err = scanKmsg(f, func(msg string) { if time.Since(lastReport) < gpuReportThrottle { return } @@ -86,6 +96,9 @@ func watchGPUInitFailure() { emitGPUInitFailureReport(msg) }) _ = f.Close() + if err != nil && !errors.Is(err, syscall.EPIPE) { + log.Printf("[guest-agent] GPU init watch read %s failed (reopening): %v", kmsgPath, err) + } time.Sleep(kmsgReopenDelay) } } @@ -100,16 +113,20 @@ func emitGPUInitFailureReport(msg string) { } // scanKmsg reads /dev/kmsg records from r and calls report for each GPU -// init-failure message. -func scanKmsg(r io.Reader, report func(msg string)) { - reader := bufio.NewReader(r) +// init-failure message. It returns the error that ended the scan, nil on +// EOF. +func scanKmsg(r io.Reader, report func(msg string)) error { + reader := bufio.NewReaderSize(r, kmsgRecordBufferBytes) for { record, err := reader.ReadString('\n') if msg, ok := gpuInitFailureMessage(record); ok { report(msg) } if err != nil { - return + if errors.Is(err, io.EOF) { + return nil + } + return err } } } diff --git a/lib/system/guest_agent/gpu_watch_test.go b/lib/system/guest_agent/gpu_watch_test.go index cca106278..f93682a1c 100644 --- a/lib/system/guest_agent/gpu_watch_test.go +++ b/lib/system/guest_agent/gpu_watch_test.go @@ -2,8 +2,10 @@ package main import ( "bytes" + "io" "log" "strings" + "syscall" "testing" "github.com/stretchr/testify/assert" @@ -77,6 +79,46 @@ func TestScanKmsgReportsEachFailureRecord(t *testing.T) { }, "\n") + "\n" var got []string - scanKmsg(strings.NewReader(records), func(msg string) { got = append(got, msg) }) + require.NoError(t, scanKmsg(strings.NewReader(records), func(msg string) { got = append(got, msg) })) assert.Len(t, got, 2) } + +// kmsgConn mimics /dev/kmsg read(2) semantics: each read returns exactly one +// record, and a buffer smaller than the record fails with EINVAL without +// consuming it. +type kmsgConn struct { + records []string + pos int +} + +func (k *kmsgConn) Read(p []byte) (int, error) { + if k.pos >= len(k.records) { + return 0, io.EOF + } + rec := k.records[k.pos] + if len(p) < len(rec) { + return 0, syscall.EINVAL + } + k.pos++ + return copy(p, rec), nil +} + +// A record larger than bufio's default 4 KiB buffer must not wedge the scan: +// /dev/kmsg rejects a short read with EINVAL without consuming the record, +// so an undersized buffer would replay into the same record on every reopen +// and never reach a failure line behind it. +func TestScanKmsgReadsOversizedRecords(t *testing.T) { + oversized := "6,1,100,-;" + strings.Repeat("x", 5000) + "\n" + failure := "3,2,200,-;NVRM: GPU 0000:00:03.0: RmInitAdapter failed! (0x22:0x65:884)\n" + + var got []string + require.NoError(t, scanKmsg(&kmsgConn{records: []string{oversized, failure}}, + func(msg string) { got = append(got, msg) })) + assert.Len(t, got, 1) + + // A record beyond even the sized buffer surfaces the EINVAL instead of + // ending the scan silently, so the watcher logs the wedge. + huge := "6,3,300,-;" + strings.Repeat("x", kmsgRecordBufferBytes) + "\n" + err := scanKmsg(&kmsgConn{records: []string{huge}}, func(string) {}) + assert.ErrorIs(t, err, syscall.EINVAL) +} From 9bd838f80fc32bbddfe709372d974b28a88291ef Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Fri, 21 Aug 2026 16:32:03 +0000 Subject: [PATCH 10/11] Fsync VF health state writes A quarantine is only real once it is on disk, but the persist renamed without syncing the file or directory, so a host crash right after a conviction could silently drop it. Sync the temp file before the rename and the directory after, and deduplicate the sorted record listing. --- lib/devices/vf_health.go | 44 ++++++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 9 deletions(-) diff --git a/lib/devices/vf_health.go b/lib/devices/vf_health.go index 8fe36e049..d19bf48a3 100644 --- a/lib/devices/vf_health.go +++ b/lib/devices/vf_health.go @@ -149,8 +149,14 @@ func VFHealthStoreUnavailable() bool { func QuarantinedVFs() []VFHealthRecord { vfHealth.mu.Lock() defer vfHealth.mu.Unlock() - records := make([]VFHealthRecord, 0, len(vfHealth.records)) - for _, record := range vfHealth.records { + return vfHealth.sortedRecordsLocked() +} + +// sortedRecordsLocked returns every quarantine record ordered by VF address. +// The caller must hold s.mu. +func (s *vfHealthStore) sortedRecordsLocked() []VFHealthRecord { + records := make([]VFHealthRecord, 0, len(s.records)) + for _, record := range s.records { records = append(records, record) } sort.Slice(records, func(i, j int) bool { return records[i].VFAddress < records[j].VFAddress }) @@ -201,12 +207,7 @@ func (s *vfHealthStore) persistLocked() error { if s.path == "" { return nil } - records := make([]VFHealthRecord, 0, len(s.records)) - for _, record := range s.records { - records = append(records, record) - } - sort.Slice(records, func(i, j int) bool { return records[i].VFAddress < records[j].VFAddress }) - data, err := json.MarshalIndent(records, "", " ") + data, err := json.MarshalIndent(s.sortedRecordsLocked(), "", " ") if err != nil { return fmt.Errorf("marshal VF health state: %w", err) } @@ -214,11 +215,36 @@ func (s *vfHealthStore) persistLocked() error { return fmt.Errorf("create VF health state dir: %w", err) } tmp := s.path + ".tmp" - if err := os.WriteFile(tmp, data, 0644); err != nil { + f, err := os.OpenFile(tmp, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644) + if err != nil { + return fmt.Errorf("create VF health state: %w", err) + } + if _, err := f.Write(data); err != nil { + f.Close() + os.Remove(tmp) return fmt.Errorf("write VF health state: %w", err) } + // A quarantine is only real once it is on disk: sync before rename so a + // host crash cannot leave an empty or partial file where a durable + // conviction should be. + if err := f.Sync(); err != nil { + f.Close() + os.Remove(tmp) + return fmt.Errorf("sync VF health state: %w", err) + } + if err := f.Close(); err != nil { + os.Remove(tmp) + return fmt.Errorf("close VF health state: %w", err) + } if err := os.Rename(tmp, s.path); err != nil { + os.Remove(tmp) return fmt.Errorf("rename VF health state: %w", err) } + // Sync the directory so the rename itself survives a crash. Best-effort + // — a directory sync failure is not worth failing the conviction over. + if dir, err := os.Open(filepath.Dir(s.path)); err == nil { + _ = dir.Sync() + _ = dir.Close() + } return nil } From b0c71460020cba54286f21d02b6b3cb565b8aed7 Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Fri, 21 Aug 2026 16:32:03 +0000 Subject: [PATCH 11/11] Persist only the matched sentinel marker The matched line is guest-controlled console bytes up to the 64 KiB line cap; logging and persisting it verbatim put up to that much guest output in error logs and vf-health.json. Keep just the marker match. --- lib/instances/vgpu_sentinel.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/instances/vgpu_sentinel.go b/lib/instances/vgpu_sentinel.go index e97a611fa..fe9a39bdd 100644 --- a/lib/instances/vgpu_sentinel.go +++ b/lib/instances/vgpu_sentinel.go @@ -9,7 +9,6 @@ import ( "os" "path/filepath" "regexp" - "strings" "time" "github.com/kernel/hypeman/lib/devices" @@ -245,7 +244,10 @@ func (c *VGPUSentinelController) convict(ctx context.Context, target vgpuSentine } // scanForSentinel reads complete lines from the tail's offset onward, -// advancing the offset and returning the first sentinel match. A partial +// advancing the offset and returning the first sentinel match — only the +// matched marker, not the whole line: the surrounding bytes are +// guest-controlled console output up to the line cap and do not belong in +// error logs or the persisted health state. A partial // trailing line is left unconsumed for the next scan. An offset past the // file size means the log was archived for a new boot, so the scan restarts // from the top. Memory is bounded by the line cap: a line that overflows the @@ -283,8 +285,8 @@ func scanForSentinel(path string, tail *vgpuSentinelTail) (string, bool, error) tail.skippingLongLine = false continue } - if vgpuSentinelPattern.Match(line) { - return strings.TrimSpace(string(line)), true, nil + if match := vgpuSentinelPattern.Find(line); match != nil { + return string(match), true, nil } case errors.Is(err, bufio.ErrBufferFull): tail.offset += int64(len(line))