<fix>[storage]: safe #2975
Open
<fix>[storage]: safe #2975
Conversation
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
Resolves: ZSV-10538 Change-Id: I666562616373786e736c6b6a7261737177666e7a
整体说明这些变更通过在消息和命令类中添加新字段 变更汇总
序列图sequenceDiagram
participant VT as VolumeTree
participant VSTB as VolumeSnapshotTreeBase
participant MSG as CommitVolumeSnapshotOnHypervisorMsg
participant KH as KVMHost
participant BCC as BlockCommitCmd
VT->>VSTB: 初始化提交流程
VSTB->>VT: getAliveChainSnapshotInstallPath()
VT->>VSTB: 返回安装路径列表
VSTB->>MSG: setChainInstallPathInDb(paths)
KH->>MSG: 接收消息
KH->>BCC: setChainInstallPathInDb(paths)
KH->>BCC: 执行块提交命令
代码审查工作量评估🎯 3 (中等) | ⏱️ ~20 分钟 需重点关注的区域：
诗歌
Pre-merge checks and finishing touches❌ Failed checks (2 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 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.40.0)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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
storage/src/main/java/org/zstack/storage/snapshot/VolumeTree.java (1)
364-366: 建议方法命名保持一致性方法名使用了单数形式
getAliveChainSnapshotInstallPath()，而对应的getAliveChainSnapshotUuids()(第360行) 使用了复数形式Uuids。为保持命名一致性，建议将方法名改为getAliveChainSnapshotInstallPaths()。根据编码规范要求,命名应使用完整单词并保持一致的风格。
建议修改:
- public List<String> getAliveChainSnapshotInstallPath() { + public List<String> getAliveChainSnapshotInstallPaths() { return aliveChain.stream().map(VolumeSnapshotInventory::getPrimaryStorageInstallPath).collect(Collectors.toList()); }注意：如果修改方法名，需要同步更新所有调用此方法的地方（如 VolumeSnapshotTreeBase.java）。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
header/src/main/java/org/zstack/header/host/CommitVolumeSnapshotOnHypervisorMsg.java(2 hunks)plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java(2 hunks)plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java(1 hunks)storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotTreeBase.java(1 hunks)storage/src/main/java/org/zstack/storage/snapshot/VolumeTree.java(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.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()），或使用枚举表达操作类型。- 命名应尽量用完整的单词组合表达意图，并在名称中体现数据类型或用途（例如在常量与变量名称中，将类型词放在末尾）。
注释：
- 代码应尽量做到自解释，对少于两行的说明可以直接写在代码中。
- 对于较长的注释，需要仔细校对并随代码更新，确保内容正确。
- 接口方法不应有多余的修饰符（例如
public），且必须配有有效的 Javadoc 注释。
4. 流程控制和结构优化
if...else 的使用：
- 应尽量减少 if...else 结构的使用，建议：
- 限制嵌套层级最多为两层，且内层不应再出现
else分支。- 尽早返回（Early Return），将条件判断中的处理逻辑提前结束或抽成独立方法。
- 使用 Java Stream 或 Lambda 表达式代替冗长的循环与条件判断。
条件判断：
- if 条件表达不宜过长或过于复杂，必要时可以将条件抽成 boolean 变量描述。
代码块长度：
- 单个 if 代码块不宜超过一屏显示，以提高可读性和后续维护性。
5. 异常处理与日志
- 捕获异常的原则：
- 对于可以通过预检查避免的 RuntimeException（如
NullPointerException、IndexOutOfBoundsException等），不建议使用 try-catch 来进行处理。- 捕获异常应仅用于处理真正的意外情况，不应将异常逻辑当作正常流程控制。
- 在必要时，应继续抛出异常，使上层业务处理者可以转换为用户友好的错误提示。
- 使用 try-with-resources 语法管理资源，确保在 finally 块中正确关闭资源，并避免在 finally 中返回值。
...
Files:
storage/src/main/java/org/zstack/storage/snapshot/VolumeTree.javaheader/src/main/java/org/zstack/header/host/CommitVolumeSnapshotOnHypervisorMsg.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.javastorage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotTreeBase.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
🧠 Learnings (7)
📓 Common learnings
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 2496
File: storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotTreeBase.java:1218-1224
Timestamp: 2025-08-24T06:33:10.771Z
Learning: ZStack团队在容量管理相关问题上响应迅速，当发现Pull快照流程中申请容量与释放容量不匹配的问题时，开发人员会及时进行修复以确保主存储容量核算的准确性。
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-08-22T05:20:30.147Z
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 2489
File: header/src/main/java/org/zstack/header/storage/snapshot/VolumeSnapshotDeletionStructs.java:5-14
Timestamp: 2025-08-22T05:20:30.147Z
Learning: In VolumeSnapshotDeletionStructs class, String fields for direction and scope are preferred over enum types (DeleteVolumeSnapshotDirection/DeleteVolumeSnapshotScope) as they meet the expected architectural design requirements.
Applied to files:
storage/src/main/java/org/zstack/storage/snapshot/VolumeTree.java
📚 Learning: 2025-08-24T06:36:54.519Z
Learnt from: zstack-robot-2
Repo: MatheMatrix/zstack PR: 2496
File: storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotCascadeExtension.java:124-129
Timestamp: 2025-08-24T06:36:54.519Z
Learning: In cascade deletion scenarios for VolumeSnapshotCascadeExtension, the default values should be DeleteVolumeSnapshotDirection.Pull and DeleteVolumeSnapshotScope.Chain to maintain backward compatibility with existing cascade deletion logic, rather than using Auto values.
Applied to files:
storage/src/main/java/org/zstack/storage/snapshot/VolumeTree.java
📚 Learning: 2025-08-24T07:56:17.047Z
Learnt from: MatheMatrix
Repo: MatheMatrix/zstack PR: 2496
File: storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotCascadeExtension.java:71-73
Timestamp: 2025-08-24T07:56:17.047Z
Learning: In VolumeSnapshotCascadeExtension, the createActionForChildResource method always wraps the context in VolumeSnapshotDeletionStructs when creating child actions, so no backward compatibility with List<VolumeSnapshotInventory> is needed in handleDeletionCleanup.
Applied to files:
storage/src/main/java/org/zstack/storage/snapshot/VolumeTree.javastorage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotTreeBase.java
📚 Learning: 2025-08-25T03:53:30.749Z
Learnt from: MatheMatrix
Repo: MatheMatrix/zstack PR: 2504
File: plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmBackend.java:1010-1010
Timestamp: 2025-08-25T03:53:30.749Z
Learning: The OFFLINE_COMMIT_PATH ("/localstorage/snapshot/offlinecommit") for local storage snapshot commit operations is properly handled on both the ZStack management node side (in LocalStorageKvmBackend.java) and the KVM Agent side, providing complete end-to-end offline snapshot commit functionality.
Applied to files:
header/src/main/java/org/zstack/header/host/CommitVolumeSnapshotOnHypervisorMsg.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.javastorage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotTreeBase.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
📚 Learning: 2025-10-20T11:27:25.928Z
Learnt from: MatheMatrix
Repo: MatheMatrix/zstack PR: 2763
File: sdk/src/main/java/org/zstack/sdk/UpdateHostKernelInterfaceAction.java:31-31
Timestamp: 2025-10-20T11:27:25.928Z
Learning: UpdateHostKernelInterface API (`sdk/src/main/java/org/zstack/sdk/UpdateHostKernelInterfaceAction.java`) 在 PR #2763 时尚未被实际使用,因此对其参数约束的变更(如 name 字段从必需改为可选)不会造成破坏性影响。
Applied to files:
header/src/main/java/org/zstack/header/host/CommitVolumeSnapshotOnHypervisorMsg.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
📚 Learning: 2025-08-25T03:55:07.988Z
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 2504
File: plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.java:2046-2078
Timestamp: 2025-08-25T03:55:07.988Z
Learning: The OFFLINE_SNAPSHOT_COMMIT path for NFS primary storage snapshot commit operations is properly handled in the test infrastructure via NfsPrimaryStorageSpec.groovy, which includes both simulator and VFS hook implementations for testing the offline snapshot commit functionality.
Applied to files:
storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotTreeBase.java
🔇 Additional comments (4)
storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotTreeBase.java (1)
1022-1022: No issues found. The change properly setschainInstallPathInDbonCommitVolumeSnapshotOnHypervisorMsgfor online snapshot commits. The offline path correctly uses a different message type (CommitVolumeSnapshotOnPrimaryStorageMsg) which has different fields appropriate for primary storage operations. ThegetAliveChainSnapshotInstallPath()method returns a non-nullList<String>and requires no additional null safety checks.plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java (1)
1013-1020: 在 BlockCommitCmd 中补充链路安装路径传递是合理且完备的这里将
msg.getChainInstallPathInDb()直接传给cmd.setChainInstallPathInDb()，与前面top/base/topChildrenInstallPathInDb的传递方式保持一致，也契合新增字段在CommitVolumeSnapshotOnHypervisorMsg与BlockCommitCmd中的设计；从 KVMHost 视角看，快照链相关路径信息已全部向下传递给 agent，逻辑完整且不引入额外行为。Based on learnings, 该改动最小化且符合 cherry-pick 一致性预期。plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java (1)
3774-3774: 代码实现正确新增的
chainInstallPathInDb字段及其 getter/setter 方法实现正确,遵循了现有的代码模式(与topChildrenInstallPathInDb字段保持一致)。字段初始化为空 ArrayList,符合防御性编程实践。Also applies to: 3815-3821
header/src/main/java/org/zstack/header/host/CommitVolumeSnapshotOnHypervisorMsg.java (1)
16-16: 代码实现正确新增的
chainInstallPathInDb字段及其 getter/setter 方法实现正确,与现有的srcChildrenInstallPathInDb字段保持一致。字段初始化为空 ArrayList,符合防御性编程要求。此修改是向后兼容的增量变更。Also applies to: 59-65
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.
Resolves: ZSV-10538
Change-Id: I666562616373786e736c6b6a7261737177666e7a
sync from gitlab !8783