Skip to content

Detect wedged vGPU VFs from the guest and report to the health store - #435

Open
yummybomb wants to merge 1 commit into
hypeship/vgpu-vf-quarantinefrom
hypeship/vgpu-wedge-quarantine
Open

Detect wedged vGPU VFs from the guest and report to the health store#435
yummybomb wants to merge 1 commit into
hypeship/vgpu-vf-quarantinefrom
hypeship/vgpu-wedge-quarantine

Conversation

@yummybomb

@yummybomb yummybomb commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Top half of the wedged-VF work, stacked on #462 (the VF health store, placement exclusion, admission, and /resources fields). This PR adds the detection path that feeds that store.

  • The guest agent watches /dev/kmsg for kernel-facility NVRM: ... RmInitAdapter failed! records and emits a standalone HYPEMAN-GPU-INIT-FAILED marker. It also probes initialization with nvidia-smi -L when available and emits HYPEMAN-GPU-INIT-OK on success.
  • A host sentinel controller tails each assigned instance's app.log and reports failures and successes into the VF health store from Quarantine unhealthy vGPU VFs via a persisted health store #462.

Safety and failure handling

  • Failure and success markers are serialized in the guest, and success is terminal, so a delayed failure watcher cannot emit stale failure evidence after HYPEMAN-GPU-INIT-OK.
  • Only a complete, standalone guest-agent marker matches; ordinary output containing the token does not. The failure marker is emitted three times so a printk split mid-marker cannot lose the report.
  • A hung nvidia-smi attempt is bounded to 30 seconds without waiting indefinitely for a process stuck in an uninterruptible ioctl.
  • Repeated markers and controller restarts do not double-count an assignment. Start archives the previous boot's app log before persisting a new assignment, and the sentinel revalidates the assignment before reporting, so stale markers cannot count against a freshly assigned VF. Create allocates the vGPU immediately before metadata persistence to minimize the unpersisted window.
  • Reconciliation preserves assignments when hypervisor liveness is uncertain and records that condition in logs and metrics.

Observability

  • hypeman_instances_vgpu_sentinel_init_failures_total
  • hypeman_instances_vgpu_sentinel_quarantines_total
  • hypeman_instances_vgpu_quarantined_vfs
  • hypeman_instances_vgpu_vf_health_store_unavailable
  • hypeman_instances_vgpu_reconcile_liveness_uncertain_total

lib/devices/GPU.md gains the detection and sentinel documentation.

Out of scope

An operator force-cycle endpoint. Recovery remains the documented manual DCGM quiesce, SR-IOV cycle, state edit, restart, and verification flow.

Testing

Passed:

go test -race ./lib/devices ./lib/paths ./lib/providers ./lib/resources ./lib/system/guest_agent ./cmd/api/config
go test -race ./lib/instances -run 'Test(VGPUSentinel|ListVGPUSentinel|ScanForSentinel|VGPUReconcileLivenessUncertain)' -count=1
go vet ./lib/devices ./lib/paths ./lib/providers ./lib/resources ./lib/system/guest_agent ./lib/instances ./cmd/api/config

This branch's tree is byte-identical to the previously reviewed head (88f3c3d) — the split into #462 + this PR rewrites history only, not content. The underlying wedge signal and manual recovery sequence were previously validated on L40S hardware. A live end-to-end run of the guest watcher and host controller is still required before merge, including two failed assignments and the success path.


Note

Medium Risk
Changes GPU placement safety (quarantine inputs), instance create/start ordering for vGPUs, and background controllers on production API hosts; incorrect marker matching or assignment races could mis-quarantine VFs or miss wedges, though tests and fail-closed reconcile mitigate this.

Overview
Adds an end-to-end path to detect guest NVIDIA driver init failures on vendor VFIO vGPUs and feed the existing VF health / quarantine store.

The guest agent (when an NVIDIA PCI device is present) tails /dev/kmsg for kernel RmInitAdapter failed! lines and optionally probes with bounded nvidia-smi -L, emitting strict standalone HYPEMAN-GPU-INIT-FAILED / HYPEMAN-GPU-INIT-OK lines into app.log. Success is terminal so later failure noise is suppressed.

