Skip to content

<feature>[sdk]: support dgpu sdk#3676

Closed
ZStack-Robot wants to merge 1 commit into5.5.12_dongzhufrom
sync/xinhao.huang/5.5.12_dongzhu-dgpu@@2
Closed

<feature>[sdk]: support dgpu sdk#3676
ZStack-Robot wants to merge 1 commit into5.5.12_dongzhufrom
sync/xinhao.huang/5.5.12_dongzhu-dgpu@@2

Conversation

@ZStack-Robot
Copy link
Copy Markdown
Collaborator

Cherry-pick dgpu-related sdk changes from upstream/feature-5.5.12-dgpu.

DBImpact

Resolves: ZSTAC-83477

Change-Id: I61746e646972787a6f72757565776d6570617768

sync from gitlab !9536

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 3, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

此次更新为GPU虚拟化和设备管理增加了数据库模式支持,包括GPU规格去重、模型评估任务排序、PCI设备虚拟化能力追踪、以及TensorFusion dGPU设备管理。同时新增资源指标绑定扩展接口、dGPU测试助手方法和相关错误码常量。

Changes

Cohort / File(s) Summary
Database Schema Migration
conf/db/upgrade/V5.5.12__schema.sql
为GPU设备规格添加normalizedModelName字段与索引;为模型评估任务添加totalScoreendTime字段用于排序,并从JSON opaque字段回填数据;创建PciDeviceVirtCapabilityVO表记录PCI设备虚拟化能力;向PciDeviceVO添加virtStatevirtMode字段;向GpuDeviceVO添加mode字段用于设备分类(DGPU/VGPU/PCI);新增TensorFusion dGPU支持表:DGpuProfileVODGpuDeviceVOVmInstanceDGpuStrategyVO
Resource Metric Binding Extension
header/src/main/java/org/zstack/header/zwatch/ResourceMetricBindingExtensionPoint.java
新增ResourceMetricBindingExtensionPoint扩展接口和ResourceMetricBinding配置类,支持定义资源指标绑定映射关系,包括资源类型、指标名称、来源标识和绑定字段等属性。
DGpu Test Helpers
testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy
添加8个dGPU相关API测试助手方法:detachDGpuFromVmdisableDGpuModeenableDGpuModegetDGpuSpecStatsqueryDGpuDevicequeryDGpuProfileremoveVmDGpuStrategysetDGpuProfilesetVmDGpuStrategy,支持API路径追踪和错误处理。
Error Code Constants
utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
新增24个错误码常量,涵盖zwatch、LDAP、存储、PCI设备、Prometheus和AI/dGPU等模块。

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 数据库建筑层层堆,GPU虚拟化展新颜,
dGPU策略深思熟,接口扩展开新局,
兔子祝贺此番功,代码升级展雄心!

🚥 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]: 格式要求,长度32字符(不超过72字符),清晰描述了PR的主要目的是支持dGPU SDK功能。
Description check ✅ Passed PR描述与变更内容相关,说明了从上游feature分支cherry-pick dGPU相关SDK变更,并关联了JIRA问题ZSTAC-83477。

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch sync/xinhao.huang/5.5.12_dongzhu-dgpu@@2

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

Copy link
Copy Markdown

@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: 1

🧹 Nitpick comments (6)
header/src/main/java/org/zstack/header/zwatch/ResourceMetricBindingExtensionPoint.java (2)

22-28: requireText 验证后返回的是未经 trim 的原始值。

当前实现检查 value.trim().isEmpty() 来验证非空,但最终返回的是原始的 value。如果输入值包含前后空格(如 " myMetric "),验证会通过,但返回的值仍带有空格,可能导致字符串比较或作为 key 使用时出现不一致的行为。

根据项目规范,用户输入可能包含复制粘贴带来的空格/换行符,建议返回 trim 后的值:

