stabilize dGPU shmem hotplug management node#4005
Conversation
总览此 PR 为 KVM 虚拟化管理平台添加共享内存设备热插拔功能,并在测试库中增加 GPU 附加 API 方法。包括 Agent 命令、消息类型、常量定义和 KVMHost 后端处理逻辑,以及测试辅助方法实现。 变更KVM 共享内存热插拔实现
GPU 附加 API 测试方法
代码审查工作量估计🎯 2 (简单) | ⏱️ ~12 分钟 诗
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (3 passed)
✨ 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 |
22261fc to
3122dff
Compare
Add VM shmem KVM agent command binding and SDK updates for dGPU attach API. APIImpact Related: ZSTAC-84067 Change-Id: I84557417caafe61a240b325e950cbc0f12f59bc1
3122dff to
fa124b1
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java (1)
2734-2743: ⚡ Quick win建议给
origin参数增加编译期类型约束,避免强转风险。
sendVmShmemCommand当前将NeedReplyMessage强转为HostMessage(第 2739、2741 行)。建议改用交叉类型约束,使传入的消息类型同时满足NeedReplyMessage和HostMessage两个接口,避免运行时类型转换失败:参考改法
- private <T extends AgentResponse> void sendVmShmemCommand(NeedReplyMessage origin, + private <M extends NeedReplyMessage & HostMessage, T extends AgentResponse> void sendVmShmemCommand(M origin, Object cmd, String path, Class<T> responseClass) { KVMHostAsyncHttpCallMsg kmsg = new KVMHostAsyncHttpCallMsg(); kmsg.setCommand(cmd); - kmsg.setHostUuid(((HostMessage) origin).getHostUuid()); + kmsg.setHostUuid(origin.getHostUuid()); kmsg.setPath(path); - bus.makeTargetServiceIdByResourceUuid(kmsg, HostConstant.SERVICE_ID, ((HostMessage) origin).getHostUuid()); + bus.makeTargetServiceIdByResourceUuid(kmsg, HostConstant.SERVICE_ID, origin.getHostUuid());🤖 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/KVMHost.java` around lines 2734 - 2743, Change sendVmShmemCommand to require a message type that is both a NeedReplyMessage and HostMessage (e.g. use a generic type parameter like M extends NeedReplyMessage & HostMessage) so you can remove the unsafe casts ((HostMessage) origin).getHostUuid(). Update usages inside the method (calls to kmsg.setHostUuid(...) and bus.makeTargetServiceIdByResourceUuid(..., ((HostMessage) origin).getHostUuid())) to use origin.getHostUuid() directly, and update any call sites to pass a message type satisfying both interfaces; keep the rest of the method (KVMHostAsyncHttpCallMsg creation, setCommand, setPath, bus.send(..., new CloudBusCallBack(origin))) unchanged.
🤖 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.
Nitpick comments:
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java`:
- Around line 2734-2743: Change sendVmShmemCommand to require a message type
that is both a NeedReplyMessage and HostMessage (e.g. use a generic type
parameter like M extends NeedReplyMessage & HostMessage) so you can remove the
unsafe casts ((HostMessage) origin).getHostUuid(). Update usages inside the
method (calls to kmsg.setHostUuid(...) and
bus.makeTargetServiceIdByResourceUuid(..., ((HostMessage)
origin).getHostUuid())) to use origin.getHostUuid() directly, and update any
call sites to pass a message type satisfying both interfaces; keep the rest of
the method (KVMHostAsyncHttpCallMsg creation, setCommand, setPath, bus.send(...,
new CloudBusCallBack(origin))) unchanged.
ℹ️ 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: 309e5b37-60f4-432d-bd88-4274a517b1b6
⛔ Files ignored due to path filters (4)
conf/springConfigXml/Kvm.xmlis excluded by!**/*.xmlsdk/src/main/java/org/zstack/sdk/AttachDGpuToVmAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/AttachDGpuToVmResult.javais excluded by!sdk/**test/src/test/resources/springConfigXml/Kvm.xmlis excluded by!**/*.xml
📒 Files selected for processing (6)
plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.javaplugin/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
🚧 Files skipped from review as they are similar to previous changes (3)
- plugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.java
- plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java
- testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy
Rebased onto upstream feature-5.5.22-aios. Source: xinhao.huang/zstack:feature/dgpu-hotplug-shmem@@2
sync from gitlab !9898