<fix>[network]: ZCF-3703 rollback failed service attach

[zstack-robot-2]

Summary

Fix ZCF-3703 by cleaning failed network service attach refs.

Changes

• Remove NetworkServiceL3NetworkRefVO refs when backend attach fails.

Testing

• zstack build passed on root@172.20.13.240
• premium build passed on root@172.20.13.240
• TestZnsDhcpLifeCycleCase passed

Resolves: ZCF-3703

sync from gitlab !9897

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)

Review profile: CHILL

Plan: Pro

Run ID: fbbb5a28-1ee3-4688-b98a-cdf0f805db39

📥 Commits

Reviewing files that changed from the base of the PR and between 6b0ef26 and 5818d95.

📒 Files selected for processing (2)

• network/src/main/java/org/zstack/network/l3/L3BasicNetwork.java
• utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java

🚧 Files skipped from review as they are similar to previous changes (2)

• network/src/main/java/org/zstack/network/l3/L3BasicNetwork.java
• utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java

---

Walkthrough

该 PR 集成大量改动：引入与扩展多个网络/SDN/ZNS 接口与迁移脚本、重构与新增 VM NIC 生命周期及相关 Flow、KVM 主机 TLS/HA 与 NIC 完成/对账逻辑、Webhook 回调客户端、快照 DB 同步、Go 客户端代码生成器更新及配套测试/模拟器改动。

Changes

Full change set

Layer / File(s)	Summary
All changes (full diff) multiple files (see hidden artifact)	包含网络、VM/NIC、KVM、SDN、LoadBalancer、存储、Webhook、Go 生成器、脚本、测试和文档等所有变更；详见隐藏的变更范围清单。

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 分钟

"兔子忙审改远山，
键声敲开新接口，
网与存、主机与云，
流程扩展并排团，
一杯萝卜庆归还。" 🐇

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch sync/shixin.ruan/shixin-ZCF-3703@@2

[coderabbitai]

Actionable comments posted: 16

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)

network/src/main/java/org/zstack/network/l2/L2NetworkApiInterceptor.java (1)

211-225: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

SDN 网络的冲突检查还在依赖 physicalInterface，这里会漏检重复网络。

创建路径已经允许带 SDN controller tag 的 Linux Bridge L2 不填 physicalInterface，但这两个 SDN 分支的去重查询仍然带着 and l2.physicalInterface = :physicalInterface。一旦该字段是 null，这里就匹配不到同 controller / 同 cluster 的已有网络，重复的 VLAN/NoVlan 网络会被放过去。

这里建议和前面的 attach 校验保持一致：SDN 管理的网络去重不要再以 physicalInterface 为条件。

Also applies to: 243-255

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@network/src/main/java/org/zstack/network/l2/L2NetworkApiInterceptor.java`
around lines 211 - 225, The SDN duplicate-check query in L2NetworkApiInterceptor
is still filtering on l2.physicalInterface which causes misses when
physicalInterface is null; update both SQL.New queries (the one at the shown
block and the similar block at lines 243-255) to skip the "and
l2.physicalInterface = :physicalInterface" condition and remove the
corresponding .param("physicalInterface", ...) when checking SDN-managed
networks (i.e. when you use the SDN controller system tag
L2NetworkSystemTags.L2_NETWORK_SDN_CONTROLLER_UUID), so the de-duplication for
SDN networks is done only by virtualNetworkId, clusterUuids, type and tag.

plugin/sdnController/src/main/java/org/zstack/sdnController/SdnControllerBase.java (1)

192-200: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

用户名更新/回滚这里仍然拿的是控制器名称。

Line 192/193 现在还是用 self.getName() 做比较和回滚快照。这样一来，变更失败时会把 username 回滚成控制器 name；而你这次又把 reload/update 持久化逻辑补齐了，错误值会更稳定地写回 DB。

🛠️ 建议修改

-                if (msg.getUserName() != null && !msg.getUserName().equals(self.getName())) {
-                    chain.getData().put(SDN_CONTROLLER_USERNAME, self.getName());
+                if (msg.getUserName() != null && !msg.getUserName().equals(self.getUsername())) {
+                    chain.getData().put(SDN_CONTROLLER_USERNAME, self.getUsername());
                     self.setUsername(msg.getUserName());
                     changed = true;
                 }

Also applies to: 205-213, 236-250

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@plugin/sdnController/src/main/java/org/zstack/sdnController/SdnControllerBase.java`
around lines 192 - 200, The username rollback snapshot incorrectly uses the
controller name (self.getName()) instead of the current username, so change the
username handling in SdnControllerBase to capture and compare the controller's
username getter (replace self.getName() with the appropriate username getter,
e.g., self.getUsername() or self.getUserName() consistent with
setUsername/msg.getUserName()) when filling chain.getData() and when comparing;
apply the same fix to the other occurrences referenced (the blocks around the
205-213 and 236-250 ranges) so updates/rollbacks persist the correct username
value rather than the controller name.

rest/src/main/resources/scripts/GoInventory.groovy (1)

1359-1368: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

在 context 导入处添加占位符引用，否则生成的 other_actions.go 可能因未使用的导入而编译失败

generateOtherActionsFile() 方法总是会生成 other_actions.go 文件，即使 methodsContent 为空。当所有 API 模板都被过滤掉时（例如全是手动维护的 API 或重复方法），就会出现这种情况。此时文件会包含 context 导入但没有任何代码使用它，导致 Go 编译失败。

推荐修改

         content.append("import (\n")
         content.append("\t\"context\"\n")
         if (needsFmt) {
             content.append("\t\"fmt\"\n")
         }
         content.append("\t\"github.com/zstackio/zstack-sdk-go-v2/pkg/param\"\n")
         content.append("\t\"github.com/zstackio/zstack-sdk-go-v2/pkg/view\"\n")
         content.append(")\n\n")
+        content.append("var _ = context.Background // avoid unused import\n")
         content.append("var _ = param.BaseParam{} // avoid unused import\n")
         content.append("var _ = view.MapView{} // avoid unused import\n\n")

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rest/src/main/resources/scripts/GoInventory.groovy` around lines 1359 - 1368,
The generated other_actions.go can import "context" without using it when
generateOtherActionsFile() emits an empty methodsContent, causing a Go compile
error; fix by adding a placeholder reference to the context package alongside
the existing unused-import guards (e.g., similar to var _ = param.BaseParam{}
and var _ = view.MapView{}), so update the generation in GoInventory.groovy to
emit a context usage placeholder (referencing context.Background or a
context.Context zero value) whenever "context" is imported to ensure
other_actions.go compiles even when methodsContent is empty.

rest/src/main/resources/scripts/GoApiTemplate.groovy (1)

1165-1172: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

generateGenericMethod 的 PUT 调用漏传 ctx，会生成不可编译的 Go 代码

在 Line 1175、Line 1188、Line 1207、Line 1220、Line 1243、Line 1256、Line 1274、Line 1287 这些拼接调用里，cli.Put/cli.PutWithRespKey 的参数串都未包含 ctx，与本次模板签名迁移不一致，生成代码会参数不匹配编译失败。

建议修复（示例 diff）

-                                String putArgs = unwrap ? 
-                                    """("${cleanPath}", ${paramName}, map[string]interface{}{
+                                String putArgs = unwrap ? 
+                                    """(ctx, "${cleanPath}", ${paramName}, map[string]interface{}{
 \t\t"${actionKey}": map[string]interface{}{},
 \t}, &resp)""" :
-                                    """("${cleanPath}", ${paramName}, "", map[string]interface{}{
+                                    """(ctx, "${cleanPath}", ${paramName}, "", map[string]interface{}{
 \t\t"${actionKey}": map[string]interface{}{},
 \t}, &resp)"""
@@
-                                String putArgs = unwrap ? 
-                                    """("${cleanPath}", ${paramName}, map[string]interface{}{}, &resp)""" :
-                                    """("${cleanPath}", ${paramName}, "", map[string]interface{}{}, &resp)"""
+                                String putArgs = unwrap ? 
+                                    """(ctx, "${cleanPath}", ${paramName}, map[string]interface{}{}, &resp)""" :
+                                    """(ctx, "${cleanPath}", ${paramName}, "", map[string]interface{}{}, &resp)"""
@@
-                            String putArgs = unwrap ?
-                                """("${cleanPath}", ${paramName}, map[string]interface{}{
+                            String putArgs = unwrap ?
+                                """(ctx, "${cleanPath}", ${paramName}, map[string]interface{}{
 \t\t"${actionKey}": params.Params,
 \t}, &resp)""" :
-                                """("${cleanPath}", ${paramName}, "", map[string]interface{}{
+                                """(ctx, "${cleanPath}", ${paramName}, "", map[string]interface{}{
 \t\t"${actionKey}": params.Params,
 \t}, &resp)"""
@@
-                            String putArgs = unwrap ?
-                                """("${cleanPath}", ${paramName}, params, &resp)""" :
-                                """("${cleanPath}", ${paramName}, "", params, &resp)"""
+                            String putArgs = unwrap ?
+                                """(ctx, "${cleanPath}", ${paramName}, params, &resp)""" :
+                                """(ctx, "${cleanPath}", ${paramName}, "", params, &resp)"""
@@
-                                String putArgs = unwrap ?
-                                    """("${cleanPath}", "", map[string]interface{}{
+                                String putArgs = unwrap ?
+                                    """(ctx, "${cleanPath}", "", map[string]interface{}{
 \t\t"${actionKey}": map[string]interface{}{},
 \t}, &resp)""" :
-                                    """("${cleanPath}", "", "", map[string]interface{}{
+                                    """(ctx, "${cleanPath}", "", "", map[string]interface{}{
 \t\t"${actionKey}": map[string]interface{}{},
 \t}, &resp)"""
@@
-                                String putArgs = unwrap ?
-                                    """("${cleanPath}", "", map[string]interface{}{}, &resp)""" :
-                                    """("${cleanPath}", "", "", map[string]interface{}{}, &resp)"""
+                                String putArgs = unwrap ?
+                                    """(ctx, "${cleanPath}", "", map[string]interface{}{}, &resp)""" :
+                                    """(ctx, "${cleanPath}", "", "", map[string]interface{}{}, &resp)"""
@@
-                            String putArgs = unwrap ?
-                                """("${cleanPath}", "", map[string]interface{}{
+                            String putArgs = unwrap ?
+                                """(ctx, "${cleanPath}", "", map[string]interface{}{
 \t\t"${actionKey}": params.Params,
 \t}, &resp)""" :
-                                """("${cleanPath}", "", "", map[string]interface{}{
+                                """(ctx, "${cleanPath}", "", "", map[string]interface{}{
 \t\t"${actionKey}": params.Params,
 \t}, &resp)"""
@@
-                            String putArgs = unwrap ?
-                                """("${cleanPath}", uuid, params, &resp)""" :
-                                """("${cleanPath}", uuid, "", params, &resp)"""
+                            String putArgs = unwrap ?
+                                """(ctx, "${cleanPath}", uuid, params, &resp)""" :
+                                """(ctx, "${cleanPath}", uuid, "", params, &resp)"""

Also applies to: 1182-1185, 1197-1204, 1214-1217, 1232-1239, 1249-1252, 1263-1270, 1280-1283

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rest/src/main/resources/scripts/GoApiTemplate.groovy` around lines 1165 -
1172, The PUT call argument strings built in generateGenericMethod (variables
putMethod/putArgs) are missing the context parameter and will produce
non-compiling Go code; update every occurrence that builds cli.Put or
cli.PutWithRespKey argument strings (e.g., where putArgs uses "${cleanPath}",
${paramName}, ... and the unwrap ternary) to include ctx as the first argument
(e.g., ctx, "${cleanPath}", ${paramName}, ...) so all generated calls match the
new signature; apply the same ctx insertion to all similar concatenations
referenced in the comment (lines around the occurrences for unwrap,
putMethod/putArgs, and any other cli.Put/cli.PutWithRespKey constructions).

