<feature>[dgpu]: update strategy API SDK#4023
Open
zstack-robot-1 wants to merge 1 commit into
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 (3)
📒 Files selected for processing (4)
Walkthrough拆分 KVM shmem 消息字段为 shmemName/shmemPath/shmemSize 并在 KVMHost 中新增 buildVmShmemDevice(...) 构造 cmd.shmem；同时将 testlib 中 ApiHelper 的 变更说明KVM shmem 字段与构造逻辑变更
VM GPU 策略方法重命名
评估代码审查工作量🎯 3 (Moderate) | ⏱️ ~20 minutes 可能相关的 PRs
庆祝诗
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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.42.2)plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaComment |
MatheMatrix
force-pushed
the
sync/xinhao.huang/feature/ZSTAC-84067-dgpu-api-update@@2
branch
2 times, most recently
from
4011d57 to
3b38ffd
Compare
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHotPlugVmShmemMsg.java`:
- Around line 34-35: Update the message setters to normalize and validate
inputs: in setShmemName (and the similar setter for shmemPath) call trim() on
the incoming String, treat empty/blank results as null or reject by throwing
IllegalArgumentException with a clear message, and assign the normalized value
to the field; in the setter for shmemSize validate that the incoming long is > 0
and throw IllegalArgumentException (or otherwise enforce a sane minimum) if it
is non‑positive so invalid sizes cannot be propagated. Use the existing setter
names (setShmemName, the shmemPath setter, and setShmemSize) to locate where to
apply trim/null checks and positive-value validation.
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHotUnplugVmShmemMsg.java`:
- Around line 34-35: The setters in KVMHotUnplugVmShmemMsg lack input
normalization and validation; update setShmemName and setVmInstanceUuid to trim
incoming Strings safely (null-check before calling trim) and update setShmemSize
to validate the value is a positive number (shmemSize > 0) and throw an
appropriate IllegalArgumentException if invalid, mirroring the checks present in
the corresponding hot-plug message class so both message paths enforce the same
constraints.
🪄 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: 30e28a11-cc56-42bf-a00a-6e6a2c118a52
⛔ Files ignored due to path filters (3)
sdk/src/main/java/org/zstack/sdk/SetVmDGpuStrategyResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/UpdateVmDGpuAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/UpdateVmDGpuResult.javais excluded by!sdk/**
📒 Files selected for processing (4)
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHotPlugVmShmemMsg.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHotUnplugVmShmemMsg.javatestlib/src/main/java/org/zstack/testlib/ApiHelper.groovy
MatheMatrix
force-pushed
the
sync/xinhao.huang/feature/ZSTAC-84067-dgpu-api-update@@2
branch
from
3b38ffd to
4622440
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
plugin/kvm/src/main/java/org/zstack/kvm/KVMHotUnplugVmShmemMsg.java (1)
34-36:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win与热插拔消息保持一致：增加
trim与shmemSize > 0约束。Line 34、Line 42、Line 50 与对应 HotPlug 消息一致地需要输入规整和边界防护，否则两条路径会出现不一致的参数质量。
建议修改
public void setShmemName(String shmemName) { - this.shmemName = shmemName; + this.shmemName = shmemName == null ? null : shmemName.trim(); } public void setShmemPath(String shmemPath) { - this.shmemPath = shmemPath; + this.shmemPath = shmemPath == null ? null : shmemPath.trim(); } public void setShmemSize(long shmemSize) { + if (shmemSize <= 0) { + throw new IllegalArgumentException("shmemSize must be greater than 0"); + } this.shmemSize = shmemSize; }As per coding guidelines "注意检查来自 Message 的参数是否做过 trim，用户可能在浏览器上复制粘贴的数据带有空格、换行符等"。
Also applies to: 42-44, 50-51
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHotUnplugVmShmemMsg.java` around lines 34 - 36, In KVMHotUnplugVmShmemMsg adjust the setters to normalize and validate inputs like the HotPlug counterpart: in setShmemName(...) and the other message setters (e.g., setBus(...)) call trim() on incoming String parameters to remove whitespace, and in setShmemSize(...) enforce shmemSize > 0 (throw IllegalArgumentException or similar) to guard against non-positive sizes; update the constructors/getters usage if any to rely on the normalized values so both hot-plug and hot-unplug paths have consistent parameter quality.plugin/kvm/src/main/java/org/zstack/kvm/KVMHotPlugVmShmemMsg.java (1)
34-36:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win补齐消息入参规整与
shmemSize边界校验。Line 34、Line 42、Line 50 目前是直接透传，建议在 setter 中统一做
trim()（含null安全）并拒绝shmemSize <= 0，避免无效参数继续下游传播。建议修改
public void setShmemName(String shmemName) { - this.shmemName = shmemName; + this.shmemName = shmemName == null ? null : shmemName.trim(); } public void setShmemPath(String shmemPath) { - this.shmemPath = shmemPath; + this.shmemPath = shmemPath == null ? null : shmemPath.trim(); } public void setShmemSize(long shmemSize) { + if (shmemSize <= 0) { + throw new IllegalArgumentException("shmemSize must be greater than 0"); + } this.shmemSize = shmemSize; }As per coding guidelines "注意检查来自 Message 的参数是否做过 trim，用户可能在浏览器上复制粘贴的数据带有空格、换行符等"。
Also applies to: 42-44, 50-51
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHotPlugVmShmemMsg.java` around lines 34 - 36, The setters for message parameters must normalize input and validate bounds: update setShmemName, setShmemPath and setShmemSize to be null-safe and trim String inputs (use something like value == null ? null : value.trim()) and for setShmemSize reject non-positive sizes by throwing an IllegalArgumentException (or other appropriate runtime exception) when shmemSize <= 0 so invalid parameters aren't propagated downstream; reference the methods setShmemName, setShmemPath and setShmemSize when making these changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHotPlugVmShmemMsg.java`:
- Around line 34-36: The setters for message parameters must normalize input and
validate bounds: update setShmemName, setShmemPath and setShmemSize to be
null-safe and trim String inputs (use something like value == null ? null :
value.trim()) and for setShmemSize reject non-positive sizes by throwing an
IllegalArgumentException (or other appropriate runtime exception) when shmemSize
<= 0 so invalid parameters aren't propagated downstream; reference the methods
setShmemName, setShmemPath and setShmemSize when making these changes.
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHotUnplugVmShmemMsg.java`:
- Around line 34-36: In KVMHotUnplugVmShmemMsg adjust the setters to normalize
and validate inputs like the HotPlug counterpart: in setShmemName(...) and the
other message setters (e.g., setBus(...)) call trim() on incoming String
parameters to remove whitespace, and in setShmemSize(...) enforce shmemSize > 0
(throw IllegalArgumentException or similar) to guard against non-positive sizes;
update the constructors/getters usage if any to rely on the normalized values so
both hot-plug and hot-unplug paths have consistent parameter quality.
ℹ️ 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: 635d8a8e-443d-4d12-9add-e66f1f5962be
⛔ Files ignored due to path filters (3)
sdk/src/main/java/org/zstack/sdk/SetVmDGpuStrategyResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/UpdateVmDGpuAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/UpdateVmDGpuResult.javais excluded by!sdk/**
📒 Files selected for processing (4)
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHotPlugVmShmemMsg.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHotUnplugVmShmemMsg.javatestlib/src/main/java/org/zstack/testlib/ApiHelper.groovy
Rename SetVmDGpuStrategy SDK bindings to UpdateVmDGpu and expose the updated testlib helper. Carry VM shmem hotplug parameters through KVM messages as primitive fields so dGPU code does not expose agent command DTOs across modules. APIImpact Related: ZSTAC-84067 Change-Id: I7a1822b01861192c4baee5e0772354487423d5ce
MatheMatrix
force-pushed
the
sync/xinhao.huang/feature/ZSTAC-84067-dgpu-api-update@@2
branch
from
4622440 to
907e5a5
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.
Rename SetVmDGpuStrategy SDK bindings to UpdateVmDGpuStrategy and expose the updated testlib helper.
APIImpact
Related: ZSTAC-84067
Change-Id: I7a1822b01861192c4baee5e0772354487423d5ce
sync from gitlab !9918