Skip to content

Conversation

@ZStack-Robot
Copy link
Collaborator

add vmPciDeviceAddress for PciDeviceVO

Resolves: ZSTAC-67275

Change-Id: I67626e67787062616679786b6576636c73747477

sync from gitlab !9006

add vmPciDeviceAddress for PciDeviceVO

Resolves: ZSTAC-67275

Change-Id: I67626e67787062616679786b6576636c73747477
@coderabbitai
Copy link

coderabbitai bot commented Jan 12, 2026

技术走查

该变更向PCI设备表增加了一个新列,并在虚拟机设备信息响应类中扩展了新的附加信息字段,为版本5.5.6提供数据结构支持。

变更内容

变更类别 / 文件 变更摘要
数据库架构升级
conf/db/upgrade/V5.5.6__schema.sql
向PciDeviceVO表新增vmPciDeviceAddress列(varchar(32))
KVM代理命令扩展
plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java
在VmDevicesInfoResponse类中添加addonInfos字段及其getter/setter方法,带有@GrayVersion(5.5.6)注解

预估代码审查工作量

🎯 1 (Trivial) | ⏱️ ~3 minutes

诗歌

🐰 新列轻轻添,字段缓缓入,
五五六更新,扩展显风采!
小改大智慧 ✨

🚥 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]: 格式,符合要求的72字符限制(实际50字符),准确描述了主要变更:向PciDeviceVO添加vmPciDeviceAddress字段。
Description check ✅ Passed 提交描述与变更集相关,清晰说明了变更内容(添加vmPciDeviceAddress字段)并关联了JIRA问题ZSTAC-67275,提供了足够的背景信息。

✏️ 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 ee9f20d and 2f262ea.

⛔ 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
🧰 Additional context used
📓 Path-based instructions (3)
**/*.*

⚙️ CodeRabbit configuration file

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

Files:

  • plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java
  • conf/db/upgrade/V5.5.6__schema.sql
**/*.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
**/*.sql

⚙️ CodeRabbit configuration file

**/*.sql: - Review the SQL code, make sure has no errors and confirm that:

  • Upgrading scene has been carefully handled
  • Do not use DEFAULT 0000-00-00 00:00:00 , use DEFAULT CURRENT_TIMESTAMP instead
  • When NOT NULL exists, must use stored procedure or other functions to process historical data, this is very very important
  • 数据库记录中,如果字符串长度不可控,不要用 vchar,用 text 类型

Files:

  • conf/db/upgrade/V5.5.6__schema.sql
🧠 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的完整性和一致性。
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 2489
File: storage/src/main/java/org/zstack/storage/snapshot/VolumeTree.java:471-545
Timestamp: 2025-08-22T05:36:40.467Z
Learning: In ZStack项目中,对于核心逻辑相关的代码,团队倾向于暂时不进行重构,即使代码复杂度较高,优先保证核心功能的稳定性。
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 2325
File: compute/src/main/java/org/zstack/compute/vm/VmMigrateCallExtensionFlow.java:29-29
Timestamp: 2025-07-24T05:53:10.246Z
Learning: 在ZStack项目的重构过程中,团队采用务实的优先级策略:优先完成影响运行时功能的代码重构,对于注释中的旧引用等非功能性问题可以延后处理,因为这些不会影响系统的实际运行。
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 2360
File: compute/src/main/java/org/zstack/compute/vm/StaticIpOperator.java:364-501
Timestamp: 2025-08-04T03:19:07.454Z
Learning: In ZStack project, developers may prefer to keep related validation logic together in a single method (like validateStaticIpTagsInApiMessage with 130+ lines) when the architecture is clear and there's no code reuse, rather than splitting into smaller methods based purely on single responsibility principle.
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 2419
File: plugin/sdnController/src/main/java/org/zstack/sdnController/SdnControllerBase.java:986-1067
Timestamp: 2025-08-13T02:31:40.023Z
Learning: 在ZStack项目中,团队倾向于保持统一的业务流程方法不进行拆分,即使方法较长也会维持在单个方法中以保持业务逻辑的完整性。
Learnt from: zstack-robot-2
Repo: MatheMatrix/zstack PR: 2496
File: plugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/KvmBackend.java:2545-2566
Timestamp: 2025-08-24T07:35:42.793Z
Learning: 在 ZStack 代码库中,当响应类包含数值字段(如 size、actualSize)时,优先使用原始类型(long)而不是包装类型(Long),以避免 NPE 风险和不必要的装箱/拆箱操作。如果 Agent 端可能不设置该字段,应在 Agent 端确保设置默认值,而不是在使用端做 null 检查。
🔍 Remote MCP AtlassianMCP

