Skip to content

<fix>[compute]: <description#3699

Open
zstack-robot-2 wants to merge 1 commit intofeature-zsv-5.0.0-vm-support-vtpm-and-secucebootfrom
sync/tao.gan/ZSV-11714
Open

<fix>[compute]: <description#3699
zstack-robot-2 wants to merge 1 commit intofeature-zsv-5.0.0-vm-support-vtpm-and-secucebootfrom
sync/tao.gan/ZSV-11714

Conversation

@zstack-robot-2
Copy link
Copy Markdown
Collaborator

Resolves: ZSV-11714

Change-Id: I6b6961746e6d7a69776a6d7962746164766a7573

sync from gitlab !9562

Resolves: ZSV-11714

Change-Id: I6b6961746e6d7a69776a6d7962746164766a7573
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 8, 2026

概览

HostManagerImpl.javasetupCanonicalEvents() 方法中,针对主存储主机状态变更事件的处理逻辑已被完全注释掉。该事件处理程序曾负责检测主机至断开连接状态的转换,并通过总线发送相应消息。现已禁用此事件监听器的全部功能。

变更

内聚体 / 文件 摘要
事件处理器禁用
compute/src/main/java/org/zstack/compute/host/HostManagerImpl.java
注释掉 PRIMARY_STORAGE_HOST_STATUS_CHANGED_PATH 事件的完整处理逻辑,包括状态检查、消息发送和事件触发的代码行。

代码审查工作量评估

🎯 2 (简单) | ⏱️ ~10 分钟

🐰✨ 事件处理成沉睡,

注释行行守安宁,

主机状态不再鸣,

代码优雅地静默,