A new VGPUSentinelController runs in the API process on vendor VFIO hosts: every few seconds it tails each assigned instance’s app.log, matches only full guest-agent marker lines, confirms the VF assignment, and calls ReportVFInitFailure / ReportVFInitSuccess. It exposes sentinel and quarantine metrics and exits cleanly on non–vendor-VFIO hosts.

Lifecycle hardening supports trustworthy attribution: vGPU allocation on create moves to just before metadata save (after disks/network/config); create and start archive the prior boot’s app log before a new assignment (start fails closed for GPU instances if archive fails); GPU init markers are filtered from streamed app logs. vGPU reconcile now surfaces ambiguous hypervisor liveness (warn + hypeman_instances_vgpu_reconcile_liveness_uncertain_total) instead of silently treating uncertainty as live. Copy and error text for failed-create retention stubs are clarified.

lib/devices/GPU.md documents the detection flow, metrics, and sentinel behavior.

Reviewed by Cursor Bugbot for commit 1fd824a. Bugbot is set up for automated code reviews on this repo. Configure here.

@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch from d312338 to 7d54fb9 Compare August 20, 2026 19:12
@yummybomb
yummybomb marked this pull request as ready for review August 20, 2026 19:33
Comment thread lib/devices/vf_health.go
@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch from 7d54fb9 to 9a90223 Compare August 20, 2026 20:17
@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch from 9a90223 to 0ff16d1 Compare August 20, 2026 21:57
Comment thread lib/devices/vf_health.go
@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch from 0ff16d1 to 3db5546 Compare August 20, 2026 22:04
@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch from 3db5546 to b35501a Compare August 21, 2026 15:13
Comment thread lib/instances/vgpu_sentinel.go
@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch from b3bf925 to 55a0d6f Compare August 21, 2026 20:44
@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch from 55a0d6f to ee1b160 Compare August 21, 2026 20:47
Comment thread lib/devices/vendor_vfio_linux.go
@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch 2 times, most recently from a5dc229 to fed19c4 Compare August 24, 2026 19:03
@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch 2 times, most recently from 7bd4f52 to 67b2b24 Compare August 24, 2026 19:34
@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch from 67b2b24 to 0662ad9 Compare August 24, 2026 20:08
@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch 2 times, most recently from f3bf0cc to ff54c20 Compare August 25, 2026 14:25
@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch from ff54c20 to 37bc190 Compare August 25, 2026 14:28
@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch from 37bc190 to 44158d3 Compare August 25, 2026 14:29
@yummybomb
yummybomb requested a review from sjmiller609 August 25, 2026 15:28

@sjmiller609 sjmiller609 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good, a few small feedbacks and improvements we should consider.

  • Versioning: use a versioned envelope for future counters and recovery state:
{"version": 1, "records": [...]}
  • Failure threshold: currently 1 detection quarantines a VF. Should this be configurable? e.g. quarantine after failures from N distinct VM assignments, default 2. Also we could have a sucess reset: successful GPU-initialization sentinel that clears the VF’s failure count. I think we should do this if there's any case where this error happens even when the VF is not wedged.

  • Resources API: expose allocatable and quarantined slots so reported capacity matches admission:

 "total_slots": 64,
 "used_slots": 5,
+"allocatable_slots": 57,
+"quarantined_slots": 2
 "profiles": [{
   "name": "NVIDIA L40S-12Q",
-  "available": 59
+  "available": 57
 }]
  • GPU-less hosts: exit the controller when no GPU framework exists; continuous five-second discovery is unnecessary.

  • Terminology: replace “convict/convictions” terminology, that seems confusing.

  • Cleanup follow-up: acknowledged that automated GPU reset and cleanup are intentionally deferred; this PR handles detection and quarantine only.

@yummybomb

Copy link
Copy Markdown
Contributor Author

