fix(vmm): enforce configured ID pool bounds - #907
Merged
Conversation
reload_vms() and reload_vms_sync() occupy CIDs scraped from processes that are already running, not ids the pool handed out. Their values come from whatever cid_start/cid_pool_size was in effect when those VMs launched, so a bounds error there must not propagate: main.rs treats a reload_vms() failure as fatal, which would leave every running CVM unmanaged after an operator (or dstackup's pick_cid_start) moved the pool window. Report the untracked CID and continue instead.
"id outside pool range" is what an operator reads when the VMM refuses a CID, and it identifies neither the offending id nor the configured window. Add Display to the Number bound and interpolate both, and document the half-open [start, end) contract on IdPool. Also retarget the tests at what they claim to cover: the "concurrent" case only serialized 8 threads behind a Mutex, so it exercised the same path as a sequential loop, and the "maximum boundary" case built an empty pool (start == end) that returned on the range check without ever reaching next(). Split them into the allocation, reconstruction, exhaustion, empty-range and error-message cases, and apply cargo fmt.
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
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.
Problem
ID allocation did not consistently enforce configured lower and upper pool bounds, including restored state. Exhaustion or stale state could allocate an ID reserved outside the pool.
Root cause and fix
Validate candidates and restored IDs against the configured interval and return exhaustion/error instead of escaping it.
Implementation
The branch records the following focused implementation work:
fix(vmm): enforce configured ID pool boundstest(vmm): cover ID pool concurrency and reconstructionChanged paths:
dstack/vmm/src/app/id_pool.rsScope
This PR addresses one logical
vmmfinding. It intentionally excludes the acceptance-test infrastructure from #841 and unrelated product fixes from #840.Dependency and merge order
This PR is based directly on
masterand does not require another split product PR to merge first.Verification
git diff --check origin/master..origin/codex/fix-vmm-id-pool-bounds: passed.