🧹 Nitpick comments (8)

compute/src/main/java/org/zstack/compute/vm/VmAllocateSdnNicFlow.java (1)

87-93: ⚡ Quick win

考虑记录所有扩展点失败错误

当前实现仅使用 errorCodeList.getCauses().get(0) 触发失败，这会丢弃其他扩展点返回的错误信息。建议在选取第一个错误之前，先将完整的错误列表记录到日志中，以便问题排查。

📝 建议的改进方案

 }).run(new WhileDoneCompletion(trigger) {
     `@Override`
     public void done(ErrorCodeList errorCodeList) {
         if (!errorCodeList.getCauses().isEmpty()) {
+            if (errorCodeList.getCauses().size() > 1) {
+                logger.warn(String.format("multiple SDN nic allocation failures occurred: %s", errorCodeList));
+            }
             trigger.fail(errorCodeList.getCauses().get(0));
         } else {
             trigger.next();
         }
     }
 });

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@compute/src/main/java/org/zstack/compute/vm/VmAllocateSdnNicFlow.java` around
lines 87 - 93, In VmAllocateSdnNicFlow's done method, when errorCodeList
contains causes, log the full list of errors from errorCodeList (e.g.,
errorCodeList.getCauses()) before calling trigger.fail with the first cause;
ensure the log includes context (method/class and any identifying VM/request
info) so all extension-point failures are preserved in logs even though
trigger.fail(errorCodeList.getCauses().get(0)) is still used to signal failure.

plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerConstants.java (1)

32-32: ⚖️ Poor tradeoff

注意：继承了已有的拼写错误

新增的常量名称 HEALTH_CHECK_TARGET_PROTOCL_NONE 中 "PROTOCL" 拼写有误（应为 "PROTOCOL"），但为了与现有常量（如 HEALTH_CHECK_TARGET_PROTOCL_TCP）保持一致而沿用了错误拼写。这是可以接受的做法，以避免破坏向后兼容性。

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerConstants.java`
at line 32, The constant HEALTH_CHECK_TARGET_PROTOCL_NONE contains a historical
misspelling ("PROTOCL") that must be preserved for compatibility; add a
correctly spelled alias constant HEALTH_CHECK_TARGET_PROTOCOL_NONE that equals
HEALTH_CHECK_TARGET_PROTOCL_NONE (and mark the original
HEALTH_CHECK_TARGET_PROTOCL_NONE with `@Deprecated` javadoc/@Deprecated if
desired) so new code can use the correct name while existing callers remain
unaffected; update references in code to prefer
HEALTH_CHECK_TARGET_PROTOCOL_NONE where possible.

header/src/main/java/org/zstack/header/vm/AfterUpdateVmNicMacExtensionPoint.java (1)

3-4: ⚡ Quick win

请为扩展点方法补充 Javadoc 契约说明。

这是公共扩展接口，建议明确回调时机和参数语义，减少实现歧义。

♻️ 建议修改

 public interface AfterUpdateVmNicMacExtensionPoint {
+    /**
+     * Called after a VM NIC MAC address is updated.
+     *
+     * `@param` nic the updated VM NIC inventory
+     * `@param` oldMac previous MAC address
+     * `@param` newMac current MAC address
+     */
     void afterUpdateVmNicMac(VmNicInventory nic, String oldMac, String newMac);
 }

As per coding guidelines "接口方法不应有多余的修饰符（例如 public），且必须配有有效的 Javadoc 注释"。

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@header/src/main/java/org/zstack/header/vm/AfterUpdateVmNicMacExtensionPoint.java`
around lines 3 - 4, 为公共扩展点 AfterUpdateVmNicMacExtensionPoint 和其方法
afterUpdateVmNicMac 补充 Javadoc，说明回调时机（在 VM NIC 的 MAC
地址已成功更新且持久化后触发）、参数语义（VmNicInventory nic 表示更新后的网卡快照，oldMac 为更新前的 MAC，newMac 为更新后的
MAC）、可空性约定（明确 oldMac/newMac 是否可能为 null）以及实现者的行为约束（不应抛出运行时异常、避免长时间阻塞或进行阻塞
IO）。同时去掉接口方法上多余的 public 修饰符，保留方法签名 afterUpdateVmNicMac(VmNicInventory nic,
String oldMac, String newMac)。

plugin/sugonSdnController/src/main/java/org/zstack/sugonSdnController/controller/SugonSdnControllerFactory.java (1)

6-13: 💤 Low value

未使用的导入。

SdnControllerFactory 导入（第 8 行）在改为 extends AbstractSdnControllerFactory 后可能不再需要，可以考虑移除。

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@plugin/sugonSdnController/src/main/java/org/zstack/sugonSdnController/controller/SugonSdnControllerFactory.java`
around lines 6 - 13, Remove the unused import SdnControllerFactory from the top
of SugonSdnControllerFactory.java since the class now extends
AbstractSdnControllerFactory; update the import list to only include the
actually used types (e.g., AbstractSdnControllerFactory, SdnController,
SdnControllerL2, SdnControllerType, SdnControllerVO) so there are no redundant
imports referencing SdnControllerFactory.

plugin/sdnController/src/main/java/org/zstack/sdnController/h3cVcfc/H3cVcfcSdnControllerFactory.java (1)

8-9: 💤 Low value

重复的导入语句。

第 8 行和第 9 行有重复的 org.zstack.sdnController.* 导入，应删除其中一个。

♻️ 建议的修复

 import org.zstack.network.securitygroup.SecurityGroupSdnBackend;
 import org.zstack.sdnController.*;
-import org.zstack.sdnController.*;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@plugin/sdnController/src/main/java/org/zstack/sdnController/h3cVcfc/H3cVcfcSdnControllerFactory.java`
around lines 8 - 9, 在 H3cVcfcSdnControllerFactory 类的导入区发现重复的导入语句 import
org.zstack.sdnController.*；删除多余的重复行，保留单一的 import
org.zstack.sdnController.*，以消除冗余并保持导入声明唯一，修改涉及的标识符为 H3cVcfcSdnControllerFactory
和 import org.zstack.sdnController.*。

compute/src/main/java/org/zstack/compute/vm/VmDetachNicFlow.java (1)

44-46: 💤 Low value

未使用的注入字段 vmMgr

VmInstanceManager vmMgr 字段被注入但在该类中从未使用。建议移除以保持代码整洁。

♻️ 建议移除未使用字段

 `@Autowired`
-    private VmInstanceManager vmMgr;
-    `@Autowired`
     private PluginRegistry pluginRgty;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@compute/src/main/java/org/zstack/compute/vm/VmDetachNicFlow.java` around
lines 44 - 46, 类 VmDetachNicFlow 中的注入字段 VmInstanceManager vmMgr 未被使用，移除该未使用字段和其
`@Autowired` 注解以保持代码整洁；如果确实需要功能则在 VmDetachNicFlow 内使用 vmMgr 的方法（引用
VmInstanceManager 和 vmMgr）并添加相应单元测试，否则删掉字段声明和导入的 VmInstanceManager 以避免未使用的注入和导入。

header/src/main/java/org/zstack/header/network/l2/L2NetworkConstant.java (1)

3-3: 💤 Low value

未使用的导入

org.springframework.security.access.method.P 未在该文件中使用。

♻️ 建议移除未使用导入

 package org.zstack.header.network.l2;