Addressed the review feedback (commits be50050, 7b24855):

  • Versioning: vf-health.json is now {"version": 1, "records": [...]}; unknown versions fail closed like any other invalid state.
  • Failure threshold: quarantine now requires failures from gpu.vf_quarantine_threshold distinct instance assignments, default 2, deduped per assignment so rescans/restarts can't double-count. Success reset is in too: the guest agent probes driver init at boot with nvidia-smi -L (the device open runs RmInitAdapter, so on a wedged VF the probe also forces detection instead of waiting for the workload) and emits HYPEMAN-GPU-INIT-OK on success, which clears the VF's failure tally. It never clears a quarantine — that stays manual.
  • Resources API: allocatable_slots and quarantined_slots are now in the /resources GPU status; profile available already excluded quarantined VFs.
  • GPU-less hosts: the controller exits when discovery finds no vGPU framework instead of re-probing every 5s (still retries on discovery errors so a transient sysfs failure doesn't kill it).
  • Terminology: convict/conviction is gone; the metric split into vgpu_sentinel_init_failures_total (below-threshold reports, the early signal for systemic non-wedge failures) and vgpu_sentinel_quarantines_total.
  • Cleanup follow-up: unchanged, force-cycle endpoint stays a follow-up.

Still pending before merge: the live end-to-end run on the dev GPU host, which should now cover the threshold path (two victim boots) and the OK probe.

Comment thread lib/instances/vgpu_sentinel.go Outdated
Comment thread lib/instances/vgpu_sentinel.go
@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown
-->

✱ stlc build

go code · compare

Your SDK build was successful.

generate ✅bootstrap ✅format ✅

116 files generated at 45ab306 (pushed)

go get github.com/kernel/hypeman-go-staging@45ab30663c023c6205ccb7567910b4555c62ffd2
python code · compare

Your SDK build was successful.

generate ✅bootstrap ✅format ✅

230 files generated at 6d080f8 (pushed)

typescript code · compare

Your SDK build was successful.

generate ✅bootstrap ✅format ✅

138 files generated at 27d7bf4 (pushed)

Diagnostics: ❗ 0 new / 1 total error, 💡 0 new / 5 total note
LevelCodeMessageTargets
Build metadata
Buildbd_76DbldwB-molten-churn
Timestamp2026-08-27T17:31:26.680Z
stlc8413509
Spec hash3652069ab265
Config hash55e15f6f4434

This comment is auto-generated by stlc and is kept up to date as you push.
If you push new commits, re-run this workflow to update this comment.
Last updated: 2026-08-27 17:31:47 UTC

Comment thread lib/instances/vgpu_sentinel.go

@yummybomb yummybomb left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

found three issues that should be addressed before merge: two false-positive quarantine paths and the unbounded nvidia-smi wait. the targeted vGPU/store/sentinel tests pass; the current linux CI failure is the unrelated TestWaitForState_ChannelClosedOnDelete timing failure.

@yummybomb yummybomb left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revising severity: these are technically reproducible edge cases, but neither should block this PR.

  • lib/instances/vgpu_sentinel.go:244-248 — non-blocking follow-up: a confirmed OK retry can be deleted only if the VF-health write fails and the same instance is reassigned between adjacent repeated OK lines. reusing an existing pending entry would close that resilience edge, but this is a compounded failure rather than a normal-path correctness issue.

  • lib/instances/vgpu_sentinel.go:343-388, lib/instances/logs.go:137-190 — non-blocking follow-up: copytruncate can move a marker to app.log.1 between sentinel scans. for this to affect the one-shot OK, the active log must already exceed the 50MB rotation threshold and the successful probe must land in the narrow pre-rotation window. worth making rotation-aware, but not a merge blocker for the initial quarantine path.

both repros prove the edge cases exist; they do not justify the blocker severity I originally assigned. the core failure detection, assignment revalidation, success ordering/rescission, and bounded probe paths look sound.

@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch from 6582c0e to 4d174d2 Compare August 26, 2026 19:37
Comment thread lib/system/guest_agent/gpu_watch.go Outdated

@yummybomb yummybomb left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one blocking test-quality issue. the sentinel/store boundaries, fail-closed placement behavior, assignment revalidation, and persistence rollback paths otherwise look coherent in context. targeted race tests and go vet pass locally; the current linux CI also exposes the changed-test failure below.

require.NoError(t, os.WriteFile(nvidiaSMI, []byte(script), 0o755))
t.Setenv("COUNT_PATH", countPath)

probeGPUInitUntil(&gpuInitReporter{}, nvidiaSMI, time.Now().Add(time.Second), 10*time.Millisecond, 0)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocker: this 10ms deadline is shorter than normal shell startup under race/loaded CI, so the nominally successful second attempt can time out too. The current Linux job failed here with actual: 3, and go test -race ... -count=100 reproduces it locally. Please make the attempt runner/clock injectable and use a deterministic fake, or otherwise give the success attempt independent scheduling margin; asserting exactly two real process launches with this deadline is flaky.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in 88f3c3d by injecting the probe attempt into the retry loop and testing the timeout/success sequence directly. go test -race ./lib/system/guest_agent -run TestProbeGPUInitRetriesAfterAttemptTimeout -count=100, the package race suite, and go vet pass.

@yummybomb yummybomb left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two code-quality findings. the store/placement boundaries, persistence rollback, assignment revalidation, and tests otherwise look coherent in repo context. the new functions peak at cyclomatic complexity 16 (loadLocked / selectLeastLoadedVF); those paths remain cohesive and well-covered, so I don't see mechanical decomposition as a merge requirement.

func (c *VGPUSentinelController) scanTarget(ctx context.Context, target vgpuSentinelTarget) {
tail := c.tails[target.instanceID]
if tail == nil || tail.vfAddress != target.vfAddress || tail.assignedAt != target.assignedAt {
tail = &vgpuSentinelTail{vfAddress: target.vfAddress, assignedAt: target.assignedAt}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tail offset is process-local, but a successful report removes the only persistent dedup record. After FAILED -> OK, restarting hypeman starts at offset 0, records the historical failure again, increments init_failures_total, then clears it again on the historical OK. With vf_quarantine_threshold: 1, replay also emits a fresh quarantine/rescind and increments both counters on every restart; if that replayed clear fails, a healthy VF stays quarantined. Please retain a bounded last-resolved assignment/watermark per VF (or coalesce a replay before mutating state) and add a restart regression test.

}

reader := bufio.NewReaderSize(f, vgpuSentinelMaxLineBytes)
for {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-blocking: please thread ctx into scanSentinelLog and check cancellation between reads (and target scans). On the first pass after controller/process start every assignment begins at byte 0, so a host with many logs near the 50 MB rotation limit can keep Run inside this loop and hold the process-level errgroup.Wait well after shutdown was requested.

@yummybomb yummybomb left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewed the change against hypeship/vendor-vfio-vgpu and traced the sentinel through guest output, host log scanning, durable health state, placement, and admission. the persistence rollback and assignment-identity handling are coherent, but the serial marker is not actually source-authenticated, and timed-out probes can overlap in the exact uninterruptible-ioctl case this code targets.

leaving the first finding as a blocker; github does not allow this account to request changes on its own PR.


// Require a standalone guest-agent line so echoed marker text cannot count against a VF.
var (
vgpuSentinelFailedPattern = regexp.MustCompile(`^\d{4}/\d{2}/\d{2} \d{2}:\d{2}:\d{2} \[guest-agent\] (HYPEMAN-GPU-INIT-FAILED ts=\S+ nvrm="NVRM: [^"\r\n]*RmInitAdapter failed![^"\r\n]*")\r?\n?$`)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocker: a standalone line does not establish that the guest agent wrote it. In exec mode the customer entrypoint's stdout/stderr is wired directly to the same serial console (lib/system/init/mode_exec.go:132-133), so an ordinary workload can print a complete line matching this regex without root or /dev/kmsg access. Repeating that across assignments can quarantine healthy VFs, and the runbook's statement that only a root guest can forge this is therefore incorrect. Please carry the report over a channel whose source the host can distinguish (for example a dedicated guest-agent RPC/transport), or otherwise authenticate the source; matching the log shape is not sufficient here.

case err := <-done:
return err
case <-timer.C:
_ = cmd.Process.Kill()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this bounds how long the caller waits, but not the attempt's lifetime. If nvidia-smi is stuck in the uninterruptible ioctl described above, Kill does not make cmd.Wait return; the process and waiter goroutine remain live while probeGPUInitUntil launches another attempt after 15s. The 10-minute loop can therefore accumulate many concurrent stuck probes on the VF. Can we retain the in-flight attempt and avoid launching another until it is reaped (or stop retrying once an attempt cannot be killed), so there is at most one outstanding nvidia-smi?

yummybomb

This comment was marked as spam.

@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch from 88f3c3d to b901282 Compare August 27, 2026 17:25
@yummybomb
yummybomb changed the base branch from hypeship/vendor-vfio-vgpu to hypeship/vgpu-vf-quarantine August 27, 2026 17:27
@yummybomb yummybomb changed the title Detect and quarantine wedged vGPU VFs Detect wedged vGPU VFs from the guest and report to the health store Aug 27, 2026
The guest agent watches /dev/kmsg for kernel-facility NVRM
RmInitAdapter-failed records and emits a standalone
HYPEMAN-GPU-INIT-FAILED marker (repeated so printk splits cannot lose
it), and probes driver init at boot with nvidia-smi -L when present,
emitting a terminal HYPEMAN-GPU-INIT-OK on success. A hung probe attempt
is bounded to 30 seconds.

A host sentinel controller tails each vendor VFIO instance's app.log,
matches only complete standalone markers, revalidates the assignment
before reporting, and feeds failures and successes into the VF health
store. Start archives the previous boot's app log before persisting a
new assignment so stale markers cannot count against the new VF, and
create allocates the vGPU immediately before metadata persistence to
minimize the unpersisted window.

vGPU reconciliation preserves assignments when hypervisor liveness is
uncertain and records the condition in logs and a metric. GPU.md gains
the detection and sentinel documentation.
@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch from b901282 to 1fd824a Compare August 27, 2026 17:29

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 3 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 1fd824a. Configure here.

// Require a standalone guest-agent line so echoed marker text cannot count against a VF.
var (
vgpuSentinelFailedPattern = regexp.MustCompile(`^\d{4}/\d{2}/\d{2} \d{2}:\d{2}:\d{2} \[guest-agent\] (HYPEMAN-GPU-INIT-FAILED ts=\S+ nvrm="NVRM: [^"\r\n]*RmInitAdapter failed![^"\r\n]*")\r?\n?$`)
vgpuSentinelOKPattern = regexp.MustCompile(`^\d{4}/\d{2}/\d{2} \d{2}:\d{2}:\d{2} \[guest-agent\] (HYPEMAN-GPU-INIT-OK ts=\S+)\r?\n?$`)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Workloads can forge GPU sentinel markers

High Severity

The host matcher accepts any complete standalone line in app.log. In exec mode the workload's stdout and stderr share that serial console, so an unprivileged guest can print a matching HYPEMAN-GPU-INIT-FAILED or HYPEMAN-GPU-INIT-OK line. Repeating the failure marker across assignments can quarantine healthy VFs; printing OK after a real failure can keep a wedged VF in rotation. The docs still describe this as root-only, which is not true for exec mode.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 1fd824a. Configure here.

assignedAt string
offset int64
skippingLongLine bool
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restart replays cleared failure markers

Medium Severity

Tail offsets live only in process memory. After FAILED then OK, a successful clear deletes the assignment's health-store dedup record. On hypeman restart the log is read from byte 0, so the historical failure is recorded again and init_failures_total increments. With vf_quarantine_threshold of 1 this also re-quarantines; if the replayed clear then fails, a healthy VF stays quarantined.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 1fd824a. Configure here.

case <-timer.C:
_ = cmd.Process.Kill()
return context.DeadlineExceeded
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hung GPU probes accumulate concurrently

Medium Severity

runGPUProbeAttempt returns after Kill without waiting for cmd.Wait. On a wedged VF, nvidia-smi can sit in an uninterruptible ioctl where Kill does not reap it, and probeGPUInitUntil starts another attempt every 15s for up to 10 minutes. That can leave many concurrent stuck probes on the same VF.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 1fd824a. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants