Skip to content

<fix>[sdnController]: set lacp mode when change host #3145

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

Closed
MatheMatrix wants to merge 1 commit into from
+22 −4
Closed

<fix>[sdnController]: set lacp mode when change host #3145

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
26 changes: 22 additions & 4 deletions plugin/sdnController/src/main/java/org/zstack/sdnController/SdnControllerApiInterceptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,16 +200,34 @@ private void validate(APISdnControllerChangeHostMsg msg) {
msg.setNicNames(new ArrayList<>(nicNamePciAddressMap.keySet()));
}

if (msg.getNicNames().size() > 1 && msg.getBondMode() == null) {
msg.setBondMode(refVO.getBondMode());
if (msg.getBondMode() == null) {
throw new ApiMessageInterceptionException(argerr("could not change host[uuid:%s] of sdn controller[uuid:%s], " +
" because bond mode is not specified", msg.getHostUuid(), msg.getSdnControllerUuid()));
}

if (msg.getNicNames().isEmpty()) {
throw new ApiMessageInterceptionException(argerr("could not change host[uuid:%s] of sdn controller[uuid:%s], " +
" because nic names is empty", msg.getHostUuid(), msg.getSdnControllerUuid()));
}

if (msg.getLacpMode() == null) {
if (msg.getNicNames().size() == 1) {
msg.setBondMode(null);
msg.setLacpMode(null);
return;
}

if (msg.getBondMode().equals(refVO.getBondMode())) {
msg.setLacpMode(refVO.getLacpMode());
}

if (msg.getBondMode() != null && msg.getBondMode().equals(L2NetworkConstant.BONDING_MODE_TCP) && msg.getLacpMode() == null) {
if (msg.getBondMode().equals(L2NetworkConstant.BONDING_MODE_TCP)) {
msg.setLacpMode(L2NetworkConstant.LACP_MODE_ACTIVE);
} else if (msg.getBondMode().equals(L2NetworkConstant.BONDING_MODE_SLB)) {
if (!(msg.getLacpMode().equals(L2NetworkConstant.LACP_MODE_PASSIVE) || msg.getLacpMode().equals(L2NetworkConstant.LACP_MODE_OFF))) {
msg.setLacpMode(L2NetworkConstant.LACP_MODE_OFF);
}
} else {
msg.setLacpMode(L2NetworkConstant.LACP_MODE_OFF);
}
Comment on lines +220 to 232
Copy link

@coderabbitai coderabbitai bot Dec 26, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

修复关键的逻辑错误和潜在的空指针异常。

存在两个严重问题：

  1. 逻辑错误：第 220-222 行的 if 语句保存了 refVO 的 lacpMode，但后续第 224-232 行的 if 语句会无条件执行并覆盖该值。这导致第 221 行的设置完全失效。

  2. 潜在的 NPE：第 227 行在 bondMode 为 SLB 时调用 msg.getLacpMode().equals()，如果 msg.getLacpMode() 为 null 会抛出空指针异常。

🔎 建议的修复方案 
 if (msg.getBondMode().equals(refVO.getBondMode())) {
     msg.setLacpMode(refVO.getLacpMode());
-}
-
-if (msg.getBondMode().equals(L2NetworkConstant.BONDING_MODE_TCP)) {
+} else if (msg.getBondMode().equals(L2NetworkConstant.BONDING_MODE_TCP)) {
     msg.setLacpMode(L2NetworkConstant.LACP_MODE_ACTIVE);
 } else if (msg.getBondMode().equals(L2NetworkConstant.BONDING_MODE_SLB)) {
-    if (!(msg.getLacpMode().equals(L2NetworkConstant.LACP_MODE_PASSIVE) || msg.getLacpMode().equals(L2NetworkConstant.LACP_MODE_OFF))) {
+    if (msg.getLacpMode() == null || 
+        !(msg.getLacpMode().equals(L2NetworkConstant.LACP_MODE_PASSIVE) || msg.getLacpMode().equals(L2NetworkConstant.LACP_MODE_OFF))) {
         msg.setLacpMode(L2NetworkConstant.LACP_MODE_OFF);
     }
 } else {
     msg.setLacpMode(L2NetworkConstant.LACP_MODE_OFF);
 }
🤖 Prompt for AI Agents 
In
plugin/sdnController/src/main/java/org/zstack/sdnController/SdnControllerApiInterceptor.java
around lines 220-232, the code currently sets msg.lacpMode from refVO then
unconditionally overwrites it and calls equals on possibly-null values; change
the logic to (1) use null-safe comparisons (e.g. Objects.equals or
constant.equals(var)) everywhere to avoid NPEs, (2) only apply the refVO
lacpMode when the incoming msg.lacpMode is null or when bondMode differs (so it
isn't immediately overwritten), and (3) when applying bond-mode defaults, only
set lacpMode if it is still null or not explicitly provided; update the
conditional order/guards accordingly so refVO values are preserved when intended
and no equals() is called on a null string.

}

Expand Down