-import org.springframework.security.access.method.P;
 import org.zstack.header.configuration.PythonClass;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@header/src/main/java/org/zstack/header/network/l2/L2NetworkConstant.java` at
line 3, The file imports an unused symbol
org.springframework.security.access.method.P; remove this unused import from the
L2NetworkConstant.java file by deleting the import line for P so the class
L2NetworkConstant no longer contains the unused import and the file compiles
without warnings.

compute/src/main/java/org/zstack/compute/vm/VmReturnReleaseNicFlow.java (1)

40-43: 💤 Low value

未使用的注入字段 vmMgr

VmInstanceManager vmMgr 字段被注入但在该类中从未使用。建议移除以保持代码整洁。

♻️ 建议移除未使用字段

 `@Autowired`
-    protected VmInstanceManager vmMgr;
-    `@Autowired`
     protected PluginRegistry pluginRgty;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@compute/src/main/java/org/zstack/compute/vm/VmReturnReleaseNicFlow.java`
around lines 40 - 43, The injected field VmInstanceManager vmMgr in class
VmReturnReleaseNicFlow is never used; remove the unused `@Autowired` protected
VmInstanceManager vmMgr declaration from VmReturnReleaseNicFlow and delete any
now-unused imports related to VmInstanceManager so the class compiles cleanly
and avoids dead injections.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@compute/src/main/java/org/zstack/compute/vm/VmAllocateNicFlow.java`:
- Around line 131-143: The code currently calls SQLBatch.execute(),
addVmNicConfig(...), and mo.deleteCustomMacSystemTag(...) without exception
handling, so any exception can skip wcomp.done() and leave a persisted VmNicVO
(nic/nicVO) as a dirty record; wrap the SQLBatch.execute() plus the following
addVmNicConfig and mo.deleteCustomMacSystemTag calls in a try/catch/finally: in
try keep the current logic, in catch log and fail the flow (call wcomp.fail(e)
or wcomp.allDone() per surrounding conventions) and attempt to cleanup the
persisted NIC (remove the VmNicVO persisted earlier via dbf.remove or
dbf.removeByUuid(nicVO.getUuid())) and revert any created tags, and in finally
ensure wcomp.done()/wcomp.allDone() is always invoked so the flow cannot hang;
reference SQLBatch, persistStaticIpIfNeeded, addVmNicConfig,
mo.deleteCustomMacSystemTag, wcomp.done()/wcomp.allDone(), VmNicVO, nic, nicVO
when making these changes.

In `@compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java`:
- Around line 3783-3784: The code calls notifyVmIpChanged(nicVO.getUuid(),
oldIpMap, newIpMap) immediately after persisting VmNicVO/UsedIpVO, which risks
failing the overall flow if a VmIpChangedExtensionPoint throws (DB changes are
not rolled back); move the post-commit notification out of this non-rollbackable
branch and invoke notifyVmIpChanged only after network services have been
reapplied (i.e. after VmApplyNetworkServiceOnChangeIPFlow completes
successfully), so that extension-point failures do not leave DB state changed
while the flow reports failure.

In `@conf/db/upgrade/V5.5.16__schema.sql`:
- Around line 357-358: The createDate column currently uses the illegal zero
timestamp DEFAULT '0000-00-00 00:00:00' which can fail under strict SQL modes;
update the definition of createDate in V5.5.16__schema.sql to use a safe
sentinel (e.g. DEFAULT '2000-01-01 00:00:00') instead of the zero value and keep
lastOpDate as TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE
CURRENT_TIMESTAMP so both TIMESTAMP columns remain compatible.

In `@conf/db/upgrade/V5.5.22__schema.sql`:
- Around line 87-116: The CREATE TABLE and FOREIGN KEY statements for
ZnsControllerVO, L2GeneveNetworkVO, ZnsTransportZoneVO (and the referenced
CONSTRAINT names fkZnsControllerVOSdnControllerVO,
fkL2GeneveNetworkVOL2NetworkEO, fkZnsTransportZoneVOSdnControllerVO) must be
fully schema-qualified with `zstack` like the rest of the script: change each
"CREATE TABLE IF NOT EXISTS `TableName`" to "CREATE TABLE IF NOT EXISTS
`zstack`.`TableName`" and update each REFERENCES target to reference
`zstack`.`ReferencedTable` (e.g., REFERENCES `zstack`.`SdnControllerVO` and
REFERENCES `zstack`.`L2NetworkEO`), and apply the same change to the other
occurrence range (lines 121-148) so tables and FK targets are consistently
prefixed with `zstack`.

In `@conf/install/zstack-server`:
- Around line 95-103: 函数 start_mariadb_service() 在没有 systemctl 时只调用 service ...
start，但探测函数 mariadb_service_exists() 把仅有 /etc/init.d/$MARIADB_SERVICE
的场景当作“服务存在”，因此需要增加与探测逻辑一致的回退链：当 command -v systemctl 不存在且 command -v service
不存在时检查 /etc/init.d/"$MARIADB_SERVICE" 是否可执行并调用它（例如
/etc/init.d/"$MARIADB_SERVICE" start），确保行为与 systemctl 分支相同并正确返回该命令的退出码，同时保留对
systemctl reset-failed 的调用和原有返回码语义（即所有分支都返回相应命令的退出状态）。

In `@docs/design/zns-pivot/ZCF-2133-zns-pivot-detailed-design.md`:
- Around line 1327-1333: The stray standalone code-fence line breaks the
following code block rendering near the "reconnectSdnController" section; remove
the isolated backticks on the lone line (or merge them with the following
"```java") so the `reconnectSdnController` example's code fence is a single
properly opened block and the subsequent Java snippet renders correctly.
- Line 1952: 文档中的三个 fenced code block 目前只使用了 ```（没有语言标记），导致持续触发 MD040；请将这三处裸 ```
替换为 ```text（即在三个出现裸 code fence 的位置将起始行由 ``` 改为 ```text），以消除 lint
警告并保持文档一致性（文件中那三个未标注语言的代码块应全部改为 ```text）。

In `@header/src/main/java/org/zstack/header/vm/VmNicInventory.java`:
- Around line 43-44: The new field interfaceId is declared with its accessor but
is not populated during VO→Inventory conversion; update the constructor
VmNicInventory(VmNicVO vo) to set this.interfaceId = vo.getInterfaceId() (and
ensure any other constructor/factory such as valueOf(VmNicVO) or the static
valueOf methods also copy interfaceId from the VmNicVO), so the Inventory
produced by valueOf(VmNicVO) contains the interfaceId value.

In
`@plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerApiInterceptor.java`:
- Around line 790-803: The HEALTH_TARGET tag is not fully validated and
malformed values can be persisted; update the validation in
LoadBalancerApiInterceptor where getSystemTagTokens(...,
LoadBalancerSystemTags.HEALTH_TARGET, ...) is processed: after extracting the
single healthCheckTarget (and before calling getHealthCheckProtocolFromTarget or
insertTagIfNotExisting), validate its format against the allowed pattern
"^(tcp|http|udp|none):(default|[1-9][0-9]{0,4})$" and ensure numeric ports are
within 1-65535 (treat "default" as allowed only for none or protocol-specific
rules), and if the token fails validation throw an
ApiMessageInterceptionException (use operr with an appropriate error message
like the existing ORG_ZSTACK_NETWORK_SERVICE_LB_* style); reference
getSystemTagTokens, LoadBalancerSystemTags.HEALTH_TARGET,
isNoneHealthCheckTargetWithSpecificPort, getHealthCheckProtocolFromTarget and
insertTagIfNotExisting when making this change so invalid tags are rejected
before persistence.

In
`@plugin/sdnController/src/main/java/org/zstack/sdnController/SdnControllerManagerImpl.java`:
- Around line 273-279: The current try/catch swallows tag creation and inventory
assembly failures causing APIs to return success without inventory or
systemTags; change the flow so that
event.setInventory(SdnControllerInventory.valueOf(dbf.findByUuid(vo.getUuid(),
SdnControllerVO.class))) is executed outside the try/catch and cannot be
suppressed by tag errors, and handle failures from
tagMgr.createTagsFromAPICreateMessage(vo.getUuid(),
SdnControllerVO.class.getSimpleName()) by either rethrowing/propagating an error
(so the create API fails) or implementing a compensating rollback path (delete
the created SdnControllerVO) before returning success; ensure logger.warn is
only used for non-fatal, truly ignorable issues, not for failures that must
surface as API errors.

In `@rest/src/main/resources/scripts/RestDocumentationGenerator.groovy`:
- Around line 853-861: The current validation only calls isEmpty() on fields
(e.g., markDown.desc_CN, markDown.name_CN) which misses cases where the section
was deleted but the parser left a non-empty placeholder; replace these plain
isEmpty() checks with a single helper check (e.g., isMissing(String s)) and use
it everywhere (markDown.desc_CN, name_CN, valueRangeRemark, defaultValueRemark,
resourcesGranularitiesRemark, additionalRemark, backgroundInformation,
isUIExposed, isCLIExposed) — the helper should treat null, s.trim().isEmpty(),
and known placeholder/default tokens (e.g., "PLACEHOLDER", "N/A", or any parser
default) as missing and also optionally consult the raw parsed-section presence
if available; then add missingFields.add(...) when isMissing(...) returns true.

In
`@storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotManagerImpl.java`:
- Around line 1037-1043: 同步数据库后应该用同步得到的 inventory 的状态来发事件，而不是继续读取可能是旧值的
snapshot.getStatus()。在调用
FireSnapShotCanonicalEvent.fireSnapShotStatusChangedEvent 时，替换
VolumeSnapshotStatus.valueOf(snapshot.getStatus()) 为基于
syncVolumeSnapshotDBAfterTakeSnapshot 返回的 VolumeSnapshotInventory sp 的状态（例如
VolumeSnapshotStatus.valueOf(sp.getStatus())），并确保对 sp.getStatus() 做非空/合法性校验后再转换。

