Skip to content

Wait for exiting VMM tasks and retain failed vGPU cleanup - #485

Merged
yummybomb merged 3 commits into
mainfrom
hypeship/fix-vgpu-teardown
Sep 18, 2026
Merged

yummybomb merged 3 commits into
mainfrom
hypeship/fix-vgpu-teardown

Conversation

@yummybomb

@yummybomb yummybomb commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Why

QEMU's main thread can exit while a kernel worker is still releasing GPU resources. After an API restart, our fallback process check treated that as fully exited, so GPU cleanup could start too early.

If cleanup failed, delete still removed the instance metadata. That lost the record of which GPU allocation needed to be released.

What changed

  • Wait for the remaining tasks. A zombie process leader still counts as alive while other tasks remain in its thread group. This makes the fallback check agree with wait4, which already waits for the whole group to exit.
  • Keep failed cleanup retryable. If GPU release fails, stop deletion before removing metadata or tearing down the remaining resources. Keep the GPU claim and the existing manual-stop marker so the restart policy cannot restart the instance automatically.

The task check covers Linux 6.4+ vhost workers, which share QEMU's thread group. Older kernels use separate kernel threads. Process-identity checks and handling of fully exited zombies are unchanged.

What callers will see

DELETE now returns an error if GPU release fails, rather than reporting success and forgetting the allocation. The existing reconciler retries the retained GPU claim; callers still need to retry DELETE to finish removing the instance.

A permanently stuck GPU leaves the instance visible. The response is still a generic 500 with no retry hint, and this PR does not add force-delete or recover allocations whose metadata was already deleted.

Testing

Passed under the race detector:

  • Targeted process-exit, identity, shutdown, vGPU, and lifecycle tests, run twice.
  • The lib/devices suite.

New regressions exercise a real zombie leader with a remaining task, a separate observer taking the non-child ECHILD path, and failed cleanup followed by a successful delete or reconciliation from persisted metadata. These regressions fail against the old implementation.

Test commands
go test -race -tags containers_image_openpgp ./lib/instances -run 'ProcessExit|ResolveLiveHypervisorPID|KillHypervisor|GracefulShutdown|ShutdownHypervisor|VGPU|LifecycleNoop|HostBootID|ProcessStartTime|TestDelete' -count=2
go test -race -tags containers_image_openpgp ./lib/devices

No end-to-end GPU test was run. The earlier full instances integration run did not pass: it hit missing mkfs.erofs, insufficient bridge/network permissions, and race reports in the umoci image-unpacking path.

Independent of #484. No dependency or generated-file changes.

Automated review summary

[!NOTE]
Medium Risk
Changes core Linux process-liveness semantics and delete failure modes for vGPU instances; incorrect liveness could delay teardown or wedge deletes, but the behavior targets a known VFIO race.

Overview
Fixes premature vGPU teardown after an API restart when QEMU is no longer a direct child and exit detection treated a zombie thread-group leader as fully gone while vhost/VFIO tasks were still running.

ProcessExists on Linux now keeps a zombie leader alive while /proc/<pid>/task has more than one entry, aligning the non-child WaitForProcessExit fallback with wait4 behavior. Fully exited zombies and existing identity/reap logic are unchanged.

Instance delete now returns an error when releaseStoredVGPUPersisted fails instead of logging a warning and continuing. Metadata and the GPU claim stay on disk so reconcile or a retried DELETE can finish safely; device/network teardown no longer runs after a failed release.

Adds Linux regression tests for zombie-leader exit waiting (including the sibling ECHILD observer path) and replaces the lifecycle test to expect retained claims and successful retry/reconcile.

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

@yummybomb
yummybomb marked this pull request as ready for review September 17, 2026 17:10
@yummybomb
yummybomb requested review from hiroTamada and removed request for sjmiller609 September 17, 2026 17:57

@hiroTamada hiroTamada left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

reviewed the process-liveness checks, deletion ordering, and regression tests. no actionable findings.

local targeted tests could not compile because required embedded VMM and guest-agent binaries were missing; test results were not independently verified.

@yummybomb
yummybomb merged commit e5d1dca into main Sep 18, 2026
9 checks passed
@yummybomb
yummybomb deleted the hypeship/fix-vgpu-teardown branch September 18, 2026 14:14
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