<feature>[sdk]: support dgpu#3472
Open
MatheMatrix wants to merge 1 commit intofeature-5.5.12-dgpufrom
Open
Conversation
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml) Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (21)
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthrough新增三张 dGPU/TensorFusion 相关数据库表；新增资源度量绑定扩展点接口；在测试库中添加大量 API 辅助方法；扩展错误码常量以包含更多 dGPU 标识。 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.41.1)utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.javaComment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
header/src/main/java/org/zstack/header/zwatch/ResourceMetricBindingExtensionPoint.java (1)
5-5: 接口及方法缺少 Javadoc 注释根据编码规范，接口方法必须配有有效的 Javadoc 注释。当前接口和
getResourceMetricBindings()方法均缺少文档说明，建议补充以便其他开发者理解此扩展点的用途和使用方式。📝 建议添加 Javadoc 注释
package org.zstack.header.zwatch; import java.util.List; +/** + * Extension point for registering resource metric bindings. + * Implementations provide mappings between logical metrics and their sources. + */ public interface ResourceMetricBindingExtensionPoint { class ResourceMetricBinding { // ... fields and methods ... } + /** + * Returns the list of resource metric bindings provided by this extension. + * + * `@return` list of ResourceMetricBinding configurations + */ List<ResourceMetricBinding> getResourceMetricBindings(); }As per coding guidelines: "接口方法不应有多余的修饰符（例如
public），且必须配有有效的 Javadoc 注释。"Also applies to: 72-72
🤖 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 5, Add Javadoc to the ResourceMetricBindingExtensionPoint interface and its getResourceMetricBindings() method describing the purpose, expected behavior, return value and usage examples; also remove any redundant access modifier from the interface method (do not declare it as public) so the method uses the implicit interface visibility. Locate the ResourceMetricBindingExtensionPoint declaration and the getResourceMetricBindings() method to add concise JavaDoc blocks that follow project conventions.conf/db/upgrade/V5.5.12__schema.sql (1)
86-86: 请确认gpuDeviceUuid的语义并对齐外键目标。列名看起来像 dGPU 设备 UUID，但这里的外键指向的是
PciDeviceVO。如果这里实际保存的是 dGPU 设备 UUID，当前外键不会保护到DGpuDeviceVO的引用完整性；如果这里保存的是物理 GPU UUID，建议把列名改成parentGpuUuid或pciDeviceUuid，避免后续代码按名字误用。Also applies to: 98-99
🤖 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` at line 86, 确认列 gpuDeviceUuid 的语义并与外键目标对齐：如果该列保存的是 dGPU 设备 UUID，则应改为引用 DGpuDeviceVO 或把外键指向 DGpuDeviceVO；如果保存的是物理 PCI 设备 UUID，则将列名改为 parentGpuUuid 或 pciDeviceUuid 并保持外键指向 PciDeviceVO；更新 V5.5.12__schema.sql 中相关列定义和外键约束（包括文件中另外提到的第98-99行）以反映最终语义，并同时调整任何依赖该列名的后续代码或注释以避免歧义。
🤖 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 37-48: 当前的 CREATE TABLE/REFERENCES 语句未显式限定
schema，导致在非默认库上下文执行时建表或外键到错误 schema；请在涉及的表定义（例如 DGpuProfileVO 的 CREATE
TABLE）和外键约束（例如 CONSTRAINT fk_dgpu_profile_spec REFERENCES
GpuDeviceSpecVO(`uuid`)）中把表名都改为带 schema 的形式，如
zstack.`DGpuProfileVO`、zstack.`GpuDeviceSpecVO`（同理修正其他两个 CREATE TABLE/REFERENCES
段及脚本中引用的表名，例如 zstack.`UsedIpVO`），确保所有 CREATE TABLE、UNIQUE KEY、FOREIGN KEY
REFERENCES 都显式使用 zstack schema。
- Around line 84-85: Change the capacity columns to unsigned to prevent negative
values and match other tables: update the column definitions for `memorySize`
and `shmemSizeMb` from `BIGINT` to `BIGINT UNSIGNED` (keeping the NOT NULL and
the default 4096 on `shmemSizeMb`), and ensure any related constraints or
inserts expecting signed values are adjusted to use non-negative values as
needed.
---
Nitpick comments:
In `@conf/db/upgrade/V5.5.12__schema.sql`:
- Line 86: 确认列 gpuDeviceUuid 的语义并与外键目标对齐：如果该列保存的是 dGPU 设备 UUID，则应改为引用
DGpuDeviceVO 或把外键指向 DGpuDeviceVO；如果保存的是物理 PCI 设备 UUID，则将列名改为 parentGpuUuid 或
pciDeviceUuid 并保持外键指向 PciDeviceVO；更新 V5.5.12__schema.sql
中相关列定义和外键约束（包括文件中另外提到的第98-99行）以反映最终语义，并同时调整任何依赖该列名的后续代码或注释以避免歧义。
In
`@header/src/main/java/org/zstack/header/zwatch/ResourceMetricBindingExtensionPoint.java`:
- Line 5: Add Javadoc to the ResourceMetricBindingExtensionPoint interface and
its getResourceMetricBindings() method describing the purpose, expected
behavior, return value and usage examples; also remove any redundant access
modifier from the interface method (do not declare it as public) so the method
uses the implicit interface visibility. Locate the
ResourceMetricBindingExtensionPoint declaration and the
getResourceMetricBindings() method to add concise JavaDoc blocks that follow
project conventions.
ℹ️ 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: 4c6b0a2e-b4ac-4896-8c5e-fd962ca8f65f
⛔ Files ignored due to path filters (21)
sdk/src/main/java/SourceClassMap.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DGpuDeviceInventory.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DGpuProfileInventory.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DGpuStatus.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DetachDGpuFromVmAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DetachDGpuFromVmResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DisableDGpuModeAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DisableDGpuModeResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/EnableDGpuModeAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/EnableDGpuModeResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/PciDeviceVirtStatus.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/QueryDGpuDeviceAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/QueryDGpuDeviceResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/QueryDGpuProfileAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/QueryDGpuProfileResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/RemoveVmDGpuStrategyAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/RemoveVmDGpuStrategyResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/SetDGpuProfileAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/SetDGpuProfileResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/SetVmDGpuStrategyAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/SetVmDGpuStrategyResult.javais excluded by!sdk/**
📒 Files selected for processing (4)
conf/db/upgrade/V5.5.12__schema.sqlheader/src/main/java/org/zstack/header/zwatch/ResourceMetricBindingExtensionPoint.javatestlib/src/main/java/org/zstack/testlib/ApiHelper.groovyutils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
conf/db/upgrade/V5.5.12__schema.sql
Outdated
Comment on lines
+84
to
+85
| `memorySize` BIGINT NOT NULL, | ||
| `shmemSizeMb` BIGINT NOT NULL DEFAULT 4096, |
There was a problem hiding this comment.
容量字段应改成无符号类型。
memorySize 和 shmemSizeMb 表示容量，这里定义成有符号 BIGINT 会允许负数入库，而且和前两张表的 BIGINT UNSIGNED 不一致。
建议改法
- `memorySize` BIGINT NOT NULL,
- `shmemSizeMb` BIGINT NOT NULL DEFAULT 4096,
+ `memorySize` BIGINT UNSIGNED NOT NULL,
+ `shmemSizeMb` BIGINT UNSIGNED NOT NULL DEFAULT 4096,🤖 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 84 - 85, Change the
capacity columns to unsigned to prevent negative values and match other tables:
update the column definitions for `memorySize` and `shmemSizeMb` from `BIGINT`
to `BIGINT UNSIGNED` (keeping the NOT NULL and the default 4096 on
`shmemSizeMb`), and ensure any related constraints or inserts expecting signed
values are adjusted to use non-negative values as needed.
MatheMatrix
force-pushed
the
sync/xinhao.huang/feature-5.5.12-dgpu@@2
branch
from
456b047 to
0b3af1f
Compare
Owner
Author
|
Comment from yaohua.wu: Review: MR !9332 — ZSTAC-82677 (zstack)Feature: 支持 dGPU — SDK/API 定义 + DB Schema + 监控扩展点 Warning
Suggestion
Verdict: APPROVEDDB schema 设计合理（3 张表职责清晰：Profile 模板、Device 实例、Strategy 策略），外键和索引配置正确。ResourceMetricBindingExtensionPoint 提供了良好的监控扩展机制。以上 Warning 建议修复后合并。 🤖 Robot Reviewer |
MatheMatrix
force-pushed
the
sync/xinhao.huang/feature-5.5.12-dgpu@@2
branch
2 times, most recently
from
9f27712 to
1a326c8
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
conf/db/upgrade/V5.5.12__schema.sql (1)
42-43: TIMESTAMP 列缺少 DEFAULT 值。
createDate和lastOpDate定义为TIMESTAMP NOT NULL但未指定 DEFAULT。在某些 MySQL 模式（如启用NO_ZERO_DATE）下，插入时若未显式赋值可能导致错误。建议显式指定默认值。建议修改
- `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,三张表的
createDate和lastOpDate均需类似调整。Also applies to: 64-65, 89-90
🤖 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 declared NOT NULL without defaults; update each table definition (for the occurrences around the shown diffs — the three tables referenced at lines ~42-43, ~64-65, ~89-90) so that createDate has DEFAULT CURRENT_TIMESTAMP and lastOpDate has DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP (or at minimum DEFAULT CURRENT_TIMESTAMP) to avoid NO_ZERO_DATE insert errors and ensure lastOpDate updates automatically; modify the column definitions for createDate and lastOpDate accordingly in each affected CREATE TABLE statement.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@conf/db/upgrade/V5.5.12__schema.sql`:
- Around line 42-43: The TIMESTAMP columns createDate and lastOpDate are
declared NOT NULL without defaults; update each table definition (for the
occurrences around the shown diffs — the three tables referenced at lines
~42-43, ~64-65, ~89-90) so that createDate has DEFAULT CURRENT_TIMESTAMP and
lastOpDate has DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP (or at
minimum DEFAULT CURRENT_TIMESTAMP) to avoid NO_ZERO_DATE insert errors and
ensure lastOpDate updates automatically; modify the column definitions for
createDate and lastOpDate accordingly in each affected CREATE TABLE statement.
ℹ️ 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: e7ab2f82-a51a-46bb-9b52-8b31857b6fbc
⛔ Files ignored due to path filters (21)
sdk/src/main/java/SourceClassMap.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DGpuDeviceInventory.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DGpuProfileInventory.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DGpuStatus.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DetachDGpuFromVmAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DetachDGpuFromVmResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DisableDGpuModeAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DisableDGpuModeResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/EnableDGpuModeAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/EnableDGpuModeResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/PciDeviceVirtStatus.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/QueryDGpuDeviceAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/QueryDGpuDeviceResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/QueryDGpuProfileAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/QueryDGpuProfileResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/RemoveVmDGpuStrategyAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/RemoveVmDGpuStrategyResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/SetDGpuProfileAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/SetDGpuProfileResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/SetVmDGpuStrategyAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/SetVmDGpuStrategyResult.javais excluded by!sdk/**
📒 Files selected for processing (4)
conf/db/upgrade/V5.5.12__schema.sqlheader/src/main/java/org/zstack/header/zwatch/ResourceMetricBindingExtensionPoint.javatestlib/src/main/java/org/zstack/testlib/ApiHelper.groovyutils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
DBImpact Resolves: ZSTAC-82677 Change-Id: I6b6a6766616d676e62676a66756f697469776764
MatheMatrix
force-pushed
the
sync/xinhao.huang/feature-5.5.12-dgpu@@2
branch
from
1a326c8 to
f7d68ea
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DBImpact
Resolves: ZSTAC-82677
Change-Id: I6b6a6766616d676e62676a66756f697469776764
sync from gitlab !9332