<feature>[storage]: register and take over sblk#3384
Open
zstack-robot-2 wants to merge 1 commit intofeature-zsv-5.0.0-vm-registrationfrom
Open
<feature>[storage]: register and take over sblk#3384zstack-robot-2 wants to merge 1 commit intofeature-zsv-5.0.0-vm-registrationfrom
zstack-robot-2 wants to merge 1 commit intofeature-zsv-5.0.0-vm-registrationfrom
Conversation
APIImpact Resolves: ZSV-10000 Change-Id: I70637377776e777070676c6a6c616e74786b6667
总览引入两个新的主存储操作API（一致性检查和接管）的完整实现，包括事件类、消息类、SDK操作类及其文档，并在处理器和测试工具中添加支持。 变更
诗词
评估代码审核工作量🎯 4 (复杂) | ⏱️ ~45 分钟 🚥 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
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
♻️ Duplicate comments (2)
sdk/src/main/java/org/zstack/sdk/CheckPrimaryStorageConsistencyAction.java (2)
5-5: 冗余的自包内通配符导入（同TakeoverPrimaryStorageAction）。与
TakeoverPrimaryStorageAction.java中相同问题：import org.zstack.sdk.*;在当前包内多余，可删除。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/src/main/java/org/zstack/sdk/CheckPrimaryStorageConsistencyAction.java` at line 5, Remove the redundant package-wide wildcard import in CheckPrimaryStorageConsistencyAction by deleting the line "import org.zstack.sdk.*;" (same issue as in TakeoverPrimaryStorageAction); rely on specific required imports or package-level visibility instead to avoid duplicate self-package wildcard imports.
32-35:systemTags和userTags使用原始List类型（同TakeoverPrimaryStorageAction）。与
TakeoverPrimaryStorageAction.java中相同，建议改为List<String>。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/src/main/java/org/zstack/sdk/CheckPrimaryStorageConsistencyAction.java` around lines 32 - 35, Change the raw List declarations in CheckPrimaryStorageConsistencyAction to use a typed generic: replace the fields "public java.util.List systemTags;" and "public java.util.List userTags;" with "public java.util.List<String> systemTags;" and "public java.util.List<String> userTags;" respectively (same fix as in TakeoverPrimaryStorageAction) so the fields are List<String> rather than raw List.
🧹 Nitpick comments (6)
sdk/src/main/java/org/zstack/sdk/TakeoverPrimaryStorageAction.java (1)
5-5: 冗余的自包内通配符导入。
import org.zstack.sdk.*;与当前类处于同一包（org.zstack.sdk）中，属于冗余导入，可以删除。♻️ 建议修改
import java.util.HashMap; import java.util.Map; -import org.zstack.sdk.*;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/src/main/java/org/zstack/sdk/TakeoverPrimaryStorageAction.java` at line 5, 移除冗余的包内通配符导入：在 TakeoverPrimaryStorageAction 类（TakeoverPrimaryStorageAction.java）删除行 import org.zstack.sdk.*;，因为该类已处于 org.zstack.sdk 包内，保留其他必要的显式导入即可。sdk/src/main/java/org/zstack/sdk/GetSharedBlockGroupLunsAction.java (2)
31-35:systemTags和userTags使用原始List类型与
SharedBlockGroupLunInfo中相同的问题，应参数化为List<String>。♻️ 建议修复
- public java.util.List systemTags; + public java.util.List<String> systemTags; - public java.util.List userTags; + public java.util.List<String> userTags;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/src/main/java/org/zstack/sdk/GetSharedBlockGroupLunsAction.java` around lines 31 - 35, Change the raw List types for the public fields systemTags and userTags to parameterized types to match SharedBlockGroupLunInfo; update their declarations from "public java.util.List systemTags" and "public java.util.List userTags" to "public java.util.List<String> systemTags" and "public java.util.List<String> userTags" so the SDK exposes typed string tag lists (no other logic changes required).
5-5: 冗余的通配符导入
GetSharedBlockGroupLunsAction本身就在org.zstack.sdk包中，import org.zstack.sdk.*;导入是多余的，应删除。♻️ 建议修复
import java.util.HashMap; import java.util.Map; -import org.zstack.sdk.*;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/src/main/java/org/zstack/sdk/GetSharedBlockGroupLunsAction.java` at line 5, 在 GetSharedBlockGroupLunsAction 类文件中删除冗余的通配符导入语句 "import org.zstack.sdk.*;"（该类已在 org.zstack.sdk 包内），保留或添加必要的显式导入项以避免 IDE/编译器报未解析类型的错误，并在保存后确认编译通过；定位标识符：类名 GetSharedBlockGroupLunsAction 和导入语句 import org.zstack.sdk.*;。sdk/src/main/java/org/zstack/sdk/GetSharedBlockGroupLunsResult.java (1)
6-12:lunInfos字段使用原始Map类型，缺少泛型参数根据
SharedBlockGroupLunInfo的语义（按主机划分的 LUN 信息），lunInfos的类型应为Map<String, SharedBlockGroupLunInfo>，使用原始类型在反序列化（Gson）和 BeanUtils 映射时均可能产生ClassCastException。♻️ 建议修复
- public java.util.Map lunInfos; - public void setLunInfos(java.util.Map lunInfos) { - this.lunInfos = lunInfos; + public java.util.Map<String, org.zstack.sdk.SharedBlockGroupLunInfo> lunInfos; + public void setLunInfos(java.util.Map<String, org.zstack.sdk.SharedBlockGroupLunInfo> lunInfos) { + this.lunInfos = lunInfos; } - public java.util.Map getLunInfos() { + public java.util.Map<String, org.zstack.sdk.SharedBlockGroupLunInfo> getLunInfos() { return this.lunInfos; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/src/main/java/org/zstack/sdk/GetSharedBlockGroupLunsResult.java` around lines 6 - 12, The lunInfos field in class GetSharedBlockGroupLunsResult is declared as a raw java.util.Map which can cause ClassCastException during Gson deserialization and BeanUtils mapping; change the field and its accessor methods (lunInfos, setLunInfos, getLunInfos) to use the generic type Map<String, SharedBlockGroupLunInfo> (import SharedBlockGroupLunInfo if needed) so callers and serializers have the correct generic information.sdk/src/main/java/org/zstack/sdk/SharedBlockGroupLunInfo.java (2)
7-13: 使用原始类型（Raw Types）会丢失类型安全性，并可能导致 BeanUtils 映射问题
candidateLuns和existLunWwidsByHost均使用了未参数化的原始类型，建议明确泛型类型参数。SourceClassMap通过commons-beanutils进行属性拷贝，原始Map类型可能导致运行时类型转换异常。♻️ 建议修复
- public java.util.List candidateLuns; - public void setCandidateLuns(java.util.List candidateLuns) { - this.candidateLuns = candidateLuns; + public java.util.List<String> candidateLuns; + public void setCandidateLuns(java.util.List<String> candidateLuns) { + this.candidateLuns = candidateLuns; } - public java.util.List getCandidateLuns() { + public java.util.List<String> getCandidateLuns() { return this.candidateLuns; } - public java.util.Map existLunWwidsByHost; - public void setExistLunWwidsByHost(java.util.Map existLunWwidsByHost) { - this.existLunWwidsByHost = existLunWwidsByHost; + public java.util.Map<String, List<String>> existLunWwidsByHost; + public void setExistLunWwidsByHost(java.util.Map<String, List<String>> existLunWwidsByHost) { + this.existLunWwidsByHost = existLunWwidsByHost; } - public java.util.Map getExistLunWwidsByHost() { + public java.util.Map<String, List<String>> getExistLunWwidsByHost() { return this.existLunWwidsByHost; }Also applies to: 23-29
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/src/main/java/org/zstack/sdk/SharedBlockGroupLunInfo.java` around lines 7 - 13, The fields and accessors in SharedBlockGroupLunInfo use raw types which lose type-safety and break commons-beanutils mapping; change the field declarations and their getters/setters to use concrete generic types (e.g., change candidateLuns from raw java.util.List to a parameterized type like java.util.List<String> or the correct element type, and change existLunWwidsByHost from raw java.util.Map to java.util.Map<String, List<String>> or the appropriate key/value types), and update the signatures of getCandidateLuns, setCandidateLuns, getExistLunWwidsByHost, and setExistLunWwidsByHost in the SharedBlockGroupLunInfo class so BeanUtils can correctly copy properties without runtime type conversion errors.
15-21: 考虑改进boolean属性的 getter 命名以遵循 JavaBean 惯例虽然
getSharedGroupComplete()在技术上符合 JavaBean 规范（boolean属性的 getter 既可用is前缀也可用get前缀），但 JavaBean 规范建议原始boolean类型优先使用is前缀。为了与常见惯例保持一致并提高代码可读性，建议改用isSharedGroupComplete()替代。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/src/main/java/org/zstack/sdk/SharedBlockGroupLunInfo.java` around lines 15 - 21, Change the boolean getter to follow JavaBean convention: in class SharedBlockGroupLunInfo replace or supplement the existing getSharedGroupComplete() with an isSharedGroupComplete() method that returns the sharedGroupComplete field; keep the existing setSharedGroupComplete(boolean) as-is and optionally retain getSharedGroupComplete() as a deprecated passthrough to isSharedGroupComplete() for backward compatibility.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@sdk/src/main/java/org/zstack/sdk/CheckPrimaryStorageConsistencyAction.java`:
- Line 5: Remove the redundant package-wide wildcard import in
CheckPrimaryStorageConsistencyAction by deleting the line "import
org.zstack.sdk.*;" (same issue as in TakeoverPrimaryStorageAction); rely on
specific required imports or package-level visibility instead to avoid duplicate
self-package wildcard imports.
- Around line 32-35: Change the raw List declarations in
CheckPrimaryStorageConsistencyAction to use a typed generic: replace the fields
"public java.util.List systemTags;" and "public java.util.List userTags;" with
"public java.util.List<String> systemTags;" and "public java.util.List<String>
userTags;" respectively (same fix as in TakeoverPrimaryStorageAction) so the
fields are List<String> rather than raw List.
---
Nitpick comments:
In `@sdk/src/main/java/org/zstack/sdk/GetSharedBlockGroupLunsAction.java`:
- Around line 31-35: Change the raw List types for the public fields systemTags
and userTags to parameterized types to match SharedBlockGroupLunInfo; update
their declarations from "public java.util.List systemTags" and "public
java.util.List userTags" to "public java.util.List<String> systemTags" and
"public java.util.List<String> userTags" so the SDK exposes typed string tag
lists (no other logic changes required).
- Line 5: 在 GetSharedBlockGroupLunsAction 类文件中删除冗余的通配符导入语句 "import
org.zstack.sdk.*;"（该类已在 org.zstack.sdk 包内），保留或添加必要的显式导入项以避免
IDE/编译器报未解析类型的错误，并在保存后确认编译通过；定位标识符：类名 GetSharedBlockGroupLunsAction 和导入语句 import
org.zstack.sdk.*;。
In `@sdk/src/main/java/org/zstack/sdk/GetSharedBlockGroupLunsResult.java`:
- Around line 6-12: The lunInfos field in class GetSharedBlockGroupLunsResult is
declared as a raw java.util.Map which can cause ClassCastException during Gson
deserialization and BeanUtils mapping; change the field and its accessor methods
(lunInfos, setLunInfos, getLunInfos) to use the generic type Map<String,
SharedBlockGroupLunInfo> (import SharedBlockGroupLunInfo if needed) so callers
and serializers have the correct generic information.
In `@sdk/src/main/java/org/zstack/sdk/SharedBlockGroupLunInfo.java`:
- Around line 7-13: The fields and accessors in SharedBlockGroupLunInfo use raw
types which lose type-safety and break commons-beanutils mapping; change the
field declarations and their getters/setters to use concrete generic types
(e.g., change candidateLuns from raw java.util.List to a parameterized type like
java.util.List<String> or the correct element type, and change
existLunWwidsByHost from raw java.util.Map to java.util.Map<String,
List<String>> or the appropriate key/value types), and update the signatures of
getCandidateLuns, setCandidateLuns, getExistLunWwidsByHost, and
setExistLunWwidsByHost in the SharedBlockGroupLunInfo class so BeanUtils can
correctly copy properties without runtime type conversion errors.
- Around line 15-21: Change the boolean getter to follow JavaBean convention: in
class SharedBlockGroupLunInfo replace or supplement the existing
getSharedGroupComplete() with an isSharedGroupComplete() method that returns the
sharedGroupComplete field; keep the existing setSharedGroupComplete(boolean)
as-is and optionally retain getSharedGroupComplete() as a deprecated passthrough
to isSharedGroupComplete() for backward compatibility.
In `@sdk/src/main/java/org/zstack/sdk/TakeoverPrimaryStorageAction.java`:
- Line 5: 移除冗余的包内通配符导入：在 TakeoverPrimaryStorageAction
类（TakeoverPrimaryStorageAction.java）删除行 import org.zstack.sdk.*;，因为该类已处于
org.zstack.sdk 包内，保留其他必要的显式导入即可。
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
conf/serviceConfig/primaryStorage.xmlis excluded by!**/*.xml
📒 Files selected for processing (18)
header/src/main/java/org/zstack/header/storage/primary/APICheckPrimaryStorageConsistencyEvent.javaheader/src/main/java/org/zstack/header/storage/primary/APICheckPrimaryStorageConsistencyEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/storage/primary/APICheckPrimaryStorageConsistencyMsg.javaheader/src/main/java/org/zstack/header/storage/primary/APICheckPrimaryStorageConsistencyMsgDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/storage/primary/APITakeoverPrimaryStorageEvent.javaheader/src/main/java/org/zstack/header/storage/primary/APITakeoverPrimaryStorageEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/storage/primary/APITakeoverPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/APITakeoverPrimaryStorageMsgDoc_zh_cn.groovysdk/src/main/java/SourceClassMap.javasdk/src/main/java/org/zstack/sdk/CheckPrimaryStorageConsistencyAction.javasdk/src/main/java/org/zstack/sdk/CheckPrimaryStorageConsistencyResult.javasdk/src/main/java/org/zstack/sdk/GetSharedBlockGroupLunsAction.javasdk/src/main/java/org/zstack/sdk/GetSharedBlockGroupLunsResult.javasdk/src/main/java/org/zstack/sdk/SharedBlockGroupLunInfo.javasdk/src/main/java/org/zstack/sdk/TakeoverPrimaryStorageAction.javasdk/src/main/java/org/zstack/sdk/TakeoverPrimaryStorageResult.javastorage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.javatestlib/src/main/java/org/zstack/testlib/ApiHelper.groovy
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
2 participants
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.
APIImpact
Resolves: ZSV-10000
Change-Id: I70637377776e777070676c6a6c616e74786b6667
sync from gitlab !9229