-
Notifications
You must be signed in to change notification settings - Fork 0
<fix>[compute]: <description #3183
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: zsv_4.10.28
Are you sure you want to change the base?
Conversation
Walkthrough新增 VM 元数据构建与下发流水线:在控制平面收集/组装 VmMetadata,新增消息与扩展点将元数据下发到主机,再由 KVM agent 接收;新增基于 GC 的串行触发器以调度元数据更新任务。 Changes
Sequence Diagram(s)sequenceDiagram
participant GC as UpdateVmInstanceMetadataGC
participant Chain as ChainTask (per-VM queue)
participant CloudBus as CloudBus
participant VMBase as VmInstanceBase
participant HostMgr as Host/VM Manager
participant KVMHost as KVMHost
participant Agent as KVM Agent
GC->>Chain: 提交串行任务 (queue: update-vm-{uuid}-metadata)
Chain->>CloudBus: 发送 UpdateVmInstanceMetadataMsg(uuid)
CloudBus->>VMBase: 触发构建 VmMetadata
VMBase-->>CloudBus: 返回 VmMetadata (JSON)
CloudBus->>HostMgr: 路由 UpdateVmInstanceMetadataOnHypervisorMsg(hostUuid, metadata)
HostMgr->>KVMHost: 派发消息
KVMHost->>Agent: POST /host/vm/metadata/update (UpdateVmInstanceMetadataCmd)
Agent-->>KVMHost: UpdateVmInstanceMetadataRsp
KVMHost-->>CloudBus: 回复 UpdateVmInstanceMetadataOnHypervisorReply
CloudBus-->>Chain: 完成回调 -> GC 结束
🎯 代码审查工作量评估🎯 4 (Complex) | ⏱️ ~45 分钟 变更概览(更新后)
🐰 庆祝诗
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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.javaComment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java:
- Around line 4863-4905: The handler for APIRecoverVmInstanceMsg has been fully
commented out while handleApiMessage still routes messages to it, causing a
silent no-op; either restore the original implementation in
VmInstanceBase.handle(APIRecoverVmInstanceMsg) (reintroduce the thdf.chainSubmit
ChainTask that calls validateOperationByState, refreshVO, recoverVm with the
Completion that publishes APIRecoverVmInstanceEvent via bus and uses
getSelfInventory on success), or explicitly remove the routing in
handleApiMessage and return/publish a clear error event immediately so callers
don’t get a silent success; ensure references to APIRecoverVmInstanceMsg,
APIRecoverVmInstanceEvent, validateOperationByState, recoverVm,
getSelfInventory, and thdf.chainSubmit are updated accordingly.
- Around line 9373-9377: The handler method
VmInstanceBase.handle(APIRecoverVmInstanceMsg msg) is empty and must publish an
APIRecoverVmInstanceEvent so callers get a response; restore the original
recovery flow: create an APIRecoverVmInstanceEvent for msg.getId(), attempt the
VM recovery logic (reuse the existing recovery code path or invoke the recovery
helper used elsewhere), on success set event.setInventory(...) and publish the
event via bus.publish(event), and on failure catch the exception, log it and
publish an event with error code/event.setError(errCode) or throw a
CloudRuntimeException as your pattern requires; ensure you reference
APIRecoverVmInstanceMsg, APIRecoverVmInstanceEvent and the
VmInstanceBase.handle(...) method when implementing the fix.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java
⚙️ CodeRabbit configuration file
**/*.java: ## 1. API 设计要求
- API 命名:
- API 名称必须唯一,不能重复。
- API 消息类需要继承
APIMessage;其返回类必须继承APIReply或APIEvent,并在注释中用@RestResponse进行标注。- API 消息上必须添加注解
@RestRequest,并满足如下规范:
path:
- 针对资源使用复数形式。
- 当 path 中引用消息类变量时,使用
{variableName}格式。- HTTP 方法对应:
- 查询操作 →
HttpMethod.GET- 更新操作 →
HttpMethod.PUT- 创建操作 →
HttpMethod.POST- 删除操作 →
HttpMethod.DELETE- API 类需要实现
__example__方法以便生成 API 文档,并确保生成对应的 Groovy API Template 与 API Markdown 文件。
2. 命名与格式规范
类名:
- 使用 UpperCamelCase 风格。
- 特殊情况:
- VO/AO/EO 类型类除外。
- 抽象类采用
Abstract或Base前缀/后缀。- 异常类应以
Exception结尾。- 测试类需要以
Test或Case结尾。方法名、参数名、成员变量和局部变量:
- 使用 lowerCamelCase 风格。
常量命名:
- 全部大写,使用下划线分隔单词。
- 要求表达清楚,避免使用含糊或不准确的名称。
包名:
- 统一使用小写,使用点分隔符,每个部分应是一个具有自然语义的英文单词(参考 Spring 框架的结构)。
命名细节:
- 避免在父子类或同一代码块中出现相同名字的成员或局部变量,防止混淆。
- 命名缩写:
- 不允许使用不必要的缩写,如:
AbsSchedulerJob、condi、Fu等。应使用完整单词提升可读性。
3. 编写自解释代码
意图表达:
- 避免使用布尔型参数造成含义不明确。例如:
- 对于
stopAgent(boolean ignoreError),建议拆分为不同函数(如stopAgentIgnoreError()),或使用枚举表达操作类型。- 命名应尽量用完整的单词组合表达意图,并在名称中体现数据类型或用途(例如在常量与变量名称中,将类型词放在末尾)。
注释:
- 代码应尽量做到自解释,对少于两行的说明可以直接写在代码中。
- 对于较长的注释,需要仔细校对并随代码更新,确保内容正确。
- 接口方法不应有多余的修饰符(例如
public),且必须配有有效的 Javadoc 注释。
4. 流程控制和结构优化
if...else 的使用:
- 应尽量减少 if...else 结构的使用,建议:
- 限制嵌套层级最多为两层,且内层不应再出现
else分支。- 尽早返回(Early Return),将条件判断中的处理逻辑提前结束或抽成独立方法。
- 使用 Java Stream 或 Lambda 表达式代替冗长的循环与条件判断。
条件判断:
- if 条件表达不宜过长或过于复杂,必要时可以将条件抽成 boolean 变量描述。
代码块长度:
- 单个 if 代码块不宜超过一屏显示,以提高可读性和后续维护性。
5. 异常处理与日志
- 捕获异常的原则:
- 对于可以通过预检查避免的 RuntimeException(如
NullPointerException、IndexOutOfBoundsException等),不建议使用 try-catch 来进行处理。- 捕获异常应仅用于处理真正的意外情况,不应将异常逻辑当作正常流程控制。
- 在必要时,应继续抛出异常,使上层业务处理者可以转换为用户友好的错误提示。
- 使用 try-with-resources 语法管理资源,确保在 finally 块中正确关闭资源,并避免在 finally 中返回值。
...
Files:
compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java
🧠 Learnings (3)
📓 Common learnings
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 2360
File: network/src/main/java/org/zstack/network/l3/L3BasicNetwork.java:449-490
Timestamp: 2025-08-04T04:48:19.103Z
Learning: ZStack项目在cherry-pick操作中,即使发现了性能优化机会(如IP地址批量保存的内存优化),也严格遵循不做额外修改的政策,优先保证cherry-pick的完整性和一致性。
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 2489
File: storage/src/main/java/org/zstack/storage/snapshot/VolumeTree.java:471-545
Timestamp: 2025-08-22T05:36:40.467Z
Learning: In ZStack项目中,对于核心逻辑相关的代码,团队倾向于暂时不进行重构,即使代码复杂度较高,优先保证核心功能的稳定性。
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 2325
File: compute/src/main/java/org/zstack/compute/vm/VmMigrateCallExtensionFlow.java:29-29
Timestamp: 2025-07-24T05:53:10.246Z
Learning: 在ZStack项目的重构过程中,团队采用务实的优先级策略:优先完成影响运行时功能的代码重构,对于注释中的旧引用等非功能性问题可以延后处理,因为这些不会影响系统的实际运行。
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 2360
File: compute/src/main/java/org/zstack/compute/vm/StaticIpOperator.java:364-501
Timestamp: 2025-08-04T03:19:07.454Z
Learning: In ZStack project, developers may prefer to keep related validation logic together in a single method (like validateStaticIpTagsInApiMessage with 130+ lines) when the architecture is clear and there's no code reuse, rather than splitting into smaller methods based purely on single responsibility principle.
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 2419
File: plugin/sdnController/src/main/java/org/zstack/sdnController/SdnControllerBase.java:986-1067
Timestamp: 2025-08-13T02:31:40.023Z
Learning: 在ZStack项目中,团队倾向于保持统一的业务流程方法不进行拆分,即使方法较长也会维持在单个方法中以保持业务逻辑的完整性。
Learnt from: zstack-robot-2
Repo: MatheMatrix/zstack PR: 2496
File: plugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/KvmBackend.java:2545-2566
Timestamp: 2025-08-24T07:35:42.793Z
Learning: 在 ZStack 代码库中,当响应类包含数值字段(如 size、actualSize)时,优先使用原始类型(long)而不是包装类型(Long),以避免 NPE 风险和不必要的装箱/拆箱操作。如果 Agent 端可能不设置该字段,应在 Agent 端确保设置默认值,而不是在使用端做 null 检查。
📚 Learning: 2025-08-14T06:56:19.585Z
Learnt from: zstack-robot-2
Repo: MatheMatrix/zstack PR: 2435
File: storage/src/main/java/org/zstack/storage/snapshot/group/VolumeSnapshotGroupBase.java:47-47
Timestamp: 2025-08-14T06:56:19.585Z
Learning: 在VolumeSnapshotGroupBase.java中,VmInstanceResourceMetadataManager的注入和SKIP_RESOURCE_ROLLBACK标记虽然在当前版本中未被使用,但这些导入在大型重构PR中是有意为之的,用于保持代码一致性或为后续功能做准备。
Applied to files:
compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java
📚 Learning: 2025-08-12T05:52:18.323Z
Learnt from: MatheMatrix
Repo: MatheMatrix/zstack PR: 2418
File: compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java:3873-3885
Timestamp: 2025-08-12T05:52:18.323Z
Learning: 在 compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java 的 setNoIpAllocationStaticIp() 流程中,静态 IP 标签删除已统一在 change-ip-in-database 步骤对 voRemoveSet 循环处理:对收集到的旧 IPv4/IPv6 UsedIpVO 均调用 ipOperator.deleteStaticIpByVmUuidAndL3Uuid(vmUuid, l3Uuid, IPv6NetworkUtils.ipv6AddessToTagValue(ip)) 清理对应的 static-ip 标签。因此无需在分支里分别删除 IPv4/IPv6 的标签。
Applied to files:
compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java
| private void handle(final APIRecoverVmInstanceMsg msg) { | ||
| // | ||
|
|
||
|
|
||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
阻断级问题:APIRecoverVmInstanceMsg handler 为空,可能导致 API 调用无事件返回/超时
当前方法体为空,且没有发布 APIRecoverVmInstanceEvent,调用侧很可能等不到事件。建议直接恢复原逻辑(或至少 publish error/success event)。
建议修复(恢复为可工作的 handler)
private void handle(final APIRecoverVmInstanceMsg msg) {
- //
+ thdf.chainSubmit(new ChainTask(msg) {
+ @Override
+ public String getSyncSignature() {
+ return syncThreadName;
+ }
+
+ @Override
+ public void run(final SyncTaskChain chain) {
+ final APIRecoverVmInstanceEvent evt = new APIRecoverVmInstanceEvent(msg.getId());
+ refreshVO();
+
+ ErrorCode error = validateOperationByState(msg, self.getState(), SysErrors.OPERATION_ERROR);
+ if (error != null) {
+ evt.setError(error);
+ bus.publish(evt);
+ chain.next();
+ return;
+ }
+
+ recoverVm(new Completion(msg, chain) {
+ @Override
+ public void success() {
+ evt.setInventory(getSelfInventory());
+ bus.publish(evt);
+ chain.next();
+ }
+
+ @Override
+ public void fail(ErrorCode errorCode) {
+ evt.setError(errorCode);
+ bus.publish(evt);
+ chain.next();
+ }
+ });
+ }
+
+ @Override
+ public String getName() {
+ return "recover-vm";
+ }
+ });
}🤖 Prompt for AI Agents
In @compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java around lines
9373 - 9377, The handler method VmInstanceBase.handle(APIRecoverVmInstanceMsg
msg) is empty and must publish an APIRecoverVmInstanceEvent so callers get a
response; restore the original recovery flow: create an
APIRecoverVmInstanceEvent for msg.getId(), attempt the VM recovery logic (reuse
the existing recovery code path or invoke the recovery helper used elsewhere),
on success set event.setInventory(...) and publish the event via
bus.publish(event), and on failure catch the exception, log it and publish an
event with error code/event.setError(errCode) or throw a CloudRuntimeException
as your pattern requires; ensure you reference APIRecoverVmInstanceMsg,
APIRecoverVmInstanceEvent and the VmInstanceBase.handle(...) method when
implementing the fix.
3ebc3ec to
4a00f1d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java`:
- Around line 9462-9464: The current logger.info call in VmInstanceBase that
logs JSONObjectUtil.toJsonString(vmMetadata) along with vms.get(0).getUuid()
exposes potentially sensitive metadata and should be changed: replace the
logger.info(...) usage with a lower level (logger.debug) and/or route through a
sanitizer/truncator before serializing (e.g., build a scrubbed map from
vmMetadata removing or masking fields like paths, uuids, account identifiers and
truncating long strings) and respect a configurable flag to enable full metadata
logging; update references in VmInstanceBase around the recover log (the
JSONObjectUtil.toJsonString(vmMetadata) and logger.info call) to use the
debug-level call and sanitized JSON or gated logging via a config switch.
- Around line 9435-9457: Replace the full-table scans and get(0) usage in
VmInstanceBase where vms, volumes, nics, snapshot, group and groupRef are loaded
into VmMetadata: query each VO by the VM UUID (use msg.getUuid() or the relevant
msg field) and by the proper foreign-key (e.g., VolumeVO.byVmUuid,
VmNicVO.byVmUuid, VolumeSnapshotVO.byVolumeUuid,
VolumeSnapshotGroupRefVO.byGroupUuid) instead of Q.New(...).list() for the
entire table; then check each resulting list for emptiness and return a proper
error event (or throw a controlled error) when required rather than calling
get(0), and build vmMetadata only from the filtered results. Ensure you update
references to vmMetadata.vmInstanceVO, vmMetadata.volumeVO, vmMetadata.vmNicVO
and the volumeSnapshots map population to use the filtered query results and
null/empty guards.
♻️ Duplicate comments (1)
compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java (1)
9397-9464: 阻断:APIRecover 变成空转且未发布事件,调用会超时/语义回归。
当前实现只记录元数据并日志输出,没有发布APIRecoverVmInstanceEvent,也不再走recoverVm,与 API 语义不一致。此问题已在此前 review 中指出,仍未修复。
compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java
Outdated
Show resolved
Hide resolved
compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java
Outdated
Show resolved
Hide resolved
2821d99 to
ab9be52
Compare
Resolves: ZSV-1 Change-Id: I746a6d7266686d637a6d76756861636a74766b74
ab9be52 to
72ceb84
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java (1)
606-747: 新增处理器未接入消息分发
handle(UpdateVmInstanceMetadataOnHypervisorMsg)未在handleLocalMessage中注册分发分支,导致该处理器不会被调用。请补充路由。
Line 606🐛 建议修正
@@ } else if (msg instanceof RestartKvmAgentMsg) { handle((RestartKvmAgentMsg) msg); + } else if (msg instanceof UpdateVmInstanceMetadataOnHypervisorMsg) { + handle((UpdateVmInstanceMetadataOnHypervisorMsg) msg); } else { super.handleLocalMessage(msg); }
🤖 Fix all issues with AI agents
In `@compute/src/main/java/org/zstack/compute/vm/UpdateVmInstanceMetadataGC.java`:
- Around line 13-33: The existence check in
UpdateVmInstanceMetadataGC.triggerNow uses dbf.isExist(vmInstanceUuid,
VolumeVO.class) which is wrong for a VM UUID; update the check to verify the VM
entity instead (e.g. dbf.isExist(vmInstanceUuid, VmInstanceVO.class)) so the GC
does not incorrectly call completion.cancel(); ensure the correct VmInstanceVO
import is present and adjust any related logic that assumes VolumeVO.
- Around line 54-67: The callback in UpdateVmInstanceMetadataGC.run sends
UpdateVmInstanceMetadataMsg but never releases the SyncTaskChain, which will
block subsequent tasks; modify the CloudBusCallBack anonymous run(MessageReply
reply) to ensure chain.next() is always invoked in a finally block (after
calling completion.success() or completion.fail(reply.getError())), so the
SyncTaskChain is advanced regardless of reply success or exceptions; locate the
bus.send(...) call and update its callback to call chain.next() in finally.
In `@compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java`:
- Around line 9494-9496: The query uses Q.New(SystemTag.class) but selects
fields from SystemTagVO_ which mismatches types and can fail; change the query
to use Q.New(SystemTagVO.class) (or the VO type consistent with SystemTagVO_) so
the select(SystemTagVO_.tag, SystemTagVO_.inherent,
SystemTagVO_.type).listValues() operates on the correct entity; update the
Q.New(...) call where tuples is defined to reference SystemTagVO instead of
SystemTag.
- Around line 9523-9531: The NIC query and subsequent snapshot/group queries are
wrong: change the VmNicVO query to use the correct metamodel field (use
VmNicVO_.vmInstanceUuid when building nics), populate volumeUuids by querying
VolumeVO for volumes with vmInstanceUuid and collecting their uuids before
calling Q.New(VolumeSnapshotVO.class).in(...), and populate groupUuids by
extracting uuids from the retrieved group (VolumeSnapshotGroupVO) list before
calling Q.New(VolumeSnapshotGroupRefVO.class).in(...); update the code around
variables nics, volumeUuids, snapshot, group, groupUuids, and groupRef
accordingly so the in(...) calls receive real UUID lists.
- Around line 9592-9635: getResourceSystemTag and getResourceConfig are
comparing resourceType incorrectly and using wrong identity comparison logic:
resourceType is a SimpleName but code compares to Class.getName(), and identity
comparison uses reference equality with reversed logic. Update
getResourceSystemTag to compare resourceType with
systemTag.getResourceClass().getSimpleName(); in getResourceConfig ensure you
compare bindResourceConfig value entries' Class.getSimpleName() (or convert
resourceType to FQN consistently) and change the identity check from
"it.getIdentity() == identity" to use equals (it.getIdentity().equals(identity))
and ensure the condition only adds matching configs (invert the current
conditional if necessary). Use the symbols SystemTag.getResourceClass(),
BindResourceConfig, getResourceConfig, getResourceSystemTag, and getIdentity()
to locate and fix the logic.
- Around line 9641-9673: The handler handle(UpdateVmInstanceMetadataMsg)
currently neither replies nor calls chain.next and also can send to a null host
UUID; update it to validate the chosen host UUID (self.getHostUuid() or
self.getLastHostUuid()) before creating/sending
UpdateVmInstanceMetadataOnHypervisorMsg and if hostUuid is null immediately set
an error on UpdateVmInstanceMetadataReply and reply to the original msg via
bus.reply(msg, reply) and call chain.next(); when invoking bus.send(..., new
CloudBusCallBack(msg)) ensure the callback always either sets reply.setError on
failure or leaves success, then calls bus.reply(msg, reply) and finally
chain.next() so the request is replied and the chain continues in all code
paths.
In `@header/src/main/java/org/zstack/header/vm/APIRegVmInstanceMsg.java`:
- Around line 30-33: The field defaultL3NetworkUuid in class APIRegVmInstanceMsg
is missing the `@APIParam` resource validation and metadata; add the annotation
`@APIParam`(resourceType = L3NetworkVO.class, required = false) to the
defaultL3NetworkUuid field so it receives L3Network resource validation and
appears correctly in docs (mirror the existing annotation used on
l3NetworkUuids).
- Around line 16-26: The RestRequest path includes a path variable "{uuid}" but
APIRegVmInstanceMsg lacks a matching field; add a private String uuid field to
APIRegVmInstanceMsg annotated with `@APIParam`(resourceType = VmInstanceVO.class,
checkAccount = true) so REST binding and resource ownership validation succeed,
and add the corresponding getter/setter for the uuid field; ensure the
annotation and field name exactly match "uuid" so the framework binds the path
parameter correctly.
In
`@header/src/main/java/org/zstack/header/vm/UpdateVmInstanceMetadataExtensionPoint.java`:
- Around line 6-8: 为 UpdateVmInstanceMetadataExtensionPoint 接口的
buildVmInstanceMetadata 方法添加完整的 Javadoc,说明该方法的用途(例如:在何时/为谁构建或修改 VM
的元数据)、每个参数的含义(VmInstanceInventory 表示哪个虚拟机的详情,VmMetadata
表示将被构建或填充的元数据对象)以及方法的预期副作用或返回行为(是否可修改
vmMetadata、线程安全或异常契约)。在注释中还应指出实现者需注意的约束或调用时机(如果适用),并在注释头部包含 `@param` 标签和必要的
`@throws/`@see 说明以符合代码规范。
In
`@header/src/main/java/org/zstack/header/vm/UpdateVmInstanceMetadataOnHypervisorMsg.java`:
- Around line 1-16: Add the `@NoLogging` annotation to the vmInstanceMetadata
field in the UpdateVmInstanceMetadataOnHypervisorMsg class to prevent sensitive
VM metadata from being logged: annotate the private String vmInstanceMetadata
field with `@NoLogging`, add the corresponding import for the NoLogging
annotation, and keep the existing getter/setter unchanged so logging is
suppressed for this field at source.
In `@header/src/main/java/org/zstack/header/vm/VmMetadata.java`:
- Around line 48-50: Rename the public field
VmMetadata.EncryptedResourceKeyRefVO to follow lowerCamelCase
(encryptedResourceKeyRefVO) and update all references accordingly: change the
field declaration in class VmMetadata, adjust any direct usages,
getters/setters, constructor params, and any serialization/deserialization
mappings (e.g., JSON/XML or reflection-based code) that rely on the old name so
the external field name remains consistent; ensure compilation by updating
imports/usages across the codebase and run tests to verify no serialization
breakage.
- Around line 7-13: The field comments for vmSystemTags and vmResourceConfigs
describe a key/value map but the actual types are List<String>; update the
comments to accurately describe their current structure (e.g., "list of system
tag strings" and "list of resource config strings") or change the field types to
a Map keyed by vm UUID if a key/value structure is intended; locate and modify
the declarations vmSystemTags and vmResourceConfigs in class VmMetadata (and
vmInstanceVO if its comment needs alignment) so the comment text matches the
chosen type.
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java`:
- Around line 4153-4167: The method
handle(UpdateVmInstanceMetadataOnHypervisorMsg msg) is constructing the wrong
reply type (UpdateVmInstanceMetadataReply) for an OnHypervisor message; replace
it with the correct UpdateVmInstanceMetadataOnHypervisorReply, populate its
success/error and any response fields from UpdateVmInstanceMetadataRsp (e.g.
setError/opStatus or specific metadata fields as appropriate) and send that via
bus.reply(msg, reply) so the bus deserializes and fills back correctly; locate
the construction and success/failure handling around
UpdateVmInstanceMetadataReply and UpdateVmInstanceMetadataCmd in this method and
switch to UpdateVmInstanceMetadataOnHypervisorReply while preserving the
existing ret.isSuccess() logic to set error details.
♻️ Duplicate comments (2)
compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java (2)
9381-9384: APIRecover 选错 VM + 全表扫描风险
这里通过全表扫描取get(0),且忽略msg.getUuid(),可能恢复到错误 VM 或在空表时抛异常。请按请求 UUID 精确查询并做空结果处理。🛠️ 建议修正
- List<VmInstanceVO> vms = Q.New(VmInstanceVO.class).list(); - String json = buildVmInstanceMetadata(vms.get(0).getUuid()); + VmInstanceVO vm = Q.New(VmInstanceVO.class) + .eq(VmInstanceVO_.uuid, msg.getUuid()) + .find(); + if (vm == null) { + APIRecoverVmInstanceEvent evt = new APIRecoverVmInstanceEvent(msg.getId()); + evt.setError(operr("vm[uuid:%s] not found", msg.getUuid())); + bus.publish(evt); + return; + } + String json = buildVmInstanceMetadata(vm.getUuid());
9381-9484: APIRecover 未发布事件/回复,调用方可能超时
新 handler 没有APIRecoverVmInstanceEvent/reply,也未走队列链路;API 调用可能一直等待。请恢复事件发布并在流程结束时返回。🛠️ 建议修正(保留原有事件语义)
- private void handle(final APIRecoverVmInstanceMsg msg) { - List<VmInstanceVO> vms = Q.New(VmInstanceVO.class).list(); - String json = buildVmInstanceMetadata(vms.get(0).getUuid()); + private void handle(final APIRecoverVmInstanceMsg msg) { + thdf.chainSubmit(new ChainTask(msg) { + `@Override` + public String getSyncSignature() { + return syncThreadName; + } + + `@Override` + public void run(SyncTaskChain chain) { + final APIRecoverVmInstanceEvent evt = new APIRecoverVmInstanceEvent(msg.getId()); + // ... 组装/恢复逻辑 + evt.setInventory(getSelfInventory()); + bus.publish(evt); + chain.next(); + } + + `@Override` + public String getName() { + return "recover-vm"; + } + }); }
| import org.zstack.header.vm.UpdateVmInstanceMetadataMsg; | ||
| import org.zstack.header.volume.VolumeConstant; | ||
| import org.zstack.header.volume.VolumeVO; | ||
|
|
||
| public class UpdateVmInstanceMetadataGC extends TimeBasedGarbageCollector { | ||
| @GC | ||
| public String vmInstanceUuid; | ||
|
|
||
| @GC | ||
| public boolean detachBeforeDeleting; | ||
|
|
||
| @GC | ||
| public String deletionPolicy; | ||
|
|
||
| @Autowired | ||
| protected ThreadFacade thdf; | ||
|
|
||
| @Override | ||
| protected void triggerNow(GCCompletion completion) { | ||
| if (!dbf.isExist(vmInstanceUuid, VolumeVO.class)) { | ||
| completion.cancel(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
存在性校验可能用错表,导致更新直接被取消。
Line 32 使用 VolumeVO 校验 vmInstanceUuid,很可能始终为 false,导致 GC 直接 cancel,从而元数据更新不触发。建议改为校验 VM 实体(如 VmInstanceVO)或实际资源类型。
🛠 建议修改
-import org.zstack.header.volume.VolumeVO;
+import org.zstack.header.vm.VmInstanceVO;
...
- if (!dbf.isExist(vmInstanceUuid, VolumeVO.class)) {
+ if (!dbf.isExist(vmInstanceUuid, VmInstanceVO.class)) {
completion.cancel();
return;
}📝 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.
| import org.zstack.header.vm.UpdateVmInstanceMetadataMsg; | |
| import org.zstack.header.volume.VolumeConstant; | |
| import org.zstack.header.volume.VolumeVO; | |
| public class UpdateVmInstanceMetadataGC extends TimeBasedGarbageCollector { | |
| @GC | |
| public String vmInstanceUuid; | |
| @GC | |
| public boolean detachBeforeDeleting; | |
| @GC | |
| public String deletionPolicy; | |
| @Autowired | |
| protected ThreadFacade thdf; | |
| @Override | |
| protected void triggerNow(GCCompletion completion) { | |
| if (!dbf.isExist(vmInstanceUuid, VolumeVO.class)) { | |
| completion.cancel(); | |
| import org.zstack.header.vm.UpdateVmInstanceMetadataMsg; | |
| import org.zstack.header.volume.VolumeConstant; | |
| import org.zstack.header.vm.VmInstanceVO; | |
| public class UpdateVmInstanceMetadataGC extends TimeBasedGarbageCollector { | |
| `@GC` | |
| public String vmInstanceUuid; | |
| `@GC` | |
| public boolean detachBeforeDeleting; | |
| `@GC` | |
| public String deletionPolicy; | |
| `@Autowired` | |
| protected ThreadFacade thdf; | |
| `@Override` | |
| protected void triggerNow(GCCompletion completion) { | |
| if (!dbf.isExist(vmInstanceUuid, VmInstanceVO.class)) { | |
| completion.cancel(); |
🤖 Prompt for AI Agents
In `@compute/src/main/java/org/zstack/compute/vm/UpdateVmInstanceMetadataGC.java`
around lines 13 - 33, The existence check in
UpdateVmInstanceMetadataGC.triggerNow uses dbf.isExist(vmInstanceUuid,
VolumeVO.class) which is wrong for a VM UUID; update the check to verify the VM
entity instead (e.g. dbf.isExist(vmInstanceUuid, VmInstanceVO.class)) so the GC
does not incorrectly call completion.cancel(); ensure the correct VmInstanceVO
import is present and adjust any related logic that assumes VolumeVO.
| public void run(final SyncTaskChain chain) { | ||
| UpdateVmInstanceMetadataMsg msg = new UpdateVmInstanceMetadataMsg(); | ||
| msg.setUuid(vmInstanceUuid); | ||
| bus.makeTargetServiceIdByResourceUuid(msg, VolumeConstant.SERVICE_ID, vmInstanceUuid); | ||
| bus.send(msg, new CloudBusCallBack(completion) { | ||
| @Override | ||
| public void run(MessageReply reply) { | ||
| if (!reply.isSuccess()) { | ||
| completion.fail(reply.getError()); | ||
| } else { | ||
| completion.success(); | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
异步回调未释放链,可能阻塞后续任务。
当前回调里没有调用 chain.next(),链会被永久占用,后续相同队列任务无法执行。建议在回调 finally 中释放链。
🛠 建议修改
- bus.send(msg, new CloudBusCallBack(completion) {
+ bus.send(msg, new CloudBusCallBack(completion) {
`@Override`
public void run(MessageReply reply) {
- if (!reply.isSuccess()) {
- completion.fail(reply.getError());
- } else {
- completion.success();
- }
+ try {
+ if (!reply.isSuccess()) {
+ completion.fail(reply.getError());
+ } else {
+ completion.success();
+ }
+ } finally {
+ chain.next();
+ }
}
});🤖 Prompt for AI Agents
In `@compute/src/main/java/org/zstack/compute/vm/UpdateVmInstanceMetadataGC.java`
around lines 54 - 67, The callback in UpdateVmInstanceMetadataGC.run sends
UpdateVmInstanceMetadataMsg but never releases the SyncTaskChain, which will
block subsequent tasks; modify the CloudBusCallBack anonymous run(MessageReply
reply) to ensure chain.next() is always invoked in a finally block (after
calling completion.success() or completion.fail(reply.getError())), so the
SyncTaskChain is advanced regardless of reply success or exceptions; locate the
bus.send(...) call and update its callback to call chain.next() in finally.
| List<String> systemTags = new ArrayList<>(); | ||
| List<Tuple> tuples = Q.New(SystemTag.class).eq(SystemTagVO_.resourceUuid, vm.getUuid()) | ||
| .select(SystemTagVO_.tag, SystemTagVO_.inherent, SystemTagVO_.type).listValues(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SystemTag 查询实体类型错误
Q.New(SystemTag.class) 与 SystemTagVO_ 不匹配,可能直接编译/运行失败。这里应查询 SystemTagVO。
🛠️ 建议修正
- List<Tuple> tuples = Q.New(SystemTag.class).eq(SystemTagVO_.resourceUuid, vm.getUuid())
+ List<Tuple> tuples = Q.New(SystemTagVO.class).eq(SystemTagVO_.resourceUuid, vm.getUuid())
.select(SystemTagVO_.tag, SystemTagVO_.inherent, SystemTagVO_.type).listValues();🤖 Prompt for AI Agents
In `@compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java` around lines
9494 - 9496, The query uses Q.New(SystemTag.class) but selects fields from
SystemTagVO_ which mismatches types and can fail; change the query to use
Q.New(SystemTagVO.class) (or the VO type consistent with SystemTagVO_) so the
select(SystemTagVO_.tag, SystemTagVO_.inherent, SystemTagVO_.type).listValues()
operates on the correct entity; update the Q.New(...) call where tuples is
defined to reference SystemTagVO instead of SystemTag.
| List<VmNicVO> nics = Q.New(VmNicVO.class).eq(VolumeVO_.vmInstanceUuid, vmInstanceUuid).list(); | ||
|
|
||
| List<String> volumeUuids = new ArrayList<>(); | ||
| List<VolumeSnapshotVO> snapshot = Q.New(VolumeSnapshotVO.class).in(VolumeSnapshotVO_.volumeUuid, volumeUuids).list(); | ||
|
|
||
| List<VolumeSnapshotGroupVO> group = Q.New(VolumeSnapshotGroupVO.class).eq(VolumeSnapshotGroupVO_.vmInstanceUuid, vmInstanceUuid).list(); | ||
| List<String> groupUuids = new ArrayList<>(); | ||
| List<VolumeSnapshotGroupRefVO> groupRef = Q.New(VolumeSnapshotGroupRefVO.class).in(VolumeSnapshotGroupRefVO_.volumeSnapshotGroupUuid, groupUuids).list(); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIC 查询条件与快照/组 ID 列表未填充
VmNicVO 查询使用了 VolumeVO_ 元模型,且 volumeUuids/groupUuids 未填充导致快照/组引用永远为空,元数据不完整。
🛠️ 建议修正
- List<VmNicVO> nics = Q.New(VmNicVO.class).eq(VolumeVO_.vmInstanceUuid, vmInstanceUuid).list();
-
- List<String> volumeUuids = new ArrayList<>();
+ List<VmNicVO> nics = Q.New(VmNicVO.class)
+ .eq(VmNicVO_.vmInstanceUuid, vmInstanceUuid)
+ .list();
+
+ List<String> volumeUuids = volumes.stream()
+ .map(VolumeVO::getUuid)
+ .collect(Collectors.toList());
List<VolumeSnapshotVO> snapshot = Q.New(VolumeSnapshotVO.class).in(VolumeSnapshotVO_.volumeUuid, volumeUuids).list();
List<VolumeSnapshotGroupVO> group = Q.New(VolumeSnapshotGroupVO.class).eq(VolumeSnapshotGroupVO_.vmInstanceUuid, vmInstanceUuid).list();
- List<String> groupUuids = new ArrayList<>();
+ List<String> groupUuids = group.stream()
+ .map(VolumeSnapshotGroupVO::getUuid)
+ .collect(Collectors.toList());
List<VolumeSnapshotGroupRefVO> groupRef = Q.New(VolumeSnapshotGroupRefVO.class).in(VolumeSnapshotGroupRefVO_.volumeSnapshotGroupUuid, groupUuids).list();🤖 Prompt for AI Agents
In `@compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java` around lines
9523 - 9531, The NIC query and subsequent snapshot/group queries are wrong:
change the VmNicVO query to use the correct metamodel field (use
VmNicVO_.vmInstanceUuid when building nics), populate volumeUuids by querying
VolumeVO for volumes with vmInstanceUuid and collecting their uuids before
calling Q.New(VolumeSnapshotVO.class).in(...), and populate groupUuids by
extracting uuids from the retrieved group (VolumeSnapshotGroupVO) list before
calling Q.New(VolumeSnapshotGroupRefVO.class).in(...); update the code around
variables nics, volumeUuids, snapshot, group, groupUuids, and groupRef
accordingly so the in(...) calls receive real UUID lists.
| private List<SystemTag> getResourceSystemTag(String resourceType) throws IllegalAccessException, InstantiationException { | ||
| List<SystemTag> systemTags = new ArrayList<>(); | ||
|
|
||
| Set<Class<?>> classes = BeanUtils.reflections.getTypesAnnotatedWith(TagDefinition.class); | ||
| for (Class clazz : classes) { | ||
| Field[] fields = clazz.getDeclaredFields(); | ||
| for (Field field : fields) { | ||
| if (!SystemTag.class.isAssignableFrom(field.getType())) { | ||
| continue; | ||
| } | ||
|
|
||
| SystemTag systemTag = (SystemTag) field.get(clazz.newInstance()); | ||
|
|
||
| if (resourceType.equals(systemTag.getResourceClass().getName())) { | ||
| systemTags.add(systemTag); | ||
| } | ||
| } | ||
| } | ||
| return systemTags; | ||
| } | ||
|
|
||
| private List<GlobalConfig> getResourceConfig(String resourceType) throws IllegalAccessException, InstantiationException { | ||
| List<GlobalConfig> globalConfigs = new ArrayList<>(); | ||
|
|
||
| Set<Class<?>> classes = BeanUtils.reflections.getTypesAnnotatedWith(GlobalConfigDefinition.class); | ||
| for (Class clazz : classes) { | ||
| Field[] fields = clazz.getDeclaredFields(); | ||
| for (Field field : fields) { | ||
| if (!GlobalConfig.class.isAssignableFrom(field.getType())) { | ||
| continue; | ||
| } | ||
| GlobalConfig globalConfig = (GlobalConfig) field.get(clazz.newInstance()); | ||
|
|
||
| BindResourceConfig bindResourceConfig = field.getAnnotation(BindResourceConfig.class); | ||
| if (bindResourceConfig == null) { | ||
| continue; | ||
| } | ||
|
|
||
| List<String> bindResourceConfigs = Arrays.stream(bindResourceConfig.value()).map(Class::getName).collect(Collectors.toList()); | ||
|
|
||
| if (bindResourceConfigs.contains(resourceType)) { | ||
| globalConfigs.add(globalConfig); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
资源类型匹配与 identity 比较逻辑错误
resourceType 传的是 SimpleName,但这里比较的是 Class.getName(),会导致永远不匹配;此外 it.getIdentity() == identity 用了引用比较且条件反了,会把不匹配的配置都写进去。
🛠️ 建议修正
- if (resourceType.equals(systemTag.getResourceClass().getName())) {
+ if (resourceType.equals(systemTag.getResourceClass().getSimpleName())) {
systemTags.add(systemTag);
}- if (it.getIdentity() == identity) {
- return;
- }
+ if (!it.getIdentity().equals(identity)) {
+ return;
+ }
ResourceConfigVO vo = new ResourceConfigVO();📝 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.
| private List<SystemTag> getResourceSystemTag(String resourceType) throws IllegalAccessException, InstantiationException { | |
| List<SystemTag> systemTags = new ArrayList<>(); | |
| Set<Class<?>> classes = BeanUtils.reflections.getTypesAnnotatedWith(TagDefinition.class); | |
| for (Class clazz : classes) { | |
| Field[] fields = clazz.getDeclaredFields(); | |
| for (Field field : fields) { | |
| if (!SystemTag.class.isAssignableFrom(field.getType())) { | |
| continue; | |
| } | |
| SystemTag systemTag = (SystemTag) field.get(clazz.newInstance()); | |
| if (resourceType.equals(systemTag.getResourceClass().getName())) { | |
| systemTags.add(systemTag); | |
| } | |
| } | |
| } | |
| return systemTags; | |
| } | |
| private List<GlobalConfig> getResourceConfig(String resourceType) throws IllegalAccessException, InstantiationException { | |
| List<GlobalConfig> globalConfigs = new ArrayList<>(); | |
| Set<Class<?>> classes = BeanUtils.reflections.getTypesAnnotatedWith(GlobalConfigDefinition.class); | |
| for (Class clazz : classes) { | |
| Field[] fields = clazz.getDeclaredFields(); | |
| for (Field field : fields) { | |
| if (!GlobalConfig.class.isAssignableFrom(field.getType())) { | |
| continue; | |
| } | |
| GlobalConfig globalConfig = (GlobalConfig) field.get(clazz.newInstance()); | |
| BindResourceConfig bindResourceConfig = field.getAnnotation(BindResourceConfig.class); | |
| if (bindResourceConfig == null) { | |
| continue; | |
| } | |
| List<String> bindResourceConfigs = Arrays.stream(bindResourceConfig.value()).map(Class::getName).collect(Collectors.toList()); | |
| if (bindResourceConfigs.contains(resourceType)) { | |
| globalConfigs.add(globalConfig); | |
| } | |
| } | |
| private List<SystemTag> getResourceSystemTag(String resourceType) throws IllegalAccessException, InstantiationException { | |
| List<SystemTag> systemTags = new ArrayList<>(); | |
| Set<Class<?>> classes = BeanUtils.reflections.getTypesAnnotatedWith(TagDefinition.class); | |
| for (Class clazz : classes) { | |
| Field[] fields = clazz.getDeclaredFields(); | |
| for (Field field : fields) { | |
| if (!SystemTag.class.isAssignableFrom(field.getType())) { | |
| continue; | |
| } | |
| SystemTag systemTag = (SystemTag) field.get(clazz.newInstance()); | |
| if (resourceType.equals(systemTag.getResourceClass().getSimpleName())) { | |
| systemTags.add(systemTag); | |
| } | |
| } | |
| } | |
| return systemTags; | |
| } | |
| private List<GlobalConfig> getResourceConfig(String resourceType) throws IllegalAccessException, InstantiationException { | |
| List<GlobalConfig> globalConfigs = new ArrayList<>(); | |
| Set<Class<?>> classes = BeanUtils.reflections.getTypesAnnotatedWith(GlobalConfigDefinition.class); | |
| for (Class clazz : classes) { | |
| Field[] fields = clazz.getDeclaredFields(); | |
| for (Field field : fields) { | |
| if (!GlobalConfig.class.isAssignableFrom(field.getType())) { | |
| continue; | |
| } | |
| GlobalConfig globalConfig = (GlobalConfig) field.get(clazz.newInstance()); | |
| BindResourceConfig bindResourceConfig = field.getAnnotation(BindResourceConfig.class); | |
| if (bindResourceConfig == null) { | |
| continue; | |
| } | |
| List<String> bindResourceConfigs = Arrays.stream(bindResourceConfig.value()).map(Class::getName).collect(Collectors.toList()); | |
| if (bindResourceConfigs.contains(resourceType)) { | |
| globalConfigs.add(globalConfig); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java` around lines
9592 - 9635, getResourceSystemTag and getResourceConfig are comparing
resourceType incorrectly and using wrong identity comparison logic: resourceType
is a SimpleName but code compares to Class.getName(), and identity comparison
uses reference equality with reversed logic. Update getResourceSystemTag to
compare resourceType with systemTag.getResourceClass().getSimpleName(); in
getResourceConfig ensure you compare bindResourceConfig value entries'
Class.getSimpleName() (or convert resourceType to FQN consistently) and change
the identity check from "it.getIdentity() == identity" to use equals
(it.getIdentity().equals(identity)) and ensure the condition only adds matching
configs (invert the current conditional if necessary). Use the symbols
SystemTag.getResourceClass(), BindResourceConfig, getResourceConfig,
getResourceSystemTag, and getIdentity() to locate and fix the logic.
| public interface UpdateVmInstanceMetadataExtensionPoint { | ||
| void buildVmInstanceMetadata(VmInstanceInventory vmInstanceInventory, VmMetadata vmMetadata); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
接口方法缺少 Javadoc
接口方法应提供 Javadoc 说明用途与参数。
🛠️ 建议修复
public interface UpdateVmInstanceMetadataExtensionPoint {
+ /**
+ * 构建/补充 VM 元数据。
+ *
+ * `@param` vmInstanceInventory VM 实例信息
+ * `@param` vmMetadata 元数据容器(可在此填充)
+ */
void buildVmInstanceMetadata(VmInstanceInventory vmInstanceInventory, VmMetadata vmMetadata);
}🤖 Prompt for AI Agents
In
`@header/src/main/java/org/zstack/header/vm/UpdateVmInstanceMetadataExtensionPoint.java`
around lines 6 - 8, 为 UpdateVmInstanceMetadataExtensionPoint 接口的
buildVmInstanceMetadata 方法添加完整的 Javadoc,说明该方法的用途(例如:在何时/为谁构建或修改 VM
的元数据)、每个参数的含义(VmInstanceInventory 表示哪个虚拟机的详情,VmMetadata
表示将被构建或填充的元数据对象)以及方法的预期副作用或返回行为(是否可修改
vmMetadata、线程安全或异常契约)。在注释中还应指出实现者需注意的约束或调用时机(如果适用),并在注释头部包含 `@param` 标签和必要的
`@throws/`@see 说明以符合代码规范。
| package org.zstack.header.vm; | ||
|
|
||
| import org.zstack.header.host.HostMessage; | ||
| import org.zstack.header.message.NeedReplyMessage; | ||
|
|
||
| public class UpdateVmInstanceMetadataOnHypervisorMsg extends NeedReplyMessage implements HostMessage { | ||
| private String vmInstanceMetadata; | ||
| private String hostUuid; | ||
|
|
||
| public String getVmInstanceMetadata() { | ||
| return vmInstanceMetadata; | ||
| } | ||
|
|
||
| public void setVmInstanceMetadata(String vmInstanceMetadata) { | ||
| this.vmInstanceMetadata = vmInstanceMetadata; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# 查找 `@NoLogging` 注解是否存在
rg -t java "@NoLogging" --max-count=10 -A 2 -B 2Repository: MatheMatrix/zstack
Length of output: 180
🏁 Script executed:
# 查找 UpdateVmInstanceMetadataOnHypervisorMsg 的使用位置
rg -t java "UpdateVmInstanceMetadataOnHypervisorMsg" -A 3 -B 3Repository: MatheMatrix/zstack
Length of output: 180
🏁 Script executed:
# 查找类似的元数据相关消息类
rg -t java "Metadata.*Msg" --max-count=15Repository: MatheMatrix/zstack
Length of output: 5570
🏁 Script executed:
# 查找所有使用 `@NoLogging` 注解的字段,了解使用模式
rg -t java "class.*Message" -A 20 | rg "@NoLogging" -B 3 -A 1 | head -50Repository: MatheMatrix/zstack
Length of output: 4281
🏁 Script executed:
# 查看 UpdateVmInstanceMetadataMsg 的完整内容
cat header/src/main/java/org/zstack/header/vm/UpdateVmInstanceMetadataMsg.javaRepository: MatheMatrix/zstack
Length of output: 480
🏁 Script executed:
# 查看 VmInstanceBase.java 中如何使用 UpdateVmInstanceMetadataOnHypervisorMsg
rg -t java "UpdateVmInstanceMetadataOnHypervisorMsg" -B 5 -A 15 compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.javaRepository: MatheMatrix/zstack
Length of output: 1213
🏁 Script executed:
# 查看 KVMHost.java 中如何处理这个消息
rg -t java "handle\(UpdateVmInstanceMetadataOnHypervisorMsg" -B 2 -A 20 plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaRepository: MatheMatrix/zstack
Length of output: 1003
🏁 Script executed:
# 查看 buildVmInstanceMetadata 方法的实现
rg -t java "buildVmInstanceMetadata" -B 2 -A 30 compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.javaRepository: MatheMatrix/zstack
Length of output: 23004
🏁 Script executed:
# 查看 `@NoLogging` 注解的定义,了解其用途
fd -t java "NoLogging.java" -x cat {}Repository: MatheMatrix/zstack
Length of output: 233
🏁 Script executed:
# 查看 NoLogging 注解的定义
find header -name "NoLogging.java" -type f -exec cat {} \;Repository: MatheMatrix/zstack
Length of output: 1034
🏁 Script executed:
# 检查是否有其他消息类的类似字段使用了 `@NoLogging`
rg -t java "Metadata.*;" -B 5 -A 5 header/src/main/java/org/zstack/header/ | grep -A 10 -B 10 "NoLogging"Repository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
# 查看 UpdateVmInstanceMetadataOnHypervisorMsg 类的完整定义
cat header/src/main/java/org/zstack/header/vm/UpdateVmInstanceMetadataOnHypervisorMsg.javaRepository: MatheMatrix/zstack
Length of output: 742
vmInstanceMetadata 字段应添加 @NoLogging 注解以屏蔽敏感信息。
该字段包含 VM 的完整元数据配置(VM 对象、系统标签、资源配置、网络、卷、快照等信息),可能包含系统敏感配置。消息在日志打印时存在信息泄露风险,应标记为禁日志字段。
建议修改
package org.zstack.header.vm;
import org.zstack.header.host.HostMessage;
import org.zstack.header.message.NeedReplyMessage;
+import org.zstack.header.log.NoLogging;
public class UpdateVmInstanceMetadataOnHypervisorMsg extends NeedReplyMessage implements HostMessage {
+ `@NoLogging`
private String vmInstanceMetadata;
private String hostUuid;🤖 Prompt for AI Agents
In
`@header/src/main/java/org/zstack/header/vm/UpdateVmInstanceMetadataOnHypervisorMsg.java`
around lines 1 - 16, Add the `@NoLogging` annotation to the vmInstanceMetadata
field in the UpdateVmInstanceMetadataOnHypervisorMsg class to prevent sensitive
VM metadata from being logged: annotate the private String vmInstanceMetadata
field with `@NoLogging`, add the corresponding import for the NoLogging
annotation, and keep the existing getter/setter unchanged so logging is
suppressed for this field at source.
| public String vmInstanceVO; | ||
| // key = vmUuid | ||
| // value = SystemTag | ||
| public List<String> vmSystemTags; | ||
| // key = vmUuid | ||
| // value = ResourceConfig | ||
| public List<String> vmResourceConfigs; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
字段注释与类型不一致
Line 8-13 的注释描述了 key/value,但字段类型是 List<String>。建议更新注释以匹配实际结构,或调整为 Map。
🛠️ 建议修复(更新注释)
- // key = vmUuid
- // value = SystemTag
+ // SystemTag.toString 列表(包含 vmUuid)
public List<String> vmSystemTags;
- // key = vmUuid
- // value = ResourceConfig
+ // ResourceConfig.toString 列表(包含 vmUuid)
public List<String> vmResourceConfigs;🤖 Prompt for AI Agents
In `@header/src/main/java/org/zstack/header/vm/VmMetadata.java` around lines 7 -
13, The field comments for vmSystemTags and vmResourceConfigs describe a
key/value map but the actual types are List<String>; update the comments to
accurately describe their current structure (e.g., "list of system tag strings"
and "list of resource config strings") or change the field types to a Map keyed
by vm UUID if a key/value structure is intended; locate and modify the
declarations vmSystemTags and vmResourceConfigs in class VmMetadata (and
vmInstanceVO if its comment needs alignment) so the comment text matches the
chosen type.
| // key = volumeUuid vmUuid | ||
| // value = vo | ||
| public Map<String, String> EncryptedResourceKeyRefVO; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
字段命名不符合 lowerCamelCase
EncryptedResourceKeyRefVO 以大写开头,不符合命名规范,也可能影响序列化字段名一致性。建议改为 encryptedResourceKeyRefVO 并同步更新引用。
🛠️ 建议修复
- public Map<String, String> EncryptedResourceKeyRefVO;
+ public Map<String, String> encryptedResourceKeyRefVO;📝 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.
| // key = volumeUuid vmUuid | |
| // value = vo | |
| public Map<String, String> EncryptedResourceKeyRefVO; | |
| // key = volumeUuid vmUuid | |
| // value = vo | |
| public Map<String, String> encryptedResourceKeyRefVO; |
🤖 Prompt for AI Agents
In `@header/src/main/java/org/zstack/header/vm/VmMetadata.java` around lines 48 -
50, Rename the public field VmMetadata.EncryptedResourceKeyRefVO to follow
lowerCamelCase (encryptedResourceKeyRefVO) and update all references
accordingly: change the field declaration in class VmMetadata, adjust any direct
usages, getters/setters, constructor params, and any
serialization/deserialization mappings (e.g., JSON/XML or reflection-based code)
that rely on the old name so the external field name remains consistent; ensure
compilation by updating imports/usages across the codebase and run tests to
verify no serialization breakage.
| private void handle(UpdateVmInstanceMetadataOnHypervisorMsg msg) { | ||
| UpdateVmInstanceMetadataReply reply = new UpdateVmInstanceMetadataReply(); | ||
|
|
||
| // before | ||
| // sblk 激活 | ||
|
|
||
| UpdateVmInstanceMetadataCmd cmd = new UpdateVmInstanceMetadataCmd(); | ||
| cmd.vmInstanceMetadata = msg.getVmInstanceMetadata(); | ||
| new Http<>(updateVmInstanceMetadataPath, cmd, UpdateVmInstanceMetadataRsp.class).call(new ReturnValueCompletion<UpdateVmInstanceMetadataRsp>(msg) { | ||
| @Override | ||
| public void success(UpdateVmInstanceMetadataRsp ret) { | ||
| if (!ret.isSuccess()) { | ||
| reply.setError(operr("operation error, because:%s", ret.getError())); | ||
| } | ||
| bus.reply(msg, reply); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
回复类型疑似不匹配
该方法处理的是 UpdateVmInstanceMetadataOnHypervisorMsg,但构造的是 UpdateVmInstanceMetadataReply,可能导致消息总线反序列化或字段回填不一致。请改为对应的 OnHypervisorReply,并按需回填响应字段。
Line 4154
🐛 建议修正
- UpdateVmInstanceMetadataReply reply = new UpdateVmInstanceMetadataReply();
+ UpdateVmInstanceMetadataOnHypervisorReply reply = new UpdateVmInstanceMetadataOnHypervisorReply();🤖 Prompt for AI Agents
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java` around lines 4153 -
4167, The method handle(UpdateVmInstanceMetadataOnHypervisorMsg msg) is
constructing the wrong reply type (UpdateVmInstanceMetadataReply) for an
OnHypervisor message; replace it with the correct
UpdateVmInstanceMetadataOnHypervisorReply, populate its success/error and any
response fields from UpdateVmInstanceMetadataRsp (e.g. setError/opStatus or
specific metadata fields as appropriate) and send that via bus.reply(msg, reply)
so the bus deserializes and fills back correctly; locate the construction and
success/failure handling around UpdateVmInstanceMetadataReply and
UpdateVmInstanceMetadataCmd in this method and switch to
UpdateVmInstanceMetadataOnHypervisorReply while preserving the existing
ret.isSuccess() logic to set error details.
APIImpact Resolves: ZSV-1 Change-Id: I6b617169616f72697766686974756465716b6c74
70d72bf to
e2b988a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@compute/src/main/java/org/zstack/compute/vm/UpdateVmInstanceMetadataGC.java`:
- Around line 50-52: The message routing uses the wrong service id:
UpdateVmInstanceMetadataMsg implements VmInstanceMessage and should be routed to
the VM service, not the volume service. In the call to
bus.makeTargetServiceIdByResourceUuid(msg, VolumeConstant.SERVICE_ID,
vmInstanceUuid) replace VolumeConstant.SERVICE_ID with the VM service constant
(e.g., VmInstanceConstant.SERVICE_ID) so UpdateVmInstanceMetadataMsg is sent to
the VM service using the same vmInstanceUuid.
In `@header/src/main/java/org/zstack/header/vm/APIRegVmInstanceMsg.java`:
- Around line 38-41: The __example__ method of APIRegVmInstanceMsg currently
returns an empty instance; update APIRegVmInstanceMsg.__example__ to populate
the message with realistic example values for its primary properties (e.g.,
setName, setUuid, setImageUuid, setInstanceOfferingUuid, setL3NetworkUuids or
other required setters present on APIRegVmInstanceMsg) so generated API docs
show meaningful sample data; ensure you use the actual setter method names
defined on APIRegVmInstanceMsg to assign non-null example strings/UUIDs and any
mandatory lists before returning msg.
- Around line 22-36: APIRegVmInstanceMsg defines private fields clusterUuid,
hostUuid, defaultL3NetworkUuid, l3NetworkUuids and vmNicParams but provides no
accessors; add standard Java bean getters and setters
(getClusterUuid/setClusterUuid, getHostUuid/setHostUuid,
getDefaultL3NetworkUuid/setDefaultL3NetworkUuid,
getL3NetworkUuids/setL3NetworkUuids, getVmNicParams/setVmNicParams) in the
APIRegVmInstanceMsg class so the framework and callers can read and modify these
properties.
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java`:
- Around line 4720-4725: The vmInstanceMetadata field on
UpdateVmInstanceMetadataCmd contains sensitive VM configuration and must not be
logged; add the `@NoLogging`(behavior = NoLogging.Behavior.Auto) annotation to the
vmInstanceMetadata field in the UpdateVmInstanceMetadataCmd class and import the
NoLogging annotation (matching how KVMHostHttpCallMsg.command is handled) so the
agent framework omits this field from logs.
♻️ Duplicate comments (10)
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java (1)
4155-4177: 回复类型疑似不匹配,需要改为 OnHypervisorReply。
当前处理的是 UpdateVmInstanceMetadataOnHypervisorMsg,但构造的是 UpdateVmInstanceMetadataReply,可能导致消息总线反序列化或字段回填不一致。🐛 建议修正
- UpdateVmInstanceMetadataReply reply = new UpdateVmInstanceMetadataReply(); + UpdateVmInstanceMetadataOnHypervisorReply reply = new UpdateVmInstanceMetadataOnHypervisorReply();header/src/main/java/org/zstack/header/vm/APIRegVmInstanceMsg.java (2)
16-26: 缺少与路径参数{uuid}对应的字段路径中包含
{uuid}但消息类中没有定义对应的uuid字段,REST 绑定将会失败。
30-30:defaultL3NetworkUuid缺少@APIParam注解该字段未标注
@APIParam,将缺失资源校验与 API 文档元数据。compute/src/main/java/org/zstack/compute/vm/UpdateVmInstanceMetadataGC.java (2)
28-33: 存在性校验使用了错误的实体类
dbf.isExist(vmInstanceUuid, VolumeVO.class)使用VolumeVO校验 VM UUID,语义不正确,可能导致 GC 被错误取消。
49-62:ChainTask.run()未调用chain.next()释放任务链当前回调完成后没有调用
chain.next(),会导致同步队列被永久占用,后续任务无法执行。compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java (5)
9402-9505: APIRecoverVmInstanceMsg 处理器未返回事件且选择 VM 错误。
当前实现既不 publish 事件/回复,也用全表查询并取get(0),会导致调用方挂起且恢复错误 VM/越界。建议恢复原有 recover 流程并严格使用 msg 目标 VM。🛠️ 建议修复
- private void handle(final APIRecoverVmInstanceMsg msg) { - List<VmInstanceVO> vms = Q.New(VmInstanceVO.class).list(); - String json = buildVmInstanceMetadata(vms.get(0).getUuid()); - ... - } + private void handle(final APIRecoverVmInstanceMsg msg) { + thdf.chainSubmit(new ChainTask(msg) { + `@Override` + public String getSyncSignature() { + return syncThreadName; + } + + `@Override` + public void run(final SyncTaskChain chain) { + final APIRecoverVmInstanceEvent evt = new APIRecoverVmInstanceEvent(msg.getId()); + refreshVO(); + + ErrorCode error = validateOperationByState(msg, self.getState(), SysErrors.OPERATION_ERROR); + if (error != null) { + evt.setError(error); + bus.publish(evt); + chain.next(); + return; + } + + recoverVm(new Completion(msg, chain) { + `@Override` + public void success() { + evt.setInventory(getSelfInventory()); + bus.publish(evt); + chain.next(); + } + + `@Override` + public void fail(ErrorCode errorCode) { + evt.setError(errorCode); + bus.publish(evt); + chain.next(); + } + }); + } + + `@Override` + public String getName() { + return "recover-vm"; + } + }); + }
9515-9517: SystemTag 查询实体类型错误。
Q.New(SystemTag.class)与SystemTagVO_不匹配,可能直接失败。应查询SystemTagVO。🛠️ 建议修复
- List<Tuple> tuples = Q.New(SystemTag.class).eq(SystemTagVO_.resourceUuid, vm.getUuid()) + List<Tuple> tuples = Q.New(SystemTagVO.class).eq(SystemTagVO_.resourceUuid, vm.getUuid()) .select(SystemTagVO_.tag, SystemTagVO_.inherent, SystemTagVO_.type).listValues();
9544-9552: NIC 查询条件与快照/组 UUID 列表未填充,导致元数据缺失。
NIC 查询使用了VolumeVO_元模型,且volumeUuids/groupUuids为空会让快照/组引用恒空。🛠️ 建议修复
- List<VmNicVO> nics = Q.New(VmNicVO.class).eq(VolumeVO_.vmInstanceUuid, vmInstanceUuid).list(); - - List<String> volumeUuids = new ArrayList<>(); + List<VmNicVO> nics = Q.New(VmNicVO.class) + .eq(VmNicVO_.vmInstanceUuid, vmInstanceUuid) + .list(); + + List<String> volumeUuids = volumes.stream() + .map(VolumeVO::getUuid) + .collect(Collectors.toList()); List<VolumeSnapshotVO> snapshot = Q.New(VolumeSnapshotVO.class).in(VolumeSnapshotVO_.volumeUuid, volumeUuids).list(); List<VolumeSnapshotGroupVO> group = Q.New(VolumeSnapshotGroupVO.class).eq(VolumeSnapshotGroupVO_.vmInstanceUuid, vmInstanceUuid).list(); - List<String> groupUuids = new ArrayList<>(); + List<String> groupUuids = group.stream() + .map(VolumeSnapshotGroupVO::getUuid) + .collect(Collectors.toList()); List<VolumeSnapshotGroupRefVO> groupRef = Q.New(VolumeSnapshotGroupRefVO.class).in(VolumeSnapshotGroupRefVO_.volumeSnapshotGroupUuid, groupUuids).list();
9613-9656: 资源类型匹配使用 FQN,导致系统标签/配置无法命中。
resourceType传入的是SimpleName,但这里比较/构造的是getName(),结果会恒不匹配。🛠️ 建议修复
- if (resourceType.equals(systemTag.getResourceClass().getName())) { + if (resourceType.equals(systemTag.getResourceClass().getSimpleName())) { systemTags.add(systemTag); } ... - List<String> bindResourceConfigs = Arrays.stream(bindResourceConfig.value()).map(Class::getName).collect(Collectors.toList()); + List<String> bindResourceConfigs = Arrays.stream(bindResourceConfig.value()) + .map(Class::getSimpleName) + .collect(Collectors.toList());
9662-9709: UpdateVmInstanceMetadataMsg 失败路径未回复且 hostUuid 可能为空。
失败分支只触发 GC 未回复/chain.next,会阻塞队列;同时 hostUuid 为空会 NPE。🛠️ 建议修复
public void run(SyncTaskChain chain) { UpdateVmInstanceMetadataReply reply = new UpdateVmInstanceMetadataReply(); UpdateVmInstanceMetadataOnHypervisorMsg umsg = new UpdateVmInstanceMetadataOnHypervisorMsg(); umsg.setVmInstanceMetadata(buildVmInstanceMetadata(msg.getUuid())); if (self.getHostUuid() != null) { umsg.setHostUuid(self.getHostUuid()); } else if (self.getLastHostUuid() != null) { umsg.setHostUuid(self.getLastHostUuid()); + } else { + reply.setError(operr("vm[uuid:%s] has no host for metadata update", self.getUuid())); + bus.reply(msg, reply); + chain.next(); + return; } bus.makeTargetServiceIdByResourceUuid(umsg, HostConstant.SERVICE_ID, umsg.getHostUuid()); bus.send(umsg, new CloudBusCallBack(msg) { `@Override` public void run(MessageReply innerReply) { if (!innerReply.isSuccess()) { reply.setError(Platform.operr("failed to update vm[uuid=%s] on hypervisor.", self.getUuid()) .withCause(innerReply.getError())); - gc(); + bus.reply(msg, reply); + chain.next(); return; } bus.reply(msg, reply); chain.next(); }
| UpdateVmInstanceMetadataMsg msg = new UpdateVmInstanceMetadataMsg(); | ||
| msg.setUuid(vmInstanceUuid); | ||
| bus.makeTargetServiceIdByResourceUuid(msg, VolumeConstant.SERVICE_ID, vmInstanceUuid); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
消息路由使用了错误的 SERVICE_ID
UpdateVmInstanceMetadataMsg 实现了 VmInstanceMessage 接口,应路由到 VM 服务,而非 Volume 服务。当前使用 VolumeConstant.SERVICE_ID 会导致消息被发送到错误的服务处理。
🛠️ 建议修改
-import org.zstack.header.volume.VolumeConstant;
-import org.zstack.header.volume.VolumeVO;
+import org.zstack.header.vm.VmInstanceConstant;
+import org.zstack.header.vm.VmInstanceVO;- bus.makeTargetServiceIdByResourceUuid(msg, VolumeConstant.SERVICE_ID, vmInstanceUuid);
+ bus.makeTargetServiceIdByResourceUuid(msg, VmInstanceConstant.SERVICE_ID, vmInstanceUuid);🤖 Prompt for AI Agents
In `@compute/src/main/java/org/zstack/compute/vm/UpdateVmInstanceMetadataGC.java`
around lines 50 - 52, The message routing uses the wrong service id:
UpdateVmInstanceMetadataMsg implements VmInstanceMessage and should be routed to
the VM service, not the volume service. In the call to
bus.makeTargetServiceIdByResourceUuid(msg, VolumeConstant.SERVICE_ID,
vmInstanceUuid) replace VolumeConstant.SERVICE_ID with the VM service constant
(e.g., VmInstanceConstant.SERVICE_ID) so UpdateVmInstanceMetadataMsg is sent to
the VM service using the same vmInstanceUuid.
| public class APIRegVmInstanceMsg extends APIMessage { | ||
| @APIParam(resourceType = ClusterVO.class) | ||
| private String clusterUuid; | ||
| @APIParam(resourceType = HostVO.class) | ||
| private String hostUuid; | ||
|
|
||
|
|
||
|
|
||
| private String defaultL3NetworkUuid; | ||
|
|
||
| @APIParam(resourceType = L3NetworkVO.class, required = false) | ||
| private List<String> l3NetworkUuids; | ||
|
|
||
| @APIParam(required = false) | ||
| private String vmNicParams; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
私有字段缺少 getter/setter 方法
所有字段均为 private,但类中未提供任何访问器方法,导致这些字段无法被框架或调用方访问和设置。
🛠️ 建议补充访问器方法
`@APIParam`(required = false)
private String vmNicParams;
-
+
+ public String getClusterUuid() {
+ return clusterUuid;
+ }
+
+ public void setClusterUuid(String clusterUuid) {
+ this.clusterUuid = clusterUuid;
+ }
+
+ public String getHostUuid() {
+ return hostUuid;
+ }
+
+ public void setHostUuid(String hostUuid) {
+ this.hostUuid = hostUuid;
+ }
+
+ public String getDefaultL3NetworkUuid() {
+ return defaultL3NetworkUuid;
+ }
+
+ public void setDefaultL3NetworkUuid(String defaultL3NetworkUuid) {
+ this.defaultL3NetworkUuid = defaultL3NetworkUuid;
+ }
+
+ public List<String> getL3NetworkUuids() {
+ return l3NetworkUuids;
+ }
+
+ public void setL3NetworkUuids(List<String> l3NetworkUuids) {
+ this.l3NetworkUuids = l3NetworkUuids;
+ }
+
+ public String getVmNicParams() {
+ return vmNicParams;
+ }
+
+ public void setVmNicParams(String vmNicParams) {
+ this.vmNicParams = vmNicParams;
+ }
+
public static APIRegVmInstanceMsg __example__() {🤖 Prompt for AI Agents
In `@header/src/main/java/org/zstack/header/vm/APIRegVmInstanceMsg.java` around
lines 22 - 36, APIRegVmInstanceMsg defines private fields clusterUuid, hostUuid,
defaultL3NetworkUuid, l3NetworkUuids and vmNicParams but provides no accessors;
add standard Java bean getters and setters (getClusterUuid/setClusterUuid,
getHostUuid/setHostUuid, getDefaultL3NetworkUuid/setDefaultL3NetworkUuid,
getL3NetworkUuids/setL3NetworkUuids, getVmNicParams/setVmNicParams) in the
APIRegVmInstanceMsg class so the framework and callers can read and modify these
properties.
| public static APIRegVmInstanceMsg __example__() { | ||
| APIRegVmInstanceMsg msg = new APIRegVmInstanceMsg(); | ||
| return msg; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
__example__() 方法未设置示例值
根据编码规范,API 类需要实现 __example__ 方法以便生成 API 文档。当前实现返回空实例,无法为文档生成有意义的示例数据。
🛠️ 建议补充示例值
public static APIRegVmInstanceMsg __example__() {
APIRegVmInstanceMsg msg = new APIRegVmInstanceMsg();
+ msg.setClusterUuid(uuid());
+ msg.setHostUuid(uuid());
+ msg.setDefaultL3NetworkUuid(uuid());
return msg;
}🤖 Prompt for AI Agents
In `@header/src/main/java/org/zstack/header/vm/APIRegVmInstanceMsg.java` around
lines 38 - 41, The __example__ method of APIRegVmInstanceMsg currently returns
an empty instance; update APIRegVmInstanceMsg.__example__ to populate the
message with realistic example values for its primary properties (e.g., setName,
setUuid, setImageUuid, setInstanceOfferingUuid, setL3NetworkUuids or other
required setters present on APIRegVmInstanceMsg) so generated API docs show
meaningful sample data; ensure you use the actual setter method names defined on
APIRegVmInstanceMsg to assign non-null example strings/UUIDs and any mandatory
lists before returning msg.
| public static class UpdateVmInstanceMetadataCmd extends AgentCommand { | ||
| public String vmInstanceMetadata; | ||
| } | ||
|
|
||
| public static class UpdateVmInstanceMetadataRsp extends AgentResponse { | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# 首先定位 KVMAgentCommands.java 文件并验证代码片段
find . -name "KVMAgentCommands.java" -type fRepository: MatheMatrix/zstack
Length of output: 125
🏁 Script executed:
# 搜索 `@NoLogging` 注解在项目中的定义和使用
rg "@NoLogging" --type java -A 2 -B 2Repository: MatheMatrix/zstack
Length of output: 50374
🏁 Script executed:
# 检查 AgentCommand 和 AgentResponse 的定义和日志机制
rg "class AgentCommand|class AgentResponse" --type java -A 5Repository: MatheMatrix/zstack
Length of output: 15498
🏁 Script executed:
# 搜索项目中其他包含元数据的字段,看如何处理
rg "metadata" --type java -i | head -30Repository: MatheMatrix/zstack
Length of output: 4785
🏁 Script executed:
# 查看文件第 4720-4725 行附近的内容
sed -n '4715,4735p' ./plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.javaRepository: MatheMatrix/zstack
Length of output: 385
🏁 Script executed:
# 搜索 UpdateVmInstanceMetadataCmd 的具体定义
rg "UpdateVmInstanceMetadataCmd" --type java -A 5 -B 2Repository: MatheMatrix/zstack
Length of output: 1641
🏁 Script executed:
# 检查是否有其他类似的元数据命令类
rg "class Update.*Metadata.*Cmd" --type java -A 3Repository: MatheMatrix/zstack
Length of output: 433
🏁 Script executed:
# 查看 vmInstanceMetadata 在项目中的使用情况
rg "vmInstanceMetadata" --type java -C 3Repository: MatheMatrix/zstack
Length of output: 3240
🏁 Script executed:
# 检查 UpdateVmInstanceMetadataOnHypervisorMsg 的完整定义
cat -n header/src/main/java/org/zstack/header/vm/UpdateVmInstanceMetadataOnHypervisorMsg.javaRepository: MatheMatrix/zstack
Length of output: 924
🏁 Script executed:
# 搜索如何构建和设置 vmInstanceMetadata 的代码
rg "setVmInstanceMetadata|vmInstanceMetadata\s*=" --type java -B 3 -A 1 | head -50Repository: MatheMatrix/zstack
Length of output: 1994
🏁 Script executed:
# 查找可能的安全敏感性相关的注释
rg "metadata" --type java -i -B 2 -A 2 | grep -i "password\|credential\|secret\|sensitive\|user.*data" | head -20Repository: MatheMatrix/zstack
Length of output: 2848
🏁 Script executed:
# 搜索 buildVmInstanceMetadata 方法的实现
rg "buildVmInstanceMetadata" --type java -A 20 -B 2 | head -80Repository: MatheMatrix/zstack
Length of output: 22371
🏁 Script executed:
# 也检查是否有其他地方设置 vmInstanceMetadata
rg "vmInstanceMetadata\s*=" --type java -B 2 -A 2Repository: MatheMatrix/zstack
Length of output: 1211
🏁 Script executed:
# 检查 `@NoLogging` 注解的定义,看其使用准则
rg "public.*@interface.*NoLogging" --type java -A 30Repository: MatheMatrix/zstack
Length of output: 2388
🏁 Script executed:
# 查看 NoLogging 注解的完整定义
cat -n header/src/main/java/org/zstack/header/log/NoLogging.javaRepository: MatheMatrix/zstack
Length of output: 1377
🏁 Script executed:
# 检查 UpdateVmInstanceMetadataOnHypervisorMsg 中 vmInstanceMetadata 是否需要脱敏
rg "UpdateVmInstanceMetadataOnHypervisorMsg" --type java -A 30 | grep -A 30 "class UpdateVmInstanceMetadataOnHypervisorMsg"Repository: MatheMatrix/zstack
Length of output: 3572
🏁 Script executed:
# 搜索项目中其他大型 JSON 或结构化数据字段如何处理脱敏
rg "@NoLogging.*behavior.*Auto|@NoLogging.*Behavior.Auto" --type java -B 3 -A 1 | head -40Repository: MatheMatrix/zstack
Length of output: 3961
为 vmInstanceMetadata 字段添加 @NoLogging 注解以防止日志泄露
vmInstanceMetadata 包含虚拟机的完整配置元数据(JSON 格式),涵盖网络配置、存储路径、系统标签和资源配置等敏感基础设施信息。在代理命令中打印该字段会导致这些信息被记录到日志。
建议添加 @NoLogging(behavior = NoLogging.Behavior.Auto) 以与项目中其他类似复杂数据结构(如 KVMHostHttpCallMsg.command)的处理方式保持一致。
🛠️ 建议修改
public static class UpdateVmInstanceMetadataCmd extends AgentCommand {
+ `@NoLogging`(behavior = NoLogging.Behavior.Auto)
public String vmInstanceMetadata;
}📝 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.
| public static class UpdateVmInstanceMetadataCmd extends AgentCommand { | |
| public String vmInstanceMetadata; | |
| } | |
| public static class UpdateVmInstanceMetadataRsp extends AgentResponse { | |
| } | |
| public static class UpdateVmInstanceMetadataCmd extends AgentCommand { | |
| `@NoLogging`(behavior = NoLogging.Behavior.Auto) | |
| public String vmInstanceMetadata; | |
| } | |
| public static class UpdateVmInstanceMetadataRsp extends AgentResponse { | |
| } |
🤖 Prompt for AI Agents
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java` around lines
4720 - 4725, The vmInstanceMetadata field on UpdateVmInstanceMetadataCmd
contains sensitive VM configuration and must not be logged; add the
`@NoLogging`(behavior = NoLogging.Behavior.Auto) annotation to the
vmInstanceMetadata field in the UpdateVmInstanceMetadataCmd class and import the
NoLogging annotation (matching how KVMHostHttpCallMsg.command is handled) so the
agent framework omits this field from logs.
Resolves: ZSV-1
Change-Id: I746a6d7266686d637a6d76756861636a74766b74
sync from gitlab !9002