chore: ship-ready cleanup from final audit#58
Merged
Conversation
2 tasks
CMGS
force-pushed
the
chore/ship-ready-cleanup
branch
from
2afefa1 to
6c44470
Compare
- images/cloudimg.New now guards conf == nil (consistent with every other backend ctor; would otherwise panic in NewConfig deref) - CloneStorageConfigs skips nil StorageConfig entries per the memory rule "Never dereference pointers without nil check" - UpdateStates(VMStateRunning) is now rejected with an error so future callers don't silently open an unaccounted compute interval. Use BatchMarkStarted to open a fresh interval. Tests adjusted. - socketProbeTimeout 2s → 500ms; AF_UNIX local-only socket doesn't need a 2s budget and batch rm of unreachable VMs stalls less - BatchMarkStarted godoc reflects hasOpenComputeInterval (the previous wording referenced State==Running which drifted after the sentinel switch) - LoadRecord godoc documents the shallow value-copy (pointer/slice fields still alias the live record) - Two pre-existing standalone public-after-private layout fixes: firecracker.DevPath moved before private decompress helpers; metadata.CreateFAT12 moved before the private builder type
CMGS
force-pushed
the
chore/ship-ready-cleanup
branch
from
6c44470 to
6b86637
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.
Summary
Final ship-readiness audit pass — small, safe fixes from the 3-agent review (SKILL / reuse / quality+efficiency).
images/cloudimg.New: addconf == nilguard. Every other backend ctor (oci/CH/FC/cni/bridge/localfile) has it; cloudimg was the lone outlier and would have panicked inNewConfigderef.hypervisor.CloneStorageConfigs: skip nil entries per the memory rule "Never dereference pointers without nil check". Callers pass validated slices today, but the helper is general-purpose.UpdateStates(VMStateRunning)rejected: the Running branch silently opens a fresh compute interval without emittingcompute.start. Production code never calls it, but the case was a latent trap — any future caller would silently open an unaccounted interval. Now returns an error pointing callers toBatchMarkStarted.socketProbeTimeout2s → 500ms: AF_UNIX has no TIME_WAIT andECONNREFUSED/ENOENTreturns immediately; the only 2s wait happened for "socket file present but VMM frozen" — 500ms is plenty.BatchMarkStartedreferencedState==Runningwhich drifted after switching to thehasOpenComputeIntervalsentinelLoadRecorddocuments the shallow value-copy (pointer/slice fields still alias the live record)Two larger followups are coming as separate PRs:
refactor/dedup-helpers— Reuse Implement VM networking via CNI with per-VM netns + bridge + tap #1/Replace bridge with TC redirect for VM networking #2/macOS support: evaluate Apple Virtualization.framework via Code-Hex/vz #4/macOS support: vfkit as hypervisor backend #5/VM snapshot (checkpoint) and restore #6/support clone and restore from specific dir #22 (helper extractions)perf/batch-tx-and-cache— Q+E Replace bridge with TC redirect for VM networking #2/Senior tech review: bugs, Go 1.25 modernization, shared funcs, simplifications #3/support bridge tap to reduce hop between tap & nic #19 (List off-lock IO, DeleteAll batch tx, FindVMMByCmdline per-batch cache)Test plan
TestUpdateStatesEmitsOnlyOnRunningToStoppedrewritten to useBatchMarkStartedfor Running transitionsTestUpdateStatesRunningIsRejected— new test that the Running case now errorsTestDeleteAfterErrorEmitsOnlyStorageStopadjusted to BatchMarkStarted pathmake fmt-check && make lint && go test -race ./...— 21 packages green, lint 0, fmt 0, AST layout audit 0