Skip to content

Conversation

@zstack-robot-2
Copy link
Collaborator

  • Add vmPciDeviceAddress field to PciDeviceVO
  • Set vmPciDeviceAddress when loading PCI device to VM
  • Clear vmPciDeviceAddress when unloading PCI device
  • Update vmPciDeviceAddress during host reconnection

Resolves: ZSTAC-67275

Change-Id: I67626e67787062616679786b6576636c73747477

sync from gitlab !9007

@coderabbitai
Copy link

coderabbitai bot commented Jan 12, 2026

演练

新增两列到数据库(PciDeviceVO.vmPciDeviceAddress、MdevDeviceVO.mdevDeviceAddress),并在 KVM 代理的 KVMAgentCommands.VmDevicesInfoResponse 中新增两组映射字段 pciDeviceInfosmdevDeviceInfos(均为 Map<String,String>,标注 GrayVersion 5.5.6)。

变更

内聚体 / 文件(s) 变更摘要
数据库架构更新
conf/db/upgrade/V5.5.6__schema.sql
PciDeviceVO 表新增列 vmPciDeviceAddressvarchar(32), 位置 1, 默认 NULL);向 MdevDeviceVO 表新增列 mdevDeviceAddressvarchar(32), 位置 1, 默认 NULL)
KVM 代理数据模型扩展
plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java
KVMAgentCommands.VmDevicesInfoResponse 中新增字段 private Map<String,String> pciDeviceInfosprivate Map<String,String> mdevDeviceInfos(均带 @GrayVersion("5.5.6")),并添加相应的 getter/setter 方法

Sequence Diagram(s)

(无序列图 — 变更为模式扩展与数据库列添加,未引入新的多组件控制流)

代码审查工作量评估

🎯 2 (Simple) | ⏱️ ~10 minutes

诗歌

🐰✨ 新列轻轻落土间,
两组映射入代理间,
pci 与 mdev 手相牵,
小兔跳跃报更新,
胡萝卜庆贺笑颜开 🥕

🚥 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 标题遵循了要求的格式([scope]: ),完整相关于变更集,涵盖了主要变更(添加vmPciDeviceAddress字段用于PCI设备跟踪),长度为60字符,符合72字符限制。
Description check ✅ Passed 描述与变更集相关,列出了vmPciDeviceAddress字段的添加和用法场景,虽然变更中也包含mdevDeviceAddress,但描述主要聚焦于PCI设备跟踪功能。

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

✨ Finishing touches
  • 📝 Generate docstrings


📜 Recent review details

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

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 272578a and 5b39a87.

⛔ Files ignored due to path filters (1)
  • sdk/src/main/java/org/zstack/sdk/PciDeviceInventory.java is excluded by !sdk/**
📒 Files selected for processing (2)
  • conf/db/upgrade/V5.5.6__schema.sql
  • plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java
🚧 Files skipped from review as they are similar to previous changes (2)
  • plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java
  • conf/db/upgrade/V5.5.6__schema.sql
🧰 Additional context used
🔍 Remote MCP AtlassianMCP

Relevant findings for PR #3188 review

  • Jira ZSTAC-67275: summary, status, priority, assignee, reporter, timestamps

    • Key: ZSTAC-67275
    • Summary: backend must provide a new identifier so frontend can distinguish multiple passthrough GPUs on the same VM
    • Status: Open
    • Priority: P1
    • Assignee: 沈今 (jin.shen@zstack.io)
    • Reporter: 段征中 (zhengzhong.duan@zstack.io)
    • Created: 2024-07-03T07:38:54Z; Updated: 2026-01-12T09:43:40Z
    • Source: AtlassianMCP_jira_get_issue
  • Confluence search for related design pages returned no results for queries "vmPciDeviceAddress OR ZSTAC-67275 OR vm PCI"

    • Source: AtlassianMCP_confluence_search (empty result)

Sources used:,

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


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

@MatheMatrix MatheMatrix force-pushed the sync/jin.shen/fix-67275@@3 branch from 3c2f228 to 502be1c Compare January 13, 2026 09:02
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java (1)

2951-2958: Getter/Setter 实现正确。

方法实现遵循标准 Java Bean 模式。注意到该 getter 直接返回 map 引用,调用方需自行处理可能的 null 值。

考虑到代码库中对此模式的处理方式不一致(如 getAddons() 方法会在 null 时初始化,而其他类似方法则不会),当前实现可以接受。如需更强的防御性编程,可选择性地添加 null 检查:

♻️ 可选的空值防护重构
 public Map<String, String> getPciDeviceInfos() {
+    if (pciDeviceInfos == null) {
+        pciDeviceInfos = new HashMap<>();
+    }
     return pciDeviceInfos;
 }
📜 Review details

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

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3c2f228 and 502be1c.

⛔ Files ignored due to path filters (1)
  • sdk/src/main/java/org/zstack/sdk/PciDeviceInventory.java is excluded by !sdk/**
📒 Files selected for processing (2)
  • conf/db/upgrade/V5.5.6__schema.sql
  • plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java
🚧 Files skipped from review as they are similar to previous changes (1)
  • conf/db/upgrade/V5.5.6__schema.sql
🧰 Additional context used
📓 Path-based instructions (2)
**/*.*

