<fix>[vm]: ZSTAC-84158 handle StopVmGC host mismatch#3995
Open
MatheMatrix wants to merge 1 commit into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
Walkthrough在StopVmGC中将StopVmOnHypervisorMsg.ignoreNotFound设为true并在更新VM为stopped前校验VM当前的vmHostUuid是否仍在GC host；新增消息与agent命令的ignoreNotFound字段并在KVMHost中传递该标志到StopVmCmd。 变更说明VM停止操作ignoreNotFound功能
Sequence DiagramsequenceDiagram
participant StopVmGC
participant KVMHost
participant KVMAgent
participant VMDB
StopVmGC->>KVMHost: send StopVmOnHypervisorMsg (ignoreNotFound=true)
KVMHost->>KVMAgent: StopVmCmd (ignoreNotFound)
KVMAgent->>KVMHost: reply (stop result)
StopVmGC->>VMDB: query vmHostUuid
VMDB-->>StopVmGC: vmHostUuid
alt vmHostUuid != GC.hostUuid
StopVmGC->>StopVmGC: log debug and trigger.next() (skip state change)
else
StopVmGC->>StopVmGC: update VM state -> stopped
end
评估复杂度🎯 2 (简单) | ⏱️ ~12分钟 诗歌
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.42.2)plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaComment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
compute/src/main/java/org/zstack/compute/vm/StopVmGC.java (1)
70-84:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift主机一致性校验存在竞态窗口，仍可能误置
Stopped状态Line 70-83 先查询
hostUuid，随后 Line 84 异步发送ChangeVmStateMsg；两步之间 VM 仍可迁移，导致校验通过后在新主机场景下仍被置为Stopped。建议把“期望主机”作为状态变更前置条件，由状态处理端做原子校验（例如带expectedHostUuid的条件更新/校验）以消除 TOCTOU 风险。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@compute/src/main/java/org/zstack/compute/vm/StopVmGC.java` around lines 70 - 84, StopVmGC does a TOCTOU check by reading VmInstanceVO.hostUuid then asynchronously sending ChangeVmStateMsg, so the VM can move between check and state change; modify the flow to include the expected host UUID as part of the state-change request (e.g. add an expectedHostUuid field to ChangeVmStateMsg) and update the VM state-change handler (the code that processes ChangeVmStateMsg / the VM state transition logic) to perform an atomic conditional update/validation using that expectedHostUuid (reject or no-op if current hostUuid != expectedHostUuid) instead of relying on the prior read in StopVmGC; reference StopVmGC, ChangeVmStateMsg, VmInstanceVO/VmInstanceVO_.hostUuid and VmInstanceConstant service handling to locate where to add the field and where to enforce the atomic check.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@compute/src/main/java/org/zstack/compute/vm/StopVmGC.java`:
- Around line 70-84: StopVmGC does a TOCTOU check by reading
VmInstanceVO.hostUuid then asynchronously sending ChangeVmStateMsg, so the VM
can move between check and state change; modify the flow to include the expected
host UUID as part of the state-change request (e.g. add an expectedHostUuid
field to ChangeVmStateMsg) and update the VM state-change handler (the code that
processes ChangeVmStateMsg / the VM state transition logic) to perform an atomic
conditional update/validation using that expectedHostUuid (reject or no-op if
current hostUuid != expectedHostUuid) instead of relying on the prior read in
StopVmGC; reference StopVmGC, ChangeVmStateMsg,
VmInstanceVO/VmInstanceVO_.hostUuid and VmInstanceConstant service handling to
locate where to add the field and where to enforce the atomic check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
Run ID: a84ac299-9006-4fc3-8510-18c3a7df9f5c
📒 Files selected for processing (4)
compute/src/main/java/org/zstack/compute/vm/StopVmGC.javaheader/src/main/java/org/zstack/header/vm/StopVmOnHypervisorMsg.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
Resolves: ZSTAC-84158 Change-Id: I134a9c60e40d5bc9f4ad2b3cc27424b29858f0a9
MatheMatrix
force-pushed
the
sync/zstackio/fix/ZSTAC-84158@@3
branch
from
0641951 to
0488d4d
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.
sync from gitlab !9888