<feature>[kvm]: support model mount by zstack-robot-1 · Pull Request #3545 · MatheMatrix/zstack

zstack-robot-1 · 2026-03-19T14:14:32Z

Resolves: ZSTAC-83157

Change-Id: Ifc0f1fab5634ef4387c6cbe8daf1a20af00664fa

sync from gitlab !9404

coderabbitai · 2026-03-19T15:41:35Z

Walkthrough

引入虚拟机模型挂载功能相关的基础设施：新增数据库表 VmModelMountVO、扩展 KVM 代理命令与路由常量，以及在测试库中新增/调整用于挂载、查询和卸载模型的 API 辅助方法。

Changes

Cohort / File(s)	Summary
数据库模式定义 `conf/db/upgrade/V5.5.12__schema.sql`	新增可幂等创建的表 `zstack.VmModelMountVO`（含 `uuid`, `vmInstanceUuid`, `modelUuid`, `modelName`, `juicefsSubdir`, `mountPath`, `status`, `createDate`, `lastOpDate`），为 `vmInstanceUuid` 建索引并添加外键 `fk_VmModelMount_vm` 指向 `VmInstanceEO.uuid`（ON DELETE CASCADE）；修正脚本末尾换行。
KVM 代理命令接口 `plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java`	新增四个代理消息类型：`KvmAttachModelCmd` / `KvmAttachModelResponse` 和 `KvmDetachModelCmd` / `KvmDetachModelResponse`（均标注灰度版本 `5.5.12`），用于模型附加与分离的请求/响应承载字段。
KVM 路由常量 `plugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.java`	新增两个 REST 路由常量：`KVM_ATTACH_AI_MODEL_PATH` = `"/aimodel/attach"`，`KVM_DETACH_AI_MODEL_PATH` = `"/aimodel/detach"`。
测试 API 辅助方法 `testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy`	新增三个公有测试助手：`mountModelToVmInstance`、`queryVmModelMount`（对 conditions 做 `toString()` 转换）和 `unmountModelFromVmInstance`，并调整了三个 BareMetal2 相关方法的 Action 映射与委派（部分空白改动存在）。

Sequence Diagram(s)

sequenceDiagram
    participant Test as Test / API 客户端
    participant Mgr as 管理节点 (API Server)
    participant KVM as KVM Agent
    participant DB as 数据库

    Test->>Mgr: 调用 mountModelToVmInstance(action)
    Mgr->>DB: 写入/更新 VmModelMountVO（uuid, vmInstanceUuid, mountPath, ...）
    Mgr->>KVM: POST /aimodel/attach (KvmAttachModelCmd)
    KVM-->>Mgr: KvmAttachModelResponse (status)
    Mgr-->>Test: 返回 API 结果

Estimated code review effort

🎯 3 (中等) | ⏱️ ~20 分钟

兔子的庆祝诗

🐰✨ 小兔子来庆贺，挂载新表齐登场，
代理路由悄然加，测试助手也到岗，
VM 与模型轻轻握，智云同行步伐忙。

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	标题完全符合要求的格式，包含了type和scope，清晰描述了主要变更（添加KVM模型挂载支持），长度为35字符，远低于72字符限制。
Description check	✅ Passed	描述与变更集相关，包含了JIRA跟踪号ZSTAC-83157和GitLab同步信息，与代码变更相符。

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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/zhong.xian/fix/ZSTAC-83157@@2

📝 Coding Plan

Generate coding plan for human review comments

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Resolves: ZSTAC-83157 Change-Id: Ifc0f1fab5634ef4387c6cbe8daf1a20af00664fa

coderabbitai

🧹 Nitpick comments (1)

plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java (1)
5208-5249: 命名建议：考虑移除 "Kvm" 前缀以保持一致性

文件中其他类似的命令类如 AttachDataVolumeCmd、DetachNicCommand 均未使用 "Kvm" 前缀，因为它们已经嵌套在 KVMAgentCommands 类中。建议将 KvmAttachModelCmd 重命名为 AttachModelCmd，KvmDetachModelCmd 重命名为 DetachModelCmd，以保持命名风格一致。

