Skip to content

ZSTAC-81797: iSCSI fencer use platform compact hostId instead of storage bdc id #3423

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
zstack-robot-2 wants to merge 1 commit into
base: 5.5.12
Choose a base branch
from
+18 −5
Open

ZSTAC-81797: iSCSI fencer use platform compact hostId instead of storage bdc id #3423

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
23 changes: 18 additions & 5 deletions plugin/iscsi/src/main/java/org/zstack/iscsi/kvm/KvmIscsiNodeServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,8 @@
import org.zstack.header.host.HostInventory;
import org.zstack.header.host.HostVO;
import org.zstack.header.message.MessageReply;
import org.zstack.header.storage.addon.primary.BaseVolumeInfo;
import org.zstack.header.storage.addon.primary.HeartbeatVolumeTO;
import org.zstack.header.storage.addon.primary.HeartbeatVolumeTopology;
import org.zstack.header.storage.addon.primary.PrimaryStorageNodeSvc;
import org.zstack.header.storage.addon.primary.*;
import org.zstack.storage.addon.primary.ExternalHostIdGetter;
import org.zstack.header.vm.VmInstanceInventory;
import org.zstack.header.vm.VmInstanceMigrateExtensionPoint;
import org.zstack.header.vm.VmInstanceSpec;
Expand All @@ -38,6 +36,8 @@
import org.zstack.kvm.*;
import org.zstack.storage.addon.primary.ExternalPrimaryStorageFactory;
import org.zstack.utils.DebugUtils;
import org.zstack.utils.Utils;
import org.zstack.utils.logging.CLogger;

import java.util.ArrayList;
import java.util.List;
Expand All @@ -49,6 +49,8 @@
public class KvmIscsiNodeServer implements Component, KVMStartVmExtensionPoint, VmInstanceMigrateExtensionPoint,
KVMConvertVolumeExtensionPoint, KVMDetachVolumeExtensionPoint, KVMAttachVolumeExtensionPoint,
KVMPreAttachIsoExtensionPoint, KvmSetupSelfFencerExtensionPoint {
private static final CLogger logger = Utils.getLogger(KvmIscsiNodeServer.class);

@Autowired
private ExternalPrimaryStorageFactory extPsFactory;

Expand Down Expand Up @@ -235,13 +237,24 @@ public void fail(ErrorCode errorCode) {

@Override
public void run(FlowTrigger trigger, Map data) {
ExternalPrimaryStorageHostRefVO ref = Q.New(ExternalPrimaryStorageHostRefVO.class)
.eq(ExternalPrimaryStorageHostRefVO_.hostUuid, param.getHostUuid())
.eq(ExternalPrimaryStorageHostRefVO_.primaryStorageUuid, param.getPrimaryStorage().getUuid())
.find();
if (ref == null || ref.getHostId() == 0) {
logger.warn(String.format("not found hostId for hostUuid[%s] and primaryStorageUuid[%s]",
param.getHostUuid(), param.getPrimaryStorage().getUuid()));
ref = new ExternalHostIdGetter(999).getOrAllocateHostIdRef(
param.getHostUuid(), param.getPrimaryStorage().getUuid());
}
Comment on lines +244 to +249
Copy link

@coderabbitai coderabbitai bot Mar 5, 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
set -euo pipefail

echo "[1/3] Locate ExternalHostIdGetter implementation"
fd 'ExternalHostIdGetter\.java$' 

echo -e "\n[2/3] Show ExternalHostIdGetter class and getOrAllocateHostIdRef method"
fd 'ExternalHostIdGetter\.java$' -x cat {} 

echo -e "\n[3/3] Check KvmIscsiNodeServer.java around lines 240-270"
fd 'KvmIscsiNodeServer\.java$' -x cat {} | head -n 270 | tail -n 40

Repository: MatheMatrix/zstack

Length of output: 11605

在分配 hostId 后添加二次校验，防止空指针和无效 ID。

getOrAllocateHostIdRef(...) 方法在分配失败场景下（如 ConstraintViolationException 或重复条目）会返回 null，最终的 find() 查询也可能无法检索到实体。第 257 行直接调用 ref.getHostId() 存在空指针风险。

🔧 建议修复 
                        if (ref == null || ref.getHostId() == 0) {
                            logger.warn(String.format("not found hostId for hostUuid[%s] and primaryStorageUuid[%s]",
                                    param.getHostUuid(), param.getPrimaryStorage().getUuid()));
                            ref = new ExternalHostIdGetter(999).getOrAllocateHostIdRef(
                                    param.getHostUuid(), param.getPrimaryStorage().getUuid());
                        }
+
+                        if (ref == null || ref.getHostId() <= 0) {
+                            trigger.fail(operr("failed to get valid hostId for hostUuid[%s], primaryStorageUuid[%s]",
+                                    param.getHostUuid(), param.getPrimaryStorage().getUuid()));
+                            return;
+                        }

                         KvmSetupSelfFencerCmd cmd = new KvmSetupSelfFencerCmd();
📝 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 (ref == null || ref.getHostId() == 0) {
logger.warn(String.format("not found hostId for hostUuid[%s] and primaryStorageUuid[%s]",
param.getHostUuid(), param.getPrimaryStorage().getUuid()));
ref = new ExternalHostIdGetter(999).getOrAllocateHostIdRef(
param.getHostUuid(), param.getPrimaryStorage().getUuid());
}
if (ref == null || ref.getHostId() == 0) {
logger.warn(String.format("not found hostId for hostUuid[%s] and primaryStorageUuid[%s]",
param.getHostUuid(), param.getPrimaryStorage().getUuid()));
ref = new ExternalHostIdGetter(999).getOrAllocateHostIdRef(
param.getHostUuid(), param.getPrimaryStorage().getUuid());
}
if (ref == null || ref.getHostId() <= 0) {
trigger.fail(operr("failed to get valid hostId for hostUuid[%s], primaryStorageUuid[%s]",
param.getHostUuid(), param.getPrimaryStorage().getUuid()));
return;
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@plugin/iscsi/src/main/java/org/zstack/iscsi/kvm/KvmIscsiNodeServer.java`
around lines 244 - 249, After calling
ExternalHostIdGetter.getOrAllocateHostIdRef(...) you must re-check that ref is
non-null and ref.getHostId() is a valid non-zero ID before using it to avoid
NullPointerException or invalid IDs; update the block around ref/param (where
logger.warn is used and getOrAllocateHostIdRef is invoked) to validate ref !=
null and ref.getHostId() != 0, handle the failure case (log an error and
abort/throw/return appropriate response) instead of proceeding to use
ref.getHostId(), and ensure any subsequent logic that relied on a successful
allocation (e.g., the code after line 257 that accesses ref.getHostId()) is
guarded by this check.


KvmSetupSelfFencerCmd cmd = new KvmSetupSelfFencerCmd();
cmd.interval = param.getInterval();
cmd.maxAttempts = param.getMaxAttempts();
cmd.coveringPaths = heartbeatVol.getCoveringPaths();
cmd.heartbeatUrl = heartbeatVol.getInstallPath();
cmd.storageCheckerTimeout = param.getStorageCheckerTimeout();
cmd.hostId = heartbeatVol.getHostId();
cmd.hostId = ref.getHostId();
cmd.heartbeatRequiredSpace = heartbeatVol.getHeartbeatRequiredSpace();
cmd.hostUuid = param.getHostUuid();
cmd.strategy = param.getStrategy();
Expand Down