只等下一个春天醒!

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning PR标题'[compute]: <description'不完整,包含占位符'<description',未清晰描述主要变更内容。 补全标题,明确描述注释掉PRIMARY_STORAGE_HOST_STATUS_CHANGED_PATH事件处理器的原因和目的。
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed PR描述提供了问题编号(ZSV-11714)和变更ID,与代码变更相关联,满足最低要求。

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch sync/tao.gan/ZSV-11714

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@compute/src/main/java/org/zstack/compute/host/HostManagerImpl.java`:
- Around line 890-905: Restore the commented "Disconnected" branch in
HostManagerImpl: re-enable the
PrimaryStorageCanonicalEvent.PrimaryStorageHostStatusChangeData handling that
checks if d.getNewStatus() == PrimaryStorageHostStatus.Disconnected &&
d.getOldStatus() != PrimaryStorageHostStatus.Disconnected &&
noStorageAccessible(d.getHostUuid()), then build and send a
ChangeHostConnectionStateMsg (setHostUuid,
setConnectionStateEvent(HostStatusEvent.disconnected.toString()), setCause),
call bus.makeTargetServiceIdByResourceUuid(msg, HostConstant.SERVICE_ID,
d.getHostUuid()) and bus.send(msg), and fire new
HostBase.HostDisconnectedCanonicalEvent(d.getHostUuid(), operr(...)) so hosts
with no accessible primary storage are marked disconnected and the
HostDisconnectedCanonicalEvent is emitted.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: ca3842c2-3781-4241-9fbe-80220406abbb

📥 Commits

Reviewing files that changed from the base of the PR and between 167977d and d945007.

📒 Files selected for processing (1)
  • compute/src/main/java/org/zstack/compute/host/HostManagerImpl.java

Comment on lines +890 to +905
// PrimaryStorageCanonicalEvent.PrimaryStorageHostStatusChangeData d =
// (PrimaryStorageCanonicalEvent.PrimaryStorageHostStatusChangeData)data;
// if (d.getNewStatus() == PrimaryStorageHostStatus.Disconnected &&
// d.getOldStatus() != PrimaryStorageHostStatus.Disconnected &&
// noStorageAccessible(d.getHostUuid())){
// ChangeHostConnectionStateMsg msg = new ChangeHostConnectionStateMsg();
// msg.setHostUuid(d.getHostUuid());
// msg.setConnectionStateEvent(HostStatusEvent.disconnected.toString());
// msg.setCause("base cause: host disconnected from other status and has no connected primary storage attached");
// bus.makeTargetServiceIdByResourceUuid(msg, HostConstant.SERVICE_ID, d.getHostUuid());
// bus.send(msg);
//
// new HostBase.HostDisconnectedCanonicalEvent(d.getHostUuid(),
// operr("primary storage[uuid:%s] becomes disconnected, the host has no connected primary storage attached",
// d.getPrimaryStorageUuid())).fire();
// }
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

恢复主存断连后的 Host 断连处理链路

Line 890-905 把 Disconnected 分支整段注释后,这个 canonical event 回调变成空实现;当前没有其他监听器补上“无可达主存时置 Host 为 disconnected + 发出 HostDisconnectedCanonicalEvent”的逻辑,属于功能回退。

建议的最小修复(恢复原处理分支)
         evtf.on(PrimaryStorageCanonicalEvent.PRIMARY_STORAGE_HOST_STATUS_CHANGED_PATH, new EventCallback() {
             `@Override`
             protected void run(Map tokens, Object data) {
-//                PrimaryStorageCanonicalEvent.PrimaryStorageHostStatusChangeData d =
-//                        (PrimaryStorageCanonicalEvent.PrimaryStorageHostStatusChangeData)data;
-//                if (d.getNewStatus() == PrimaryStorageHostStatus.Disconnected &&
-//                        d.getOldStatus() != PrimaryStorageHostStatus.Disconnected &&
-//                        noStorageAccessible(d.getHostUuid())){
-//                    ChangeHostConnectionStateMsg msg = new ChangeHostConnectionStateMsg();
-//                    msg.setHostUuid(d.getHostUuid());
-//                    msg.setConnectionStateEvent(HostStatusEvent.disconnected.toString());
-//                    msg.setCause("base cause: host disconnected from other status and has no connected primary storage attached");
-//                    bus.makeTargetServiceIdByResourceUuid(msg, HostConstant.SERVICE_ID, d.getHostUuid());
-//                    bus.send(msg);
-//
-//                    new HostBase.HostDisconnectedCanonicalEvent(d.getHostUuid(),
-//                            operr("primary storage[uuid:%s] becomes disconnected, the host has no connected primary storage attached",
-//                                    d.getPrimaryStorageUuid())).fire();
-//                }
+                PrimaryStorageCanonicalEvent.PrimaryStorageHostStatusChangeData d =
+                        (PrimaryStorageCanonicalEvent.PrimaryStorageHostStatusChangeData) data;
+                if (d.getNewStatus() == PrimaryStorageHostStatus.Disconnected &&
+                        d.getOldStatus() != PrimaryStorageHostStatus.Disconnected &&
+                        noStorageAccessible(d.getHostUuid())) {
+                    ChangeHostConnectionStateMsg msg = new ChangeHostConnectionStateMsg();
+                    msg.setHostUuid(d.getHostUuid());
+                    msg.setConnectionStateEvent(HostStatusEvent.disconnected.toString());
+                    msg.setCause("base cause: host disconnected from other status and has no connected primary storage attached");
+                    bus.makeTargetServiceIdByResourceUuid(msg, HostConstant.SERVICE_ID, d.getHostUuid());
+                    bus.send(msg);
+
+                    new HostBase.HostDisconnectedCanonicalEvent(
+                            d.getHostUuid(),
+                            operr("primary storage[uuid:%s] becomes disconnected, the host has no connected primary storage attached",
+                                    d.getPrimaryStorageUuid())
+                    ).fire();
+                }
             }
         });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@compute/src/main/java/org/zstack/compute/host/HostManagerImpl.java` around
lines 890 - 905, Restore the commented "Disconnected" branch in HostManagerImpl:
re-enable the PrimaryStorageCanonicalEvent.PrimaryStorageHostStatusChangeData
handling that checks if d.getNewStatus() ==
PrimaryStorageHostStatus.Disconnected && d.getOldStatus() !=
PrimaryStorageHostStatus.Disconnected && noStorageAccessible(d.getHostUuid()),
then build and send a ChangeHostConnectionStateMsg (setHostUuid,
setConnectionStateEvent(HostStatusEvent.disconnected.toString()), setCause),
call bus.makeTargetServiceIdByResourceUuid(msg, HostConstant.SERVICE_ID,
d.getHostUuid()) and bus.send(msg), and fire new
HostBase.HostDisconnectedCanonicalEvent(d.getHostUuid(), operr(...)) so hosts
with no accessible primary storage are marked disconnected and the
HostDisconnectedCanonicalEvent is emitted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants