WIP <fix>[cloudbus]: remove some db op #3202
Open
+66
−12
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在 KVM 主机连接流程中查询并注入集群的附带主存储类型至连接上下文；存储插件据此短路或继续初始化；调整事件处理的 glob 检测与匹配并临时禁用 webhook 调用；若干工厂/方法签名与辅助方法新增或调整。 Changes
Sequence Diagram(s)sequenceDiagram
participant KVM as KVMHost.continueConnect
participant DB as Database
participant Ctx as KVMHostConnectingFlowContext[]
participant Ext as StorageExtension (Ceph/NFS/External/SMP)
KVM->>DB: 查询集群的附加主存储类型
DB-->>KVM: 返回 Set<String> attachedPsTypes
KVM->>Ctx: 为每个 context 设置 attachedPrimaryStorageTypes(attachedPsTypes)
loop 对每个 Extension 调用
KVM->>Ext: createKvmHostConnectingFlow(context)
alt attachedPsTypes 包含 extension 类型
Ext-->>KVM: 返回 初始化 Flow（例如 NoRollbackFlow）
else 不包含
Ext-->>KVM: 返回 NopeFlow（短路）
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 分钟 诗
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (1 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml) Review profile: CHILL Plan: Pro 📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (5)
🧰 Additional context used📓 Path-based instructions (2)**/*.*⚙️ CodeRabbit configuration file
Files:
**/*.java⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (15)📓 Common learnings📚 Learning: 2025-08-14T06:56:19.585ZApplied to files:
📚 Learning: 2025-10-20T11:27:25.928ZApplied to files:
📚 Learning: 2025-10-23T07:15:36.501ZApplied to files:
📚 Learning: 2025-08-12T05:39:14.846ZApplied to files:
📚 Learning: 2025-08-03T04:10:21.683ZApplied to files:
📚 Learning: 2025-08-25T03:52:37.301ZApplied to files:
📚 Learning: 2025-10-28T02:29:38.803ZApplied to files:
📚 Learning: 2025-09-05T10:14:54.816ZApplied to files:
📚 Learning: 2025-10-24T06:45:56.228ZApplied to files:
📚 Learning: 2025-08-12T05:52:18.323ZApplied to files:
📚 Learning: 2025-07-22T02:30:46.123ZApplied to files:
📚 Learning: 2025-08-04T03:35:51.225ZApplied to files:
📚 Learning: 2025-07-22T02:30:46.123ZApplied to files:
📚 Learning: 2025-08-25T03:55:07.988ZApplied to files:
🧬 Code graph analysis (1)plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java (1)
🔍 Remote MCP AtlassianMCPRelevant Jira context (QI-1170)
Attachments and traces cited in the issue (profiles, images, and links to source MRs) are present in the Jira comments and were used as motivation for these code changes. 🔇 Additional comments (5)
✏️ Tip: You can disable this entire section by setting 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.5)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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/src/main/java/org/zstack/core/cloudbus/EventFacadeImpl.java (1)
280-284: 严重缺陷：handleEvent方法未处理glob为 null 的情况，会导致 NPE
CallbackWrapper构造函数现在只在路径是 glob 模式时才初始化glob字段。fireLocal方法已更新以处理glob == null的情况，但handleEvent方法没有同步更新。当通过
on()方法注册非 glob 路径时，w.getGlob()返回 null，String.matches(null)将抛出NullPointerException。🐛 修复 NPE 问题
for (CallbackWrapper w : wrappers.values()) { - if (cevt.getPath().matches(w.getGlob())) { + boolean match = w.getGlob() == null ? cevt.getPath().equals(w.path) : cevt.getPath().matches(w.getGlob()); + if (match) { w.call(cevt); } }
🤖 Fix all issues with AI agents
In `@core/src/main/java/org/zstack/core/cloudbus/EventFacadeImpl.java`:
- Line 232: The commented-out call to callWebhooks(evt) in EventFacadeImpl
silently changes behavior; update the code to either (A) restore a documented
temporary disable by replacing the commented line with a clear TODO comment
above it referencing the issue/PR (include issue ID or URL), rationale, expected
re-enable conditions, owner and ETA, or (B) if the webhook removal is permanent,
remove the unused callWebhooks(...) method and any related dead code; ensure
references to callWebhooks(evt) and the callWebhooks method implementation are
handled consistently so no dead/undocumented behavior remains.
In
`@plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephKvmExtension.java`:
- Around line 132-135: The current check in CephKvmExtension uses
context.getAttachedPrimaryStorageTypes().contains(CephConstants.CEPH_PRIMARY_STORAGE_TYPE)
which can NPE if getAttachedPrimaryStorageTypes() is null; update the
conditional in CephKvmExtension to first guard against a null collection (e.g.,
check getAttachedPrimaryStorageTypes() == null or use a safe-empty/default)
before calling contains and return new NopeFlow() when the collection is null or
does not contain CephConstants.CEPH_PRIMARY_STORAGE_TYPE; reference
getAttachedPrimaryStorageTypes(), CephConstants.CEPH_PRIMARY_STORAGE_TYPE,
CephKvmExtension and NopeFlow when locating the change.
In
`@plugin/externalStorage/src/main/java/org/zstack/externalStorage/primary/kvm/ExternalPrimaryStorageKvmFactory.java`:
- Around line 83-86: The null check for context.getAttachedPrimaryStorageTypes()
is missing and may throw NPE; update ExternalPrimaryStorageKvmFactory so the
early-return checks for null first (e.g., in the same method/block that
currently calls context.getAttachedPrimaryStorageTypes().contains(...)) and
return new NopeFlow() when the collection is null or does not contain
PrimaryStorageConstant.EXTERNAL_PRIMARY_STORAGE_TYPE; mirror the null-safe check
pattern used in KvmFactory.java.
- Line 33: Remove the unused import ExternalPrimaryStorageConstants from
ExternalPrimaryStorageKvmFactory and ensure the existing type check uses the
correct symbol PrimaryStorageConstant.EXTERNAL_PRIMARY_STORAGE_TYPE;
specifically, delete the line importing
org.zstack.storage.addon.primary.ExternalPrimaryStorageConstants and retain the
existing logic that references
PrimaryStorageConstant.EXTERNAL_PRIMARY_STORAGE_TYPE in the
ExternalPrimaryStorageKvmFactory class.
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java`:
- Around line 1032-1035: The getHostContextByUuid method is dead/unsafe:
dbf.findByUuid(hostUuid, KVMHostVO.class) can return null and passing that into
createHostContext will cause a NullPointerException; either remove
getHostContextByUuid if unused or make it safe by checking the KVMHostVO result
before calling createHostContext (e.g., if dbf.findByUuid(...) == null return
null or throw a descriptive exception) and ensure callers of
getHostContextByUuid handle the null/exception; reference the method name
getHostContextByUuid, the dbf.findByUuid call, the KVMHostVO result, and
createHostContext in your change.
In
`@storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorageFactory.java`:
- Line 4: Remove the unused import
org.springframework.security.core.parameters.P from
ExternalPrimaryStorageFactory; locate the import statement at the top of the
class and delete it so the file no longer references that Spring Security
annotation which isn't used in any methods or fields.
🧹 Nitpick comments (2)
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java (1)
17-18:ExternalPrimaryStorageVOimport 可能不需要
如果按上面建议把SQL.New(..., ExternalPrimaryStorageVO.class)改为String.class，这里的 import 大概率应删除，避免误导/残留依赖。plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java (1)
244-248: 存在无法触及的死代码由于
getHostContext(KVMHostVO kvo)方法（第 1028-1030 行）现在始终返回createHostContext(kvo)的结果，而createHostContext方法永远不会返回 null，因此第 245-247 行的 null 检查永远不会为真，已成为死代码。建议移除这段冗余的条件判断：
♻️ 建议的修改
`@Override` public Host getHost(HostVO vo) { KVMHostVO kvo = dbf.findByUuid(vo.getUuid(), KVMHostVO.class); - KVMHostContext context = getHostContext(kvo); - if (context == null) { - context = createHostContext(kvo); - } + KVMHostContext context = getHostContext(kvo); return new KVMHost(kvo, context); }
📜 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 (9)
core/src/main/java/org/zstack/core/cloudbus/EventFacadeImpl.javaplugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephKvmExtension.javaplugin/externalStorage/src/main/java/org/zstack/externalStorage/primary/kvm/ExternalPrimaryStorageKvmFactory.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHostConnectedContext.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.javaplugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/KvmFactory.javastorage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorageFactory.java
🧰 Additional context used
📓 Path-based instructions (2)
**/*.*
⚙️ CodeRabbit configuration file
**/*.*: - 代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写
Files:
plugin/externalStorage/src/main/java/org/zstack/externalStorage/primary/kvm/ExternalPrimaryStorageKvmFactory.javastorage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorageFactory.javaplugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/KvmFactory.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHostConnectedContext.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaplugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephKvmExtension.javacore/src/main/java/org/zstack/core/cloudbus/EventFacadeImpl.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java
**/*.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:
plugin/externalStorage/src/main/java/org/zstack/externalStorage/primary/kvm/ExternalPrimaryStorageKvmFactory.javastorage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorageFactory.javaplugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/KvmFactory.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHostConnectedContext.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaplugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephKvmExtension.javacore/src/main/java/org/zstack/core/cloudbus/EventFacadeImpl.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java
🧠 Learnings (8)
📚 Learning: 2025-08-25T03:52:37.301Z
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 2504
File: storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotTreeBase.java:775-819
Timestamp: 2025-08-25T03:52:37.301Z
Learning: 在ZStack项目的VolumeSnapshotTreeBase类中，当通过dbf.findByUuid()方法获取VolumeVO对象时，需要进行null检查，因为该方法可能在找不到对应记录时返回null，直接使用可能导致NullPointerException。
Applied to files:
plugin/externalStorage/src/main/java/org/zstack/externalStorage/primary/kvm/ExternalPrimaryStorageKvmFactory.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:
plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.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:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHostConnectedContext.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java
📚 Learning: 2025-10-23T07:15:36.501Z
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 2796
File: plugin/kvm/src/main/java/org/zstack/kvm/KVMHostUtils.java:50-53
Timestamp: 2025-10-23T07:15:36.501Z
Learning: In the KVMHostUtils.getNormalizedBridgeName method in plugin/kvm/src/main/java/org/zstack/kvm/KVMHostUtils.java, the second check for L2_BRIDGE_NAME tag (after checking for name conflict) is intentional to handle concurrency issues. This double-check pattern prevents multiple threads from generating different bridge names for the same l2Uuid when they execute concurrently.
Applied to files:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java
📚 Learning: 2025-08-12T05:39:14.846Z
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 2418
File: compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java:3856-3865
Timestamp: 2025-08-12T05:39:14.846Z
Learning: 在 compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java 的 setNoIpAllocationStaticIp 流程中，从数据库加载的 VmNicVO 实例其 getUsedIps() 为空时也不会为 null（返回空集合）。因此无需对 getUsedIps() 再做 Optional/空列表归一化的空指针保护；若找不到对应 NIC，使用 orElse(new VmNicVO()) 的约定允许后续逻辑通过 NPE 暴露问题，与项目既有约定一致。
Applied to files:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
📚 Learning: 2025-08-12T05:52:18.323Z
Learnt from: MatheMatrix
Repo: MatheMatrix/zstack PR: 2418
File: compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java:3873-3885
Timestamp: 2025-08-12T05:52:18.323Z
Learning: 在 compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java 的 setNoIpAllocationStaticIp() 流程中，静态 IP 标签删除已统一在 change-ip-in-database 步骤对 voRemoveSet 循环处理：对收集到的旧 IPv4/IPv6 UsedIpVO 均调用 ipOperator.deleteStaticIpByVmUuidAndL3Uuid(vmUuid, l3Uuid, IPv6NetworkUtils.ipv6AddessToTagValue(ip)) 清理对应的 static-ip 标签。因此无需在分支里分别删除 IPv4/IPv6 的标签。
Applied to files:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
📚 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:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java
📚 Learning: 2025-08-04T03:35:51.225Z
Learnt from: zstack-robot-2
Repo: MatheMatrix/zstack PR: 2360
File: compute/src/main/java/org/zstack/compute/vm/VmNicParamBuilder.java:27-28
Timestamp: 2025-08-04T03:35:51.225Z
Learning: 在 VmNicParamBuilder.buildByVmUuid 方法中，上层调用保证传入的 VM UUID 对应的实例一定存在，因此使用 Q.New(VmInstanceVO.class).eq(VmInstanceVO_.uuid, vmInstanceUuid).find() 查询后不需要进行 null 检查。
Applied to files:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java
🧬 Code graph analysis (2)
plugin/externalStorage/src/main/java/org/zstack/externalStorage/primary/kvm/ExternalPrimaryStorageKvmFactory.java (1)
header/src/main/java/org/zstack/header/storage/primary/PrimaryStorageHistoricalUsageBaseVO.java (1)
org(11-45)
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java (1)
header/src/main/java/org/zstack/header/storage/primary/PrimaryStorageHistoricalUsageBaseVO.java (1)
org(11-45)
🔍 Remote MCP AtlassianMCP
Perfect! I now have comprehensive information about the PR and the related Jira issue. Let me compile the findings.
Additional Context Summary
Background - Issue QI-1170: ZStack Scalability Assessment and Optimization
The PR is related to issue QI-1170 which focuses on assessing single ZStack instance's management capacity for physical machines and corresponding resource/service quantities, with optimization targets of managing 1000 physical machines by end of December.
Performance Optimization Context:
The issue identified multiple performance bottlenecks in a large-scale environment, including the Prometheus host hardware status monitor consuming excessive thread pool capacity when monitoring 2000+ physical machines. Testing showed that after removing the host hardware status monitor, the system could manage up to 10,000 simulator hosts.
Key Optimizations Being Implemented:
-
Host Connection Performance: Adding a host existence check after HTTP callback completion was causing performance degradation and needs to be replaced with an event-based mechanism (currently being removed).
-
Webhook Query Optimization: Webhook dispatch was performing a SQL query every time (fetching WebhookVO list), which should be cached and converted to an event-based mechanism (currently being removed as a temporary fix).
-
Glob Pattern Memory Optimization: Memory profiling showed that event glob pattern matching data structures consumed 20% of heap memory; optimization identified that only global config events require glob pattern support, enabling conditional initialization of glob patterns.
-
Primary Storage Plugin Efficiency: The issue highlighted that storage plugins should not be loaded if their corresponding storage type is not present in the environment, and unnecessary SQL queries for checking storage availability should be eliminated by checking attached primary storage types in-memory first.
Referenced Merge Request Changes:
The issue references GitLab merge request 9021 which contains the optimization changes being ported to this PR.
Alignment with PR Changes
The code modifications in this PR directly implement the optimization strategies identified in QI-1170:
- EventFacadeImpl.java: Implements glob pattern conditional initialization to reduce memory consumption
- KVMHost.java & KVMHostConnectingFlow Context changes: Pass attached primary storage type information from the host's cluster to avoid redundant database queries
- Storage plugin files (Ceph, NFS, External, SMP): Add guards to skip flow initialization when respective storage types are not attached
- KVMHostFactory.java: Refactors method signatures to work with KVMHostVO objects directly rather than UUIDs, supporting more efficient context retrieval
🔇 Additional comments (8)
core/src/main/java/org/zstack/core/cloudbus/EventFacadeImpl.java (2)
79-88: LGTM! Glob 模式的条件初始化优化合理此优化通过仅在路径包含 glob 字符时才创建正则表达式，有效减少了非 glob 路径的内存开销。
isGlobPath检查*、?和{是合理的，因为{用于 token 模式（如{uuid}），最终也会被转换为正则表达式。Also applies to: 132-134
258-263: LGTM! 本地事件匹配逻辑更新正确对于非 glob 路径使用精确匹配（
equals），对于 glob 路径使用正则匹配，这与CallbackWrapper中的条件初始化逻辑保持一致，且精确匹配比正则匹配更高效。storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorageFactory.java (1)
269-271: LGTM!新增的
getAllControllerSvcs()方法实现正确，通过返回新的ArrayList进行防御性拷贝，避免外部直接修改内部的controllers集合。plugin/kvm/src/main/java/org/zstack/kvm/KVMHostConnectedContext.java (1)
12-12: LGTM!新增的
attachedPrimaryStorageTypes字段及其访问器方法实现清晰，用于在 KVM 主机连接流程中传递已挂载的主存储类型信息，支持各存储扩展进行快速类型检查，避免不必要的数据库查询。Also applies to: 46-52
plugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/KvmFactory.java (1)
62-65: 评论是错误的。
context.getAttachedPrimaryStorageTypes()不会返回null。在KVMHost.java的第 5193 行，attachedPsTypes被初始化为new HashSet(...)。即使 SQL 查询返回空结果集，new HashSet()也会创建一个空的 HashSet 对象而不是null。该对象随后被传递给ctx.setAttachedPrimaryStorageTypes(attachedPsTypes)，因此 getter 方法总是返回一个有效的 Set 对象。这一设计与代码库中其他存储扩展的实现一致（如 NFS、Ceph、External Storage），它们都使用相同的模式而不添加空值检查。
Likely an incorrect or invalid review comment.
plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.java (1)
1743-1745: 优化逻辑正确，无需添加空值检查。该优化正确地跳过了未附加 NFS 存储时的不必要的重新挂载流程。
getAttachedPrimaryStorageTypes()保证返回非空集合（由 KVMHost.java 中的new HashSet(SQL.New(...).list())初始化），因此无需进行空值防护。此模式与 SMP、Ceph 等其他存储插件保持一致。plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java (1)
510-516: 去掉 host existence 检查会改变语义：请确认“host 删除后仍回调 success”是安全且预期的
现在commandStr != null分支不再检查 host 是否存在，但cmd分支仍保留检查；两条路径行为不一致，且注释掉的代码建议清理掉或用明确的开关/说明替代。建议至少补充一次验证：在 host 删除/被接管的场景下，确认该回调继续
completion.success(ret)不会触发后续对已删除 host 的 DB 写/任务投递/NPE（例如 connect 流程、ping/reconnect、host queue 等）。plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java (1)
1028-1030: LGTM!方法签名的重构使得调用方可以直接传入已有的
KVMHostVO对象，有效避免了冗余的数据库查询操作，符合 QI-1170 的性能优化目标。
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| fireLocal(evt); | ||
|
|
||
| callWebhooks(evt); | ||
| // callWebhooks(evt); |
There was a problem hiding this comment.
Webhook 功能被静默禁用，缺少必要的注释说明
直接注释掉 callWebhooks(evt) 会导致事件触发时不再调用 webhook，这是一个行为变更。根据 PR 上下文，这是为了优化 webhook 查询性能的临时措施，但代码中没有任何说明：
- 为什么禁用此功能
- 何时/如何恢复
- 对用户的影响
建议添加 TODO 注释并关联 issue，或者如果这是永久性移除，应删除 callWebhooks 方法以避免死代码。
- // callWebhooks(evt);
+ // TODO(QI-1170): Webhook dispatch is temporarily disabled for performance optimization.
+ // Will be replaced with cached/event-based mechanism in a future PR.
+ // callWebhooks(evt);🤖 Prompt for AI Agents
In `@core/src/main/java/org/zstack/core/cloudbus/EventFacadeImpl.java` at line
232, The commented-out call to callWebhooks(evt) in EventFacadeImpl silently
changes behavior; update the code to either (A) restore a documented temporary
disable by replacing the commented line with a clear TODO comment above it
referencing the issue/PR (include issue ID or URL), rationale, expected
re-enable conditions, owner and ETA, or (B) if the webhook removal is permanent,
remove the unused callWebhooks(...) method and any related dead code; ensure
references to callWebhooks(evt) and the callWebhooks method implementation are
handled consistently so no dead/undocumented behavior remains.
Comment on lines
+132
to
+135
| if (!context.getAttachedPrimaryStorageTypes().contains(CephConstants.CEPH_PRIMARY_STORAGE_TYPE)) { | ||
| return new NopeFlow(); | ||
| } | ||
|
|
There was a problem hiding this comment.
潜在的空指针异常风险，与其他存储扩展存在相同问题。
与 KvmFactory.java 和 ExternalPrimaryStorageKvmFactory.java 类似，需要对 getAttachedPrimaryStorageTypes() 进行空值检查。
🔧 建议修复
+ if (context.getAttachedPrimaryStorageTypes() == null ||
+ !context.getAttachedPrimaryStorageTypes().contains(CephConstants.CEPH_PRIMARY_STORAGE_TYPE)) {
- if (!context.getAttachedPrimaryStorageTypes().contains(CephConstants.CEPH_PRIMARY_STORAGE_TYPE)) {
return new NopeFlow();
}🤖 Prompt for AI Agents
In
`@plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephKvmExtension.java`
around lines 132 - 135, The current check in CephKvmExtension uses
context.getAttachedPrimaryStorageTypes().contains(CephConstants.CEPH_PRIMARY_STORAGE_TYPE)
which can NPE if getAttachedPrimaryStorageTypes() is null; update the
conditional in CephKvmExtension to first guard against a null collection (e.g.,
check getAttachedPrimaryStorageTypes() == null or use a safe-empty/default)
before calling contains and return new NopeFlow() when the collection is null or
does not contain CephConstants.CEPH_PRIMARY_STORAGE_TYPE; reference
getAttachedPrimaryStorageTypes(), CephConstants.CEPH_PRIMARY_STORAGE_TYPE,
CephKvmExtension and NopeFlow when locating the change.
| import org.zstack.header.volume.VolumeVO; | ||
| import org.zstack.header.volume.VolumeVO_; | ||
| import org.zstack.kvm.*; | ||
| import org.zstack.storage.addon.primary.ExternalPrimaryStorageConstants; |
There was a problem hiding this comment.
删除未使用的导入。
ExternalPrimaryStorageConstants 被导入但未在代码中使用。第 83 行的类型检查使用的是 PrimaryStorageConstant.EXTERNAL_PRIMARY_STORAGE_TYPE。
🔧 建议修复
-import org.zstack.storage.addon.primary.ExternalPrimaryStorageConstants;📝 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
| import org.zstack.storage.addon.primary.ExternalPrimaryStorageConstants; |
🤖 Prompt for AI Agents
In
`@plugin/externalStorage/src/main/java/org/zstack/externalStorage/primary/kvm/ExternalPrimaryStorageKvmFactory.java`
at line 33, Remove the unused import ExternalPrimaryStorageConstants from
ExternalPrimaryStorageKvmFactory and ensure the existing type check uses the
correct symbol PrimaryStorageConstant.EXTERNAL_PRIMARY_STORAGE_TYPE;
specifically, delete the line importing
org.zstack.storage.addon.primary.ExternalPrimaryStorageConstants and retain the
existing logic that references
PrimaryStorageConstant.EXTERNAL_PRIMARY_STORAGE_TYPE in the
ExternalPrimaryStorageKvmFactory class.
Comment on lines
+83
to
+86
| if (!context.getAttachedPrimaryStorageTypes().contains(PrimaryStorageConstant.EXTERNAL_PRIMARY_STORAGE_TYPE)) { | ||
| return new NopeFlow(); | ||
| } | ||
|
|
There was a problem hiding this comment.
潜在的空指针异常风险，与其他存储扩展存在相同问题。
与 KvmFactory.java 类似，如果 context.getAttachedPrimaryStorageTypes() 返回 null，将抛出 NullPointerException。
🔧 建议修复
+ if (context.getAttachedPrimaryStorageTypes() == null ||
+ !context.getAttachedPrimaryStorageTypes().contains(PrimaryStorageConstant.EXTERNAL_PRIMARY_STORAGE_TYPE)) {
- if (!context.getAttachedPrimaryStorageTypes().contains(PrimaryStorageConstant.EXTERNAL_PRIMARY_STORAGE_TYPE)) {
return new NopeFlow();
}📝 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
| if (!context.getAttachedPrimaryStorageTypes().contains(PrimaryStorageConstant.EXTERNAL_PRIMARY_STORAGE_TYPE)) { | |
| return new NopeFlow(); | |
| } | |
| if (context.getAttachedPrimaryStorageTypes() == null || | |
| !context.getAttachedPrimaryStorageTypes().contains(PrimaryStorageConstant.EXTERNAL_PRIMARY_STORAGE_TYPE)) { | |
| return new NopeFlow(); | |
| } |
🤖 Prompt for AI Agents
In
`@plugin/externalStorage/src/main/java/org/zstack/externalStorage/primary/kvm/ExternalPrimaryStorageKvmFactory.java`
around lines 83 - 86, The null check for
context.getAttachedPrimaryStorageTypes() is missing and may throw NPE; update
ExternalPrimaryStorageKvmFactory so the early-return checks for null first
(e.g., in the same method/block that currently calls
context.getAttachedPrimaryStorageTypes().contains(...)) and return new
NopeFlow() when the collection is null or does not contain
PrimaryStorageConstant.EXTERNAL_PRIMARY_STORAGE_TYPE; mirror the null-safe check
pattern used in KvmFactory.java.
Comment on lines
5192
to
5205
|
|
||
| Set<String> attachedPsTypes = new HashSet(SQL.New("select pri.type from PrimaryStorageVO pri, PrimaryStorageClusterRefVO ref" + | ||
| " where pri.uuid = ref.primaryStorageUuid" + | ||
| " and ref.clusterUuid = :cuuid", ExternalPrimaryStorageVO.class) | ||
| .param("cuuid", self.getClusterUuid()) | ||
| .list()); | ||
| for (KVMHostConnectExtensionPoint extp : factory.getConnectExtensions()) { | ||
| KVMHostConnectedContext ctx = new KVMHostConnectedContext(); | ||
| ctx.setInventory((KVMHostInventory) getSelfInventory()); | ||
| ctx.setNewAddedHost(info.isNewAdded()); | ||
| ctx.setBaseUrl(baseUrl); | ||
| ctx.setSkipPackages(info.getSkipPackages()); | ||
|
|
||
| ctx.setAttachedPrimaryStorageTypes(attachedPsTypes); | ||
| chain.then(extp.createKvmHostConnectingFlow(ctx)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find SQL.java =="
fd -a 'SQL\.java$' .
echo -e "\n== Look at KVMHost.java lines 5190-5210 =="
sed -n '5190,5210p' plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
echo -e "\n== Find SQL.New(...) method signature =="
rg -A 10 'public\s+static\s+.*New\s*\(' $(fd -a 'SQL\.java$' .) | head -50
echo -e "\n== Check how SQL is used with String.class in codebase =="
rg 'SQL\.New\([^)]*String\.class\)' -A 3 | head -40Repository: MatheMatrix/zstack
Length of output: 6383
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Examine SQL.java constructor/initialization to understand returnClass usage =="
sed -n '1,150p' /home/jailuser/git/core/src/main/java/org/zstack/core/db/SQL.java | grep -A 30 "class SQL"
echo -e "\n== Check ExternalPrimaryStorageVO.class import in KVMHost.java =="
head -50 plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java | grep -i "externalprimary\|import"
echo -e "\n== Look for any similar incorrect return type usage patterns =="
rg 'SQL\.New\([^)]*ExternalPrimaryStorageVO\.class\)' -B 2 -A 2
echo -e "\n== Verify PrimaryStorageVO.type field is String =="
rg -A 30 'class PrimaryStorageVO' core/src/main/java/org/zstack/storage/primary/PrimaryStorageVO.java | grep -i "type"Repository: MatheMatrix/zstack
Length of output: 8705
修正 SQL 查询的返回类型和避免集合共享问题
此处 SQL 查询仅选取 pri.type 列（单个字符串值），但传入 ExternalPrimaryStorageVO.class 作为返回类型参数将导致类型不匹配。根据 JPA 的 createQuery(sql, returnClass) 语义，返回类型应为 String.class。此外，同一个 attachedPsTypes 集合实例被传递给所有扩展点，任何扩展若修改该集合会影响其他扩展的行为。应为每个扩展创建独立的副本。
建议修改
- Set<String> attachedPsTypes = new HashSet(SQL.New("select pri.type from PrimaryStorageVO pri, PrimaryStorageClusterRefVO ref" +
- " where pri.uuid = ref.primaryStorageUuid" +
- " and ref.clusterUuid = :cuuid", ExternalPrimaryStorageVO.class)
- .param("cuuid", self.getClusterUuid())
- .list());
+ Set<String> attachedPsTypes = new HashSet<>(SQL.New("select pri.type from PrimaryStorageVO pri, PrimaryStorageClusterRefVO ref" +
+ " where pri.uuid = ref.primaryStorageUuid" +
+ " and ref.clusterUuid = :cuuid", String.class)
+ .param("cuuid", self.getClusterUuid())
+ .list());
for (KVMHostConnectExtensionPoint extp : factory.getConnectExtensions()) {
KVMHostConnectedContext ctx = new KVMHostConnectedContext();
ctx.setInventory((KVMHostInventory) getSelfInventory());
ctx.setNewAddedHost(info.isNewAdded());
ctx.setBaseUrl(baseUrl);
ctx.setSkipPackages(info.getSkipPackages());
- ctx.setAttachedPrimaryStorageTypes(attachedPsTypes);
+ ctx.setAttachedPrimaryStorageTypes(new HashSet<>(attachedPsTypes));
chain.then(extp.createKvmHostConnectingFlow(ctx));
}
Comment on lines
+1032
to
1035
| public KVMHostContext getHostContextByUuid(String hostUuid) { | ||
| KVMHostVO kvo = dbf.findByUuid(hostUuid, KVMHostVO.class); | ||
| return createHostContext(kvo); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find all usages of getHostContextByUuid method
rg -n 'getHostContextByUuid' --type javaRepository: MatheMatrix/zstack
Length of output: 192
🏁 Script executed:
# Check the createHostContext method implementation
cd plugin/kvm/src/main/java/org/zstack/kvm && grep -n -A 15 'private KVMHostContext createHostContext' KVMHostFactory.javaRepository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
# Search for all places that might call this method indirectly
rg -n 'KVMHostFactory' --type java -A 3 -B 1 | head -100Repository: MatheMatrix/zstack
Length of output: 10742
🏁 Script executed:
# Read the KVMHostFactory file around the methods in question
cat -n plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java | sed -n '1010,1040p'Repository: MatheMatrix/zstack
Length of output: 1348
🏁 Script executed:
# Search for actual invocations/calls to getHostContextByUuid method
rg -n '\.getHostContextByUuid\(' --type javaRepository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
# Search for any reference to getHostContextByUuid to see if it's used at all
rg -n 'getHostContextByUuid' --type java -C 2Repository: MatheMatrix/zstack
Length of output: 565
🏁 Script executed:
# Check if method is referenced in any way (tests, groovy, etc)
rg -i 'getHostContextByUuid' --type groovy --type javaRepository: MatheMatrix/zstack
Length of output: 187
🏁 Script executed:
# Check git history to see when this method was added and why
cd plugin/kvm/src/main/java/org/zstack/kvm && git log --oneline -p KVMHostFactory.java | grep -A 10 -B 5 'getHostContextByUuid' | head -50Repository: MatheMatrix/zstack
Length of output: 515
该方法是未使用的死代码，应删除或完善
getHostContextByUuid 方法未在代码库中任何地方被调用。此外，该方法存在空值安全问题：dbf.findByUuid() 在找不到记录时返回 null，但代码直接将其传递给 createHostContext()，后者立即调用 vo.getManagementIp()，会抛出 NullPointerException。
若此方法的确无实际用途，应将其删除。若该方法是计划中的 API，则必须在调用 createHostContext() 前添加 null 检查，或在找不到对应主机时返回有意义的错误。
🤖 Prompt for AI Agents
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java` around lines
1032 - 1035, The getHostContextByUuid method is dead/unsafe:
dbf.findByUuid(hostUuid, KVMHostVO.class) can return null and passing that into
createHostContext will cause a NullPointerException; either remove
getHostContextByUuid if unused or make it safe by checking the KVMHostVO result
before calling createHostContext (e.g., if dbf.findByUuid(...) == null return
null or throw a descriptive exception) and ensure callers of
getHostContextByUuid handle the null/exception; reference the method name
getHostContextByUuid, the dbf.findByUuid call, the KVMHostVO result, and
createHostContext in your change.
| package org.zstack.storage.addon.primary; | ||
|
|
||
| import org.springframework.beans.factory.annotation.Autowired; | ||
| import org.springframework.security.core.parameters.P; |
There was a problem hiding this comment.
删除未使用的导入。
org.springframework.security.core.parameters.P 是 Spring Security 的注解类，在此文件中未被使用，且与当前代码逻辑无关。这可能是意外添加的导入。
🔧 建议修复
-import org.springframework.security.core.parameters.P;📝 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
| import org.springframework.security.core.parameters.P; |
🤖 Prompt for AI Agents
In
`@storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorageFactory.java`
at line 4, Remove the unused import
org.springframework.security.core.parameters.P from
ExternalPrimaryStorageFactory; locate the import statement at the top of the
class and delete it so the file no longer references that Spring Security
annotation which isn't used in any methods or fields.
MatheMatrix
force-pushed
the
sync/jin.ma/hotfix-large-env
branch
from
649ca4c to
ebe4d0b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorageFactory.java (1)
4-4: 移除未使用的导入。
org.springframework.security.core.parameters.P在此文件中未被使用，应删除此导入。plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java (1)
1032-1035: 该方法缺少空值检查，且可能是未使用的代码。
dbf.findByUuid()在找不到记录时会返回 null，但代码直接将其传递给createHostContext()，后者会调用vo.getManagementIp()导致 NullPointerException。建议：若此方法确无调用方，应删除；若有使用计划，需添加 null 检查。
🧹 Nitpick comments (1)
plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java (1)
244-248: 调用方式更新正确，但存在相邻的死代码。将
getHostContext(kvo)改为传递已加载的KVMHostVO对象是正确的优化。但需要注意：由于新的
getHostContext(KVMHostVO kvo)方法直接调用createHostContext(kvo)并总是返回非 null 的对象，第 245-247 行的 null 检查if (context == null)现在永远不会为 true，已成为死代码。♻️ 建议清理死代码
public Host getHost(HostVO vo) { KVMHostVO kvo = dbf.findByUuid(vo.getUuid(), KVMHostVO.class); - KVMHostContext context = getHostContext(kvo); - if (context == null) { - context = createHostContext(kvo); - } + KVMHostContext context = getHostContext(kvo); return new KVMHost(kvo, context); }
📜 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 (10)
core/src/main/java/org/zstack/core/CoreGlobalProperty.javacore/src/main/java/org/zstack/core/cloudbus/EventFacadeImpl.javaplugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephKvmExtension.javaplugin/externalStorage/src/main/java/org/zstack/externalStorage/primary/kvm/ExternalPrimaryStorageKvmFactory.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHostConnectedContext.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.javaplugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/KvmFactory.javastorage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorageFactory.java
🚧 Files skipped from review as they are similar to previous changes (7)
- plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.java
- plugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/KvmFactory.java
- plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
- plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephKvmExtension.java
- core/src/main/java/org/zstack/core/cloudbus/EventFacadeImpl.java
- plugin/kvm/src/main/java/org/zstack/kvm/KVMHostConnectedContext.java
- plugin/externalStorage/src/main/java/org/zstack/externalStorage/primary/kvm/ExternalPrimaryStorageKvmFactory.java
🧰 Additional context used
📓 Path-based instructions (2)
**/*.*
⚙️ CodeRabbit configuration file
**/*.*: - 代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写
Files:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.javastorage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorageFactory.javacore/src/main/java/org/zstack/core/CoreGlobalProperty.java
**/*.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:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.javastorage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorageFactory.javacore/src/main/java/org/zstack/core/CoreGlobalProperty.java
🧠 Learnings (8)
📓 Common learnings
Learnt from: MatheMatrix
Repo: MatheMatrix/zstack PR: 2823
File: plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java:786-831
Timestamp: 2025-10-28T02:29:38.803Z
Learning: APIUpdateHostnameMsg 的主机名有效性已在 API 层（HostApiInterceptor/VmHostnameUtils）完成校验；KVMHost.handle(UpdateHostnameMsg) 不应重复校验。
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 2798
File: plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java:6888-6942
Timestamp: 2025-10-24T06:45:56.228Z
Learning: KVM 数据面（agent）在/host/file/download路径会校验上传URL的scheme；管理面KVMHost.uploadFileToHost当前仅做非空scheme检测。
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 字段从必需改为可选)不会造成破坏性影响。
📚 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:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java
📚 Learning: 2025-10-23T07:15:36.501Z
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 2796
File: plugin/kvm/src/main/java/org/zstack/kvm/KVMHostUtils.java:50-53
Timestamp: 2025-10-23T07:15:36.501Z
Learning: In the KVMHostUtils.getNormalizedBridgeName method in plugin/kvm/src/main/java/org/zstack/kvm/KVMHostUtils.java, the second check for L2_BRIDGE_NAME tag (after checking for name conflict) is intentional to handle concurrency issues. This double-check pattern prevents multiple threads from generating different bridge names for the same l2Uuid when they execute concurrently.
Applied to files:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java
📚 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:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java
📚 Learning: 2025-08-04T03:35:51.225Z
Learnt from: zstack-robot-2
Repo: MatheMatrix/zstack PR: 2360
File: compute/src/main/java/org/zstack/compute/vm/VmNicParamBuilder.java:27-28
Timestamp: 2025-08-04T03:35:51.225Z
Learning: 在 VmNicParamBuilder.buildByVmUuid 方法中，上层调用保证传入的 VM UUID 对应的实例一定存在，因此使用 Q.New(VmInstanceVO.class).eq(VmInstanceVO_.uuid, vmInstanceUuid).find() 查询后不需要进行 null 检查。
Applied to files:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java
📚 Learning: 2025-08-25T03:52:37.301Z
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 2504
File: storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotTreeBase.java:775-819
Timestamp: 2025-08-25T03:52:37.301Z
Learning: 在ZStack项目的VolumeSnapshotTreeBase类中，当通过dbf.findByUuid()方法获取VolumeVO对象时，需要进行null检查，因为该方法可能在找不到对应记录时返回null，直接使用可能导致NullPointerException。
Applied to files:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java
📚 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:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java
📚 Learning: 2025-08-14T06:56:19.585Z
Learnt from: zstack-robot-2
Repo: MatheMatrix/zstack PR: 2435
File: storage/src/main/java/org/zstack/storage/snapshot/group/VolumeSnapshotGroupBase.java:47-47
Timestamp: 2025-08-14T06:56:19.585Z
Learning: 在VolumeSnapshotGroupBase.java中，VmInstanceResourceMetadataManager的注入和SKIP_RESOURCE_ROLLBACK标记虽然在当前版本中未被使用，但这些导入在大型重构PR中是有意为之的，用于保持代码一致性或为后续功能做准备。
Applied to files:
storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorageFactory.java
🔍 Remote MCP AtlassianMCP
Additional context retrieved (concise, relevant to PR review)
- Jira QI-1170: "单套ZStack管控能力评估及优化" — scope and rationale match this PR's changes (performance/scalability optimizations: reduce DB ops, avoid unnecessary plugin loads, change event/glob handling, remove per-fire webhook SQL, change host-connect flows). Key comments highlight:
- Removing host hardware status monitor improved simulator scale to 10k; host HTTP callback post-check for host deletion harms performance and should be replaced by event-based deletion handling.
- Webhook dispatch currently does a DB query per event and should be cached/converted to event-driven (the PR temporarily removes that DB path).
- Glob pattern matching for events consumed ~20% heap; only global-config events need glob support — conditional initialization reduces memory.
- Plugins should be skipped when their primary storage type is not present; prefer checking attached primary storage types in-memory to avoid DB queries.
- Other observations: transaction/connection contention and heavy CPU in recordElaboration; links to MR 9021 referenced as source of changes.
(Source: AtlassianMCP_jira_get_issue)
🔇 Additional comments (2)
storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorageFactory.java (1)
269-271: LGTM!方法实现正确，使用
new ArrayList<>()创建防御性副本，避免暴露内部 map 的可变视图。此方法符合 PR 目标，为插件提供查询所有已注册的主存储控制器服务的能力。plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java (1)
1028-1030: LGTM! 方法签名重构减少了不必要的数据库查询。将方法参数从
String hostUuid改为KVMHostVO kvo，当调用方已经持有 VO 对象时可以避免重复的数据库查询，符合 PR 目标中减少数据库操作的优化方向。
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Comment on lines
+88
to
89
| @GlobalProperty(name = "enableElaboration", defaultValue = "false") | ||
| public static boolean ENABLE_ELABORATION; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 检查是否存在 elaboration 相关的升级处理逻辑
echo "=== Searching for upgrade handling related to enableElaboration ==="
rg -n -C3 'enableElaboration|ENABLE_ELABORATION' --type=java
echo ""
echo "=== Searching for upgrade scripts mentioning elaboration ==="
fd -e sql -e py -e sh | xargs rg -l -i 'elaboration' 2>/dev/null || echo "No upgrade scripts found"
echo ""
echo "=== Checking how GlobalProperty changes are typically handled ==="
fd -n "upgrade" -type f | head -20Repository: MatheMatrix/zstack
Length of output: 2676
🏁 Script executed:
#!/bin/bash
# 检查 GlobalProperty 的加载和初始化机制
echo "=== Searching for GlobalProperty class definition and loading logic ==="
rg -n "class GlobalProperty|@GlobalProperty" --type=java -A 5 | head -50
echo ""
echo "=== Searching for how GlobalProperty values are loaded from storage ==="
rg -n "GlobalProperty.*load|readGlobalProperty|updateGlobalProperty" --type=java -B 2 -A 5 | head -80
echo ""
echo "=== Checking if global properties are stored in database ==="
rg -n "GlobalProperty.*table|insert.*global" --type=java -i | head -30Repository: MatheMatrix/zstack
Length of output: 16485
🏁 Script executed:
#!/bin/bash
# 搜索 GlobalProperty 注解处理器和加载机制
echo "=== Searching for GlobalProperty annotation processor ==="
fd -e java | xargs rg -l "GlobalProperty" | xargs rg -l "processor|handler|loader" | head -20
echo ""
echo "=== Searching for GlobalPropertyDefinition and how it's processed ==="
rg -n "@GlobalPropertyDefinition" --type=java -B 2 -A 10 | head -60
echo ""
echo "=== Searching for property value persistence mechanism ==="
rg -n "putGlobalProperty|GlobalConfigVO|GlobalConfig" --type=java | head -40
echo ""
echo "=== Check GlobalConfig table in database schema ==="
fd -e sql | xargs rg -l "GlobalConfig|global_config" | head -10Repository: MatheMatrix/zstack
Length of output: 14887
🏁 Script executed:
#!/bin/bash
# 查找 GlobalConfigVO 和相关初始化逻辑
echo "=== Searching for GlobalConfigVO ==="
fd -e java -path "*/header/*" | xargs rg -l "GlobalConfigVO|GlobalConfigInventory" | head -5
echo ""
echo "=== Looking for global config initialization ==="
rg -n "GlobalPropertyDefinition|GlobalConfigVO" --type=java -B 2 -A 5 core/src/main/java/org/zstack/core/config/ | head -80
echo ""
echo "=== Checking how global properties are loaded at startup ==="
fd -e java -path "*/core/*" | xargs rg -l "GlobalProperty" | xargs rg -l "init|load|startup" | head -10
echo ""
echo "=== Looking for update/reset global config logic ==="
rg -n "UpdateGlobalConfigMsg|ResetGlobalConfigMsg" --type=java | head -20Repository: MatheMatrix/zstack
Length of output: 11083
🏁 Script executed:
#!/bin/bash
# 查看 GlobalConfigFacadeImpl 中的完整配置更新逻辑
echo "=== Reading GlobalConfigFacadeImpl to understand config update logic ==="
wc -l core/src/main/java/org/zstack/core/config/GlobalConfigFacadeImpl.java
echo ""
echo "=== Examining the persistConfigInXmlButNotInDatabase method ==="
sed -n '375,500p' core/src/main/java/org/zstack/core/config/GlobalConfigFacadeImpl.javaRepository: MatheMatrix/zstack
Length of output: 6749
🏁 Script executed:
#!/bin/bash
# 理解完整的升级逻辑后，现在检查是否有文档说明这个改动
echo "=== Checking CHANGELOG or release notes for this change ==="
fd -e md -e txt | xargs rg -l "enableElaboration|ENABLE_ELABORATION|QI-1170" 2>/dev/null | head -10
echo ""
echo "=== Checking git log for context about this change ==="
git log --oneline --all | grep -i "elaboration" | head -10
echo ""
echo "=== Looking for any configuration migration guide ==="
fd -e md -e txt -path "*/docs/*" -o -path "*/upgrade/*" | xargs rg -l "migration|upgrade" 2>/dev/null | head -5Repository: MatheMatrix/zstack
Length of output: 679
明确升级迁移策略或添加配置保留机制。
将 ENABLE_ELABORATION 的默认值从 "true" 改为 "false" 在升级现有部署时会产生破坏性变更。经代码分析，GlobalConfigFacadeImpl 的升级逻辑会在以下场景改写该配置：
- 若用户未曾显式设置 enableElaboration（即
defaultValue == value），升级时会将配置值同时更新为新的默认值"false"（详见 GlobalConfigFacadeImpl 第 390-441 行） - 只有用户显式定制过该属性的部署才能保留自己的设置
由于 recordElaboration 性能优化的需要，禁用 elaboration 是合理的。但根据编码规范的向后兼容原则，建议：
- 添加升级迁移脚本，为已有安装显式设置
enableElaboration=true，以保留现有行为 - 或在升级文档中明确说明此行为变更，并提供用户手动调整的指引
MatheMatrix
force-pushed
the
sync/jin.ma/hotfix-large-env
branch
from
ebe4d0b to
5375eed
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@plugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/KvmFactory.java`:
- Around line 62-65: The code in KvmFactory uses
context.getAttachedPrimaryStorageTypes().contains(SMPConstants.SMP_TYPE) and can
throw NPE if getAttachedPrimaryStorageTypes() returns null; add a null-safe
check before calling contains (e.g., if the collection is null treat as empty)
or adjust KVMHostConnectedContext.getAttachedPrimaryStorageTypes() to never
return null; update the conditional in KvmFactory to explicitly handle null (and
still return new NopeFlow() when there is no matching type) so contains is only
called on a non-null collection.
♻️ Duplicate comments (5)
storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorageFactory.java (1)
4-4: 删除未使用的导入。
org.springframework.security.core.parameters.P在此文件中未被使用，应删除该导入。core/src/main/java/org/zstack/core/cloudbus/EventFacadeImpl.java (1)
232-232: Webhook 功能被禁用需要文档说明此问题已在之前的 review 中指出。建议添加 TODO 注释关联 QI-1170，说明这是临时优化措施。
plugin/externalStorage/src/main/java/org/zstack/externalStorage/primary/kvm/ExternalPrimaryStorageKvmFactory.java (2)
33-33: 删除未使用的导入。
ExternalPrimaryStorageConstants被导入但未在代码中使用。第 83 行的类型检查使用的是PrimaryStorageConstant.EXTERNAL_PRIMARY_STORAGE_TYPE。🔧 建议修复
-import org.zstack.storage.addon.primary.ExternalPrimaryStorageConstants;
83-86: 存在空指针异常风险。如果
context.getAttachedPrimaryStorageTypes()返回null，调用contains()方法将抛出NullPointerException。这与其他存储扩展（如 SMP 的 KvmFactory）存在相同的问题。🔧 建议修复
- if (!context.getAttachedPrimaryStorageTypes().contains(PrimaryStorageConstant.EXTERNAL_PRIMARY_STORAGE_TYPE)) { + if (context.getAttachedPrimaryStorageTypes() == null || + !context.getAttachedPrimaryStorageTypes().contains(PrimaryStorageConstant.EXTERNAL_PRIMARY_STORAGE_TYPE)) { return new NopeFlow(); }plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java (1)
1032-1035: 该方法未被使用且存在空指针异常风险。
getHostContextByUuid方法在代码库中未被调用，属于死代码。此外，dbf.findByUuid()在找不到记录时返回null，直接将其传递给createHostContext()会导致在第 1015 行调用vo.getManagementIp()时抛出NullPointerException。若此方法无实际用途，建议删除。若为计划中的 API，则需添加 null 检查。
🔧 建议修复（如需保留该方法）
public KVMHostContext getHostContextByUuid(String hostUuid) { KVMHostVO kvo = dbf.findByUuid(hostUuid, KVMHostVO.class); + if (kvo == null) { + return null; + } return createHostContext(kvo); }
🧹 Nitpick comments (2)
core/src/main/java/org/zstack/core/cloudbus/EventFacadeImpl.java (1)
237-252: 考虑移除或标记为废弃的死代码
callWebhooks方法当前未被调用，成为死代码。如果这是临时禁用，建议添加注释说明；如果是永久移除，建议删除此方法以保持代码整洁。如果是永久移除，建议删除
- private void callWebhooks(CanonicalEvent event) { - new WebhookCaller() { - `@Override` - public void call() { - List<WebhookVO> vos = Q.New(WebhookVO.class).eq(WebhookVO_.type, EventFacade.WEBHOOK_TYPE).list(); - vos = vos.stream().filter( - vo -> event.getPath().matches( - createRegexFromGlob(vo.getOpaque().replaceAll("\\{.*\\}", ".*")) - )).collect(Collectors.toList()); - - if (!vos.isEmpty()) { - postToWebhooks(WebhookInventory.valueOf(vos), JSONObjectUtil.toJsonString(event)); - } - } - }.call(); - }如果是临时禁用，建议添加注释
+ /** + * TODO(QI-1170): Temporarily disabled for performance optimization. + * This method queries DB on every event fire, causing performance issues in large environments. + * Will be replaced with a cached mechanism. + */ + `@SuppressWarnings`("unused") private void callWebhooks(CanonicalEvent event) {plugin/kvm/src/main/java/org/zstack/kvm/KVMHostConnectedContext.java (1)
12-12: 建议考虑 null 安全性。新增的
attachedPrimaryStorageTypes字段可能为null（未设置时），调用方在使用getAttachedPrimaryStorageTypes().contains(...)时可能触发NullPointerException。建议在 getter 中返回空集合作为默认值，或在字段声明时初始化为空集合：
♻️ 建议修复（方案一：getter 返回空集合）
public Set<String> getAttachedPrimaryStorageTypes() { - return attachedPrimaryStorageTypes; + return attachedPrimaryStorageTypes != null ? attachedPrimaryStorageTypes : java.util.Collections.emptySet(); }♻️ 建议修复（方案二：字段初始化）
-private Set<String> attachedPrimaryStorageTypes; +private Set<String> attachedPrimaryStorageTypes = java.util.Collections.emptySet();Also applies to: 46-48
📜 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 (10)
core/src/main/java/org/zstack/core/CoreGlobalProperty.javacore/src/main/java/org/zstack/core/cloudbus/EventFacadeImpl.javaplugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephKvmExtension.javaplugin/externalStorage/src/main/java/org/zstack/externalStorage/primary/kvm/ExternalPrimaryStorageKvmFactory.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHostConnectedContext.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.javaplugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/KvmFactory.javastorage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorageFactory.java
🚧 Files skipped from review as they are similar to previous changes (4)
- plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
- plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephKvmExtension.java
- core/src/main/java/org/zstack/core/CoreGlobalProperty.java
- plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.java
🧰 Additional context used
📓 Path-based instructions (2)
**/*.*
⚙️ CodeRabbit configuration file
**/*.*: - 代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写
Files:
plugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/KvmFactory.javaplugin/externalStorage/src/main/java/org/zstack/externalStorage/primary/kvm/ExternalPrimaryStorageKvmFactory.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHostConnectedContext.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.javacore/src/main/java/org/zstack/core/cloudbus/EventFacadeImpl.javastorage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorageFactory.java
**/*.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:
plugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/KvmFactory.javaplugin/externalStorage/src/main/java/org/zstack/externalStorage/primary/kvm/ExternalPrimaryStorageKvmFactory.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHostConnectedContext.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.javacore/src/main/java/org/zstack/core/cloudbus/EventFacadeImpl.javastorage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorageFactory.java
🧠 Learnings (8)
📓 Common learnings
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 字段从必需改为可选)不会造成破坏性影响。
Learnt from: MatheMatrix
Repo: MatheMatrix/zstack PR: 2823
File: plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java:786-831
Timestamp: 2025-10-28T02:29:38.803Z
Learning: APIUpdateHostnameMsg 的主机名有效性已在 API 层（HostApiInterceptor/VmHostnameUtils）完成校验；KVMHost.handle(UpdateHostnameMsg) 不应重复校验。
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 2798
File: plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java:6888-6942
Timestamp: 2025-10-24T06:45:56.228Z
Learning: KVM 数据面（agent）在/host/file/download路径会校验上传URL的scheme；管理面KVMHost.uploadFileToHost当前仅做非空scheme检测。
📚 Learning: 2025-08-14T06:56:19.585Z
Learnt from: zstack-robot-2
Repo: MatheMatrix/zstack PR: 2435
File: storage/src/main/java/org/zstack/storage/snapshot/group/VolumeSnapshotGroupBase.java:47-47
Timestamp: 2025-08-14T06:56:19.585Z
Learning: 在VolumeSnapshotGroupBase.java中，VmInstanceResourceMetadataManager的注入和SKIP_RESOURCE_ROLLBACK标记虽然在当前版本中未被使用，但这些导入在大型重构PR中是有意为之的，用于保持代码一致性或为后续功能做准备。
Applied to files:
plugin/externalStorage/src/main/java/org/zstack/externalStorage/primary/kvm/ExternalPrimaryStorageKvmFactory.javastorage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorageFactory.java
📚 Learning: 2025-08-25T03:52:37.301Z
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 2504
File: storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotTreeBase.java:775-819
Timestamp: 2025-08-25T03:52:37.301Z
Learning: 在ZStack项目的VolumeSnapshotTreeBase类中，当通过dbf.findByUuid()方法获取VolumeVO对象时，需要进行null检查，因为该方法可能在找不到对应记录时返回null，直接使用可能导致NullPointerException。
Applied to files:
plugin/externalStorage/src/main/java/org/zstack/externalStorage/primary/kvm/ExternalPrimaryStorageKvmFactory.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.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:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java
📚 Learning: 2025-10-23T07:15:36.501Z
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 2796
File: plugin/kvm/src/main/java/org/zstack/kvm/KVMHostUtils.java:50-53
Timestamp: 2025-10-23T07:15:36.501Z
Learning: In the KVMHostUtils.getNormalizedBridgeName method in plugin/kvm/src/main/java/org/zstack/kvm/KVMHostUtils.java, the second check for L2_BRIDGE_NAME tag (after checking for name conflict) is intentional to handle concurrency issues. This double-check pattern prevents multiple threads from generating different bridge names for the same l2Uuid when they execute concurrently.
Applied to files:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java
📚 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:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java
📚 Learning: 2025-08-04T03:35:51.225Z
Learnt from: zstack-robot-2
Repo: MatheMatrix/zstack PR: 2360
File: compute/src/main/java/org/zstack/compute/vm/VmNicParamBuilder.java:27-28
Timestamp: 2025-08-04T03:35:51.225Z
Learning: 在 VmNicParamBuilder.buildByVmUuid 方法中，上层调用保证传入的 VM UUID 对应的实例一定存在，因此使用 Q.New(VmInstanceVO.class).eq(VmInstanceVO_.uuid, vmInstanceUuid).find() 查询后不需要进行 null 检查。
Applied to files:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java
📚 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:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java
🔍 Remote MCP AtlassianMCP
Additional relevant context from Jira QI-1170
- Issue: QI-1170 — 单套ZStack管控能力评估及优化
- Summary: 评估单套ZStack能纳管的最大物理机数量并进行优化（目标：到12月底先做到1000台物理机；simulator 目标 10k）. Key focus areas: reduce DB ops, remove synchronization/waits, investigate reconnection bottlenecks, optimize high-cost parts of single reconnect tasks.
- Status: To Do
- Assignee: jin.ma
- Reporter: jiajun.xu
- Created: 2025-11-27; Last updated: 2026-01-14
(Source: Jira issue QI-1170)
🔇 Additional comments (8)
storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorageFactory.java (1)
269-272: LGTM!方法实现正确，通过创建新的
ArrayList返回防御性副本，避免了外部代码直接修改内部集合状态。core/src/main/java/org/zstack/core/cloudbus/EventFacadeImpl.java (4)
79-88: LGTM!通过条件判断避免对非 glob 路径创建正则表达式，与后续的精确匹配逻辑配合良好，是合理的性能优化。
132-134: LGTM!将 glob 检测逻辑抽取为独立方法，提高了代码可读性。
{字符的检测与第 82 行中{...}占位符替换逻辑相匹配。
254-263: LGTM!对于非 glob 路径使用精确匹配替代正则匹配，是有效的性能优化。逻辑正确且与
handleEvent中的实现保持一致。
272-288: LGTM!与
fireLocal保持一致的匹配逻辑优化，通过对非 glob 路径使用精确匹配提升性能。plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java (2)
243-248: LGTM!调用方式从
getHostContext(vo.getUuid())改为getHostContext(kvo)，直接传递 VO 对象避免了重复查询数据库，符合 PR 目标（减少 DB 操作）。
1028-1030: LGTM!方法签名从接受
String hostUuid改为接受KVMHostVO kvo，直接委托给createHostContext(kvo)，减少了不必要的数据库查询。plugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/KvmFactory.java (1)
13-13: LGTM!
NopeFlow导入正确且被使用。
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Comment on lines
+62
to
+65
| if (!context.getAttachedPrimaryStorageTypes().contains(SMPConstants.SMP_TYPE)) { | ||
| return new NopeFlow(); | ||
| } | ||
|
|
There was a problem hiding this comment.
存在空指针异常风险。
与 ExternalPrimaryStorageKvmFactory 相同的问题：如果 context.getAttachedPrimaryStorageTypes() 返回 null，调用 contains() 方法将抛出 NullPointerException。
建议统一在 KVMHostConnectedContext.getAttachedPrimaryStorageTypes() 中处理 null 安全性，或在此处添加 null 检查。
🔧 建议修复
- if (!context.getAttachedPrimaryStorageTypes().contains(SMPConstants.SMP_TYPE)) {
+ if (context.getAttachedPrimaryStorageTypes() == null ||
+ !context.getAttachedPrimaryStorageTypes().contains(SMPConstants.SMP_TYPE)) {
return new NopeFlow();
}📝 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
| if (!context.getAttachedPrimaryStorageTypes().contains(SMPConstants.SMP_TYPE)) { | |
| return new NopeFlow(); | |
| } | |
| if (context.getAttachedPrimaryStorageTypes() == null || | |
| !context.getAttachedPrimaryStorageTypes().contains(SMPConstants.SMP_TYPE)) { | |
| return new NopeFlow(); | |
| } |
🤖 Prompt for AI Agents
In
`@plugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/KvmFactory.java`
around lines 62 - 65, The code in KvmFactory uses
context.getAttachedPrimaryStorageTypes().contains(SMPConstants.SMP_TYPE) and can
throw NPE if getAttachedPrimaryStorageTypes() returns null; add a null-safe
check before calling contains (e.g., if the collection is null treat as empty)
or adjust KVMHostConnectedContext.getAttachedPrimaryStorageTypes() to never
return null; update the conditional in KvmFactory to explicitly handle null (and
still return new NopeFlow() when there is no matching type) so contains is only
called on a non-null collection.
5375eed to
31c870b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java`:
- Around line 510-516: The async HTTP callback's success(T) handling is
inconsistent: the branch for commandStr != null no longer checks host existence
while the cmd branch still calls dbf.isExist(self.getUuid(), HostVO.class) and
fails if the Host was deleted, leading to divergent semantics; make the behavior
consistent by either (A) restoring the host-existence guard in the commandStr !=
null success path (use dbf.isExist(self.getUuid(), HostVO.class) and call
completion.fail(operr(...)) when missing) or (B) removing the host-existence
checks from both branches and ensuring Host deletion is handled centrally via
the Host deletion/task-cancel mechanism; update the success(T ret)
implementation so both code paths (commandStr != null branch and cmd branch)
behave identically and add a short comment near success(T) explaining the chosen
approach (referencing success(T), completion.success/completion.fail,
commandStr, cmd, dbf.isExist, HostVO, and self.getUuid()).
♻️ Duplicate comments (5)
core/src/main/java/org/zstack/core/CoreGlobalProperty.java (1)
88-89: 默认值变更可能导致升级时的破坏性行为变更将
ENABLE_ELABORATION的默认值从"true"改为"false"会影响现有部署的升级行为。根据编码规范的向后兼容原则，之前的代码产生的行为不应直接改动。此问题已在之前的审查中提出，建议：
- 添加升级迁移脚本，为已有安装显式设置
enableElaboration=true以保留现有行为- 或在升级文档中明确说明此行为变更
core/src/main/java/org/zstack/core/cloudbus/EventFacadeImpl.java (1)
232-232: Webhook 功能被静默禁用
callWebhooks(evt)被注释掉会导致事件触发时不再调用 webhook。根据 QI-1170 的上下文，这是为了优化性能的临时措施。建议添加 TODO 注释说明：
- 为什么禁用此功能
- 何时/如何恢复（如通过缓存机制替代）
- // callWebhooks(evt); + // TODO(QI-1170): Webhook dispatch temporarily disabled for performance optimization. + // Will be replaced with cached/event-based mechanism. + // callWebhooks(evt);plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java (1)
1032-1035:getHostContextByUuid方法存在空指针风险
dbf.findByUuid(hostUuid, KVMHostVO.class)在找不到记录时返回 null，但代码直接将其传递给createHostContext()。根据 Line 1015，createHostContext会调用vo.getManagementIp()，当kvo为 null 时会抛出 NullPointerException。此外，该方法目前在代码库中未被调用。建议：
- 如果是计划中的 API，添加 null 检查
- 如果无实际用途，考虑删除
🔧 建议的修复方案
public KVMHostContext getHostContextByUuid(String hostUuid) { KVMHostVO kvo = dbf.findByUuid(hostUuid, KVMHostVO.class); + if (kvo == null) { + return null; + } return createHostContext(kvo); }plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java (1)
5193-5205: SQL 查询返回类型错误 + 共享 Set 传递给扩展点有互相污染风险
当前 SQL 只select pri.type（字符串），但SQL.New(..., PrimaryStorageVO.class)会导致类型不匹配（高概率运行时异常/转换错误）。另外同一个attachedPsTypes实例传给所有扩展点，任一扩展修改集合会影响其它扩展。建议修改（修复返回类型 + 避免集合共享 + 可选 distinct 去重）
- Set<String> attachedPsTypes = new HashSet(SQL.New("select pri.type from PrimaryStorageVO pri, PrimaryStorageClusterRefVO ref" + + Set<String> attachedPsTypes = new HashSet<>(SQL.New("select distinct pri.type from PrimaryStorageVO pri, PrimaryStorageClusterRefVO ref" + " where pri.uuid = ref.primaryStorageUuid" + " and ref.clusterUuid = :cuuid", PrimaryStorageVO.class) .param("cuuid", self.getClusterUuid()) - .list()); + .list()); for (KVMHostConnectExtensionPoint extp : factory.getConnectExtensions()) { KVMHostConnectedContext ctx = new KVMHostConnectedContext(); ctx.setInventory((KVMHostInventory) getSelfInventory()); ctx.setNewAddedHost(info.isNewAdded()); ctx.setBaseUrl(baseUrl); ctx.setSkipPackages(info.getSkipPackages()); - ctx.setAttachedPrimaryStorageTypes(attachedPsTypes); + ctx.setAttachedPrimaryStorageTypes(new HashSet<>(attachedPsTypes)); chain.then(extp.createKvmHostConnectingFlow(ctx)); }- " and ref.clusterUuid = :cuuid", PrimaryStorageVO.class) + " and ref.clusterUuid = :cuuid", String.class)plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephKvmExtension.java (1)
132-135: 潜在的空指针异常风险（已有评论指出）。此优化逻辑可有效跳过不必要的 Ceph 主存储初始化流程。但如之前评论所述，若
getAttachedPrimaryStorageTypes()返回null，调用contains()将抛出 NPE。建议添加空值保护：
🔧 建议修复
- if (!context.getAttachedPrimaryStorageTypes().contains(CephConstants.CEPH_PRIMARY_STORAGE_TYPE)) { + if (context.getAttachedPrimaryStorageTypes() == null || + !context.getAttachedPrimaryStorageTypes().contains(CephConstants.CEPH_PRIMARY_STORAGE_TYPE)) { return new NopeFlow(); }
📜 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 (10)
core/src/main/java/org/zstack/core/CoreGlobalProperty.javacore/src/main/java/org/zstack/core/cloudbus/EventFacadeImpl.javaplugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephKvmExtension.javaplugin/externalStorage/src/main/java/org/zstack/externalStorage/primary/kvm/ExternalPrimaryStorageKvmFactory.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHostConnectedContext.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.javaplugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/KvmFactory.javastorage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorageFactory.java
🚧 Files skipped from review as they are similar to previous changes (4)
- plugin/externalStorage/src/main/java/org/zstack/externalStorage/primary/kvm/ExternalPrimaryStorageKvmFactory.java
- plugin/kvm/src/main/java/org/zstack/kvm/KVMHostConnectedContext.java
- storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorageFactory.java
- plugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/KvmFactory.java
🧰 Additional context used
📓 Path-based instructions (2)
**/*.*
⚙️ CodeRabbit configuration file
**/*.*: - 代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写
Files:
plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javacore/src/main/java/org/zstack/core/cloudbus/EventFacadeImpl.javacore/src/main/java/org/zstack/core/CoreGlobalProperty.javaplugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephKvmExtension.java
**/*.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:
plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javacore/src/main/java/org/zstack/core/cloudbus/EventFacadeImpl.javacore/src/main/java/org/zstack/core/CoreGlobalProperty.javaplugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephKvmExtension.java
🧠 Learnings (11)
📓 Common learnings
Learnt from: MatheMatrix
Repo: MatheMatrix/zstack PR: 2823
File: plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java:786-831
Timestamp: 2025-10-28T02:29:38.803Z
Learning: APIUpdateHostnameMsg 的主机名有效性已在 API 层（HostApiInterceptor/VmHostnameUtils）完成校验；KVMHost.handle(UpdateHostnameMsg) 不应重复校验。
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 字段从必需改为可选)不会造成破坏性影响。
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 2798
File: plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java:6888-6942
Timestamp: 2025-10-24T06:45:56.228Z
Learning: KVM 数据面（agent）在/host/file/download路径会校验上传URL的scheme；管理面KVMHost.uploadFileToHost当前仅做非空scheme检测。
📚 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:
plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.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:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
📚 Learning: 2025-10-23T07:15:36.501Z
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 2796
File: plugin/kvm/src/main/java/org/zstack/kvm/KVMHostUtils.java:50-53
Timestamp: 2025-10-23T07:15:36.501Z
Learning: In the KVMHostUtils.getNormalizedBridgeName method in plugin/kvm/src/main/java/org/zstack/kvm/KVMHostUtils.java, the second check for L2_BRIDGE_NAME tag (after checking for name conflict) is intentional to handle concurrency issues. This double-check pattern prevents multiple threads from generating different bridge names for the same l2Uuid when they execute concurrently.
Applied to files:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
📚 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:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java
📚 Learning: 2025-08-04T03:35:51.225Z
Learnt from: zstack-robot-2
Repo: MatheMatrix/zstack PR: 2360
File: compute/src/main/java/org/zstack/compute/vm/VmNicParamBuilder.java:27-28
Timestamp: 2025-08-04T03:35:51.225Z
Learning: 在 VmNicParamBuilder.buildByVmUuid 方法中，上层调用保证传入的 VM UUID 对应的实例一定存在，因此使用 Q.New(VmInstanceVO.class).eq(VmInstanceVO_.uuid, vmInstanceUuid).find() 查询后不需要进行 null 检查。
Applied to files:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java
📚 Learning: 2025-08-25T03:52:37.301Z
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 2504
File: storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotTreeBase.java:775-819
Timestamp: 2025-08-25T03:52:37.301Z
Learning: 在ZStack项目的VolumeSnapshotTreeBase类中，当通过dbf.findByUuid()方法获取VolumeVO对象时，需要进行null检查，因为该方法可能在找不到对应记录时返回null，直接使用可能导致NullPointerException。
Applied to files:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
📚 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:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java
📚 Learning: 2025-08-12T05:39:14.846Z
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 2418
File: compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java:3856-3865
Timestamp: 2025-08-12T05:39:14.846Z
Learning: 在 compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java 的 setNoIpAllocationStaticIp 流程中，从数据库加载的 VmNicVO 实例其 getUsedIps() 为空时也不会为 null（返回空集合）。因此无需对 getUsedIps() 再做 Optional/空列表归一化的空指针保护；若找不到对应 NIC，使用 orElse(new VmNicVO()) 的约定允许后续逻辑通过 NPE 暴露问题，与项目既有约定一致。
Applied to files:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaplugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephKvmExtension.java
📚 Learning: 2025-08-03T04:10:21.683Z
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 0
File: :0-0
Timestamp: 2025-08-03T04:10:21.683Z
Learning: ZStack 数据库升级脚本 V4.10.16__schema.sql 中的 UPGRADE_VM_METADATA_TABLES_DIRECT 存储过程使用直接 RENAME TABLE 操作，不具备幂等性。需要通过 information_schema 检查表和约束的存在性来确保脚本可以安全重复执行。
Applied to files:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
📚 Learning: 2025-08-12T05:52:18.323Z
Learnt from: MatheMatrix
Repo: MatheMatrix/zstack PR: 2418
File: compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java:3873-3885
Timestamp: 2025-08-12T05:52:18.323Z
Learning: 在 compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java 的 setNoIpAllocationStaticIp() 流程中，静态 IP 标签删除已统一在 change-ip-in-database 步骤对 voRemoveSet 循环处理：对收集到的旧 IPv4/IPv6 UsedIpVO 均调用 ipOperator.deleteStaticIpByVmUuidAndL3Uuid(vmUuid, l3Uuid, IPv6NetworkUtils.ipv6AddessToTagValue(ip)) 清理对应的 static-ip 标签。因此无需在分支里分别删除 IPv4/IPv6 的标签。
Applied to files:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
🔍 Remote MCP AtlassianMCP
Additional context retrieved (concise, relevant for reviewing PR #3202)
-
Jira QI-1170: "单套ZStack管控能力评估及优化" — goal: evaluate and optimize single ZStack instance max managed physical hosts (target 1k → simulator 10k). Assignee: jin.ma; status: To Do; priority: P0.
-
Key findings and suggestions (from QI-1170 comments relevant to this PR):
- Major focus: reduce DB operations, remove synchronization/waits, investigate reconnection bottlenecks, optimize heavy parts of single reconnect tasks.
- Observations that motivated code changes in this PR:
- Host http-call path includes a host-deleted check that affects performance; suggestion: replace with deletion-event mechanism (comment references "先 h 删了这部分逻辑").
- Each webhook currently queries DB on every event; suggestion: cache webhooks and switch to event-driven updates (comment references removing related logic because APIs marked deprecated). This aligns with PR edits that bypass per-fire webhook SQL and optimize glob matching.
- "matches" (glob/regex matching for events) accounted for ~20% memory; optimizing glob handling reduced YGC frequency. This directly relates to PR changes that guard glob handling and use exact-match when no glob.
- Other performance notes: recordElaboration is CPU-heavy (recommend disable via enableElaboration=false), transaction blocking/unreturned connection issues, and suggestions to avoid heavy computation inside DB transactions.
-
Links referenced in the Jira comments (for reviewer traceability):
- MR synchronized from GitLab: http://dev.zstack.io:9080/zstackio/zstack/-/merge_requests/9021 (mentioned as source of related changes).
- Related premium MR: http://dev.zstack.io:9080/zstackio/premium/-/merge_requests/12750/diffs (mentioned).
Sources used:
- AtlassianMCP jira_get_issue for QI-1170 (issue details and full comment history).
🔇 Additional comments (5)
core/src/main/java/org/zstack/core/cloudbus/EventFacadeImpl.java (2)
259-262: 事件匹配优化逻辑正确当
wrapper.glob为 null 时使用精确路径匹配，否则使用正则匹配。这是合理的性能优化：
- 对于非 glob 路径，直接使用
equals()比matches()快得多- 这与 QI-1170 中提到的减少 glob 匹配内存开销的目标一致
132-134: Glob 路径检测逻辑正确，无需修改
isGlobPath方法通过检测*、?或{来识别 glob 路径，相关的正则生成和匹配逻辑都是正确的。虽然理论上单独的{字符（不配对的}）可能导致行为异常，但代码库中的所有实际调用都遵循正确的占位符格式（如{name}），因此这不是现实中的问题。该优化改进是合理的。plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java (1)
242-249: 重构改进：直接传递 VO 对象避免重复查询将
getHostContext方法改为接收KVMHostVO对象而非 UUID 是合理的优化：
- 避免了
getHost方法中已经查询到kvo后再次通过 UUID 查询的冗余操作- 符合 PR 目标中减少数据库操作的要求
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java (1)
17-17: 请确认ExternalPrimaryStorageVO的 import 是否必要（可能触发 CI/Checkstyle）
如果后续代码未使用该类型，建议删除该 import，避免风格检查失败。plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.java (1)
1743-1746: 删除：此处不存在空指针异常风险。
getAttachedPrimaryStorageTypes()返回的集合对象在KVMHost.java:5193-5198初始化为new HashSet(SQL.New(...).list())，即使查询结果为空，也会返回空 HashSet 而非 null。该值在传递给所有扩展之前就已设置，因此在createKvmHostConnectingFlow()方法中，getAttachedPrimaryStorageTypes()永远不会返回 null，不需要添加额外的空值检查。此处代码模式与 CephKvmExtension、ExternalPrimaryStorageKvmFactory、SMP 存储扩展完全一致，都是直接调用contains()方法，证明该模式是安全的。Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Comment on lines
510
to
516
| public void success(T ret) { | ||
| if (dbf.isExist(self.getUuid(), HostVO.class)) { | ||
| //if (dbf.isExist(self.getUuid(), HostVO.class)) { | ||
| completion.success(ret); | ||
| } else { | ||
| completion.fail(operr("host[uuid:%s] has been deleted", self.getUuid())); | ||
| } | ||
| //} else { | ||
| // completion.fail(operr("host[uuid:%s] has been deleted", self.getUuid())); | ||
| //} | ||
| } |
There was a problem hiding this comment.
异步 HTTP 回调对“Host 已删除”的处理现在不一致，可能改变语义
commandStr != null 分支不再检查 Host 是否存在，但 cmd 分支仍会在 Host 删除时 fail；这会导致同类请求行为不一致，并且可能让已删除 Host 的回调继续推进后续流程。建议：要么两边都保留检查，要么两边都移除并在 Host 删除事件/任务取消机制里统一兜底，同时加注释说明取舍。
🤖 Prompt for AI Agents
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java` around lines 510 - 516,
The async HTTP callback's success(T) handling is inconsistent: the branch for
commandStr != null no longer checks host existence while the cmd branch still
calls dbf.isExist(self.getUuid(), HostVO.class) and fails if the Host was
deleted, leading to divergent semantics; make the behavior consistent by either
(A) restoring the host-existence guard in the commandStr != null success path
(use dbf.isExist(self.getUuid(), HostVO.class) and call
completion.fail(operr(...)) when missing) or (B) removing the host-existence
checks from both branches and ensuring Host deletion is handled centrally via
the Host deletion/task-cancel mechanism; update the success(T ret)
implementation so both code paths (commandStr != null branch and cmd branch)
behave identically and add a short comment near success(T) explaining the chosen
approach (referencing success(T), completion.success/completion.fail,
commandStr, cmd, dbf.isExist, HostVO, and self.getUuid()).
Resolves: QI-1170 Change-Id: I7163747275767a6e736a6e76707679776b69746f
MatheMatrix
force-pushed
the
sync/jin.ma/hotfix-large-env
branch
from
31c870b to
65008ff
Compare
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: QI-1170
Change-Id: I7163747275767a6e736a6e76707679776b69746f
sync from gitlab !9021