In `@testlib/src/main/java/org/zstack/testlib/EnvSpec.groovy`:
- Around line 1038-1044: The catch block that swallows multipart read errors
must be changed so failures abort the test: in the try around
req.getParts().each (and the call to appendMultipartPart) replace the current
logger.debug-only handling with logging the error and then rethrowing (or throw
a RuntimeException) so the request processing stops; keep the debug log but
propagate the exception from the req.getParts() read instead of returning
partial content.
- Around line 1055-1057: The code uses req.fileMap which maps a field name to a
single MultipartFile and thus drops multiple files for the same field; change
the loop to iterate all uploaded files per field (use req.getMultiFileMap() or
req.getFiles(name)) and call appendMultipartPart(sb, name,
file.originalFilename, file.contentType, file.bytes) for each file to ensure all
files are included; update the usage around req.fileMap and the
appendMultipartPart invocation accordingly.

In
`@utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java`:
- Around line 15074-15097: The sequence of AI error constants in
CloudOperationsErrorCode skips ORG_ZSTACK_AI_10151–ORG_ZSTACK_AI_10156; add a
single-line reserved comment matching the project's convention (e.g. "//
10151-10156 reserved") immediately after the ORG_ZSTACK_AI_10150 declaration and
before ORG_ZSTACK_AI_10157 to document those reserved codes and keep the
numbering sequence clear and consistent.

In `@utils/src/main/java/org/zstack/utils/ssh/SshShell.java`:
- Around line 225-243: 这些超时参数的
setter（setConnectTimeoutSeconds、setServerAliveIntervalSeconds、setServerAliveCountMax）当前接受任意整数，会把负值拼接进
sshTimeoutOptions() 导致生成非法 SSH 选项；在每个 setter 中加入非负校验（value >= 0），在不满足时抛出
IllegalArgumentException（或与类中 checkParams() 一致的校验异常/行为），确保只有 >=0 的值被赋给对应字段，从而避免
sshTimeoutOptions() 拼接出非法参数。

---

Outside diff comments:
In `@network/src/main/java/org/zstack/network/l2/L2NetworkApiInterceptor.java`:
- Around line 211-225: The SDN duplicate-check query in L2NetworkApiInterceptor
is still filtering on l2.physicalInterface which causes misses when
physicalInterface is null; update both SQL.New queries (the one at the shown
block and the similar block at lines 243-255) to skip the "and
l2.physicalInterface = :physicalInterface" condition and remove the
corresponding .param("physicalInterface", ...) when checking SDN-managed
networks (i.e. when you use the SDN controller system tag
L2NetworkSystemTags.L2_NETWORK_SDN_CONTROLLER_UUID), so the de-duplication for
SDN networks is done only by virtualNetworkId, clusterUuids, type and tag.

In
`@plugin/sdnController/src/main/java/org/zstack/sdnController/SdnControllerBase.java`:
- Around line 192-200: The username rollback snapshot incorrectly uses the
controller name (self.getName()) instead of the current username, so change the
username handling in SdnControllerBase to capture and compare the controller's
username getter (replace self.getName() with the appropriate username getter,
e.g., self.getUsername() or self.getUserName() consistent with
setUsername/msg.getUserName()) when filling chain.getData() and when comparing;
apply the same fix to the other occurrences referenced (the blocks around the
205-213 and 236-250 ranges) so updates/rollbacks persist the correct username
value rather than the controller name.

In `@rest/src/main/resources/scripts/GoApiTemplate.groovy`:
- Around line 1165-1172: The PUT call argument strings built in
generateGenericMethod (variables putMethod/putArgs) are missing the context
parameter and will produce non-compiling Go code; update every occurrence that
builds cli.Put or cli.PutWithRespKey argument strings (e.g., where putArgs uses
"${cleanPath}", ${paramName}, ... and the unwrap ternary) to include ctx as the
first argument (e.g., ctx, "${cleanPath}", ${paramName}, ...) so all generated
calls match the new signature; apply the same ctx insertion to all similar
concatenations referenced in the comment (lines around the occurrences for
unwrap, putMethod/putArgs, and any other cli.Put/cli.PutWithRespKey
constructions).

In `@rest/src/main/resources/scripts/GoInventory.groovy`:
- Around line 1359-1368: The generated other_actions.go can import "context"
without using it when generateOtherActionsFile() emits an empty methodsContent,
causing a Go compile error; fix by adding a placeholder reference to the context
package alongside the existing unused-import guards (e.g., similar to var _ =
param.BaseParam{} and var _ = view.MapView{}), so update the generation in
GoInventory.groovy to emit a context usage placeholder (referencing
context.Background or a context.Context zero value) whenever "context" is
imported to ensure other_actions.go compiles even when methodsContent is empty.

---

Nitpick comments:
In `@compute/src/main/java/org/zstack/compute/vm/VmAllocateSdnNicFlow.java`:
- Around line 87-93: In VmAllocateSdnNicFlow's done method, when errorCodeList
contains causes, log the full list of errors from errorCodeList (e.g.,
errorCodeList.getCauses()) before calling trigger.fail with the first cause;
ensure the log includes context (method/class and any identifying VM/request
info) so all extension-point failures are preserved in logs even though
trigger.fail(errorCodeList.getCauses().get(0)) is still used to signal failure.

In `@compute/src/main/java/org/zstack/compute/vm/VmDetachNicFlow.java`:
- Around line 44-46: 类 VmDetachNicFlow 中的注入字段 VmInstanceManager vmMgr
未被使用，移除该未使用字段和其 `@Autowired` 注解以保持代码整洁；如果确实需要功能则在 VmDetachNicFlow 内使用 vmMgr 的方法（引用
VmInstanceManager 和 vmMgr）并添加相应单元测试，否则删掉字段声明和导入的 VmInstanceManager 以避免未使用的注入和导入。

In `@compute/src/main/java/org/zstack/compute/vm/VmReturnReleaseNicFlow.java`:
- Around line 40-43: The injected field VmInstanceManager vmMgr in class
VmReturnReleaseNicFlow is never used; remove the unused `@Autowired` protected
VmInstanceManager vmMgr declaration from VmReturnReleaseNicFlow and delete any
now-unused imports related to VmInstanceManager so the class compiles cleanly
and avoids dead injections.

In `@header/src/main/java/org/zstack/header/network/l2/L2NetworkConstant.java`:
- Line 3: The file imports an unused symbol
org.springframework.security.access.method.P; remove this unused import from the
L2NetworkConstant.java file by deleting the import line for P so the class
L2NetworkConstant no longer contains the unused import and the file compiles
without warnings.

In
`@header/src/main/java/org/zstack/header/vm/AfterUpdateVmNicMacExtensionPoint.java`:
- Around line 3-4: 为公共扩展点 AfterUpdateVmNicMacExtensionPoint 和其方法
afterUpdateVmNicMac 补充 Javadoc，说明回调时机（在 VM NIC 的 MAC
地址已成功更新且持久化后触发）、参数语义（VmNicInventory nic 表示更新后的网卡快照，oldMac 为更新前的 MAC，newMac 为更新后的
MAC）、可空性约定（明确 oldMac/newMac 是否可能为 null）以及实现者的行为约束（不应抛出运行时异常、避免长时间阻塞或进行阻塞
IO）。同时去掉接口方法上多余的 public 修饰符，保留方法签名 afterUpdateVmNicMac(VmNicInventory nic,
String oldMac, String newMac)。

In
`@plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerConstants.java`:
- Line 32: The constant HEALTH_CHECK_TARGET_PROTOCL_NONE contains a historical
misspelling ("PROTOCL") that must be preserved for compatibility; add a
correctly spelled alias constant HEALTH_CHECK_TARGET_PROTOCOL_NONE that equals
HEALTH_CHECK_TARGET_PROTOCL_NONE (and mark the original
HEALTH_CHECK_TARGET_PROTOCL_NONE with `@Deprecated` javadoc/@Deprecated if
desired) so new code can use the correct name while existing callers remain
unaffected; update references in code to prefer
HEALTH_CHECK_TARGET_PROTOCOL_NONE where possible.

In
`@plugin/sdnController/src/main/java/org/zstack/sdnController/h3cVcfc/H3cVcfcSdnControllerFactory.java`:
- Around line 8-9: 在 H3cVcfcSdnControllerFactory 类的导入区发现重复的导入语句 import
org.zstack.sdnController.*；删除多余的重复行，保留单一的 import
org.zstack.sdnController.*，以消除冗余并保持导入声明唯一，修改涉及的标识符为 H3cVcfcSdnControllerFactory
和 import org.zstack.sdnController.*。

In
`@plugin/sugonSdnController/src/main/java/org/zstack/sugonSdnController/controller/SugonSdnControllerFactory.java`:
- Around line 6-13: Remove the unused import SdnControllerFactory from the top
of SugonSdnControllerFactory.java since the class now extends
AbstractSdnControllerFactory; update the import list to only include the
actually used types (e.g., AbstractSdnControllerFactory, SdnController,
SdnControllerL2, SdnControllerType, SdnControllerVO) so there are no redundant
imports referencing SdnControllerFactory.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

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

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)

Review profile: CHILL

Plan: Pro

Run ID: c3cb75b7-3e6b-4010-814a-e8a36adffdf4

📥 Commits

Reviewing files that changed from the base of the PR and between 5abf764 and addf72f.

⛔ Files ignored due to path filters (68)