另外，如果 zdfsUrl 可能包含嵌入式凭据（如 protocol://user:password@host/path），建议添加 @NoLogging 注解以避免敏感信息泄露到日志中。
♻️ 建议的命名修改
-    public static class KvmAttachModelCmd extends AgentCommand {
+    public static class AttachModelCmd extends AgentCommand {
         `@GrayVersion`(value = "5.5.12")
         private String vmInstanceUuid;
         `@GrayVersion`(value = "5.5.12")
+        `@NoLogging`  // 如果可能包含凭据
         private String zdfsUrl;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java` around lines
5208 - 5249, Rename the nested command class KvmAttachModelCmd to AttachModelCmd
(and likewise KvmDetachModelCmd to DetachModelCmd) to match the naming
convention used by other nested commands in KVMAgentCommands and update all
references/usages accordingly; also, if zdfsUrl can contain embedded
credentials, annotate the zdfsUrl field with `@NoLogging` to prevent sensitive
data from being logged (ensure the import/annotation is available and add it to
the field in the AttachModelCmd class).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java`:
- Around line 5208-5249: Rename the nested command class KvmAttachModelCmd to
AttachModelCmd (and likewise KvmDetachModelCmd to DetachModelCmd) to match the
naming convention used by other nested commands in KVMAgentCommands and update
all references/usages accordingly; also, if zdfsUrl can contain embedded
credentials, annotate the zdfsUrl field with `@NoLogging` to prevent sensitive
data from being logged (ensure the import/annotation is available and add it to
the field in the AttachModelCmd class).

ℹ️ 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: 559d8172-5d21-4814-ad7c-9e1d28a5fde1

📥 Commits

Reviewing files that changed from the base of the PR and between 12663bb and a0227cf.

⛔ Files ignored due to path filters (8)

sdk/src/main/java/SourceClassMap.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/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/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/VmModelMountInventory.java is excluded by !sdk/**

📒 Files selected for processing (4)

conf/db/upgrade/V5.5.12__schema.sql
plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java
plugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.java
testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy

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

plugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.java
conf/db/upgrade/V5.5.12__schema.sql

zstack-robot-1 · 2026-03-20T02:29:50Z

Comment from ye.zou:

Code Review: ZSTAC-83157 — 云主机直接挂载模型仓库模型

关联 MR: zstack !9404 + premium !13252 | 36 files | Verdict: CONDITIONAL

🔴 Critical

1. 挂载路径未存回 normalized 值 — 重复检测可绕过

AIModelManagerApiInterceptor.validate() 里 Paths.get(mountPath).normalize() 之后只用 normalized 做校验，没有写回 msg。AIModelManagerImpl Flow 1 拿到的还是原始路径。

后果：/mnt/models/./a 和 /mnt/models/a 可以同时挂到同一 VM，DB 存的是未 normalize 的值，重复检测查不到。

修复：interceptor 里加 msg.setMountPath(normalized);

2. 无 DB 唯一约束 — 并发挂载竞态

VmModelMountVO 表没有 UNIQUE(vmInstanceUuid, modelUuid)。两个并发请求都能通过 interceptor 的 check-then-act，都插入记录。

修复：SQL schema 加 UNIQUE KEY idx_vm_model (vmInstanceUuid, modelUuid)

🟡 Major

3. 双重 dispatch — handleLocalMessage 里塞了 API 消息（死代码）

AIModelManagerImpl 在 handleLocalMessage() (line ~341) 和 handleApiMessage() (line ~2428) 两处都 dispatch 了 APIMountModelToVmInstanceMsg。API 消息只走 handleApiMessage，前者是死代码。删掉。

4. TOCTOU — VM 状态和 hostUuid 在 interceptor 和 agent 调用之间可变

interceptor 检查 vm.state == Running，Flow 2 取 vm.getHostUuid() 时 VM 可能已迁移。agent 调用打到旧 host。建议 Flow 2 里重新查 VM 状态。

5. CascadeExtension 是空壳

VmModelMountCascadeExtension.handleDeletion() 查 DB、打日志，然后依赖 FK CASCADE 删除。查 DB 就为了 log？要么删掉这个 extension，要么做点有用的事（如通知 agent 清理宿主机挂载点）。

6. VM 重启后 DB 状态变脏

KVM agent 做的挂载在 VM 重启后消失，DB 仍然 status=Mounted。需要 reconnect 机制或至少在 VM start 时标记为需要重挂。

7. Unmount 不处理 Stopped VM

vm.getHostUuid() 在 VM Stopped 时返回 null，KVMHostAsyncHttpCallMsg 会炸。处理了 vm==null 但没处理 hostUuid==null。关机 VM 的挂载已经不在了，应直接删记录。

🔵 Minor

8. SQL schema 缺 modelUuid FK — JPA 有 @ForeignKey(ModelVO.class, CASCADE) 但 SQL 没有，model 被 SQL 直接删除时 mount 记录成孤儿。

9. KvmAttachModelCmd.zdfsUrl — 泄露内部实现命名，建议改为 metaUrl 或 storageUrl。

10. SQL 文件缺末尾换行 — \ No newline at end of file

✅ 做得好

Interceptor 5 层校验（推理服务 VM 保护 + 状态 + 权限 + 重复 + 路径安全）设计到位
mountPath 自动生成的 sanitize 逻辑考虑了特殊字符
错误处理：mount 失败清理记录，unmount 失败保留 Failed 允许重试
测试覆盖 mount/query/unmount/duplicate 四条核心路径

结论：整体设计方向正确，FlowChain + interceptor + cascade 三层符合 ZStack 惯例。路径 normalize 和唯一约束是必须修的，其他 major 建议本轮修复或加 TODO 跟踪。

— AI Code Review (Linus Mode)

MatheMatrix · 2026-03-20T03:38:12Z

Comment from ye.zou:

补充：字段命名和协议设计 — 不要把实现细节烧进接口

当前 DB schema 和 agent 协议里有多个字段硬编码了 JuiceFS 实现：

位置	字段	问题
`VmModelMountVO` (SQL + JPA)	`juicefsSubdir`	列名绑死 JuiceFS
`KvmAttachModelCmd`	`zdfsUrl`	字段名绑死 ZDFS
`KvmAttachModelCmd`	`juicefsSubdir`	字段名绑死 JuiceFS
`KVMConstant`	`/aimodel/attach`	OK，这个是通用的 ✅

为什么现在必须改

DB 列名改不动：一旦 5.5.12 发布，juicefsSubdir 这个列名就永久烧进升级脚本链了。后续如果切 virtiofs / NFS / 9p，你得加新列 + 迁移数据 + 兼容旧列，成本是现在改名的 10 倍
Agent 协议改不动：kvmagent 和 MN 之间的 JSON 字段名一旦发布就成了兼容性负担，灰度升级时新老 agent 要同时支持

建议改为

当前	建议	理由
`juicefsSubdir` (DB + Agent)	`sourcePath`	通用：JuiceFS 是子目录，virtiofs 是宿主机路径，NFS 是 export path
`zdfsUrl` (Agent)	`storageUrl` 或 `metaUrl`	通用：不暴露存储实现

API 层（APIMountModelToVmInstanceMsg）的 mountPath 命名是好的——面向用户的字段不包含实现细节。内部字段也应该保持同样的抽象层级。

原则：面向能力命名，不面向实现命名。 你的能力是"把模型源路径挂载到 VM 目标路径"，不是"把 JuiceFS 子目录通过 ZDFS URL 挂到 VM"。

— AI Code Review (补充)

MatheMatrix force-pushed the sync/zhong.xian/fix/ZSTAC-83157@@2 branch from e0824ea to 12663bb Compare March 19, 2026 15:41

<feature>[kvm]: support model mount

a0227cf

Resolves: ZSTAC-83157 Change-Id: Ifc0f1fab5634ef4387c6cbe8daf1a20af00664fa

MatheMatrix force-pushed the sync/zhong.xian/fix/ZSTAC-83157@@2 branch from 12663bb to a0227cf Compare March 19, 2026 20:30

coderabbitai bot reviewed Mar 19, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

<feature>[kvm]: support model mount#3545

<feature>[kvm]: support model mount#3545
zstack-robot-1 wants to merge 1 commit into5.5.12from
sync/zhong.xian/fix/ZSTAC-83157@@2

zstack-robot-1 commented Mar 19, 2026

Uh oh!

coderabbitai bot commented Mar 19, 2026 •

edited

Loading

❌ Failed checks (1 warning)

Uh oh!

coderabbitai bot left a comment

Uh oh!

zstack-robot-1 commented Mar 20, 2026

Uh oh!

MatheMatrix commented Mar 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

zstack-robot-1 commented Mar 19, 2026

Uh oh!

coderabbitai bot commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

兔子的庆祝诗

❌ Failed checks (1 warning)

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

zstack-robot-1 commented Mar 20, 2026

Code Review: ZSTAC-83157 — 云主机直接挂载模型仓库模型

🔴 Critical

🟡 Major

🔵 Minor

✅ 做得好

Uh oh!

MatheMatrix commented Mar 20, 2026

补充：字段命名和协议设计 — 不要把实现细节烧进接口

为什么现在必须改

建议改为

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai bot commented Mar 19, 2026 •

edited

Loading