<fix>[compute]: vm lacks state transition handling#3184
<fix>[compute]: vm lacks state transition handling#3184zstack-robot-2 wants to merge 1 commit into5.5.0from
Conversation
vm is displayed as "Paused" and the control plane is missing the state transition logic from "Paused" to "NoState" Resolves: ZSTAC-79579 Change-Id: I697465636f646b6368686d71696d706b6e676e67
概览该PR为虚拟机异常生命周期状态变更添加了新的处理场景。当VM从暂停状态转换到无状态且主机保持不变时,系统现在能够检测并处理此种情况,与现有的从运行状态和崩溃状态转换的处理逻辑保持一致。 变更
评估代码审查工作量🎯 2 (Simple) | ⏱️ ~12 分钟 诗
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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.40.4)compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.javaComment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
test/src/test/groovy/org/zstack/test/integration/kvm/vm/CreateVmWithSameHostnameCase.groovy (3)
3-10: 新增依赖引入符合测试目标;建议补一处 Objects.equals 以降低回调 NPE 风险当前回调里存在若事件字段为空导致的
.equals(...)NPE 风险(见后续方法体)。建议在测试中统一用Objects.equals做字符串比较,避免偶发失败导致排障困难。Also applies to: 20-20
45-49: 把新用例挂到主 test 流程没问题;注意与原用例的“环境/事件监听”相互影响当前在同一个
env.create { ... }块里顺序执行两个测试方法是合理的;但由于后续新增用例会注册事件监听,建议确保监听在用例结束时被清理(否则后续扩展本 case 时容易引入互相干扰)。
82-122: 测试覆盖点到位,但事件回调的空指针与监听清理会影响稳定性
- 回调里
d.getOriginalHostUuid().equals(hostUuid)/d.getCurrentHostUuid().equals(hostUuid)若返回 null 会 NPE;建议改为Objects.equals(...)。evtf.on(...)监听建议在断言完成后反注册/关闭(若框架支持),避免同一用例后续逻辑或未来新增步骤被动“吃到”事件。建议修改(仅限测试增强,不改变业务语义)
@@ import org.zstack.testlib.SubCase +import java.util.Objects @@ if (d.getVmUuid().equals(vm.uuid) && d.getFrom() == VmInstanceState.Paused && d.getTo() == VmInstanceState.NoState - && d.getOriginalHostUuid().equals(hostUuid) - && d.getCurrentHostUuid().equals(hostUuid)) { + && Objects.equals(d.getOriginalHostUuid(), hostUuid) + && Objects.equals(d.getCurrentHostUuid(), hostUuid)) { eventReceived.set(true) } } })
📜 Review details
Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.javaheader/src/main/java/org/zstack/header/vm/VmAbnormalLifeCycleStruct.javatest/src/test/groovy/org/zstack/test/integration/kvm/vm/CreateVmWithSameHostnameCase.groovy
🧰 Additional context used
📓 Path-based instructions (2)
**/*.*
⚙️ CodeRabbit configuration file
**/*.*: - 代码里不应当有有中文,包括报错、注释等都应当使用正确的、无拼写错误的英文来写
Files:
header/src/main/java/org/zstack/header/vm/VmAbnormalLifeCycleStruct.javacompute/src/main/java/org/zstack/compute/vm/VmInstanceBase.javatest/src/test/groovy/org/zstack/test/integration/kvm/vm/CreateVmWithSameHostnameCase.groovy
**/*.java
⚙️ CodeRabbit configuration file
**/*.java: ## 1. API 设计要求
- API 命名:
- API 名称必须唯一,不能重复。
- API 消息类需要继承
APIMessage;其返回类必须继承APIReply或APIEvent,并在注释中用@RestResponse进行标注。- API 消息上必须添加注解
@RestRequest,并满足如下规范:
path:
- 针对资源使用复数形式。
- 当 path 中引用消息类变量时,使用
{variableName}格式。- HTTP 方法对应:
- 查询操作 →
HttpMethod.GET- 更新操作 →
HttpMethod.PUT- 创建操作 →
HttpMethod.POST- 删除操作 →
HttpMethod.DELETE- API 类需要实现
__example__方法以便生成 API 文档,并确保生成对应的 Groovy API Template 与 API Markdown 文件。
2. 命名与格式规范
类名:
- 使用 UpperCamelCase 风格。
- 特殊情况:
- VO/AO/EO 类型类除外。
- 抽象类采用
Abstract或Base前缀/后缀。- 异常类应以
Exception结尾。- 测试类需要以
Test或Case结尾。方法名、参数名、成员变量和局部变量:
- 使用 lowerCamelCase 风格。
常量命名:
- 全部大写,使用下划线分隔单词。
- 要求表达清楚,避免使用含糊或不准确的名称。
包名:
- 统一使用小写,使用点分隔符,每个部分应是一个具有自然语义的英文单词(参考 Spring 框架的结构)。
命名细节:
- 避免在父子类或同一代码块中出现相同名字的成员或局部变量,防止混淆。
- 命名缩写:
- 不允许使用不必要的缩写,如:
AbsSchedulerJob、condi、Fu等。应使用完整单词提升可读性。
3. 编写自解释代码
意图表达:
- 避免使用布尔型参数造成含义不明确。例如:
- 对于
stopAgent(boolean ignoreError),建议拆分为不同函数(如stopAgentIgnoreError()),或使用枚举表达操作类型。- 命名应尽量用完整的单词组合表达意图,并在名称中体现数据类型或用途(例如在常量与变量名称中,将类型词放在末尾)。
- 避免使用魔法值(Magic Value):
直接使用未经定义的数值或字符串(如 if (status == 5))应替换为枚举或常量。
示例:
// 错误示例:魔法值
if (user.getStatus() == 5) { ... }
// 正确示例:常量或枚举
public static final int STATUS_ACTIVE = 5;
if (user.getStatus() == STATUS_ACTIVE) { ... }
// 或使用枚举
enum UserStatus { ACTIVE, INACTIVE }
注释:
- 代码应尽量做到自解释,对少于两行的说明可以直接写在代码中。
- 对于较长的注释,需要仔细校对并随代码更新,确保内容正确。
- 接口方法不应有多余的修饰符(例如
public),且必须配有有效的 Javadoc 注释。
4. 流程控制和结构优化
if...else 的使用:
- 应尽量减少 if...else 结构的使用,建议:
- 限制嵌套层级最多为两层,且内层不应再出现
else分支。- 尽早返回(Early Return),将条件判断中的处理逻辑提前结束或抽成独立方法。
- 使用 Java Stream 或 Lambda 表达式代替冗长的循环与条件判断。
条件判断:
- if 条件表达不宜过长或过于复杂,必要时可以将条件抽成 boolean 变量描述。
代码块长度:
...
Files:
header/src/main/java/org/zstack/header/vm/VmAbnormalLifeCycleStruct.javacompute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java
🧠 Learnings (5)
📓 Common learnings
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 2360
File: network/src/main/java/org/zstack/network/l3/L3BasicNetwork.java:449-490
Timestamp: 2025-08-04T04:48:19.103Z
Learning: ZStack项目在cherry-pick操作中,即使发现了性能优化机会(如IP地址批量保存的内存优化),也严格遵循不做额外修改的政策,优先保证cherry-pick的完整性和一致性。
📚 Learning: 2025-07-22T02:30:46.123Z
Learnt from: MatheMatrix
Repo: MatheMatrix/zstack PR: 2311
File: plugin/sdnController/src/main/java/org/zstack/sdnController/hardwareVxlan/KVMRealizeHardwareVxlanNetworkBackend.java:52-53
Timestamp: 2025-07-22T02:30:46.123Z
Learning: In ZStack, backend realization code (such as KVMRealizeHardwareVxlanNetworkBackend) can safely assume the existence of the VO for the resource being operated on (e.g., HardwareL2VxlanNetworkVO for a given L2NetworkInventory), due to system lifecycle management and strong foreign key constraints in the database schema. Explicit null checks after findByUuid are not required in these contexts.
Applied to files:
test/src/test/groovy/org/zstack/test/integration/kvm/vm/CreateVmWithSameHostnameCase.groovy
📚 Learning: 2025-09-01T03:42:45.051Z
Learnt from: MatheMatrix
Repo: MatheMatrix/zstack PR: 2541
File: sdk/src/main/java/org/zstack/sdk/guesttools/GuestToolsStateInventory.java:1-1
Timestamp: 2025-09-01T03:42:45.051Z
Learning: The GuestToolsStateInventory SDK class in org.zstack.sdk package is not referenced by external clients, so package migrations for this component don't require backward compatibility aliases.
Applied to files:
test/src/test/groovy/org/zstack/test/integration/kvm/vm/CreateVmWithSameHostnameCase.groovy
📚 Learning: 2025-07-22T02:30:46.123Z
Learnt from: MatheMatrix
Repo: MatheMatrix/zstack PR: 2311
File: plugin/sdnController/src/main/java/org/zstack/sdnController/hardwareVxlan/KVMRealizeHardwareVxlanNetworkBackend.java:52-53
Timestamp: 2025-07-22T02:30:46.123Z
Learning: In ZStack, DatabaseFacade.findByUuid() calls for VO objects in contexts like KVMRealizeHardwareVxlanNetworkBackend do not require null checks because the L2NetworkInventory passed to these methods is guaranteed to have a corresponding VO record in the database by the system's design and calling patterns.
Applied to files:
test/src/test/groovy/org/zstack/test/integration/kvm/vm/CreateVmWithSameHostnameCase.groovy
📚 Learning: 2025-07-07T04:01:27.830Z
Learnt from: MatheMatrix
Repo: MatheMatrix/zstack PR: 2250
File: test/src/test/groovy/org/zstack/test/integration/network/l3network/IpRangeCase.groovy:528-531
Timestamp: 2025-07-07T04:01:27.830Z
Learning: 在ZStack测试框架中,测试用例之间是相互隔离的(case by case),在测试用例结尾修改全局配置(如VmGlobalConfig.VM_DELETION_POLICY)不需要恢复原始值,因为不会影响其他测试用例的执行。
Applied to files:
test/src/test/groovy/org/zstack/test/integration/kvm/vm/CreateVmWithSameHostnameCase.groovy
🔇 Additional comments (2)
compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java (1)
1512-1522: 补齐 Paused → NoState 的异常生命周期处理逻辑是合理的,且与现有 NoState 分支保持一致。把
VmNoStateFromPausedStateHostNotChanged并入该分支后,VM 在 host 上报NoState时会同步 DB 状态并刷新hostUuid,行为与Running/Crashed/Unknown → NoState的"host not changed"处理对齐,符合 PR 目标(避免控制面卡在 Paused)。枚举定义完整,
match()方法正确匹配 Paused → NoState 的转移,测试用例testVmStateChangedFromPausedToNoState()已覆盖此场景。按项目"sync/cherry-pick 尽量不做额外改动"的惯例,这个改动面很克制,质量良好。header/src/main/java/org/zstack/header/vm/VmAbnormalLifeCycleStruct.java (1)
174-181: 新增 Paused→NoState(同宿主)异常生命周期匹配逻辑合理,且做了更好的空指针防护这里用
Objects.equals(currentHostUuid, originalHostUuid)能避免currentHostUuid或originalHostUuid为空时的 NPE;与文件内多处直接.equals(...)的写法相比更稳。另提醒:该枚举的命中依赖values()顺序的findFirst(),当前插入位置会决定优先级;请确认这是期望的优先级(尤其当Paused可能属于intermediateStates时)。
vm is displayed as "Paused"
and the control plane is missing
the state transition logic from "Paused" to "NoState"
Resolves: ZSTAC-79579
Change-Id: I697465636f646b6368686d71696d706b6e676e67
sync from gitlab !9003