• build/pom.xml is excluded by !**/*.xml
• conf/globalConfig/vmNicLifecycle.xml is excluded by !**/*.xml
• conf/i18n/globalErrorCodeMapping/global-error-de-DE.json is excluded by !**/*.json
• conf/i18n/globalErrorCodeMapping/global-error-en_US.json is excluded by !**/*.json
• conf/i18n/globalErrorCodeMapping/global-error-fr-FR.json is excluded by !**/*.json
• conf/i18n/globalErrorCodeMapping/global-error-id-ID.json is excluded by !**/*.json
• conf/i18n/globalErrorCodeMapping/global-error-ja-JP.json is excluded by !**/*.json
• conf/i18n/globalErrorCodeMapping/global-error-ko-KR.json is excluded by !**/*.json
• conf/i18n/globalErrorCodeMapping/global-error-ru-RU.json is excluded by !**/*.json
• conf/i18n/globalErrorCodeMapping/global-error-th-TH.json is excluded by !**/*.json
• conf/i18n/globalErrorCodeMapping/global-error-zh_CN.json is excluded by !**/*.json
• conf/i18n/globalErrorCodeMapping/global-error-zh_TW.json is excluded by !**/*.json
• conf/serviceConfig/externalPrimaryStorage.xml is excluded by !**/*.xml
• conf/serviceConfig/sdnController.xml is excluded by !**/*.xml
• conf/springConfigXml/Kvm.xml is excluded by !**/*.xml
• conf/springConfigXml/VmInstanceManager.xml is excluded by !**/*.xml
• conf/springConfigXml/VolumeManager.xml is excluded by !**/*.xml
• conf/springConfigXml/sdnController.xml is excluded by !**/*.xml
• conf/springConfigXml/volumeSnapshot.xml is excluded by !**/*.xml
• sdk/src/main/java/SourceClassMap.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/AddModelAction.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/AutoMatchModelServiceByModelAction.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/AutoMatchModelServiceByModelResult.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/BareMetal2DpuChassisConfig.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/ChangeHaNetworkGroupStateAction.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/ChangeHaNetworkGroupStateResult.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/ChangeLoadBalancerListenerAction.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/CreateHaNetworkGroupAction.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/CreateHaNetworkGroupResult.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/CreateLoadBalancerListenerAction.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/DeleteHaNetworkGroupAction.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/DeleteHaNetworkGroupResult.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/HaNetworkGroupInventory.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/MatchEvidence.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/MatchedStep.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/ModelServiceRefInventory.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/MountModelToVmInstanceAction.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/MountModelToVmInstanceResult.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/Neighbor.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/NicTO.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/PodInventory.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/QueryHaNetworkGroupAction.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/QueryHaNetworkGroupResult.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/QueryVmModelMountAction.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/QueryVmModelMountResult.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/RegisterLicenseClientAction.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/RegisterLicenseClientResult.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/UnmountModelFromVmInstanceAction.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/UnmountModelFromVmInstanceResult.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/UpdateHaNetworkGroupAction.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/UpdateHaNetworkGroupResult.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/UpdateModelAction.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/VmModelMountInventory.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/VmModelMountStatus.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/YuccaBareMetal2DpuChassisConfig.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/network/zns/CreateL2GeneveNetworkAction.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/network/zns/L2GeneveNetworkInventory.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/network/zns/QueryZnsTenantAction.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/network/zns/QueryZnsTenantResult.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/network/zns/QueryZnsTenantRouterAction.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/network/zns/QueryZnsTenantRouterResult.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/network/zns/ZnsControllerInventory.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/network/zns/ZnsTenantInventory.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/network/zns/ZnsTenantRouterInventory.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/network/zns/ZnsTransportZoneInventory.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/zwatch/alarm/AddLabelToAlarmAction.java is excluded by !sdk/**
• sdk/src/main/java/org/zstack/sdk/zwatch/alarm/UpdateAlarmLabelAction.java is excluded by !sdk/**
• test/src/test/resources/springConfigXml/Kvm.xml is excluded by !**/*.xml

📒 Files selected for processing (122)

• VERSION
• abstraction/src/main/java/org/zstack/abstraction/sso/OAuth2PluginConstants.java
• compute/src/main/java/org/zstack/compute/host/HostTrackImpl.java
• compute/src/main/java/org/zstack/compute/vm/VmAllocateNicFlow.java
• compute/src/main/java/org/zstack/compute/vm/VmAllocateSdnNicFlow.java
• compute/src/main/java/org/zstack/compute/vm/VmCascadeExtension.java
• compute/src/main/java/org/zstack/compute/vm/VmDetachNicFlow.java
• compute/src/main/java/org/zstack/compute/vm/VmExpungeSdnNicFlow.java
• compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java
• compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java
• compute/src/main/java/org/zstack/compute/vm/VmInstanceExtensionPointEmitter.java
• compute/src/main/java/org/zstack/compute/vm/VmNicLifecycleGlobalConfig.java
• compute/src/main/java/org/zstack/compute/vm/VmNicLifecycleManager.java
• compute/src/main/java/org/zstack/compute/vm/VmNicManager.java
• compute/src/main/java/org/zstack/compute/vm/VmNicManagerImpl.java
• compute/src/main/java/org/zstack/compute/vm/VmReturnReleaseNicFlow.java
• compute/src/main/java/org/zstack/compute/vm/VmSystemTags.java
• conf/db/upgrade/V5.5.16__schema.sql
• conf/db/upgrade/V5.5.22__schema.sql
• conf/install/zstack-server
• core/src/main/java/org/zstack/core/rest/webhook/WebhookCallbackClient.java
• core/src/main/java/org/zstack/core/rest/webhook/WebhookProtocol.java
• docs/design/zns-pivot/ZCF-2133-p3-review-gpt55.md
• docs/design/zns-pivot/ZCF-2133-zns-pivot-detailed-design.md
• docs/modules/network/nav.adoc
• docs/modules/network/pages/networkResource/ZStackL2NetworkType.adoc
• docs/modules/network/pages/networkResource/ZnsIntegration.adoc
• docs/modules/network/pages/networkResource/networkResource.adoc
• header/src/main/java/org/zstack/header/allocator/AllocateHostMsg.java
• header/src/main/java/org/zstack/header/allocator/HostAllocationPurpose.java
• header/src/main/java/org/zstack/header/allocator/HostAllocatorSpec.java
• header/src/main/java/org/zstack/header/network/l2/L2NetworkConstant.java
• header/src/main/java/org/zstack/header/network/l3/AfterSetL3NetworkMtuExtensionPoint.java
• header/src/main/java/org/zstack/header/network/l3/AfterUpdateIpRangeExtensionPoint.java
• header/src/main/java/org/zstack/header/network/l3/L3NetworkInventory.java
• header/src/main/java/org/zstack/header/network/l3/L3NetworkType.java
• header/src/main/java/org/zstack/header/network/l3/L3NetworkVO.java
• header/src/main/java/org/zstack/header/network/sdncontroller/SdnControllerConstant.java
• header/src/main/java/org/zstack/header/network/sdncontroller/SdnControllerInventory.java
• header/src/main/java/org/zstack/header/network/sdncontroller/SdnControllerStatusEvent.java
• header/src/main/java/org/zstack/header/storage/snapshot/VolumeSnapshotDBSyncExtensionPoint.java
• header/src/main/java/org/zstack/header/vm/AfterAllocateSdnNicExtensionPoint.java
• header/src/main/java/org/zstack/header/vm/AfterAllocateVmNicIpExtensionPoint.java
• header/src/main/java/org/zstack/header/vm/AfterReleaseVmNicExtensionPoint.java
• header/src/main/java/org/zstack/header/vm/AfterUpdateVmNicMacExtensionPoint.java
• header/src/main/java/org/zstack/header/vm/BeforeAllocateVmNicExtensionPoint.java
• header/src/main/java/org/zstack/header/vm/GetVmStartingCandidateClustersHostsMsg.java
• header/src/main/java/org/zstack/header/vm/VmInstanceConstant.java
• header/src/main/java/org/zstack/header/vm/VmInstanceNicFactory.java
• header/src/main/java/org/zstack/header/vm/VmNicInventory.java
• header/src/main/java/org/zstack/header/vm/VmNicLifecycleContext.java
• header/src/main/java/org/zstack/header/vm/VmNicLifecycleExtensionPoint.java
• header/src/main/java/org/zstack/header/vm/VmOvsNicConstant.java
• identity/src/main/java/org/zstack/identity/login/LoginManagerImpl.java
• longjob/src/main/java/org/zstack/longjob/LongJobManagerImpl.java
• longjob/src/main/java/org/zstack/longjob/LongJobUtils.java
• network/src/main/java/org/zstack/network/l2/L2NetworkApiInterceptor.java
• network/src/main/java/org/zstack/network/l2/L2NetworkExtensionPointEmitter.java
• network/src/main/java/org/zstack/network/l2/L2NetworkHostHelper.java
• network/src/main/java/org/zstack/network/l2/L2NoVlanNetwork.java
• network/src/main/java/org/zstack/network/l3/L3BasicNetwork.java
• network/src/main/java/org/zstack/network/l3/L3NetworkManagerImpl.java
• plugin/applianceVm/src/main/java/org/zstack/appliancevm/ApplianceVmConstant.java
• plugin/applianceVm/src/main/java/org/zstack/appliancevm/ApplianceVmFacadeImpl.java
• plugin/externalStorage/src/main/java/org/zstack/externalStorage/primary/kvm/ExternalPrimaryStorageKvmFactory.java
• plugin/flatNetworkProvider/src/main/java/org/zstack/network/service/flat/FlatUserdataBackend.java
• plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java
• plugin/kvm/src/main/java/org/zstack/kvm/KVMApiInterceptor.java
• plugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.java
• plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
• plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java
• plugin/kvm/src/main/java/org/zstack/kvm/KVMHostUtils.java
• plugin/kvm/src/main/java/org/zstack/kvm/KVMRealizeL2NoVlanNetworkBackend.java
• plugin/kvm/src/main/java/org/zstack/kvm/KVMRealizeL2VlanNetworkBackend.java
• plugin/kvm/src/main/java/org/zstack/kvm/VmNicLifecycleKvmBridge.java
• plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/APIChangeLoadBalancerListenerMsg.java
• plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/APIChangeLoadBalancerListenerMsgDoc_zh_cn.groovy
• plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/APICreateLoadBalancerListenerMsg.java
• plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/APICreateLoadBalancerListenerMsgDoc_zh_cn.groovy
• plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerApiInterceptor.java
• plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerBase.java
• plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerConstants.java
• plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerManagerImpl.java
• plugin/sdnController/src/main/java/org/zstack/sdnController/AbstractSdnControllerFactory.java
• plugin/sdnController/src/main/java/org/zstack/sdnController/SdnControllerBase.java
• plugin/sdnController/src/main/java/org/zstack/sdnController/SdnControllerFactory.java
• plugin/sdnController/src/main/java/org/zstack/sdnController/SdnControllerL2.java
• plugin/sdnController/src/main/java/org/zstack/sdnController/SdnControllerManagerImpl.java
• plugin/sdnController/src/main/java/org/zstack/sdnController/SdnControllerPingTracker.java
• plugin/sdnController/src/main/java/org/zstack/sdnController/h3cVcfc/H3cVcfcSdnControllerFactory.java
• plugin/sugonSdnController/src/main/java/org/zstack/sugonSdnController/controller/SugonSdnControllerFactory.java
• plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/ReconnectVirtualRouterVmMsg.java
• plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/VirtualRouter.java
• plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/portforwarding/PortForwardingConfigProxy.java
• plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/vyos/VyosDeployAgentFlow.java
• plugin/vxlan/src/main/java/org/zstack/network/l2/vxlan/vxlanNetworkPool/VxlanNetworkCheckerImpl.java
• plugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniStorageController.java
• rest/src/main/resources/scripts/GoApiTemplate.groovy
• rest/src/main/resources/scripts/GoInventory.groovy
• rest/src/main/resources/scripts/RestDocumentationGenerator.groovy
• rest/src/main/resources/scripts/templates/base_param_types.go.template
• search/src/main/java/org/zstack/zql/ast/ZQLMetadata.java
• simulator/simulatorImpl/src/main/java/org/zstack/simulator/kvm/KVMSimulatorConfig.java
• simulator/simulatorImpl/src/main/java/org/zstack/simulator/kvm/KVMSimulatorController.java
• storage/src/main/java/org/zstack/storage/primary/PrimaryStorageApiInterceptor.java
• storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotManagerImpl.java
• test/src/test/bash/test_zstack_server_mariadb_recovery.sh
• test/src/test/groovy/org/zstack/test/integration/image/DeleteIsoCase.groovy
• test/src/test/groovy/org/zstack/test/integration/networkservice/provider/flat/userdata/CreateRebootVmKeepUserdataContentCase.groovy
• test/src/test/groovy/org/zstack/test/integration/networkservice/provider/virtualrouter/loadbalancer/VirtualRouterLoadBalancerUDPCase.groovy
• test/src/test/groovy/org/zstack/test/integration/storage/primary/addon/zbs/ZbsPrimaryStorageCase.groovy
• test/src/test/groovy/org/zstack/test/integration/storage/snapshot/CreateSnapshotRollbackAfterDataPlaneSuccessCase.groovy
• test/src/test/java/org/zstack/test/kvm/KVMHostUtilsTest.java
• testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy
• testlib/src/main/java/org/zstack/testlib/EnvSpec.groovy
• testlib/src/main/java/org/zstack/testlib/KVMSimulator.groovy
• testlib/src/main/java/org/zstack/testlib/SdnControllerSpec.groovy
• testlib/src/main/java/org/zstack/testlib/Test.groovy
• testlib/src/main/java/org/zstack/testlib/TestLibController.java
• testlib/src/main/java/org/zstack/testlib/VirtualRouterOfferingSpec.groovy
• utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
• utils/src/main/java/org/zstack/utils/ssh/SshShell.java

