Skip to content

Harden vGPU release ordering in the instance lifecycle - #322

Open
yummybomb wants to merge 14 commits into
hypeship/vgpu-framework-abstractionfrom
hypeship/generalize-vgpu-device
Open

Harden vGPU release ordering in the instance lifecycle#322
yummybomb wants to merge 14 commits into
hypeship/vgpu-framework-abstractionfrom
hypeship/generalize-vgpu-device

Conversation

@yummybomb

@yummybombyummybomb commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Layer 2 of the vendor VFIO vGPU stack (#366this#363#364#321). Where #366 is a behavior-preserving refactor, this layer changes lifecycle semantics — release ordering and retention — still on mdev-era machinery, before the vendor VFIO backend lands above:

  • Delete releases the vGPU before other teardown — a successful release persists the cleared assignment before later teardown. A failed release keeps the pre-refactor contract: it is logged and the delete continues, since the release guards never destroy a device they cannot prove is unowned and a skipped release is recovered by startup reconciliation. Stopped instances retry a previously failed release.
  • Stop keeps its no-op contract — a failed retained-release on stop logs and retains the assignment metadata instead of failing the stop; assignments are passed to DestroyVGPU as a struct.
  • Forks don't inherit retained assignments — a retained (failed-release) vGPU assignment is stripped from fork metadata so a fork can't release its parent's device.
  • Start persists a stale release immediately — releasing a stale assignment before start writes the cleared metadata at once, so a later start failure can't leave metadata pointing at a device that's already gone.

Testing

  • go build ./..., go vet clean
  • go test -race targeted lib/instances lifecycle suites pass

Note

Medium Risk
Changes GPU device teardown and metadata persistence on start/stop/delete/fork. Incorrect ordering could leak or double-destroy vGPUs, but failed releases remain best-effort and are covered by new lifecycle tests.

Overview
Tightens vGPU assignment ownership so a failed release cannot leak a device to another instance or leave stale host paths in metadata.

Stop now retains assignment metadata when destroy fails (instead of clearing it anyway). A later stop of an already-stopped instance retries that release without breaking the no-op contract.

Start releases any retained assignment and persists the cleared fields before recreating a vGPU, so a later start failure cannot point at a device that is already gone.

Delete releases the vGPU immediately after the VMM is dead (before network/device/volume teardown) and persists a successful clear. Failed release still continues teardown. Restart policy is blocked and metadata reloaded first so later saves do not wipe the block.

Fork/restore never share a live assignment: forks strip device path/UUID (profile is kept); snapshot restore keeps the instance’s current assignment instead of resurrecting the snapshot copy.

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

@yummybomb

Copy link
Copy Markdown
ContributorAuthor

Trimmed over-engineering after an architectural review against the parent task (host kernel 6.8 / Ubuntu 24.04 support):

  • 1c1f52e — dropped the HasMdev/IsAllocated() compatibility shim. VirtualFunction is internal and never serialized in an API response or persisted, so a single Allocated field is enough.

Kept as-is:

  • the delete/stop behavior change (fail and retain assignment metadata when release fails) — orphan reconciliation only runs at startup, so failing the delete is the correct fail-closed behavior;
  • the darwin DestroyVGPU unknown-framework error — it keeps the fail-closed rollback semantics identical on macOS, which the lib/instances tests exercise in CI.

Tests: go test -race ./lib/devices, ./lib/resources, ./lib/hypervisor/qemu, ./lib/hypervisor/cloudhypervisor, and the vGPU-targeted ./lib/instances tests all pass; the remaining lib/instances E2E failures require KVM/images and fail identically on an unmodified checkout.

@yummybomb
yummybomb marked this pull request as ready for review August 5, 2026 19:47
@yummybomb
yummybombforce-pushed the hypeship/generalize-vgpu-device branch from 8abb859 to d489510CompareAugust 5, 2026 19:47
Comment threadlib/instances/stop.go Outdated
@yummybomb
yummybombforce-pushed the hypeship/generalize-vgpu-device branch from d489510 to 186cbf3CompareAugust 5, 2026 20:24
Comment threadlib/instances/start.go
@yummybomb
yummybomb changed the base branch from main to hypeship/vgpu-framework-abstractionAugust 6, 2026 19:16
@yummybombyummybomb changed the title Generalize vGPU device lifecycleHarden vGPU release ordering in the instance lifecycleAug 6, 2026
@yummybomb
yummybombforce-pushed the hypeship/generalize-vgpu-device branch from e458102 to 7e121c6CompareAugust 6, 2026 19:26
@yummybomb
yummybombforce-pushed the hypeship/generalize-vgpu-device branch 2 times, most recently from 7e10cf7 to cf815e2CompareAugust 7, 2026 14:02
Comment threadlib/instances/snapshot.go
@yummybomb
yummybombforce-pushed the hypeship/generalize-vgpu-device branch from 8cf6df0 to ffbcfbbCompareAugust 7, 2026 20:52
Comment threadlib/instances/delete.go
Comment threadlib/instances/delete.go
@yummybomb
yummybombforce-pushed the hypeship/generalize-vgpu-device branch from 3ec8640 to e43b46fCompareAugust 10, 2026 18:58
Comment threadlib/instances/snapshot.go
@yummybomb
yummybombforce-pushed the hypeship/generalize-vgpu-device branch from e43b46f to a4e00beCompareAugust 10, 2026 19:08
@yummybomb
yummybombforce-pushed the hypeship/generalize-vgpu-device branch from bb72152 to fd80867CompareAugust 12, 2026 15:20
@yummybomb
yummybombforce-pushed the hypeship/generalize-vgpu-device branch 2 times, most recently from d7cb958 to d9fbf20CompareAugust 13, 2026 18:28
@yummybomb
yummybombforce-pushed the hypeship/generalize-vgpu-device branch 2 times, most recently from 74862bf to e22b845CompareAugust 17, 2026 15:00
@yummybomb
yummybombforce-pushed the hypeship/generalize-vgpu-device branch 2 times, most recently from 82e452f to 7681ba1CompareAugust 21, 2026 20:44
…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.
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.
Delete goes back to the pre-refactor behavior: a failed vGPU release is
logged and teardown continues, instead of failing the delete and
retaining the instance. The release-side guards still never destroy a
device they cannot prove is unowned, so continuing only tolerates a
leaked slot until startup reconciliation recovers it - the same
tradeoff the mdev path always made. The restart-policy block before
teardown stays: a delete can still fail earlier when the hypervisor
cannot be confirmed dead.
@yummybomb
yummybombforce-pushed the hypeship/generalize-vgpu-device branch from 7681ba1 to c1048a4CompareAugust 21, 2026 20:47
Sign up for freeto 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

@yummybomb@sjmiller609