Harden vGPU cleanup failure handling - #377
Closed
yummybomb wants to merge 59 commits into
Closed
Conversation
Since vGPU devices attach through VGPUDevicePath, PCIDevices only carries whole-device passthrough paths; the mdev special case was unreachable.
Stop and delete keep best-effort vGPU release (log and continue, metadata always cleared) and start no longer releases a stale stored assignment. Those behavior changes belong to the lifecycle-hardening layer, not this behavior-preserving refactor.
…led release Start now releases any stored assignment before acquiring a new one and fails the start if that release fails. Stop and delete retain assignment metadata when release fails instead of clearing it, so a failed release can be retried later instead of leaking the device.
…ances retry a failed release
…ssignments to DestroyVGPU as a struct
Fork cloned the source's StoredMetadata wholesale, so an assignment retained by a failed release during stop was shared with the fork and either instance's later release could invalidate the other's. Clear the assignment fields on the fork while keeping GPUProfile; the fork acquires its own vGPU on start.
Start released a retained assignment but only saved metadata on the success path, so a failure later in start left on-disk metadata pointing at a device that was already released. Save right after the release, matching the retained-release retry on stop.
Restoring a snapshot rehydrated the vGPU assignment fields embedded in the snapshot metadata. A snapshot taken while an assignment was retained after a failed release could resurrect that claim after the release later succeeded, pointing the instance at a device that is gone or reused. Keep the instance's current assignment instead: device assignments are host state, not snapshot payload, and a claim retained at restore time must survive for the next release retry.
kill(pid, 0) returning EPERM means the process exists but cannot be signaled, and a zombie PID passes a bare kill(0) probe. Export the EPERM-aware, zombie-filtering processExists helper so every hypervisor liveness check shares one definition.
After a hypeman restart the hypervisor is not our child, so Wait4 returns ECHILD immediately and the kill loop finished before the process had exited. Poll for actual process exit in that case.
A bare liveness probe treats any process that reused a stored hypervisor PID as the owning VMM. Require the PID to own the instance's hypervisor socket on Linux before reporting it alive.
Accepted server-side sockets appear in /proc/net/unix with the same bound path as the listener, so any connected API client made socketRefForPath report multiple inodes and pid-reuse protection fell back to unconfirmed while the control socket was in use. Only entries with __SO_ACCEPTCON identify the owning process; duplicate listeners from unlink-and-rebind still resolve as unconfirmed.
The claim guard compared raw GPUDevicePath, which is empty on records persisted before the framework migration; a live claimant with only a legacy GPUMdevUUID was invisible to the check. Normalize the inventory side with storedVGPUDevicePath, matching the release subject.
…r seam Replace the go:linkname shadow of devices.hostVendorVFIO with createVGPU/destroyVGPU manager fields, and wrap failed creates whose rollback release also failed in VGPUCleanupPendingError so the API can point callers at the retained instance record.
Enable vendor VFIO dispatch in CreateVGPU now that the lifecycle persists assignments durably and guards releases. Protect nil-PID claims in the release guard: the hypervisor PID is only persisted after the claimant boots, so a matching assignment without a PID must be treated as live, matching the startup reconcile protection. Scan raw metadata instead of hydrating instances for the claim check. Hydration derives state through hypervisor queries for every instance on the host, which every vendor VFIO release would pay; the guard only needs the stored assignment, PID, and socket. Unreadable metadata still fails the release closed. Report pending vGPU cleanup even when retaining the rollback record fails: the destroy already failed, so the caller must learn about the outstanding assignment either way.
yummybomb
force-pushed
the
hypeship/vendor-vfio-vgpu
branch
7 times, most recently
from
August 10, 2026 15:37
3710e98 to
c1c272e
Compare
yummybomb
force-pushed
the
hypeship/vendor-vfio-vgpu
branch
9 times, most recently
from
August 12, 2026 15:20
4bdfbe3 to
78d9812
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes two correctness issues found in review of the vGPU stack (#366 → #322 → #363 → #364 → #321). Based on the stack top so it can be folded into #363/#321 or merged after.
1.
WaitForProcessExittreated an unsignalable process as exited (P1, #363)Both
Wait4fallback branches usedsyscall.Kill(pid, 0) != nilas proof of exit, which treatsEPERMthe same asESRCH. If the resolved socket-owner PID can't be signaled, SIGKILL fails withEPERM(previously only logged),Wait4returnsECHILD,kill(pid, 0)returnsEPERM, and delete proceeded to unlink the socket and release the vGPU/network while the VMM was still alive.killHypervisornow returns any SIGKILL error other thanESRCH, so delete fails closed and retains metadata.ProcessExists(EPERM⇒ alive,ESRCH/zombie ⇒ gone).2. Failed retention write left stale full metadata that reconcile protects forever (P2, #321)
If
startAndBootVMfails after full metadata is persisted (step 18), the rollback vGPU destroy also fails, andcleanupFailedCreate's minimal retention write then fails, it returnedfalse(Retained=false, message promising recovery "on the next startup reconcile") — but the step-18 metadata stayed on disk.liveInstanceVGPUDevicePathstreats a nil-PID claim as live, so startup reconcile shielded the VF indefinitely: a phantom claim requiring manual delete, while the error said nothing was retained.Both retention-failure branches now best-effort
deleteInstanceData(id), so startupReconcileVGPUsreclaims the VF as the error message promises.Tests
TestWaitForProcessExit_EPERMProcessIsAlive— asserts pid 1 (EPERM+ECHILD, exactly the buggy path) is not reported as exited; skipped when running as root.TestCleanupFailedCreateReportsUnpersistedRetention— now also asserts the stale metadata is gone after the retention write fails.go build ./...andgo test ./lib/instances/(unit set) pass locally; integration tests need host deps (mkfs.erofs) not available here.