<feature>[storage]: res sblk #3208
Open
+186
−0
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
Walkthrough在主存储中添加“接管（takeover）”功能：新增API消息/事件、内部消息/回复类，并在PrimaryStorageBase中加入接管处理、执行逻辑与扩展钩子以支持接管流程和状态重载（包含成功/失败路径与日志、事件发布）。 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API_H as API Handler
participant PS as PrimaryStorage
participant Reload as ReloadLogic
Client->>API_H: PUT /primary-storage/{uuid}/takeover (APITakeoverPrimaryStorageMsg)
API_H->>PS: route to handle(APITakeoverPrimaryStorageMsg)
PS->>PS: construct TakeoverPrimaryStorageMsg -> handle(TakeoverPrimaryStorageMsg)
PS->>PS: doTakeover(ConnectParam, Completion)
PS->>Reload: request state reload
Reload-->>PS: reload result (success/failure)
alt success
PS->>PS: takeoverHook -> mark success, log, track
PS-->>API_H: TakeoverPrimaryStorageReply (success)
API_H-->>Client: APITakeoverPrimaryStorageEvent (inventory)
else failure
PS->>PS: track failure, maybe fire Disconnected event, log
PS-->>API_H: TakeoverPrimaryStorageReply (error set)
API_H-->>Client: APITakeoverPrimaryStorageEvent (error)
end
预估代码审查工作量🎯 3 (Moderate) | ⏱️ ~20 minutes 诗
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java (2)
324-423: 严重错误：缺少 ReloadPrimaryStorageMsg 消息处理分发
handleLocalMessage方法中没有添加对ReloadPrimaryStorageMsg的处理分发。虽然handle(ReloadPrimaryStorageMsg msg)方法已实现（Line 606-620），但消息不会被路由到该处理器，最终会走到bus.dealWithUnknownMessage(msg)。🐛 修复建议：在 handleLocalMessage 中添加消息分发
} else if (msg instanceof CleanUpStorageTrashOnPrimaryStorageMsg) { handle((CleanUpStorageTrashOnPrimaryStorageMsg)msg); + } else if (msg instanceof ReloadPrimaryStorageMsg) { + handle((ReloadPrimaryStorageMsg) msg); } else { bus.dealWithUnknownMessage(msg); }
981-1009: 严重错误：缺少 APIReloadPrimaryStorageMsg API 消息处理分发
handleApiMessage方法中没有添加对APIReloadPrimaryStorageMsg的处理分发。虽然handle(APIReloadPrimaryStorageMsg msg)方法已实现（Line 1468-1487），但 API 消息不会被路由到该处理器。🐛 修复建议：在 handleApiMessage 中添加 API 消息分发
} else if (msg instanceof APIAddStorageProtocolMsg) { handle((APIAddStorageProtocolMsg) msg); + } else if (msg instanceof APIReloadPrimaryStorageMsg) { + handle((APIReloadPrimaryStorageMsg) msg); } else { bus.dealWithUnknownMessage(msg); }
🤖 Fix all issues with AI agents
In
`@header/src/main/java/org/zstack/header/storage/primary/APIReloadPrimaryStorageEvent.java`:
- Line 1: The file's package declaration is misspelled as "ackage" causing
compilation to fail; change the leading token to "package" so the declaration
reads a proper package statement for the org.zstack.header.storage.primary
namespace (affecting APIReloadPrimaryStorageEvent and any classes in this file),
ensuring the package name matches the directory structure and rebuilds
successfully.
In `@storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java`:
- Around line 1468-1487: The handler for APIReloadPrimaryStorageMsg incorrectly
constructs and uses APIReconnectPrimaryStorageEvent (variable evt) which
mismatches the declared response type; in the handle(APIReloadPrimaryStorageMsg
msg) method replace APIReconnectPrimaryStorageEvent with
APIReloadPrimaryStorageEvent (instantiate APIReloadPrimaryStorageEvent evt = new
APIReloadPrimaryStorageEvent(msg.getId())), keep the rest of the logic (setting
error, reloading self, setting inventory and bus.publish(evt)) unchanged so the
published event type matches the `@RestRequest` responseClass.
🧹 Nitpick comments (2)
storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java (1)
715-764: 代码重复：doReload 与 doConnect 存在大量重复逻辑
doReload方法（Lines 715-764）与doConnect方法（Lines 662-713）有约 90% 的代码重复，主要区别仅在于doConnect会先设置状态为Connecting，而doReload不会。根据 DRY 原则，建议考虑提取公共逻辑到一个参数化的私有方法中。
♻️ 重构建议
private void doConnectOrReload(ConnectParam param, boolean setConnectingStatus, final Completion completion) { thdf.chainSubmit(new ChainTask(completion) { `@Override` public String getSyncSignature() { return getSyncId(); } `@Override` public void run(SyncTaskChain chain) { if (setConnectingStatus) { changeStatus(PrimaryStorageStatus.Connecting); } connectHook(param, new Completion(chain, completion) { // ... common success/fail logic }); } `@Override` public String getName() { return String.format("reconnect-primary-storage-%s", self.getUuid()); } }); } private void doConnect(ConnectParam param, final Completion completion) { doConnectOrReload(param, true, completion); } private void doReload(ConnectParam param, final Completion completion) { doConnectOrReload(param, false, completion); }header/src/main/java/org/zstack/header/storage/primary/APIReloadPrimaryStorageEvent.java (1)
5-5: 冗余导入
PrimaryStorageInventory与当前类在同一个包org.zstack.header.storage.primary中，无需显式导入。♻️ 建议删除冗余导入
import org.zstack.header.message.APIEvent; import org.zstack.header.rest.RestResponse; -import org.zstack.header.storage.primary.PrimaryStorageInventory; import java.util.Collections;
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
header/src/main/java/org/zstack/header/storage/primary/APIReloadPrimaryStorageEvent.javaheader/src/main/java/org/zstack/header/storage/primary/APIReloadPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/ReloadPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/ReloadPrimaryStorageReply.javastorage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java
🧰 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:
header/src/main/java/org/zstack/header/storage/primary/ReloadPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/APIReloadPrimaryStorageMsg.javastorage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.javaheader/src/main/java/org/zstack/header/storage/primary/ReloadPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/storage/primary/APIReloadPrimaryStorageEvent.java
🧠 Learnings (1)
📓 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的完整性和一致性。
🧬 Code graph analysis (2)
header/src/main/java/org/zstack/header/storage/primary/ReloadPrimaryStorageMsg.java (1)
header/src/main/java/org/zstack/header/storage/primary/PrimaryStorageHistoricalUsageBaseVO.java (1)
org(11-45)
header/src/main/java/org/zstack/header/storage/primary/APIReloadPrimaryStorageMsg.java (2)
header/src/main/java/org/zstack/header/storage/primary/PrimaryStorageHistoricalUsageBaseVO.java (1)
org(11-45)header/src/main/java/org/zstack/header/message/APIMessage.java (1)
APIMessage(16-245)
🔇 Additional comments (5)
header/src/main/java/org/zstack/header/storage/primary/ReloadPrimaryStorageReply.java (1)
1-6: LGTM!
ReloadPrimaryStorageReply类结构正确，遵循 ZStack 的消息回复模式。作为不需要返回额外数据的操作回复类，空类设计符合项目惯例。header/src/main/java/org/zstack/header/storage/primary/ReloadPrimaryStorageMsg.java (1)
1-16: LGTM!
ReloadPrimaryStorageMsg内部消息类设计合理，正确继承NeedReplyMessage并实现PrimaryStorageMessage接口。字段命名符合 lowerCamelCase 规范。header/src/main/java/org/zstack/header/storage/primary/APIReloadPrimaryStorageMsg.java (1)
1-38: LGTM!API 消息类设计符合 ZStack 规范：
@RestRequest注解配置正确，使用 PUT 方法符合更新操作的 HTTP 方法对应规范@APIParam正确关联PrimaryStorageVO资源类型__example__()方法实现正确，用于生成 API 文档storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java (1)
606-621: LGTM!
handle(ReloadPrimaryStorageMsg msg)方法实现正确，遵循与handle(ReconnectPrimaryStorageMsg msg)相同的模式。header/src/main/java/org/zstack/header/storage/primary/APIReloadPrimaryStorageEvent.java (1)
28-39: LGTM!
__example__()方法实现正确，提供了用于 API 文档生成的示例数据。
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
header/src/main/java/org/zstack/header/storage/primary/APIReloadPrimaryStorageEvent.java
Outdated
Show resolved
Hide resolved
MatheMatrix
force-pushed
the
sync/tao.gan/res-sblk-ZSV-1@@4
branch
from
4aff333 to
27ab9ee
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java`:
- Around line 749-759: Update the incorrect connect-related strings copied into
the reload flow in PrimaryStorageBase: change the logger message in the reload
handler (the debug call that currently says "failed to connect primary
storage[uuid:%s], %s") to reflect "failed to reload primary storage" and update
the anonymous task name returned by getName() (currently returning
"reconnect-primary-storage-%s") to "reload-primary-storage-%s" so both message
and task name correctly reference the reload operation in the doReload flow.
- Around line 610-624: handleLocalMessage() is missing routing for
ReloadPrimaryStorageMsg so ReloadPrimaryStorageMsg will never be processed; add
a branch in the if/else chain inside handleLocalMessage() (near the existing
ReconnectPrimaryStorageMsg handling) to detect ReloadPrimaryStorageMsg and call
the existing handler by invoking handle((ReloadPrimaryStorageMsg) msg) (or
equivalent), ensuring the method signature handle(ReloadPrimaryStorageMsg) is
used and the message is routed to that handler.
- Around line 1466-1485: The APIReloadPrimaryStorageMsg handler exists
(handle(APIReloadPrimaryStorageMsg)) but handleApiMessage() does not route that
API type, so reload requests never reach it; update handleApiMessage() to add a
branch similar to the APIReconnectPrimaryStorageMsg case that checks for
APIReloadPrimaryStorageMsg and dispatches it to handle(msg) (or calls
handle((APIReloadPrimaryStorageMsg)msg)) so the new handler is reachable and API
messages are properly routed.
♻️ Duplicate comments (1)
storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java (1)
1466-1467: 使用了错误的事件类型APIReconnectPrimaryStorageEventLine 1467 应使用
APIReloadPrimaryStorageEvent而非APIReconnectPrimaryStorageEvent，否则与@RestRequest注解中声明的responseClass不一致。
🧹 Nitpick comments (1)
header/src/main/java/org/zstack/header/storage/primary/APIReloadPrimaryStorageEvent.java (1)
5-5: 可选优化：移除冗余导入
PrimaryStorageInventory与当前类在同一包org.zstack.header.storage.primary下，无需显式导入。♻️ 建议修改
import org.zstack.header.message.APIEvent; import org.zstack.header.rest.RestResponse; -import org.zstack.header.storage.primary.PrimaryStorageInventory; import java.util.Collections;
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
header/src/main/java/org/zstack/header/storage/primary/APIReloadPrimaryStorageEvent.javaheader/src/main/java/org/zstack/header/storage/primary/APIReloadPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/ReloadPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/ReloadPrimaryStorageReply.javastorage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java
🚧 Files skipped from review as they are similar to previous changes (2)
- header/src/main/java/org/zstack/header/storage/primary/ReloadPrimaryStorageReply.java
- header/src/main/java/org/zstack/header/storage/primary/ReloadPrimaryStorageMsg.java
🧰 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/primary/PrimaryStorageBase.javaheader/src/main/java/org/zstack/header/storage/primary/APIReloadPrimaryStorageEvent.javaheader/src/main/java/org/zstack/header/storage/primary/APIReloadPrimaryStorageMsg.java
🧠 Learnings (1)
📓 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的完整性和一致性。
🧬 Code graph analysis (1)
header/src/main/java/org/zstack/header/storage/primary/APIReloadPrimaryStorageMsg.java (2)
header/src/main/java/org/zstack/header/storage/primary/PrimaryStorageHistoricalUsageBaseVO.java (1)
org(11-45)header/src/main/java/org/zstack/header/message/APIMessage.java (1)
APIMessage(16-245)
🔇 Additional comments (3)
header/src/main/java/org/zstack/header/storage/primary/APIReloadPrimaryStorageMsg.java (1)
8-37: LGTM!API 消息类设计规范：
- REST 注解配置正确，使用 PUT 方法进行重载操作
@APIParam正确配置了resourceType- 实现了
PrimaryStorageMessage接口并提供getPrimaryStorageUuid()方法- 包含
__example__()方法用于 API 文档生成storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java (1)
180-182: LGTM!
reloadHook作为子类扩展点设计合理，默认实现直接成功，符合现有 hook 方法模式。header/src/main/java/org/zstack/header/storage/primary/APIReloadPrimaryStorageEvent.java (1)
9-39: LGTM!API 事件类结构正确：
@RestResponse(allTo = "inventory")注解配置正确- 包含必要的无参和带
apiId的构造函数__example__()方法提供了完整的示例数据用于 API 文档生成
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Comment on lines
610
to
624
| protected void handle(ReloadPrimaryStorageMsg msg) { | ||
| ReloadPrimaryStorageReply reply = new ReloadPrimaryStorageReply(); | ||
| doReload(new ConnectParam(), new Completion(msg) { | ||
| @Override | ||
| public void success() { | ||
| bus.reply(msg, reply); | ||
| } | ||
|
|
||
| @Override | ||
| public void fail(ErrorCode errorCode) { | ||
| reply.setError(errorCode); | ||
| bus.reply(msg, reply); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
严重错误：ReloadPrimaryStorageMsg 缺少消息路由
handle(ReloadPrimaryStorageMsg) 方法已定义，但在 handleLocalMessage() 方法（Lines 328-427）的 if-else 链中缺少对应的路由分支。这意味着该消息永远不会被处理，导致 reload 功能失效。
🐛 修复建议：在 handleLocalMessage() 中添加路由
在 handleLocalMessage() 方法中（约 Line 375 附近，ReconnectPrimaryStorageMsg 之后）添加：
} else if (msg instanceof ReconnectPrimaryStorageMsg) {
handle((ReconnectPrimaryStorageMsg) msg);
+ } else if (msg instanceof ReloadPrimaryStorageMsg) {
+ handle((ReloadPrimaryStorageMsg) msg);
} else if (msg instanceof RevertVolumeFromSnapshotOnPrimaryStorageMsg) {🤖 Prompt for AI Agents
In `@storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java`
around lines 610 - 624, handleLocalMessage() is missing routing for
ReloadPrimaryStorageMsg so ReloadPrimaryStorageMsg will never be processed; add
a branch in the if/else chain inside handleLocalMessage() (near the existing
ReconnectPrimaryStorageMsg handling) to detect ReloadPrimaryStorageMsg and call
the existing handler by invoking handle((ReloadPrimaryStorageMsg) msg) (or
equivalent), ensuring the method signature handle(ReloadPrimaryStorageMsg) is
used and the message is routed to that handler.
Comment on lines
+749
to
+758
| logger.debug(String.format("failed to connect primary storage[uuid:%s], %s", self.getUuid(), errorCode)); | ||
|
|
||
| completion.fail(errorCode); | ||
| chain.next(); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| @Override | ||
| public String getName() { | ||
| return String.format("reconnect-primary-storage-%s", self.getUuid()); |
There was a problem hiding this comment.
日志消息和任务名称与操作不一致
复制自 doConnect 方法时未更新相关字符串：
- Line 749: 日志消息显示 "failed to connect" 但实际是 reload 操作
- Line 759: 任务名称为 "reconnect-primary-storage" 应为 "reload-primary-storage"
✏️ 修复建议
- logger.debug(String.format("failed to connect primary storage[uuid:%s], %s", self.getUuid(), errorCode));
+ logger.debug(String.format("failed to reload primary storage[uuid:%s], %s", self.getUuid(), errorCode));
completion.fail(errorCode);
chain.next();
}
});
}
`@Override`
public String getName() {
- return String.format("reconnect-primary-storage-%s", self.getUuid());
+ return String.format("reload-primary-storage-%s", self.getUuid());
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| logger.debug(String.format("failed to connect primary storage[uuid:%s], %s", self.getUuid(), errorCode)); | |
| completion.fail(errorCode); | |
| chain.next(); | |
| } | |
| }); | |
| } | |
| @Override | |
| public String getName() { | |
| return String.format("reconnect-primary-storage-%s", self.getUuid()); | |
| logger.debug(String.format("failed to reload primary storage[uuid:%s], %s", self.getUuid(), errorCode)); | |
| completion.fail(errorCode); | |
| chain.next(); | |
| } | |
| }); | |
| } | |
| `@Override` | |
| public String getName() { | |
| return String.format("reload-primary-storage-%s", self.getUuid()); | |
| } |
🤖 Prompt for AI Agents
In `@storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java`
around lines 749 - 759, Update the incorrect connect-related strings copied into
the reload flow in PrimaryStorageBase: change the logger message in the reload
handler (the debug call that currently says "failed to connect primary
storage[uuid:%s], %s") to reflect "failed to reload primary storage" and update
the anonymous task name returned by getName() (currently returning
"reconnect-primary-storage-%s") to "reload-primary-storage-%s" so both message
and task name correctly reference the reload operation in the doReload flow.
Comment on lines
1466
to
1485
| protected void handle(APIReloadPrimaryStorageMsg msg) { | ||
| final APIReconnectPrimaryStorageEvent evt = new APIReconnectPrimaryStorageEvent(msg.getId()); | ||
|
|
||
| ReloadPrimaryStorageMsg rmsg = new ReloadPrimaryStorageMsg(); | ||
| rmsg.setPrimaryStorageUuid(msg.getPrimaryStorageUuid()); | ||
| bus.makeTargetServiceIdByResourceUuid(rmsg, PrimaryStorageConstant.SERVICE_ID, rmsg.getPrimaryStorageUuid()); | ||
| bus.send(rmsg, new CloudBusCallBack(msg) { | ||
| @Override | ||
| public void run(MessageReply reply) { | ||
| if (!reply.isSuccess()) { | ||
| evt.setError(reply.getError()); | ||
| } else { | ||
| self = dbf.reload(self); | ||
| evt.setInventory(getSelfInventory()); | ||
| } | ||
|
|
||
| bus.publish(evt); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
严重错误：APIReloadPrimaryStorageMsg 缺少 API 消息路由
handle(APIReloadPrimaryStorageMsg) 方法已定义，但在 handleApiMessage() 方法（Lines 979-1007）中缺少对应的路由分支。API 消息将无法被正确分发处理。
🐛 修复建议：在 handleApiMessage() 中添加路由
在 handleApiMessage() 方法中（约 Line 989 附近，APIReconnectPrimaryStorageMsg 之后）添加：
} else if (msg instanceof APIReconnectPrimaryStorageMsg) {
handle((APIReconnectPrimaryStorageMsg) msg);
+ } else if (msg instanceof APIReloadPrimaryStorageMsg) {
+ handle((APIReloadPrimaryStorageMsg) msg);
} else if (msg instanceof APIUpdatePrimaryStorageMsg) {🤖 Prompt for AI Agents
In `@storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java`
around lines 1466 - 1485, The APIReloadPrimaryStorageMsg handler exists
(handle(APIReloadPrimaryStorageMsg)) but handleApiMessage() does not route that
API type, so reload requests never reach it; update handleApiMessage() to add a
branch similar to the APIReconnectPrimaryStorageMsg case that checks for
APIReloadPrimaryStorageMsg and dispatches it to handle(msg) (or calls
handle((APIReloadPrimaryStorageMsg)msg)) so the new handler is reachable and API
messages are properly routed.
27ab9ee to
d59a322
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java (1)
978-1006: 严重错误：APITakeoverPrimaryStorageMsg缺少 API 消息路由
handle(APITakeoverPrimaryStorageMsg)方法已定义（Lines 1465-1484），但在handleApiMessage()方法中缺少对应的路由分支。API 消息将无法被正确分发处理。🐛 修复建议：在 handleApiMessage() 中添加路由
在
handleApiMessage()方法中（约 Line 989 附近，APIReconnectPrimaryStorageMsg之后）添加：} else if (msg instanceof APIReconnectPrimaryStorageMsg) { handle((APIReconnectPrimaryStorageMsg) msg); + } else if (msg instanceof APITakeoverPrimaryStorageMsg) { + handle((APITakeoverPrimaryStorageMsg) msg); } else if (msg instanceof APIUpdatePrimaryStorageMsg) {
🤖 Fix all issues with AI agents
In `@storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java`:
- Around line 1465-1484: The API event type is incorrect: replace creation of
APIReconnectPrimaryStorageEvent with APITakeoverPrimaryStorageEvent so the
response matches the `@RestRequest` responseClass; specifically, in
handle(APITakeoverPrimaryStorageMsg msg) change the local variable declaration
from APIReconnectPrimaryStorageEvent evt = new
APIReconnectPrimaryStorageEvent(msg.getId()) to APITakeoverPrimaryStorageEvent
evt = new APITakeoverPrimaryStorageEvent(msg.getId()), leaving the rest of the
callback (bus.publish(evt), evt.setError(...), evt.setInventory(...), and use of
dbf.reload(self) / getSelfInventory()) unchanged.
- Around line 610-624: The TakeoverPrimaryStorageMsg handler
(handle(TakeoverPrimaryStorageMsg)) is implemented but never routed from
handleLocalMessage(), so takeover messages are ignored; update
handleLocalMessage() to branch on TakeoverPrimaryStorageMsg (insert after the
ReconnectPrimaryStorageMsg branch) and invoke the existing
handle(TakeoverPrimaryStorageMsg) method; ensure the new branch checks
instanceof TakeoverPrimaryStorageMsg and calls
handle((TakeoverPrimaryStorageMsg) msg) so doTakeover(...) executes as intended.
- Around line 718-761: doTakeover中调用了不存在的reloadHook并且若干字符串与任务名语义错误：将对 reloadHook
的调用替换为已定义的 takeoverHook(即在 doTakeover 的 ChainTask.run 内调用 takeoverHook(new
Completion(...)))，并更新成功/失败日志消息中 self.getUuid() 的文本分别为 "successfully takeover" 和
"failed to takeover"；同时把 ChainTask 的 getName() 返回值从
"reconnect-primary-storage-%s" 改为 "takeover-primary-storage-%s"；其余逻辑（使用
tracker.track(self.getUuid()), dbf.reload(self),
changeStatus(PrimaryStorageStatus.Disconnected),
fireDisconnectedCanonicalEvent(errorCode), completion.success()/fail() 等）保持不变。
storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java
Show resolved
Hide resolved
Comment on lines
718
to
761
| private void doTakeover(ConnectParam param, final Completion completion) { | ||
| thdf.chainSubmit(new ChainTask(completion) { | ||
| @Override | ||
| public String getSyncSignature() { | ||
| return getSyncId(); | ||
| } | ||
|
|
||
| @Override | ||
| public void run(SyncTaskChain chain) { | ||
| reloadHook(new Completion(chain, completion) { | ||
| @Override | ||
| public void success() { | ||
| self = dbf.reload(self); | ||
| logger.debug(String.format("successfully reload primary storage[uuid:%s]", self.getUuid())); | ||
|
|
||
| tracker.track(self.getUuid()); | ||
|
|
||
| completion.success(); | ||
| chain.next(); | ||
| } | ||
|
|
||
| @Override | ||
| public void fail(ErrorCode errorCode) { | ||
| tracker.track(self.getUuid()); | ||
|
|
||
| self = dbf.reload(self); | ||
| if (changeStatus(PrimaryStorageStatus.Disconnected) && !errorCode.isError(PrimaryStorageErrors.DISCONNECTED)) { | ||
| fireDisconnectedCanonicalEvent(errorCode); | ||
| } | ||
|
|
||
| logger.debug(String.format("failed to connect primary storage[uuid:%s], %s", self.getUuid(), errorCode)); | ||
|
|
||
| completion.fail(errorCode); | ||
| chain.next(); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| @Override | ||
| public String getName() { | ||
| return String.format("reconnect-primary-storage-%s", self.getUuid()); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# 查找 reloadHook 和 takeoverHook 方法定义
rg -n "reloadHook|takeoverHook" storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java -B 2 -A 5Repository: MatheMatrix/zstack
Length of output: 767
🏁 Script executed:
# 也查看 doTakeover 方法周围的代码和注解
sed -n '700,780p' storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.javaRepository: MatheMatrix/zstack
Length of output: 2920
🏁 Script executed:
# 查找这两个方法的所有定义（包括可能的abstract/protected定义）
ast-grep --pattern 'reloadHook($_) { $$$ }'Repository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
# 使用rg查找方法定义签名
rg -n "(void|protected|private|public).*(reloadHook|takeoverHook)" storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java -B 1 -A 2Repository: MatheMatrix/zstack
Length of output: 170
🏁 Script executed:
# 最后验证：确保没有继承或其他地方定义的 reloadHook
rg -n "reloadHook" storage/src/main/java/org/zstack/storage/primary/ -A 2Repository: MatheMatrix/zstack
Length of output: 429
doTakeover方法存在编译错误和语义不一致问题
-
编译错误 (Line 727): 调用了不存在的方法
reloadHook()。应该调用takeoverHook()，该方法已在第180行定义。 -
日志消息错误 (Lines 731, 748):
- Line 731: "successfully reload" 应为 "successfully takeover"
- Line 748: "failed to connect" 应为 "failed to takeover"
-
任务名称错误 (Line 758): "reconnect-primary-storage" 应为 "takeover-primary-storage"
✏️ 修复建议
`@Override`
public void run(SyncTaskChain chain) {
- reloadHook(new Completion(chain, completion) {
+ takeoverHook(new Completion(chain, completion) {
`@Override`
public void success() {
self = dbf.reload(self);
- logger.debug(String.format("successfully reload primary storage[uuid:%s]", self.getUuid()));
+ logger.debug(String.format("successfully takeover primary storage[uuid:%s]", self.getUuid()));
tracker.track(self.getUuid());
completion.success();
chain.next();
}
`@Override`
public void fail(ErrorCode errorCode) {
tracker.track(self.getUuid());
self = dbf.reload(self);
if (changeStatus(PrimaryStorageStatus.Disconnected) && !errorCode.isError(PrimaryStorageErrors.DISCONNECTED)) {
fireDisconnectedCanonicalEvent(errorCode);
}
- logger.debug(String.format("failed to connect primary storage[uuid:%s], %s", self.getUuid(), errorCode));
+ logger.debug(String.format("failed to takeover primary storage[uuid:%s], %s", self.getUuid(), errorCode));
completion.fail(errorCode);
chain.next();
}
});
}
`@Override`
public String getName() {
- return String.format("reconnect-primary-storage-%s", self.getUuid());
+ return String.format("takeover-primary-storage-%s", self.getUuid());
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private void doTakeover(ConnectParam param, final Completion completion) { | |
| thdf.chainSubmit(new ChainTask(completion) { | |
| @Override | |
| public String getSyncSignature() { | |
| return getSyncId(); | |
| } | |
| @Override | |
| public void run(SyncTaskChain chain) { | |
| reloadHook(new Completion(chain, completion) { | |
| @Override | |
| public void success() { | |
| self = dbf.reload(self); | |
| logger.debug(String.format("successfully reload primary storage[uuid:%s]", self.getUuid())); | |
| tracker.track(self.getUuid()); | |
| completion.success(); | |
| chain.next(); | |
| } | |
| @Override | |
| public void fail(ErrorCode errorCode) { | |
| tracker.track(self.getUuid()); | |
| self = dbf.reload(self); | |
| if (changeStatus(PrimaryStorageStatus.Disconnected) && !errorCode.isError(PrimaryStorageErrors.DISCONNECTED)) { | |
| fireDisconnectedCanonicalEvent(errorCode); | |
| } | |
| logger.debug(String.format("failed to connect primary storage[uuid:%s], %s", self.getUuid(), errorCode)); | |
| completion.fail(errorCode); | |
| chain.next(); | |
| } | |
| }); | |
| } | |
| @Override | |
| public String getName() { | |
| return String.format("reconnect-primary-storage-%s", self.getUuid()); | |
| } | |
| }); | |
| } | |
| private void doTakeover(ConnectParam param, final Completion completion) { | |
| thdf.chainSubmit(new ChainTask(completion) { | |
| `@Override` | |
| public String getSyncSignature() { | |
| return getSyncId(); | |
| } | |
| `@Override` | |
| public void run(SyncTaskChain chain) { | |
| takeoverHook(new Completion(chain, completion) { | |
| `@Override` | |
| public void success() { | |
| self = dbf.reload(self); | |
| logger.debug(String.format("successfully takeover primary storage[uuid:%s]", self.getUuid())); | |
| tracker.track(self.getUuid()); | |
| completion.success(); | |
| chain.next(); | |
| } | |
| `@Override` | |
| public void fail(ErrorCode errorCode) { | |
| tracker.track(self.getUuid()); | |
| self = dbf.reload(self); | |
| if (changeStatus(PrimaryStorageStatus.Disconnected) && !errorCode.isError(PrimaryStorageErrors.DISCONNECTED)) { | |
| fireDisconnectedCanonicalEvent(errorCode); | |
| } | |
| logger.debug(String.format("failed to takeover primary storage[uuid:%s], %s", self.getUuid(), errorCode)); | |
| completion.fail(errorCode); | |
| chain.next(); | |
| } | |
| }); | |
| } | |
| `@Override` | |
| public String getName() { | |
| return String.format("takeover-primary-storage-%s", self.getUuid()); | |
| } | |
| }); | |
| } |
🤖 Prompt for AI Agents
In `@storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java`
around lines 718 - 761, doTakeover中调用了不存在的reloadHook并且若干字符串与任务名语义错误：将对
reloadHook 的调用替换为已定义的 takeoverHook(即在 doTakeover 的 ChainTask.run 内调用
takeoverHook(new Completion(...)))，并更新成功/失败日志消息中 self.getUuid() 的文本分别为
"successfully takeover" 和 "failed to takeover"；同时把 ChainTask 的 getName() 返回值从
"reconnect-primary-storage-%s" 改为 "takeover-primary-storage-%s"；其余逻辑（使用
tracker.track(self.getUuid()), dbf.reload(self),
changeStatus(PrimaryStorageStatus.Disconnected),
fireDisconnectedCanonicalEvent(errorCode), completion.success()/fail() 等）保持不变。
storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java
Show resolved
Hide resolved
APIImpact Resolves: ZSV-1 Change-Id: I70637377776e777070676c6a6c616e74786b6667
MatheMatrix
force-pushed
the
sync/tao.gan/res-sblk-ZSV-1@@4
branch
from
d59a322 to
f09a837
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java (1)
978-1003: API 消息未路由 APITakeoverPrimaryStorageMsg。
handleApiMessage 缺少分支会导致 API 无法触达 handle(APITakeoverPrimaryStorageMsg)。🐛 建议修复
} else if (msg instanceof APIReconnectPrimaryStorageMsg) { handle((APIReconnectPrimaryStorageMsg) msg); + } else if (msg instanceof APITakeoverPrimaryStorageMsg) { + handle((APITakeoverPrimaryStorageMsg) msg); } else if (msg instanceof APIUpdatePrimaryStorageMsg) {
♻️ Duplicate comments (2)
storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java (2)
371-377: 本地消息未路由 TakeoverPrimaryStorageMsg。
handleLocalMessage 缺少分支会导致 takeover 请求无法进入处理逻辑。🐛 建议修复
} else if (msg instanceof ReconnectPrimaryStorageMsg) { handle((ReconnectPrimaryStorageMsg) msg); + } else if (msg instanceof TakeoverPrimaryStorageMsg) { + handle((TakeoverPrimaryStorageMsg) msg); } else if (msg instanceof RevertVolumeFromSnapshotOnPrimaryStorageMsg) {
731-759: takeover 日志/任务名仍沿用 reload/connect 语义。
会误导排障与任务追踪。✏️ 建议修复
- logger.debug(String.format("successfully reload primary storage[uuid:%s]", self.getUuid())); + logger.debug(String.format("successfully takeover primary storage[uuid:%s]", self.getUuid())); ... - logger.debug(String.format("failed to connect primary storage[uuid:%s], %s", self.getUuid(), errorCode)); + logger.debug(String.format("failed to takeover primary storage[uuid:%s], %s", self.getUuid(), errorCode)); ... - return String.format("reconnect-primary-storage-%s", self.getUuid()); + return String.format("takeover-primary-storage-%s", self.getUuid());
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.
APIImpact
Resolves: ZSV-1
Change-Id: I70637377776e777070676c6a6c616e74786b6667
sync from gitlab !9027