♻️ 建议修改
 private static String requireText(String fieldName, String value) {
     requireValue(fieldName, value);
-    if (value.trim().isEmpty()) {
+    String trimmed = value.trim();
+    if (trimmed.isEmpty()) {
         throw new IllegalStateException(String.format("ResourceMetricBinding.%s must not be empty", fieldName));
     }
-    return value;
+    return trimmed;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@header/src/main/java/org/zstack/header/zwatch/ResourceMetricBindingExtensionPoint.java`
around lines 22 - 28, The requireText method currently validates emptiness using
value.trim() but returns the original untrimmed value; update the implementation
of requireText (in class ResourceMetricBindingExtensionPoint) to trim the input
once (e.g., String trimmed = value == null ? null : value.trim()), call
requireValue on the original or trimmed as appropriate, validate using the
trimmed variable (throwing the same IllegalStateException if trimmed.isEmpty()),
and return the trimmed string so callers receive the canonicalized value.

5-88: 建议添加 Javadoc 文档注释。

根据编码规范,接口方法必须配有有效的 Javadoc 注释。建议为接口和方法添加文档说明其用途。

📝 建议添加文档
+/**
+ * Extension point for providing resource metric binding configurations.
+ * Implementations supply mappings between logical metrics and their source definitions.
+ */
 public interface ResourceMetricBindingExtensionPoint {
     // ... nested class ...

+    /**
+     * Returns the list of resource metric bindings provided by this extension point.
+     *
+     * `@return` list of metric binding configurations
+     */
     List<ResourceMetricBinding> getResourceMetricBindings();
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@header/src/main/java/org/zstack/header/zwatch/ResourceMetricBindingExtensionPoint.java`
around lines 5 - 88, The interface ResourceMetricBindingExtensionPoint and its
nested class ResourceMetricBinding lack Javadoc; add clear Javadoc comments to
the interface, the nested class, and the public API method
getResourceMetricBindings(), and also document the purpose of
ResourceMetricBinding and each of its public getters/setters (getResourceType,
getLogicalMetricName, getSourceNamespace, getSourceMetricName, getResourceField,
getSourceLabel, isRequireUniqueSourceKey) explaining expected semantics,
null/empty constraints and side effects so the API intent and usage are clear.
utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java (1)

6283-6284: 建议保持命名模式一致性

常量 ORG_ZSTACK_STORAGE_BACKUP_CANCEL_TIMEOUT 使用了描述性后缀,而整个文件中其他所有错误码常量都遵循 ORG_ZSTACK_[MODULE]_[数字] 的模式(如 ORG_ZSTACK_STORAGE_BACKUP_10133)。

虽然描述性命名本身是清晰的,但打破统一模式可能导致:

  • 开发人员在添加新错误码时不确定应遵循哪种模式
  • 维护和自动化工具处理时的不一致性

建议后续将此常量重命名为类似 ORG_ZSTACK_STORAGE_BACKUP_10134 的格式以保持一致性。

:考虑到这是 cherry-pick 操作,如果上游分支已使用此名称,可在后续统一优化时处理。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java`
around lines 6283 - 6284, The constant ORG_ZSTACK_STORAGE_BACKUP_CANCEL_TIMEOUT
in CloudOperationsErrorCode breaks the file's naming convention; rename this
constant to a numeric-style identifier (e.g., ORG_ZSTACK_STORAGE_BACKUP_10134)
and update all references/usages accordingly (ensure the constant value string
matches the new identifier) so it follows the ORG_ZSTACK_[MODULE]_[NUMBER]
pattern used by other error codes.
conf/db/upgrade/V5.5.12__schema.sql (3)

37-48: TIMESTAMP 列建议添加显式默认值。

根据编码规范,应使用 DEFAULT CURRENT_TIMESTAMP 而非隐式依赖 MySQL 行为。在不同的 MySQL 配置(如 explicit_defaults_for_timestamp)下,未指定默认值的 TIMESTAMP NOT NULL 列可能导致插入失败。

建议的修复
 CREATE TABLE IF NOT EXISTS `zstack`.`PciDeviceVirtCapabilityVO` (
     `id`            BIGINT UNSIGNED NOT NULL AUTO_INCREMENT,
     `pciDeviceUuid` VARCHAR(32)     NOT NULL,
     `capability`    VARCHAR(32)     NOT NULL,
-    `createDate`    TIMESTAMP       NOT NULL,
-    `lastOpDate`    TIMESTAMP       NOT NULL,
+    `createDate`    TIMESTAMP       NOT NULL DEFAULT CURRENT_TIMESTAMP,
+    `lastOpDate`    TIMESTAMP       NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
     PRIMARY KEY (`id`),

As per coding guidelines: "Do not use DEFAULT 0000-00-00 00:00:00, use DEFAULT CURRENT_TIMESTAMP instead"

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@conf/db/upgrade/V5.5.12__schema.sql` around lines 37 - 48, The TIMESTAMP
columns in PciDeviceVirtCapabilityVO (createDate, lastOpDate) lack explicit
defaults and should use CURRENT_TIMESTAMP per guidelines; change createDate to
TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP and change lastOpDate to TIMESTAMP
NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP (avoid DEFAULT
'0000-00-00 00:00:00'), so inserts and automatic updates behave consistently
across MySQL configurations.

126-168: TIMESTAMP 列需要添加显式默认值。

PciDeviceVirtCapabilityVO 相同的问题,createDatelastOpDate 列应添加 DEFAULT CURRENT_TIMESTAMP。此问题影响 DGpuProfileVO(第131-132行)和 DGpuDeviceVO(第153-154行)。

建议的修复
 CREATE TABLE IF NOT EXISTS `zstack`.`DGpuProfileVO` (
     ...
-    `createDate`  TIMESTAMP        NOT NULL,
-    `lastOpDate`  TIMESTAMP        NOT NULL,
+    `createDate`  TIMESTAMP        NOT NULL DEFAULT CURRENT_TIMESTAMP,
+    `lastOpDate`  TIMESTAMP        NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
     PRIMARY KEY (`uuid`),
 CREATE TABLE IF NOT EXISTS `zstack`.`DGpuDeviceVO` (
     ...
-    `createDate`       TIMESTAMP        NOT NULL,
-    `lastOpDate`       TIMESTAMP        NOT NULL,
+    `createDate`       TIMESTAMP        NOT NULL DEFAULT CURRENT_TIMESTAMP,
+    `lastOpDate`       TIMESTAMP        NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
     PRIMARY KEY (`uuid`),

As per coding guidelines: "Do not use DEFAULT 0000-00-00 00:00:00, use DEFAULT CURRENT_TIMESTAMP instead"

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@conf/db/upgrade/V5.5.12__schema.sql` around lines 126 - 168, The TIMESTAMP
columns createDate and lastOpDate in tables DGpuProfileVO and DGpuDeviceVO lack
explicit defaults; update the table definitions for DGpuProfileVO (`createDate`,
`lastOpDate`) and DGpuDeviceVO (`createDate`, `lastOpDate`) to use DEFAULT
CURRENT_TIMESTAMP (and for lastOpDate consider DEFAULT CURRENT_TIMESTAMP ON
UPDATE CURRENT_TIMESTAMP if matching PciDeviceVirtCapabilityVO behavior) so they
no longer rely on implicit/zero timestamps and comply with the guideline to
avoid DEFAULT '0000-00-00 00:00:00'.

170-190: TIMESTAMP 列需要添加显式默认值。

VmInstanceDGpuStrategyVO 表的 createDatelastOpDate 列(第179-180行)同样需要添加 DEFAULT CURRENT_TIMESTAMP

建议的修复
     `autoDetachOnStop` TINYINT(1)      NOT NULL DEFAULT 1,
-    `createDate`       TIMESTAMP       NOT NULL,
-    `lastOpDate`       TIMESTAMP       NOT NULL,
+    `createDate`       TIMESTAMP       NOT NULL DEFAULT CURRENT_TIMESTAMP,
+    `lastOpDate`       TIMESTAMP       NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
     PRIMARY KEY (`id`),

As per coding guidelines: "Do not use DEFAULT 0000-00-00 00:00:00, use DEFAULT CURRENT_TIMESTAMP instead"

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@conf/db/upgrade/V5.5.12__schema.sql` around lines 170 - 190, The TIMESTAMP
columns in VmInstanceDGpuStrategyVO (createDate and lastOpDate) lack explicit
defaults; update the CREATE TABLE DDL for VmInstanceDGpuStrategyVO to add
DEFAULT CURRENT_TIMESTAMP to both createDate and lastOpDate (and optionally add
ON UPDATE CURRENT_TIMESTAMP for lastOpDate if the intent is to auto-update it)
so they no longer rely on zero-dates; modify the column definitions for
createDate and lastOpDate accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@conf/db/upgrade/V5.5.12__schema.sql`:
- Around line 17-21: The UPDATE on table ModelEvaluationTaskVO sets totalScore
from Json_getKeyValue(opaque, 'total_score') but lacks a check for empty-string
results, which can cause CAST to yield 0; modify the WHERE clause (and/or the
expression) in the UPDATE that references Json_getKeyValue(`opaque`,
'total_score') to also require Json_getKeyValue(`opaque`, 'total_score') != ''
so only non-empty values are cast and written to totalScore (mirror the same !=
'' check used for endTime).

---

Nitpick comments:
In `@conf/db/upgrade/V5.5.12__schema.sql`:
- Around line 37-48: The TIMESTAMP columns in PciDeviceVirtCapabilityVO
(createDate, lastOpDate) lack explicit defaults and should use CURRENT_TIMESTAMP
per guidelines; change createDate to TIMESTAMP NOT NULL DEFAULT
CURRENT_TIMESTAMP and change lastOpDate to TIMESTAMP NOT NULL DEFAULT
CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP (avoid DEFAULT '0000-00-00
00:00:00'), so inserts and automatic updates behave consistently across MySQL
configurations.
- Around line 126-168: The TIMESTAMP columns createDate and lastOpDate in tables
DGpuProfileVO and DGpuDeviceVO lack explicit defaults; update the table
definitions for DGpuProfileVO (`createDate`, `lastOpDate`) and DGpuDeviceVO
(`createDate`, `lastOpDate`) to use DEFAULT CURRENT_TIMESTAMP (and for
lastOpDate consider DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP if
matching PciDeviceVirtCapabilityVO behavior) so they no longer rely on
implicit/zero timestamps and comply with the guideline to avoid DEFAULT
'0000-00-00 00:00:00'.
- Around line 170-190: The TIMESTAMP columns in VmInstanceDGpuStrategyVO
(createDate and lastOpDate) lack explicit defaults; update the CREATE TABLE DDL
for VmInstanceDGpuStrategyVO to add DEFAULT CURRENT_TIMESTAMP to both createDate
and lastOpDate (and optionally add ON UPDATE CURRENT_TIMESTAMP for lastOpDate if
the intent is to auto-update it) so they no longer rely on zero-dates; modify
the column definitions for createDate and lastOpDate accordingly.

In
`@header/src/main/java/org/zstack/header/zwatch/ResourceMetricBindingExtensionPoint.java`:
- Around line 22-28: The requireText method currently validates emptiness using
value.trim() but returns the original untrimmed value; update the implementation
of requireText (in class ResourceMetricBindingExtensionPoint) to trim the input
once (e.g., String trimmed = value == null ? null : value.trim()), call
requireValue on the original or trimmed as appropriate, validate using the
trimmed variable (throwing the same IllegalStateException if trimmed.isEmpty()),
and return the trimmed string so callers receive the canonicalized value.
- Around line 5-88: The interface ResourceMetricBindingExtensionPoint and its
nested class ResourceMetricBinding lack Javadoc; add clear Javadoc comments to
the interface, the nested class, and the public API method
getResourceMetricBindings(), and also document the purpose of
ResourceMetricBinding and each of its public getters/setters (getResourceType,
getLogicalMetricName, getSourceNamespace, getSourceMetricName, getResourceField,
getSourceLabel, isRequireUniqueSourceKey) explaining expected semantics,
null/empty constraints and side effects so the API intent and usage are clear.

In
`@utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java`:
- Around line 6283-6284: The constant ORG_ZSTACK_STORAGE_BACKUP_CANCEL_TIMEOUT
in CloudOperationsErrorCode breaks the file's naming convention; rename this
constant to a numeric-style identifier (e.g., ORG_ZSTACK_STORAGE_BACKUP_10134)
and update all references/usages accordingly (ensure the constant value string
matches the new identifier) so it follows the ORG_ZSTACK_[MODULE]_[NUMBER]
pattern used by other error codes.
🪄 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: e1402635-d6da-4bf1-a62e-581c60a4bfc6

📥 Commits

Reviewing files that changed from the base of the PR and between cf48ab8 and 40e352e.

⛔ Files ignored due to path filters (31)
  • conf/i18n/globalErrorCodeMapping/global-error-en_US.json is excluded by !**/*.json
  • conf/i18n/globalErrorCodeMapping/global-error-zh_CN.json is excluded by !**/*.json
  • sdk/src/main/java/SourceClassMap.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DGpuDeviceInventory.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DGpuProfileInventory.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DGpuSpecStatsInventory.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DGpuStatus.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DetachDGpuFromVmAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DetachDGpuFromVmResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DisableDGpuModeAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DisableDGpuModeResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/EnableDGpuModeAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/EnableDGpuModeResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/GetDGpuSpecStatsAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/GetDGpuSpecStatsResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/GpuDeviceInventory.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/GpuDeviceSpecCandidateInventory.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/PciDeviceInventory.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/PciDeviceVirtMode.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/PciDeviceVirtState.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/PciDeviceVirtStatus.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/QueryDGpuDeviceAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/QueryDGpuDeviceResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/QueryDGpuProfileAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/QueryDGpuProfileResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/RemoveVmDGpuStrategyAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/RemoveVmDGpuStrategyResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/SetDGpuProfileAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/SetDGpuProfileResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/SetVmDGpuStrategyAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/SetVmDGpuStrategyResult.java is excluded by !sdk/**
📒 Files selected for processing (4)
  • conf/db/upgrade/V5.5.12__schema.sql
  • header/src/main/java/org/zstack/header/zwatch/ResourceMetricBindingExtensionPoint.java
  • testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy
  • utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java

@MatheMatrix MatheMatrix force-pushed the sync/xinhao.huang/5.5.12_dongzhu-dgpu@@2 branch 2 times, most recently from ae6122d to 45a57a0 Compare April 7, 2026 02:36
Copy link
Copy Markdown

@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: 2

🧹 Nitpick comments (1)
conf/db/upgrade/V5.5.12__schema.sql (1)

38-49: TIMESTAMP 列应显式指定 DEFAULT CURRENT_TIMESTAMP

在 MySQL 8.0 中,explicit_defaults_for_timestamps 默认为 ON,TIMESTAMP 列不会自动获得默认值。建议为 createDatelastOpDate 列添加显式默认值以确保兼容性。

此问题同样存在于 DGpuProfileVO(第132-133行)、DGpuDeviceVO(第154-155行)和 VmInstanceDGpuStrategyVO(第180-181行)表中。

建议修改
 CREATE TABLE IF NOT EXISTS `zstack`.`PciDeviceVirtCapabilityVO` (
     `id`            BIGINT UNSIGNED NOT NULL AUTO_INCREMENT,
     `pciDeviceUuid` VARCHAR(32)     NOT NULL,
     `capability`    VARCHAR(32)     NOT NULL,
-    `createDate`    TIMESTAMP       NOT NULL,
-    `lastOpDate`    TIMESTAMP       NOT NULL,
+    `createDate`    TIMESTAMP       NOT NULL DEFAULT CURRENT_TIMESTAMP,
+    `lastOpDate`    TIMESTAMP       NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
     PRIMARY KEY (`id`),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@conf/db/upgrade/V5.5.12__schema.sql` around lines 38 - 49, The TIMESTAMP
columns lack explicit defaults; update the schema for tables
PciDeviceVirtCapabilityVO, DGpuProfileVO, DGpuDeviceVO, and
VmInstanceDGpuStrategyVO so that createDate is defined with DEFAULT
CURRENT_TIMESTAMP and lastOpDate is defined with DEFAULT CURRENT_TIMESTAMP ON
UPDATE CURRENT_TIMESTAMP (i.e., modify the column definitions for createDate and
lastOpDate in each table to include these explicit defaults to ensure MySQL 8+
compatibility).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@conf/db/upgrade/V5.5.12__schema.sql`:
- Around line 109-115: The UPDATE is missing a WHERE clause and will overwrite
g.`mode` every run; change the statement that joins `GpuDeviceVO` and
`PciDeviceVO` (referencing g.`mode`, p.`virtState`, p.`virtMode`) to only set
mode when it is unset or empty to make the migration idempotent (for example add
a WHERE such as g.`mode` IS NULL OR g.`mode` = '' so only rows without a mode
are updated).

In
`@header/src/main/java/org/zstack/header/zwatch/ResourceMetricBindingExtensionPoint.java`:
- Line 87: 在 ResourceMetricBindingExtensionPoint 接口的核心方法
getResourceMetricBindings() 上补充完整的 Javadoc,说明该方法的返回语义(返回项代表什么)、每个
ResourceMetricBinding 对象中哪些字段为必填/可选、是否允许返回 null 或空列表(建议明确为“不可为
null,可返回空列表”或反之),以及调用方对返回值的期望(是否需要做深拷贝或不可修改返回集合)。在文档中使用接口名
ResourceMetricBindingExtensionPoint 和方法名 getResourceMetricBindings()
以便定位,并遵守接口规范不添加多余修饰符。

---

Nitpick comments:
In `@conf/db/upgrade/V5.5.12__schema.sql`:
- Around line 38-49: The TIMESTAMP columns lack explicit defaults; update the
schema for tables PciDeviceVirtCapabilityVO, DGpuProfileVO, DGpuDeviceVO, and
VmInstanceDGpuStrategyVO so that createDate is defined with DEFAULT
CURRENT_TIMESTAMP and lastOpDate is defined with DEFAULT CURRENT_TIMESTAMP ON
UPDATE CURRENT_TIMESTAMP (i.e., modify the column definitions for createDate and
lastOpDate in each table to include these explicit defaults to ensure MySQL 8+
compatibility).
🪄 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: f4170d95-1abb-45fc-8818-4e67d7a1e294

📥 Commits

Reviewing files that changed from the base of the PR and between ae6122d and 45a57a0.

⛔ Files ignored due to path filters (31)
  • conf/i18n/globalErrorCodeMapping/global-error-en_US.json is excluded by !**/*.json
  • conf/i18n/globalErrorCodeMapping/global-error-zh_CN.json is excluded by !**/*.json
  • sdk/src/main/java/SourceClassMap.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DGpuDeviceInventory.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DGpuProfileInventory.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DGpuSpecStatsInventory.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DGpuStatus.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DetachDGpuFromVmAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DetachDGpuFromVmResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DisableDGpuModeAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DisableDGpuModeResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/EnableDGpuModeAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/EnableDGpuModeResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/GetDGpuSpecStatsAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/GetDGpuSpecStatsResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/GpuDeviceInventory.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/GpuDeviceSpecCandidateInventory.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/PciDeviceInventory.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/PciDeviceVirtMode.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/PciDeviceVirtState.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/PciDeviceVirtStatus.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/QueryDGpuDeviceAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/QueryDGpuDeviceResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/QueryDGpuProfileAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/QueryDGpuProfileResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/RemoveVmDGpuStrategyAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/RemoveVmDGpuStrategyResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/SetDGpuProfileAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/SetDGpuProfileResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/SetVmDGpuStrategyAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/SetVmDGpuStrategyResult.java is excluded by !sdk/**
📒 Files selected for processing (4)
  • conf/db/upgrade/V5.5.12__schema.sql
  • header/src/main/java/org/zstack/header/zwatch/ResourceMetricBindingExtensionPoint.java
  • testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy
  • utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
✅ Files skipped from review due to trivial changes (1)
  • utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
🚧 Files skipped from review as they are similar to previous changes (1)
  • testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy

Comment on lines +109 to +115
UPDATE `zstack`.`GpuDeviceVO` g
INNER JOIN `zstack`.`PciDeviceVO` p ON g.`uuid` = p.`uuid`
SET g.`mode` = CASE
WHEN p.`virtState` = 'VIRTUALIZED' AND p.`virtMode` = 'TENSORFUSION' THEN 'DGPU'
WHEN p.`virtState` = 'VIRTUALIZED' AND p.`virtMode` IN ('VFIO_MDEV', 'SRIOV') THEN 'VGPU'
ELSE 'PCI'
END;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

缺少 WHERE 条件,可能影响升级幂等性。

此 UPDATE 语句没有 WHERE 条件,每次执行升级脚本时都会覆盖 mode 列的值。如果应用程序已经设置了该值,或脚本重复执行,可能导致数据被意外覆盖。

建议修改
 UPDATE `zstack`.`GpuDeviceVO` g
 INNER JOIN `zstack`.`PciDeviceVO` p ON g.`uuid` = p.`uuid`
 SET g.`mode` = CASE
     WHEN p.`virtState` = 'VIRTUALIZED' AND p.`virtMode` = 'TENSORFUSION' THEN 'DGPU'
     WHEN p.`virtState` = 'VIRTUALIZED' AND p.`virtMode` IN ('VFIO_MDEV', 'SRIOV') THEN 'VGPU'
     ELSE 'PCI'
-END;
+END
+WHERE g.`mode` IS NULL;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@conf/db/upgrade/V5.5.12__schema.sql` around lines 109 - 115, The UPDATE is
missing a WHERE clause and will overwrite g.`mode` every run; change the
statement that joins `GpuDeviceVO` and `PciDeviceVO` (referencing g.`mode`,
p.`virtState`, p.`virtMode`) to only set mode when it is unset or empty to make
the migration idempotent (for example add a WHERE such as g.`mode` IS NULL OR
g.`mode` = '' so only rows without a mode are updated).

}
}

List<ResourceMetricBinding> getResourceMetricBindings();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

请为接口方法补充 Javadoc(当前不满足接口规范)。

getResourceMetricBindings() 是扩展点核心入口,建议补充返回值语义和字段约束(如哪些字段必填、是否允许空列表)。

建议修改
+    /**
+     * Return metric bindings contributed by this extension.
+     * Each binding should provide all required fields in {`@link` ResourceMetricBinding}.
+     *
+     * `@return` resource metric bindings, empty list if no bindings are provided
+     */
     List<ResourceMetricBinding> getResourceMetricBindings();

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

📝 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
List<ResourceMetricBinding> getResourceMetricBindings();
/**
* Return metric bindings contributed by this extension.
* Each binding should provide all required fields in {`@link` ResourceMetricBinding}.
*
* `@return` resource metric bindings, empty list if no bindings are provided
*/
List<ResourceMetricBinding> getResourceMetricBindings();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@header/src/main/java/org/zstack/header/zwatch/ResourceMetricBindingExtensionPoint.java`
at line 87, 在 ResourceMetricBindingExtensionPoint 接口的核心方法
getResourceMetricBindings() 上补充完整的 Javadoc,说明该方法的返回语义(返回项代表什么)、每个
ResourceMetricBinding 对象中哪些字段为必填/可选、是否允许返回 null 或空列表(建议明确为“不可为
null,可返回空列表”或反之),以及调用方对返回值的期望(是否需要做深拷贝或不可修改返回集合)。在文档中使用接口名
ResourceMetricBindingExtensionPoint 和方法名 getResourceMetricBindings()
以便定位,并遵守接口规范不添加多余修饰符。

@MatheMatrix MatheMatrix force-pushed the sync/xinhao.huang/5.5.12_dongzhu-dgpu@@2 branch from 1905be8 to 1b22ec2 Compare April 7, 2026 03:45
Copy link
Copy Markdown

@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: 3

♻️ Duplicate comments (1)
header/src/main/java/org/zstack/header/zwatch/ResourceMetricBindingExtensionPoint.java (1)

87-87: 🛠️ Refactor suggestion | 🟠 Major

请为接口方法 getResourceMetricBindings() 补充 Javadoc。

根据编码规范,接口方法必须配有有效的 Javadoc 注释。建议说明返回值语义、是否允许返回空列表或 null。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@header/src/main/java/org/zstack/header/zwatch/ResourceMetricBindingExtensionPoint.java`
at line 87, 为 ResourceMetricBindingExtensionPoint 接口的
getResourceMetricBindings() 方法补充 Javadoc:说明该方法返回的语义(返回一个包含 ResourceMetricBinding
的列表,代表该扩展点注册或支持的资源指标绑定),明确是否允许返回 null 或空列表(例如:不应返回
null,空列表表示无绑定),以及调用方应如何处理返回值;在注释中引用返回类型 ResourceMetricBinding 和 接口名
ResourceMetricBindingExtensionPoint 以便定位。
🧹 Nitpick comments (10)
core/src/main/java/org/zstack/core/log/LogSafeGson.java (1)

217-219: 建议:toJson 方法缺少相同的空值保护。

toJson 方法也存在相同的 NPE 风险:当 o 为 null 时,o.getClass() 会抛出异常。为保持一致性和健壮性,建议添加类似的空值检查。

♻️ 建议的修复
 public static String toJson(Object o) {
+    if (o == null) {
+        return "null";
+    }
     return logSafeGson.toJson(o, getGsonType(o.getClass()));
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@core/src/main/java/org/zstack/core/log/LogSafeGson.java` around lines 217 -
219, The toJson method can throw an NPE because it calls o.getClass() when o is
null; update the toJson(Object o) implementation to first check for null and
handle it (e.g., return the JSON null representation by calling
logSafeGson.toJson(null) or returning "null") before calling
getGsonType(o.getClass()), ensuring getGsonType and o.getClass() are only
invoked when o is non-null.
plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageMonBase.java (1)

142-145: 防御性空值检查与代码一致性问题

此处添加 self == null 检查是合理的防御性编程,可防止 getName() 方法抛出 NPE。但需要注意,构造函数(第 546 行)中 syncId 的初始化同样直接调用了 self.getUuid()

syncId = String.format("ceph-primary-storage-mon-%s", self.getUuid());

如果 self 在运行时确实可能为 null,构造函数会先于此处抛出异常。建议确认 selfnull 的实际场景,若仅为防御性措施则可保留现状。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageMonBase.java`
around lines 142 - 145, The getName() null-check for self is defensive but
inconsistent with the constructor where syncId is initialized using
self.getUuid() (in CephPrimaryStorageMonBase), which would NPE earlier; either
make self null-safe in the constructor the same way (set syncId =
String.format("ceph-primary-storage-mon-%s", self == null ? "unknown" :
self.getUuid())) or remove the getName() defensive branch and enforce/validate
that self is non-null at object construction (throw IllegalArgumentException or
assert) so both getName() and syncId initialization are consistent; update the
constructor and/or add an explicit null-check for self in the
CephPrimaryStorageMonBase constructor to reflect the chosen approach.
header/src/main/java/org/zstack/header/zwatch/ResourceMetricBindingExtensionPoint.java (2)

34-76: 建议在 setter 中进行校验,而非延迟到 getter 抛出异常。

当前设计中,setter 不做校验,getter 在值为 null/空时抛出 IllegalStateException。这种"延迟失败"模式会使调试更困难——问题在 setter 调用时产生,却在 getter 调用时才暴露。建议将校验移至 setter,或提供一个显式的 validate() 方法在对象构建完成后调用。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@header/src/main/java/org/zstack/header/zwatch/ResourceMetricBindingExtensionPoint.java`
around lines 34 - 76, The setters (e.g., setLogicalMetricName,
setSourceNamespace, setSourceMetricName, setResourceField, setSourceLabel,
setResourceType) currently allow null/empty values while their getters call
requireText and throw at access time; move the validation to setters (or add an
explicit validate() method to call after construction) so failures surface at
assignment: in each setter call the existing requireText (or equivalent check)
and throw IllegalArgumentException/IllegalStateException immediately when input
is null/empty, or implement a validate() method that runs requireText for all
fields and document that callers must invoke it after building the object.

22-28: requireText 检查 trim 后是否为空,但返回的是未 trim 的原值,存在不一致。

当前逻辑在第 24 行检查 value.trim().isEmpty(),但第 27 行返回的是原始未 trim 的 value。这意味着 " test " 会通过校验并返回带空格的字符串。根据编码规范,用户输入可能带有空格、换行符等,建议统一返回 trim 后的值。

建议修改
     private static String requireText(String fieldName, String value) {
         requireValue(fieldName, value);
-        if (value.trim().isEmpty()) {
+        String trimmed = value.trim();
+        if (trimmed.isEmpty()) {
             throw new IllegalStateException(String.format("ResourceMetricBinding.%s must not be empty", fieldName));
         }
-        return value;
+        return trimmed;
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@header/src/main/java/org/zstack/header/zwatch/ResourceMetricBindingExtensionPoint.java`
around lines 22 - 28, The requireText method currently trims only for the
emptiness check but returns the original untrimmed value; change requireText in
ResourceMetricBindingExtensionPoint to trim the input once (e.g., assign value =
value.trim() after null check or use a local trimmed variable), use that trimmed
value for the isEmpty() check and return the trimmed value to ensure consistent
behavior for inputs like "  test  ".
compute/src/main/java/org/zstack/compute/vm/VmNicManagerImpl.java (1)

56-64: 空指针保护逻辑正确,但需注意执行顺序

空指针检查本身没有问题,可以有效防止后续 nic.getUsedIpUuid() 的 NPE。

但请注意:第 58 行在查询 NIC 之前就已经更新了 UsedIpVO.vmNicUuid。如果 NIC 不存在,UsedIpVO 将持有一个指向不存在 NIC 的引用,可能导致数据不一致。建议确认这是否符合预期设计,或者考虑调整执行顺序。

可选:调整执行顺序的参考方案
 `@Override`
 public void afterAddIpAddress(String vmNicUUid, String usedIpUuid) {
-    /* update UsedIpVO */
-    SQL.New(UsedIpVO.class).eq(UsedIpVO_.uuid, usedIpUuid).set(UsedIpVO_.vmNicUuid, vmNicUUid).update();
-
     VmNicVO nic = Q.New(VmNicVO.class).eq(VmNicVO_.uuid, vmNicUUid).find();
     if (nic == null) {
         logger.debug(String.format("VmNic[uuid:%s] not found, skip afterAddIpAddress", vmNicUUid));
         return;
     }
+
+    /* update UsedIpVO */
+    SQL.New(UsedIpVO.class).eq(UsedIpVO_.uuid, usedIpUuid).set(UsedIpVO_.vmNicUuid, vmNicUUid).update();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@compute/src/main/java/org/zstack/compute/vm/VmNicManagerImpl.java` around
lines 56 - 64, The current afterAddIpAddress in VmNicManagerImpl updates
UsedIpVO (SQL.New(UsedIpVO.class)...set(UsedIpVO_.vmNicUuid,
vmNicUUid).update()) before verifying the NIC exists via
Q.New(VmNicVO.class).eq(VmNicVO_.uuid, vmNicUUid).find(), which can leave
UsedIpVO pointing to a non-existent NIC; change the order so you first query
VmNicVO (using Q.New(...).find()), perform the null check on nic (and return if
null), and only then execute the UsedIpVO update, or alternatively add
compensating logic to rollback/clear UsedIpVO.vmNicUuid if the nic is missing to
avoid stale foreign references.
conf/db/upgrade/V5.5.12__schema.sql (5)

154-155: TIMESTAMP 列缺少 DEFAULT 值。

与其他表相同,建议为 createDatelastOpDate 添加显式 DEFAULT CURRENT_TIMESTAMP

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@conf/db/upgrade/V5.5.12__schema.sql` around lines 154 - 155, 表定义中 createDate
和 lastOpDate 两个 TIMESTAMP 列缺少显式默认值;请在 V5.5.12__schema.sql 中对应表的列定义里为
`createDate` 和 `lastOpDate` 添加 `DEFAULT CURRENT_TIMESTAMP`(或根据需要为 `lastOpDate`
使用 `DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP`),以与其他表保持一致并保证自动填充时间戳。

42-43: 建议为 TIMESTAMP 列添加显式 DEFAULT。

createDatelastOpDateNOT NULL 但没有显式 DEFAULT 值。在 MySQL 严格模式下,第二个 TIMESTAMP 列可能获得不兼容的默认值(如 0000-00-00 00:00:00),导致插入失败。根据编码规范,应使用 DEFAULT CURRENT_TIMESTAMP

建议的修复
-    `createDate`    TIMESTAMP       NOT NULL,
-    `lastOpDate`    TIMESTAMP       NOT NULL,
+    `createDate`    TIMESTAMP       NOT NULL DEFAULT CURRENT_TIMESTAMP,
+    `lastOpDate`    TIMESTAMP       NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@conf/db/upgrade/V5.5.12__schema.sql` around lines 42 - 43, The TIMESTAMP
columns createDate and lastOpDate are NOT NULL but lack explicit defaults which
can cause invalid default values in strict MySQL modes; update the column
definitions in V5.5.12__schema.sql so createDate has DEFAULT CURRENT_TIMESTAMP
and lastOpDate has DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP (or at
minimum DEFAULT CURRENT_TIMESTAMP) to ensure valid defaults and proper automatic
update behavior.

180-181: TIMESTAMP 列缺少 DEFAULT 值。

与其他新建表相同,建议为 createDatelastOpDate 添加显式 DEFAULT CURRENT_TIMESTAMP

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@conf/db/upgrade/V5.5.12__schema.sql` around lines 180 - 181, 在表定义中,createDate
和 lastOpDate 列缺少默认值;请在 V5.5.12 的建表语句中为 `createDate` 和 `lastOpDate` 两个 TIMESTAMP
列显式添加 DEFAULT CURRENT_TIMESTAMP(并根据其他表的约定可选地为 `lastOpDate` 添加 ON UPDATE
CURRENT_TIMESTAMP),以使这两个字段在插入/更新时有一致的默认行为。

132-133: TIMESTAMP 列缺少 DEFAULT 值。

PciDeviceVirtCapabilityVO 相同的问题,建议为 createDatelastOpDate 添加 DEFAULT CURRENT_TIMESTAMP

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@conf/db/upgrade/V5.5.12__schema.sql` around lines 132 - 133, 在该迁移脚本中,表定义里的
`createDate` 和 `lastOpDate` 列缺少默认值;请将这两列的定义更新为带有默认时间戳(与
PciDeviceVirtCapabilityVO 保持一致),即为 `createDate` 和 `lastOpDate` 添加 `DEFAULT
CURRENT_TIMESTAMP`(如业务需要可对 `lastOpDate` 同时加入 `ON UPDATE
CURRENT_TIMESTAMP`),修改对应的 CREATE TABLE 或 ALTER TABLE 语句中的列定义以保证新行自动填充时间戳。

90-104: 建议添加 virtMode IS NULL 条件以提高幂等性。

虽然当前逻辑通过 ELSE virtMode 保留了现有值,但每次迁移脚本执行时仍会对所有匹配行执行 UPDATE。添加 AND virtMode IS NULL 可以避免不必要的数据库操作。

建议的修复
 WHERE `virtStatus` IN (
     'SRIOV_VIRTUALIZED', 'SRIOV_VIRTUAL',
     'VFIO_MDEV_VIRTUALIZED', 'VIRTUALIZED_BYPASS_ZSTACK',
     'TENSORFUSION_VIRTUALIZED', 'HAMI_VIRTUALIZED'
-)
+)
+  AND `virtMode` IS NULL;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@conf/db/upgrade/V5.5.12__schema.sql` around lines 90 - 104, The UPDATE on
table PciDeviceVO that sets virtMode based on virtStatus should be made
idempotent by adding a check for virtMode IS NULL in the WHERE clause; modify
the UPDATE (the statement updating `PciDeviceVO` and columns `virtMode` /
`virtStatus`) to include AND `virtMode` IS NULL so only rows without a set
virtMode are updated and unnecessary writes are avoided while keeping the
CASE/ELSE logic intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@compute/src/main/java/org/zstack/compute/allocator/HostAllocatorChain.java`:
- Around line 149-152: The current exception path in HostAllocatorChain swallows
errors when only dryRunCompletion is set because the code only calls
completion.fail; update the error handling to also invoke dryRunCompletion.fail
when completion is null and dryRunCompletion is non-null (use the same
inerr(ORG_ZSTACK_COMPUTE_ALLOCATOR_10019, errMsg) payload), ensuring both
dryRun() and normal completion paths receive the failure; locate the block
around the t/errMsg handling in HostAllocatorChain and add the conditional
dryRunCompletion.fail call (referencing dryRunCompletion, completion, inerr and
ORG_ZSTACK_COMPUTE_ALLOCATOR_10019).

In `@conf/db/upgrade/V5.5.12__schema.sql`:
- Around line 117-123: The UPDATE on GpuDeviceVO/PciDeviceVO is missing a WHERE
clause and will overwrite allocateStatus on every migration run; restrict the
update to only rows that haven't been initialized by adding a WHERE
g.`allocateStatus` IS NULL (or g.`allocateStatus` = '' depending on existing
conventions) so the CASE only sets values for uninitialized records; reference
the GpuDeviceVO and PciDeviceVO tables and the columns g.`allocateStatus`,
p.`vmInstanceUuid`, p.`virtState`, p.`virtMode` when applying this change.

In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmBackend.java`:
- Around line 3800-3801: The failure branch inside
initializeHostAttachedPSMountPath() currently calls
errorCode.getCause().getDetails() without null-check and can NPE when cause is
null; change that branch to mirror the earlier fix by computing String
causeDetails = errorCode.getCause() != null ? errorCode.getCause().getDetails()
: errorCode.getDetails() and use causeDetails in the
completion.fail()/operr(...) call (same pattern as the other fix), replacing the
direct errorCode.getCause().getDetails() usage to prevent NPE.

---

Duplicate comments:
In
`@header/src/main/java/org/zstack/header/zwatch/ResourceMetricBindingExtensionPoint.java`:
- Line 87: 为 ResourceMetricBindingExtensionPoint 接口的 getResourceMetricBindings()
方法补充 Javadoc:说明该方法返回的语义(返回一个包含 ResourceMetricBinding
的列表,代表该扩展点注册或支持的资源指标绑定),明确是否允许返回 null 或空列表(例如:不应返回
null,空列表表示无绑定),以及调用方应如何处理返回值;在注释中引用返回类型 ResourceMetricBinding 和 接口名
ResourceMetricBindingExtensionPoint 以便定位。

---

Nitpick comments:
In `@compute/src/main/java/org/zstack/compute/vm/VmNicManagerImpl.java`:
- Around line 56-64: The current afterAddIpAddress in VmNicManagerImpl updates
UsedIpVO (SQL.New(UsedIpVO.class)...set(UsedIpVO_.vmNicUuid,
vmNicUUid).update()) before verifying the NIC exists via
Q.New(VmNicVO.class).eq(VmNicVO_.uuid, vmNicUUid).find(), which can leave
UsedIpVO pointing to a non-existent NIC; change the order so you first query
VmNicVO (using Q.New(...).find()), perform the null check on nic (and return if
null), and only then execute the UsedIpVO update, or alternatively add
compensating logic to rollback/clear UsedIpVO.vmNicUuid if the nic is missing to
avoid stale foreign references.

In `@conf/db/upgrade/V5.5.12__schema.sql`:
- Around line 154-155: 表定义中 createDate 和 lastOpDate 两个 TIMESTAMP 列缺少显式默认值;请在
V5.5.12__schema.sql 中对应表的列定义里为 `createDate` 和 `lastOpDate` 添加 `DEFAULT
CURRENT_TIMESTAMP`(或根据需要为 `lastOpDate` 使用 `DEFAULT CURRENT_TIMESTAMP ON UPDATE
CURRENT_TIMESTAMP`),以与其他表保持一致并保证自动填充时间戳。
- Around line 42-43: The TIMESTAMP columns createDate and lastOpDate are NOT
NULL but lack explicit defaults which can cause invalid default values in strict
MySQL modes; update the column definitions in V5.5.12__schema.sql so createDate
has DEFAULT CURRENT_TIMESTAMP and lastOpDate has DEFAULT CURRENT_TIMESTAMP ON
UPDATE CURRENT_TIMESTAMP (or at minimum DEFAULT CURRENT_TIMESTAMP) to ensure
valid defaults and proper automatic update behavior.
- Around line 180-181: 在表定义中,createDate 和 lastOpDate 列缺少默认值;请在 V5.5.12 的建表语句中为
`createDate` 和 `lastOpDate` 两个 TIMESTAMP 列显式添加 DEFAULT
CURRENT_TIMESTAMP(并根据其他表的约定可选地为 `lastOpDate` 添加 ON UPDATE
CURRENT_TIMESTAMP),以使这两个字段在插入/更新时有一致的默认行为。
- Around line 132-133: 在该迁移脚本中,表定义里的 `createDate` 和 `lastOpDate`
列缺少默认值;请将这两列的定义更新为带有默认时间戳(与 PciDeviceVirtCapabilityVO 保持一致),即为 `createDate` 和
`lastOpDate` 添加 `DEFAULT CURRENT_TIMESTAMP`(如业务需要可对 `lastOpDate` 同时加入 `ON UPDATE
CURRENT_TIMESTAMP`),修改对应的 CREATE TABLE 或 ALTER TABLE 语句中的列定义以保证新行自动填充时间戳。
- Around line 90-104: The UPDATE on table PciDeviceVO that sets virtMode based
on virtStatus should be made idempotent by adding a check for virtMode IS NULL
in the WHERE clause; modify the UPDATE (the statement updating `PciDeviceVO` and
columns `virtMode` / `virtStatus`) to include AND `virtMode` IS NULL so only
rows without a set virtMode are updated and unnecessary writes are avoided while
keeping the CASE/ELSE logic intact.

In `@core/src/main/java/org/zstack/core/log/LogSafeGson.java`:
- Around line 217-219: The toJson method can throw an NPE because it calls
o.getClass() when o is null; update the toJson(Object o) implementation to first
check for null and handle it (e.g., return the JSON null representation by
calling logSafeGson.toJson(null) or returning "null") before calling
getGsonType(o.getClass()), ensuring getGsonType and o.getClass() are only
invoked when o is non-null.

In
`@header/src/main/java/org/zstack/header/zwatch/ResourceMetricBindingExtensionPoint.java`:
- Around line 34-76: The setters (e.g., setLogicalMetricName,
setSourceNamespace, setSourceMetricName, setResourceField, setSourceLabel,
setResourceType) currently allow null/empty values while their getters call
requireText and throw at access time; move the validation to setters (or add an
explicit validate() method to call after construction) so failures surface at
assignment: in each setter call the existing requireText (or equivalent check)
and throw IllegalArgumentException/IllegalStateException immediately when input
is null/empty, or implement a validate() method that runs requireText for all
fields and document that callers must invoke it after building the object.
- Around line 22-28: The requireText method currently trims only for the
emptiness check but returns the original untrimmed value; change requireText in
ResourceMetricBindingExtensionPoint to trim the input once (e.g., assign value =
value.trim() after null check or use a local trimmed variable), use that trimmed
value for the isEmpty() check and return the trimmed value to ensure consistent
behavior for inputs like "  test  ".

In
`@plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageMonBase.java`:
- Around line 142-145: The getName() null-check for self is defensive but
inconsistent with the constructor where syncId is initialized using
self.getUuid() (in CephPrimaryStorageMonBase), which would NPE earlier; either
make self null-safe in the constructor the same way (set syncId =
String.format("ceph-primary-storage-mon-%s", self == null ? "unknown" :
self.getUuid())) or remove the getName() defensive branch and enforce/validate
that self is non-null at object construction (throw IllegalArgumentException or
assert) so both getName() and syncId initialization are consistent; update the
constructor and/or add an explicit null-check for self in the
CephPrimaryStorageMonBase constructor to reflect the chosen approach.
🪄 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: b1c02024-e51c-461a-8943-979d9cb5aa9a

📥 Commits

Reviewing files that changed from the base of the PR and between 45a57a0 and 1b22ec2.

⛔ Files ignored due to path filters (34)
  • conf/i18n/globalErrorCodeMapping/global-error-en_US.json is excluded by !**/*.json
  • conf/i18n/globalErrorCodeMapping/global-error-zh_CN.json is excluded by !**/*.json
  • sdk/src/main/java/SourceClassMap.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DGpuDeviceInventory.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DGpuProfileInventory.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DGpuSpecStatsInventory.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DGpuStatus.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DetachDGpuFromVmAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DetachDGpuFromVmResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DisableDGpuModeAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DisableDGpuModeResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/EnableDGpuModeAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/EnableDGpuModeResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/GetDGpuSpecStatsAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/GetDGpuSpecStatsResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/GetGpuDeviceCandidatesAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/GetGpuDeviceCandidatesResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/GetGpuDeviceSpecCandidatesAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/GpuDeviceInventory.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/GpuDeviceSpecCandidateInventory.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/PciDeviceInventory.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/PciDeviceVirtMode.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/PciDeviceVirtState.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/PciDeviceVirtStatus.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/QueryDGpuDeviceAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/QueryDGpuDeviceResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/QueryDGpuProfileAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/QueryDGpuProfileResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/RemoveVmDGpuStrategyAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/RemoveVmDGpuStrategyResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/SetDGpuProfileAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/SetDGpuProfileResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/SetVmDGpuStrategyAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/SetVmDGpuStrategyResult.java is excluded by !sdk/**
📒 Files selected for processing (17)
  • compute/src/main/java/org/zstack/compute/allocator/HostAllocatorChain.java
  • compute/src/main/java/org/zstack/compute/host/HostBase.java
  • compute/src/main/java/org/zstack/compute/vm/VmCapabilitiesJudger.java
  • compute/src/main/java/org/zstack/compute/vm/VmNicManagerImpl.java
  • conf/db/upgrade/V5.5.12__schema.sql
  • core/src/main/java/org/zstack/core/db/UpdateQueryImpl.java
  • core/src/main/java/org/zstack/core/log/LogSafeGson.java
  • header/src/main/java/org/zstack/header/vm/VmAbnormalLifeCycleStruct.java
  • header/src/main/java/org/zstack/header/vm/VmInstanceSpec.java
  • header/src/main/java/org/zstack/header/zwatch/ResourceMetricBindingExtensionPoint.java
  • identity/src/main/java/org/zstack/identity/AuthorizationManager.java
  • plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageBase.java
  • plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageMonBase.java
  • plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageAllocatorFactory.java
  • plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmBackend.java
  • testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy
  • utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
✅ Files skipped from review due to trivial changes (4)
  • core/src/main/java/org/zstack/core/db/UpdateQueryImpl.java
  • utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
  • plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageBase.java
  • testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy

Comment on lines +149 to +152
String errMsg = t != null ? t.toString() : "unknown error";
if (completion != null) {
completion.fail(inerr(ORG_ZSTACK_COMPUTE_ALLOCATOR_10019, errMsg));
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

DryRun 异常路径缺少失败回调,可能导致请求悬挂

Line 150-152 仅在 completion != null 时回调;dryRun() 场景通常只设置 dryRunCompletion,这里会吞掉异常,调用方收不到结果。

🔧 建议修复
         } catch (Throwable t) {
             logger.warn("unhandled throwable", t);
-            String errMsg = t != null ? t.toString() : "unknown error";
-            if (completion != null) {
-                completion.fail(inerr(ORG_ZSTACK_COMPUTE_ALLOCATOR_10019, errMsg));
-            }
+            String errMsg = t != null ? t.toString() : "unknown error";
+            ErrorCode unhandled = inerr(ORG_ZSTACK_COMPUTE_ALLOCATOR_10019, errMsg);
+            if (isDryRun) {
+                if (dryRunCompletion != null) {
+                    dryRunCompletion.fail(unhandled);
+                }
+            } else if (completion != null) {
+                completion.fail(unhandled);
+            }
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@compute/src/main/java/org/zstack/compute/allocator/HostAllocatorChain.java`
around lines 149 - 152, The current exception path in HostAllocatorChain
swallows errors when only dryRunCompletion is set because the code only calls
completion.fail; update the error handling to also invoke dryRunCompletion.fail
when completion is null and dryRunCompletion is non-null (use the same
inerr(ORG_ZSTACK_COMPUTE_ALLOCATOR_10019, errMsg) payload), ensuring both
dryRun() and normal completion paths receive the failure; locate the block
around the t/errMsg handling in HostAllocatorChain and add the conditional
dryRunCompletion.fail call (referencing dryRunCompletion, completion, inerr and
ORG_ZSTACK_COMPUTE_ALLOCATOR_10019).

Comment on lines +117 to +123
UPDATE `zstack`.`GpuDeviceVO` g
INNER JOIN `zstack`.`PciDeviceVO` p ON g.`uuid` = p.`uuid`
SET g.`allocateStatus` = CASE
WHEN p.`vmInstanceUuid` IS NOT NULL THEN 'Allocated'
WHEN p.`virtState` = 'VIRTUALIZED' AND p.`virtMode` IS NOT NULL THEN 'Unallocatable'
ELSE 'Unallocated'
END;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

缺少 WHERE 条件,影响升级幂等性。

与上面的 mode 更新类似,此 UPDATE 语句没有 WHERE 条件,每次迁移都会覆盖 allocateStatus。如果应用程序运行时已修改过此值,迁移重跑会导致数据回退。

建议修改
 UPDATE `zstack`.`GpuDeviceVO` g
 INNER JOIN `zstack`.`PciDeviceVO` p ON g.`uuid` = p.`uuid`
 SET g.`allocateStatus` = CASE
     WHEN p.`vmInstanceUuid` IS NOT NULL THEN 'Allocated'
     WHEN p.`virtState` = 'VIRTUALIZED' AND p.`virtMode` IS NOT NULL THEN 'Unallocatable'
     ELSE 'Unallocated'
-END;
+END
+WHERE g.`allocateStatus` IS NULL;
📝 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
UPDATE `zstack`.`GpuDeviceVO` g
INNER JOIN `zstack`.`PciDeviceVO` p ON g.`uuid` = p.`uuid`
SET g.`allocateStatus` = CASE
WHEN p.`vmInstanceUuid` IS NOT NULL THEN 'Allocated'
WHEN p.`virtState` = 'VIRTUALIZED' AND p.`virtMode` IS NOT NULL THEN 'Unallocatable'
ELSE 'Unallocated'
END;
UPDATE `zstack`.`GpuDeviceVO` g
INNER JOIN `zstack`.`PciDeviceVO` p ON g.`uuid` = p.`uuid`
SET g.`allocateStatus` = CASE
WHEN p.`vmInstanceUuid` IS NOT NULL THEN 'Allocated'
WHEN p.`virtState` = 'VIRTUALIZED' AND p.`virtMode` IS NOT NULL THEN 'Unallocatable'
ELSE 'Unallocated'
END
WHERE g.`allocateStatus` IS NULL;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@conf/db/upgrade/V5.5.12__schema.sql` around lines 117 - 123, The UPDATE on
GpuDeviceVO/PciDeviceVO is missing a WHERE clause and will overwrite
allocateStatus on every migration run; restrict the update to only rows that
haven't been initialized by adding a WHERE g.`allocateStatus` IS NULL (or
g.`allocateStatus` = '' depending on existing conventions) so the CASE only sets
values for uninitialized records; reference the GpuDeviceVO and PciDeviceVO
tables and the columns g.`allocateStatus`, p.`vmInstanceUuid`, p.`virtState`,
p.`virtMode` when applying this change.

Comment on lines +3800 to +3801
String causeDetails = errorCode.getCause() != null ? errorCode.getCause().getDetails() : errorCode.getDetails();
completion.fail(operr(ORG_ZSTACK_STORAGE_PRIMARY_LOCAL_10081, "cannot find flag file [%s] on host [%s], because: %s", makeInitializedFilePath(), hostUuid, causeDetails));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

空指针防护修复不完整,另一路失败分支仍可能 NPE

这里的 cause 兜底是正确的;但同类逻辑在 initializeHostAttachedPSMountPath() 的失败分支(Line 3822)仍直接 errorCode.getCause().getDetails()cause == null 时会再次抛 NPE 并掩盖原始错误。

🔧 建议同步修复(同文件另一处)
             `@Override`
             public void fail(ErrorCode errorCode) {
-                completion.fail(operr(ORG_ZSTACK_STORAGE_PRIMARY_LOCAL_10082, "cannot create flag file [%s] on host [%s], because: %s", makeInitializedFilePath(), hostUuid, errorCode.getCause().getDetails()));
+                String causeDetails = errorCode.getCause() != null
+                        ? errorCode.getCause().getDetails()
+                        : errorCode.getDetails();
+                completion.fail(operr(ORG_ZSTACK_STORAGE_PRIMARY_LOCAL_10082,
+                        "cannot create flag file [%s] on host [%s], because: %s",
+                        makeInitializedFilePath(), hostUuid, causeDetails));
             }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmBackend.java`
around lines 3800 - 3801, The failure branch inside
initializeHostAttachedPSMountPath() currently calls
errorCode.getCause().getDetails() without null-check and can NPE when cause is
null; change that branch to mirror the earlier fix by computing String
causeDetails = errorCode.getCause() != null ? errorCode.getCause().getDetails()
: errorCode.getDetails() and use causeDetails in the
completion.fail()/operr(...) call (same pattern as the other fix), replacing the
direct errorCode.getCause().getDetails() usage to prevent NPE.

@MatheMatrix MatheMatrix force-pushed the sync/xinhao.huang/5.5.12_dongzhu-dgpu@@2 branch from 1b22ec2 to 6b186e8 Compare April 7, 2026 04:31
Copy link
Copy Markdown

@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.

♻️ Duplicate comments (1)
header/src/main/java/org/zstack/header/zwatch/ResourceMetricBindingExtensionPoint.java (1)

87-87: ⚠️ Potential issue | 🟡 Minor

请在 Line 87 为接口方法补充 Javadoc 合同说明。

getResourceMetricBindings() 目前缺少接口级文档,调用方无法明确返回值是否允许 null、空列表语义、以及 ResourceMetricBinding 必填字段约束。这个点在历史评审中已提过,建议本次一并补齐。

建议修改
+    /**
+     * Returns metric bindings provided by this extension.
+     * The returned list must not be {`@code` null}; return an empty list when no binding is provided.
+     * Each {`@link` ResourceMetricBinding} entry must provide required fields before consumption.
+     *
+     * `@return` non-null metric binding list
+     */
     List<ResourceMetricBinding> getResourceMetricBindings();

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

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@header/src/main/java/org/zstack/header/zwatch/ResourceMetricBindingExtensionPoint.java`
at line 87, 为接口 ResourceMetricBindingExtensionPoint 的方法
getResourceMetricBindings 添加 Javadoc 合同:说明方法绝不返回
null(若无绑定应返回空列表)、调用方可安全遍历返回值;明确每个 ResourceMetricBinding
实例的必填字段约束(例如必须包含资源标识、metric 名称和采样周期等不可为 null 的属性,参见 ResourceMetricBinding
的字段定义),并约定是否允许列表中包含重复或排序要求;在注释中给出异常/错误情形的处理约定(若有)。
🧹 Nitpick comments (1)
conf/db/upgrade/V5.5.12__schema.sql (1)

42-43: 建议为 TIMESTAMP 列添加 DEFAULT CURRENT_TIMESTAMP

当前 createDatelastOpDate 列声明为 NOT NULL 但未指定默认值。虽然后续 INSERT 语句显式使用 NOW(),但为了防止未来插入遗漏和符合编码规范,建议添加默认值。

建议修改
-    `createDate`    TIMESTAMP       NOT NULL,
-    `lastOpDate`    TIMESTAMP       NOT NULL,
+    `createDate`    TIMESTAMP       NOT NULL DEFAULT CURRENT_TIMESTAMP,
+    `lastOpDate`    TIMESTAMP       NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@conf/db/upgrade/V5.5.12__schema.sql` around lines 42 - 43, The TIMESTAMP
columns createDate and lastOpDate are NOT NULL but lack defaults; update their
column definitions (the `createDate` and `lastOpDate` declarations) to include
DEFAULT CURRENT_TIMESTAMP so new rows get automatic timestamps (and optionally
add ON UPDATE CURRENT_TIMESTAMP to lastOpDate if you want it updated on row
changes).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In
`@header/src/main/java/org/zstack/header/zwatch/ResourceMetricBindingExtensionPoint.java`:
- Line 87: 为接口 ResourceMetricBindingExtensionPoint 的方法 getResourceMetricBindings
添加 Javadoc 合同:说明方法绝不返回 null(若无绑定应返回空列表)、调用方可安全遍历返回值;明确每个 ResourceMetricBinding
实例的必填字段约束(例如必须包含资源标识、metric 名称和采样周期等不可为 null 的属性,参见 ResourceMetricBinding
的字段定义),并约定是否允许列表中包含重复或排序要求;在注释中给出异常/错误情形的处理约定(若有)。

---

Nitpick comments:
In `@conf/db/upgrade/V5.5.12__schema.sql`:
- Around line 42-43: The TIMESTAMP columns createDate and lastOpDate are NOT
NULL but lack defaults; update their column definitions (the `createDate` and
`lastOpDate` declarations) to include DEFAULT CURRENT_TIMESTAMP so new rows get
automatic timestamps (and optionally add ON UPDATE CURRENT_TIMESTAMP to
lastOpDate if you want it updated on row changes).

ℹ️ 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: d499b856-ad4c-4f3f-a7b2-79ee7b260a9f

📥 Commits

Reviewing files that changed from the base of the PR and between 1b22ec2 and 6b186e8.

⛔ Files ignored due to path filters (35)
  • conf/i18n/globalErrorCodeMapping/global-error-en_US.json is excluded by !**/*.json
  • conf/i18n/globalErrorCodeMapping/global-error-zh_CN.json is excluded by !**/*.json
  • conf/springConfigXml/DatabaseFacade.xml is excluded by !**/*.xml
  • sdk/src/main/java/SourceClassMap.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DGpuDeviceInventory.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DGpuProfileInventory.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DGpuSpecStatsInventory.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DGpuStatus.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DetachDGpuFromVmAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DetachDGpuFromVmResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DisableDGpuModeAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/DisableDGpuModeResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/EnableDGpuModeAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/EnableDGpuModeResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/GetDGpuSpecStatsAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/GetDGpuSpecStatsResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/GetGpuDeviceCandidatesAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/GetGpuDeviceCandidatesResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/GetGpuDeviceSpecCandidatesAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/GpuDeviceInventory.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/GpuDeviceSpecCandidateInventory.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/PciDeviceInventory.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/PciDeviceVirtMode.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/PciDeviceVirtState.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/PciDeviceVirtStatus.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/QueryDGpuDeviceAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/QueryDGpuDeviceResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/QueryDGpuProfileAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/QueryDGpuProfileResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/RemoveVmDGpuStrategyAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/RemoveVmDGpuStrategyResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/SetDGpuProfileAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/SetDGpuProfileResult.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/SetVmDGpuStrategyAction.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/SetVmDGpuStrategyResult.java is excluded by !sdk/**
📒 Files selected for processing (4)
  • conf/db/upgrade/V5.5.12__schema.sql
  • header/src/main/java/org/zstack/header/zwatch/ResourceMetricBindingExtensionPoint.java
  • testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy
  • utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
✅ Files skipped from review due to trivial changes (1)
  • utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java

DBImpact

Resolves: ZSTAC-83477

Change-Id: I61746e646972787a6f72757565776d6570617768
@MatheMatrix MatheMatrix force-pushed the sync/xinhao.huang/5.5.12_dongzhu-dgpu@@2 branch from 92da9b0 to f5f0d25 Compare April 7, 2026 07:25
@MatheMatrix MatheMatrix closed this Apr 7, 2026
@zstack-robot-2 zstack-robot-2 deleted the sync/xinhao.huang/5.5.12_dongzhu-dgpu@@2 branch April 7, 2026 12:19
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