PR Context and Background

Issue Details: ZSTAC-67275

The issue is titled "【5.5.6必须解】CLONE - Gpu-透传的Gpu目前是用VMUuid 来做唯一标识,当vm加载多块Gpu时,Gpu 的工作状态、监控数据的显示其实都是一样的,后端需提供一个能标识的新字段来解决" (Translation: GPU pass-through devices currently use VMUuid as a unique identifier. When a VM loads multiple GPUs, their working status and monitoring data display are identical. The backend needs to provide a new field to identify each GPU).

Status and Priority: The issue is marked as Open with P1 (highest) priority

Problem Analysis: The frontend can obtain the pciDeviceAddress from the VM, but cannot determine which GPU device it corresponds to. Current approach using vmUuid+SN has issues because some GPU devices lack an SN number. The solution is for the backend to add a vmPciDeviceAddress field to PciDeviceVO so the frontend can query GPU devices by the VM's PCI address. This field will work for both physical GPUs and vGPUs since they differ only in PCI device address.

Implementation Work Plan: The work involves: (1) Database changes and upgrade handling (~1d); (2) VM GPU load/unload handling with new parameter returns (~1d); (3) Creating VMs with GPU scenarios (~0.5-1d); (4) Testing (~1d); (5) Review and optimization (~1d)

Recent Progress: The developer has written upgrade logic and discovered that initial self-testing failed. The approach was changed to assign vmPciDeviceAddress when syncing host information, with detailed notes available at a Confluence page for ZSTAC-67275.

Related Issues

The issue is linked to two other critical P1 issues:

  • ZSTAC-69036: "云主机加载多张gpu卡,如果gpu设备本身没有sn号,会导致监控数据错误、GPU卡片数量统计异常" (Multiple GPU cards on a VM without SN cause incorrect monitoring data and GPU count statistics)
  • ZSTAC-72850: "【5.5.6必须解】首页物理GPU相关资源top排序 有重复的GPU卡名字" (Duplicate GPU names in homepage resource ranking)
🔇 Additional comments (2)
conf/db/upgrade/V5.5.6__schema.sql (1)

1-1: 确认 ADD_COLUMN 第4参数的含义和历史数据处理需求

根据代码库中 ADD_COLUMN 的使用模式分析,参数4为 1 时表示 NOT NULL 约束,参数5为 NULL 时表示无默认值。当前代码向 PciDeviceVO 表添加 NOT NULL 列且无默认值:

CALL ADD_COLUMN('PciDeviceVO', 'vmPciDeviceAddress', 'varchar(32)', 1, NULL);

根据编码规范,当存在 NOT NULL 约束时必须使用存储过程处理历史数据。建议确认:

  1. ADD_COLUMN 存储过程是否自动处理现有记录的 NULL 值填充
  2. 如果不自动处理,需先通过存储过程为现有 PciDeviceVO 记录赋初值(如空字符串 ''),或改为可空列 / 提供默认值
plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java (1)

2917-2958: 字段添加正确,但原评论中的验证假设不成立

新增的 addonInfos 字段实现规范:

  • @GrayVersion(value = "5.5.6") 注解与版本相符
  • getter/setter 方法实现标准,遵循现有代码模式
  • Map<String, Object> 作为灵活扩展字段的设计合理

需说明的是:代码库中不存在 vmPciDeviceAddress 属性和 PciDeviceVO 类。addonInfos 是作为新增的扩展字段,供 KVMSyncVmDeviceInfoExtensionPoint 等扩展点在 afterReceiveVmDeviceInfoResponse 方法中使用,以支持未来的扩展需求。


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

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.

2 participants