💤 Files with no reviewable changes (2)

• header/src/main/java/org/zstack/header/vm/VmOvsNicConstant.java
• plugin/flatNetworkProvider/src/main/java/org/zstack/network/service/flat/FlatUserdataBackend.java

✅ Files skipped from review due to trivial changes (17)

• VERSION
• header/src/main/java/org/zstack/header/network/sdncontroller/SdnControllerConstant.java
• header/src/main/java/org/zstack/header/network/sdncontroller/SdnControllerStatusEvent.java
• header/src/main/java/org/zstack/header/network/l3/AfterUpdateIpRangeExtensionPoint.java
• plugin/sdnController/src/main/java/org/zstack/sdnController/SdnControllerL2.java
• compute/src/main/java/org/zstack/compute/vm/VmNicLifecycleGlobalConfig.java
• header/src/main/java/org/zstack/header/vm/BeforeAllocateVmNicExtensionPoint.java
• docs/modules/network/nav.adoc
• test/src/test/groovy/org/zstack/test/integration/networkservice/provider/flat/userdata/CreateRebootVmKeepUserdataContentCase.groovy
• docs/modules/network/pages/networkResource/networkResource.adoc
• compute/src/main/java/org/zstack/compute/vm/VmExpungeSdnNicFlow.java
• search/src/main/java/org/zstack/zql/ast/ZQLMetadata.java
• plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java
• header/src/main/java/org/zstack/header/vm/VmInstanceNicFactory.java
• compute/src/main/java/org/zstack/compute/vm/VmCascadeExtension.java
• compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java
• docs/modules/network/pages/networkResource/ZStackL2NetworkType.adoc

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Completion.success() 缺少异常兜底，会导致流程悬挂并可能遗留 VmNicVO

Line 131-143 中 SQLBatch.execute()、addVmNicConfig(...) 或 mo.deleteCustomMacSystemTag(...) 任一抛异常时，当前分支不会调用 wcomp.done() 或 wcomp.allDone()；同时 NIC 已在 Line 126 持久化，可能留下脏记录。