⚙️ CodeRabbit configuration file

**/*.*: - 代码里不应当有有中文,包括报错、注释等都应当使用正确的、无拼写错误的英文来写

Files:

  • plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java
**/*.java

⚙️ CodeRabbit configuration file

**/*.java: ## 1. API 设计要求

  • API 命名:
    • API 名称必须唯一,不能重复。
    • API 消息类需要继承 APIMessage;其返回类必须继承 APIReplyAPIEvent,并在注释中用 @RestResponse 进行标注。
    • API 消息上必须添加注解 @RestRequest,并满足如下规范:
      • path:
        • 针对资源使用复数形式。
        • 当 path 中引用消息类变量时,使用 {variableName} 格式。
      • HTTP 方法对应:
        • 查询操作 → HttpMethod.GET
        • 更新操作 → HttpMethod.PUT
        • 创建操作 → HttpMethod.POST
        • 删除操作 → HttpMethod.DELETE
    • API 类需要实现 __example__ 方法以便生成 API 文档,并确保生成对应的 Groovy API Template 与 API Markdown 文件。

2. 命名与格式规范

  • 类名:

    • 使用 UpperCamelCase 风格。
    • 特殊情况:
      • VO/AO/EO 类型类除外。
      • 抽象类采用 AbstractBase 前缀/后缀。
      • 异常类应以 Exception 结尾。
      • 测试类需要以 TestCase 结尾。
  • 方法名、参数名、成员变量和局部变量:

    • 使用 lowerCamelCase 风格。
  • 常量命名:

    • 全部大写,使用下划线分隔单词。
    • 要求表达清楚,避免使用含糊或不准确的名称。
  • 包名:

    • 统一使用小写,使用点分隔符,每个部分应是一个具有自然语义的英文单词(参考 Spring 框架的结构)。
  • 命名细节:

    • 避免在父子类或同一代码块中出现相同名字的成员或局部变量,防止混淆。
    • 命名缩写:
      • 不允许使用不必要的缩写,如:AbsSchedulerJobcondiFu 等。应使用完整单词提升可读性。

3. 编写自解释代码

  • 意图表达:

    • 避免使用布尔型参数造成含义不明确。例如:
      • 对于 stopAgent(boolean ignoreError),建议拆分为不同函数(如 stopAgentIgnoreError()),或使用枚举表达操作类型。
    • 命名应尽量用完整的单词组合表达意图,并在名称中体现数据类型或用途(例如在常量与变量名称中,将类型词放在末尾)。
    • 避免使用魔法值(Magic Value):
      • 直接使用未经定义的数值或字符串(如 if (status == 5))应替换为枚举或常量。

      • 示例:

      • // 错误示例:魔法值

      • if (user.getStatus() == 5) { ... }

      • // 正确示例:常量或枚举

      • public static final int STATUS_ACTIVE = 5;

      • if (user.getStatus() == STATUS_ACTIVE) { ... }

      • // 或使用枚举

      • enum UserStatus { ACTIVE, INACTIVE }

  • 注释:

    • 代码应尽量做到自解释,对少于两行的说明可以直接写在代码中。
    • 对于较长的注释,需要仔细校对并随代码更新,确保内容正确。
    • 接口方法不应有多余的修饰符(例如 public),且必须配有有效的 Javadoc 注释。

4. 流程控制和结构优化

  • if...else 的使用:

    • 应尽量减少 if...else 结构的使用,建议:
      • 限制嵌套层级最多为两层,且内层不应再出现 else 分支。
      • 尽早返回(Early Return),将条件判断中的处理逻辑提前结束或抽成独立方法。
      • 使用 Java Stream 或 Lambda 表达式代替冗长的循环与条件判断。
  • 条件判断:

    • if 条件表达不宜过长或过于复杂,必要时可以将条件抽成 boolean 变量描述。
  • 代码块长度:
    ...

Files:

  • plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java
🧠 Learnings (1)
📓 Common learnings
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 2566
File: conf/db/upgrade/V5.4.0__schema.sql:33-34
Timestamp: 2025-09-05T10:14:54.816Z
Learning: 在ZStack的HostNetworkInterfaceVO表中,pciDeviceAddress字段出现结尾换行符(\n)的脏数据仅在嵌套环境中出现,删除这些记录是安全的,不会产生影响。这种情况通常发生在主机重新连接时。
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 2360
File: network/src/main/java/org/zstack/network/l3/L3BasicNetwork.java:449-490
Timestamp: 2025-08-04T04:48:19.103Z
Learning: ZStack项目在cherry-pick操作中,即使发现了性能优化机会(如IP地址批量保存的内存优化),也严格遵循不做额外修改的政策,优先保证cherry-pick的完整性和一致性。
🔍 Remote MCP AtlassianMCP

Relevant Jira context (ZSTAC-67275) — concise facts for PR review

  • Jira summary: backend must provide a new identifier (vmPciDeviceAddress) so front-end can distinguish multiple passthrough GPUs on the same VM. Status: Open. Assignee: jin.shen. (ZSTAC-67275)
  • Fix version: 5.5.6 (issue marked required for 5.5.6) — confirms PR target branch is appropriate.
  • Required behavior called out in comments/changelog:
    • Add vmPciDeviceAddress column to PciDeviceVO and handle DB upgrade/population (MN to call dataplane to get VM PCI mapping).
    • Set vmPciDeviceAddress when VM loads/attaches a PCI device (cold-create, start, hotplug). Clear on unload.
    • Populate/update vmPciDeviceAddress during host reconnection (sync).
    • Frontend mapping rationale: vmUuid+SN insufficient; vmPciDeviceAddress allows mapping VM PCI address to GPU device (applies to GPU and vGPU).
  • Implementation references linked in Jira comments: GitLab Merge Requests (e.g., MR 9007) and Confluence design page for ZSTAC-67275 — useful cross-checks for DB upgrade and dataplane interactions.
  • Time / testing notes: comments list planned upgrade handling, MN/dataplane calls, and test coverage steps; worklogs indicate active development and review iterations.

Sources queried:

  • AtlassianMCP: jira_get_issue for ZSTAC-67275
🔇 Additional comments (1)
plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java (1)

2917-2918: 字段声明正确,符合灰度升级规范。

新增的 pciDeviceInfos 字段使用 Map<String, String> 类型存储 PCI 设备映射信息,@GrayVersion(value = "5.5.6") 注解与目标分支版本一致,确保灰度升级兼容性。

@MatheMatrix MatheMatrix force-pushed the sync/jin.shen/fix-67275@@3 branch 2 times, most recently from 1af977c to 272578a Compare January 15, 2026 09:59
- Add vmPciDeviceAddress field to PciDeviceVO
- Set vmPciDeviceAddress when loading PCI device to VM
- Clear vmPciDeviceAddress when unloading PCI device
- Update vmPciDeviceAddress during host reconnection
- Add mdevDeviceAddress field to MdevDeviceVO

Resolves: ZSTAC-67275

Change-Id: I67626e67787062616679786b6576636c73747477
@MatheMatrix MatheMatrix force-pushed the sync/jin.shen/fix-67275@@3 branch from 272578a to 5b39a87 Compare January 15, 2026 10:22
@zstack-robot-2 zstack-robot-2 deleted the sync/jin.shen/fix-67275@@3 branch January 16, 2026 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants