Open
Conversation
Walkthrough这个PR引入了虚拟机实例元数据管理功能，包括在管理程序和主存储上读写元数据的消息、元数据更新的垃圾回收、具有敏感字段处理的安全序列化，以及VM注册和元数据检索的API端点。 Changes
Sequence Diagram(s)sequenceDiagram
participant Client as 客户端
participant VmInstance as VM实例
participant GC as 垃圾回收器
participant Hypervisor as 管理程序
participant PrimaryStorage as 主存储
Client->>VmInstance: UpdateVmInstanceMetadataMsg
VmInstance->>VmInstance: 构建VM元数据<br/>(卷、NIC、快照等)
VmInstance->>PrimaryStorage: UpdateVmInstanceMetadataOnPrimaryStorageMsg
PrimaryStorage->>Hypervisor: UpdateVmInstanceMetadataOnHypervisorMsg
Hypervisor->>Hypervisor: 写入元数据文件
Hypervisor-->>PrimaryStorage: 完成/失败回复
PrimaryStorage-->>VmInstance: 完成/失败回复
alt 失败情况
VmInstance->>GC: SubmitTimeBasedGarbageCollectorMsg
GC->>GC: 安排重试UpdateVmInstanceMetadataGC
else 成功
VmInstance->>Client: 成功回复
end
sequenceDiagram
participant API as 注册API
participant PrimaryStorage as 主存储
participant Hypervisor as 管理程序
participant VmManager as VM管理器
participant Database as 数据库
API->>PrimaryStorage: APIRegisterVmInstanceMsg<br/>(元数据路径)
PrimaryStorage->>Hypervisor: ReadVmInstanceMetadataOnHypervisorMsg
Hypervisor->>Hypervisor: 读取元数据文件
Hypervisor-->>PrimaryStorage: 元数据内容
PrimaryStorage->>Database: 持久化卷/快照
Database-->>PrimaryStorage: 确认
PrimaryStorage->>VmManager: 创建VmInstance实体
VmManager->>Database: 保存VmInstanceVO
Database-->>VmManager: 确认
VmManager-->>PrimaryStorage: VmInstanceInventory
PrimaryStorage-->>API: APIRegisterVmInstanceReply
API->>API: 发布注册事件
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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.40.5)compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🤖 Fix all issues with AI agents
In `@compute/src/main/java/org/zstack/compute/vm/UpdateVmInstanceMetadataGC.java`:
- Around line 35-40: There is a TOCTOU race between checking
chainInfo.getPendingTask().isEmpty() (from thdf.getChainTaskInfo(...)) and
actually submitting the GC task (e.g. chainSubmit), so add a second verification
just before submission: re-fetch ChainInfo via
thdf.getChainTaskInfo(getUpdateVmInstanceMetadataSyncSignature(vmInstanceUuid))
and only proceed with the submission if getPendingTask().isEmpty() still holds
otherwise call completion.cancel(); alternatively document that duplicate
submissions are acceptable for GC; update the code paths around
getUpdateVmInstanceMetadataSyncSignature, thdf.getChainTaskInfo,
completion.cancel and the chain submission site to perform this double-check or
explicit synchronization.
In `@compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java`:
- Line 24: Remove the unused import of APIRegisterVmInstanceMsg from
VmInstanceApiInterceptor: open the VmInstanceApiInterceptor class, locate the
import statement "import
org.zstack.header.storage.primary.APIRegisterVmInstanceMsg" and delete it (the
intercept() method in VmInstanceApiInterceptor does not handle this message and
APIRegisterVmInstanceMsg is handled by PrimaryStorageBase), leaving only the
imports actually used by the class.
In `@compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java`:
- Around line 9399-9411: In the bus.send callback (run method) that builds an
UpdateVmInstanceMetadataOnPrimaryStorageReply, ensure the failure branch calls
bus.reply(msg, reply) before invoking submitUpdateVmInstanceMetadataGC();
specifically, in the run override inside VmInstanceBase where you create the
reply on !r.isSuccess(), set the error on reply and immediately call
bus.reply(msg, reply) (using the same msg and reply variables) and only then
call submitUpdateVmInstanceMetadataGC() so the caller is replied to and won't
hang.
- Around line 9389-9392: The code assumes Tuple tuple =
Q.New(VolumeVO.class)...findTuple() always returns non-null; if tuple is null
the subsequent tuple.get(...) causes NPE. Fix: after obtaining tuple in
VmInstanceBase (the block that queries VolumeVO/VolumeVO_/VolumeType.Root with
msg.getUuid()), check if tuple == null and if so construct and send an error
reply and return (do not proceed to read primaryStorageUuid/rootVolumeUuid);
ensure the error contains context (vm uuid) and use the normal reply/error path
used elsewhere in this class.
In `@core/src/main/java/org/zstack/core/gc/GarbageCollectorManagerImpl.java`:
- Around line 287-291: 在 handle(SubmitTimeBasedGarbageCollectorMsg) 中对
msg.getGcInterval()（Long）和 msg.getUnit() 做非空与范围校验，避免自动拆箱 NPE 和无效调度：如果 gcInterval
为 null 或 gcInterval <= 0 或 unit 为 null，则不要调用 msg.getGc().submit(...); 构造并通过
bus.reply(msg, reply) 返回一个错误 MessageReply（含描述性错误信息），否则正常调用
msg.getGc().submit(msg.getGcInterval(), msg.getUnit()) 并返回成功回复；参照方法和字段名：handle,
SubmitTimeBasedGarbageCollectorMsg, getGc(), getGcInterval(), getUnit(), submit。
In
`@core/src/main/java/org/zstack/core/gc/SubmitTimeBasedGarbageCollectorMsg.java`:
- Around line 7-10: SubmitTimeBasedGarbageCollectorMsg currently includes a
TimeBasedGarbageCollector instance which breaks cross-node Gson serialization;
replace the gc field with a serializable identifier (e.g. String gcUuid or
String gcClassName) and any minimal state (gcInterval, unit), and update
send/receive flows to reconstruct the GC on the receiver side (use a GC
registry/factory or Spring bean lookup to create the TimeBasedGarbageCollector
such as UpdateVmInstanceMetadataGC instead of serializing
ThreadFacade-containing instances). Locate SubmitTimeBasedGarbageCollectorMsg
(field gc), the send path that calls makeTargetServiceIdByResourceUuid(), and
the message handler that schedules the GC, and change them to pass the
identifier and call your TimeBasedGarbageCollectorFactory/manager to rehydrate
the concrete GC before scheduling.
In
`@header/src/main/java/org/zstack/header/storage/primary/APIRegisterVmInstanceEventDoc_zh_cn.groovy`:
- Around line 10-23: Update the documentation entries for the
APIRegisterVmInstanceEvent: replace the placeholder desc "null" under the ref
block for name "inventory" (path
"org.zstack.header.storage.primary.APIRegisterVmInstanceEvent.inventory", clz
VmInstanceInventory.class) with a concise Chinese description of the
VmInstanceInventory (e.g., “虚拟机实例信息” plus any brief detail), and fill the field
block for name "success" (type "boolean") with a short Chinese description
indicating it denotes whether the operation succeeded (e.g., “操作是否成功”); keep the
descriptions brief and context-appropriate.
In
`@header/src/main/java/org/zstack/header/storage/primary/APIRegisterVmInstanceMsg.java`:
- Around line 26-27: 在 APIRegisterVmInstanceMsg 中，给 metadataPath 明确声明约束并在
__example__ 中填充示例值：将字段上的注解由 `@APIParam`() 改为 `@APIParam`(required = true)
以显式表明必需性，并在类的 __example__ 方法里通过对应的 setter（例如 setMetadataPath(...)）为 metadataPath
赋予一个合理的示例值，确保生成的 API 文档包含所有必填字段的示例。
In
`@header/src/main/java/org/zstack/header/storage/primary/APIRegisterVmInstanceMsgDoc_zh_cn.groovy`:
- Around line 49-57: Add a human-readable Chinese description for the
metadataPath parameter in APIRegisterVmInstanceMsgDoc_zh_cn.groovy by filling
the empty desc field inside the column block for "metadataPath"; mention the
parameter's purpose (path or URL to VM metadata), expected format (e.g.,
absolute path or URL, allowed schemes like http(s) or file, any encoding/size
constraints) and whether it is required, and include since "4.10.0" context —
update the desc to a concise sentence that guides API users on what to provide
and any validation rules.
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java`:
- Around line 4720-4733: Add the `@NoLogging` annotation to the sensitive metadata
fields to prevent them from being written to logs: annotate the 'metadata' field
in class WriteVmInstanceMetadataCmd and the 'metadata' field in class
ReadVmInstanceMetadataRsp with `@NoLogging`, and import the NoLogging annotation
(where other agent DTO annotations are defined) so the serializer/logging
framework will skip these fields.
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java`:
- Around line 7382-7388: In the success handler for ReadVmInstanceMetadataRsp
(the anonymous success(ReadVmInstanceMetadataRsp rsp) method), avoid setting
reply metadata when rsp.isSuccess() is false: only call
reply.setMetadata(rsp.metadata) inside the success branch; when rsp.isSuccess()
is false set the error on reply (reply.setError(...)) and do not set metadata
before bus.reply(msg, reply) and completion.done(); update the success(...)
logic accordingly so metadata is only populated on successful responses.
In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmBackend.java`:
- Around line 3800-3824: The code calls findValue() into installPath and
immediately uses replaceFirst causing an NPE if installPath is null; also the
error reply uses self.getUuid() (primary storage) instead of the root volume or
VM id. Update the handle(UpdateVmInstanceMetadataOnPrimaryStorageMsg...) to
check if installPath == null (or blank) before calling replaceFirst; if null,
create an UpdateVmInstanceMetadataOnPrimaryStorageReply, setError using
Platform.operr(...) that references msg.getRootVolumeUuid() (or
msg.getVmInstanceUuid() if available) and include the underlying cause/context,
then bus.reply(msg, reply) and return. Keep the rest of the flow (creating
metadataPath, sending UpdateVmInstanceMetadataOnHypervisorMsg) unchanged.
In
`@plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorage.java`:
- Around line 1928-1951: The handler
handle(UpdateVmInstanceMetadataOnPrimaryStorageMsg) must null-check the backend
returned by getUsableBackend() and should select the backend based on the host
UUID to avoid NPEs and wrong routing; change the logic to call
getHostUuidFromVolume(msg.getRootVolumeUuid()), then obtain a backend via a
host-aware selector (e.g. a new/getBackendForHost(hostUuid) or similar lookup)
instead of blindly using getUsableBackend(), validate that the returned
NfsPrimaryStorageBackend is non-null and return an error reply if it is null,
and only then call backend.handle(msg, hostUuid, new ReturnValueCompletion...).
In
`@plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.java`:
- Around line 2052-2067: The callback in
NfsPrimaryStorageKVMBackend.handle(UpdateVmInstanceMetadataOnPrimaryStorageMsg...)
is replying with the wrong reply type and never completing the provided
ReturnValueCompletion; change the CloudBusCallBack to construct and send an
UpdateVmInstanceMetadataOnPrimaryStorageReply and call completion.success(reply)
on success or completion.fail(error) on failure (use the hypervisor MessageReply
error via r.getError()), and stop using
RevertVolumeFromSnapshotOnPrimaryStorageReply or bus.reply(msg, ...) so the
caller's completion is properly resolved and the UUID in error messages is the
vm/primary storage UUID from msg.
In `@storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java`:
- Around line 1870-2134: The registerVmInstance flow chain currently uses only
NoRollbackFlow steps (read-metadata, register-volume, register-snapshot,
register-vmInstance) and persists entities via dbf.persist/dbf.persistCollection
without compensation, leaving stale
VolumeVO/VolumeSnapshotVO/VolumeSnapshotTreeVO/VolumeSnapshotGroupVO/VolumeSnapshotGroupRefVO/VmInstanceVO
rows on failure; change the persisting flows to support rollback by either (a)
switch those NoRollbackFlow instances to Flow and implement rollback handlers
that delete any entities created in that flow (track created VO UUIDs in
flow-local lists) or (b) wrap the whole chain in a single
transactional/compensating flow that deletes all created entities on error (use
the collected created lists from register-volume, register-snapshot,
register-vmInstance and remove via dbf.remove or appropriate DAO methods in the
Flow.rollback), ensuring completion/error handlers remain unchanged so
completion.fail is called on error.
- Around line 1837-1867: The fail branch in handle(APIRegisterVmInstanceMsg msg)
publishes the APIRegisterVmInstanceEvent without setting the error, so callers
may think the operation succeeded; in the
ReturnValueCompletion<VmInstanceInventory>.fail(ErrorCode errorCode)
implementation, call event.setError(errorCode) (or set an appropriate error on
the APIRegisterVmInstanceEvent) before bus.publish(event) and chain.next() so
the failure is propagated back to the caller when registerVmInstance fails.
🧹 Nitpick comments (7)
testlib/src/main/java/org/zstack/testlib/KVMSimulator.groovy (1)
684-691: 建议将超长 JSON 响应外置或构建生成。当前内联字符串过长，维护和阅读成本高；可考虑放到资源文件或用结构化对象再序列化生成。
sdk/src/main/java/org/zstack/sdk/RegisterVmInstanceAction.java (2)
5-5: 冗余的同包导入。
import org.zstack.sdk.*;是冗余的，因为该类本身就在org.zstack.sdk包中。♻️ 建议移除冗余导入
import java.util.HashMap; import java.util.Map; -import org.zstack.sdk.*;
43-47: 建议使用泛型参数化 List 类型。
systemTags和userTags使用了原始类型List，建议参数化为List<String>以提高类型安全性。♻️ 建议的类型参数化
`@Param`(required = false) - public java.util.List systemTags; + public java.util.List<String> systemTags; `@Param`(required = false) - public java.util.List userTags; + public java.util.List<String> userTags;header/src/main/java/org/zstack/header/storage/primary/ReadVmInstanceMetadataOnHypervisorReply.java (1)
5-14: 建议字段命名保持一致性。
ReadVmInstanceMetadataOnHypervisorReply使用字段名metadata，而ReadVmInstanceMetadataReply使用vmMetadata。建议统一字段命名以提高代码可维护性和数据映射的清晰度。♻️ 建议的修改
public class ReadVmInstanceMetadataOnHypervisorReply extends MessageReply { - private String metadata; + private String vmMetadata; - public String getMetadata() { - return metadata; + public String getVmMetadata() { + return vmMetadata; } - public void setMetadata(String metadata) { - this.metadata = metadata; + public void setVmMetadata(String vmMetadata) { + this.vmMetadata = vmMetadata; } }storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java (1)
2138-2185: 反射读取静态字段不应依赖 clazz.newInstance()。
newInstance()已废弃且要求 public 无参构造；对 static 字段可用field.get(null)。建议改为getDeclaredConstructor().newInstance()（并处理异常）或按是否 static 取值。header/src/main/java/org/zstack/header/storage/primary/APIRegisterVmInstanceEvent.java (1)
24-25: 建议与现有 API Event 保持一致的构造函数风格参考
APIReimageVmInstanceEvent的无参构造函数实现：public APIReimageVmInstanceEvent() { super(null); }当前实现虽然功能上没有问题，但建议保持一致性。
♻️ 可选的修改建议
public APIRegisterVmInstanceEvent() { + super(null); }header/src/main/java/org/zstack/header/storage/primary/APIRegisterVmInstanceMsg.java (1)
70-72:__example__()返回空对象，文档示例信息不足。建议填充代表性字段值，提升自动生成的 API 文档可读性与可用性。
♻️ 建议示例填充
public static APIRegisterVmInstanceMsg __example__() { APIRegisterVmInstanceMsg msg = new APIRegisterVmInstanceMsg(); + msg.setZoneUuid(uuid()); + msg.setClusterUuid(uuid()); + msg.setPrimaryStorageUuid(uuid()); + msg.setHostUuid(uuid()); + msg.setMetadataPath("/zstack/metadata/vm-uuid"); return msg; }
Comment on lines
+35
to
+40
| String queueName = getUpdateVmInstanceMetadataSyncSignature(vmInstanceUuid); | ||
| ChainInfo chainInfo = thdf.getChainTaskInfo(queueName); | ||
| if (!chainInfo.getPendingTask().isEmpty()) { | ||
| completion.cancel(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
潜在的 TOCTOU 竞态条件
在检查 chainInfo.getPendingTask().isEmpty() 和提交 chainSubmit 之间存在时间窗口，其他线程可能在此期间提交任务。不过考虑到这是 GC 场景，偶尔的重复提交不会造成数据一致性问题（因为后续会通过 syncSignature 串行化执行），影响较小。
建议确认这种行为是否符合预期设计。
🤖 Prompt for AI Agents
In `@compute/src/main/java/org/zstack/compute/vm/UpdateVmInstanceMetadataGC.java`
around lines 35 - 40, There is a TOCTOU race between checking
chainInfo.getPendingTask().isEmpty() (from thdf.getChainTaskInfo(...)) and
actually submitting the GC task (e.g. chainSubmit), so add a second verification
just before submission: re-fetch ChainInfo via
thdf.getChainTaskInfo(getUpdateVmInstanceMetadataSyncSignature(vmInstanceUuid))
and only proceed with the submission if getPendingTask().isEmpty() still holds
otherwise call completion.cancel(); alternatively document that duplicate
submissions are acceptable for GC; update the code paths around
getUpdateVmInstanceMetadataSyncSignature, thdf.getChainTaskInfo,
completion.cancel and the chain submission site to perform this double-check or
explicit synchronization.
| import org.zstack.header.message.APIMessage; | ||
| import org.zstack.header.network.l2.*; | ||
| import org.zstack.header.network.l3.*; | ||
| import org.zstack.header.storage.primary.APIRegisterVmInstanceMsg; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if APIRegisterVmInstanceMsg is used in VmInstanceApiInterceptor.java
rg -n "APIRegisterVmInstanceMsg" compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java
# Also check the intercept method to see all handled message types
rg -n "instanceof.*Msg" compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java | head -30Repository: MatheMatrix/zstack
Length of output: 2170
🏁 Script executed:
#!/bin/bash
# Check if there's a handler method for APIRegisterVmInstanceMsg anywhere in the intercept method
cat -n compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java | sed -n '85,180p'
# Also check if the method signature and pattern continues beyond line 146
tail -n +140 compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java | head -50Repository: MatheMatrix/zstack
Length of output: 8697
🏁 Script executed:
#!/bin/bash
# Verify if APIRegisterVmInstanceMsg is used in other parts of the codebase
rg -r "APIRegisterVmInstanceMsg" compute/src/main/java/org/zstack/compute/vm/ --type java
# Check if there's any mention of RegisterVmInstance in other interceptor or handler files
rg -r "RegisterVmInstance" compute/src/main/java/org/zstack/compute/ --type java | head -20Repository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Double-check: search entire repository for APIRegisterVmInstanceMsg to be thorough
rg "APIRegisterVmInstanceMsg" --type javaRepository: MatheMatrix/zstack
Length of output: 1264
移除未使用的 APIRegisterVmInstanceMsg 导入。
该导入在 intercept() 方法中不存在对应的处理逻辑。APIRegisterVmInstanceMsg 实际由 PrimaryStorageBase 处理，与本文件无关。
🤖 Prompt for AI Agents
In `@compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java` at
line 24, Remove the unused import of APIRegisterVmInstanceMsg from
VmInstanceApiInterceptor: open the VmInstanceApiInterceptor class, locate the
import statement "import
org.zstack.header.storage.primary.APIRegisterVmInstanceMsg" and delete it (the
intercept() method in VmInstanceApiInterceptor does not handle this message and
APIRegisterVmInstanceMsg is handled by PrimaryStorageBase), leaving only the
imports actually used by the class.
Comment on lines
+9389
to
+9392
| Tuple tuple = Q.New(VolumeVO.class).select(VolumeVO_.primaryStorageUuid, VolumeVO_.uuid) | ||
| .eq(VolumeVO_.vmInstanceUuid, msg.getUuid()).eq(VolumeVO_.type, VolumeType.Root).findTuple(); | ||
| String primaryStorageUuid = tuple.get(0, String.class); | ||
| String rootVolumeUuid = tuple.get(1, String.class); |
There was a problem hiding this comment.
补充根卷查询为空的兜底处理。
Line 9389-9392 若根卷不存在，findTuple() 可能返回 null，直接 tuple.get(...) 会 NPE。建议先返回错误并回复消息。
🛠️ 建议修复
- Tuple tuple = Q.New(VolumeVO.class).select(VolumeVO_.primaryStorageUuid, VolumeVO_.uuid)
- .eq(VolumeVO_.vmInstanceUuid, msg.getUuid()).eq(VolumeVO_.type, VolumeType.Root).findTuple();
- String primaryStorageUuid = tuple.get(0, String.class);
- String rootVolumeUuid = tuple.get(1, String.class);
+ Tuple tuple = Q.New(VolumeVO.class).select(VolumeVO_.primaryStorageUuid, VolumeVO_.uuid)
+ .eq(VolumeVO_.vmInstanceUuid, msg.getUuid()).eq(VolumeVO_.type, VolumeType.Root).findTuple();
+ if (tuple == null) {
+ UpdateVmInstanceMetadataOnPrimaryStorageReply reply = new UpdateVmInstanceMetadataOnPrimaryStorageReply();
+ reply.setError(operr("cannot find root volume for vm[uuid:%s]", msg.getUuid()));
+ bus.reply(msg, reply);
+ return;
+ }
+ String primaryStorageUuid = tuple.get(0, String.class);
+ String rootVolumeUuid = tuple.get(1, String.class);🤖 Prompt for AI Agents
In `@compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java` around lines
9389 - 9392, The code assumes Tuple tuple = Q.New(VolumeVO.class)...findTuple()
always returns non-null; if tuple is null the subsequent tuple.get(...) causes
NPE. Fix: after obtaining tuple in VmInstanceBase (the block that queries
VolumeVO/VolumeVO_/VolumeType.Root with msg.getUuid()), check if tuple == null
and if so construct and send an error reply and return (do not proceed to read
primaryStorageUuid/rootVolumeUuid); ensure the error contains context (vm uuid)
and use the normal reply/error path used elsewhere in this class.
Comment on lines
+9399
to
+9411
| bus.send(umsg, new CloudBusCallBack(msg) { | ||
| @Override | ||
| public void run(MessageReply r) { | ||
| UpdateVmInstanceMetadataOnPrimaryStorageReply reply = new UpdateVmInstanceMetadataOnPrimaryStorageReply(); | ||
|
|
||
| if (!r.isSuccess()) { | ||
| reply.setError(Platform.operr("failed to update vm[uuid=%s] on hypervisor.", self.getUuid()) | ||
| .withCause(r.getError())); | ||
|
|
||
| submitUpdateVmInstanceMetadataGC(); | ||
| return; | ||
| } | ||
| bus.reply(msg, reply); |
There was a problem hiding this comment.
失败路径缺少回复会导致调用方悬挂。
Line 9404-9410 仅设置 error 并提交 GC，但没有 bus.reply(msg, reply)，上游会一直等待。建议先回复错误再提交 GC。
🛠️ 建议修复
if (!r.isSuccess()) {
reply.setError(Platform.operr("failed to update vm[uuid=%s] on hypervisor.", self.getUuid())
.withCause(r.getError()));
+ bus.reply(msg, reply);
submitUpdateVmInstanceMetadataGC();
return;
}
bus.reply(msg, reply);🤖 Prompt for AI Agents
In `@compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java` around lines
9399 - 9411, In the bus.send callback (run method) that builds an
UpdateVmInstanceMetadataOnPrimaryStorageReply, ensure the failure branch calls
bus.reply(msg, reply) before invoking submitUpdateVmInstanceMetadataGC();
specifically, in the run override inside VmInstanceBase where you create the
reply on !r.isSuccess(), set the error on reply and immediately call
bus.reply(msg, reply) (using the same msg and reply variables) and only then
call submitUpdateVmInstanceMetadataGC() so the caller is replied to and won't
hang.
Comment on lines
+287
to
+291
| private void handle(final SubmitTimeBasedGarbageCollectorMsg msg) { | ||
| MessageReply reply = new MessageReply(); | ||
| msg.getGc().submit(msg.getGcInterval(), msg.getUnit()); | ||
| bus.reply(msg, reply); | ||
| } |
There was a problem hiding this comment.
补充参数非空与范围校验，避免 NPE 与无效调度。
gcInterval 为 Long，为空时自动拆箱会抛 NPE；interval <= 0 也可能导致错误调度。建议在回复前校验并返回错误。
🛡️ 建议修复
private void handle(final SubmitTimeBasedGarbageCollectorMsg msg) {
MessageReply reply = new MessageReply();
- msg.getGc().submit(msg.getGcInterval(), msg.getUnit());
- bus.reply(msg, reply);
+ if (msg.getGc() == null || msg.getGcInterval() == null || msg.getUnit() == null) {
+ reply.setError(operr("gc/gcInterval/unit cannot be null"));
+ bus.reply(msg, reply);
+ return;
+ }
+ if (msg.getGcInterval() <= 0) {
+ reply.setError(operr("gcInterval must be > 0"));
+ bus.reply(msg, reply);
+ return;
+ }
+ msg.getGc().submit(msg.getGcInterval(), msg.getUnit());
+ bus.reply(msg, reply);
}🤖 Prompt for AI Agents
In `@core/src/main/java/org/zstack/core/gc/GarbageCollectorManagerImpl.java`
around lines 287 - 291, 在 handle(SubmitTimeBasedGarbageCollectorMsg) 中对
msg.getGcInterval()（Long）和 msg.getUnit() 做非空与范围校验，避免自动拆箱 NPE 和无效调度：如果 gcInterval
为 null 或 gcInterval <= 0 或 unit 为 null，则不要调用 msg.getGc().submit(...); 构造并通过
bus.reply(msg, reply) 返回一个错误 MessageReply（含描述性错误信息），否则正常调用
msg.getGc().submit(msg.getGcInterval(), msg.getUnit()) 并返回成功回复；参照方法和字段名：handle,
SubmitTimeBasedGarbageCollectorMsg, getGc(), getGcInterval(), getUnit(), submit。
Comment on lines
+3800
to
+3824
| @Override | ||
| void handle(UpdateVmInstanceMetadataOnPrimaryStorageMsg msg, String hostUuid, ReturnValueCompletion<UpdateVmInstanceMetadataOnPrimaryStorageReply> completion) { | ||
| String installPath = Q.New(VolumeVO.class).eq(VolumeVO_.uuid, msg.getRootVolumeUuid()).select(VolumeVO_.installPath).findValue(); | ||
| // /vms_ds/rootVolumes/acct-36c27e8ff05c4780bf6d2fa65700f22e/vol-829a91b68e794a03865eab8a5918600a/snapshots/f2c31aeede604917aa8cee24848d8bfa.qcow2 | ||
| // /vms_ds/rootVolumes/acct-36c27e8ff05c4780bf6d2fa65700f22e/vol-829a91b68e794a03865eab8a5918600a/829a91b68e794a03865eab8a5918600a.qcow2 | ||
|
|
||
| String path = installPath.replaceFirst("^(.+/vol-[^/]+/).*$", "$1"); | ||
| String metadataPath = String.format("%s%s", path, VM_META_SUFFIX); | ||
|
|
||
| UpdateVmInstanceMetadataOnHypervisorMsg umsg = new UpdateVmInstanceMetadataOnHypervisorMsg(); | ||
| umsg.setMetadata(msg.getMetadata()); | ||
| umsg.setMetadataPath(metadataPath); | ||
| umsg.setHostUuid(hostUuid); | ||
| bus.makeTargetServiceIdByResourceUuid(umsg, HostConstant.SERVICE_ID, hostUuid); | ||
| bus.send(umsg, new CloudBusCallBack(msg) { | ||
| @Override | ||
| public void run(MessageReply r) { | ||
| UpdateVmInstanceMetadataOnPrimaryStorageReply reply = new UpdateVmInstanceMetadataOnPrimaryStorageReply(); | ||
| if (!r.isSuccess()) { | ||
| reply.setError(Platform.operr("failed to update vm[uuid=%s] on hypervisor.", self.getUuid()) | ||
| .withCause(r.getError())); | ||
| } | ||
| bus.reply(msg, reply); | ||
| } | ||
| }); |
There was a problem hiding this comment.
未校验 root volume installPath，可能触发 NPE。
findValue() 为空时 replaceFirst 会 NPE；同时错误信息使用了 primary storage uuid。建议先判空并用 rootVolumeUuid/VM uuid 提示。
🔧 建议修复
- String installPath = Q.New(VolumeVO.class).eq(VolumeVO_.uuid, msg.getRootVolumeUuid()).select(VolumeVO_.installPath).findValue();
+ String installPath = Q.New(VolumeVO.class).eq(VolumeVO_.uuid, msg.getRootVolumeUuid()).select(VolumeVO_.installPath).findValue();
+ final UpdateVmInstanceMetadataOnPrimaryStorageReply reply = new UpdateVmInstanceMetadataOnPrimaryStorageReply();
+ if (installPath == null) {
+ reply.setError(operr("cannot find root volume[uuid:%s] installPath", msg.getRootVolumeUuid()));
+ bus.reply(msg, reply);
+ return;
+ }
@@
- bus.send(umsg, new CloudBusCallBack(msg) {
+ bus.send(umsg, new CloudBusCallBack(msg) {
`@Override`
public void run(MessageReply r) {
- UpdateVmInstanceMetadataOnPrimaryStorageReply reply = new UpdateVmInstanceMetadataOnPrimaryStorageReply();
if (!r.isSuccess()) {
- reply.setError(Platform.operr("failed to update vm[uuid=%s] on hypervisor.", self.getUuid())
+ reply.setError(Platform.operr("failed to update vm[rootVolumeUuid=%s] on hypervisor.", msg.getRootVolumeUuid())
.withCause(r.getError()));
}
bus.reply(msg, reply);
}
});🤖 Prompt for AI Agents
In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmBackend.java`
around lines 3800 - 3824, The code calls findValue() into installPath and
immediately uses replaceFirst causing an NPE if installPath is null; also the
error reply uses self.getUuid() (primary storage) instead of the root volume or
VM id. Update the handle(UpdateVmInstanceMetadataOnPrimaryStorageMsg...) to
check if installPath == null (or blank) before calling replaceFirst; if null,
create an UpdateVmInstanceMetadataOnPrimaryStorageReply, setError using
Platform.operr(...) that references msg.getRootVolumeUuid() (or
msg.getVmInstanceUuid() if available) and include the underlying cause/context,
then bus.reply(msg, reply) and return. Keep the rest of the flow (creating
metadataPath, sending UpdateVmInstanceMetadataOnHypervisorMsg) unchanged.
Comment on lines
+1928
to
+1951
| protected void handle(UpdateVmInstanceMetadataOnPrimaryStorageMsg msg) { | ||
| UpdateVmInstanceMetadataOnPrimaryStorageReply reply = new UpdateVmInstanceMetadataOnPrimaryStorageReply(); | ||
|
|
||
| String hostUuid = getHostUuidFromVolume(msg.getRootVolumeUuid()); | ||
| if (hostUuid == null || hostUuid.isEmpty()) { | ||
| reply.setError(operr("no host found for volume[uuid:%s]", msg.getRootVolumeUuid())); | ||
| bus.reply(msg, reply); | ||
| return; | ||
| } | ||
|
|
||
| final NfsPrimaryStorageBackend backend = getUsableBackend(); | ||
|
|
||
| backend.handle(msg, hostUuid, new ReturnValueCompletion<UpdateVmInstanceMetadataOnPrimaryStorageReply>(msg) { | ||
| @Override | ||
| public void success(UpdateVmInstanceMetadataOnPrimaryStorageReply r) { | ||
| bus.reply(msg, r); | ||
| } | ||
|
|
||
| @Override | ||
| public void fail(ErrorCode errorCode) { | ||
| reply.setError(errorCode); | ||
| bus.reply(msg, reply); | ||
| } | ||
| }); |
There was a problem hiding this comment.
backend 选择与空值校验缺失，存在 NPE/路由错误风险。
这里直接使用 getUsableBackend() 且无空值检查；若无可用 backend 会直接 NPE。同时建议基于 hostUuid 选择 backend，以避免多 hypervisor 场景走错实现。
✅ 建议修复
- final NfsPrimaryStorageBackend backend = getUsableBackend();
+ final NfsPrimaryStorageBackend backend = getBackendByHostUuid(hostUuid);
+ if (backend == null) {
+ reply.setError(operr("no backend found for host[uuid:%s]", hostUuid));
+ bus.reply(msg, reply);
+ return;
+ }
backend.handle(msg, hostUuid, new ReturnValueCompletion<UpdateVmInstanceMetadataOnPrimaryStorageReply>(msg) {
`@Override`
public void success(UpdateVmInstanceMetadataOnPrimaryStorageReply r) {
bus.reply(msg, r);
}📝 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
| protected void handle(UpdateVmInstanceMetadataOnPrimaryStorageMsg msg) { | |
| UpdateVmInstanceMetadataOnPrimaryStorageReply reply = new UpdateVmInstanceMetadataOnPrimaryStorageReply(); | |
| String hostUuid = getHostUuidFromVolume(msg.getRootVolumeUuid()); | |
| if (hostUuid == null || hostUuid.isEmpty()) { | |
| reply.setError(operr("no host found for volume[uuid:%s]", msg.getRootVolumeUuid())); | |
| bus.reply(msg, reply); | |
| return; | |
| } | |
| final NfsPrimaryStorageBackend backend = getUsableBackend(); | |
| backend.handle(msg, hostUuid, new ReturnValueCompletion<UpdateVmInstanceMetadataOnPrimaryStorageReply>(msg) { | |
| @Override | |
| public void success(UpdateVmInstanceMetadataOnPrimaryStorageReply r) { | |
| bus.reply(msg, r); | |
| } | |
| @Override | |
| public void fail(ErrorCode errorCode) { | |
| reply.setError(errorCode); | |
| bus.reply(msg, reply); | |
| } | |
| }); | |
| protected void handle(UpdateVmInstanceMetadataOnPrimaryStorageMsg msg) { | |
| UpdateVmInstanceMetadataOnPrimaryStorageReply reply = new UpdateVmInstanceMetadataOnPrimaryStorageReply(); | |
| String hostUuid = getHostUuidFromVolume(msg.getRootVolumeUuid()); | |
| if (hostUuid == null || hostUuid.isEmpty()) { | |
| reply.setError(operr("no host found for volume[uuid:%s]", msg.getRootVolumeUuid())); | |
| bus.reply(msg, reply); | |
| return; | |
| } | |
| final NfsPrimaryStorageBackend backend = getUsableBackend(); | |
| if (backend == null) { | |
| reply.setError(operr("no backend found for volume[uuid:%s]", msg.getRootVolumeUuid())); | |
| bus.reply(msg, reply); | |
| return; | |
| } | |
| backend.handle(msg, hostUuid, new ReturnValueCompletion<UpdateVmInstanceMetadataOnPrimaryStorageReply>(msg) { | |
| `@Override` | |
| public void success(UpdateVmInstanceMetadataOnPrimaryStorageReply r) { | |
| bus.reply(msg, r); | |
| } | |
| `@Override` | |
| public void fail(ErrorCode errorCode) { | |
| reply.setError(errorCode); | |
| bus.reply(msg, reply); | |
| } | |
| }); | |
| } |
🤖 Prompt for AI Agents
In
`@plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorage.java`
around lines 1928 - 1951, The handler
handle(UpdateVmInstanceMetadataOnPrimaryStorageMsg) must null-check the backend
returned by getUsableBackend() and should select the backend based on the host
UUID to avoid NPEs and wrong routing; change the logic to call
getHostUuidFromVolume(msg.getRootVolumeUuid()), then obtain a backend via a
host-aware selector (e.g. a new/getBackendForHost(hostUuid) or similar lookup)
instead of blindly using getUsableBackend(), validate that the returned
NfsPrimaryStorageBackend is non-null and return an error reply if it is null,
and only then call backend.handle(msg, hostUuid, new ReturnValueCompletion...).
Comment on lines
+2052
to
+2067
| public void handle(UpdateVmInstanceMetadataOnPrimaryStorageMsg msg, String hostUuid, ReturnValueCompletion<UpdateVmInstanceMetadataOnPrimaryStorageReply> completion) { | ||
| UpdateVmInstanceMetadataOnHypervisorMsg umsg = new UpdateVmInstanceMetadataOnHypervisorMsg(); | ||
| umsg.setMetadata(msg.getMetadata()); | ||
| umsg.setHostUuid(hostUuid); | ||
| bus.makeTargetServiceIdByResourceUuid(umsg, HostConstant.SERVICE_ID, hostUuid); | ||
| bus.send(umsg, new CloudBusCallBack(msg) { | ||
| @Override | ||
| public void run(MessageReply r) { | ||
| RevertVolumeFromSnapshotOnPrimaryStorageReply reply = new RevertVolumeFromSnapshotOnPrimaryStorageReply(); | ||
| if (!r.isSuccess()) { | ||
| reply.setError(Platform.operr("failed to update vm[uuid=%s] on hypervisor.", msg.getPrimaryStorageUuid()) | ||
| .withCause(r.getError())); | ||
| } | ||
| bus.reply(msg, reply); | ||
| } | ||
| }); |
There was a problem hiding this comment.
回调路径与回复类型不匹配，导致上层永远不完成。
当前实现忽略 completion 且使用了错误的 reply 类型，容易造成上层等待超时，并且错误信息中的 UUID 也不准确。请改为通过 completion.success/fail 传递结果并使用正确的 reply 类型。
✅ 建议修复
public void handle(UpdateVmInstanceMetadataOnPrimaryStorageMsg msg, String hostUuid, ReturnValueCompletion<UpdateVmInstanceMetadataOnPrimaryStorageReply> completion) {
UpdateVmInstanceMetadataOnHypervisorMsg umsg = new UpdateVmInstanceMetadataOnHypervisorMsg();
umsg.setMetadata(msg.getMetadata());
umsg.setHostUuid(hostUuid);
bus.makeTargetServiceIdByResourceUuid(umsg, HostConstant.SERVICE_ID, hostUuid);
bus.send(umsg, new CloudBusCallBack(msg) {
`@Override`
public void run(MessageReply r) {
- RevertVolumeFromSnapshotOnPrimaryStorageReply reply = new RevertVolumeFromSnapshotOnPrimaryStorageReply();
if (!r.isSuccess()) {
- reply.setError(Platform.operr("failed to update vm[uuid=%s] on hypervisor.", msg.getPrimaryStorageUuid())
- .withCause(r.getError()));
+ completion.fail(Platform.operr("failed to update vm metadata for root volume[uuid=%s] on hypervisor.",
+ msg.getRootVolumeUuid()).withCause(r.getError()));
+ return;
}
- bus.reply(msg, reply);
+ completion.success(new UpdateVmInstanceMetadataOnPrimaryStorageReply());
}
});
}📝 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
| public void handle(UpdateVmInstanceMetadataOnPrimaryStorageMsg msg, String hostUuid, ReturnValueCompletion<UpdateVmInstanceMetadataOnPrimaryStorageReply> completion) { | |
| UpdateVmInstanceMetadataOnHypervisorMsg umsg = new UpdateVmInstanceMetadataOnHypervisorMsg(); | |
| umsg.setMetadata(msg.getMetadata()); | |
| umsg.setHostUuid(hostUuid); | |
| bus.makeTargetServiceIdByResourceUuid(umsg, HostConstant.SERVICE_ID, hostUuid); | |
| bus.send(umsg, new CloudBusCallBack(msg) { | |
| @Override | |
| public void run(MessageReply r) { | |
| RevertVolumeFromSnapshotOnPrimaryStorageReply reply = new RevertVolumeFromSnapshotOnPrimaryStorageReply(); | |
| if (!r.isSuccess()) { | |
| reply.setError(Platform.operr("failed to update vm[uuid=%s] on hypervisor.", msg.getPrimaryStorageUuid()) | |
| .withCause(r.getError())); | |
| } | |
| bus.reply(msg, reply); | |
| } | |
| }); | |
| public void handle(UpdateVmInstanceMetadataOnPrimaryStorageMsg msg, String hostUuid, ReturnValueCompletion<UpdateVmInstanceMetadataOnPrimaryStorageReply> completion) { | |
| UpdateVmInstanceMetadataOnHypervisorMsg umsg = new UpdateVmInstanceMetadataOnHypervisorMsg(); | |
| umsg.setMetadata(msg.getMetadata()); | |
| umsg.setHostUuid(hostUuid); | |
| bus.makeTargetServiceIdByResourceUuid(umsg, HostConstant.SERVICE_ID, hostUuid); | |
| bus.send(umsg, new CloudBusCallBack(msg) { | |
| `@Override` | |
| public void run(MessageReply r) { | |
| if (!r.isSuccess()) { | |
| completion.fail(Platform.operr("failed to update vm metadata for root volume[uuid=%s] on hypervisor.", | |
| msg.getRootVolumeUuid()).withCause(r.getError())); | |
| return; | |
| } | |
| completion.success(new UpdateVmInstanceMetadataOnPrimaryStorageReply()); | |
| } | |
| }); | |
| } |
🤖 Prompt for AI Agents
In
`@plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.java`
around lines 2052 - 2067, The callback in
NfsPrimaryStorageKVMBackend.handle(UpdateVmInstanceMetadataOnPrimaryStorageMsg...)
is replying with the wrong reply type and never completing the provided
ReturnValueCompletion; change the CloudBusCallBack to construct and send an
UpdateVmInstanceMetadataOnPrimaryStorageReply and call completion.success(reply)
on success or completion.fail(error) on failure (use the hypervisor MessageReply
error via r.getError()), and stop using
RevertVolumeFromSnapshotOnPrimaryStorageReply or bus.reply(msg, ...) so the
caller's completion is properly resolved and the UUID in error messages is the
vm/primary storage UUID from msg.
Comment on lines
1837
to
1867
| private void handle(APIRegisterVmInstanceMsg msg) { | ||
| APIRegisterVmInstanceEvent event = new APIRegisterVmInstanceEvent(msg.getId()); | ||
| thdf.chainSubmit(new ChainTask(msg) { | ||
| @Override | ||
| public String getSyncSignature() { | ||
| return String.format("register-vm-from-%s", msg.getMetadataPath()); | ||
| } | ||
|
|
||
| @Override | ||
| public void run(SyncTaskChain chain) { | ||
| registerVmInstance(msg, new ReturnValueCompletion<VmInstanceInventory>(chain, msg) { | ||
| @Override | ||
| public void success(VmInstanceInventory vmInstanceInventory) { | ||
| event.setInventory(vmInstanceInventory); | ||
| bus.publish(event); | ||
| chain.next(); | ||
| } | ||
|
|
||
| @Override | ||
| public void fail(ErrorCode errorCode) { | ||
| bus.publish(event); | ||
| chain.next(); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| @Override | ||
| public String getName() { | ||
| return String.format("register-vm-from-%s", msg.getMetadataPath()); | ||
| } | ||
| }); |
There was a problem hiding this comment.
失败分支未设置 event 错误。
当前 fail 只 publish event，调用方可能被视为成功；应设置 error。
🔧 建议修复
- public void fail(ErrorCode errorCode) {
- bus.publish(event);
- chain.next();
- }
+ public void fail(ErrorCode errorCode) {
+ event.setError(errorCode);
+ bus.publish(event);
+ chain.next();
+ }🤖 Prompt for AI Agents
In `@storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java`
around lines 1837 - 1867, The fail branch in handle(APIRegisterVmInstanceMsg
msg) publishes the APIRegisterVmInstanceEvent without setting the error, so
callers may think the operation succeeded; in the
ReturnValueCompletion<VmInstanceInventory>.fail(ErrorCode errorCode)
implementation, call event.setError(errorCode) (or set an appropriate error on
the APIRegisterVmInstanceEvent) before bus.publish(event) and chain.next() so
the failure is propagated back to the caller when registerVmInstance fails.
Comment on lines
1870
to
2134
| private void registerVmInstance(APIRegisterVmInstanceMsg msg, ReturnValueCompletion<VmInstanceInventory> completion) { | ||
| FlowChain chain = new ShareFlowChain(); | ||
| chain.setName("register-vm-from-metadata"); | ||
| chain.then(new ShareFlow() { | ||
| VmMetadata vmMetadata; | ||
| VmInstanceInventory vmInstanceInventory; | ||
|
|
||
| @Override | ||
| public void setup() { | ||
| flow(new NoRollbackFlow() { | ||
| String __name__ = "read-metadata"; | ||
|
|
||
| @Override | ||
| public void run(FlowTrigger trigger, Map data) { | ||
| ReadVmInstanceMetadataOnHypervisorMsg umsg = new ReadVmInstanceMetadataOnHypervisorMsg(); | ||
| umsg.setHostUuid(msg.getHostUuid()); | ||
| umsg.setMetadataPath(msg.getMetadataPath()); | ||
| bus.makeTargetServiceIdByResourceUuid(umsg, HostConstant.SERVICE_ID, msg.getHostUuid()); | ||
| bus.send(umsg, new CloudBusCallBack(msg) { | ||
| @Override | ||
| public void run(MessageReply r) { | ||
| if (!r.isSuccess()) { | ||
| trigger.fail(operr("failed to update vm[uuid=%s] on hypervisor.", | ||
| self.getUuid()).withCause(r.getError())); | ||
| return; | ||
| } | ||
| ReadVmInstanceMetadataOnHypervisorReply reply = r.castReply(); | ||
| vmMetadata = JSONObjectUtil.toObject(reply.getMetadata(), VmMetadata.class); | ||
| trigger.next(); | ||
| } | ||
| }); | ||
| } | ||
| }); | ||
|
|
||
| flow(new NoRollbackFlow() { | ||
| String __name__ = "register-volume"; | ||
|
|
||
| @Override | ||
| public void run(FlowTrigger trigger, Map data) { | ||
| List<String> volumesString = vmMetadata.volumeVOs; | ||
|
|
||
| List<VolumeVO> volumes = new ArrayList<>(); | ||
| volumesString.forEach(v -> volumes.add(JSONObjectUtil.toObject(v, VolumeVO.class))); | ||
|
|
||
| List<VolumeVO> newVolumes = new ArrayList<>(); | ||
| volumes.forEach(v -> { | ||
| VolumeVO vo = new VolumeVO(); | ||
| // vo.setRootImageUuid(vo.getRootImageUuid()); | ||
| vo.setAccountUuid(msg.getSession().getAccountUuid()); | ||
| vo.setPrimaryStorageUuid(msg.getPrimaryStorageUuid()); | ||
| vo.setInstallPath(v.getInstallPath()); | ||
|
|
||
| vo.setCreateDate(v.getCreateDate()); | ||
| vo.setDescription(v.getDescription()); | ||
| vo.setName(v.getName()); | ||
| vo.setSize(v.getSize()); | ||
| vo.setActualSize(v.getActualSize()); | ||
| vo.setState(v.getState()); | ||
| vo.setUuid(v.getUuid()); | ||
| vo.setVmInstanceUuid(v.getVmInstanceUuid()); | ||
| vo.setType(v.getType()); | ||
| vo.setCreateDate(v.getCreateDate()); | ||
| vo.setLastOpDate(v.getLastOpDate()); | ||
| vo.setDeviceId(v.getDeviceId()); | ||
| vo.setStatus(v.getStatus()); | ||
| vo.setFormat(v.getFormat()); | ||
| vo.setShareable(v.isShareable()); | ||
| vo.setVolumeQos(v.getVolumeQos()); | ||
| vo.setLastDetachDate(v.getLastDetachDate()); | ||
| vo.setLastVmInstanceUuid(v.getLastVmInstanceUuid()); | ||
| vo.setLastAttachDate(v.getLastAttachDate()); | ||
| vo.setProtocol(v.getProtocol()); | ||
| newVolumes.add(vo); | ||
| }); | ||
| dbf.persistCollection(newVolumes); | ||
| trigger.next(); | ||
| } | ||
| }); | ||
|
|
||
| flow(new NoRollbackFlow() { | ||
| String __name__ = "register-snapshot"; | ||
|
|
||
| @Override | ||
| public void run(FlowTrigger trigger, Map data) { | ||
| // 快照 | ||
| vmMetadata.volumeSnapshots.forEach((volumeUuid, snapshotList) -> { | ||
| // 一个 volume 有多个快照树 | ||
| // key = treeuuid | ||
| // value = snapshosts | ||
| Map<String, List<VolumeSnapshotInventory>> snapshotsByTreeUuid = new HashMap<>(); | ||
| snapshotList.forEach(snapshot -> { | ||
| VolumeSnapshotInventory inv = JSONObjectUtil.toObject(snapshot, VolumeSnapshotInventory.class); | ||
| if (snapshotsByTreeUuid.containsKey(inv.getTreeUuid())) { | ||
| snapshotsByTreeUuid.get(inv.getTreeUuid()).add(inv); | ||
| } else { | ||
| snapshotsByTreeUuid.put(inv.getTreeUuid(), new ArrayList<>()); | ||
| snapshotsByTreeUuid.get(inv.getTreeUuid()).add(inv); | ||
| } | ||
| }); | ||
|
|
||
| // 遍历每一颗树 | ||
| snapshotsByTreeUuid.forEach((treeUuid, snapshots) -> { | ||
| //构建快照树 | ||
| VolumeSnapshotTree tree = VolumeSnapshotTree.fromInventories(snapshots); | ||
| // 层级遍历 快照 | ||
| List<VolumeSnapshotInventory> levelOrderTraversals = tree.levelOrderTraversal(); | ||
| // 判断当前树有没有 latest 节点 | ||
| boolean treeIsCurrent = levelOrderTraversals.stream().anyMatch(VolumeSnapshotInventory::isLatest); | ||
|
|
||
| // 先创建快照树，VolumeSnapshotVO 外键依赖 VolumeSnapshotTreeVO | ||
| VolumeSnapshotTreeVO newTree = new VolumeSnapshotTreeVO(); | ||
| newTree.setCurrent(treeIsCurrent); | ||
| newTree.setVolumeUuid(volumeUuid); | ||
| newTree.setUuid(treeUuid); | ||
| newTree.setStatus(VolumeSnapshotTreeStatus.Completed); | ||
| dbf.persist(newTree); | ||
|
|
||
| // 按照层级遍历的快照构建VolumeSnapshotTreeVO | ||
| levelOrderTraversals.forEach(snapshot -> { | ||
| VolumeSnapshotVO vo = new VolumeSnapshotVO(); | ||
| vo.setPrimaryStorageUuid(msg.getPrimaryStorageUuid()); | ||
| vo.setPrimaryStorageInstallPath(snapshot.getPrimaryStorageInstallPath()); | ||
|
|
||
| vo.setName(snapshot.getName()); | ||
| vo.setCreateDate(snapshot.getCreateDate()); | ||
| vo.setDescription(snapshot.getDescription()); | ||
| vo.setLastOpDate(snapshot.getLastOpDate()); | ||
| vo.setParentUuid(snapshot.getParentUuid()); | ||
| vo.setState(VolumeSnapshotState.valueOf(snapshot.getState())); | ||
| vo.setType(snapshot.getType()); | ||
| vo.setVolumeUuid(snapshot.getVolumeUuid()); | ||
| vo.setFormat(snapshot.getFormat()); | ||
| vo.setUuid(snapshot.getUuid()); | ||
| vo.setStatus(VolumeSnapshotStatus.valueOf(snapshot.getStatus())); | ||
| vo.setLatest(snapshot.isLatest()); | ||
| vo.setSize(snapshot.getSize()); | ||
| vo.setVolumeType(snapshot.getVolumeType()); | ||
| vo.setTreeUuid(snapshot.getTreeUuid()); | ||
| vo.setDistance(snapshot.getDistance()); | ||
| dbf.persist(vo); | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| // 快照组 | ||
| List<VolumeSnapshotGroupVO> newGroups = new ArrayList<>(); | ||
| vmMetadata.volumeSnapshotGroupVO.forEach(group -> { | ||
| VolumeSnapshotGroupVO vo = JSONObjectUtil.toObject(group, VolumeSnapshotGroupVO.class); | ||
| vo.setAccountUuid(msg.getSession().getAccountUuid()); | ||
| newGroups.add(vo); | ||
| }); | ||
| dbf.persistCollection(newGroups); | ||
|
|
||
| // 快照组ref | ||
| List<VolumeSnapshotGroupRefVO> newGroupRefs = new ArrayList<>(); | ||
| vmMetadata.volumeSnapshotGroupRefVO.forEach(group -> { | ||
| VolumeSnapshotGroupRefVO vo = JSONObjectUtil.toObject(group, VolumeSnapshotGroupRefVO.class); | ||
| newGroupRefs.add(vo); | ||
| }); | ||
| dbf.persistCollection(newGroupRefs); | ||
|
|
||
| trigger.next(); | ||
| } | ||
| }); | ||
|
|
||
| flow(new NoRollbackFlow() { | ||
| String __name__ = "register-vmInstance"; | ||
|
|
||
| @Override | ||
| public void run(FlowTrigger trigger, Map data) { | ||
| VmInstanceVO metaVm = JSONObjectUtil.toObject(vmMetadata.vmInstanceVO, VmInstanceVO.class); | ||
| VmInstanceVO newVm = new VmInstanceVO(); | ||
|
|
||
| newVm.setClusterUuid(msg.getClusterUuid()); | ||
| newVm.setZoneUuid(msg.getZoneUuid()); | ||
| newVm.setHostUuid(msg.getHostUuid()); | ||
| // 寻找有没有cache的tag lv 构建imageCache | ||
| // newVm.setImageUuid(); | ||
|
|
||
| newVm.setUuid(metaVm.getUuid()); | ||
| newVm.setName(metaVm.getName()); | ||
| newVm.setDescription(metaVm.getDescription()); | ||
| newVm.setType(metaVm.getType()); | ||
| newVm.setHypervisorType(metaVm.getHypervisorType()); | ||
| newVm.setCreateDate(metaVm.getCreateDate()); | ||
| newVm.setLastOpDate(metaVm.getLastOpDate()); | ||
| newVm.setState(metaVm.getState()); | ||
| newVm.setRootVolumeUuid(metaVm.getRootVolumeUuid()); | ||
| newVm.setInternalId(metaVm.getInternalId()); | ||
| newVm.setCpuNum(metaVm.getCpuNum()); | ||
| newVm.setCpuSpeed(metaVm.getCpuSpeed()); | ||
| newVm.setMemorySize(metaVm.getMemorySize()); | ||
| newVm.setReservedMemorySize(metaVm.getReservedMemorySize()); | ||
| newVm.setAllocatorStrategy(metaVm.getAllocatorStrategy()); | ||
| newVm.setPlatform(metaVm.getPlatform()); | ||
| newVm.setArchitecture(metaVm.getArchitecture()); | ||
| newVm.setGuestOsType(metaVm.getGuestOsType()); | ||
| dbf.persist(newVm); | ||
| vmInstanceInventory = VmInstanceInventory.valueOf(newVm); | ||
| trigger.next(); | ||
| // List<String> vmSystemTags = vmMetadata.vmSystemTags; | ||
| // List<String> vmResourceConfigs = vmMetadata.vmResourceConfigs; | ||
| // | ||
| // try { | ||
| // List<SystemTag> systemTags = getResourceSystemTagFromSystem(VmInstanceVO.class.getSimpleName()); | ||
| // List<GlobalConfig> resourceConfigs = getResourceConfigFromSystem(VmInstanceVO.class.getSimpleName()); | ||
| // | ||
| // List<SystemTagVO> tagVOS = new ArrayList<>(); | ||
| // vmSystemTags.forEach(tag -> { | ||
| // List<String> info = asList(tag.split("_")); | ||
| // String t = info.get(0); | ||
| // Boolean inherent = Boolean.valueOf(info.get(1)); | ||
| // String type = info.get(2); | ||
| // systemTags.forEach(it -> { | ||
| // if (!it.isMatch(t)) { | ||
| // return; | ||
| // } | ||
| // SystemTagVO vo = new SystemTagVO(); | ||
| // vo.setTag(t); | ||
| // vo.setType(TagType.valueOf(type)); | ||
| // vo.setInherent(inherent); | ||
| // vo.setResourceType(VmInstanceVO.class.getSimpleName()); | ||
| // vo.setResourceUuid(newVm.getUuid()); | ||
| // tagVOS.add(vo); | ||
| // }); | ||
| // }); | ||
| // | ||
| // List<ResourceConfigVO> configVOS = new ArrayList<>(); | ||
| // vmResourceConfigs.forEach(tag -> { | ||
| // List<String> info = asList(tag.split("_")); | ||
| // String identity = info.get(0); | ||
| // String value = info.get(1); | ||
| // resourceConfigs.forEach(it -> { | ||
| // if (it.getIdentity() == identity) { | ||
| // return; | ||
| // } | ||
| // ResourceConfigVO vo = new ResourceConfigVO(); | ||
| // vo.setCategory(identity); | ||
| // vo.setName(identity); | ||
| // vo.setValue(value); | ||
| // vo.setResourceType(VmInstanceVO.class.getSimpleName()); | ||
| // vo.setResourceUuid(newVm.getUuid()); | ||
| // configVOS.add(vo); | ||
| // }); | ||
| // }); | ||
| // } catch (IllegalAccessException | InstantiationException e) { | ||
| // throw new RuntimeException(e); | ||
| // } | ||
| } | ||
| }); | ||
|
|
||
| done(new FlowDoneHandler(completion) { | ||
| @Override | ||
| public void handle(Map data) { | ||
| completion.success(vmInstanceInventory); | ||
| } | ||
| }); | ||
|
|
||
| error(new FlowErrorHandler(msg) { | ||
| @Override | ||
| public void handle(ErrorCode errCode, Map data) { | ||
| completion.fail(errCode); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
注册流程缺少回滚会留下脏数据。
当前全是 NoRollbackFlow，任一步失败都会留下已持久化的 Volume/Snapshot/VM/Group 记录。建议增加可回滚 flow / 补偿删除，或统一事务回滚。
🤖 Prompt for AI Agents
In `@storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java`
around lines 1870 - 2134, The registerVmInstance flow chain currently uses only
NoRollbackFlow steps (read-metadata, register-volume, register-snapshot,
register-vmInstance) and persists entities via dbf.persist/dbf.persistCollection
without compensation, leaving stale
VolumeVO/VolumeSnapshotVO/VolumeSnapshotTreeVO/VolumeSnapshotGroupVO/VolumeSnapshotGroupRefVO/VmInstanceVO
rows on failure; change the persisting flows to support rollback by either (a)
switch those NoRollbackFlow instances to Flow and implement rollback handlers
that delete any entities created in that flow (track created VO UUIDs in
flow-local lists) or (b) wrap the whole chain in a single
transactional/compensating flow that deletes all created entities on error (use
the collected created lists from register-volume, register-snapshot,
register-vmInstance and remove via dbf.remove or appropriate DAO methods in the
Flow.rollback), ensuring completion/error handlers remain unchanged so
completion.fail is called on error.
Resolves: ZSV-1 Change-Id: I746a6d7266686d637a6d76756861636a74766b74
MatheMatrix
force-pushed
the
sync/tao.gan/res-vm-ZSV-1@@3
branch
from
be2f06d to
47a2c23
Compare
There was a problem hiding this comment.
Actionable comments posted: 20
🤖 Fix all issues with AI agents
In
`@compute/src/main/java/org/zstack/compute/vm/metadata/MetadataVersionConverter.java`:
- Around line 19-26: The code can NPE when VmMetadataDTO.getSchemaVersion()
returns null; in upgradeToCurrentVersion(VmMetadataDTO) ensure inputVersion is
null-safe by checking if input.getSchemaVersion() == null and treating it as an
older/default version (e.g., set inputVersion to a safe default like "v0" or
handle it as "older than CURRENT_VERSION") before any
CURRENT_VERSION.equals(...) or compareVersion(...) calls; alternatively make
compareVersion(String, String) null-safe (handle null/empty first) so calls from
upgradeToCurrentVersion and any other callers cannot throw NPEs.
- Around line 1-4: Remove the duplicate/incorrect package declaration and make
the file use the single correct package declaration "com.zstack.core.vmmetadata"
in MetadataVersionConverter.java; delete the "package
org.zstack.compute.vm.metadata;" line, ensure the remaining package line is
"package com.zstack.core.vmmetadata;", and then update any imports/usages across
the codebase that reference org.zstack.compute.vm.metadata to the new package
com.zstack.core.vmmetadata so compilation succeeds.
In `@compute/src/main/java/org/zstack/compute/vm/metadata/ResourceConfigDTO.java`:
- Around line 3-4: ResourceConfigDTO is empty but code calls setConfigKey,
setConfigValue, setIsSensitive from VmMetadataConverter.convertResourceConfig;
add private fields configKey (String), configValue (String) and isSensitive
(boolean) to ResourceConfigDTO and implement corresponding getters and setters
(getConfigKey/setConfigKey, getConfigValue/setConfigValue,
isSensitive/setIsSensitive) so the converter can compile and populate these
values.
In
`@compute/src/main/java/org/zstack/compute/vm/metadata/SensitiveFieldHandler.java`:
- Around line 30-32: The isSensitiveField method currently calls
SENSITIVE_PATTERN.matcher(key) without guarding against null and can throw NPE;
update isSensitiveField(String key) to check for null (or empty) first and
return false (or appropriate non-sensitive default) when key is null before
calling SENSITIVE_PATTERN.matcher(key). Refer to the isSensitiveField method and
SENSITIVE_PATTERN constant to locate the change and add the null check as the
first statement in the method.
- Around line 18-25: SENSITIVE_TAGS is declared but never initialized which
causes a NullPointerException in the static initializer that builds
SENSITIVE_PATTERN; initialize SENSITIVE_TAGS (e.g., as an immutable/empty List
or populated from config) before the static block and guard the regex
construction in the static initializer so it handles an empty list (build a safe
regex or skip Pattern.compile if no tags). Update the static block referencing
SENSITIVE_TAGS and SENSITIVE_PATTERN in SensitiveFieldHandler to use the
initialized list or a fallback to avoid NPEs.
- Around line 3-8: SensitiveFieldHandler is using several types/constants that
are not imported; add import declarations for GlobalConfig,
VM_METADATA_SENSITIVE_KEY, AESCipher, AlertManager, and SystemTagDTO at the top
of SensitiveFieldHandler.java (or the correct packages in your project) so the
compiler can resolve those symbols used in methods of SensitiveFieldHandler and
any references to the VM_METADATA_SENSITIVE_KEY constant, AESCipher encryption
calls, AlertManager alerts, and SystemTagDTO handling.
In `@compute/src/main/java/org/zstack/compute/vm/metadata/SystemTagDTO.java`:
- Around line 3-4: SystemTagDTO is empty but other classes call its
getters/setters; add private fields String tagKey, String tagValue and boolean
isSensitive, and implement the matching public accessors used elsewhere:
getTagKey(), setTagKey(String), getTagValue(), setTagValue(String), and
setIsSensitive(boolean) plus a boolean getter (getIsSensitive() or
isSensitive()) so calls from VmMetadataConverter.convertSystemTag() and
SensitiveFieldHandler compile; keep standard Java bean naming and simple
field-backed implementations inside class SystemTagDTO.
In
`@compute/src/main/java/org/zstack/compute/vm/metadata/VmMetadataConverter.java`:
- Around line 27-139: The convert(...) helpers currently call stream() or
getSnapshotRefs() on input lists/fields that may be null, causing NPEs; update
convert, convertVmNic, convertVolume, convertSnapshot, convertSnapshotGroup (and
any other converter methods) to defensively treat null lists by using
Collections.emptyList() (or Optional.ofNullable(...).orElse(...)) before
streaming, and in convertSnapshotGroup ensure vo.getSnapshotRefs() is
null-checked/empty-defaulted before mapping
VolumeSnapshotGroupRefVO::getSnapshotUuid so snapshotUuidList is an empty list
instead of throwing.
- Around line 1-4: Remove the duplicate package declaration by deleting "package
com.zstack.core.vmmetadata;" so VmMetadataConverter has only the correct
package; add missing imports for VolumeSnapshotGroupRefVO,
VolumeSnapshotReferenceVO, and VolumeSnapshotReferenceTreeVO (use their correct
fully-qualified class names) and remove or replace any references to the
non-existent CoreMemorySnapshotConfigs class (either import the correct
replacement or delete related logic) in the VmMetadataConverter class; and add
null checks before any .stream() calls in the conversion methods (e.g.,
convertVmNic, convertVolume, convertSnapshot and similar converters) to return
an empty collection or handle null input safely to avoid NullPointerException.
In `@compute/src/main/java/org/zstack/compute/vm/metadata/VmMetadataDTO.java`:
- Around line 7-42: VmMetadataDTO currently lacks getter/setter methods and a
serialVersionUID, causing compilation and deserialization failures; add a
private static final long serialVersionUID and implement public JavaBean getters
and setters for every private field declared in VmMetadataDTO (schemaVersion,
createTime, name, description, hypervisorType, guestOsType, architecture,
platform, type, rootVolumeUuid, imageUuid, memorySize, reservedMemorySize,
cpuNum, cpuSpeed, volumeList, nicList, snapshotList, snapshotGroupList,
systemTags, resourceConfigs). Also ensure the DTO exposes the exact accessor
names expected by callers (e.g., provide methods matching usages in
VmMetadataConverter/VmMetadataSerializer such as setVmUuid/getVmUuid and
setNumCpus/getNumCpus or add delegating methods that map to the existing fields)
so JSONObjectUtil and converters can bind properties correctly.
In
`@compute/src/main/java/org/zstack/compute/vm/metadata/VmMetadataSerializer.java`:
- Around line 76-81: The loop in VmMetadataSerializer that decrypts sensitive
fields calls dto.getResourceConfigs().forEach(...) without null-check, causing
NPE; before iterating, guard the call by checking dto.getResourceConfigs() !=
null (or use Optional/Collections.emptyList()) so you only call forEach when the
list is non-null, and keep using SensitiveFieldHandler.isSensitiveField(...) and
SensitiveFieldHandler.decrypt(...) inside the guarded block.
- Around line 62-71: In VmMetadataSerializer, the comparison
storedChecksum.equals(calculatedChecksum) can NPE if storedChecksum is null;
change the check to first handle null (e.g., treat null as mismatch) by
replacing the equality test with a null-safe check (for example: if
(storedChecksum == null || !storedChecksum.equals(calculatedChecksum)) ) so that
getSha256Checksum() being null triggers the same AlertManager.alert and
SecurityException path; reference storedChecksum, getSha256Checksum(),
calculatedChecksum, Sha256Utils.hash and JSONObjectUtil.toJsonString to locate
the code to update.
- Around line 22-31: VmMetadataSerializer.serialize currently calls
dto.getSystemTags() and dto.getResourceConfigs() without null checks which can
cause NPEs; update serialize to null-check VmMetadataDTO.getSystemTags() before
passing to SensitiveFieldHandler.filterSensitiveTags and only call
dto.setSystemTags(...) when non-null (or set an empty list if desired), and
null-check VmMetadataDTO.getResourceConfigs() before iterating: if non-null,
iterate and for each config check
SensitiveFieldHandler.isSensitiveField(config.getConfigKey()) then set
config.setConfigValue(SensitiveFieldHandler.encrypt(config.getConfigValue()));
ensure all accessors (getSystemTags, getResourceConfigs, config.getConfigKey,
config.getConfigValue) are guarded so no method is invoked on null.
- Around line 3-6: The imports in VmMetadataSerializer use the wrong base
packages; replace com.zstack.* imports with the project's canonical packages so
the class imports GlobalConfig from org.zstack.core.config (use
org.zstack.core.config.GlobalConfig), AlertManager from org.zstack.core.alert
(org.zstack.core.alert.AlertManager), and the JSON/codec utilities from
org.zstack.utils (org.zstack.utils.json.JSONObjectUtil and
org.zstack.utils.codec.Sha256Utils) to match project conventions.
In `@compute/src/main/java/org/zstack/compute/vm/metadata/VolumeMetadataDTO.java`:
- Around line 47-53: The class VolumeMetadataDTO is missing the String field
referenced by getPrimaryStorageUuid() and setPrimaryStorageUuid(); add a private
static String primaryStorageUuid declaration (matching the static access in
those methods) alongside the other DTO fields in VolumeMetadataDTO so the getter
and setter compile correctly, and ensure any intended visibility (private) and
default initialization are consistent with the other fields.
- Around line 6-26: All fields in VolumeMetadataDTO (e.g., description, name,
vmInstanceUuid, rootImageUuid, diskOfferingUuid, format, installPath, type,
size, actualSize, deviceId, state, status, createDate, lastOpDate,
lastDetachDate, lastAttachDate, lastVmInstanceUuid, isShareable, volumeQos,
protocol) are incorrectly declared static; change each to an instance field
(remove the static modifier) and convert any static getter/setter methods to
instance methods on VolumeMetadataDTO so each DTO holds independent state; after
that, search for call sites that reference class-level access
(VolumeMetadataDTO.field or VolumeMetadataDTO.getX()) and update them to use an
instance (dto.getX()/dto.setX()) or adjust construction, and run tests to ensure
no serialization/ORM annotations rely on static members.
In `@compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java`:
- Around line 9489-9499: The code in VmInstanceBase contains a residual
DTO/serializer block referencing undefined symbols (vmUuid, volumeVos,
snapshotVos, groupVos, tagVos, configVos, snapshotDto) and won't compile; either
remove the entire DTO/serializer/supplementSnapshotReference lines (the calls to
VmMetadataConverter.convert, VmMetadataSerializer.serialize and
VmMetadataConverter.supplementSnapshotReference) if metadata serialization is no
longer needed, or properly obtain and declare the missing variables before use
(ensure vmUuid is set to the current VM UUID, populate
volumeVos/snapshotVos/groupVos/tagVos/configVos from the DB or method context,
and get snapshotDto from the snapshot handling code) so the calls in
VmInstanceBase compile and have valid inputs.
In `@header/src/main/java/org/zstack/header/vm/VmMetadata.java`:
- Around line 33-36: The two list fields in VmMetadata—volumeSnapshotGroupVO and
volumeSnapshotGroupRefVO—are named in singular form while their type is List;
rename them to plural (e.g., volumeSnapshotGroupsVO and
volumeSnapshotGroupRefsVO or volumeSnapshotGroupVOs and
volumeSnapshotGroupRefVOs) and update all references/usages (constructors,
getters/setters, JSON/mapping annotations, code that reads/writes these fields)
to use the new names to keep naming consistent with their List semantics; ensure
any serialization names or API contracts are adjusted if needed.
In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmBackend.java`:
- Around line 3814-3824: The CloudBusCallBack currently constructs an
UpdateVmInstanceMetadataOnPrimaryStorageReply but never invokes the provided
ReturnValueCompletion<UpdateVmInstanceMetadataOnPrimaryStorageReply> completion,
so the caller is never notified; inside the CloudBusCallBack.run(MessageReply r)
use completion.success(reply) when r.isSuccess() and completion.fail(...) with
the same Platform-operr-created error when !r.isSuccess(), and then still send
bus.reply(msg, reply) if needed; ensure you reference the existing
UpdateVmInstanceMetadataOnPrimaryStorageReply, CloudBusCallBack, completion
parameter and the current error construction
(Platform.operr(...).withCause(r.getError())) so the completion and bus.reply
behavior are consistent and the caller receives the finish notification.
In `@storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java`:
- Around line 1886-1891: The error message in run(MessageReply r) incorrectly
logs self.getUuid() (PrimaryStorageBase) instead of the VM's UUID; update the
operr call in the failure branch to reference the VM's UUID from the surrounding
VM context (e.g., use the vm instance or VM inventory variable such as
vm.getUuid() or vmInventory.getUuid(), or extract it from VmInstanceSpec/VmSpec
if those are in scope) so the error message correctly identifies the VM; ensure
the chosen VM UUID variable is in scope and compile-clean.
🧹 Nitpick comments (9)
header/src/main/java/org/zstack/header/storage/primary/RegisterVmInstanceException.java (1)
3-9: 接口命名不当：使用了 "Exception" 后缀但并非异常类。根据编码规范，
Exception后缀应保留给实际的异常类（继承自Exception或Throwable）。当前RegisterVmInstanceException是一个普通接口，定义了 VM 注册相关的扩展方法，命名可能造成误解。建议重命名为更能体现其职责的名称，例如：
RegisterVmInstanceExtensionRegisterVmInstanceHandlerRegisterVmInstancePathUpdater♻️ 建议的重命名
-public interface RegisterVmInstanceException { +public interface RegisterVmInstanceExtension { String updateVolumeInstallPath(String installPath); String updateVolumeSnapshotInstallPath(String installPath); PrimaryStorageType getPrimaryStorageType(); }As per coding guidelines: "异常类应以
Exception结尾"，此规则隐含了Exception后缀应仅用于实际的异常类。compute/src/main/java/org/zstack/compute/vm/metadata/SnapshotGroupMetadataDTO.java (1)
7-13: 字段可见性不一致
refList字段使用包级私有访问（无修饰符），而其他字段都是public。建议保持一致性。♻️ 建议统一字段可见性
public Timestamp createDate; public Timestamp lastOpDate; - List<SnapshotGroupRefMetadataDTO> refList; + public List<SnapshotGroupRefMetadataDTO> refList;或者，如果需要封装，将所有字段改为 private 并添加 getter/setter。
header/src/main/java/org/zstack/header/storage/primary/APIGetVmInstanceMetadataFromPrimaryStorageReply.java (1)
22-25:__example__()方法应填充示例数据以生成更完整的 API 文档。当前实现返回空的 reply 对象，没有设置
vmInstanceMetadata的示例值。参考其他类似的 Reply 类（如APIRegisterVmInstanceReply），建议添加示例数据以提升 API 文档的可读性。📝 建议的修改
public static APIGetVmInstanceMetadataFromPrimaryStorageReply __example__() { APIGetVmInstanceMetadataFromPrimaryStorageReply reply = new APIGetVmInstanceMetadataFromPrimaryStorageReply(); + reply.setVmInstanceMetadata(java.util.Arrays.asList("{\"schemaVersion\":\"v1.0\",\"name\":\"example-vm\"}")); return reply; }header/src/main/java/org/zstack/header/storage/primary/APIGetVmInstanceMetadataFromPrimaryStorageMsg.java (1)
15-16: 建议：字段名uuid语义不够明确，可考虑改为primaryStorageUuid。当前类名
APIGetVmInstanceMetadataFromPrimaryStorageMsg同时涉及 VmInstance 和 PrimaryStorage，字段名仅用uuid容易产生歧义。参考同 PR 中APIRegisterVmInstanceMsg.java使用primaryStorageUuid命名，更清晰地表达字段用途。♻️ 建议重命名
public class APIGetVmInstanceMetadataFromPrimaryStorageMsg extends APISyncCallMessage implements PrimaryStorageMessage { `@APIParam`(resourceType = PrimaryStorageVO.class) - private String uuid; + private String primaryStorageUuid; - public String getUuid() { - return uuid; + public String getPrimaryStorageUuid() { + return primaryStorageUuid; } - public void setUuid(String uuid) { - this.uuid = uuid; + public void setPrimaryStorageUuid(String primaryStorageUuid) { + this.primaryStorageUuid = primaryStorageUuid; } `@Override` - public String getPrimaryStorageUuid() { - return uuid; + public String getPrimaryStorageUuid() { + return primaryStorageUuid; } public static APIGetVmInstanceMetadataFromPrimaryStorageMsg __example__() { APIGetVmInstanceMetadataFromPrimaryStorageMsg msg = new APIGetVmInstanceMetadataFromPrimaryStorageMsg(); - msg.setUuid(uuid()); + msg.setPrimaryStorageUuid(uuid()); return msg; } }compute/src/main/java/org/zstack/compute/vm/metadata/MetadataVersionConverter.java (1)
87-99:compareVersion缺少异常处理。
Integer.parseInt可能因非数字版本号抛出NumberFormatException，建议添加防御性处理。♻️ 建议改进
private static int compareVersion(String version1, String version2) { + if (version1 == null || version2 == null) { + throw new IllegalArgumentException("版本号不能为空"); + } String[] v1 = version1.replace("v", "").split("\\."); String[] v2 = version2.replace("v", "").split("\\."); int len = Math.max(v1.length, v2.length); for (int i = 0; i < len; i++) { - int num1 = i < v1.length ? Integer.parseInt(v1[i]) : 0; - int num2 = i < v2.length ? Integer.parseInt(v2[i]) : 0; + int num1 = 0; + int num2 = 0; + try { + num1 = i < v1.length ? Integer.parseInt(v1[i]) : 0; + num2 = i < v2.length ? Integer.parseInt(v2[i]) : 0; + } catch (NumberFormatException e) { + throw new IllegalArgumentException("版本号格式错误: " + version1 + " 或 " + version2, e); + } if (num1 != num2) { return num1 - num2; } } return 0; }storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java (2)
1867-1867: 建议使用FlowChainBuilder工厂方法。项目中其他地方（如 Line 1260）使用
FlowChainBuilder.newShareFlowChain()创建 ShareFlowChain，保持一致性有助于代码可维护性。♻️ 建议修改
- FlowChain chain = new ShareFlowChain(); + FlowChain chain = FlowChainBuilder.newShareFlowChain();
2133-2152: 使用已弃用的newInstance()方法且效率较低。
clazz.newInstance()在 Java 9+ 已被弃用，应使用clazz.getDeclaredConstructor().newInstance()。- 每个字段都创建新实例效率低下，应在循环外创建一次。
♻️ 建议改进
private List<SystemTag> getResourceSystemTagFromSystem(String resourceType) throws IllegalAccessException, InstantiationException { List<SystemTag> systemTags = new ArrayList<>(); Set<Class<?>> classes = BeanUtils.reflections.getTypesAnnotatedWith(TagDefinition.class); for (Class clazz : classes) { + Object instance = null; + try { + instance = clazz.getDeclaredConstructor().newInstance(); + } catch (Exception e) { + continue; + } Field[] fields = clazz.getDeclaredFields(); for (Field field : fields) { if (!SystemTag.class.isAssignableFrom(field.getType())) { continue; } - SystemTag systemTag = (SystemTag) field.get(clazz.newInstance()); + SystemTag systemTag = (SystemTag) field.get(instance); if (resourceType.equals(systemTag.getResourceClass().getName())) { systemTags.add(systemTag); } } } return systemTags; }sdk/src/main/java/org/zstack/sdk/RegisterVmInstanceAction.java (2)
5-5: 冗余的导入声明。
import org.zstack.sdk.*是多余的，因为当前类已经在org.zstack.sdk包中。♻️ 建议修改
import java.util.HashMap; import java.util.Map; -import org.zstack.sdk.*;
43-47: 建议使用泛型参数类型。
systemTags和userTags使用原始List类型，建议使用List<String>以提高类型安全性。♻️ 建议修改
`@Param`(required = false) - public java.util.List systemTags; + public java.util.List<String> systemTags; `@Param`(required = false) - public java.util.List userTags; + public java.util.List<String> userTags;
Comment on lines
+1
to
+4
| package org.zstack.compute.vm.metadata; | ||
|
|
||
| package com.zstack.core.vmmetadata; | ||
|
|
There was a problem hiding this comment.
重复的包声明导致编译错误。
文件包含两个包声明（Line 1 和 Line 3），这会导致编译失败。
🐛 建议修复
package org.zstack.compute.vm.metadata;
-package com.zstack.core.vmmetadata;
-
import com.zstack.core.GlobalConfig;同时需要修正导入路径：
-import com.zstack.core.GlobalConfig;
-import com.zstack.utils.AssertUtils;
+import org.zstack.core.config.GlobalConfig;
+import org.zstack.utils.AssertUtils;🤖 Prompt for AI Agents
In
`@compute/src/main/java/org/zstack/compute/vm/metadata/MetadataVersionConverter.java`
around lines 1 - 4, Remove the duplicate/incorrect package declaration and make
the file use the single correct package declaration "com.zstack.core.vmmetadata"
in MetadataVersionConverter.java; delete the "package
org.zstack.compute.vm.metadata;" line, ensure the remaining package line is
"package com.zstack.core.vmmetadata;", and then update any imports/usages across
the codebase that reference org.zstack.compute.vm.metadata to the new package
com.zstack.core.vmmetadata so compilation succeeds.
Comment on lines
+19
to
+26
| public static VmMetadataDTO upgradeToCurrentVersion(VmMetadataDTO input) { | ||
| AssertUtils.notNull(input, "VmMetadataDTO cannot be null"); | ||
| String inputVersion = input.getSchemaVersion(); | ||
|
|
||
| // 版本相同，无需升级 | ||
| if (CURRENT_VERSION.equals(inputVersion)) { | ||
| return input; | ||
| } |
There was a problem hiding this comment.
inputVersion 可能为 null，导致 NPE。
如果 DTO 的 schemaVersion 未设置，input.getSchemaVersion() 返回 null，后续 CURRENT_VERSION.equals(inputVersion) 虽然安全，但 compareVersion(inputVersion, CURRENT_VERSION) 在 Line 88 会因 version1.replace("v", "") 抛出 NPE。
🛡️ 建议修复
public static VmMetadataDTO upgradeToCurrentVersion(VmMetadataDTO input) {
AssertUtils.notNull(input, "VmMetadataDTO cannot be null");
String inputVersion = input.getSchemaVersion();
+ // 版本为空，视为最旧版本需要升级
+ if (inputVersion == null) {
+ inputVersion = "v1.0";
+ }
+
// 版本相同，无需升级
if (CURRENT_VERSION.equals(inputVersion)) {
return input;
}📝 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
| public static VmMetadataDTO upgradeToCurrentVersion(VmMetadataDTO input) { | |
| AssertUtils.notNull(input, "VmMetadataDTO cannot be null"); | |
| String inputVersion = input.getSchemaVersion(); | |
| // 版本相同，无需升级 | |
| if (CURRENT_VERSION.equals(inputVersion)) { | |
| return input; | |
| } | |
| public static VmMetadataDTO upgradeToCurrentVersion(VmMetadataDTO input) { | |
| AssertUtils.notNull(input, "VmMetadataDTO cannot be null"); | |
| String inputVersion = input.getSchemaVersion(); | |
| // 版本为空，视为最旧版本需要升级 | |
| if (inputVersion == null) { | |
| inputVersion = "v1.0"; | |
| } | |
| // 版本相同，无需升级 | |
| if (CURRENT_VERSION.equals(inputVersion)) { | |
| return input; | |
| } |
🤖 Prompt for AI Agents
In
`@compute/src/main/java/org/zstack/compute/vm/metadata/MetadataVersionConverter.java`
around lines 19 - 26, The code can NPE when VmMetadataDTO.getSchemaVersion()
returns null; in upgradeToCurrentVersion(VmMetadataDTO) ensure inputVersion is
null-safe by checking if input.getSchemaVersion() == null and treating it as an
older/default version (e.g., set inputVersion to a safe default like "v0" or
handle it as "older than CURRENT_VERSION") before any
CURRENT_VERSION.equals(...) or compareVersion(...) calls; alternatively make
compareVersion(String, String) null-safe (handle null/empty first) so calls from
upgradeToCurrentVersion and any other callers cannot throw NPEs.
Comment on lines
+3
to
+4
| public class ResourceConfigDTO { | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for ResourceConfigDTO usage and related context
echo "=== ResourceConfigDTO Usage ==="
rg -n "ResourceConfigDTO" --type java -B 2 -A 2
echo -e "\n=== Checking VmMetadataDTO for context ==="
fd "VmMetadataDTO.java" --type fRepository: MatheMatrix/zstack
Length of output: 3676
🏁 Script executed:
#!/bin/bash
# Check the complete convertResourceConfig method
echo "=== Complete ResourceConfigDTO conversion method ==="
sed -n '170,180p' compute/src/main/java/org/zstack/compute/vm/metadata/VmMetadataConverter.java
# Also check if there are any other usages
echo -e "\n=== All lines using ResourceConfigDTO methods ==="
rg "ResourceConfigDTO" --type java -A 5 | grep -E "(setConfig|getConfig|ResourceConfigDTO\()"Repository: MatheMatrix/zstack
Length of output: 1169
🏁 Script executed:
#!/bin/bash
# Read the complete ResourceConfigDTO file
echo "=== Complete ResourceConfigDTO.java content ==="
cat -n compute/src/main/java/org/zstack/compute/vm/metadata/ResourceConfigDTO.java
# Also check if there's a getter/setter pattern or fields we're missing
echo -e "\n=== Check for any related backup or alternative files ==="
find compute/src -name "*ResourceConfig*" -type fRepository: MatheMatrix/zstack
Length of output: 717
ResourceConfigDTO 类缺少必要的字段和方法实现
ResourceConfigDTO 虽然被声明为空类，但在 VmMetadataConverter.convertResourceConfig() 方法中（第172-178行）已调用以下方法：
setConfigKey()setConfigValue()setIsSensitive()
这些方法在类中并不存在，会导致编译或运行时错误。需要补充以下字段及对应的 getter/setter 方法：
configKey（配置键）configValue（配置值）isSensitive（是否敏感字段）
🤖 Prompt for AI Agents
In `@compute/src/main/java/org/zstack/compute/vm/metadata/ResourceConfigDTO.java`
around lines 3 - 4, ResourceConfigDTO is empty but code calls setConfigKey,
setConfigValue, setIsSensitive from VmMetadataConverter.convertResourceConfig;
add private fields configKey (String), configValue (String) and isSensitive
(boolean) to ResourceConfigDTO and implement corresponding getters and setters
(getConfigKey/setConfigKey, getConfigValue/setConfigValue,
isSensitive/setIsSensitive) so the converter can compile and populate these
values.
Comment on lines
+3
to
+8
|
|
||
| import org.apache.commons.lang.StringUtils; | ||
| import org.zstack.utils.CollectionUtils; | ||
|
|
||
| import java.util.List; | ||
| import java.util.regex.Pattern; |
There was a problem hiding this comment.
缺少关键的导入声明。
以下类/常量被使用但未导入：
GlobalConfigVM_METADATA_SENSITIVE_KEYAESCipherAlertManagerSystemTagDTO
🐛 建议添加导入
package org.zstack.compute.vm.metadata;
+import org.zstack.core.config.GlobalConfig;
+import org.zstack.core.alert.AlertManager;
+import org.zstack.utils.cipher.AESCipher;
+import static org.zstack.compute.vm.VmGlobalConfig.VM_METADATA_SENSITIVE_KEY;
import org.apache.commons.lang.StringUtils;
import org.zstack.utils.CollectionUtils;
import java.util.List;
import java.util.regex.Pattern;📝 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
| import org.apache.commons.lang.StringUtils; | |
| import org.zstack.utils.CollectionUtils; | |
| import java.util.List; | |
| import java.util.regex.Pattern; | |
| package org.zstack.compute.vm.metadata; | |
| import org.zstack.core.config.GlobalConfig; | |
| import org.zstack.core.alert.AlertManager; | |
| import org.zstack.utils.cipher.AESCipher; | |
| import static org.zstack.compute.vm.VmGlobalConfig.VM_METADATA_SENSITIVE_KEY; | |
| import org.apache.commons.lang.StringUtils; | |
| import org.zstack.utils.CollectionUtils; | |
| import java.util.List; | |
| import java.util.regex.Pattern; |
🤖 Prompt for AI Agents
In
`@compute/src/main/java/org/zstack/compute/vm/metadata/SensitiveFieldHandler.java`
around lines 3 - 8, SensitiveFieldHandler is using several types/constants that
are not imported; add import declarations for GlobalConfig,
VM_METADATA_SENSITIVE_KEY, AESCipher, AlertManager, and SystemTagDTO at the top
of SensitiveFieldHandler.java (or the correct packages in your project) so the
compiler can resolve those symbols used in methods of SensitiveFieldHandler and
any references to the VM_METADATA_SENSITIVE_KEY constant, AESCipher encryption
calls, AlertManager alerts, and SystemTagDTO handling.
Comment on lines
+18
to
+25
| // 敏感标签列表 | ||
| private static final List<String> SENSITIVE_TAGS ; | ||
|
|
||
| static { | ||
| // 初始化敏感关键词正则 | ||
| String regex = String.join("|", SENSITIVE_TAGS); | ||
| SENSITIVE_PATTERN = Pattern.compile(regex, Pattern.CASE_INSENSITIVE); | ||
| } |
There was a problem hiding this comment.
SENSITIVE_TAGS 未初始化，会导致静态初始化块 NPE。
SENSITIVE_TAGS 在 Line 19 声明但未赋值，Line 23 中 String.join("|", SENSITIVE_TAGS) 会抛出 NullPointerException。
🐛 建议修复
// 敏感标签列表
- private static final List<String> SENSITIVE_TAGS ;
+ private static final List<String> SENSITIVE_TAGS = Arrays.asList(
+ "password", "secret", "token", "credential", "apikey"
+ // 根据实际需求添加更多敏感关键词
+ );📝 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
| // 敏感标签列表 | |
| private static final List<String> SENSITIVE_TAGS ; | |
| static { | |
| // 初始化敏感关键词正则 | |
| String regex = String.join("|", SENSITIVE_TAGS); | |
| SENSITIVE_PATTERN = Pattern.compile(regex, Pattern.CASE_INSENSITIVE); | |
| } | |
| // 敏感标签列表 | |
| private static final List<String> SENSITIVE_TAGS = Arrays.asList( | |
| "password", "secret", "token", "credential", "apikey" | |
| // 根据实际需求添加更多敏感关键词 | |
| ); | |
| static { | |
| // 初始化敏感关键词正则 | |
| String regex = String.join("|", SENSITIVE_TAGS); | |
| SENSITIVE_PATTERN = Pattern.compile(regex, Pattern.CASE_INSENSITIVE); | |
| } |
🤖 Prompt for AI Agents
In
`@compute/src/main/java/org/zstack/compute/vm/metadata/SensitiveFieldHandler.java`
around lines 18 - 25, SENSITIVE_TAGS is declared but never initialized which
causes a NullPointerException in the static initializer that builds
SENSITIVE_PATTERN; initialize SENSITIVE_TAGS (e.g., as an immutable/empty List
or populated from config) before the static block and guard the regex
construction in the static initializer so it handles an empty list (build a safe
regex or skip Pattern.compile if no tags). Update the static block referencing
SENSITIVE_TAGS and SENSITIVE_PATTERN in SensitiveFieldHandler to use the
initialized list or a fallback to avoid NPEs.
Comment on lines
+47
to
+53
| public static String getPrimaryStorageUuid() { | ||
| return primaryStorageUuid; | ||
| } | ||
|
|
||
| public static void setPrimaryStorageUuid(String primaryStorageUuid) { | ||
| VolumeMetadataDTO.primaryStorageUuid = primaryStorageUuid; | ||
| } |
There was a problem hiding this comment.
编译错误：引用了未声明的字段 primaryStorageUuid
getPrimaryStorageUuid() 和 setPrimaryStorageUuid() 方法引用了 primaryStorageUuid 字段，但该字段未在类中声明（Lines 6-26 的字段列表中缺失）。这将导致编译失败。
🐛 建议修复：添加缺失的字段声明
public class VolumeMetadataDTO {
- public static String description;
+ private String primaryStorageUuid;
+ private String description;
// ... other fields🤖 Prompt for AI Agents
In `@compute/src/main/java/org/zstack/compute/vm/metadata/VolumeMetadataDTO.java`
around lines 47 - 53, The class VolumeMetadataDTO is missing the String field
referenced by getPrimaryStorageUuid() and setPrimaryStorageUuid(); add a private
static String primaryStorageUuid declaration (matching the static access in
those methods) alongside the other DTO fields in VolumeMetadataDTO so the getter
and setter compile correctly, and ensure any intended visibility (private) and
default initialization are consistent with the other fields.
Comment on lines
+9489
to
+9499
|
|
||
| VmInstanceVO vmVo = dbf.findByUuid(vmUuid, VmInstanceVO.class); | ||
| List<VmNicVO> nicVos = dbf.listBySql("select * from VmNicVO where vmInstanceUuid = ?", VmNicVO.class, vmUuid); | ||
|
|
||
| VmMetadataDTO vmDto = VmMetadataConverter.convert(vmVo, nicVos, volumeVos, snapshotVos, groupVos, tagVos, configVos); | ||
|
|
||
| String json = VmMetadataSerializer.serialize(vmDto); | ||
|
|
||
| // | ||
| VolumeSnapshotReferenceVO refVo = dbf.findByUuid(snapshotDto.getSnapshotUuid(), VolumeSnapshotReferenceVO.class); | ||
| VmMetadataConverter.supplementSnapshotReference(snapshotDto, refVo); |
There was a problem hiding this comment.
存在未定义变量与残留逻辑，当前代码无法编译。
vmUuid、volumeVos、snapshotVos、groupVos、tagVos、configVos、snapshotDto 均未定义，导致编译失败。若确实要走 DTO/Serializer，请完整补齐变量来源；否则应删除这段残留代码。
🧹 建议移除未定义变量块（如不再需要 DTO/Serializer）
- VmInstanceVO vmVo = dbf.findByUuid(vmUuid, VmInstanceVO.class);
- List<VmNicVO> nicVos = dbf.listBySql("select * from VmNicVO where vmInstanceUuid = ?", VmNicVO.class, vmUuid);
-
- VmMetadataDTO vmDto = VmMetadataConverter.convert(vmVo, nicVos, volumeVos, snapshotVos, groupVos, tagVos, configVos);
-
- String json = VmMetadataSerializer.serialize(vmDto);
-
- //
- VolumeSnapshotReferenceVO refVo = dbf.findByUuid(snapshotDto.getSnapshotUuid(), VolumeSnapshotReferenceVO.class);
- VmMetadataConverter.supplementSnapshotReference(snapshotDto, refVo);📝 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
| VmInstanceVO vmVo = dbf.findByUuid(vmUuid, VmInstanceVO.class); | |
| List<VmNicVO> nicVos = dbf.listBySql("select * from VmNicVO where vmInstanceUuid = ?", VmNicVO.class, vmUuid); | |
| VmMetadataDTO vmDto = VmMetadataConverter.convert(vmVo, nicVos, volumeVos, snapshotVos, groupVos, tagVos, configVos); | |
| String json = VmMetadataSerializer.serialize(vmDto); | |
| // | |
| VolumeSnapshotReferenceVO refVo = dbf.findByUuid(snapshotDto.getSnapshotUuid(), VolumeSnapshotReferenceVO.class); | |
| VmMetadataConverter.supplementSnapshotReference(snapshotDto, refVo); |
🤖 Prompt for AI Agents
In `@compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java` around lines
9489 - 9499, The code in VmInstanceBase contains a residual DTO/serializer block
referencing undefined symbols (vmUuid, volumeVos, snapshotVos, groupVos, tagVos,
configVos, snapshotDto) and won't compile; either remove the entire
DTO/serializer/supplementSnapshotReference lines (the calls to
VmMetadataConverter.convert, VmMetadataSerializer.serialize and
VmMetadataConverter.supplementSnapshotReference) if metadata serialization is no
longer needed, or properly obtain and declare the missing variables before use
(ensure vmUuid is set to the current VM UUID, populate
volumeVos/snapshotVos/groupVos/tagVos/configVos from the DB or method context,
and get snapshotDto from the snapshot handling code) so the calls in
VmInstanceBase compile and have valid inputs.
Comment on lines
+33
to
+36
| // VolumeSnapshotGroupVO.toString | ||
| public List<String> volumeSnapshotGroupVO = new ArrayList<>(); | ||
| // VolumeSnapshotGroupRefVO.toString | ||
| public List<String> volumeSnapshotGroupRefVO = new ArrayList<>(); |
There was a problem hiding this comment.
列表字段命名建议复数化以匹配类型
volumeSnapshotGroupVO 与 volumeSnapshotGroupRefVO 是 List，但名称为单数，类型语义不清晰，容易误用。建议改为复数形式并同步更新引用。
As per coding guidelines: “命名应尽量用完整的单词组合表达意图，并在名称中体现数据类型或用途（例如在常量与变量名称中，将类型词放在末尾）”。
✅ 建议修改
- public List<String> volumeSnapshotGroupVO = new ArrayList<>();
+ public List<String> volumeSnapshotGroupVOs = new ArrayList<>();
// VolumeSnapshotGroupRefVO.toString
- public List<String> volumeSnapshotGroupRefVO = new ArrayList<>();
+ public List<String> volumeSnapshotGroupRefVOs = new ArrayList<>();📝 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
| // VolumeSnapshotGroupVO.toString | |
| public List<String> volumeSnapshotGroupVO = new ArrayList<>(); | |
| // VolumeSnapshotGroupRefVO.toString | |
| public List<String> volumeSnapshotGroupRefVO = new ArrayList<>(); | |
| // VolumeSnapshotGroupVO.toString | |
| public List<String> volumeSnapshotGroupVOs = new ArrayList<>(); | |
| // VolumeSnapshotGroupRefVO.toString | |
| public List<String> volumeSnapshotGroupRefVOs = new ArrayList<>(); |
🤖 Prompt for AI Agents
In `@header/src/main/java/org/zstack/header/vm/VmMetadata.java` around lines 33 -
36, The two list fields in VmMetadata—volumeSnapshotGroupVO and
volumeSnapshotGroupRefVO—are named in singular form while their type is List;
rename them to plural (e.g., volumeSnapshotGroupsVO and
volumeSnapshotGroupRefsVO or volumeSnapshotGroupVOs and
volumeSnapshotGroupRefVOs) and update all references/usages (constructors,
getters/setters, JSON/mapping annotations, code that reads/writes these fields)
to use the new names to keep naming consistent with their List semantics; ensure
any serialization names or API contracts are adjusted if needed.
Comment on lines
+3814
to
+3824
| bus.send(umsg, new CloudBusCallBack(msg) { | ||
| @Override | ||
| public void run(MessageReply r) { | ||
| UpdateVmInstanceMetadataOnPrimaryStorageReply reply = new UpdateVmInstanceMetadataOnPrimaryStorageReply(); | ||
| if (!r.isSuccess()) { | ||
| reply.setError(Platform.operr("failed to update vm[uuid=%s] on hypervisor.", self.getUuid()) | ||
| .withCause(r.getError())); | ||
| } | ||
| bus.reply(msg, reply); | ||
| } | ||
| }); |
There was a problem hiding this comment.
严重错误：completion 回调从未被调用。
方法签名包含 ReturnValueCompletion<UpdateVmInstanceMetadataOnPrimaryStorageReply> completion 参数，但在 CloudBusCallBack 中直接使用了 bus.reply(msg, reply) 而不是调用 completion.success(reply) 或 completion.fail(errorCode)。
这会导致调用方永远无法收到完成通知，可能造成调用方挂起或行为异常。
🐛 建议修复
bus.send(umsg, new CloudBusCallBack(msg) {
`@Override`
public void run(MessageReply r) {
UpdateVmInstanceMetadataOnPrimaryStorageReply reply = new UpdateVmInstanceMetadataOnPrimaryStorageReply();
if (!r.isSuccess()) {
- reply.setError(Platform.operr("failed to update vm[uuid=%s] on hypervisor.", self.getUuid())
- .withCause(r.getError()));
+ completion.fail(Platform.operr("failed to update vm metadata on hypervisor for rootVolume[uuid=%s].", msg.getRootVolumeUuid())
+ .causedBy(r.getError()));
+ return;
}
- bus.reply(msg, reply);
+ completion.success(reply);
}
});🤖 Prompt for AI Agents
In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmBackend.java`
around lines 3814 - 3824, The CloudBusCallBack currently constructs an
UpdateVmInstanceMetadataOnPrimaryStorageReply but never invokes the provided
ReturnValueCompletion<UpdateVmInstanceMetadataOnPrimaryStorageReply> completion,
so the caller is never notified; inside the CloudBusCallBack.run(MessageReply r)
use completion.success(reply) when r.isSuccess() and completion.fail(...) with
the same Platform-operr-created error when !r.isSuccess(), and then still send
bus.reply(msg, reply) if needed; ensure you reference the existing
UpdateVmInstanceMetadataOnPrimaryStorageReply, CloudBusCallBack, completion
parameter and the current error construction
(Platform.operr(...).withCause(r.getError())) so the completion and bus.reply
behavior are consistent and the caller receives the finish notification.
Comment on lines
+1886
to
+1891
| public void run(MessageReply r) { | ||
| if (!r.isSuccess()) { | ||
| trigger.fail(operr("failed to update vm[uuid=%s] on hypervisor.", | ||
| self.getUuid()).withCause(r.getError())); | ||
| return; | ||
| } |
There was a problem hiding this comment.
错误消息中的 UUID 引用有误。
Line 1888-1889 的错误消息使用 self.getUuid()，但 self 是 PrimaryStorageBase，这里应该是 VM 元数据读取失败的错误，不应引用主存储 UUID。
🐛 建议修复
`@Override`
public void run(MessageReply r) {
if (!r.isSuccess()) {
- trigger.fail(operr("failed to update vm[uuid=%s] on hypervisor.",
- self.getUuid()).withCause(r.getError()));
+ trigger.fail(operr("failed to read vm metadata from hypervisor[hostUuid=%s], path=%s",
+ msg.getHostUuid(), msg.getMetadataPath()).withCause(r.getError()));
return;
}🤖 Prompt for AI Agents
In `@storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java`
around lines 1886 - 1891, The error message in run(MessageReply r) incorrectly
logs self.getUuid() (PrimaryStorageBase) instead of the VM's UUID; update the
operr call in the failure branch to reference the VM's UUID from the surrounding
VM context (e.g., use the vm instance or VM inventory variable such as
vm.getUuid() or vmInventory.getUuid(), or extract it from VmInstanceSpec/VmSpec
if those are in scope) so the error message correctly identifies the VM; ensure
the chosen VM UUID variable is in scope and compile-clean.
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
2 participants
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.
Resolves: ZSV-1
Change-Id: I746a6d7266686d637a6d76756861636a74766b74
sync from gitlab !9082