建议修复

                 `@Override`
                 public void success() {
-                    new SQLBatch() {
-                        `@Override`
-                        protected void scripts() {
-                            persistStaticIpIfNeeded(nic, nicVO, nw, nicNetworkInfoMap, spec);
-                            nics.add(nic);
-                            VmNicVO updated = dbf.updateAndRefresh(nicVO);
-                            addVmNicConfig(updated, spec, nicSpec);
-                        }
-                    }.execute();
-                    if (customMac != null) {
-                        mo.deleteCustomMacSystemTag(spec.getVmInventory().getUuid(), nw.getUuid(), customMac);
-                    }
-                    wcomp.done();
+                    try {
+                        new SQLBatch() {
+                            `@Override`
+                            protected void scripts() {
+                                persistStaticIpIfNeeded(nic, nicVO, nw, nicNetworkInfoMap, spec);
+                                nics.add(nic);
+                                VmNicVO updated = dbf.updateAndRefresh(nicVO);
+                                addVmNicConfig(updated, spec, nicSpec);
+                            }
+                        }.execute();
+                        if (customMac != null) {
+                            mo.deleteCustomMacSystemTag(spec.getVmInventory().getUuid(), nw.getUuid(), customMac);
+                        }
+                        wcomp.done();
+                    } catch (Throwable t) {
+                        try {
+                            dbf.removeByPrimaryKey(nicVO.getUuid(), VmNicVO.class);
+                        } catch (Throwable cleanupEx) {
+                            logger.warn(String.format("failed to remove VmNicVO[uuid:%s] after success-path exception",
+                                    nicVO.getUuid()), cleanupEx);
+                        }
+                        errs.add(operr("failed to allocate vm nic[%s], %s", nic.getUuid(), t.getMessage()));
+                        wcomp.allDone();
+                    }
                 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@compute/src/main/java/org/zstack/compute/vm/VmAllocateNicFlow.java` around
lines 131 - 143, The code currently calls SQLBatch.execute(),
addVmNicConfig(...), and mo.deleteCustomMacSystemTag(...) without exception
handling, so any exception can skip wcomp.done() and leave a persisted VmNicVO
(nic/nicVO) as a dirty record; wrap the SQLBatch.execute() plus the following
addVmNicConfig and mo.deleteCustomMacSystemTag calls in a try/catch/finally: in
try keep the current logic, in catch log and fail the flow (call wcomp.fail(e)
or wcomp.allDone() per surrounding conventions) and attempt to cleanup the
persisted NIC (remove the VmNicVO persisted earlier via dbf.remove or
dbf.removeByUuid(nicVO.getUuid())) and revert any created tags, and in finally
ensure wcomp.done()/wcomp.allDone() is always invoked so the flow cannot hang;
reference SQLBatch, persistStaticIpIfNeeded, addVmNicConfig,
mo.deleteCustomMacSystemTag, wcomp.done()/wcomp.allDone(), VmNicVO, nic, nicVO
when making these changes.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

不要在已落库且不可回滚的分支里同步触发 VmIpChangedExtensionPoint。

Line 3780-3783 已经持久化了 VmNicVO 和 UsedIpVO，这里再调用 notifyVmIpChanged()，一旦某个扩展抛错，流程会直接失败，但 IP 变更本身不会回滚；运行态 VM 还会跳过后续的 VmApplyNetworkServiceOnChangeIPFlow()。结果就是 API 返回失败，但数据库里的 IP 已经改掉了。

建议把这个通知放到网络服务重新应用之后，至少保证主状态已经稳定。

Based on learnings, post-commit extension point hooks should generally be treated as fail-loud by convention.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java` around lines
3783 - 3784, The code calls notifyVmIpChanged(nicVO.getUuid(), oldIpMap,
newIpMap) immediately after persisting VmNicVO/UsedIpVO, which risks failing the
overall flow if a VmIpChangedExtensionPoint throws (DB changes are not rolled
back); move the post-commit notification out of this non-rollbackable branch and
invoke notifyVmIpChanged only after network services have been reapplied (i.e.
after VmApplyNetworkServiceOnChangeIPFlow completes successfully), so that
extension-point failures do not leave DB state changed while the flow reports
failure.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

修复 createDate 的零时间默认值，避免升级兼容性风险。

Line 357 使用 DEFAULT '0000-00-00 00:00:00' 可能在严格 SQL 模式下导致迁移失败。这里建议改为仓库既定的哨兵时间值（例如 2000-01-01 00:00:00），并保持与 lastOpDate 的双 TIMESTAMP 兼容策略一致。

💡 建议修改

-    `createDate`      TIMESTAMP     NOT NULL DEFAULT '0000-00-00 00:00:00',
+    `createDate`      TIMESTAMP     NOT NULL DEFAULT '2000-01-01 00:00:00',

As per coding guidelines, “Do not use DEFAULT 0000-00-00 00:00:00, use DEFAULT CURRENT_TIMESTAMP instead.” Based on learnings, “when lastOpDate already uses ON UPDATE CURRENT_TIMESTAMP, createDate should use a non-CURRENT_TIMESTAMP sentinel like 2000-01-01 00:00:00 to avoid MySQL 5.7 Error 1293.”

📝 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

	`createDate` TIMESTAMP NOT NULL DEFAULT '0000-00-00 00:00:00',
	`lastOpDate` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
	`createDate` TIMESTAMP NOT NULL DEFAULT '2000-01-01 00:00:00',
	`lastOpDate` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@conf/db/upgrade/V5.5.16__schema.sql` around lines 357 - 358, The createDate
column currently uses the illegal zero timestamp DEFAULT '0000-00-00 00:00:00'
which can fail under strict SQL modes; update the definition of createDate in
V5.5.16__schema.sql to use a safe sentinel (e.g. DEFAULT '2000-01-01 00:00:00')
instead of the zero value and keep lastOpDate as TIMESTAMP NOT NULL DEFAULT
CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP so both TIMESTAMP columns remain
compatible.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

ZNS 这几张新表和外键少了 zstack schema 限定。

这个升级脚本前半段一直显式使用 zstack....，但这里的新表和 FK 引用改成了裸表名。升级执行时如果当前默认 schema 不是 zstack，这些表会建到错误的库里，外键也可能直接失败。

请把这几处统一成 CREATE TABLE IF NOT EXISTS \zstack`.`...`，并把 REFERENCES目标也改成`zstack`.`...``。

Based on learnings: 在 conf/db/upgrade 下的升级脚本里，schema 固定为 zstack，应继续使用 zstack.\TableName`` 这种全限定写法。

Also applies to: 121-148

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@conf/db/upgrade/V5.5.22__schema.sql` around lines 87 - 116, The CREATE TABLE
and FOREIGN KEY statements for ZnsControllerVO, L2GeneveNetworkVO,
ZnsTransportZoneVO (and the referenced CONSTRAINT names
fkZnsControllerVOSdnControllerVO, fkL2GeneveNetworkVOL2NetworkEO,
fkZnsTransportZoneVOSdnControllerVO) must be fully schema-qualified with
`zstack` like the rest of the script: change each "CREATE TABLE IF NOT EXISTS
`TableName`" to "CREATE TABLE IF NOT EXISTS `zstack`.`TableName`" and update
each REFERENCES target to reference `zstack`.`ReferencedTable` (e.g., REFERENCES
`zstack`.`SdnControllerVO` and REFERENCES `zstack`.`L2NetworkEO`), and apply the
same change to the other occurrence range (lines 121-148) so tables and FK
targets are consistently prefixed with `zstack`.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

补上 /etc/init.d 的启动回退。

mariadb_service_exists() 已经把仅有 init script 的场景当成“服务存在”，但这里在没有 systemctl 时只调用 service ... start。如果目标机只有 /etc/init.d/$MARIADB_SERVICE 而没有 service 包装器，前面的 stale socket 会被删掉，随后启动直接失败。

可直接收敛成和探测逻辑一致的回退链

 start_mariadb_service() {
     if command -v systemctl >/dev/null 2>&1; then
         systemctl reset-failed "$MARIADB_SERVICE" >/dev/null 2>&1
         systemctl start "$MARIADB_SERVICE"
         return $?
     fi
 
-    service "$MARIADB_SERVICE" start
+    if command -v service >/dev/null 2>&1; then
+        service "$MARIADB_SERVICE" start
+        return $?
+    fi
+
+    if [ -x "/etc/init.d/$MARIADB_SERVICE" ]; then
+        "/etc/init.d/$MARIADB_SERVICE" start
+        return $?
+    fi
+
+    return 1
 }

📝 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

	start_mariadb_service() {
	if command -v systemctl >/dev/null 2>&1; then
	systemctl reset-failed "$MARIADB_SERVICE" >/dev/null 2>&1
	systemctl start "$MARIADB_SERVICE"
	return $?
	fi
	service "$MARIADB_SERVICE" start
	}
	start_mariadb_service() {
	if command -v systemctl >/dev/null 2>&1; then
	systemctl reset-failed "$MARIADB_SERVICE" >/dev/null 2>&1
	systemctl start "$MARIADB_SERVICE"
	return $?
	fi
	if command -v service >/dev/null 2>&1; then
	service "$MARIADB_SERVICE" start
	return $?
	fi
	if [ -x "/etc/init.d/$MARIADB_SERVICE" ]; then
	"/etc/init.d/$MARIADB_SERVICE" start
	return $?
	fi
	return 1
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@conf/install/zstack-server` around lines 95 - 103, 函数 start_mariadb_service()
在没有 systemctl 时只调用 service ... start，但探测函数 mariadb_service_exists() 把仅有
/etc/init.d/$MARIADB_SERVICE 的场景当作“服务存在”，因此需要增加与探测逻辑一致的回退链：当 command -v
systemctl 不存在且 command -v service 不存在时检查 /etc/init.d/"$MARIADB_SERVICE"
是否可执行并调用它（例如 /etc/init.d/"$MARIADB_SERVICE" start），确保行为与 systemctl
分支相同并正确返回该命令的退出码，同时保留对 systemctl reset-failed 的调用和原有返回码语义（即所有分支都返回相应命令的退出状态）。

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

这里应该用同步后的状态发 canonical event。

syncVolumeSnapshotDBAfterTakeSnapshot() 刚把 DB 状态规范成 Ready，但事件状态还是取自 snapshot.getStatus()。如果后端回复里还是旧值或空值，订阅方会收到和 sp 不一致的状态事件。

建议直接复用已同步的 inventory

 new FireSnapShotCanonicalEvent().
         fireSnapShotStatusChangedEvent(
-                VolumeSnapshotStatus.valueOf(snapshot.getStatus()),
+                VolumeSnapshotStatus.valueOf(sp.getStatus()),
                 sp);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotManagerImpl.java`
around lines 1037 - 1043, 同步数据库后应该用同步得到的 inventory 的状态来发事件，而不是继续读取可能是旧值的
snapshot.getStatus()。在调用
FireSnapShotCanonicalEvent.fireSnapShotStatusChangedEvent 时，替换
VolumeSnapshotStatus.valueOf(snapshot.getStatus()) 为基于
syncVolumeSnapshotDBAfterTakeSnapshot 返回的 VolumeSnapshotInventory sp 的状态（例如
VolumeSnapshotStatus.valueOf(sp.getStatus())），并确保对 sp.getStatus() 做非空/合法性校验后再转换。

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

不要吞掉 multipart 读取失败。

这里捕获异常后直接返回已累积的内容，模拟器会继续拿着空/残缺 body 往下跑，最后很容易把请求解析问题伪装成业务行为。这个分支更适合在记录日志后直接失败，让用例在根因处中断。

可参考的修改

         try {
             req.getParts().each { part ->
                 appendMultipartPart(sb, part.name, part.submittedFileName, part.contentType, part.inputStream.bytes)
             }
         } catch (Throwable t) {
-            logger.debug("failed to read multipart parts for ${req.requestURI}", t)
+            logger.warn("failed to read multipart parts for ${req.requestURI}", t)
+            throw t
         }
         return sb.toString()

📝 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

	try {
	req.getParts().each { part ->
	appendMultipartPart(sb, part.name, part.submittedFileName, part.contentType, part.inputStream.bytes)
	}
	} catch (Throwable t) {
	logger.debug("failed to read multipart parts for ${req.requestURI}", t)
	}
	try {
	req.getParts().each { part ->
	appendMultipartPart(sb, part.name, part.submittedFileName, part.contentType, part.inputStream.bytes)
	}
	} catch (Throwable t) {
	logger.warn("failed to read multipart parts for ${req.requestURI}", t)
	throw t
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@testlib/src/main/java/org/zstack/testlib/EnvSpec.groovy` around lines 1038 -
1044, The catch block that swallows multipart read errors must be changed so
failures abort the test: in the try around req.getParts().each (and the call to
appendMultipartPart) replace the current logger.debug-only handling with logging
the error and then rethrowing (or throw a RuntimeException) so the request
processing stops; keep the debug log but propagate the exception from the
req.getParts() read instead of returning partial content.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

In Spring's MultipartHttpServletRequest, does getFileMap() keep only one MultipartFile per field name, while getMultiFileMap()/getFiles(name) preserves repeated multipart file fields?

💡 Result:

Yes, that is correct. In Spring's MultipartHttpServletRequest, the methods behave differently regarding how they handle multiple files submitted with the same field name [1][2][3]: getFileMap returns a Map<String, MultipartFile> [4][3]. Because a standard Map cannot hold multiple values for a single key, this method typically returns only one MultipartFile per field name (often the first one encountered) [1][5]. It is effectively a single-value view of the multipart files [1]. getMultiFileMap returns a MultiValueMap<String, MultipartFile> [4][3]. This structure is designed to preserve all files associated with a given field name, mapping each key to a List of MultipartFile objects [6][3]. Similarly, the getFiles(String name) method returns a List for a specific field name, allowing you to access all files uploaded under that name [6]. If you need to handle multiple files for the same parameter name, you should use getMultiFileMap or getFiles(name) instead of getFileMap [1][2].

Citations:

• 1: Support Multiple Files for One Form Field in multipart/form-data spring-projects/spring-integration#2872
• 2: https://stackoverflow.com/questions/60291483/how-to-get-all-submitted-multipartfiles-regardless-of-their-name
• 3: https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/web/multipart/MultipartRequest.html
• 4: https://docs.spring.io/spring-framework/docs/6.2.0-M4/javadoc-api/org/springframework/web/multipart/MultipartRequest.html
• 5: https://www.tothenew.com/blog/uploading-multiple-files-with-same-name/
• 6: https://docs.spring.io/spring-framework/docs/5.3.11/javadoc-api/org/springframework/web/multipart/MultipartRequest.html

---

fileMap 会丢失同名字段的多个上传文件。

MultipartHttpServletRequest.fileMap 返回 Map<String, MultipartFile>，每个字段名只能对应一个文件。若请求中同一字段上传多个文件，该方法仅保留其中一个，导致重建的 multipart body 不完整。应改用 getMultiFileMap() 或 getFiles(name) 来遍历所有文件。

修改建议

-        req.fileMap.each { String name, file ->
-            appendMultipartPart(sb, name, file.originalFilename, file.contentType, file.bytes)
-        }
+        req.multiFileMap.each { String name, files ->
+            files.each { file ->
+                appendMultipartPart(sb, name, file.originalFilename, file.contentType, file.bytes)
+            }
+        }
         return sb.toString()

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@testlib/src/main/java/org/zstack/testlib/EnvSpec.groovy` around lines 1055 -
1057, The code uses req.fileMap which maps a field name to a single
MultipartFile and thus drops multiple files for the same field; change the loop
to iterate all uploaded files per field (use req.getMultiFileMap() or
req.getFiles(name)) and call appendMultipartPart(sb, name,
file.originalFilename, file.contentType, file.bytes) for each file to ensure all
files are included; update the usage around req.fileMap and the
appendMultipartPart invocation accordingly.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# 检查 AI 错误码 10151-10156 是否在代码库的其他位置被使用或定义

echo "Searching for AI error codes 10151-10156..."
rg -n 'ORG_ZSTACK_AI_1015[1-6]' --type java

Repository: MatheMatrix/zstack

Length of output: 106

---

🏁 Script executed:

# Check the file to see if there are comments explaining the reserved codes
cat -n utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java | sed -n '15070,15100p'

Repository: MatheMatrix/zstack

Length of output: 1832

---

🏁 Script executed:

# Search for comments mentioning "reserved" or "future" in the error code file
rg -n -i 'reserved|future|skip|gap' utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java

Repository: MatheMatrix/zstack

Length of output: 118

---

🏁 Script executed:

# Check if there are similar gaps in other error code ranges in the same file
rg -n 'ORG_ZSTACK_AI_101[0-4][0-9]' utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java | head -50

Repository: MatheMatrix/zstack

Length of output: 4162

---

为 AI 错误码 10151-10156 的保留添加文档注释

错误码序列从 ORG_ZSTACK_AI_10150（第 15086 行）直接跳至 ORG_ZSTACK_AI_10157（第 15088 行），缺少 10151-10156 共 6 个错误码。该文件已在其他位置使用 // XXX reserved 的模式来文档化保留的错误码（如第 12055 行的 // 10008 reserved），建议在第 15086 行后添加类似注释，明确说明这些错误码已保留供未来使用。

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java`
around lines 15074 - 15097, The sequence of AI error constants in
CloudOperationsErrorCode skips ORG_ZSTACK_AI_10151–ORG_ZSTACK_AI_10156; add a
single-line reserved comment matching the project's convention (e.g. "//
10151-10156 reserved") immediately after the ORG_ZSTACK_AI_10150 declaration and
before ORG_ZSTACK_AI_10157 to document those reserved codes and keep the
numbering sequence clear and consistent.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

OpenSSH 文档中 ConnectTimeout、ServerAliveInterval、ServerAliveCountMax 是否允许负数？请给出参数取值范围与官方说明链接。

💡 Result:

在 OpenSSH 的 ssh_config 配置中，ConnectTimeout、ServerAliveInterval 和 ServerAliveCountMax 参数均不支持负数 [1][2][3]。 以下是各参数的详细说明与取值范围： 1. ConnectTimeout - 说明：指定连接 SSH 服务器时的超时时间（以秒为单位），用于替代系统默认的 TCP 超时时间 [2][4]。 - 取值范围：必须为非负整数。通常设置为正整数以定义具体的超时秒数。 - 官方文档：ssh_config(5) - OpenBSD manual pages 2. ServerAliveInterval - 说明：指定一个超时时间间隔（以秒为单位），如果在此时间内没有从服务器接收到任何数据，ssh(1) 将通过加密通道发送消息以请求服务器响应 [1][4]。 - 取值范围：非负整数。默认值为 0，表示不向服务器发送此类消息 [1][4]。 - 官方文档：ssh_config(5) - OpenBSD manual pages 3. ServerAliveCountMax - 说明：指定在 ssh(1) 未收到服务器任何响应的情况下，允许发送的服务器存活消息（server alive messages）的最大数量 [1][2]。如果达到此阈值，ssh 将断开与服务器的连接 [1][4]。 - 取值范围：非负整数。默认值为 3 [1][4]。 - 官方文档：ssh_config(5) - OpenBSD manual pages 总结：这些参数在 OpenSSH 中均被视为时间或计数类配置，逻辑上要求为非负值。配置为负数通常会被视为无效配置或导致解析错误。建议始终使用非负整数进行配置 [1][2][3]。

Citations:

• 1: https://github.com/openssh/openssh-portable/blob/master/ssh_config.5
• 2: https://man.openbsd.org/i386/ssh_config
• 3: https://man.openbsd.org/ssh_config
• 4: https://man7.org/linux/man-pages/man5/ssh_config.5.html

---

🏁 Script executed:

# 首先，定位并检查 SshShell.java 文件
find . -name "SshShell.java" -type f

Repository: MatheMatrix/zstack

Length of output: 119

---

🏁 Script executed:

# 查看文件的相关部分，包括 setters 和 sshTimeoutOptions 方法
cat -n utils/src/main/java/org/zstack/utils/ssh/SshShell.java | head -300

Repository: MatheMatrix/zstack

Length of output: 10515

---

🏁 Script executed:

# 搜索 sshTimeoutOptions 方法，了解这些参数如何被使用
rg "sshTimeoutOptions" -A 20 -B 5

Repository: MatheMatrix/zstack

Length of output: 9621

---

🏁 Script executed:

# 检查 DebugUtils.Assert 的可用性和使用模式
rg "DebugUtils\.Assert" -B 2 -A 2 | head -50

Repository: MatheMatrix/zstack

Length of output: 5736

---

为超时参数 setter 增加非负值校验，避免拼接出非法 SSH 选项。

setConnectTimeoutSeconds、setServerAliveIntervalSeconds、setServerAliveCountMax 目前接受任意整数。根据 OpenSSH 官方文档，这三个参数均要求非负整数。负值会直接进入 sshTimeoutOptions() 方法拼接到 SSH 命令中（第 57、62、110、125 行），导致 SSH 执行失败。建议在 setter 处增加 >= 0 校验，与类中现有的 checkParams() 方法保持一致的验证模式。

🛠️ 建议修改

 public void setConnectTimeoutSeconds(int connectTimeoutSeconds) {
+    DebugUtils.Assert(connectTimeoutSeconds >= 0, "connectTimeoutSeconds must be >= 0");
     this.connectTimeoutSeconds = connectTimeoutSeconds;
 }

 public void setServerAliveIntervalSeconds(int serverAliveIntervalSeconds) {
+    DebugUtils.Assert(serverAliveIntervalSeconds >= 0, "serverAliveIntervalSeconds must be >= 0");
     this.serverAliveIntervalSeconds = serverAliveIntervalSeconds;
 }

 public void setServerAliveCountMax(int serverAliveCountMax) {
+    DebugUtils.Assert(serverAliveCountMax >= 0, "serverAliveCountMax must be >= 0");
     this.serverAliveCountMax = serverAliveCountMax;
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@utils/src/main/java/org/zstack/utils/ssh/SshShell.java` around lines 225 -
243, 这些超时参数的
setter（setConnectTimeoutSeconds、setServerAliveIntervalSeconds、setServerAliveCountMax）当前接受任意整数，会把负值拼接进
sshTimeoutOptions() 导致生成非法 SSH 选项；在每个 setter 中加入非负校验（value >= 0），在不满足时抛出
IllegalArgumentException（或与类中 checkParams() 一致的校验异常/行为），确保只有 >=0 的值被赋给对应字段，从而避免
sshTimeoutOptions() 拼接出非法参数。