Skip to content

WIP <fix>[cloudbus]: remove some db op #3202

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MatheMatrix wants to merge 1 commit into
base: 5.4.2
Choose a base branch
from
Open

WIP <fix>[cloudbus]: remove some db op #3202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/src/main/java/org/zstack/core/CoreGlobalProperty.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public class CoreGlobalProperty {
public static boolean UPDATE_PKG_WHEN_CONNECT;
@GlobalProperty(name = "syncNodeTime", defaultValue = "true")
public static boolean SYNC_NODE_TIME;
@GlobalProperty(name = "enableElaboration", defaultValue = "true")
@GlobalProperty(name = "enableElaboration", defaultValue = "false")
public static boolean ENABLE_ELABORATION;
Comment on lines +88 to 89
Copy link

@coderabbitai coderabbitai bot Jan 14, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 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 -20

Repository: 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 -30

Repository: 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 -10

Repository: 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 -20

Repository: 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.java

Repository: 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 -5

Repository: MatheMatrix/zstack

Length of output: 679

明确升级迁移策略或添加配置保留机制。

将 ENABLE_ELABORATION 的默认值从 "true" 改为 "false" 在升级现有部署时会产生破坏性变更。经代码分析，GlobalConfigFacadeImpl 的升级逻辑会在以下场景改写该配置：

  • 若用户未曾显式设置 enableElaboration（即 defaultValue == value），升级时会将配置值同时更新为新的默认值 "false"（详见 GlobalConfigFacadeImpl 第 390-441 行）
  • 只有用户显式定制过该属性的部署才能保留自己的设置

由于 recordElaboration 性能优化的需要，禁用 elaboration 是合理的。但根据编码规范的向后兼容原则，建议：

  1. 添加升级迁移脚本，为已有安装显式设置 enableElaboration=true，以保留现有行为
  2. 或在升级文档中明确说明此行为变更，并提供用户手动调整的指引

@GlobalProperty(name = "recordElaboration", defaultValue = "false")
public static boolean RECORD_TO_DB_ELABORATION;
Expand Down
16 changes: 12 additions & 4 deletions core/src/main/java/org/zstack/core/cloudbus/EventFacadeImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ private class CallbackWrapper {

CallbackWrapper(String path, AbstractEventFacadeCallback callback) {
this.path = path;
this.glob = createRegexFromGlob(path.replaceAll("\\{.*\\}", ".*"));
if (isGlobPath(path)) {
this.glob = createRegexFromGlob(path.replaceAll("\\{.*\\}", ".*"));
}
this.callback = callback;
if (callback instanceof AutoOffEventCallback) {
hasRun = new AtomicBoolean(false);
Expand Down Expand Up @@ -127,6 +129,10 @@ void call(CanonicalEvent e) {
}
}

private boolean isGlobPath(String path) {
return path.contains("*") || path.contains("?") || path.contains("{");
}

public String createRegexFromGlob(String glob) {
String out = "^";
for(int i = 0; i < glob.length(); ++i) {
Expand Down Expand Up @@ -223,7 +229,7 @@ public void fire(String path, Object data) {

fireLocal(evt);

callWebhooks(evt);
// callWebhooks(evt);
Copy link

@coderabbitai coderabbitai bot Jan 14, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Webhook 功能被静默禁用，缺少必要的注释说明

直接注释掉 callWebhooks(evt) 会导致事件触发时不再调用 webhook，这是一个行为变更。根据 PR 上下文，这是为了优化 webhook 查询性能的临时措施，但代码中没有任何说明：

  1. 为什么禁用此功能
  2. 何时/如何恢复
  3. 对用户的影响

建议添加 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.


bus.publish(evt);
}
Expand All @@ -250,7 +256,8 @@ private void fireLocal(CanonicalEvent cevt) {
wrappers.putAll(local);

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);
}
}
Expand All @@ -271,7 +278,8 @@ public boolean handleEvent(Event evt) {
Map<String, CallbackWrapper> wrappers = new HashMap<>();
wrappers.putAll(global);
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);
}
}
Expand Down
4 changes: 4 additions & 0 deletions plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephKvmExtension.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,10 @@ public void done(ErrorCodeList errorCodeList) {

@Override
public Flow createKvmHostConnectingFlow(final KVMHostConnectedContext context) {
if (!context.getAttachedPrimaryStorageTypes().contains(CephConstants.CEPH_PRIMARY_STORAGE_TYPE)) {
return new NopeFlow();
}

Comment on lines +132 to +135
Copy link

@coderabbitai coderabbitai bot Jan 14, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

潜在的空指针异常风险，与其他存储扩展存在相同问题。

与 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.

return new NoRollbackFlow() {
String __name__ = "prepare-ceph-primary-storage";

Expand Down
5 changes: 5 additions & 0 deletions ...rc/main/java/org/zstack/externalStorage/primary/kvm/ExternalPrimaryStorageKvmFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.zstack.header.volume.VolumeVO;
import org.zstack.header.volume.VolumeVO_;
import org.zstack.kvm.*;
import org.zstack.storage.addon.primary.ExternalPrimaryStorageConstants;
Copy link

@coderabbitai coderabbitai bot Jan 14, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

删除未使用的导入。

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.

import org.zstack.storage.addon.primary.ExternalPrimaryStorageFactory;
import org.zstack.utils.CollectionUtils;
import org.zstack.utils.Utils;
Expand Down Expand Up @@ -79,6 +80,10 @@ private Map<String, PrimaryStorageHostStatus> getHostStatus(List<ExternalPrimary

@Override
public Flow createKvmHostConnectingFlow(KVMHostConnectedContext context) {
if (!context.getAttachedPrimaryStorageTypes().contains(PrimaryStorageConstant.EXTERNAL_PRIMARY_STORAGE_TYPE)) {
return new NopeFlow();
}

Comment on lines +83 to +86
Copy link

@coderabbitai coderabbitai bot Jan 14, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

潜在的空指针异常风险，与其他存储扩展存在相同问题。

与 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.

List<ExternalPrimaryStorageVO> extPss = findExternalPsByClusterUuid(context.getInventory().getClusterUuid());
if (extPss.isEmpty()) {
return new NopeFlow();
Expand Down
17 changes: 12 additions & 5 deletions plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import org.zstack.compute.vm.*;
import org.zstack.core.timeout.TimeHelper;
import org.zstack.header.core.*;
import org.zstack.header.storage.addon.primary.ExternalPrimaryStorageVO;
import org.zstack.header.vm.devices.VirtualDeviceInfo;
import org.zstack.header.vm.devices.VmInstanceDeviceManager;
import org.zstack.core.CoreGlobalProperty;
Expand Down Expand Up @@ -507,11 +508,11 @@ public void fail(ErrorCode err) {

@Override
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()));
//}
}
Comment on lines 510 to 516
Copy link

@coderabbitai coderabbitai bot Jan 14, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

异步 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()).


@Override
Expand Down Expand Up @@ -5188,13 +5189,19 @@ private void continueConnect(final ConnectHostInfo info, final Completion comple
chain.setName(String.format("continue-connecting-kvm-host-%s-%s", self.getManagementIp(), self.getUuid()));
chain.getData().put(KVMConstant.CONNECT_HOST_PRIMARYSTORAGE_ERROR, new ErrorCodeList());
chain.allowWatch();

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);
chain.then(extp.createKvmHostConnectingFlow(ctx));
}

Expand Down
11 changes: 11 additions & 0 deletions plugin/kvm/src/main/java/org/zstack/kvm/KVMHostConnectedContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
package org.zstack.kvm;

import java.util.Set;

/**
*/
public class KVMHostConnectedContext {
private KVMHostInventory inventory;
private boolean newAddedHost;
private String baseUrl;
private String skipPackages;
private Set<String> attachedPrimaryStorageTypes;

public boolean isNewAddedHost() {
return newAddedHost;
Expand Down Expand Up @@ -39,4 +42,12 @@ public String getSkipPackages() {
public void setSkipPackages(String skipPackages) {
this.skipPackages = skipPackages;
}

public Set<String> getAttachedPrimaryStorageTypes() {
return attachedPrimaryStorageTypes;
}

public void setAttachedPrimaryStorageTypes(Set<String> attachedPrimaryStorageTypes) {
this.attachedPrimaryStorageTypes = attachedPrimaryStorageTypes;
}
}
8 changes: 6 additions & 2 deletions plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ private List<AddKVMHostMsg> loadMsgFromFile(String content, ValidateFunction<? s
@Override
public Host getHost(HostVO vo) {
KVMHostVO kvo = dbf.findByUuid(vo.getUuid(), KVMHostVO.class);
KVMHostContext context = getHostContext(vo.getUuid());
KVMHostContext context = getHostContext(kvo);
if (context == null) {
context = createHostContext(kvo);
}
Expand Down Expand Up @@ -1025,7 +1025,11 @@ public KVMHostContext createHostContext(KVMHostVO vo) {
return context;
}

public KVMHostContext getHostContext(String hostUuid) {
public KVMHostContext getHostContext(KVMHostVO kvo) {
return createHostContext(kvo);
}

public KVMHostContext getHostContextByUuid(String hostUuid) {
KVMHostVO kvo = dbf.findByUuid(hostUuid, KVMHostVO.class);
return createHostContext(kvo);
}
Comment on lines +1032 to 1035
Copy link

@coderabbitai coderabbitai bot Jan 14, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# Find all usages of getHostContextByUuid method
rg -n 'getHostContextByUuid' --type java

Repository: 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.java

Repository: 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 -100

Repository: 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 java

Repository: 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 2

Repository: 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 java

Repository: 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 -50

Repository: 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.

Expand Down
4 changes: 4 additions & 0 deletions ...alstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageCapacityUpdater.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,10 @@ protected void scripts() {
final long totalChange = rsp.getTotalCapacity() - ref.getTotalPhysicalCapacity();
final long availChange = rsp.getAvailableCapacity() - ref.getAvailablePhysicalCapacity();

if (totalChange == 0 && availChange == 0) {
return;
}

new PrimaryStorageCapacityUpdater(psUuid).run(new PrimaryStorageCapacityUpdaterRunnable() {
@Override
public PrimaryStorageCapacityVO call(PrimaryStorageCapacityVO cap) {
Expand Down
5 changes: 5 additions & 0 deletions ...n/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.zstack.header.core.workflow.Flow;
import org.zstack.header.core.workflow.FlowTrigger;
import org.zstack.header.core.workflow.NoRollbackFlow;
import org.zstack.header.core.workflow.NopeFlow;
import org.zstack.header.errorcode.ErrorCode;
import org.zstack.header.host.AddHostMessage;
import org.zstack.header.host.FailToAddHostExtensionPoint;
Expand Down Expand Up @@ -76,6 +77,10 @@ private List<String> findLocalStorageUuidByHostUuid(String clusterUuid) {

@Override
public Flow createKvmHostConnectingFlow(final KVMHostConnectedContext context) {
if (!context.getAttachedPrimaryStorageTypes().contains(LocalStorageConstants.LOCAL_STORAGE_TYPE)) {
return new NopeFlow();
}

return new NoRollbackFlow() {
String __name__ = "init-local-storage";

Expand Down
5 changes: 5 additions & 0 deletions ...maryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1740,6 +1740,11 @@ public void fail(ErrorCode errorCode) {

@Override
public Flow createKvmHostConnectingFlow(final KVMHostConnectedContext context) {
if (!context.getAttachedPrimaryStorageTypes().contains(NfsPrimaryStorageConstant.NFS_PRIMARY_STORAGE_TYPE)) {
return new NopeFlow();
}


Comment on lines +1743 to +1747
Copy link

@coderabbitai coderabbitai bot Jan 16, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

潜在空指针风险：与 LocalStorageKvmFactory 相同的 null 检查问题

context.getAttachedPrimaryStorageTypes() 可能返回 null，建议添加防御性检查：

🐛 建议修复 
-        if (!context.getAttachedPrimaryStorageTypes().contains(NfsPrimaryStorageConstant.NFS_PRIMARY_STORAGE_TYPE)) {
+        Set<String> attachedTypes = context.getAttachedPrimaryStorageTypes();
+        if (attachedTypes == null || !attachedTypes.contains(NfsPrimaryStorageConstant.NFS_PRIMARY_STORAGE_TYPE)) {
             return new NopeFlow();
         }
-

另外，第 1747 行有多余的空行，可以删除以保持代码整洁。

🤖 Prompt for AI Agents 
In
`@plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.java`
around lines 1743 - 1747, Add a defensive null check before calling
context.getAttachedPrimaryStorageTypes().contains(...) in
NfsPrimaryStorageKVMBackend so you avoid a potential NPE: first verify
context.getAttachedPrimaryStorageTypes() is non-null (and optionally empty) and
only then test for NfsPrimaryStorageConstant.NFS_PRIMARY_STORAGE_TYPE; return
new NopeFlow() when the set is null or does not contain the type. Also remove
the extra blank line after this conditional to keep the code tidy.

return new NoRollbackFlow() {
String __name__ = "remount-nfs-primary-storage";

Expand Down
5 changes: 5 additions & 0 deletions ...aredMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/KvmFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import org.zstack.header.core.workflow.Flow;
import org.zstack.header.core.workflow.FlowTrigger;
import org.zstack.header.core.workflow.NoRollbackFlow;
import org.zstack.header.core.workflow.NopeFlow;
import org.zstack.header.errorcode.ErrorCode;
import org.zstack.header.message.MessageReply;
import org.zstack.header.storage.primary.*;
Expand Down Expand Up @@ -58,6 +59,10 @@ private PrimaryStorageInventory findSMPByHostUuid(String clusterUuid) {

@Override
public Flow createKvmHostConnectingFlow(final KVMHostConnectedContext context) {
if (!context.getAttachedPrimaryStorageTypes().contains(SMPConstants.SMP_TYPE)) {
return new NopeFlow();
}

Comment on lines +62 to +65
Copy link

@coderabbitai coderabbitai bot Jan 14, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

存在空指针异常风险。

与 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.

return new NoRollbackFlow() {
String __init__ = "init-smp-primary-storage";

Expand Down
5 changes: 5 additions & 0 deletions storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorageFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.zstack.storage.addon.primary;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.security.core.parameters.P;
Copy link

@coderabbitai coderabbitai bot Jan 14, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

删除未使用的导入。

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.

import org.zstack.core.Platform;
import org.zstack.core.asyncbatch.While;
import org.zstack.core.cloudbus.CloudBus;
Expand Down Expand Up @@ -265,6 +266,10 @@ public void validateStorageProtocol(String protocol) {
}
}

public List<PrimaryStorageControllerSvc> getAllControllerSvcs() {
return new ArrayList<>(controllers.values());
}

public PrimaryStorageControllerSvc getControllerSvc(String primaryStorageUuid) {
return controllers.get(primaryStorageUuid);
}
Expand Down