<feature>[vm]: add MetadataImpact#3652
Open
MatheMatrix wants to merge 1 commit intofeature-zsv-5.0.0-vm-support-vtpm-and-secucebootfrom
Open
<feature>[vm]: add MetadataImpact#3652MatheMatrix wants to merge 1 commit intofeature-zsv-5.0.0-vm-support-vtpm-and-secucebootfrom
MatheMatrix wants to merge 1 commit intofeature-zsv-5.0.0-vm-support-vtpm-and-secucebootfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough新增 VM 实例元数据子系统，包含 API/消息/DTO/VO、扩展点、数据库表、主存储（Local/NFS）后端支持、调度/GC、SDK 与测试仿真，覆盖元数据写入、读取、扫描、替换、清理与注册流程。 Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant API as API 层
participant Compute as Compute 服务
participant PS as PrimaryStorage 服务
participant Host as Host/Agent
participant GC as 时间驱动 GC
Client->>API: 发起 Register/Scan/Get/Update/Cleanup 请求
API->>Compute: 参数校验并构造消息
Compute->>PS: 发送 Update/Get/Scan/Cleanup 消息（目标 primaryStorageUuid）
PS->>Host: 转发到选定 Host 的 agent HTTP 接口
Host-->>PS: 返回成功/失败响应
alt 成功
PS-->>Compute: reply 成功
Compute-->>API: 发布 Event / 返回结果
API-->>Client: 返回成功
else 失败且需重试
Compute->>GC: 提交/更新 Cleanup GC（含 vmUuid/psUuid/metadataPath/hostUuid）
GC->>PS: 定时重试发送 Cleanup 消息
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 分钟 Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Collaborator
|
Comment from yaohua.wu: Review: MR !9513 — ZSV-11559
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (18)
header/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyService.java (1)
20-28: 避免在接口方法中使用布尔参数表达行为分支。Line 28 的
boolean storageStructureChange会让调用点语义不直观（markDirty(uuid, true/false)可读性弱），建议改为拆分显式方法（如markDirtyWithStorageStructureChange）或使用枚举参数表达变更类型。As per coding guidelines, "避免使用布尔型参数造成含义不明确。建议拆分为不同函数，或使用枚举表达操作类型。"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyService.java` around lines 20 - 28, The markDirty API in VmMetadataDirtyService uses a boolean parameter storageStructureChange which makes call sites like markDirty(uuid, true) ambiguous; update the interface by replacing boolean storageStructureChange with an explicit operation indicator: either add two clear methods (e.g., markDirty(String vmInstanceUuid) and markDirtyWithStorageStructureChange(String vmInstanceUuid)) or change the signature to accept an enum (e.g., VmMetadataChangeType changeType) and update all implementors and callers to use the new methods/enum (locate VmMetadataDirtyService.markDirty) so callers become self-documenting and the implementations honor the new contract.header/src/main/java/org/zstack/header/vm/metadata/VmUuidFromApiResolver.java (2)
9-24: 可考虑使用 Java Stream 简化批量处理逻辑。当前实现逻辑正确，但根据编码规范建议使用 Java Stream 或 Lambda 表达式代替冗长的循环与条件判断，可提升代码简洁性。
♻️ 使用 Stream API 重构
default List<String> batchResolveVmUuids(List<String> fieldValues) { - List<String> result = new ArrayList<>(); if (fieldValues == null || fieldValues.isEmpty()) { - return result; - } - for (String v : fieldValues) { - if (v == null) { - continue; - } - String vmUuid = resolveVmUuid(v); - if (vmUuid != null) { - result.add(vmUuid); - } + return new ArrayList<>(); } - return result; + return fieldValues.stream() + .filter(v -> v != null) + .map(this::resolveVmUuid) + .filter(uuid -> uuid != null) + .collect(java.util.stream.Collectors.toList()); }或使用
Objects::nonNull使代码更简洁：return fieldValues.stream() .filter(Objects::nonNull) .map(this::resolveVmUuid) .filter(Objects::nonNull) .collect(Collectors.toList());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/vm/metadata/VmUuidFromApiResolver.java` around lines 9 - 24, The batchResolveVmUuids implementation in VmUuidFromApiResolver uses an explicit loop and null checks; refactor it to use the Stream API to make the code concise and readable by streaming fieldValues, filtering nulls, mapping via resolveVmUuid, filtering non-null results, and collecting to a List; update the default method batchResolveVmUuids to return fieldValues.stream().filter(Objects::nonNull).map(this::resolveVmUuid).filter(Objects::nonNull).collect(Collectors.toList()) (ensure imports for java.util.Objects and java.util.stream.Collectors are added).
6-8: 缺少接口和方法的 Javadoc 注释。根据编码规范，接口方法必须配有有效的 Javadoc 注释。建议为接口及其方法添加文档说明，包括：
- 接口的用途
resolveVmUuid方法的作用及fieldValue参数的含义- 返回值的预期行为（例如找不到时返回 null）
♻️ 建议添加 Javadoc
+/** + * Interface for resolving VM UUIDs from API field values. + */ public interface VmUuidFromApiResolver { + /** + * Resolves a VM UUID from the given API field value. + * + * `@param` fieldValue the field value from API request + * `@return` the resolved VM UUID, or null if not found + */ String resolveVmUuid(String fieldValue);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/vm/metadata/VmUuidFromApiResolver.java` around lines 6 - 8, Add proper Javadoc to the VmUuidFromApiResolver interface and its method resolveVmUuid: document the interface purpose (resolving a VM UUID from an API-provided value), describe the resolveVmUuid(String fieldValue) parameter meaning (what forms of fieldValue are accepted), and specify the return contract (what is returned on success and that it returns null when no VM UUID is found or cannot be resolved). Ensure the method Javadoc mentions any expected error handling/side-effects and the expected null behavior so callers know how to handle missing results.header/src/main/java/org/zstack/header/vm/metadata/VmMetadataErrors.java (1)
18-22: 建议将code设为不可变并简化字符串构造。Line 18 当前字段可变，但该值仅在构造时赋值；可改为
final以提升不可变性，同时用字符串拼接替代String.format。♻️ 建议修改
- private String code; + private final String code; private VmMetadataErrors(int id) { - code = String.format("VM_METADATA.%s", id); + this.code = "VM_METADATA." + id; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/vm/metadata/VmMetadataErrors.java` around lines 18 - 22, The field 'code' in the VmMetadataErrors enum is currently mutable and built with String.format in the private VmMetadataErrors(int id) constructor; make 'code' immutable by declaring it final (private final String code) and simplify construction by using plain string concatenation ("VM_METADATA." + id) inside the constructor or as an initializer so the value is set once and cannot change; update any usages accordingly (refer to VmMetadataErrors, the 'code' field, and the VmMetadataErrors(int id) constructor).header/src/main/java/org/zstack/header/storage/primary/ReadVmInstanceMetadataMsg.java (1)
7-20: 建议将uuid语义化为vmInstanceUuid，降低歧义。Line 7 及 Line 9-20 当前实现功能正确，但字段名过于泛化；在存储域消息里
uuid容易与其他资源 UUID 混淆。建议将内部字段和访问器改为更明确的 VM 语义命名。可选重构示例
public class ReadVmInstanceMetadataMsg extends NeedReplyMessage implements VmInstanceMessage { - private String uuid; + private String vmInstanceUuid; public String getUuid() { - return uuid; + return vmInstanceUuid; } public void setUuid(String uuid) { - this.uuid = uuid; + this.vmInstanceUuid = uuid; } `@Override` public String getVmInstanceUuid() { - return uuid; + return vmInstanceUuid; } }As per coding guidelines "命名应尽量用完整的单词组合表达意图，并在名称中体现数据类型或用途（例如在常量与变量名称中，将类型词放在末尾）"。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/storage/primary/ReadVmInstanceMetadataMsg.java` around lines 7 - 20, The field name uuid in ReadVmInstanceMetadataMsg is too generic; rename the field to vmInstanceUuid and update its accessor methods (replace getUuid/setUuid with getVmInstanceUuid/setVmInstanceUuid) so that getVmInstanceUuid() returns the new vmInstanceUuid field; also update all internal references/usages of ReadVmInstanceMetadataMsg, getUuid, and setUuid to the new names to avoid breaking callers and ensure serialization/deserialization annotations (if any) are adjusted accordingly.header/src/main/java/org/zstack/header/vm/metadata/VmMetadataResourcePersistExtensionPoint.java (1)
7-11: 建议为扩展点核心方法补充 Javadoc 契约说明
getPrimaryStorageType、afterVolumePersist、afterSnapshotPersist当前缺少方法级注释，resourceType/size/now的语义边界不够明确，建议补齐以减少不同实现间的行为偏差。可参考的最小补充示例
public interface VmMetadataResourcePersistExtensionPoint { + /** + * `@return` 当前扩展支持的主存储类型 + */ String getPrimaryStorageType(); + + /** + * 卷元数据落库后回调。 + */ void afterVolumePersist(String primaryStorageUuid, String resourceUuid, String resourceType, String hostUuid, long size, Timestamp now); + + /** + * 快照元数据落库后回调。 + */ void afterSnapshotPersist(String primaryStorageUuid, String resourceUuid, String resourceType, String hostUuid, long size, Timestamp now);As per coding guidelines "接口方法不应有多余的修饰符（例如 public），且必须配有有效的 Javadoc 注释。"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/vm/metadata/VmMetadataResourcePersistExtensionPoint.java` around lines 7 - 11, 为接口方法补充规范的 Javadoc：在 VmMetadataResourcePersistExtensionPoint 的 getPrimaryStorageType、afterVolumePersist 和 afterSnapshotPersist 上分别添加方法注释，说明 getPrimaryStorageType 返回的字符串语义（如主存储类型标识符）；在 afterVolumePersist/afterSnapshotPersist 中明确 resourceUuid 表示的资源（卷或快照）含义、resourceType 的取值范围/格式、size 的单位（例如字节）及其含义、hostUuid 的语义和可空性，以及 now 表示“已持久化的事件发生时间（Timestamp）”且是否应为系统时间；同时遵循接口方法无多余修饰符的规范（不要加 public 等）。sdk/src/main/java/org/zstack/sdk/RegisterVmInstanceFromMetadataResult.java (1)
3-3: 冗余导入语句。
VmInstanceInventory与当前类在同一包org.zstack.sdk中，此导入语句是多余的。♻️ 建议移除冗余导入
package org.zstack.sdk; -import org.zstack.sdk.VmInstanceInventory; - public class RegisterVmInstanceFromMetadataResult {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/src/main/java/org/zstack/sdk/RegisterVmInstanceFromMetadataResult.java` at line 3, The import of VmInstanceInventory at the top of RegisterVmInstanceFromMetadataResult is redundant because VmInstanceInventory is in the same package org.zstack.sdk; remove the line "import org.zstack.sdk.VmInstanceInventory;" from the RegisterVmInstanceFromMetadataResult class to clean up the unused/ redundant import.sdk/src/main/java/org/zstack/sdk/ScanVmInstanceMetadataFromPrimaryStorageResult.java (1)
6-11: SDK Result 类的原始List类型符合项目既有规范，建议作为更广泛的 SDK 重构考虑。虽然为
vmInstanceMetadata添加泛型List<VmMetadataScanEntry>能提升类型安全性，但检查表明：
- 所有 1503 个 SDK Result 类都统一使用原始
java.util.List（无泛型）- 这是项目 SDK 模块的既有设计约定，而非生成器产物
- 对应的内部类（header 模块）已使用泛型，类型转换在运行时通过类型擦除安全进行
单独修改此文件会与整个 SDK Result 类体系不一致。若要改进类型安全，建议作为项目级 SDK 重构统一处理，而不在单个 PR 中调整。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/src/main/java/org/zstack/sdk/ScanVmInstanceMetadataFromPrimaryStorageResult.java` around lines 6 - 11, The field and accessors in ScanVmInstanceMetadataFromPrimaryStorageResult should remain using the raw java.util.List type to keep consistency with the SDK Result classes; ensure the field vmInstanceMetadata and its methods setVmInstanceMetadata and getVmInstanceMetadata use plain java.util.List (no generics) and do not introduce List<VmMetadataScanEntry> here—if you already added generics, revert that change and leave any type-safety improvements to a coordinated, project-wide SDK refactor.header/src/main/java/org/zstack/header/vm/metadata/ResourceMetadata.java (1)
3-7: 考虑使用更具描述性的字段名。字段
vo的命名较为模糊，根据编码规范中关于命名的要求："不允许使用不必要的缩写...应使用完整单词提升可读性"。建议考虑使用更具描述性的名称，如voJson、voData或valueObject，以便更清晰地表达该字段的用途。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/vm/metadata/ResourceMetadata.java` around lines 3 - 7, The field name vo in class ResourceMetadata is ambiguous; rename it to a more descriptive identifier (e.g., voJson, voData, or valueObject) across the class and any callers to improve readability: update the private field declaration in ResourceMetadata, its getter/setter methods, constructor parameters, any JSON/property mappings, and all usages in codebase to use the new name to avoid breaking references.header/src/main/java/org/zstack/header/vm/metadata/MetadataImpact.java (1)
9-27: 建议把“CONFIG/STORAGE 必填字段”从注释约定升级为可校验约束当前
resolver/field的必填条件只在注释里表达，容易被误用后在运行期才失败。建议在元数据处理入口增加统一 fail-fast 校验；同时把updateOnFailure的注释改为“可选，默认 false”，避免“布尔必填”歧义。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/vm/metadata/MetadataImpact.java` around lines 9 - 27, The MetadataImpact annotation currently only documents that resolver/field are required for CONFIG or STORAGE and that updateOnFailure is optional; make this fail-fast by validating annotated usage at metadata-processing startup: when reading MetadataImpact.value() == Impact.CONFIG or Impact.STORAGE, assert that MetadataImpact.resolver() and MetadataImpact.field() are non-empty and throw a clear IllegalStateException (or similar) with the annotation/class/method context if they are missing; also update the MetadataImpact javadoc for updateOnFailure to read “optional, default false” to remove the “boolean required” ambiguity. Ensure the validation runs in the central metadata entry point that loads/inspects MetadataImpact annotations so misconfigurations fail at startup rather than at runtime.header/src/main/java/org/zstack/header/vm/metadata/VmInstanceMetadataDTO.java (1)
5-13: 建议初始化 List 字段以避免潜在的 NPE。
volumes、nics、snapshots、snapshotGroups和snapshotGroupRefs字段默认为null。如果调用方在反序列化后直接遍历这些字段而未做空值检查，可能导致NullPointerException。♻️ 建议初始化 List 字段
public class VmInstanceMetadataDTO { private String schemaVersion; private VmMetadataCategory vmCategory; private ResourceMetadata vm; - private List<VolumeResourceMetadata> volumes; - private List<ResourceMetadata> nics; - private List<String> snapshots; - private List<String> snapshotGroups; - private List<String> snapshotGroupRefs; + private List<VolumeResourceMetadata> volumes = new ArrayList<>(); + private List<ResourceMetadata> nics = new ArrayList<>(); + private List<String> snapshots = new ArrayList<>(); + private List<String> snapshotGroups = new ArrayList<>(); + private List<String> snapshotGroupRefs = new ArrayList<>();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/vm/metadata/VmInstanceMetadataDTO.java` around lines 5 - 13, The list fields volumes, nics, snapshots, snapshotGroups and snapshotGroupRefs in VmInstanceMetadataDTO are null by default and can cause NPEs; initialize them to empty lists (e.g., new ArrayList<>() or Collections.emptyList()) either at field declaration or in the class constructor (and ensure any deserialization path preserves empty lists) so callers can safely iterate without null checks; update VmInstanceMetadataDTO to set these defaults for the mentioned fields.plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsVmMetadataExtension.java (1)
91-100: 建议提取公共方法以减少重复代码。第 91-100 行的
baseDir解析逻辑与resolveBaseDir()方法（第 29-38 行）重复。建议重构以复用现有方法：♻️ 建议的重构方案
`@Override` public PathReplacementResult calculatePathReplacements(String targetPsUuid, List<String> allOldPaths) { if (allOldPaths == null || allOldPaths.isEmpty()) { PathReplacementResult result = new PathReplacementResult(); result.setMetadataToCurrentPathMap(Collections.emptyMap()); return result; } - String baseDir = Q.New(PrimaryStorageVO.class).select(PrimaryStorageVO_.mountPath).eq(PrimaryStorageVO_.uuid, targetPsUuid).findValue(); - if (baseDir == null) { - baseDir = Q.New(PrimaryStorageVO.class).select(PrimaryStorageVO_.url).eq(PrimaryStorageVO_.uuid, targetPsUuid).findValue(); - } - if (baseDir == null) { + String baseDir; + try { + baseDir = resolveBaseDir(targetPsUuid); + } catch (CloudRuntimeException e) { logger.warn(String.format("NFS PS[uuid:%s] has no mountPath or url, path replacement disabled", targetPsUuid)); PathReplacementResult result = new PathReplacementResult(); result.setMetadataToCurrentPathMap(Collections.emptyMap()); return result; } - String newPrefix = baseDir.endsWith("/") ? baseDir : baseDir + "/"; + String newPrefix = baseDir + "/";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsVmMetadataExtension.java` around lines 91 - 100, The baseDir resolution logic in NfsVmMetadataExtension duplicates code already implemented in resolveBaseDir(); refactor the block that queries PrimaryStorageVO (lines creating baseDir and checking null, plus the warning and empty PathReplacementResult) to reuse resolveBaseDir(targetPsUuid) or move the shared query into a new private method (e.g., resolveBaseDir or getPrimaryStorageBaseDir) and call it from both places (ensure you preserve the warning log and the empty PathReplacementResult behavior when the result is null); update references in the method that currently contains the duplicated code (the code that constructs PathReplacementResult) to call the shared method instead of repeating the queries.plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java (3)
3424-3428: 简化冗余的 null 检查逻辑当前代码模式可以简化，直接赋值即可。
♻️ 建议的简化
- String hostUuid = null; - if (msg.getHostUuid() != null) { - hostUuid = msg.getHostUuid(); - } + String hostUuid = msg.getHostUuid();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java` around lines 3424 - 3428, 在 LocalStorageBase.java 中将当前冗余的 null 检查简化：直接用 hostUuid = msg.getHostUuid() 代替当前的 if (msg.getHostUuid() != null) {...} 逻辑，保留变量名 hostUuid 并仍然使用 msg.getHostUuid() 作为赋值来源（无需额外的分支或判断）。
3543-3547: 同步签名命名不一致且存在歧义
CleanupVmInstanceMetadataOnPrimaryStorageMsg的同步签名为"update-metadata-on-ps-%s-%s"，但实际操作是 cleanup 而非 update。与UpdateVmInstanceMetadataOnPrimaryStorageMsg（Line 3372）的签名"update-metadata-on-ps-%s"对比，命名易造成混淆。建议：
- 将 cleanup 的签名改为
"cleanup-metadata-on-ps-%s-%s"- 或统一两者的同步粒度策略
♻️ 建议的修复
`@Override` public String getSyncSignature() { - return String.format("update-metadata-on-ps-%s-%s", self.getUuid(), msg.getVmInstanceUuid()); + return String.format("cleanup-metadata-on-ps-%s-%s", self.getUuid(), msg.getVmInstanceUuid()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java` around lines 3543 - 3547, The sync signature for CleanupVmInstanceMetadataOnPrimaryStorageMsg is misleading—getSyncSignature currently returns "update-metadata-on-ps-%s-%s" while the operation is a cleanup and UpdateVmInstanceMetadataOnPrimaryStorageMsg uses "update-metadata-on-ps-%s"; update the getSyncSignature implementation for CleanupVmInstanceMetadataOnPrimaryStorageMsg (the anonymous ChainTask passed to thdf.chainSubmit) to a clear, non-ambiguous value such as "cleanup-metadata-on-ps-%s-%s" (or alternatively harmonize both message classes to the same synchronization granularity by changing UpdateVmInstanceMetadataOnPrimaryStorageMsg signature), ensuring the returned string uniquely identifies the cleanup operation.
1652-1661: 捕获泛型Exception不够精确此处捕获了泛型
Exception，这可能会掩盖真正的错误类型，不利于问题定位。建议捕获更具体的异常类型，如OperationFailureException或CloudRuntimeException。类似的问题也存在于其他几处（Lines 3388, 3447, 3492, 3576）。
♻️ 建议的修复方案
- try { - LocalStorageHypervisorFactory f = getHypervisorBackendFactoryByHostUuid(msg.getHostUuid()); - bkd = f.getHypervisorBackend(self); - } catch (Exception e) { + try { + LocalStorageHypervisorFactory f = getHypervisorBackendFactoryByHostUuid(msg.getHostUuid()); + bkd = f.getHypervisorBackend(self); + } catch (OperationFailureException | CloudRuntimeException e) { RebaseVolumeBackingFileOnPrimaryStorageReply reply = new RebaseVolumeBackingFileOnPrimaryStorageReply(); reply.setError(operr("failed to resolve host/backend for rebase on local primary storage[uuid:%s], hostUuid[%s]: %s", self.getUuid(), msg.getHostUuid(), e.getMessage())); bus.reply(msg, reply); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java` around lines 1652 - 1661, Replace the broad catch(Exception e) around the calls to getHypervisorBackendFactoryByHostUuid(msg.getHostUuid()) and f.getHypervisorBackend(self) with more specific exception handlers (e.g., catch(OperationFailureException e) and catch(CloudRuntimeException e)); in each specific catch create and reply with the RebaseVolumeBackingFileOnPrimaryStorageReply using operr(...) including e.getMessage(), and optionally add a final catch(Throwable t) that logs and replies as a last-resort; apply the same change pattern where similar generic Exception catches occur (references: LocalStorageHypervisorFactory, getHypervisorBackendFactoryByHostUuid, getHypervisorBackend, RebaseVolumeBackingFileOnPrimaryStorageReply, self, msg).sdk/src/main/java/org/zstack/sdk/CleanupVmInstanceMetadataAction.java (1)
28-35: 原始类型List缺少泛型参数
vmUuids、systemTags和userTags均使用了原始类型java.util.List，缺少泛型参数。建议使用java.util.List<String>以提高类型安全性。不过，如果此文件是通过代码生成器自动生成的，则可以忽略此建议。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/src/main/java/org/zstack/sdk/CleanupVmInstanceMetadataAction.java` around lines 28 - 35, The fields vmUuids, systemTags and userTags in class CleanupVmInstanceMetadataAction are declared as raw java.util.List types; change their declarations to use generics (java.util.List<String>) to restore type safety and update any related usages and imports (or method signatures) that reference these fields (e.g., getters/setters or JSON serialization handlers) to accept/return List<String> instead of raw List.plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageVmMetadataExtension.java (1)
33-54:buildMetadataDir和buildVmMetadataPath重复查询数据库两个方法都独立查询
PrimaryStorageVO.url。如果调用方需要同时使用这两个方法，会产生重复的数据库访问。考虑提供一个接收已查询
url的重载方法，或在调用层缓存查询结果。这是一个可选的优化建议。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageVmMetadataExtension.java` around lines 33 - 54, Both buildMetadataDir and buildVmMetadataPath independently query PrimaryStorageVO.url causing duplicate DB access when both are used; refactor by adding an overloaded/private helper that accepts the already-fetched url (e.g., buildMetadataDirWithUrl(String url) or a private String buildMetadataDirFromUrl(String url)) and a corresponding buildVmMetadataPathFromUrl(String url) or single helper that returns both values, then have the existing buildMetadataDir(String primaryStorageUuid) and buildVmMetadataPath(String primaryStorageUuid) do the DB lookup once and delegate to the new helper(s) that call normalizeBaseDir and use VmInstanceMetadataConstants to construct paths; callers that already have the url can use the new overload to avoid re-querying.plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmBackend.java (1)
954-962: 可选优化：移除冗余的类名限定符。
PrefixRebaseBackingFilesCmd和PrefixRebaseBackingFilesRsp使用了完全限定的父类名LocalStorageKvmBackend.AgentCommand和LocalStorageKvmBackend.AgentResponse。由于这些类已经定义在LocalStorageKvmBackend内部，可以直接使用AgentCommand和AgentResponse，与文件中其他 DTO 类保持一致。♻️ 建议的修改
- public static class PrefixRebaseBackingFilesCmd extends LocalStorageKvmBackend.AgentCommand { + public static class PrefixRebaseBackingFilesCmd extends AgentCommand { public List<String> filePaths; public String oldPrefix; public String newPrefix; } - public static class PrefixRebaseBackingFilesRsp extends LocalStorageKvmBackend.AgentResponse { + public static class PrefixRebaseBackingFilesRsp extends AgentResponse { public int rebasedCount; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmBackend.java` around lines 954 - 962, The nested DTO classes PrefixRebaseBackingFilesCmd and PrefixRebaseBackingFilesRsp redundantly use the fully qualified parent types LocalStorageKvmBackend.AgentCommand and LocalStorageKvmBackend.AgentResponse; change their declarations to extend the inner types directly (use AgentCommand for PrefixRebaseBackingFilesCmd and AgentResponse for PrefixRebaseBackingFilesRsp) so they match the style of the other DTOs defined in LocalStorageKvmBackend and remove the unnecessary qualifier.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@compute/src/main/java/org/zstack/compute/vm/CleanupVmInstanceMetadataOnPrimaryStorageGC.java`:
- Around line 50-56: The code in CleanupVmInstanceMetadataOnPrimaryStorageGC
reads psType via Q.New(...).findValue() and then calls
pluginRgty.getExtensionFromMap(psType, VmMetadataPathBuildExtensionPoint.class)
which can misbehave if psType is null due to a race; add a null check after
obtaining psType and handle the missing primary storage case (e.g., treat as no
extension: set ext = null or return/skip cleanup) before calling
pluginRgty.getExtensionFromMap and before invoking ext.requireHostForCleanup(),
ensuring no NPE or unexpected behavior when psType is null.
In `@compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java`:
- Around line 1376-1385: The host availability check in VmInstanceApiInterceptor
currently only verifies HostVO_.uuid/state/status but doesn't constrain
HostVO_.clusterUuid to msg.getClusterUuid(), so a host from another cluster can
pass; update the boolean hostAvailable query used when msg.getHostUuid() != null
to also .eq(HostVO_.clusterUuid, msg.getClusterUuid()) (or skip cluster check
only if msg.getClusterUuid() is null) so that the
ApiMessageInterceptionException is thrown when the host exists/connected but
belongs to a different cluster.
In `@conf/db/zsv/V5.0.0__schema.sql`:
- Around line 131-156: The baseline schema defines two new tables
(VmMetadataDirtyVO and VmMetadataFlushStateVO) but there's no corresponding
upgrade script under conf/db/upgrade, so create a new upgrade DDL file named
V5.0.0__schema.sql in conf/db/upgrade containing the CREATE TABLE statements for
`VmMetadataDirtyVO` and `VmMetadataFlushStateVO` (including columns like
vmInstanceUuid, managementNodeUuid, dirtyVersion, metadataSnapshot, constraints
fkVmMetadataDirtyVOVmInstanceEO, fkVmMetadataDirtyVOManagementNodeVO,
fkVmMetadataFlushStateVOVmInstanceEO, ENGINE=InnoDB, DEFAULT CHARSET=utf8,
timestamps and defaults) exactly as in the baseline
conf/db/zsv/V5.0.0__schema.sql so upgrades from any V4.x will create these
tables.
In
`@header/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageMsg.java`:
- Around line 9-10: Update the `@RestRequest` path in
APIScanVmInstanceMetadataFromPrimaryStorageMsg to use the plural resource form:
change "primary-storage" to "primary-storages" (so the path becomes
"/primary-storages/vm-instances/metadata/scan"), leaving the HttpMethod and
other attributes unchanged; modify the path attribute in the annotation on the
APIScanVmInstanceMetadataFromPrimaryStorageMsg class to match the repository
coding guideline for plural resource names.
In
`@header/src/main/java/org/zstack/header/storage/primary/ScanVmInstanceMetadataFromPrimaryStorageReply.java`:
- Line 10: Normalize failedHostUuids to use empty-list semantics like
vmInstanceMetadata: update ScanVmInstanceMetadataFromPrimaryStorageReply so
failedHostUuids is never null (initialize to Collections.emptyList() or new
ArrayList<>() and/or return Collections.emptyList() from its getter), and apply
the same null-to-empty normalization to the other similar fields noted in this
class to prevent NPEs and ensure consistent return structure.
In `@header/src/main/java/org/zstack/header/tag/APIUpdateSystemTagMsg.java`:
- Line 18: The metadata-impact annotation on APIUpdateSystemTagMsg and
APIDeleteTagMsg currently binds SystemTagUuidToVmUuidResolver which only
resolves VM UUIDs and returns empty for non-VM SystemTagVOs, causing metadata
changes to be skipped for non-VM resources; update the code to either remove the
`@MetadataImpact`(..., resolver = "SystemTagUuidToVmUuidResolver", field = "uuid")
annotation from APIUpdateSystemTagMsg and APIDeleteTagMsg or replace the
resolver with a general-purpose resolver that inspects SystemTagVO.resourceType
and resolves the appropriate resource UUID(s) (or returns a special marker) so
metadata impact is always calculated correctly; if you change the resolver
implementor (instead of removal), update/extend SystemTagUuidToVmUuidResolver
(or add a new SystemTagResourceResolver) and its
batchResolveVmUuids()/equivalent method to handle non-VM resource types and
propagate resolved UUIDs for metadata tracking.
In
`@header/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataMsg.java`:
- Around line 11-14: The RestRequest on APICleanupVmInstanceMetadataMsg
currently uses HttpMethod.PUT but the operation semantics are deletion; change
the RestRequest annotation to use HttpMethod.DELETE for
APICleanupVmInstanceMetadataMsg (update the annotation value where RestRequest
is declared) and also update any associated documentation and SDK action
descriptions to reflect the DELETE method (ensure
APICleanupVmInstanceMetadataEvent mapping remains the same).
In
`@header/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataEvent.java`:
- Around line 7-18: Add a metadata field to APIUpdateVmInstanceMetadataEvent so
the API returns the updated metadata: declare a private metadata field with the
same type as UpdateVmInstanceMetadataReply.metadata, add public getter and
setter for metadata in APIUpdateVmInstanceMetadataEvent, and populate an example
value in the static __example__() method; ensure the field name is exactly
metadata and the class/API event constructors remain unchanged so callers can
receive the updated metadata from UpdateVmInstanceMetadataReply.
In `@header/src/main/java/org/zstack/header/vm/VmInstanceState.java`:
- Around line 205-207: Registering 的事务映射目前只处理 VmInstanceStateEvent.stopped ->
VmInstanceState.Stopped，导致收到其它事件（比如 unknown/noState）时走不到映射而抛
CloudRuntimeException；在 Registering.transactions(...) 中补充对
VmInstanceStateEvent.unknown 和 VmInstanceStateEvent.noState 的映射（例如映射到
VmInstanceState.Unknown 或一个明确的兜底状态），以确保 Transaction(...) 调用覆盖这些事件并避免运行时异常（参考
Registering.transactions、Transaction、VmInstanceStateEvent.stopped、VmInstanceState.Stopped
的使用位置来定位修改点）。
In `@header/src/main/java/org/zstack/header/volume/APIFlattenVolumeMsg.java`:
- Line 21: 在 APIFlattenVolumeMsg 类的 `@MetadataImpact` 注解上添加 updateOnFailure =
true；也就是说，找到类 APIFlattenVolumeMsg 中的注解 `@MetadataImpact`(value =
MetadataImpact.Impact.STORAGE, resolver = "VolumeUuidToVmUuidResolver", field =
"uuid") 并将其修改为包含 updateOnFailure = true，以和其他修改/恢复类操作（例如
APICreateVolumeSnapshotMsg、APIRevertVolumeFromSnapshotMsg、APIUndoSnapshotCreationMsg）保持一致。
In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageSimulator.java`:
- Around line 375-383: The prefixRebaseBackingFiles endpoint currently does not
record the incoming PrefixRebaseBackingFilesCmd into the simulator config;
update the method prefixRebaseBackingFiles to add the received cmd to a new list
on LocalStorageSimulatorConfig (e.g., add a List<PrefixRebaseBackingFilesCmd>
prefixRebaseBackingFilesCmds) and append cmd before replying so tests can
inspect it, and add that corresponding list field and its accessor in the
LocalStorageSimulatorConfig class.
In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageVmMetadataExtension.java`:
- Around line 151-167: The existence check in createResourceRef is racy: two
threads can pass the Q.New(...).isExists() check and both call dbf.persist(ref),
causing duplicate-key errors; update createResourceRef to handle this atomically
by wrapping dbf.persist(ref) in a try-catch that catches the database
duplicate-key/constraint exception (e.g., DataIntegrityViolationException /
SQLIntegrityConstraintViolationException) and silently ignore or reload the
existing LocalStorageResourceRefVO, or alternatively perform the insert with an
upsert/ON DUPLICATE KEY UPDATE at the DAO level; ensure you reference
LocalStorageResourceRefVO, LocalStorageResourceRefVO_.resourceUuid,
createResourceRef, and the dbf.persist(...) call when applying the change.
In
`@plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorage.java`:
- Around line 1953-1962: The code binds NFS metadata update to the VM's current
host via getHostUuidFromVolume() without verifying the host is Connected; change
the flow so UpdateVmInstanceMetadataOnPrimaryStorageReply is handled by a
connected host: call isHostConnected(hostUuid) (or equivalent) after
getHostUuidFromVolume() and if not connected, select an alternative connected
host for this primary storage (e.g., via a helper like
findConnectedHostForPrimaryStorage or getBackendByHostUuid for a connected
host), then call backend.handle(msg, chosenHostUuid, ...); only reply with error
if no connected host is available. Ensure you reference getHostUuidFromVolume,
isHostConnected/findConnectedHostForPrimaryStorage, getBackendByHostUuid, and
the UpdateVmInstanceMetadataOnPrimaryStorageReply handling in the change.
In
`@resourceconfig/src/main/java/org/zstack/resourceconfig/APIUpdateResourceConfigMsg.java`:
- Around line 15-16: 实现名为 ResourceUuidToVmUuidResolver 的类，使其实现
VmUuidFromApiResolver 接口并在实现的解析方法中根据 resourceUuid 返回对应 VM 的 UUID（对非 VM 资源返回
null），然后将该类注册为 Spring Bean（例如添加 `@Component` 或在配置类中显式声明为
`@Bean`），以便注解中引用的解析器能被容器找到；在实现中参考 APIUpdateResourceConfigMsg 和
ResourceConfigMessage 的使用场景以确保返回值与框架期望一致。
---
Nitpick comments:
In
`@header/src/main/java/org/zstack/header/storage/primary/ReadVmInstanceMetadataMsg.java`:
- Around line 7-20: The field name uuid in ReadVmInstanceMetadataMsg is too
generic; rename the field to vmInstanceUuid and update its accessor methods
(replace getUuid/setUuid with getVmInstanceUuid/setVmInstanceUuid) so that
getVmInstanceUuid() returns the new vmInstanceUuid field; also update all
internal references/usages of ReadVmInstanceMetadataMsg, getUuid, and setUuid to
the new names to avoid breaking callers and ensure serialization/deserialization
annotations (if any) are adjusted accordingly.
In `@header/src/main/java/org/zstack/header/vm/metadata/MetadataImpact.java`:
- Around line 9-27: The MetadataImpact annotation currently only documents that
resolver/field are required for CONFIG or STORAGE and that updateOnFailure is
optional; make this fail-fast by validating annotated usage at
metadata-processing startup: when reading MetadataImpact.value() ==
Impact.CONFIG or Impact.STORAGE, assert that MetadataImpact.resolver() and
MetadataImpact.field() are non-empty and throw a clear IllegalStateException (or
similar) with the annotation/class/method context if they are missing; also
update the MetadataImpact javadoc for updateOnFailure to read “optional, default
false” to remove the “boolean required” ambiguity. Ensure the validation runs in
the central metadata entry point that loads/inspects MetadataImpact annotations
so misconfigurations fail at startup rather than at runtime.
In `@header/src/main/java/org/zstack/header/vm/metadata/ResourceMetadata.java`:
- Around line 3-7: The field name vo in class ResourceMetadata is ambiguous;
rename it to a more descriptive identifier (e.g., voJson, voData, or
valueObject) across the class and any callers to improve readability: update the
private field declaration in ResourceMetadata, its getter/setter methods,
constructor parameters, any JSON/property mappings, and all usages in codebase
to use the new name to avoid breaking references.
In
`@header/src/main/java/org/zstack/header/vm/metadata/VmInstanceMetadataDTO.java`:
- Around line 5-13: The list fields volumes, nics, snapshots, snapshotGroups and
snapshotGroupRefs in VmInstanceMetadataDTO are null by default and can cause
NPEs; initialize them to empty lists (e.g., new ArrayList<>() or
Collections.emptyList()) either at field declaration or in the class constructor
(and ensure any deserialization path preserves empty lists) so callers can
safely iterate without null checks; update VmInstanceMetadataDTO to set these
defaults for the mentioned fields.
In
`@header/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyService.java`:
- Around line 20-28: The markDirty API in VmMetadataDirtyService uses a boolean
parameter storageStructureChange which makes call sites like markDirty(uuid,
true) ambiguous; update the interface by replacing boolean
storageStructureChange with an explicit operation indicator: either add two
clear methods (e.g., markDirty(String vmInstanceUuid) and
markDirtyWithStorageStructureChange(String vmInstanceUuid)) or change the
signature to accept an enum (e.g., VmMetadataChangeType changeType) and update
all implementors and callers to use the new methods/enum (locate
VmMetadataDirtyService.markDirty) so callers become self-documenting and the
implementations honor the new contract.
In `@header/src/main/java/org/zstack/header/vm/metadata/VmMetadataErrors.java`:
- Around line 18-22: The field 'code' in the VmMetadataErrors enum is currently
mutable and built with String.format in the private VmMetadataErrors(int id)
constructor; make 'code' immutable by declaring it final (private final String
code) and simplify construction by using plain string concatenation
("VM_METADATA." + id) inside the constructor or as an initializer so the value
is set once and cannot change; update any usages accordingly (refer to
VmMetadataErrors, the 'code' field, and the VmMetadataErrors(int id)
constructor).
In
`@header/src/main/java/org/zstack/header/vm/metadata/VmMetadataResourcePersistExtensionPoint.java`:
- Around line 7-11: 为接口方法补充规范的 Javadoc：在 VmMetadataResourcePersistExtensionPoint
的 getPrimaryStorageType、afterVolumePersist 和 afterSnapshotPersist 上分别添加方法注释，说明
getPrimaryStorageType 返回的字符串语义（如主存储类型标识符）；在
afterVolumePersist/afterSnapshotPersist 中明确 resourceUuid
表示的资源（卷或快照）含义、resourceType 的取值范围/格式、size 的单位（例如字节）及其含义、hostUuid 的语义和可空性，以及 now
表示“已持久化的事件发生时间（Timestamp）”且是否应为系统时间；同时遵循接口方法无多余修饰符的规范（不要加 public 等）。
In
`@header/src/main/java/org/zstack/header/vm/metadata/VmUuidFromApiResolver.java`:
- Around line 9-24: The batchResolveVmUuids implementation in
VmUuidFromApiResolver uses an explicit loop and null checks; refactor it to use
the Stream API to make the code concise and readable by streaming fieldValues,
filtering nulls, mapping via resolveVmUuid, filtering non-null results, and
collecting to a List; update the default method batchResolveVmUuids to return
fieldValues.stream().filter(Objects::nonNull).map(this::resolveVmUuid).filter(Objects::nonNull).collect(Collectors.toList())
(ensure imports for java.util.Objects and java.util.stream.Collectors are
added).
- Around line 6-8: Add proper Javadoc to the VmUuidFromApiResolver interface and
its method resolveVmUuid: document the interface purpose (resolving a VM UUID
from an API-provided value), describe the resolveVmUuid(String fieldValue)
parameter meaning (what forms of fieldValue are accepted), and specify the
return contract (what is returned on success and that it returns null when no VM
UUID is found or cannot be resolved). Ensure the method Javadoc mentions any
expected error handling/side-effects and the expected null behavior so callers
know how to handle missing results.
In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java`:
- Around line 3424-3428: 在 LocalStorageBase.java 中将当前冗余的 null 检查简化：直接用 hostUuid
= msg.getHostUuid() 代替当前的 if (msg.getHostUuid() != null) {...} 逻辑，保留变量名 hostUuid
并仍然使用 msg.getHostUuid() 作为赋值来源（无需额外的分支或判断）。
- Around line 3543-3547: The sync signature for
CleanupVmInstanceMetadataOnPrimaryStorageMsg is misleading—getSyncSignature
currently returns "update-metadata-on-ps-%s-%s" while the operation is a cleanup
and UpdateVmInstanceMetadataOnPrimaryStorageMsg uses "update-metadata-on-ps-%s";
update the getSyncSignature implementation for
CleanupVmInstanceMetadataOnPrimaryStorageMsg (the anonymous ChainTask passed to
thdf.chainSubmit) to a clear, non-ambiguous value such as
"cleanup-metadata-on-ps-%s-%s" (or alternatively harmonize both message classes
to the same synchronization granularity by changing
UpdateVmInstanceMetadataOnPrimaryStorageMsg signature), ensuring the returned
string uniquely identifies the cleanup operation.
- Around line 1652-1661: Replace the broad catch(Exception e) around the calls
to getHypervisorBackendFactoryByHostUuid(msg.getHostUuid()) and
f.getHypervisorBackend(self) with more specific exception handlers (e.g.,
catch(OperationFailureException e) and catch(CloudRuntimeException e)); in each
specific catch create and reply with the
RebaseVolumeBackingFileOnPrimaryStorageReply using operr(...) including
e.getMessage(), and optionally add a final catch(Throwable t) that logs and
replies as a last-resort; apply the same change pattern where similar generic
Exception catches occur (references: LocalStorageHypervisorFactory,
getHypervisorBackendFactoryByHostUuid, getHypervisorBackend,
RebaseVolumeBackingFileOnPrimaryStorageReply, self, msg).
In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmBackend.java`:
- Around line 954-962: The nested DTO classes PrefixRebaseBackingFilesCmd and
PrefixRebaseBackingFilesRsp redundantly use the fully qualified parent types
LocalStorageKvmBackend.AgentCommand and LocalStorageKvmBackend.AgentResponse;
change their declarations to extend the inner types directly (use AgentCommand
for PrefixRebaseBackingFilesCmd and AgentResponse for
PrefixRebaseBackingFilesRsp) so they match the style of the other DTOs defined
in LocalStorageKvmBackend and remove the unnecessary qualifier.
In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageVmMetadataExtension.java`:
- Around line 33-54: Both buildMetadataDir and buildVmMetadataPath independently
query PrimaryStorageVO.url causing duplicate DB access when both are used;
refactor by adding an overloaded/private helper that accepts the already-fetched
url (e.g., buildMetadataDirWithUrl(String url) or a private String
buildMetadataDirFromUrl(String url)) and a corresponding
buildVmMetadataPathFromUrl(String url) or single helper that returns both
values, then have the existing buildMetadataDir(String primaryStorageUuid) and
buildVmMetadataPath(String primaryStorageUuid) do the DB lookup once and
delegate to the new helper(s) that call normalizeBaseDir and use
VmInstanceMetadataConstants to construct paths; callers that already have the
url can use the new overload to avoid re-querying.
In
`@plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsVmMetadataExtension.java`:
- Around line 91-100: The baseDir resolution logic in NfsVmMetadataExtension
duplicates code already implemented in resolveBaseDir(); refactor the block that
queries PrimaryStorageVO (lines creating baseDir and checking null, plus the
warning and empty PathReplacementResult) to reuse resolveBaseDir(targetPsUuid)
or move the shared query into a new private method (e.g., resolveBaseDir or
getPrimaryStorageBaseDir) and call it from both places (ensure you preserve the
warning log and the empty PathReplacementResult behavior when the result is
null); update references in the method that currently contains the duplicated
code (the code that constructs PathReplacementResult) to call the shared method
instead of repeating the queries.
In `@sdk/src/main/java/org/zstack/sdk/CleanupVmInstanceMetadataAction.java`:
- Around line 28-35: The fields vmUuids, systemTags and userTags in class
CleanupVmInstanceMetadataAction are declared as raw java.util.List types; change
their declarations to use generics (java.util.List<String>) to restore type
safety and update any related usages and imports (or method signatures) that
reference these fields (e.g., getters/setters or JSON serialization handlers) to
accept/return List<String> instead of raw List.
In `@sdk/src/main/java/org/zstack/sdk/RegisterVmInstanceFromMetadataResult.java`:
- Line 3: The import of VmInstanceInventory at the top of
RegisterVmInstanceFromMetadataResult is redundant because VmInstanceInventory is
in the same package org.zstack.sdk; remove the line "import
org.zstack.sdk.VmInstanceInventory;" from the
RegisterVmInstanceFromMetadataResult class to clean up the unused/ redundant
import.
In
`@sdk/src/main/java/org/zstack/sdk/ScanVmInstanceMetadataFromPrimaryStorageResult.java`:
- Around line 6-11: The field and accessors in
ScanVmInstanceMetadataFromPrimaryStorageResult should remain using the raw
java.util.List type to keep consistency with the SDK Result classes; ensure the
field vmInstanceMetadata and its methods setVmInstanceMetadata and
getVmInstanceMetadata use plain java.util.List (no generics) and do not
introduce List<VmMetadataScanEntry> here—if you already added generics, revert
that change and leave any type-safety improvements to a coordinated,
project-wide SDK refactor.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4f3c8a96-37f0-4d2f-a8f3-682fdae71b3c
⛔ Files ignored due to path filters (5)
conf/globalConfig/vm.xmlis excluded by!**/*.xmlconf/persistence.xmlis excluded by!**/*.xmlconf/serviceConfig/primaryStorage.xmlis excluded by!**/*.xmlconf/serviceConfig/vmInstance.xmlis excluded by!**/*.xmlconf/springConfigXml/VmInstanceManager.xmlis excluded by!**/*.xml
📒 Files selected for processing (146)
compute/src/main/java/org/zstack/compute/vm/CleanupVmInstanceMetadataOnPrimaryStorageGC.javacompute/src/main/java/org/zstack/compute/vm/VmExpungeMetadataFlow.javacompute/src/main/java/org/zstack/compute/vm/VmGlobalConfig.javacompute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.javacompute/src/main/java/org/zstack/compute/vm/VmSystemTags.javaconf/db/zsv/V5.0.0__schema.sqlheader/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageEvent.javaheader/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageMsgDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/storage/primary/CleanupVmInstanceMetadataOnPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/CleanupVmInstanceMetadataOnPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/storage/primary/GetVmInstanceMetadataFromPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/GetVmInstanceMetadataFromPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/storage/primary/ReadVmInstanceMetadataMsg.javaheader/src/main/java/org/zstack/header/storage/primary/ReadVmInstanceMetadataReply.javaheader/src/main/java/org/zstack/header/storage/primary/RebaseVolumeBackingFileOnPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/RebaseVolumeBackingFileOnPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/storage/primary/ScanVmInstanceMetadataFromPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/ScanVmInstanceMetadataFromPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/storage/primary/VmMetadataScanEntry.javaheader/src/main/java/org/zstack/header/storage/primary/VmMetadataScanEntryDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/storage/snapshot/APIDeleteVolumeSnapshotMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/APIRevertVolumeFromSnapshotMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/APIShrinkVolumeSnapshotMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/APIUpdateVolumeSnapshotMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/group/APIDeleteVolumeSnapshotGroupMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/group/APIRevertVmFromSnapshotGroupMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/group/APIUngroupVolumeSnapshotGroupMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/group/APIUpdateVolumeSnapshotGroupMsg.javaheader/src/main/java/org/zstack/header/tag/APICreateSystemTagMsg.javaheader/src/main/java/org/zstack/header/tag/APICreateSystemTagsMsg.javaheader/src/main/java/org/zstack/header/tag/APIDeleteTagMsg.javaheader/src/main/java/org/zstack/header/tag/APIUpdateSystemTagMsg.javaheader/src/main/java/org/zstack/header/vm/APIAttachIsoToVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIAttachL3NetworkToVmMsg.javaheader/src/main/java/org/zstack/header/vm/APIAttachVmNicToVmMsg.javaheader/src/main/java/org/zstack/header/vm/APIChangeInstanceOfferingMsg.javaheader/src/main/java/org/zstack/header/vm/APIChangeVmNicNetworkMsg.javaheader/src/main/java/org/zstack/header/vm/APIChangeVmNicStateMsg.javaheader/src/main/java/org/zstack/header/vm/APIConvertTemplatedVmInstanceToVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIConvertVmInstanceToTemplatedVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIDeleteVmBootModeMsg.javaheader/src/main/java/org/zstack/header/vm/APIDeleteVmConsolePasswordMsg.javaheader/src/main/java/org/zstack/header/vm/APIDeleteVmHostnameMsg.javaheader/src/main/java/org/zstack/header/vm/APIDeleteVmSshKeyMsg.javaheader/src/main/java/org/zstack/header/vm/APIDeleteVmStaticIpMsg.javaheader/src/main/java/org/zstack/header/vm/APIDestroyVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIDetachIsoFromVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIDetachL3NetworkFromVmMsg.javaheader/src/main/java/org/zstack/header/vm/APIMigrateVmMsg.javaheader/src/main/java/org/zstack/header/vm/APIRecoverVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIReimageVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmBootModeMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmBootOrderMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmBootVolumeMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmClockTrackMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmConsolePasswordMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmHostnameMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmQxlMemoryMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmSoundTypeMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmSshKeyMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmStaticIpMsg.javaheader/src/main/java/org/zstack/header/vm/APIUpdateVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIUpdateVmNicDriverMsg.javaheader/src/main/java/org/zstack/header/vm/APIUpdateVmPriorityMsg.javaheader/src/main/java/org/zstack/header/vm/VmInstanceState.javaheader/src/main/java/org/zstack/header/vm/cdrom/APISetVmInstanceDefaultCdRomMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataEvent.javaheader/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataMsgDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageEvent.javaheader/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageMsgDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataEvent.javaheader/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataMsgDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataEvent.javaheader/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataMsgDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/MetadataImpact.javaheader/src/main/java/org/zstack/header/vm/metadata/ResourceMetadata.javaheader/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataOnPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataOnPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataReply.javaheader/src/main/java/org/zstack/header/vm/metadata/VmInstanceMetadataConstants.javaheader/src/main/java/org/zstack/header/vm/metadata/VmInstanceMetadataDTO.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataCategory.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyService.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyVO.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyVO_.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataErrors.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataFlushStateVO.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataFlushStateVO_.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataPathBuildExtensionPoint.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataPathReplacementExtensionPoint.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataResourcePersistExtensionPoint.javaheader/src/main/java/org/zstack/header/vm/metadata/VmUuidFromApiResolver.javaheader/src/main/java/org/zstack/header/vm/metadata/VolumeResourceMetadata.javaheader/src/main/java/org/zstack/header/volume/APIAttachDataVolumeToVmMsg.javaheader/src/main/java/org/zstack/header/volume/APIChangeVolumeStateMsg.javaheader/src/main/java/org/zstack/header/volume/APICreateVolumeSnapshotGroupMsg.javaheader/src/main/java/org/zstack/header/volume/APICreateVolumeSnapshotMsg.javaheader/src/main/java/org/zstack/header/volume/APIDeleteDataVolumeMsg.javaheader/src/main/java/org/zstack/header/volume/APIDetachDataVolumeFromVmMsg.javaheader/src/main/java/org/zstack/header/volume/APIExpungeDataVolumeMsg.javaheader/src/main/java/org/zstack/header/volume/APIFlattenVolumeMsg.javaheader/src/main/java/org/zstack/header/volume/APIRecoverDataVolumeMsg.javaheader/src/main/java/org/zstack/header/volume/APISyncVolumeSizeMsg.javaheader/src/main/java/org/zstack/header/volume/APIUndoSnapshotCreationMsg.javaheader/src/main/java/org/zstack/header/volume/APIUpdateVolumeMsg.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/APILocalStorageMigrateVolumeMsg.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageHypervisorBackend.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmBackend.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageSimulator.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageSimulatorConfig.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageVmMetadataExtension.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorage.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageBackend.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackendCommands.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsVmMetadataExtension.javaresourceconfig/src/main/java/org/zstack/resourceconfig/APIDeleteResourceConfigMsg.javaresourceconfig/src/main/java/org/zstack/resourceconfig/APIUpdateResourceConfigMsg.javasdk/src/main/java/SourceClassMap.javasdk/src/main/java/org/zstack/sdk/CleanupVmInstanceMetadataAction.javasdk/src/main/java/org/zstack/sdk/CleanupVmInstanceMetadataResult.javasdk/src/main/java/org/zstack/sdk/GetVmInstanceMetadataFromPrimaryStorageAction.javasdk/src/main/java/org/zstack/sdk/GetVmInstanceMetadataFromPrimaryStorageResult.javasdk/src/main/java/org/zstack/sdk/RegisterVmInstanceFromMetadataAction.javasdk/src/main/java/org/zstack/sdk/RegisterVmInstanceFromMetadataResult.javasdk/src/main/java/org/zstack/sdk/ScanVmInstanceMetadataFromPrimaryStorageAction.javasdk/src/main/java/org/zstack/sdk/ScanVmInstanceMetadataFromPrimaryStorageResult.javasdk/src/main/java/org/zstack/sdk/UpdateVmInstanceMetadataAction.javasdk/src/main/java/org/zstack/sdk/UpdateVmInstanceMetadataResult.javasdk/src/main/java/org/zstack/sdk/VmMetadataScanEntry.javastorage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.javatestlib/src/main/java/org/zstack/testlib/ApiHelper.groovytestlib/src/main/java/org/zstack/testlib/LocalStorageSpec.groovytestlib/src/main/java/org/zstack/testlib/NfsPrimaryStorageSpec.groovy
compute/src/main/java/org/zstack/compute/vm/CleanupVmInstanceMetadataOnPrimaryStorageGC.java
Outdated
Show resolved
Hide resolved
compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java
Show resolved
Hide resolved
Comment on lines
+131
to
+156
| CREATE TABLE IF NOT EXISTS `zstack`.`VmMetadataDirtyVO` ( | ||
| `vmInstanceUuid` VARCHAR(32) NOT NULL, | ||
| `managementNodeUuid` VARCHAR(32) DEFAULT NULL, | ||
| `dirtyVersion` BIGINT NOT NULL DEFAULT 1, | ||
| `lastClaimTime` TIMESTAMP NULL DEFAULT NULL, | ||
| `storageStructureChange` TINYINT(1) NOT NULL DEFAULT 0, | ||
| `retryCount` INT NOT NULL DEFAULT 0, | ||
| `nextRetryTime` TIMESTAMP NULL DEFAULT NULL, | ||
| `lastOpDate` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, | ||
| `createDate` timestamp NOT NULL DEFAULT '1999-12-31 23:59:59', | ||
| PRIMARY KEY (`vmInstanceUuid`), | ||
| CONSTRAINT `fkVmMetadataDirtyVOVmInstanceEO` FOREIGN KEY (`vmInstanceUuid`) REFERENCES `VmInstanceEO` (`uuid`) ON DELETE CASCADE, | ||
| CONSTRAINT `fkVmMetadataDirtyVOManagementNodeVO` FOREIGN KEY (`managementNodeUuid`) REFERENCES `ManagementNodeVO` (`uuid`) ON DELETE SET NULL | ||
| ) ENGINE=InnoDB DEFAULT CHARSET=utf8; | ||
|
|
||
| CREATE TABLE IF NOT EXISTS `zstack`.`VmMetadataFlushStateVO` ( | ||
| `vmInstanceUuid` VARCHAR(32) NOT NULL, | ||
| `metadataSnapshot` LONGTEXT, | ||
| `lastFlushFinishTime` TIMESTAMP NULL DEFAULT NULL, | ||
| `pendingStaleRecovery` TINYINT(1) NOT NULL DEFAULT 0, | ||
| `staleRecoveryCount` INT NOT NULL DEFAULT 0, | ||
| `lastOpDate` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, | ||
| `createDate` timestamp NOT NULL DEFAULT '1999-12-31 23:59:59', | ||
| PRIMARY KEY (`vmInstanceUuid`), | ||
| CONSTRAINT `fkVmMetadataFlushStateVOVmInstanceEO` FOREIGN KEY (`vmInstanceUuid`) REFERENCES `VmInstanceEO` (`uuid`) ON DELETE CASCADE | ||
| ) ENGINE=InnoDB DEFAULT CHARSET=utf8; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) 先看升级脚本里是否出现新表名
rg -n --iglob 'V*.sql' 'VmMetadataDirtyVO|VmMetadataFlushStateVO' conf/db/upgrade
# 2) 再确认命中的是 DDL 语句（不是仅注释）
rg -n --iglob 'V*.sql' 'CREATE TABLE|ALTER TABLE|RENAME TABLE' conf/db/upgrade \
| rg -n 'VmMetadataDirtyVO|VmMetadataFlushStateVO'Repository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 检查 conf/db 目录结构
echo "=== Checking conf/db directory structure ==="
ls -la conf/db/ 2>/dev/null | head -20
echo ""
echo "=== Checking if conf/db/upgrade exists ==="
if [ -d "conf/db/upgrade" ]; then
echo "upgrade directory exists"
echo "Contents:"
ls -la conf/db/upgrade/ | head -20
echo ""
echo "=== Searching for SQL files in upgrade directory ==="
find conf/db/upgrade -name "*.sql" -type f 2>/dev/null | head -20
else
echo "upgrade directory does not exist"
fi
echo ""
echo "=== Checking baseline schema file ==="
if [ -f "conf/db/zsv/V5.0.0__schema.sql" ]; then
echo "File exists. Checking for the two tables..."
rg -n 'VmMetadataDirtyVO|VmMetadataFlushStateVO' conf/db/zsv/V5.0.0__schema.sql || echo "Tables not found in baseline"
fiRepository: MatheMatrix/zstack
Length of output: 1892
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== List all SQL files in conf/db/upgrade ==="
ls -1 conf/db/upgrade/*.sql | sort -V
echo ""
echo "=== Search for the two tables in all upgrade scripts ==="
rg -l 'VmMetadataDirtyVO|VmMetadataFlushStateVO' conf/db/upgrade/*.sql 2>/dev/null || echo "Tables not found in any upgrade script"
echo ""
echo "=== Check the baseline schema file ==="
if [ -f "conf/db/zsv/V5.0.0__schema.sql" ]; then
echo "V5.0.0__schema.sql baseline exists"
wc -l conf/db/zsv/V5.0.0__schema.sql
echo ""
echo "Searching for tables in baseline..."
rg -n 'VmMetadataDirtyVO|VmMetadataFlushStateVO' conf/db/zsv/V5.0.0__schema.sql || echo "Not found"
else
echo "V5.0.0__schema.sql baseline does not exist"
fi
echo ""
echo "=== List all files in conf/db/zsv ==="
ls -la conf/db/zsv/Repository: MatheMatrix/zstack
Length of output: 7010
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Reading V5.0.0__schema.sql lines 131-156 ==="
sed -n '131,156p' conf/db/zsv/V5.0.0__schema.sql
echo ""
echo "=== Check for DEFAULT 0000-00-00 violations in the file ==="
rg 'DEFAULT.*0000-00-00' conf/db/zsv/V5.0.0__schema.sql || echo "No 0000-00-00 DEFAULT found"
echo ""
echo "=== Check for potentially problematic DEFAULT values ==="
rg "DEFAULT\s+'[0-9]{4}-" conf/db/zsv/V5.0.0__schema.sql
echo ""
echo "=== Check if this schema file has any procedures/functions for data migration ==="
rg -i 'PROCEDURE|FUNCTION|TRIGGER' conf/db/zsv/V5.0.0__schema.sql || echo "No procedures/functions found"Repository: MatheMatrix/zstack
Length of output: 2623
升级脚本缺失：V5.0.0 版本无对应的 upgrade DDL。
这两张新表仅定义在基线 schema conf/db/zsv/V5.0.0__schema.sql（第 131、146 行）中，但 conf/db/upgrade/ 目录下不存在 V5.0.0__schema.sql 升级脚本（最新的升级脚本为 V4.10.28__schema.sql）。
这导致从任何 V4.x 版本升级的用户将无法自动创建这两张表，造成新环境和升级环境的不一致。
需要在 conf/db/upgrade/ 下添加 V5.0.0__schema.sql 升级脚本，包含这两张表的 CREATE TABLE 语句。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@conf/db/zsv/V5.0.0__schema.sql` around lines 131 - 156, The baseline schema
defines two new tables (VmMetadataDirtyVO and VmMetadataFlushStateVO) but
there's no corresponding upgrade script under conf/db/upgrade, so create a new
upgrade DDL file named V5.0.0__schema.sql in conf/db/upgrade containing the
CREATE TABLE statements for `VmMetadataDirtyVO` and `VmMetadataFlushStateVO`
(including columns like vmInstanceUuid, managementNodeUuid, dirtyVersion,
metadataSnapshot, constraints fkVmMetadataDirtyVOVmInstanceEO,
fkVmMetadataDirtyVOManagementNodeVO, fkVmMetadataFlushStateVOVmInstanceEO,
ENGINE=InnoDB, DEFAULT CHARSET=utf8, timestamps and defaults) exactly as in the
baseline conf/db/zsv/V5.0.0__schema.sql so upgrades from any V4.x will create
these tables.
Comment on lines
+9
to
+10
| path = "/primary-storage/vm-instances/metadata/scan", | ||
| method = HttpMethod.GET, |
There was a problem hiding this comment.
REST 路径建议改为复数资源形式
当前 path 使用 primary-storage（单数），与仓库规范不一致，建议在合入前统一为复数资源路径。
建议修改
`@RestRequest`(
- path = "/primary-storage/vm-instances/metadata/scan",
+ path = "/primary-storages/vm-instances/metadata/scan",
method = HttpMethod.GET,
responseClass = APIScanVmInstanceMetadataFromPrimaryStorageEvent.class
)As per coding guidelines @RestRequest 的 path 规则要求“针对资源使用复数形式”。
📝 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
| path = "/primary-storage/vm-instances/metadata/scan", | |
| method = HttpMethod.GET, | |
| `@RestRequest`( | |
| path = "/primary-storages/vm-instances/metadata/scan", | |
| method = HttpMethod.GET, | |
| responseClass = APIScanVmInstanceMetadataFromPrimaryStorageEvent.class | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@header/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageMsg.java`
around lines 9 - 10, Update the `@RestRequest` path in
APIScanVmInstanceMetadataFromPrimaryStorageMsg to use the plural resource form:
change "primary-storage" to "primary-storages" (so the path becomes
"/primary-storages/vm-instances/metadata/scan"), leaving the HttpMethod and
other attributes unchanged; modify the path attribute in the annotation on the
APIScanVmInstanceMetadataFromPrimaryStorageMsg class to match the repository
coding guideline for plural resource names.
...in/java/org/zstack/header/storage/primary/ScanVmInstanceMetadataFromPrimaryStorageReply.java
Outdated
Show resolved
Hide resolved
| method = HttpMethod.PUT, | ||
| responseClass = APIFlattenVolumeEvent.class | ||
| ) | ||
| @MetadataImpact(value = MetadataImpact.Impact.STORAGE, resolver = "VolumeUuidToVmUuidResolver", field = "uuid") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 检查其他 STORAGE 类型的 MetadataImpact 注解是否设置了 updateOnFailure
rg -n "@MetadataImpact.*STORAGE" --type=java -A1 | head -50Repository: MatheMatrix/zstack
Length of output: 6674
考虑是否需要设置 updateOnFailure = true
根据同一类型的存储操作分析，涉及数据修改或恢复的操作（如 APICreateVolumeSnapshotMsg、APIRevertVolumeFromSnapshotMsg、APIUndoSnapshotCreationMsg 等）都设置了 updateOnFailure = true。卷展平（flatten）是一个数据转换操作，语义上更接近数据修改类操作，建议确认是否应该添加 updateOnFailure = true 以保持一致性。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@header/src/main/java/org/zstack/header/volume/APIFlattenVolumeMsg.java` at
line 21, 在 APIFlattenVolumeMsg 类的 `@MetadataImpact` 注解上添加 updateOnFailure =
true；也就是说，找到类 APIFlattenVolumeMsg 中的注解 `@MetadataImpact`(value =
MetadataImpact.Impact.STORAGE, resolver = "VolumeUuidToVmUuidResolver", field =
"uuid") 并将其修改为包含 updateOnFailure = true，以和其他修改/恢复类操作（例如
APICreateVolumeSnapshotMsg、APIRevertVolumeFromSnapshotMsg、APIUndoSnapshotCreationMsg）保持一致。
Comment on lines
+375
to
+383
| @RequestMapping(value=LocalStorageKvmBackend.PREFIX_REBASE_BACKING_FILES_PATH, method= RequestMethod.POST) | ||
| public @ResponseBody | ||
| String prefixRebaseBackingFiles(HttpEntity<String> entity) { | ||
| PrefixRebaseBackingFilesCmd cmd = JSONObjectUtil.toObject(entity.getBody(), PrefixRebaseBackingFilesCmd.class); | ||
| PrefixRebaseBackingFilesRsp rsp = new PrefixRebaseBackingFilesRsp(); | ||
| rsp.rebasedCount = cmd.filePaths == null ? 0 : cmd.filePaths.size(); | ||
| reply(entity, rsp); | ||
| return null; | ||
| } |
There was a problem hiding this comment.
prefixRebaseBackingFiles 端点未将命令记录到 config 中。
其他新增端点（writeVmMetadata、getVmInstanceMetadata、scanVmMetadata、cleanupVmMetadata）都将命令对象添加到 config 的对应列表中以便测试验证。但 prefixRebaseBackingFiles 未将 cmd 添加到任何 config 列表，这可能影响测试时对该命令的验证能力。
🔧 建议添加命令记录
`@RequestMapping`(value=LocalStorageKvmBackend.PREFIX_REBASE_BACKING_FILES_PATH, method= RequestMethod.POST)
public `@ResponseBody`
String prefixRebaseBackingFiles(HttpEntity<String> entity) {
PrefixRebaseBackingFilesCmd cmd = JSONObjectUtil.toObject(entity.getBody(), PrefixRebaseBackingFilesCmd.class);
+ config.prefixRebaseBackingFilesCmds.add(cmd);
PrefixRebaseBackingFilesRsp rsp = new PrefixRebaseBackingFilesRsp();
rsp.rebasedCount = cmd.filePaths == null ? 0 : cmd.filePaths.size();
reply(entity, rsp);
return null;
}同时需要在 LocalStorageSimulatorConfig 中添加对应的列表字段。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageSimulator.java`
around lines 375 - 383, The prefixRebaseBackingFiles endpoint currently does not
record the incoming PrefixRebaseBackingFilesCmd into the simulator config;
update the method prefixRebaseBackingFiles to add the received cmd to a new list
on LocalStorageSimulatorConfig (e.g., add a List<PrefixRebaseBackingFilesCmd>
prefixRebaseBackingFilesCmds) and append cmd before replying so tests can
inspect it, and add that corresponding list field and its accessor in the
LocalStorageSimulatorConfig class.
Comment on lines
+151
to
+167
| private void createResourceRef(String primaryStorageUuid, String resourceUuid, | ||
| String resourceType, String hostUuid, long size, Timestamp now) { | ||
| boolean exists = Q.New(LocalStorageResourceRefVO.class).eq(LocalStorageResourceRefVO_.resourceUuid, resourceUuid).isExists(); | ||
| if (exists) { | ||
| logger.debug(String.format("LocalStorageResourceRefVO for resource[uuid:%s] already exists, skip creation", resourceUuid)); | ||
| return; | ||
| } | ||
|
|
||
| LocalStorageResourceRefVO ref = new LocalStorageResourceRefVO(); | ||
| ref.setPrimaryStorageUuid(primaryStorageUuid); | ||
| ref.setResourceUuid(resourceUuid); | ||
| ref.setResourceType(resourceType); | ||
| ref.setHostUuid(hostUuid); | ||
| ref.setSize(size); | ||
| ref.setCreateDate(now); | ||
| ref.setLastOpDate(now); | ||
| dbf.persist(ref); |
There was a problem hiding this comment.
createResourceRef 存在检查-执行竞态条件
isExists() 检查与 persist() 操作之间存在时间窗口，在并发场景下，两个线程可能同时通过存在性检查并尝试插入相同的 resourceUuid，导致重复键异常或数据不一致。
建议使用数据库层面的 INSERT ... ON DUPLICATE KEY UPDATE 或在事务中加锁来保证原子性。
🛡️ 建议的修复方案（使用 try-catch 处理重复键）
private void createResourceRef(String primaryStorageUuid, String resourceUuid,
String resourceType, String hostUuid, long size, Timestamp now) {
- boolean exists = Q.New(LocalStorageResourceRefVO.class).eq(LocalStorageResourceRefVO_.resourceUuid, resourceUuid).isExists();
- if (exists) {
- logger.debug(String.format("LocalStorageResourceRefVO for resource[uuid:%s] already exists, skip creation", resourceUuid));
- return;
- }
-
LocalStorageResourceRefVO ref = new LocalStorageResourceRefVO();
ref.setPrimaryStorageUuid(primaryStorageUuid);
ref.setResourceUuid(resourceUuid);
ref.setResourceType(resourceType);
ref.setHostUuid(hostUuid);
ref.setSize(size);
ref.setCreateDate(now);
ref.setLastOpDate(now);
- dbf.persist(ref);
+ try {
+ dbf.persist(ref);
+ } catch (Exception e) {
+ // Handle duplicate key - record already exists
+ if (Q.New(LocalStorageResourceRefVO.class).eq(LocalStorageResourceRefVO_.resourceUuid, resourceUuid).isExists()) {
+ logger.debug(String.format("LocalStorageResourceRefVO for resource[uuid:%s] already exists, skip creation", resourceUuid));
+ return;
+ }
+ throw e;
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageVmMetadataExtension.java`
around lines 151 - 167, The existence check in createResourceRef is racy: two
threads can pass the Q.New(...).isExists() check and both call dbf.persist(ref),
causing duplicate-key errors; update createResourceRef to handle this atomically
by wrapping dbf.persist(ref) in a try-catch that catches the database
duplicate-key/constraint exception (e.g., DataIntegrityViolationException /
SQLIntegrityConstraintViolationException) and silently ignore or reload the
existing LocalStorageResourceRefVO, or alternatively perform the insert with an
upsert/ON DUPLICATE KEY UPDATE at the DAO level; ensure you reference
LocalStorageResourceRefVO, LocalStorageResourceRefVO_.resourceUuid,
createResourceRef, and the dbf.persist(...) call when applying the change.
Comment on lines
+1953
to
+1962
| 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); | ||
| chain.next(); | ||
| return; | ||
| } | ||
|
|
||
| final NfsPrimaryStorageBackend backend = getBackendByHostUuid(hostUuid); | ||
| backend.handle(msg, hostUuid, new ReturnValueCompletion<UpdateVmInstanceMetadataOnPrimaryStorageReply>(msg, chain) { |
There was a problem hiding this comment.
NFS 元数据更新不应绑定到可能已失联的 VM host。
这里通过 getHostUuidFromVolume() 取 host；当 VM 处于 Running/Paused 时，它会优先返回 VM 当前 host，但这里没有再校验该 host 是否仍然 Connected。对 NFS 这种共享存储来说，元数据更新可以像下面的 Get/Cleanup/Rebase 一样落到任意 connected host；否则 VM host 短暂失联时，这条路径会把本可成功的更新直接做失败。
建议修改
- String hostUuid = getHostUuidFromVolume(msg.getRootVolumeUuid());
- if (hostUuid == null || hostUuid.isEmpty()) {
- reply.setError(operr("no host found for volume[uuid:%s]", msg.getRootVolumeUuid()));
+ List<HostInventory> connectedHosts = factory.getConnectedHostForOperation(getSelfInventory());
+ if (connectedHosts.isEmpty()) {
+ reply.setError(operr("no connected host found for NFS primary storage[uuid:%s]", self.getUuid()));
bus.reply(msg, reply);
chain.next();
return;
}
+ String hostUuid = connectedHosts.get(0).getUuid();
final NfsPrimaryStorageBackend backend = getBackendByHostUuid(hostUuid);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorage.java`
around lines 1953 - 1962, The code binds NFS metadata update to the VM's current
host via getHostUuidFromVolume() without verifying the host is Connected; change
the flow so UpdateVmInstanceMetadataOnPrimaryStorageReply is handled by a
connected host: call isHostConnected(hostUuid) (or equivalent) after
getHostUuidFromVolume() and if not connected, select an alternative connected
host for this primary storage (e.g., via a helper like
findConnectedHostForPrimaryStorage or getBackendByHostUuid for a connected
host), then call backend.handle(msg, chosenHostUuid, ...); only reply with error
if no connected host is available. Ensure you reference getHostUuidFromVolume,
isHostConnected/findConnectedHostForPrimaryStorage, getBackendByHostUuid, and
the UpdateVmInstanceMetadataOnPrimaryStorageReply handling in the change.
Comment on lines
+15
to
16
| @MetadataImpact(value = MetadataImpact.Impact.CONFIG, resolver = "ResourceUuidToVmUuidResolver", field = "resourceUuid") | ||
| public class APIUpdateResourceConfigMsg extends APIMessage implements ResourceConfigMessage { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 查找 ResourceUuidToVmUuidResolver 的实现，确认其处理逻辑
ast-grep --pattern 'class ResourceUuidToVmUuidResolver $$$'
# 如果上面没找到，尝试搜索文件
rg -l "ResourceUuidToVmUuidResolver" --type=javaRepository: MatheMatrix/zstack
Length of output: 376
🏁 Script executed:
# Search for the class definition in all Java files
rg "class ResourceUuidToVmUuidResolver" --type=java
# Also search in imports and variable declarations
rg "ResourceUuidToVmUuidResolver" --type=java -B 2 -A 5
# Look for resolver implementations in metadata/resourceconfig packages
fd -e java -path "*resourceconfig*" -o -path "*metadata*" | head -20Repository: MatheMatrix/zstack
Length of output: 4324
🏁 Script executed:
# Search more broadly for the resolver class definition
rg "ResourceUuidToVmUuidResolver" --type=java -l | xargs -I {} sh -c 'echo "=== {} ===" && grep -n "class.*ResourceUuidToVmUuidResolver\|new ResourceUuidToVmUuidResolver" {}'
# Search for all resolver implementations to understand the pattern
rg "extends.*Resolver|implements.*Resolver" --type=java | head -20
# Look for metadata resolver registry or configuration
fd -type f -name "*.java" | xargs rg "ResourceUuidToVmUuidResolver" -B 3 -A 10 | head -100Repository: MatheMatrix/zstack
Length of output: 7324
🏁 Script executed:
# Search for resolver-related files in metadata packages
find . -type f -name "*.java" | xargs grep -l "Resolver" | grep -i metadata | head -10
# Look for resolver factory or registry patterns
rg "ResourceUuidToVm|Resolver.*register|resolver.*map" -i --type=java | head -30
# Search for the string in all files including configs
find . -type f \( -name "*.java" -o -name "*.xml" -o -name "*.properties" \) | xargs grep -l "ResourceUuidToVmUuidResolver" 2>/dev/null
# Check what files are in the resourceconfig module
find resourceconfig -type f -name "*.java" | head -20Repository: MatheMatrix/zstack
Length of output: 3053
🏁 Script executed:
# Look at MetadataImpact.java to understand resolver mechanism
cat header/src/main/java/org/zstack/header/vm/metadata/MetadataImpact.java
# Look at VmUuidFromApiResolver as example
cat header/src/main/java/org/zstack/header/vm/metadata/VmUuidFromApiResolver.java
# Search for all resolver implementations in metadata package
find . -path "*/metadata/*" -name "*Resolver*.java" -type fRepository: MatheMatrix/zstack
Length of output: 1691
🏁 Script executed:
# Search for ResourceUuidToVmUuidResolver class implementation
rg "class ResourceUuidToVmUuidResolver" --type=java -B 2 -A 15
# If not found, search for registrations
rg "@Bean.*ResourceUuidToVmUuidResolver|new ResourceUuidToVmUuidResolver" --type=java
# Look for Spring bean registrations in resourceconfig
rg "@Component|@Bean|@Service" resourceconfig -A 2 | head -40
# Search more broadly with partial string matching
rg "ResourceUuidToVmUuid" --type=javaRepository: MatheMatrix/zstack
Length of output: 860
🏁 Script executed:
# Search for all implementations of VmUuidFromApiResolver
rg "implements VmUuidFromApiResolver|extends.*VmUuidFromApiResolver" --type=java -B 2 -A 10
# Look for where MetadataImpact is processed/used
rg "MetadataImpact" --type=java -l | head -10
# Check if there are any other resolver implementations
rg "implements.*Resolver|extends.*Resolver" --type=java | grep -i "resolver" | head -20Repository: MatheMatrix/zstack
Length of output: 859
🏁 Script executed:
# Search in resourceconfig package specifically for resolver implementations
find resourceconfig -name "*.java" -type f | xargs grep -l "VmUuidFromApiResolver\|Resolver"
# Look for all classes in resourceconfig package
find resourceconfig -name "*.java" -type f
# Try searching with ast-grep for implements pattern
ast-grep --pattern 'class $_ implements VmUuidFromApiResolver { $$$ }'
# Look for bean definitions or Spring configuration
find . -name "*Config*.java" -o -name "*Configuration*.java" | xargs grep -l "ResourceUuidToVmUuid\|resourceconfig" 2>/dev/null | head -10Repository: MatheMatrix/zstack
Length of output: 3697
🏁 Script executed:
# Search for where metadata resolvers are actually processed/registered
rg "getResolver|lookupResolver|resolver.*bean|BeanFactory.*resolver" --type=java | head -20
# Look for any reference to how resolvers are discovered
rg "VmUuidFromApiResolver" --type=java -B 3 -A 3
# Check if resolver might be in a different location - search all Java files
find . -name "*.java" -type f -exec grep -l "class.*VmUuidFromApiResolver\|ResourceUuidToVmUuidResolver" {} \;
# Maybe search in test files
find . -path "*/test/*" -name "*ResourceUuidToVmUuid*" -type fRepository: MatheMatrix/zstack
Length of output: 1861
🏁 Script executed:
# Let's check if ResourceUuidToVmUuidResolver might be in a pending commit or not yet created
git log --oneline --all -S "ResourceUuidToVmUuidResolver" 2>/dev/null | head -20
# Check git status to see if it's a new untracked file
git ls-files | grep -i "resolver" | grep -i "resource"
# Try to find it in the entire repository including uncommitted changes
find . -name "*ResourceUuidToVmUuid*" -type f
# Last attempt - search in all files for class definition patterns
grep -r "class.*ResourceUuidToVmUuid" . --include="*.java" 2>/dev/nullRepository: MatheMatrix/zstack
Length of output: 148
🏁 Script executed:
# Quick search for resolver in current directory structure
ls -la resourceconfig/src/main/java/org/zstack/resourceconfig/ | grep -i resolver
# Check if there are any recent files
find resourceconfig -name "*.java" -type f -mtime -1 2>/dev/null | head
# List all Java files in resourceconfig to see what exists
ls -1 resourceconfig/src/main/java/org/zstack/resourceconfig/*.java | sortRepository: MatheMatrix/zstack
Length of output: 3599
实现 ResourceUuidToVmUuidResolver 并完成 Spring Bean 注册
ResourceUuidToVmUuidResolver 在注解中被引用为 Spring Bean，但该类在代码库中不存在。根据 VmUuidFromApiResolver 接口的设计，该解析器应该对非 VM 资源返回 null（框架会正确处理 null 返回值）。需要实现此解析器并将其注册为 Spring Bean，否则应用启动时会因 BeanNotFoundException 而失败。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@resourceconfig/src/main/java/org/zstack/resourceconfig/APIUpdateResourceConfigMsg.java`
around lines 15 - 16, 实现名为 ResourceUuidToVmUuidResolver 的类，使其实现
VmUuidFromApiResolver 接口并在实现的解析方法中根据 resourceUuid 返回对应 VM 的 UUID（对非 VM 资源返回
null），然后将该类注册为 Spring Bean（例如添加 `@Component` 或在配置类中显式声明为
`@Bean`），以便注解中引用的解析器能被容器找到；在实现中参考 APIUpdateResourceConfigMsg 和
ResourceConfigMessage 的使用场景以确保返回值与框架期望一致。
MatheMatrix
force-pushed
the
sync/tao.gan/fb-reg-2-ZSV-11559@@3
branch
from
2e57288 to
596132b
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageVmMetadataExtension.java (1)
151-167:⚠️ Potential issue | 🟡 Minor这里的存在性检查不是原子的。
两个并发流程可以同时通过
isExists()，随后一起persist()同一个resourceUuid，最后在唯一键处失败并把注册流程回滚。这里需要把幂等性下沉到数据库层：要么 upsert，要么把唯一键冲突按“已存在”处理。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageVmMetadataExtension.java` around lines 151 - 167, The existence check in createResourceRef is not atomic: two threads can pass Q.New(...).isExists() and both call dbf.persist(ref), causing a unique-key rollback; change createResourceRef (in LocalStorageVmMetadataExtension) to perform the idempotent write at the DB layer instead of pre-checking—either convert the insert to an upsert/merge using the DB/ORM upsert mechanism for LocalStorageResourceRefVO or keep the insert but wrap dbf.persist(ref) in a try/catch that catches unique-constraint/DuplicateKey/Persistence exceptions (e.g. DuplicateKeyException / ConstraintViolationException) and treat them as "already exists" (log and return) rather than failing; remove the non-atomic Q.New(...).isExists() pre-check or keep it only as an optimization.
🧹 Nitpick comments (5)
sdk/src/main/java/org/zstack/sdk/UpdateVmInstanceMetadataAction.java (2)
3-5: 冗余的包内通配符导入
import org.zstack.sdk.*;在同一个包org.zstack.sdk内是多余的，Java 不需要导入同包下的类。♻️ 建议移除冗余导入
import java.util.HashMap; import java.util.Map; -import org.zstack.sdk.*;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/src/main/java/org/zstack/sdk/UpdateVmInstanceMetadataAction.java` around lines 3 - 5, In UpdateVmInstanceMetadataAction remove the redundant package wildcard import "import org.zstack.sdk.*;"—since UpdateVmInstanceMetadataAction is in the same package, delete that import line and keep only the explicit imports (e.g., java.util.HashMap, java.util.Map) so the class compiles without unnecessary wildcard imports.
31-35: systemTags 和 userTags 也应使用泛型类型与
vmUuids相同的问题，这两个字段也应声明为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;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/src/main/java/org/zstack/sdk/UpdateVmInstanceMetadataAction.java` around lines 31 - 35, The fields systemTags and userTags in UpdateVmInstanceMetadataAction are declared as raw java.util.List; change their declarations to use generics (java.util.List<String>) to match vmUuids and ensure type safety and proper compilation; update any annotations or usages expecting raw List to use List<String> (refer to the systemTags and userTags fields and the UpdateVmInstanceMetadataAction class).header/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageMsg.java (1)
10-14: 将uuid移到路径参数中，遵循 ZStack REST API 约定ZStack REST API 标准规范要求在 URI 路径中直接包含资源的 UUID 标识符。当前路径
/primary-storage/vm-instances/metadata未遵循此约定。建议改为/primary-storage/vm-instances/{uuid}/metadata，以与项目 API 设计规范一致。建议的路径修改
`@RestRequest`( - path = "/primary-storage/vm-instances/metadata", + path = "/primary-storage/vm-instances/{uuid}/metadata", method = HttpMethod.GET, responseClass = APIGetVmInstanceMetadataFromPrimaryStorageEvent.class )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageMsg.java` around lines 10 - 14, Update the RestRequest path on APIGetVmInstanceMetadataFromPrimaryStorageMsg to include the resource UUID by changing the annotation path to "/primary-storage/vm-instances/{uuid}/metadata"; ensure the class's uuid field (or add one if missing) is bound to that path variable so the framework maps the URI segment into the message, and adjust any callers/tests or API docs that reference the old path to the new URI form to follow ZStack REST conventions.plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageVmMetadataExtension.java (1)
39-54: 路径规则建议抽到公共实现。这里和
plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsVmMetadataExtension.java:29-127只有 baseDir 获取方式不同，buildMetadataDir、validateMetadataPath、calculatePathReplacements基本是同一份逻辑。后续一边修路径规则、另一边很容易漏改，行为会漂移。建议把公共部分下沉到共享 helper/抽象类，只保留不同主存储的 baseDir 解析。As per coding guidelines “每个模块、类、方法应只承担单一的责任” 和 “策略与机制分离”。
Also applies to: 57-97, 105-137
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageVmMetadataExtension.java` around lines 39 - 54, The LocalStorageVmMetadataExtension and NfsVmMetadataExtension duplicate path-building/validation logic (buildMetadataDir, buildVmMetadataPath, validateMetadataPath, calculatePathReplacements); refactor by extracting the shared logic into a common abstract class or helper (e.g., AbstractVmMetadataExtension) that implements buildMetadataDir, buildVmMetadataPath, validateMetadataPath, calculatePathReplacements and depends on a single abstract method for base-dir resolution (e.g., protected abstract String resolveBaseDir(String primaryStorageUrl) or normalizeBaseDirFromUrl(String url)); then have LocalStorageVmMetadataExtension and NfsVmMetadataExtension only implement that base-dir resolver (and keep existing normalizeBaseDir usage confined there) so all path rules live in one place and each subclass only provides storage-specific URL→baseDir parsing.plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java (1)
1652-1655: 这里别用catch (Exception)兜底。这些分支只是把 host/backend 解析失败转成 reply；现在直接吃掉
Exception，会把NullPointerException之类的编程错误也包装成普通业务失败，排障会很难。这里最好只捕获预期异常（例如本文件里已经在用的OperationFailureException/CloudRuntimeException一类），非预期异常继续抛出。As per coding guidelines, “捕获异常时需严格匹配期望的异常类型或其父类，避免捕获错误的异常类型。”
Also applies to: 3384-3388, 3447-3450, 3576-3579
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java` around lines 1652 - 1655, The current catch(Exception e) around calls to getHypervisorBackendFactoryByHostUuid(...) and f.getHypervisorBackend(self) swallows programming errors; change it to catch only the expected business exceptions (e.g. OperationFailureException and CloudRuntimeException used elsewhere in this file) and convert those to the reply, while rethrowing any other unexpected Throwables; apply the same change for the other occurrences that wrap those host/backend resolution calls (the blocks invoking LocalStorageHypervisorFactory, getHypervisorBackendFactoryByHostUuid, and getHypervisorBackend).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@compute/src/main/java/org/zstack/compute/vm/VmExpungeMetadataFlow.java`:
- Around line 87-93: The current branch in VmExpungeMetadataFlow that checks if
hostUuid == null && ext.requireHostForCleanup() incorrectly skips cleanup by
calling trigger.next() and returning; instead ensure metadata is not left behind
by submitting a GC task (or otherwise enqueue metadata cleanup) when hostUuid is
null for backends that require a host. Modify the branch around hostUuid,
ext.requireHostForCleanup(), psUuid/psType so that rather than only logging and
calling trigger.next(), it invokes the existing GC submission path (or creates a
submitMetadataGC(vmUuid, psUuid, psType) helper) and then proceeds with
trigger.next(); alternatively, ensure the required hostUuid is persisted earlier
into VmInstanceSpec/flow context before this flow runs (update the earlier flow
that sets ext/lastHostUuid if you choose that approach).
In `@compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java`:
- Around line 1366-1399: Add a zone/cluster ownership check: before validating
primary storage and host, verify the cluster belongs to the message's zone by
querying ClusterVO for uuid == msg.getClusterUuid() and zoneUuid ==
msg.getZoneUuid() (use ClusterVO and ClusterVO_); if the query returns false
throw an ApiMessageInterceptionException with an argerr indicating the
cluster[uuid:%s] is not in zone[uuid:%s]. Update imports to include
org.zstack.header.cluster.ClusterVO and org.zstack.header.cluster.ClusterVO_ and
place this check in VmInstanceApiInterceptor alongside the existing
PrimaryStorageClusterRefVO and HostVO validations.
In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java`:
- Around line 3371-3378: The code in LocalStorageBase defines getSyncSignature()
and getSyncLevel() for the update/cleanup path using a primary-storage
(PS)-level key (String.format("update-metadata-on-ps-%s", self.getUuid())) which
differs from the cleanup path that uses a VM-level key, causing update/flush and
cleanup to end up in different serial queues and race; change getSyncSignature()
to use the same VM-level key format used by the cleanup path (e.g., include the
VM UUID/identifier instead of PS UUID) and make getSyncLevel() return the same
concurrency level used by cleanup so update/cleanup share the same serial queue;
update references in LocalStorageBase (getSyncSignature, getSyncLevel) to match
the cleanup implementation.
---
Duplicate comments:
In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageVmMetadataExtension.java`:
- Around line 151-167: The existence check in createResourceRef is not atomic:
two threads can pass Q.New(...).isExists() and both call dbf.persist(ref),
causing a unique-key rollback; change createResourceRef (in
LocalStorageVmMetadataExtension) to perform the idempotent write at the DB layer
instead of pre-checking—either convert the insert to an upsert/merge using the
DB/ORM upsert mechanism for LocalStorageResourceRefVO or keep the insert but
wrap dbf.persist(ref) in a try/catch that catches
unique-constraint/DuplicateKey/Persistence exceptions (e.g.
DuplicateKeyException / ConstraintViolationException) and treat them as "already
exists" (log and return) rather than failing; remove the non-atomic
Q.New(...).isExists() pre-check or keep it only as an optimization.
---
Nitpick comments:
In
`@header/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageMsg.java`:
- Around line 10-14: Update the RestRequest path on
APIGetVmInstanceMetadataFromPrimaryStorageMsg to include the resource UUID by
changing the annotation path to "/primary-storage/vm-instances/{uuid}/metadata";
ensure the class's uuid field (or add one if missing) is bound to that path
variable so the framework maps the URI segment into the message, and adjust any
callers/tests or API docs that reference the old path to the new URI form to
follow ZStack REST conventions.
In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java`:
- Around line 1652-1655: The current catch(Exception e) around calls to
getHypervisorBackendFactoryByHostUuid(...) and f.getHypervisorBackend(self)
swallows programming errors; change it to catch only the expected business
exceptions (e.g. OperationFailureException and CloudRuntimeException used
elsewhere in this file) and convert those to the reply, while rethrowing any
other unexpected Throwables; apply the same change for the other occurrences
that wrap those host/backend resolution calls (the blocks invoking
LocalStorageHypervisorFactory, getHypervisorBackendFactoryByHostUuid, and
getHypervisorBackend).
In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageVmMetadataExtension.java`:
- Around line 39-54: The LocalStorageVmMetadataExtension and
NfsVmMetadataExtension duplicate path-building/validation logic
(buildMetadataDir, buildVmMetadataPath, validateMetadataPath,
calculatePathReplacements); refactor by extracting the shared logic into a
common abstract class or helper (e.g., AbstractVmMetadataExtension) that
implements buildMetadataDir, buildVmMetadataPath, validateMetadataPath,
calculatePathReplacements and depends on a single abstract method for base-dir
resolution (e.g., protected abstract String resolveBaseDir(String
primaryStorageUrl) or normalizeBaseDirFromUrl(String url)); then have
LocalStorageVmMetadataExtension and NfsVmMetadataExtension only implement that
base-dir resolver (and keep existing normalizeBaseDir usage confined there) so
all path rules live in one place and each subclass only provides
storage-specific URL→baseDir parsing.
In `@sdk/src/main/java/org/zstack/sdk/UpdateVmInstanceMetadataAction.java`:
- Around line 3-5: In UpdateVmInstanceMetadataAction remove the redundant
package wildcard import "import org.zstack.sdk.*;"—since
UpdateVmInstanceMetadataAction is in the same package, delete that import line
and keep only the explicit imports (e.g., java.util.HashMap, java.util.Map) so
the class compiles without unnecessary wildcard imports.
- Around line 31-35: The fields systemTags and userTags in
UpdateVmInstanceMetadataAction are declared as raw java.util.List; change their
declarations to use generics (java.util.List<String>) to match vmUuids and
ensure type safety and proper compilation; update any annotations or usages
expecting raw List to use List<String> (refer to the systemTags and userTags
fields and the UpdateVmInstanceMetadataAction class).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b4093a98-b67d-4340-b9bb-a90b8d236145
⛔ Files ignored due to path filters (5)
conf/globalConfig/vm.xmlis excluded by!**/*.xmlconf/persistence.xmlis excluded by!**/*.xmlconf/serviceConfig/primaryStorage.xmlis excluded by!**/*.xmlconf/serviceConfig/vmInstance.xmlis excluded by!**/*.xmlconf/springConfigXml/VmInstanceManager.xmlis excluded by!**/*.xml
📒 Files selected for processing (146)
compute/src/main/java/org/zstack/compute/vm/CleanupVmInstanceMetadataOnPrimaryStorageGC.javacompute/src/main/java/org/zstack/compute/vm/VmExpungeMetadataFlow.javacompute/src/main/java/org/zstack/compute/vm/VmGlobalConfig.javacompute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.javacompute/src/main/java/org/zstack/compute/vm/VmSystemTags.javaconf/db/zsv/V5.0.0__schema.sqlheader/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageEvent.javaheader/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageMsgDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/storage/primary/CleanupVmInstanceMetadataOnPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/CleanupVmInstanceMetadataOnPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/storage/primary/GetVmInstanceMetadataFromPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/GetVmInstanceMetadataFromPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/storage/primary/ReadVmInstanceMetadataMsg.javaheader/src/main/java/org/zstack/header/storage/primary/ReadVmInstanceMetadataReply.javaheader/src/main/java/org/zstack/header/storage/primary/RebaseVolumeBackingFileOnPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/RebaseVolumeBackingFileOnPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/storage/primary/ScanVmInstanceMetadataFromPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/ScanVmInstanceMetadataFromPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/storage/primary/VmMetadataScanEntry.javaheader/src/main/java/org/zstack/header/storage/primary/VmMetadataScanEntryDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/storage/snapshot/APIDeleteVolumeSnapshotMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/APIRevertVolumeFromSnapshotMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/APIShrinkVolumeSnapshotMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/APIUpdateVolumeSnapshotMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/group/APIDeleteVolumeSnapshotGroupMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/group/APIRevertVmFromSnapshotGroupMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/group/APIUngroupVolumeSnapshotGroupMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/group/APIUpdateVolumeSnapshotGroupMsg.javaheader/src/main/java/org/zstack/header/tag/APICreateSystemTagMsg.javaheader/src/main/java/org/zstack/header/tag/APICreateSystemTagsMsg.javaheader/src/main/java/org/zstack/header/tag/APIDeleteTagMsg.javaheader/src/main/java/org/zstack/header/tag/APIUpdateSystemTagMsg.javaheader/src/main/java/org/zstack/header/vm/APIAttachIsoToVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIAttachL3NetworkToVmMsg.javaheader/src/main/java/org/zstack/header/vm/APIAttachVmNicToVmMsg.javaheader/src/main/java/org/zstack/header/vm/APIChangeInstanceOfferingMsg.javaheader/src/main/java/org/zstack/header/vm/APIChangeVmNicNetworkMsg.javaheader/src/main/java/org/zstack/header/vm/APIChangeVmNicStateMsg.javaheader/src/main/java/org/zstack/header/vm/APIConvertTemplatedVmInstanceToVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIConvertVmInstanceToTemplatedVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIDeleteVmBootModeMsg.javaheader/src/main/java/org/zstack/header/vm/APIDeleteVmConsolePasswordMsg.javaheader/src/main/java/org/zstack/header/vm/APIDeleteVmHostnameMsg.javaheader/src/main/java/org/zstack/header/vm/APIDeleteVmSshKeyMsg.javaheader/src/main/java/org/zstack/header/vm/APIDeleteVmStaticIpMsg.javaheader/src/main/java/org/zstack/header/vm/APIDestroyVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIDetachIsoFromVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIDetachL3NetworkFromVmMsg.javaheader/src/main/java/org/zstack/header/vm/APIMigrateVmMsg.javaheader/src/main/java/org/zstack/header/vm/APIRecoverVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIReimageVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmBootModeMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmBootOrderMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmBootVolumeMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmClockTrackMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmConsolePasswordMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmHostnameMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmQxlMemoryMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmSoundTypeMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmSshKeyMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmStaticIpMsg.javaheader/src/main/java/org/zstack/header/vm/APIUpdateVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIUpdateVmNicDriverMsg.javaheader/src/main/java/org/zstack/header/vm/APIUpdateVmPriorityMsg.javaheader/src/main/java/org/zstack/header/vm/VmInstanceState.javaheader/src/main/java/org/zstack/header/vm/cdrom/APISetVmInstanceDefaultCdRomMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataEvent.javaheader/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataMsgDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageEvent.javaheader/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageMsgDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataEvent.javaheader/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataMsgDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataEvent.javaheader/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataMsgDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/MetadataImpact.javaheader/src/main/java/org/zstack/header/vm/metadata/ResourceMetadata.javaheader/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataOnPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataOnPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataReply.javaheader/src/main/java/org/zstack/header/vm/metadata/VmInstanceMetadataConstants.javaheader/src/main/java/org/zstack/header/vm/metadata/VmInstanceMetadataDTO.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataCategory.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyService.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyVO.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyVO_.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataErrors.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataFlushStateVO.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataFlushStateVO_.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataPathBuildExtensionPoint.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataPathReplacementExtensionPoint.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataResourcePersistExtensionPoint.javaheader/src/main/java/org/zstack/header/vm/metadata/VmUuidFromApiResolver.javaheader/src/main/java/org/zstack/header/vm/metadata/VolumeResourceMetadata.javaheader/src/main/java/org/zstack/header/volume/APIAttachDataVolumeToVmMsg.javaheader/src/main/java/org/zstack/header/volume/APIChangeVolumeStateMsg.javaheader/src/main/java/org/zstack/header/volume/APICreateVolumeSnapshotGroupMsg.javaheader/src/main/java/org/zstack/header/volume/APICreateVolumeSnapshotMsg.javaheader/src/main/java/org/zstack/header/volume/APIDeleteDataVolumeMsg.javaheader/src/main/java/org/zstack/header/volume/APIDetachDataVolumeFromVmMsg.javaheader/src/main/java/org/zstack/header/volume/APIExpungeDataVolumeMsg.javaheader/src/main/java/org/zstack/header/volume/APIFlattenVolumeMsg.javaheader/src/main/java/org/zstack/header/volume/APIRecoverDataVolumeMsg.javaheader/src/main/java/org/zstack/header/volume/APISyncVolumeSizeMsg.javaheader/src/main/java/org/zstack/header/volume/APIUndoSnapshotCreationMsg.javaheader/src/main/java/org/zstack/header/volume/APIUpdateVolumeMsg.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/APILocalStorageMigrateVolumeMsg.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageHypervisorBackend.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmBackend.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageSimulator.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageSimulatorConfig.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageVmMetadataExtension.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorage.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageBackend.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackendCommands.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsVmMetadataExtension.javaresourceconfig/src/main/java/org/zstack/resourceconfig/APIDeleteResourceConfigMsg.javaresourceconfig/src/main/java/org/zstack/resourceconfig/APIUpdateResourceConfigMsg.javasdk/src/main/java/SourceClassMap.javasdk/src/main/java/org/zstack/sdk/CleanupVmInstanceMetadataAction.javasdk/src/main/java/org/zstack/sdk/CleanupVmInstanceMetadataResult.javasdk/src/main/java/org/zstack/sdk/GetVmInstanceMetadataFromPrimaryStorageAction.javasdk/src/main/java/org/zstack/sdk/GetVmInstanceMetadataFromPrimaryStorageResult.javasdk/src/main/java/org/zstack/sdk/RegisterVmInstanceFromMetadataAction.javasdk/src/main/java/org/zstack/sdk/RegisterVmInstanceFromMetadataResult.javasdk/src/main/java/org/zstack/sdk/ScanVmInstanceMetadataFromPrimaryStorageAction.javasdk/src/main/java/org/zstack/sdk/ScanVmInstanceMetadataFromPrimaryStorageResult.javasdk/src/main/java/org/zstack/sdk/UpdateVmInstanceMetadataAction.javasdk/src/main/java/org/zstack/sdk/UpdateVmInstanceMetadataResult.javasdk/src/main/java/org/zstack/sdk/VmMetadataScanEntry.javastorage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.javatestlib/src/main/java/org/zstack/testlib/ApiHelper.groovytestlib/src/main/java/org/zstack/testlib/LocalStorageSpec.groovytestlib/src/main/java/org/zstack/testlib/NfsPrimaryStorageSpec.groovy
✅ Files skipped from review due to trivial changes (90)
- header/src/main/java/org/zstack/header/storage/primary/GetVmInstanceMetadataFromPrimaryStorageReply.java
- sdk/src/main/java/org/zstack/sdk/UpdateVmInstanceMetadataResult.java
- header/src/main/java/org/zstack/header/storage/primary/CleanupVmInstanceMetadataOnPrimaryStorageReply.java
- header/src/main/java/org/zstack/header/vm/APISetVmBootOrderMsg.java
- header/src/main/java/org/zstack/header/vm/APIDeleteVmBootModeMsg.java
- header/src/main/java/org/zstack/header/vm/APIMigrateVmMsg.java
- header/src/main/java/org/zstack/header/storage/primary/ReadVmInstanceMetadataMsg.java
- header/src/main/java/org/zstack/header/vm/APISetVmClockTrackMsg.java
- header/src/main/java/org/zstack/header/vm/APIUpdateVmPriorityMsg.java
- header/src/main/java/org/zstack/header/vm/APIDetachL3NetworkFromVmMsg.java
- header/src/main/java/org/zstack/header/vm/APIUpdateVmNicDriverMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataOnPrimaryStorageReply.java
- header/src/main/java/org/zstack/header/vm/APIRecoverVmInstanceMsg.java
- header/src/main/java/org/zstack/header/volume/APIUndoSnapshotCreationMsg.java
- header/src/main/java/org/zstack/header/storage/snapshot/APIRevertVolumeFromSnapshotMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataMsg.java
- header/src/main/java/org/zstack/header/vm/APISetVmHostnameMsg.java
- header/src/main/java/org/zstack/header/vm/APIDetachIsoFromVmInstanceMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/ResourceMetadata.java
- header/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataEventDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataCategory.java
- header/src/main/java/org/zstack/header/volume/APIChangeVolumeStateMsg.java
- header/src/main/java/org/zstack/header/volume/APIRecoverDataVolumeMsg.java
- header/src/main/java/org/zstack/header/volume/APISyncVolumeSizeMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/VmUuidFromApiResolver.java
- header/src/main/java/org/zstack/header/vm/metadata/VolumeResourceMetadata.java
- header/src/main/java/org/zstack/header/storage/primary/VmMetadataScanEntryDoc_zh_cn.groovy
- resourceconfig/src/main/java/org/zstack/resourceconfig/APIUpdateResourceConfigMsg.java
- header/src/main/java/org/zstack/header/vm/APIDeleteVmConsolePasswordMsg.java
- header/src/main/java/org/zstack/header/volume/APICreateVolumeSnapshotGroupMsg.java
- sdk/src/main/java/org/zstack/sdk/ScanVmInstanceMetadataFromPrimaryStorageResult.java
- header/src/main/java/org/zstack/header/vm/APIChangeInstanceOfferingMsg.java
- sdk/src/main/java/org/zstack/sdk/GetVmInstanceMetadataFromPrimaryStorageResult.java
- header/src/main/java/org/zstack/header/volume/APIAttachDataVolumeToVmMsg.java
- sdk/src/main/java/org/zstack/sdk/RegisterVmInstanceFromMetadataResult.java
- header/src/main/java/org/zstack/header/vm/APISetVmConsolePasswordMsg.java
- header/src/main/java/org/zstack/header/vm/APIConvertTemplatedVmInstanceToVmInstanceMsg.java
- header/src/main/java/org/zstack/header/storage/snapshot/APIShrinkVolumeSnapshotMsg.java
- header/src/main/java/org/zstack/header/vm/APISetVmStaticIpMsg.java
- header/src/main/java/org/zstack/header/vm/APIAttachVmNicToVmMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataMsgDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/tag/APICreateSystemTagsMsg.java
- header/src/main/java/org/zstack/header/vm/APIDestroyVmInstanceMsg.java
- compute/src/main/java/org/zstack/compute/vm/VmSystemTags.java
- header/src/main/java/org/zstack/header/tag/APICreateSystemTagMsg.java
- header/src/main/java/org/zstack/header/vm/APIDeleteVmStaticIpMsg.java
- header/src/main/java/org/zstack/header/vm/APISetVmSoundTypeMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/MetadataImpact.java
- header/src/main/java/org/zstack/header/vm/metadata/VmInstanceMetadataDTO.java
- header/src/main/java/org/zstack/header/vm/APISetVmBootVolumeMsg.java
- header/src/main/java/org/zstack/header/vm/APIDeleteVmSshKeyMsg.java
- header/src/main/java/org/zstack/header/storage/primary/VmMetadataScanEntry.java
- sdk/src/main/java/SourceClassMap.java
- header/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataReply.java
- compute/src/main/java/org/zstack/compute/vm/CleanupVmInstanceMetadataOnPrimaryStorageGC.java
- header/src/main/java/org/zstack/header/storage/snapshot/APIUpdateVolumeSnapshotMsg.java
- header/src/main/java/org/zstack/header/volume/APIDeleteDataVolumeMsg.java
- header/src/main/java/org/zstack/header/storage/primary/ScanVmInstanceMetadataFromPrimaryStorageMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataEventDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageEventDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageMsgDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataPathBuildExtensionPoint.java
- header/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataEvent.java
- header/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataEvent.java
- header/src/main/java/org/zstack/header/storage/primary/CleanupVmInstanceMetadataOnPrimaryStorageMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataPathReplacementExtensionPoint.java
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataFlushStateVO_.java
- header/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataEvent.java
- header/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataResourcePersistExtensionPoint.java
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyService.java
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyVO.java
- header/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataEventDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataMsg.java
- header/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageEventDoc_zh_cn.groovy
- conf/db/zsv/V5.0.0__schema.sql
- sdk/src/main/java/org/zstack/sdk/VmMetadataScanEntry.java
- header/src/main/java/org/zstack/header/storage/primary/ScanVmInstanceMetadataFromPrimaryStorageReply.java
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataErrors.java
- header/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageMsgDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataMsgDoc_zh_cn.groovy
- sdk/src/main/java/org/zstack/sdk/CleanupVmInstanceMetadataResult.java
- header/src/main/java/org/zstack/header/vm/APIChangeVmNicNetworkMsg.java
- header/src/main/java/org/zstack/header/vm/APIDeleteVmHostnameMsg.java
- header/src/main/java/org/zstack/header/storage/snapshot/group/APIDeleteVolumeSnapshotGroupMsg.java
- header/src/main/java/org/zstack/header/vm/APISetVmBootModeMsg.java
- header/src/main/java/org/zstack/header/storage/primary/ReadVmInstanceMetadataReply.java
- testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyVO_.java
- plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackendCommands.java
🚧 Files skipped from review as they are similar to previous changes (39)
- header/src/main/java/org/zstack/header/tag/APIDeleteTagMsg.java
- header/src/main/java/org/zstack/header/vm/cdrom/APISetVmInstanceDefaultCdRomMsg.java
- header/src/main/java/org/zstack/header/vm/APISetVmSshKeyMsg.java
- header/src/main/java/org/zstack/header/volume/APIDetachDataVolumeFromVmMsg.java
- header/src/main/java/org/zstack/header/vm/APIUpdateVmInstanceMsg.java
- header/src/main/java/org/zstack/header/vm/APIConvertVmInstanceToTemplatedVmInstanceMsg.java
- header/src/main/java/org/zstack/header/vm/APISetVmQxlMemoryMsg.java
- header/src/main/java/org/zstack/header/vm/APIAttachL3NetworkToVmMsg.java
- header/src/main/java/org/zstack/header/storage/snapshot/group/APIRevertVmFromSnapshotGroupMsg.java
- header/src/main/java/org/zstack/header/vm/APIReimageVmInstanceMsg.java
- header/src/main/java/org/zstack/header/vm/APIAttachIsoToVmInstanceMsg.java
- header/src/main/java/org/zstack/header/storage/snapshot/APIDeleteVolumeSnapshotMsg.java
- header/src/main/java/org/zstack/header/volume/APIFlattenVolumeMsg.java
- resourceconfig/src/main/java/org/zstack/resourceconfig/APIDeleteResourceConfigMsg.java
- header/src/main/java/org/zstack/header/volume/APICreateVolumeSnapshotMsg.java
- header/src/main/java/org/zstack/header/storage/snapshot/group/APIUpdateVolumeSnapshotGroupMsg.java
- testlib/src/main/java/org/zstack/testlib/NfsPrimaryStorageSpec.groovy
- header/src/main/java/org/zstack/header/storage/snapshot/group/APIUngroupVolumeSnapshotGroupMsg.java
- header/src/main/java/org/zstack/header/storage/primary/RebaseVolumeBackingFileOnPrimaryStorageReply.java
- header/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataMsgDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/vm/APIChangeVmNicStateMsg.java
- testlib/src/main/java/org/zstack/testlib/LocalStorageSpec.groovy
- header/src/main/java/org/zstack/header/tag/APIUpdateSystemTagMsg.java
- plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageSimulatorConfig.java
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataFlushStateVO.java
- header/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageEvent.java
- plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageBackend.java
- header/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageMsg.java
- plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageSimulator.java
- plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageHypervisorBackend.java
- header/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataOnPrimaryStorageMsg.java
- header/src/main/java/org/zstack/header/volume/APIExpungeDataVolumeMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/VmInstanceMetadataConstants.java
- sdk/src/main/java/org/zstack/sdk/GetVmInstanceMetadataFromPrimaryStorageAction.java
- header/src/main/java/org/zstack/header/storage/primary/GetVmInstanceMetadataFromPrimaryStorageMsg.java
- plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmBackend.java
- compute/src/main/java/org/zstack/compute/vm/VmGlobalConfig.java
- plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.java
Comment on lines
+87
to
+93
| if (hostUuid == null && ext.requireHostForCleanup()) { | ||
| logger.warn(String.format("[MetadataExpunge] vm[uuid:%s] hostUuid is null, " + | ||
| "ps[uuid:%s, type:%s] requires host for cleanup, skip without submitting GC", | ||
| vmUuid, psUuid, psType)); | ||
| trigger.next(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
缺少 host 时不要直接放弃 metadata cleanup。
这是 expunge 链里的 cleanup flow。对 requireHostForCleanup() 的后端，这个分支既不下发清理也不提交 GC；只要 expunge 时 hostUuid/lastHostUuid 已经为空，这份 metadata 就没有任何后续重试路径，最终会永久遗留在主存上。至少要提交 GC，或者在更早的流程里把 cleanup 所需的 hostUuid 固化到 VmInstanceSpec/flow context。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@compute/src/main/java/org/zstack/compute/vm/VmExpungeMetadataFlow.java`
around lines 87 - 93, The current branch in VmExpungeMetadataFlow that checks if
hostUuid == null && ext.requireHostForCleanup() incorrectly skips cleanup by
calling trigger.next() and returning; instead ensure metadata is not left behind
by submitting a GC task (or otherwise enqueue metadata cleanup) when hostUuid is
null for backends that require a host. Modify the branch around hostUuid,
ext.requireHostForCleanup(), psUuid/psType so that rather than only logging and
calling trigger.next(), it invokes the existing GC submission path (or creates a
submitMetadataGC(vmUuid, psUuid, psType) helper) and then proceeds with
trigger.next(); alternatively, ensure the required hostUuid is persisted earlier
into VmInstanceSpec/flow context before this flow runs (update the earlier flow
that sets ext/lastHostUuid if you choose that approach).
Comment on lines
+1366
to
+1399
| boolean psAttachedToCluster = Q.New(PrimaryStorageClusterRefVO.class) | ||
| .eq(PrimaryStorageClusterRefVO_.primaryStorageUuid, psUuid) | ||
| .eq(PrimaryStorageClusterRefVO_.clusterUuid, msg.getClusterUuid()) | ||
| .isExists(); | ||
| if (!psAttachedToCluster) { | ||
| throw new ApiMessageInterceptionException(argerr( | ||
| "primary storage[uuid:%s] is not attached to cluster[uuid:%s]", | ||
| psUuid, msg.getClusterUuid())); | ||
| } | ||
|
|
||
| if (msg.getHostUuid() != null) { | ||
| boolean hostAvailable = Q.New(HostVO.class) | ||
| .eq(HostVO_.uuid, msg.getHostUuid()) | ||
| .eq(HostVO_.clusterUuid, msg.getClusterUuid()) | ||
| .eq(HostVO_.state, HostState.Enabled) | ||
| .eq(HostVO_.status, HostStatus.Connected) | ||
| .isExists(); | ||
| if (!hostAvailable) { | ||
| throw new ApiMessageInterceptionException(argerr( | ||
| "host[uuid:%s] is not in cluster[uuid:%s] or not Enabled/Connected", msg.getHostUuid(), msg.getClusterUuid())); | ||
| } | ||
| } else { | ||
| boolean hasHost = Q.New(HostVO.class) | ||
| .eq(HostVO_.clusterUuid, msg.getClusterUuid()) | ||
| .eq(HostVO_.state, HostState.Enabled) | ||
| .eq(HostVO_.status, HostStatus.Connected) | ||
| .isExists(); | ||
| if (!hasHost) { | ||
| throw new ApiMessageInterceptionException(argerr( | ||
| "no available host found in cluster[uuid:%s], " + | ||
| "please specify hostUuid or ensure there is at least one connected host in the cluster", | ||
| msg.getClusterUuid())); | ||
| } | ||
| } |
There was a problem hiding this comment.
还需要校验 clusterUuid 和 zoneUuid 的归属关系。
现在只校验了主存储挂载到 clusterUuid、host 也在 clusterUuid 里，但没有约束这个 cluster 是否属于 msg.getZoneUuid()。传入跨 Zone 的 zoneUuid/clusterUuid 组合仍然能通过拦截，后续要么把 VM 以不一致的 zone/cluster 关系注册进去，要么在更深层流程才失败。建议在这里补一条 ClusterVO_.zoneUuid == msg.getZoneUuid() 的校验。
建议修改
+ boolean clusterInZone = Q.New(ClusterVO.class)
+ .eq(ClusterVO_.uuid, msg.getClusterUuid())
+ .eq(ClusterVO_.zoneUuid, msg.getZoneUuid())
+ .isExists();
+ if (!clusterInZone) {
+ throw new ApiMessageInterceptionException(argerr(
+ "cluster[uuid:%s] is not in zone[uuid:%s]",
+ msg.getClusterUuid(), msg.getZoneUuid()));
+ }
+
boolean psAttachedToCluster = Q.New(PrimaryStorageClusterRefVO.class)
.eq(PrimaryStorageClusterRefVO_.primaryStorageUuid, psUuid)
.eq(PrimaryStorageClusterRefVO_.clusterUuid, msg.getClusterUuid())
.isExists();import org.zstack.header.cluster.ClusterVO;
import org.zstack.header.cluster.ClusterVO_;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java`
around lines 1366 - 1399, Add a zone/cluster ownership check: before validating
primary storage and host, verify the cluster belongs to the message's zone by
querying ClusterVO for uuid == msg.getClusterUuid() and zoneUuid ==
msg.getZoneUuid() (use ClusterVO and ClusterVO_); if the query returns false
throw an ApiMessageInterceptionException with an argerr indicating the
cluster[uuid:%s] is not in zone[uuid:%s]. Update imports to include
org.zstack.header.cluster.ClusterVO and org.zstack.header.cluster.ClusterVO_ and
place this check in VmInstanceApiInterceptor alongside the existing
PrimaryStorageClusterRefVO and HostVO validations.
Comment on lines
+3371
to
+3378
| public String getSyncSignature() { | ||
| return String.format("update-metadata-on-ps-%s", self.getUuid()); | ||
| } | ||
|
|
||
| @Override | ||
| public int getSyncLevel() { | ||
| return 10; | ||
| } |
There was a problem hiding this comment.
Update/Cleanup 没有进入同一个串行队列。
Line 3372 用的是 PS 级 key，Line 3546 用的是 VM 级 key。这样同一台 VM 的异步 flush 和 cleanup 会落到不同 queue，上层一旦出现 expunge/GC 与 flush 交错，就可能出现“刚删掉又被旧快照写回”或“新 flush 被 cleanup 删掉”的竞态。
🛠 建议修正
`@Override`
public String getSyncSignature() {
- return String.format("update-metadata-on-ps-%s", self.getUuid());
+ return String.format("vm-metadata-on-ps-%s-vm-%s", self.getUuid(), msg.getVmInstanceUuid());
} `@Override`
public String getSyncSignature() {
- return String.format("update-metadata-on-ps-%s-%s", self.getUuid(), msg.getVmInstanceUuid());
+ return String.format("vm-metadata-on-ps-%s-vm-%s", self.getUuid(), msg.getVmInstanceUuid());
}如果这里的 getSyncLevel() 语义仍然是同一 signature 的并发度，也建议一起和 cleanup 对齐，避免同一 VM 并发写删。
Also applies to: 3545-3552
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java`
around lines 3371 - 3378, The code in LocalStorageBase defines
getSyncSignature() and getSyncLevel() for the update/cleanup path using a
primary-storage (PS)-level key (String.format("update-metadata-on-ps-%s",
self.getUuid())) which differs from the cleanup path that uses a VM-level key,
causing update/flush and cleanup to end up in different serial queues and race;
change getSyncSignature() to use the same VM-level key format used by the
cleanup path (e.g., include the VM UUID/identifier instead of PS UUID) and make
getSyncLevel() return the same concurrency level used by cleanup so
update/cleanup share the same serial queue; update references in
LocalStorageBase (getSyncSignature, getSyncLevel) to match the cleanup
implementation.
fe4d398 to
2f6db66
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (6)
header/src/main/java/org/zstack/header/vm/VmInstanceState.java (1)
205-207:⚠️ Potential issue | 🟠 Major
Registering的事件映射仍过窄，存在运行时异常风险。当前只处理
stopped，若收到unknown或noState，会在nextState()直接抛异常。建议补齐兜底迁移，避免注册流程因状态噪声失败。建议修改
Registering.transactions( - new Transaction(VmInstanceStateEvent.stopped, VmInstanceState.Stopped) + new Transaction(VmInstanceStateEvent.stopped, VmInstanceState.Stopped), + new Transaction(VmInstanceStateEvent.noState, VmInstanceState.NoState), + new Transaction(VmInstanceStateEvent.unknown, VmInstanceState.Unknown) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/vm/VmInstanceState.java` around lines 205 - 207, Registering 的事件映射在调用 Registering.transactions(new Transaction(VmInstanceStateEvent.stopped, VmInstanceState.Stopped)) 时只覆盖了 VmInstanceStateEvent.stopped，会导致当接收到 VmInstanceStateEvent.unknown 或 VmInstanceStateEvent.noState 时在 nextState() 抛出异常；请在 Registering 的 transactions 注册中补齐对 VmInstanceStateEvent.unknown 和 VmInstanceStateEvent.noState 的映射（例如映射到一个安全的兜底状态或保持当前状态），确保 Transaction 构造器调用覆盖这些事件，从而避免注册流程因状态噪声失败并消除 nextState() 的运行时异常风险。compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java (1)
1366-1399:⚠️ Potential issue | 🟠 Major还缺少
clusterUuid与zoneUuid的归属校验。这里现在只验证了主存储挂载关系和 host/cluster 关系，但没有确认
msg.getClusterUuid()属于msg.getZoneUuid()。跨 Zone 的zoneUuid/clusterUuid组合仍然可能通过拦截，后面的元数据注册会在不一致的拓扑上继续执行。建议修改
+ boolean clusterInZone = Q.New(ClusterVO.class) + .eq(ClusterVO_.uuid, msg.getClusterUuid()) + .eq(ClusterVO_.zoneUuid, msg.getZoneUuid()) + .isExists(); + if (!clusterInZone) { + throw new ApiMessageInterceptionException(argerr( + "cluster[uuid:%s] is not in zone[uuid:%s]", + msg.getClusterUuid(), msg.getZoneUuid())); + } + boolean psAttachedToCluster = Q.New(PrimaryStorageClusterRefVO.class) .eq(PrimaryStorageClusterRefVO_.primaryStorageUuid, psUuid) .eq(PrimaryStorageClusterRefVO_.clusterUuid, msg.getClusterUuid()) .isExists();另需补上
ClusterVO/ClusterVO_的 import。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java` around lines 1366 - 1399, The interceptor in VmInstanceApiInterceptor currently validates primary storage and host/cluster existence but fails to ensure the provided cluster (msg.getClusterUuid()) actually belongs to the provided zone (msg.getZoneUuid()); add a check using Q.New(ClusterVO.class).eq(ClusterVO_.uuid, msg.getClusterUuid()).eq(ClusterVO_.zoneUuid, msg.getZoneUuid()).isExists() and throw an ApiMessageInterceptionException(argerr(...)) when it does not; place this check before host availability logic (so topology is validated early) and add imports for ClusterVO and ClusterVO_.header/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageMsg.java (1)
8-12:⚠️ Potential issue | 🟠 MajorREST 路径这里仍是单数资源名。
@RestRequest现在还是/primary-storage/...。按仓库 API 规范，资源路径应使用复数形式；这里如果继续保留单数，后面修正时sdk/src/main/java/org/zstack/sdk/ScanVmInstanceMetadataFromPrimaryStorageAction.java的 Line 93-95 也要一起更新，否则 SDK 还会继续请求旧路径。建议修改
`@RestRequest`( - path = "/primary-storage/vm-instances/metadata/scan", + path = "/primary-storages/vm-instances/metadata/scan", method = HttpMethod.GET, responseClass = APIScanVmInstanceMetadataFromPrimaryStorageEvent.class )As per coding guidelines
@RestRequest的path规则要求“针对资源使用复数形式”。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageMsg.java` around lines 8 - 12, The REST path in APIScanVmInstanceMetadataFromPrimaryStorageMsg uses a singular resource name; change the `@RestRequest` path from "/primary-storage/vm-instances/metadata/scan" to the plural "/primary-storages/vm-instances/metadata/scan" to comply with the API guideline, and make the corresponding update in the generated SDK action ScanVmInstanceMetadataFromPrimaryStorageAction (the usage around the HTTP path in that class, currently around lines 93-95) so the SDK and server paths match.plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorage.java (1)
1953-1962:⚠️ Potential issue | 🟠 MajorNFS 元数据写入不应固定落到 VM 当前 host。
这里仍然直接用
getHostUuidFromVolume()的结果；当 VM 在 Running/Paused 且当前 host 已失联时，这条路径会比下面的Get/Cleanup/Rebase更脆弱——后者已经会回退到任意 connected host。对共享 NFS 来说，metadata write 也应该先优先用该 host（如果还连着），否则回退到可用 host，而不是直接失败。建议修改
public void run(SyncTaskChain chain) { UpdateVmInstanceMetadataOnPrimaryStorageReply reply = new UpdateVmInstanceMetadataOnPrimaryStorageReply(); + List<HostInventory> connectedHosts = factory.getConnectedHostForOperation(getSelfInventory()); + if (connectedHosts.isEmpty()) { + reply.setError(operr("no connected host found for NFS primary storage[uuid:%s]", self.getUuid())); + bus.reply(msg, reply); + chain.next(); + return; + } 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); - chain.next(); - return; + if (hostUuid == null || hostUuid.isEmpty() + || connectedHosts.stream().noneMatch(h -> h.getUuid().equals(hostUuid))) { + hostUuid = connectedHosts.get(0).getUuid(); } final NfsPrimaryStorageBackend backend = getBackendByHostUuid(hostUuid);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorage.java` around lines 1953 - 1962, The method currently fails fast when getHostUuidFromVolume(msg.getRootVolumeUuid()) returns null/unreachable; change the flow in the UpdateVmInstanceMetadataOnPrimaryStorage message handling so it first attempts to use the returned host UUID if that host is still connected, but if that host is null or not connected, pick a fallback connected host on the NFS primary storage (e.g., via a helper like getAnyConnectedHostUuidForPrimaryStorage or similar) and call getBackendByHostUuid(fallbackUuid) -> backend.handle(...); update the error path to only fail after no connected host can be found; reference the existing getHostUuidFromVolume(), getBackendByHostUuid(), and NfsPrimaryStorageBackend.handle(...) symbols when making this change.plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageVmMetadataExtension.java (1)
151-167:⚠️ Potential issue | 🟡 Minor
createResourceRef()仍然有并发窗口。Line 153 的
isExists()和 Line 167 的persist()之间是典型的 check-then-act。并发注册或重试同一个resourceUuid时，两个线程都可能通过检查，随后一个在线程在persist()处打出唯一键异常；如果底层没有唯一约束，还会直接写出重复 ref。这里需要把创建做成原子幂等：优先用 upsert，至少把唯一键冲突转成“已存在”成功。🛠 一个保守的兜底改法
private void createResourceRef(String primaryStorageUuid, String resourceUuid, String resourceType, String hostUuid, long size, Timestamp now) { - boolean exists = Q.New(LocalStorageResourceRefVO.class).eq(LocalStorageResourceRefVO_.resourceUuid, resourceUuid).isExists(); - if (exists) { - logger.debug(String.format("LocalStorageResourceRefVO for resource[uuid:%s] already exists, skip creation", resourceUuid)); - return; - } - LocalStorageResourceRefVO ref = new LocalStorageResourceRefVO(); ref.setPrimaryStorageUuid(primaryStorageUuid); ref.setResourceUuid(resourceUuid); ref.setResourceType(resourceType); ref.setHostUuid(hostUuid); ref.setSize(size); ref.setCreateDate(now); ref.setLastOpDate(now); - dbf.persist(ref); + try { + dbf.persist(ref); + } catch (RuntimeException e) { + if (Q.New(LocalStorageResourceRefVO.class) + .eq(LocalStorageResourceRefVO_.resourceUuid, resourceUuid) + .isExists()) { + logger.debug(String.format("LocalStorageResourceRefVO for resource[uuid:%s] already exists, skip creation", resourceUuid)); + return; + } + throw e; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageVmMetadataExtension.java` around lines 151 - 167, The createResourceRef method has a race between the Q.New(...).isExists() check and dbf.persist(ref) causing duplicate-key errors; make creation atomic and idempotent by replacing the check-then-act with an upsert or by catching unique-key/constraint exceptions around dbf.persist(ref) and treating them as success; specifically modify createResourceRef/LocalStorageResourceRefVO handling so that either (a) use an atomic upsert API on dbf to insert-or-update by resourceUuid, or (b) keep the insert but wrap dbf.persist(ref) in a try-catch that converts unique constraint errors on LocalStorageResourceRefVO.resourceUuid into a benign "already exists" outcome, while preserving other errors.plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java (1)
3371-3378:⚠️ Potential issue | 🟠 Major让 Update/Cleanup 走同一条串行队列。
Line 3372 现在按 PS 维度排队，而且 Line 3376 允许同一 signature 并发 10 个任务；Line 3546/3550 则按 VM 维度串行。这样同一台 VM 的 flush 和 cleanup 仍可能并发，出现“刚清掉又被旧快照写回”或“新 flush 被 cleanup 删掉”的竞态。
🛠 建议修改
`@Override` public String getSyncSignature() { - return String.format("update-metadata-on-ps-%s", self.getUuid()); + return String.format("vm-metadata-on-ps-%s-vm-%s", self.getUuid(), msg.getVmInstanceUuid()); } `@Override` public int getSyncLevel() { - return 10; + return 1; }`@Override` public String getSyncSignature() { - return String.format("update-metadata-on-ps-%s-%s", self.getUuid(), msg.getVmInstanceUuid()); + return String.format("vm-metadata-on-ps-%s-vm-%s", self.getUuid(), msg.getVmInstanceUuid()); }Also applies to: 3545-3552
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java` around lines 3371 - 3378, 当前实现的 getSyncSignature() 返回基于主存储 (PS) 的 signature 且 getSyncLevel() 返回 10，导致同一 VM 的 update（flush）和 cleanup 可能并发。将 getSyncSignature() 改为基于 VM（例如使用 self.getVmInstanceUuid() 或类似 VM 唯一标识）的统一 signature（使 update 和 cleanup 使用相同队列），并把 getSyncLevel() 调整为 1 以保证严格串行执行；修改涉及的方法名：getSyncSignature() 与 getSyncLevel()（确保与负责 update-metadata-on-ps / cleanup 的任务使用同一 signature）。
🧹 Nitpick comments (3)
sdk/src/main/java/org/zstack/sdk/UpdateVmInstanceMetadataAction.java (1)
28-29: 建议为批量vmUuids增加上限约束。Line 28 目前只校验
nonempty，没有限制批量大小。该接口是批量更新元数据，建议补充maxLength（例如 500）以避免超大请求放大后端压力，并与后端消息约束保持一致。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/src/main/java/org/zstack/sdk/UpdateVmInstanceMetadataAction.java` around lines 28 - 29, The vmUuids parameter in UpdateVmInstanceMetadataAction.java currently only enforces nonempty but lacks a batch-size cap; update the `@Param` annotation on the public java.util.List vmUuids field to include a maxLength (e.g. maxLength = 500) so the API rejects oversized requests and aligns with backend message constraints, keeping other attributes (required, nonempty, nullElements, emptyString, noTrim) unchanged.plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmBackend.java (2)
954-962: 内部类继承使用冗余的全限定类名。
PrefixRebaseBackingFilesCmd和PrefixRebaseBackingFilesRsp在类内部定义时，无需使用LocalStorageKvmBackend.AgentCommand和LocalStorageKvmBackend.AgentResponse，直接使用短名称即可，与文件中其他 DTO 保持一致。♻️ 建议修改
-public static class PrefixRebaseBackingFilesCmd extends LocalStorageKvmBackend.AgentCommand { +public static class PrefixRebaseBackingFilesCmd extends AgentCommand { public List<String> filePaths; public String oldPrefix; public String newPrefix; } -public static class PrefixRebaseBackingFilesRsp extends LocalStorageKvmBackend.AgentResponse { +public static class PrefixRebaseBackingFilesRsp extends AgentResponse { public int rebasedCount; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmBackend.java` around lines 954 - 962, The two inner DTO classes PrefixRebaseBackingFilesCmd and PrefixRebaseBackingFilesRsp currently extend LocalStorageKvmBackend.AgentCommand and LocalStorageKvmBackend.AgentResponse using redundant fully-qualified names; change their extends clauses to the short nested type names AgentCommand and AgentResponse respectively (i.e., make PrefixRebaseBackingFilesCmd extend AgentCommand and PrefixRebaseBackingFilesRsp extend AgentResponse) so they match other DTOs in the file and avoid redundant qualification—ensure the nested types AgentCommand and AgentResponse are visible in this scope (they already are as static nested types).
75-75: 通配符静态导入可接受，但显式导入更清晰。使用
import static org.zstack.core.Platform.*;会导入所有静态成员，可能引入不必要的符号。建议保持显式导入（如operr、inerr、multiErr）以提高代码可读性和避免命名冲突。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmBackend.java` at line 75, Replace the wildcard static import in LocalStorageKvmBackend (import static org.zstack.core.Platform.*;) with explicit static imports for only the used Platform symbols (for example operr, inerr, multiErr) so the class imports only those exact static methods; update the import line to list those symbols explicitly to improve clarity and avoid accidental name collisions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@header/src/main/java/org/zstack/header/vm/APIMigrateVmMsg.java`:
- Line 48: The MetadataImpact annotation on APIMigrateVmMsg (the line with
`@MetadataImpact`(value = MetadataImpact.Impact.STORAGE, resolver =
"VmUuidDirectResolver", field = "vmInstanceUuid")) should explicitly allow
updates on failure; change the annotation to include updateOnFailure = true (or,
if you intentionally require false, add a clear comment on APIMigrateVmMsg
explaining why failures must not update metadata). This ensures STORAGE-level
migration operations (and partial state changes on failure) are reflected in
metadata or document the rationale for keeping updateOnFailure=false.
In
`@header/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataMsg.java`:
- Around line 92-100: The example in
APIRegisterVmInstanceFromMetadataMsg.__example__() uses metadataPath
"/mnt/ps/vm-metadata/vm-uuid.vmmeta" which doesn't satisfy the required format
"<32 hex uuid>.vmmeta"; update the example to use a valid 32-character
hexadecimal UUID filename (e.g. "0123456789abcdef0123456789abcdef.vmmeta") for
the metadataPath field in the __example__() method so generated API docs/SDK
examples pass validation and are usable.
In
`@plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsVmMetadataExtension.java`:
- Around line 29-37: The method NfsVmMetadataExtension.resolveBaseDir currently
falls back to PrimaryStorageVO.url when mountPath is missing, producing
agent-inaccessible metadata paths; change resolveBaseDir to only read
PrimaryStorageVO_.mountPath and if that value is null throw a
CloudRuntimeException indicating mountPath is required (remove the url
fallback), and apply the same change for the analogous logic referenced around
lines 91-99 so any code that previously used url as a fallback fails fast and
clearly asks for mountPath to be configured.
In `@sdk/src/main/java/org/zstack/sdk/CleanupVmInstanceMetadataAction.java`:
- Around line 28-29: The vmUuids field in CleanupVmInstanceMetadataAction allows
unlimited and empty-string elements; update the `@Param` on the vmUuids field in
class CleanupVmInstanceMetadataAction to enforce a sensible batch upper bound
(e.g., max = 500) and disallow blank elements (set emptyString = false, keep
nullElements = false and nonempty = true), and add/ensure an element-level UUID
validity check (either via an elementPattern/regex in the annotation or by
validating each entry in the action's validate method) so requests with empty or
non-UUID values are rejected client-side before reaching the backend.
In `@storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java`:
- Around line 1908-1910: The filter on metadata uses e.getVmCategory() but
doesn't guard against null, so entries with vmCategory==null slip through;
update the filtering in the method handling List<VmMetadataScanEntry> (the
stream that references VmMetadataCategory.VM_TEMPLATE_CACHE and
e.getVmCategory()) to explicitly exclude null/empty categories (e.g., check
e.getVmCategory() != null && !e.getVmCategory().isEmpty()) before comparing to
VM_TEMPLATE_CACHE so incomplete/damaged scan entries are dropped; alternatively,
add a validation step earlier when building VmMetadataScanEntry to set or reject
null categories, but ensure the stream filter for VmMetadataScanEntry is made
null-safe.
In `@testlib/src/main/java/org/zstack/testlib/LocalStorageSpec.groovy`:
- Around line 604-618: The metadata simulators for LocalStorageKvmBackend
(WRITE_VM_METADATA_PATH, GET_VM_INSTANCE_METADATA_PATH, SCAN_VM_METADATA_PATH,
CLEANUP_VM_METADATA_PATH) currently return empty Rsp objects and do not share
state; implement a shared VFS-backed/in-memory state and update each simulator
to use VFS.vfsHook (or the same VFS helper used by other agent paths) so that
WriteVmMetadata simulator persists data, GetVmInstanceMetadata and
ScanVmMetadata read from that shared state, and CleanupVmMetadata deletes
entries; locate the simulators by the symbols
LocalStorageKvmBackend.WRITE_VM_METADATA_PATH,
LocalStorageKvmBackend.GET_VM_INSTANCE_METADATA_PATH,
LocalStorageKvmBackend.SCAN_VM_METADATA_PATH,
LocalStorageKvmBackend.CLEANUP_VM_METADATA_PATH and make them call the VFS
helper to mirror disk write/scan/delete behavior used elsewhere in the file.
---
Duplicate comments:
In `@compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java`:
- Around line 1366-1399: The interceptor in VmInstanceApiInterceptor currently
validates primary storage and host/cluster existence but fails to ensure the
provided cluster (msg.getClusterUuid()) actually belongs to the provided zone
(msg.getZoneUuid()); add a check using
Q.New(ClusterVO.class).eq(ClusterVO_.uuid,
msg.getClusterUuid()).eq(ClusterVO_.zoneUuid, msg.getZoneUuid()).isExists() and
throw an ApiMessageInterceptionException(argerr(...)) when it does not; place
this check before host availability logic (so topology is validated early) and
add imports for ClusterVO and ClusterVO_.
In
`@header/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageMsg.java`:
- Around line 8-12: The REST path in
APIScanVmInstanceMetadataFromPrimaryStorageMsg uses a singular resource name;
change the `@RestRequest` path from "/primary-storage/vm-instances/metadata/scan"
to the plural "/primary-storages/vm-instances/metadata/scan" to comply with the
API guideline, and make the corresponding update in the generated SDK action
ScanVmInstanceMetadataFromPrimaryStorageAction (the usage around the HTTP path
in that class, currently around lines 93-95) so the SDK and server paths match.
In `@header/src/main/java/org/zstack/header/vm/VmInstanceState.java`:
- Around line 205-207: Registering 的事件映射在调用 Registering.transactions(new
Transaction(VmInstanceStateEvent.stopped, VmInstanceState.Stopped)) 时只覆盖了
VmInstanceStateEvent.stopped，会导致当接收到 VmInstanceStateEvent.unknown 或
VmInstanceStateEvent.noState 时在 nextState() 抛出异常；请在 Registering 的 transactions
注册中补齐对 VmInstanceStateEvent.unknown 和 VmInstanceStateEvent.noState
的映射（例如映射到一个安全的兜底状态或保持当前状态），确保 Transaction 构造器调用覆盖这些事件，从而避免注册流程因状态噪声失败并消除
nextState() 的运行时异常风险。
In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java`:
- Around line 3371-3378: 当前实现的 getSyncSignature() 返回基于主存储 (PS) 的 signature 且
getSyncLevel() 返回 10，导致同一 VM 的 update（flush）和 cleanup 可能并发。将 getSyncSignature()
改为基于 VM（例如使用 self.getVmInstanceUuid() 或类似 VM 唯一标识）的统一 signature（使 update 和
cleanup 使用相同队列），并把 getSyncLevel() 调整为 1 以保证严格串行执行；修改涉及的方法名：getSyncSignature() 与
getSyncLevel()（确保与负责 update-metadata-on-ps / cleanup 的任务使用同一 signature）。
In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageVmMetadataExtension.java`:
- Around line 151-167: The createResourceRef method has a race between the
Q.New(...).isExists() check and dbf.persist(ref) causing duplicate-key errors;
make creation atomic and idempotent by replacing the check-then-act with an
upsert or by catching unique-key/constraint exceptions around dbf.persist(ref)
and treating them as success; specifically modify
createResourceRef/LocalStorageResourceRefVO handling so that either (a) use an
atomic upsert API on dbf to insert-or-update by resourceUuid, or (b) keep the
insert but wrap dbf.persist(ref) in a try-catch that converts unique constraint
errors on LocalStorageResourceRefVO.resourceUuid into a benign "already exists"
outcome, while preserving other errors.
In
`@plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorage.java`:
- Around line 1953-1962: The method currently fails fast when
getHostUuidFromVolume(msg.getRootVolumeUuid()) returns null/unreachable; change
the flow in the UpdateVmInstanceMetadataOnPrimaryStorage message handling so it
first attempts to use the returned host UUID if that host is still connected,
but if that host is null or not connected, pick a fallback connected host on the
NFS primary storage (e.g., via a helper like
getAnyConnectedHostUuidForPrimaryStorage or similar) and call
getBackendByHostUuid(fallbackUuid) -> backend.handle(...); update the error path
to only fail after no connected host can be found; reference the existing
getHostUuidFromVolume(), getBackendByHostUuid(), and
NfsPrimaryStorageBackend.handle(...) symbols when making this change.
---
Nitpick comments:
In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmBackend.java`:
- Around line 954-962: The two inner DTO classes PrefixRebaseBackingFilesCmd and
PrefixRebaseBackingFilesRsp currently extend LocalStorageKvmBackend.AgentCommand
and LocalStorageKvmBackend.AgentResponse using redundant fully-qualified names;
change their extends clauses to the short nested type names AgentCommand and
AgentResponse respectively (i.e., make PrefixRebaseBackingFilesCmd extend
AgentCommand and PrefixRebaseBackingFilesRsp extend AgentResponse) so they match
other DTOs in the file and avoid redundant qualification—ensure the nested types
AgentCommand and AgentResponse are visible in this scope (they already are as
static nested types).
- Line 75: Replace the wildcard static import in LocalStorageKvmBackend (import
static org.zstack.core.Platform.*;) with explicit static imports for only the
used Platform symbols (for example operr, inerr, multiErr) so the class imports
only those exact static methods; update the import line to list those symbols
explicitly to improve clarity and avoid accidental name collisions.
In `@sdk/src/main/java/org/zstack/sdk/UpdateVmInstanceMetadataAction.java`:
- Around line 28-29: The vmUuids parameter in
UpdateVmInstanceMetadataAction.java currently only enforces nonempty but lacks a
batch-size cap; update the `@Param` annotation on the public java.util.List
vmUuids field to include a maxLength (e.g. maxLength = 500) so the API rejects
oversized requests and aligns with backend message constraints, keeping other
attributes (required, nonempty, nullElements, emptyString, noTrim) unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 625420e2-f8af-4a6d-8fde-73ddf33a7ec6
⛔ Files ignored due to path filters (5)
conf/globalConfig/vm.xmlis excluded by!**/*.xmlconf/persistence.xmlis excluded by!**/*.xmlconf/serviceConfig/primaryStorage.xmlis excluded by!**/*.xmlconf/serviceConfig/vmInstance.xmlis excluded by!**/*.xmlconf/springConfigXml/VmInstanceManager.xmlis excluded by!**/*.xml
📒 Files selected for processing (146)
compute/src/main/java/org/zstack/compute/vm/CleanupVmInstanceMetadataOnPrimaryStorageGC.javacompute/src/main/java/org/zstack/compute/vm/VmExpungeMetadataFlow.javacompute/src/main/java/org/zstack/compute/vm/VmGlobalConfig.javacompute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.javacompute/src/main/java/org/zstack/compute/vm/VmSystemTags.javaconf/db/zsv/V5.0.0__schema.sqlheader/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageEvent.javaheader/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageMsgDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/storage/primary/CleanupVmInstanceMetadataOnPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/CleanupVmInstanceMetadataOnPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/storage/primary/GetVmInstanceMetadataFromPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/GetVmInstanceMetadataFromPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/storage/primary/ReadVmInstanceMetadataMsg.javaheader/src/main/java/org/zstack/header/storage/primary/ReadVmInstanceMetadataReply.javaheader/src/main/java/org/zstack/header/storage/primary/RebaseVolumeBackingFileOnPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/RebaseVolumeBackingFileOnPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/storage/primary/ScanVmInstanceMetadataFromPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/ScanVmInstanceMetadataFromPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/storage/primary/VmMetadataScanEntry.javaheader/src/main/java/org/zstack/header/storage/primary/VmMetadataScanEntryDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/storage/snapshot/APIDeleteVolumeSnapshotMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/APIRevertVolumeFromSnapshotMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/APIShrinkVolumeSnapshotMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/APIUpdateVolumeSnapshotMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/group/APIDeleteVolumeSnapshotGroupMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/group/APIRevertVmFromSnapshotGroupMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/group/APIUngroupVolumeSnapshotGroupMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/group/APIUpdateVolumeSnapshotGroupMsg.javaheader/src/main/java/org/zstack/header/tag/APICreateSystemTagMsg.javaheader/src/main/java/org/zstack/header/tag/APICreateSystemTagsMsg.javaheader/src/main/java/org/zstack/header/tag/APIDeleteTagMsg.javaheader/src/main/java/org/zstack/header/tag/APIUpdateSystemTagMsg.javaheader/src/main/java/org/zstack/header/vm/APIAttachIsoToVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIAttachL3NetworkToVmMsg.javaheader/src/main/java/org/zstack/header/vm/APIAttachVmNicToVmMsg.javaheader/src/main/java/org/zstack/header/vm/APIChangeInstanceOfferingMsg.javaheader/src/main/java/org/zstack/header/vm/APIChangeVmNicNetworkMsg.javaheader/src/main/java/org/zstack/header/vm/APIChangeVmNicStateMsg.javaheader/src/main/java/org/zstack/header/vm/APIConvertTemplatedVmInstanceToVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIConvertVmInstanceToTemplatedVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIDeleteVmBootModeMsg.javaheader/src/main/java/org/zstack/header/vm/APIDeleteVmConsolePasswordMsg.javaheader/src/main/java/org/zstack/header/vm/APIDeleteVmHostnameMsg.javaheader/src/main/java/org/zstack/header/vm/APIDeleteVmSshKeyMsg.javaheader/src/main/java/org/zstack/header/vm/APIDeleteVmStaticIpMsg.javaheader/src/main/java/org/zstack/header/vm/APIDestroyVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIDetachIsoFromVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIDetachL3NetworkFromVmMsg.javaheader/src/main/java/org/zstack/header/vm/APIMigrateVmMsg.javaheader/src/main/java/org/zstack/header/vm/APIRecoverVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIReimageVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmBootModeMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmBootOrderMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmBootVolumeMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmClockTrackMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmConsolePasswordMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmHostnameMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmQxlMemoryMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmSoundTypeMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmSshKeyMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmStaticIpMsg.javaheader/src/main/java/org/zstack/header/vm/APIUpdateVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIUpdateVmNicDriverMsg.javaheader/src/main/java/org/zstack/header/vm/APIUpdateVmPriorityMsg.javaheader/src/main/java/org/zstack/header/vm/VmInstanceState.javaheader/src/main/java/org/zstack/header/vm/cdrom/APISetVmInstanceDefaultCdRomMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataEvent.javaheader/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataMsgDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageEvent.javaheader/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageMsgDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataEvent.javaheader/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataMsgDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataEvent.javaheader/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataMsgDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/MetadataImpact.javaheader/src/main/java/org/zstack/header/vm/metadata/ResourceMetadata.javaheader/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataOnPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataOnPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataReply.javaheader/src/main/java/org/zstack/header/vm/metadata/VmInstanceMetadataConstants.javaheader/src/main/java/org/zstack/header/vm/metadata/VmInstanceMetadataDTO.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataCategory.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyService.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyVO.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyVO_.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataErrors.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataFlushStateVO.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataFlushStateVO_.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataPathBuildExtensionPoint.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataPathReplacementExtensionPoint.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataResourcePersistExtensionPoint.javaheader/src/main/java/org/zstack/header/vm/metadata/VmUuidFromApiResolver.javaheader/src/main/java/org/zstack/header/vm/metadata/VolumeResourceMetadata.javaheader/src/main/java/org/zstack/header/volume/APIAttachDataVolumeToVmMsg.javaheader/src/main/java/org/zstack/header/volume/APIChangeVolumeStateMsg.javaheader/src/main/java/org/zstack/header/volume/APICreateVolumeSnapshotGroupMsg.javaheader/src/main/java/org/zstack/header/volume/APICreateVolumeSnapshotMsg.javaheader/src/main/java/org/zstack/header/volume/APIDeleteDataVolumeMsg.javaheader/src/main/java/org/zstack/header/volume/APIDetachDataVolumeFromVmMsg.javaheader/src/main/java/org/zstack/header/volume/APIExpungeDataVolumeMsg.javaheader/src/main/java/org/zstack/header/volume/APIFlattenVolumeMsg.javaheader/src/main/java/org/zstack/header/volume/APIRecoverDataVolumeMsg.javaheader/src/main/java/org/zstack/header/volume/APISyncVolumeSizeMsg.javaheader/src/main/java/org/zstack/header/volume/APIUndoSnapshotCreationMsg.javaheader/src/main/java/org/zstack/header/volume/APIUpdateVolumeMsg.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/APILocalStorageMigrateVolumeMsg.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageHypervisorBackend.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmBackend.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageSimulator.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageSimulatorConfig.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageVmMetadataExtension.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorage.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageBackend.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackendCommands.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsVmMetadataExtension.javaresourceconfig/src/main/java/org/zstack/resourceconfig/APIDeleteResourceConfigMsg.javaresourceconfig/src/main/java/org/zstack/resourceconfig/APIUpdateResourceConfigMsg.javasdk/src/main/java/SourceClassMap.javasdk/src/main/java/org/zstack/sdk/CleanupVmInstanceMetadataAction.javasdk/src/main/java/org/zstack/sdk/CleanupVmInstanceMetadataResult.javasdk/src/main/java/org/zstack/sdk/GetVmInstanceMetadataFromPrimaryStorageAction.javasdk/src/main/java/org/zstack/sdk/GetVmInstanceMetadataFromPrimaryStorageResult.javasdk/src/main/java/org/zstack/sdk/RegisterVmInstanceFromMetadataAction.javasdk/src/main/java/org/zstack/sdk/RegisterVmInstanceFromMetadataResult.javasdk/src/main/java/org/zstack/sdk/ScanVmInstanceMetadataFromPrimaryStorageAction.javasdk/src/main/java/org/zstack/sdk/ScanVmInstanceMetadataFromPrimaryStorageResult.javasdk/src/main/java/org/zstack/sdk/UpdateVmInstanceMetadataAction.javasdk/src/main/java/org/zstack/sdk/UpdateVmInstanceMetadataResult.javasdk/src/main/java/org/zstack/sdk/VmMetadataScanEntry.javastorage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.javatestlib/src/main/java/org/zstack/testlib/ApiHelper.groovytestlib/src/main/java/org/zstack/testlib/LocalStorageSpec.groovytestlib/src/main/java/org/zstack/testlib/NfsPrimaryStorageSpec.groovy
✅ Files skipped from review due to trivial changes (84)
- header/src/main/java/org/zstack/header/storage/primary/GetVmInstanceMetadataFromPrimaryStorageReply.java
- header/src/main/java/org/zstack/header/storage/primary/ReadVmInstanceMetadataReply.java
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataCategory.java
- header/src/main/java/org/zstack/header/storage/primary/CleanupVmInstanceMetadataOnPrimaryStorageReply.java
- header/src/main/java/org/zstack/header/tag/APICreateSystemTagsMsg.java
- header/src/main/java/org/zstack/header/storage/primary/VmMetadataScanEntryDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/vm/APIChangeInstanceOfferingMsg.java
- header/src/main/java/org/zstack/header/vm/APISetVmStaticIpMsg.java
- header/src/main/java/org/zstack/header/vm/APISetVmSshKeyMsg.java
- header/src/main/java/org/zstack/header/storage/primary/RebaseVolumeBackingFileOnPrimaryStorageReply.java
- header/src/main/java/org/zstack/header/vm/APIChangeVmNicNetworkMsg.java
- header/src/main/java/org/zstack/header/vm/APISetVmBootVolumeMsg.java
- header/src/main/java/org/zstack/header/storage/snapshot/group/APIDeleteVolumeSnapshotGroupMsg.java
- header/src/main/java/org/zstack/header/vm/APISetVmHostnameMsg.java
- header/src/main/java/org/zstack/header/tag/APIUpdateSystemTagMsg.java
- header/src/main/java/org/zstack/header/volume/APIRecoverDataVolumeMsg.java
- header/src/main/java/org/zstack/header/storage/snapshot/APIUpdateVolumeSnapshotMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataReply.java
- header/src/main/java/org/zstack/header/vm/APIReimageVmInstanceMsg.java
- compute/src/main/java/org/zstack/compute/vm/VmSystemTags.java
- header/src/main/java/org/zstack/header/vm/APIChangeVmNicStateMsg.java
- header/src/main/java/org/zstack/header/vm/APISetVmSoundTypeMsg.java
- header/src/main/java/org/zstack/header/storage/primary/ReadVmInstanceMetadataMsg.java
- header/src/main/java/org/zstack/header/vm/APIRecoverVmInstanceMsg.java
- header/src/main/java/org/zstack/header/vm/APIDeleteVmBootModeMsg.java
- header/src/main/java/org/zstack/header/volume/APIChangeVolumeStateMsg.java
- header/src/main/java/org/zstack/header/tag/APICreateSystemTagMsg.java
- header/src/main/java/org/zstack/header/vm/APIAttachVmNicToVmMsg.java
- header/src/main/java/org/zstack/header/vm/APIDeleteVmSshKeyMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataOnPrimaryStorageReply.java
- header/src/main/java/org/zstack/header/vm/APISetVmConsolePasswordMsg.java
- header/src/main/java/org/zstack/header/volume/APIUndoSnapshotCreationMsg.java
- header/src/main/java/org/zstack/header/vm/APISetVmBootOrderMsg.java
- header/src/main/java/org/zstack/header/vm/APIUpdateVmPriorityMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataEventDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/vm/APIDeleteVmHostnameMsg.java
- plugin/localstorage/src/main/java/org/zstack/storage/primary/local/APILocalStorageMigrateVolumeMsg.java
- header/src/main/java/org/zstack/header/vm/APIDeleteVmStaticIpMsg.java
- header/src/main/java/org/zstack/header/volume/APIDeleteDataVolumeMsg.java
- header/src/main/java/org/zstack/header/storage/primary/ScanVmInstanceMetadataFromPrimaryStorageMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/ResourceMetadata.java
- header/src/main/java/org/zstack/header/vm/APISetVmBootModeMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyService.java
- header/src/main/java/org/zstack/header/storage/primary/GetVmInstanceMetadataFromPrimaryStorageMsg.java
- sdk/src/main/java/org/zstack/sdk/ScanVmInstanceMetadataFromPrimaryStorageResult.java
- header/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataMsgDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataEvent.java
- sdk/src/main/java/org/zstack/sdk/GetVmInstanceMetadataFromPrimaryStorageResult.java
- header/src/main/java/org/zstack/header/vm/APISetVmClockTrackMsg.java
- header/src/main/java/org/zstack/header/vm/APIDetachIsoFromVmInstanceMsg.java
- sdk/src/main/java/org/zstack/sdk/RegisterVmInstanceFromMetadataResult.java
- header/src/main/java/org/zstack/header/vm/APIConvertVmInstanceToTemplatedVmInstanceMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageEvent.java
- sdk/src/main/java/org/zstack/sdk/CleanupVmInstanceMetadataResult.java
- header/src/main/java/org/zstack/header/volume/APISyncVolumeSizeMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/VolumeResourceMetadata.java
- resourceconfig/src/main/java/org/zstack/resourceconfig/APIDeleteResourceConfigMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataEventDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/storage/primary/ScanVmInstanceMetadataFromPrimaryStorageReply.java
- header/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/MetadataImpact.java
- header/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataEventDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/storage/primary/VmMetadataScanEntry.java
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataResourcePersistExtensionPoint.java
- header/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataMsgDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageMsgDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataPathBuildExtensionPoint.java
- header/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataMsgDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/storage/primary/CleanupVmInstanceMetadataOnPrimaryStorageMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageEventDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/vm/metadata/VmInstanceMetadataDTO.java
- header/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataEvent.java
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataPathReplacementExtensionPoint.java
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataFlushStateVO_.java
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataErrors.java
- header/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageEventDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyVO_.java
- header/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageMsgDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataFlushStateVO.java
- sdk/src/main/java/org/zstack/sdk/VmMetadataScanEntry.java
- header/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataOnPrimaryStorageMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataEvent.java
- sdk/src/main/java/org/zstack/sdk/GetVmInstanceMetadataFromPrimaryStorageAction.java
- testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy
🚧 Files skipped from review as they are similar to previous changes (34)
- header/src/main/java/org/zstack/header/vm/APIAttachIsoToVmInstanceMsg.java
- header/src/main/java/org/zstack/header/vm/APIAttachL3NetworkToVmMsg.java
- compute/src/main/java/org/zstack/compute/vm/VmGlobalConfig.java
- header/src/main/java/org/zstack/header/tag/APIDeleteTagMsg.java
- header/src/main/java/org/zstack/header/vm/APIDeleteVmConsolePasswordMsg.java
- sdk/src/main/java/org/zstack/sdk/UpdateVmInstanceMetadataResult.java
- header/src/main/java/org/zstack/header/storage/snapshot/group/APIUngroupVolumeSnapshotGroupMsg.java
- header/src/main/java/org/zstack/header/volume/APICreateVolumeSnapshotMsg.java
- resourceconfig/src/main/java/org/zstack/resourceconfig/APIUpdateResourceConfigMsg.java
- header/src/main/java/org/zstack/header/volume/APIExpungeDataVolumeMsg.java
- header/src/main/java/org/zstack/header/storage/snapshot/group/APIUpdateVolumeSnapshotGroupMsg.java
- header/src/main/java/org/zstack/header/storage/snapshot/APIRevertVolumeFromSnapshotMsg.java
- header/src/main/java/org/zstack/header/vm/APIUpdateVmNicDriverMsg.java
- header/src/main/java/org/zstack/header/vm/APIDestroyVmInstanceMsg.java
- header/src/main/java/org/zstack/header/volume/APIFlattenVolumeMsg.java
- header/src/main/java/org/zstack/header/volume/APIUpdateVolumeMsg.java
- header/src/main/java/org/zstack/header/vm/APISetVmQxlMemoryMsg.java
- header/src/main/java/org/zstack/header/vm/cdrom/APISetVmInstanceDefaultCdRomMsg.java
- header/src/main/java/org/zstack/header/volume/APICreateVolumeSnapshotGroupMsg.java
- header/src/main/java/org/zstack/header/volume/APIDetachDataVolumeFromVmMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataMsg.java
- plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageSimulatorConfig.java
- sdk/src/main/java/SourceClassMap.java
- testlib/src/main/java/org/zstack/testlib/NfsPrimaryStorageSpec.groovy
- header/src/main/java/org/zstack/header/storage/snapshot/APIDeleteVolumeSnapshotMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/VmUuidFromApiResolver.java
- header/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageMsg.java
- plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageSimulator.java
- conf/db/zsv/V5.0.0__schema.sql
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyVO.java
- plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageBackend.java
- plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackendCommands.java
- compute/src/main/java/org/zstack/compute/vm/VmExpungeMetadataFlow.java
| ) | ||
| @SkipVmTracer(replyClass = APIMigrateVmEvent.class) | ||
| @DefaultTimeout(timeunit = TimeUnit.HOURS, value = 1) | ||
| @MetadataImpact(value = MetadataImpact.Impact.STORAGE, resolver = "VmUuidDirectResolver", field = "vmInstanceUuid") |
There was a problem hiding this comment.
建议迁移失败路径也更新元数据影响标记。
迁移属于 STORAGE 级别操作，失败时也可能发生部分状态变化；当前未设置 updateOnFailure = true，会增加元数据与实际状态不一致的风险。建议显式开启，或补充注释说明为何此处必须保持 false。
建议修改
-@MetadataImpact(value = MetadataImpact.Impact.STORAGE, resolver = "VmUuidDirectResolver", field = "vmInstanceUuid")
+@MetadataImpact(value = MetadataImpact.Impact.STORAGE, resolver = "VmUuidDirectResolver", field = "vmInstanceUuid", updateOnFailure = true)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@header/src/main/java/org/zstack/header/vm/APIMigrateVmMsg.java` at line 48,
The MetadataImpact annotation on APIMigrateVmMsg (the line with
`@MetadataImpact`(value = MetadataImpact.Impact.STORAGE, resolver =
"VmUuidDirectResolver", field = "vmInstanceUuid")) should explicitly allow
updates on failure; change the annotation to include updateOnFailure = true (or,
if you intentionally require false, add a clear comment on APIMigrateVmMsg
explaining why failures must not update metadata). This ensures STORAGE-level
migration operations (and partial state changes on failure) are reflected in
metadata or document the rationale for keeping updateOnFailure=false.
header/src/main/java/org/zstack/header/vm/APIRegisterVmInstanceFromMetadataMsg.java
Show resolved
Hide resolved
...n/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsVmMetadataExtension.java
Show resolved
Hide resolved
Comment on lines
+28
to
+29
| @Param(required = true, nonempty = true, nullElements = false, emptyString = true, noTrim = false) | ||
| public java.util.List vmUuids; |
There was a problem hiding this comment.
为 vmUuids 增加批量上限与元素有效性校验
Line 28 仅校验了 nonempty，但没有限制批量大小，且当前 emptyString = true 允许空字符串元素；这会把超大请求或明显非法 UUID 直接放到后端链路，增加清理任务与主存储侧压力。建议在 SDK 侧增加上限（如 500）并拒绝空白 UUID。
建议修改（示例）
public class CleanupVmInstanceMetadataAction extends AbstractAction {
+ private static final int MAX_VM_UUIDS = 500;
@@
`@Param`(required = true, nonempty = true, nullElements = false, emptyString = true, noTrim = false)
public java.util.List vmUuids;
@@
+ private void validateVmUuids() {
+ if (vmUuids == null || vmUuids.isEmpty()) {
+ throw new ApiException("vmUuids cannot be empty");
+ }
+ if (vmUuids.size() > MAX_VM_UUIDS) {
+ throw new ApiException(String.format("vmUuids size cannot exceed %d", MAX_VM_UUIDS));
+ }
+ for (Object it : vmUuids) {
+ if (!(it instanceof String) || ((String) it).trim().isEmpty()) {
+ throw new ApiException("vmUuids contains empty or invalid uuid");
+ }
+ }
+ }
@@
public Result call() {
+ validateVmUuids();
ApiResult res = ZSClient.call(this);
return makeResult(res);
}
@@
public void call(final Completion<Result> completion) {
+ validateVmUuids();
ZSClient.call(this, new InternalCompletion() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sdk/src/main/java/org/zstack/sdk/CleanupVmInstanceMetadataAction.java` around
lines 28 - 29, The vmUuids field in CleanupVmInstanceMetadataAction allows
unlimited and empty-string elements; update the `@Param` on the vmUuids field in
class CleanupVmInstanceMetadataAction to enforce a sensible batch upper bound
(e.g., max = 500) and disallow blank elements (set emptyString = false, keep
nullElements = false and nonempty = true), and add/ensure an element-level UUID
validity check (either via an elementPattern/regex in the annotation or by
validating each entry in the action's validate method) so requests with empty or
non-UUID values are rejected client-side before reaching the backend.
storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java
Show resolved
Hide resolved
Comment on lines
+604
to
+618
| simulator(LocalStorageKvmBackend.WRITE_VM_METADATA_PATH) { | ||
| return new LocalStorageKvmBackend.WriteVmMetadataRsp() | ||
| } | ||
|
|
||
| simulator(LocalStorageKvmBackend.GET_VM_INSTANCE_METADATA_PATH) { | ||
| return new LocalStorageKvmBackend.GetVmInstanceMetadataRsp() | ||
| } | ||
|
|
||
| simulator(LocalStorageKvmBackend.SCAN_VM_METADATA_PATH) { | ||
| return new LocalStorageKvmBackend.ScanVmMetadataRsp() | ||
| } | ||
|
|
||
| simulator(LocalStorageKvmBackend.CLEANUP_VM_METADATA_PATH) { | ||
| return new LocalStorageKvmBackend.CleanupVmMetadataRsp() | ||
| } |
There was a problem hiding this comment.
这组 metadata simulator 现在不会保存任何状态。
WRITE_VM_METADATA_PATH、GET_VM_INSTANCE_METADATA_PATH、SCAN_VM_METADATA_PATH 和 CLEANUP_VM_METADATA_PATH 这里只返回空 Rsp，没有像同文件其他 agent path 一样通过 VFS.vfsHook(...) 去落盘、扫描或删除。这样 testlib 里写后读、scan 结果和 cleanup 行为都测不到，新增元数据流程的用例很容易变成假阳性。建议至少共用一份 VFS/内存状态，让 WRITE 修改状态、GET/SCAN 读取状态、CLEANUP 删除状态。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@testlib/src/main/java/org/zstack/testlib/LocalStorageSpec.groovy` around
lines 604 - 618, The metadata simulators for LocalStorageKvmBackend
(WRITE_VM_METADATA_PATH, GET_VM_INSTANCE_METADATA_PATH, SCAN_VM_METADATA_PATH,
CLEANUP_VM_METADATA_PATH) currently return empty Rsp objects and do not share
state; implement a shared VFS-backed/in-memory state and update each simulator
to use VFS.vfsHook (or the same VFS helper used by other agent paths) so that
WriteVmMetadata simulator persists data, GetVmInstanceMetadata and
ScanVmMetadata read from that shared state, and CleanupVmMetadata deletes
entries; locate the simulators by the symbols
LocalStorageKvmBackend.WRITE_VM_METADATA_PATH,
LocalStorageKvmBackend.GET_VM_INSTANCE_METADATA_PATH,
LocalStorageKvmBackend.SCAN_VM_METADATA_PATH,
LocalStorageKvmBackend.CLEANUP_VM_METADATA_PATH and make them call the VFS
helper to mirror disk write/scan/delete behavior used elsewhere in the file.
1a04544 to
22eabd1
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
header/src/main/java/org/zstack/header/vm/VmInstanceState.java (1)
205-207:⚠️ Potential issue | 🟠 Major
Registering的状态迁移仍然过窄，存在未映射事件抛异常风险。Line 205-207 当前只处理
stopped，当收到unknown或noState时会在nextState()抛CloudRuntimeException。这个问题与之前评审意见一致，当前仍未消除。建议修复
Registering.transactions( - new Transaction(VmInstanceStateEvent.stopped, VmInstanceState.Stopped) + new Transaction(VmInstanceStateEvent.stopped, VmInstanceState.Stopped), + new Transaction(VmInstanceStateEvent.noState, VmInstanceState.NoState), + new Transaction(VmInstanceStateEvent.unknown, VmInstanceState.Unknown) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/vm/VmInstanceState.java` around lines 205 - 207, The Registering state transition table is too narrow and only maps VmInstanceStateEvent.stopped to VmInstanceState.Stopped, causing nextState() to throw on events like unknown or noState; update the Registering.transactions(...) block to include mappings for VmInstanceStateEvent.unknown and VmInstanceStateEvent.noState (or a default/fallback transaction) to appropriate VmInstanceState values (e.g., VmInstanceState.Unknown or VmInstanceState.NoState) or add a safe default transaction so nextState() never throws for unhandled events; modify the Transaction entries in the Registering.transactions call accordingly to cover these events.header/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataMsg.java (1)
92-100:⚠️ Potential issue | 🟡 Minor
__example__()里的metadataPath仍然是无效示例。这里还是
vm-uuid.vmmeta，不满足当前<32位十六进制uuid>.vmmeta的校验规则；生成的 API 文档和 SDK 示例会直接不可用。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataMsg.java` around lines 92 - 100, Update the invalid example in APIRegisterVmInstanceFromMetadataMsg.__example__(): metadataPath must follow the "<32-hex-uuid>.vmmeta" rule, so construct the path using a generated VM UUID (e.g., use uuid(VmInstanceVO.class) or equivalent) and append ".vmmeta" so metadataPath becomes a valid "<32hex>.vmmeta" filename under the existing directory; keep all other example fields unchanged.plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageVmMetadataExtension.java (1)
151-167:⚠️ Potential issue | 🟠 Major
createResourceRef()的判重条件过宽，而且不是原子操作。
plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageResourceRefVO.java:15-35里的主键是(resourceUuid, primaryStorageUuid, hostUuid)，但这里先只按resourceUuid做isExists()，再执行dbf.persist(ref)。这既会把不同 host/PS 组合的记录误判成已存在，也存在典型的 check-then-insert 竞态，并发时仍可能打到主键冲突。请按完整主键做幂等化，并在插入点兜住重复键异常或改成原子 upsert。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageVmMetadataExtension.java` around lines 151 - 167, The current createResourceRef method only checks existence by resourceUuid and does a non-atomic persist, which misidentifies distinct (resourceUuid, primaryStorageUuid, hostUuid) rows and is prone to race conditions; change the existence check to match the full composite key (resourceUuid, primaryStorageUuid, hostUuid) using Q.New(LocalStorageResourceRefVO.class).eq(...).eq(...).eq(...).isExists(), and make the insert atomic by either performing an upsert (replace/insert semantics) or wrapping dbf.persist(ref) in a try/catch that catches the duplicate-key/constraint exception and treats it as a no-op; reference LocalStorageResourceRefVO (composite key), createResourceRef, and dbf.persist when implementing the fix.sdk/src/main/java/org/zstack/sdk/CleanupVmInstanceMetadataAction.java (1)
28-29:⚠️ Potential issue | 🟠 Major收紧
vmUuids的客户端约束。这里只有
nonempty，没有批量上限，且emptyString = true仍会放过空字符串元素；超大列表或空 UUID 会直接进入后端清理链路，放大主存储侧的负载和失败重试成本。若注解层表达不了 size/UUID 规则，至少在call()/call(Completion<Result>)前做显式校验。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/src/main/java/org/zstack/sdk/CleanupVmInstanceMetadataAction.java` around lines 28 - 29, The vmUuids field in CleanupVmInstanceMetadataAction lacks strict client-side validation, so add explicit checks in the class (before any remote invocation) — validate that vmUuids is non-null, non-empty, does not exceed a reasonable batch limit (e.g., N entries), and that each element is a non-blank, trimmed UUID matching the UUID pattern; reject or short-circuit with a clear error (throw IllegalArgumentException or fail the Result) when validation fails. Implement these checks at the start of both call() and call(Completion<Result>) methods (referencing class CleanupVmInstanceMetadataAction and field vmUuids) so oversized lists or empty/invalid UUIDs are caught client-side and never sent to the backend. Ensure error messages identify the failing index/value and mention the configured max size.plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java (1)
3371-3378:⚠️ Potential issue | 🟠 Major让 flush 和 cleanup 共用同一条 VM 级串行队列。
Line 3372 仍按 PS 维度排队且 Line 3376 的并发度是 10，而 Line 3546/Line 3550 已经按 VM 维度串行化。这样同一台 VM 的异步 flush 和 cleanup 还是会并发，旧快照可能在 cleanup 之后被重新写回，或者新 flush 被 cleanup 删除。这个点前一轮已经提过，当前提交里还没对齐。
🛠 建议修正
`@Override` public String getSyncSignature() { - return String.format("update-metadata-on-ps-%s", self.getUuid()); + return String.format("vm-metadata-on-ps-%s-vm-%s", self.getUuid(), msg.getVmInstanceUuid()); } `@Override` public int getSyncLevel() { - return 10; + return 1; }`@Override` public String getSyncSignature() { - return String.format("update-metadata-on-ps-%s-%s", self.getUuid(), msg.getVmInstanceUuid()); + return String.format("vm-metadata-on-ps-%s-vm-%s", self.getUuid(), msg.getVmInstanceUuid()); }Also applies to: 3545-3552
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java` around lines 3371 - 3378, The current getSyncSignature() and getSyncLevel() (methods getSyncSignature and getSyncLevel in this diff) use the primary-storage UUID and a concurrency of 10, which keeps queuing at PS-level and allows flush and cleanup for the same VM to run concurrently; change getSyncSignature() to return a VM-scoped key (use the VM's UUID / vmInstanceUuid used by the VM-related operations) so it matches the VM-level serialization used elsewhere, and set getSyncLevel() to the same sync level/concurrency used by the VM-scoped operations (the same value used in the code around the VM-serialized block at lines ~3546/3550) so flush and cleanup share the same VM-level serial queue. Ensure the string format identifies the VM (e.g., "update-metadata-on-vm-%s") and that both methods are updated together so flush and cleanup use identical queue keys and levels.
🧹 Nitpick comments (2)
header/src/main/java/org/zstack/header/vm/metadata/VmMetadataFlushStateVO.java (1)
32-32: 显式指定表名以避免命名策略漂移。当前
@Table未指定 name 属性，Hibernate 将使用默认命名策略（即类名VmMetadataFlushStateVO）。虽然当前与 SQL 迁移脚本中的表名一致，但若实体名被重构或命名策略改变，将导致 JPA 映射与已固定的数据库表名产生偏差。建议修改
-@Table +@Table(name = "VmMetadataFlushStateVO") public class VmMetadataFlushStateVO {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/vm/metadata/VmMetadataFlushStateVO.java` at line 32, 在 VmMetadataFlushStateVO 上的 `@Table` 注解没有显式 name，可能随实体重构或命名策略变化导致映射偏移；修改 VmMetadataFlushStateVO 类，将 `@Table` 更改为显式指定数据库表名（name="你们迁移脚本中使用的表名"，例如 "vm_metadata_flush_state"）以保证 JPA 实体永远映射到固定的迁移表名。plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorage.java (1)
1994-1998: 可考虑简化 lambda 变量捕获写法。当前使用
finalHostUuid作为中间变量仅为满足 lambda effectively final 要求。功能正确，但可以考虑简化：♻️ 可选的简化方案
- String hostUuid = msg.getHostUuid(); - String finalHostUuid = hostUuid; - if (hostUuid == null || !connectedHosts.stream().anyMatch(h -> h.getUuid().equals(finalHostUuid))) { - hostUuid = connectedHosts.get(0).getUuid(); - } + String requestedHostUuid = msg.getHostUuid(); + String hostUuid = (requestedHostUuid != null && connectedHosts.stream().anyMatch(h -> h.getUuid().equals(requestedHostUuid))) + ? requestedHostUuid + : connectedHosts.get(0).getUuid();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorage.java` around lines 1994 - 1998, Replace the redundant intermediate variable finalHostUuid by calling msg.getHostUuid() directly inside the lambda (or store it in a single final local if you prefer a named final). Concretely, change the check to use if (hostUuid == null || !connectedHosts.stream().anyMatch(h -> h.getUuid().equals(msg.getHostUuid()))) { hostUuid = connectedHosts.get(0).getUuid(); } so you remove finalHostUuid while preserving the effectively-final requirement for the lambda and keep the same behavior for hostUuid/connectedHosts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@header/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataMsg.java`:
- Around line 17-25: Add RBAC action entries for the new metadata APIs so they
are grantable under existing roles: update the main role definition JSON
(conf/roleDefinitions/roleDefinition.json) to include actions for
APIRegisterVmInstanceFromMetadataMsg, APICleanupVmInstanceMetadataMsg,
APIScanVmInstanceMetadataFromPrimaryStorageMsg,
APIGetVmInstanceMetadataFromPrimaryStorageMsg, and
APIUpdateVmInstanceMetadataMsg; ensure each entry uses the same action naming
scheme as other VM instance APIs and points to the VmInstance resource type so
the RBAC engine can authorize these new endpoints.
In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java`:
- Around line 3474-3481: The current LocalStorageBase logic only queries
connected hosts into connectedHostUuids and silently skips disconnected hosts,
which leads to partial, misleading success replies; update the scan flow (the
SQL.New query/variable connectedHostUuids and the code that builds
failedHostUuids/reply) to include disconnected hosts as failures (add their
UUIDs to failedHostUuids) and ensure the method surfaces partial failures to the
upper-layer event (or, at minimum, return an error when any host was unscanned)
so callers do not receive a “success + incomplete result.”
---
Duplicate comments:
In
`@header/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataMsg.java`:
- Around line 92-100: Update the invalid example in
APIRegisterVmInstanceFromMetadataMsg.__example__(): metadataPath must follow the
"<32-hex-uuid>.vmmeta" rule, so construct the path using a generated VM UUID
(e.g., use uuid(VmInstanceVO.class) or equivalent) and append ".vmmeta" so
metadataPath becomes a valid "<32hex>.vmmeta" filename under the existing
directory; keep all other example fields unchanged.
In `@header/src/main/java/org/zstack/header/vm/VmInstanceState.java`:
- Around line 205-207: The Registering state transition table is too narrow and
only maps VmInstanceStateEvent.stopped to VmInstanceState.Stopped, causing
nextState() to throw on events like unknown or noState; update the
Registering.transactions(...) block to include mappings for
VmInstanceStateEvent.unknown and VmInstanceStateEvent.noState (or a
default/fallback transaction) to appropriate VmInstanceState values (e.g.,
VmInstanceState.Unknown or VmInstanceState.NoState) or add a safe default
transaction so nextState() never throws for unhandled events; modify the
Transaction entries in the Registering.transactions call accordingly to cover
these events.
In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java`:
- Around line 3371-3378: The current getSyncSignature() and getSyncLevel()
(methods getSyncSignature and getSyncLevel in this diff) use the primary-storage
UUID and a concurrency of 10, which keeps queuing at PS-level and allows flush
and cleanup for the same VM to run concurrently; change getSyncSignature() to
return a VM-scoped key (use the VM's UUID / vmInstanceUuid used by the
VM-related operations) so it matches the VM-level serialization used elsewhere,
and set getSyncLevel() to the same sync level/concurrency used by the VM-scoped
operations (the same value used in the code around the VM-serialized block at
lines ~3546/3550) so flush and cleanup share the same VM-level serial queue.
Ensure the string format identifies the VM (e.g., "update-metadata-on-vm-%s")
and that both methods are updated together so flush and cleanup use identical
queue keys and levels.
In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageVmMetadataExtension.java`:
- Around line 151-167: The current createResourceRef method only checks
existence by resourceUuid and does a non-atomic persist, which misidentifies
distinct (resourceUuid, primaryStorageUuid, hostUuid) rows and is prone to race
conditions; change the existence check to match the full composite key
(resourceUuid, primaryStorageUuid, hostUuid) using
Q.New(LocalStorageResourceRefVO.class).eq(...).eq(...).eq(...).isExists(), and
make the insert atomic by either performing an upsert (replace/insert semantics)
or wrapping dbf.persist(ref) in a try/catch that catches the
duplicate-key/constraint exception and treats it as a no-op; reference
LocalStorageResourceRefVO (composite key), createResourceRef, and dbf.persist
when implementing the fix.
In `@sdk/src/main/java/org/zstack/sdk/CleanupVmInstanceMetadataAction.java`:
- Around line 28-29: The vmUuids field in CleanupVmInstanceMetadataAction lacks
strict client-side validation, so add explicit checks in the class (before any
remote invocation) — validate that vmUuids is non-null, non-empty, does not
exceed a reasonable batch limit (e.g., N entries), and that each element is a
non-blank, trimmed UUID matching the UUID pattern; reject or short-circuit with
a clear error (throw IllegalArgumentException or fail the Result) when
validation fails. Implement these checks at the start of both call() and
call(Completion<Result>) methods (referencing class
CleanupVmInstanceMetadataAction and field vmUuids) so oversized lists or
empty/invalid UUIDs are caught client-side and never sent to the backend. Ensure
error messages identify the failing index/value and mention the configured max
size.
---
Nitpick comments:
In
`@header/src/main/java/org/zstack/header/vm/metadata/VmMetadataFlushStateVO.java`:
- Line 32: 在 VmMetadataFlushStateVO 上的 `@Table` 注解没有显式
name，可能随实体重构或命名策略变化导致映射偏移；修改 VmMetadataFlushStateVO 类，将 `@Table`
更改为显式指定数据库表名（name="你们迁移脚本中使用的表名"，例如 "vm_metadata_flush_state"）以保证 JPA
实体永远映射到固定的迁移表名。
In
`@plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorage.java`:
- Around line 1994-1998: Replace the redundant intermediate variable
finalHostUuid by calling msg.getHostUuid() directly inside the lambda (or store
it in a single final local if you prefer a named final). Concretely, change the
check to use if (hostUuid == null || !connectedHosts.stream().anyMatch(h ->
h.getUuid().equals(msg.getHostUuid()))) { hostUuid =
connectedHosts.get(0).getUuid(); } so you remove finalHostUuid while preserving
the effectively-final requirement for the lambda and keep the same behavior for
hostUuid/connectedHosts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ae918d06-01a7-4753-910f-7f6fce786aa2
⛔ Files ignored due to path filters (5)
conf/globalConfig/vm.xmlis excluded by!**/*.xmlconf/persistence.xmlis excluded by!**/*.xmlconf/serviceConfig/primaryStorage.xmlis excluded by!**/*.xmlconf/serviceConfig/vmInstance.xmlis excluded by!**/*.xmlconf/springConfigXml/VmInstanceManager.xmlis excluded by!**/*.xml
📒 Files selected for processing (146)
compute/src/main/java/org/zstack/compute/vm/CleanupVmInstanceMetadataOnPrimaryStorageGC.javacompute/src/main/java/org/zstack/compute/vm/VmExpungeMetadataFlow.javacompute/src/main/java/org/zstack/compute/vm/VmGlobalConfig.javacompute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.javacompute/src/main/java/org/zstack/compute/vm/VmSystemTags.javaconf/db/zsv/V5.0.0__schema.sqlheader/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageEvent.javaheader/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageMsgDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/storage/primary/CleanupVmInstanceMetadataOnPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/CleanupVmInstanceMetadataOnPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/storage/primary/GetVmInstanceMetadataFromPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/GetVmInstanceMetadataFromPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/storage/primary/ReadVmInstanceMetadataMsg.javaheader/src/main/java/org/zstack/header/storage/primary/ReadVmInstanceMetadataReply.javaheader/src/main/java/org/zstack/header/storage/primary/RebaseVolumeBackingFileOnPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/RebaseVolumeBackingFileOnPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/storage/primary/ScanVmInstanceMetadataFromPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/ScanVmInstanceMetadataFromPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/storage/primary/VmMetadataScanEntry.javaheader/src/main/java/org/zstack/header/storage/primary/VmMetadataScanEntryDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/storage/snapshot/APIDeleteVolumeSnapshotMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/APIRevertVolumeFromSnapshotMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/APIShrinkVolumeSnapshotMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/APIUpdateVolumeSnapshotMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/group/APIDeleteVolumeSnapshotGroupMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/group/APIRevertVmFromSnapshotGroupMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/group/APIUngroupVolumeSnapshotGroupMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/group/APIUpdateVolumeSnapshotGroupMsg.javaheader/src/main/java/org/zstack/header/tag/APICreateSystemTagMsg.javaheader/src/main/java/org/zstack/header/tag/APICreateSystemTagsMsg.javaheader/src/main/java/org/zstack/header/tag/APIDeleteTagMsg.javaheader/src/main/java/org/zstack/header/tag/APIUpdateSystemTagMsg.javaheader/src/main/java/org/zstack/header/vm/APIAttachIsoToVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIAttachL3NetworkToVmMsg.javaheader/src/main/java/org/zstack/header/vm/APIAttachVmNicToVmMsg.javaheader/src/main/java/org/zstack/header/vm/APIChangeInstanceOfferingMsg.javaheader/src/main/java/org/zstack/header/vm/APIChangeVmNicNetworkMsg.javaheader/src/main/java/org/zstack/header/vm/APIChangeVmNicStateMsg.javaheader/src/main/java/org/zstack/header/vm/APIConvertTemplatedVmInstanceToVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIConvertVmInstanceToTemplatedVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIDeleteVmBootModeMsg.javaheader/src/main/java/org/zstack/header/vm/APIDeleteVmConsolePasswordMsg.javaheader/src/main/java/org/zstack/header/vm/APIDeleteVmHostnameMsg.javaheader/src/main/java/org/zstack/header/vm/APIDeleteVmSshKeyMsg.javaheader/src/main/java/org/zstack/header/vm/APIDeleteVmStaticIpMsg.javaheader/src/main/java/org/zstack/header/vm/APIDestroyVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIDetachIsoFromVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIDetachL3NetworkFromVmMsg.javaheader/src/main/java/org/zstack/header/vm/APIMigrateVmMsg.javaheader/src/main/java/org/zstack/header/vm/APIRecoverVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIReimageVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmBootModeMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmBootOrderMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmBootVolumeMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmClockTrackMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmConsolePasswordMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmHostnameMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmQxlMemoryMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmSoundTypeMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmSshKeyMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmStaticIpMsg.javaheader/src/main/java/org/zstack/header/vm/APIUpdateVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIUpdateVmNicDriverMsg.javaheader/src/main/java/org/zstack/header/vm/APIUpdateVmPriorityMsg.javaheader/src/main/java/org/zstack/header/vm/VmInstanceState.javaheader/src/main/java/org/zstack/header/vm/cdrom/APISetVmInstanceDefaultCdRomMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataEvent.javaheader/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataMsgDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageEvent.javaheader/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageMsgDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataEvent.javaheader/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataMsgDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataEvent.javaheader/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataMsgDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/MetadataImpact.javaheader/src/main/java/org/zstack/header/vm/metadata/ResourceMetadata.javaheader/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataOnPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataOnPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataReply.javaheader/src/main/java/org/zstack/header/vm/metadata/VmInstanceMetadataConstants.javaheader/src/main/java/org/zstack/header/vm/metadata/VmInstanceMetadataDTO.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataCategory.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyService.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyVO.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyVO_.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataErrors.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataFlushStateVO.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataFlushStateVO_.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataPathBuildExtensionPoint.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataPathReplacementExtensionPoint.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataResourcePersistExtensionPoint.javaheader/src/main/java/org/zstack/header/vm/metadata/VmUuidFromApiResolver.javaheader/src/main/java/org/zstack/header/vm/metadata/VolumeResourceMetadata.javaheader/src/main/java/org/zstack/header/volume/APIAttachDataVolumeToVmMsg.javaheader/src/main/java/org/zstack/header/volume/APIChangeVolumeStateMsg.javaheader/src/main/java/org/zstack/header/volume/APICreateVolumeSnapshotGroupMsg.javaheader/src/main/java/org/zstack/header/volume/APICreateVolumeSnapshotMsg.javaheader/src/main/java/org/zstack/header/volume/APIDeleteDataVolumeMsg.javaheader/src/main/java/org/zstack/header/volume/APIDetachDataVolumeFromVmMsg.javaheader/src/main/java/org/zstack/header/volume/APIExpungeDataVolumeMsg.javaheader/src/main/java/org/zstack/header/volume/APIFlattenVolumeMsg.javaheader/src/main/java/org/zstack/header/volume/APIRecoverDataVolumeMsg.javaheader/src/main/java/org/zstack/header/volume/APISyncVolumeSizeMsg.javaheader/src/main/java/org/zstack/header/volume/APIUndoSnapshotCreationMsg.javaheader/src/main/java/org/zstack/header/volume/APIUpdateVolumeMsg.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/APILocalStorageMigrateVolumeMsg.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageHypervisorBackend.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmBackend.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageSimulator.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageSimulatorConfig.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageVmMetadataExtension.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorage.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageBackend.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackendCommands.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsVmMetadataExtension.javaresourceconfig/src/main/java/org/zstack/resourceconfig/APIDeleteResourceConfigMsg.javaresourceconfig/src/main/java/org/zstack/resourceconfig/APIUpdateResourceConfigMsg.javasdk/src/main/java/SourceClassMap.javasdk/src/main/java/org/zstack/sdk/CleanupVmInstanceMetadataAction.javasdk/src/main/java/org/zstack/sdk/CleanupVmInstanceMetadataResult.javasdk/src/main/java/org/zstack/sdk/GetVmInstanceMetadataFromPrimaryStorageAction.javasdk/src/main/java/org/zstack/sdk/GetVmInstanceMetadataFromPrimaryStorageResult.javasdk/src/main/java/org/zstack/sdk/RegisterVmInstanceFromMetadataAction.javasdk/src/main/java/org/zstack/sdk/RegisterVmInstanceFromMetadataResult.javasdk/src/main/java/org/zstack/sdk/ScanVmInstanceMetadataFromPrimaryStorageAction.javasdk/src/main/java/org/zstack/sdk/ScanVmInstanceMetadataFromPrimaryStorageResult.javasdk/src/main/java/org/zstack/sdk/UpdateVmInstanceMetadataAction.javasdk/src/main/java/org/zstack/sdk/UpdateVmInstanceMetadataResult.javasdk/src/main/java/org/zstack/sdk/VmMetadataScanEntry.javastorage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.javatestlib/src/main/java/org/zstack/testlib/ApiHelper.groovytestlib/src/main/java/org/zstack/testlib/LocalStorageSpec.groovytestlib/src/main/java/org/zstack/testlib/NfsPrimaryStorageSpec.groovy
✅ Files skipped from review due to trivial changes (89)
- header/src/main/java/org/zstack/header/storage/primary/VmMetadataScanEntryDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/vm/APIDeleteVmConsolePasswordMsg.java
- header/src/main/java/org/zstack/header/vm/APISetVmHostnameMsg.java
- header/src/main/java/org/zstack/header/vm/APIDeleteVmSshKeyMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataCategory.java
- header/src/main/java/org/zstack/header/storage/primary/CleanupVmInstanceMetadataOnPrimaryStorageReply.java
- header/src/main/java/org/zstack/header/volume/APIChangeVolumeStateMsg.java
- header/src/main/java/org/zstack/header/storage/snapshot/group/APIUngroupVolumeSnapshotGroupMsg.java
- header/src/main/java/org/zstack/header/vm/APIChangeInstanceOfferingMsg.java
- header/src/main/java/org/zstack/header/tag/APICreateSystemTagMsg.java
- header/src/main/java/org/zstack/header/vm/APISetVmSshKeyMsg.java
- header/src/main/java/org/zstack/header/vm/APIMigrateVmMsg.java
- header/src/main/java/org/zstack/header/vm/APISetVmClockTrackMsg.java
- header/src/main/java/org/zstack/header/vm/APISetVmBootOrderMsg.java
- header/src/main/java/org/zstack/header/vm/APIChangeVmNicStateMsg.java
- header/src/main/java/org/zstack/header/vm/APISetVmStaticIpMsg.java
- header/src/main/java/org/zstack/header/vm/APISetVmConsolePasswordMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataOnPrimaryStorageReply.java
- header/src/main/java/org/zstack/header/vm/APIConvertTemplatedVmInstanceToVmInstanceMsg.java
- header/src/main/java/org/zstack/header/vm/APIUpdateVmPriorityMsg.java
- header/src/main/java/org/zstack/header/volume/APIExpungeDataVolumeMsg.java
- header/src/main/java/org/zstack/header/vm/APIUpdateVmNicDriverMsg.java
- header/src/main/java/org/zstack/header/tag/APICreateSystemTagsMsg.java
- header/src/main/java/org/zstack/header/storage/primary/GetVmInstanceMetadataFromPrimaryStorageReply.java
- header/src/main/java/org/zstack/header/storage/primary/RebaseVolumeBackingFileOnPrimaryStorageReply.java
- header/src/main/java/org/zstack/header/vm/APISetVmQxlMemoryMsg.java
- header/src/main/java/org/zstack/header/vm/APIDeleteVmBootModeMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataReply.java
- header/src/main/java/org/zstack/header/vm/APIDeleteVmHostnameMsg.java
- header/src/main/java/org/zstack/header/storage/primary/ReadVmInstanceMetadataMsg.java
- header/src/main/java/org/zstack/header/storage/primary/ReadVmInstanceMetadataReply.java
- resourceconfig/src/main/java/org/zstack/resourceconfig/APIUpdateResourceConfigMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataMsgDoc_zh_cn.groovy
- plugin/localstorage/src/main/java/org/zstack/storage/primary/local/APILocalStorageMigrateVolumeMsg.java
- sdk/src/main/java/org/zstack/sdk/GetVmInstanceMetadataFromPrimaryStorageResult.java
- header/src/main/java/org/zstack/header/volume/APIFlattenVolumeMsg.java
- header/src/main/java/org/zstack/header/vm/APIRecoverVmInstanceMsg.java
- header/src/main/java/org/zstack/header/vm/APISetVmBootModeMsg.java
- header/src/main/java/org/zstack/header/vm/APIDestroyVmInstanceMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataEventDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataEvent.java
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyService.java
- header/src/main/java/org/zstack/header/vm/metadata/VmUuidFromApiResolver.java
- header/src/main/java/org/zstack/header/volume/APIDeleteDataVolumeMsg.java
- header/src/main/java/org/zstack/header/vm/APIUpdateVmInstanceMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/ResourceMetadata.java
- header/src/main/java/org/zstack/header/vm/metadata/VolumeResourceMetadata.java
- sdk/src/main/java/org/zstack/sdk/CleanupVmInstanceMetadataResult.java
- header/src/main/java/org/zstack/header/volume/APIUndoSnapshotCreationMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/MetadataImpact.java
- header/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataMsgDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/vm/APISetVmSoundTypeMsg.java
- testlib/src/main/java/org/zstack/testlib/LocalStorageSpec.groovy
- header/src/main/java/org/zstack/header/volume/APIRecoverDataVolumeMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataEventDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageEvent.java
- sdk/src/main/java/org/zstack/sdk/UpdateVmInstanceMetadataResult.java
- header/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataMsgDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/storage/primary/ScanVmInstanceMetadataFromPrimaryStorageReply.java
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataPathBuildExtensionPoint.java
- header/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataEventDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/storage/primary/GetVmInstanceMetadataFromPrimaryStorageMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataMsg.java
- header/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageEvent.java
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataErrors.java
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataResourcePersistExtensionPoint.java
- header/src/main/java/org/zstack/header/storage/primary/RebaseVolumeBackingFileOnPrimaryStorageMsg.java
- header/src/main/java/org/zstack/header/storage/primary/VmMetadataScanEntry.java
- sdk/src/main/java/org/zstack/sdk/VmMetadataScanEntry.java
- header/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataEvent.java
- sdk/src/main/java/org/zstack/sdk/GetVmInstanceMetadataFromPrimaryStorageAction.java
- header/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageMsgDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/vm/metadata/VmInstanceMetadataDTO.java
- header/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyVO_.java
- sdk/src/main/java/org/zstack/sdk/ScanVmInstanceMetadataFromPrimaryStorageResult.java
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataFlushStateVO_.java
- header/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageEventDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/storage/primary/ScanVmInstanceMetadataFromPrimaryStorageMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageEventDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataPathReplacementExtensionPoint.java
- sdk/src/main/java/org/zstack/sdk/RegisterVmInstanceFromMetadataResult.java
- conf/db/zsv/V5.0.0__schema.sql
- header/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageMsgDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataOnPrimaryStorageMsg.java
- plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageSimulatorConfig.java
- header/src/main/java/org/zstack/header/storage/snapshot/APIRevertVolumeFromSnapshotMsg.java
- header/src/main/java/org/zstack/header/volume/APICreateVolumeSnapshotGroupMsg.java
- compute/src/main/java/org/zstack/compute/vm/VmGlobalConfig.java
🚧 Files skipped from review as they are similar to previous changes (38)
- header/src/main/java/org/zstack/header/vm/APIAttachL3NetworkToVmMsg.java
- header/src/main/java/org/zstack/header/storage/snapshot/APIShrinkVolumeSnapshotMsg.java
- header/src/main/java/org/zstack/header/vm/APISetVmBootVolumeMsg.java
- header/src/main/java/org/zstack/header/storage/snapshot/group/APIUpdateVolumeSnapshotGroupMsg.java
- header/src/main/java/org/zstack/header/vm/APIDetachIsoFromVmInstanceMsg.java
- header/src/main/java/org/zstack/header/vm/cdrom/APISetVmInstanceDefaultCdRomMsg.java
- header/src/main/java/org/zstack/header/vm/APIAttachIsoToVmInstanceMsg.java
- header/src/main/java/org/zstack/header/tag/APIUpdateSystemTagMsg.java
- header/src/main/java/org/zstack/header/vm/APIAttachVmNicToVmMsg.java
- header/src/main/java/org/zstack/header/vm/APIChangeVmNicNetworkMsg.java
- header/src/main/java/org/zstack/header/volume/APIUpdateVolumeMsg.java
- header/src/main/java/org/zstack/header/volume/APIAttachDataVolumeToVmMsg.java
- testlib/src/main/java/org/zstack/testlib/NfsPrimaryStorageSpec.groovy
- header/src/main/java/org/zstack/header/volume/APIDetachDataVolumeFromVmMsg.java
- resourceconfig/src/main/java/org/zstack/resourceconfig/APIDeleteResourceConfigMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataMsg.java
- header/src/main/java/org/zstack/header/volume/APICreateVolumeSnapshotMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataMsg.java
- sdk/src/main/java/SourceClassMap.java
- header/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageMsg.java
- header/src/main/java/org/zstack/header/vm/APIConvertVmInstanceToTemplatedVmInstanceMsg.java
- header/src/main/java/org/zstack/header/vm/APIReimageVmInstanceMsg.java
- compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java
- header/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataEvent.java
- header/src/main/java/org/zstack/header/storage/primary/CleanupVmInstanceMetadataOnPrimaryStorageMsg.java
- header/src/main/java/org/zstack/header/volume/APISyncVolumeSizeMsg.java
- plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageSimulator.java
- plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageHypervisorBackend.java
- compute/src/main/java/org/zstack/compute/vm/CleanupVmInstanceMetadataOnPrimaryStorageGC.java
- compute/src/main/java/org/zstack/compute/vm/VmSystemTags.java
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyVO.java
- sdk/src/main/java/org/zstack/sdk/UpdateVmInstanceMetadataAction.java
- compute/src/main/java/org/zstack/compute/vm/VmExpungeMetadataFlow.java
- plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackendCommands.java
- sdk/src/main/java/org/zstack/sdk/ScanVmInstanceMetadataFromPrimaryStorageAction.java
- plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.java
- plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmBackend.java
- plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsVmMetadataExtension.java
Comment on lines
+17
to
+25
| @TagResourceType(VmInstanceVO.class) | ||
| @RestRequest( | ||
| path = "/vm-instances/metadata/register", | ||
| method = HttpMethod.POST, | ||
| responseClass = APIRegisterVmInstanceFromMetadataEvent.class, | ||
| parameterName = "params" | ||
| ) | ||
| @DefaultTimeout(timeunit = TimeUnit.HOURS, value = 1) | ||
| public class APIRegisterVmInstanceFromMetadataMsg extends APIMessage { |
There was a problem hiding this comment.
补齐这组 metadata API 的 ACL/action 配置。
conf/roleDefinitions/roleDefinition.json:1-100 的主 role definition 里还没有 APIRegisterVmInstanceFromMetadataMsg、APICleanupVmInstanceMetadataMsg、APIScanVmInstanceMetadataFromPrimaryStorageMsg、APIGetVmInstanceMetadataFromPrimaryStorageMsg、APIUpdateVmInstanceMetadataMsg 的 action 条目。新增公共 API 但不把它们纳入 role definition，会让这批接口在 RBAC 环境下无法按现有角色模型授权/限制。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@header/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataMsg.java`
around lines 17 - 25, Add RBAC action entries for the new metadata APIs so they
are grantable under existing roles: update the main role definition JSON
(conf/roleDefinitions/roleDefinition.json) to include actions for
APIRegisterVmInstanceFromMetadataMsg, APICleanupVmInstanceMetadataMsg,
APIScanVmInstanceMetadataFromPrimaryStorageMsg,
APIGetVmInstanceMetadataFromPrimaryStorageMsg, and
APIUpdateVmInstanceMetadataMsg; ensure each entry uses the same action naming
scheme as other VM instance APIs and points to the VmInstance resource type so
the RBAC engine can authorize these new endpoints.
plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (6)
sdk/src/main/java/org/zstack/sdk/CleanupVmInstanceMetadataAction.java (1)
28-29:⚠️ Potential issue | 🟠 Major
vmUuids仍缺少批量上限且允许空字符串元素。当前配置会放行超大列表与空白 UUID 到后端清理链路，建议在 SDK 侧加上限并拒绝空白元素。
建议修复
-@Param(required = true, nonempty = true, nullElements = false, emptyString = true, noTrim = false) +@Param(required = true, nonempty = true, nullElements = false, emptyString = false, noTrim = false) public java.util.List vmUuids; +private static final int MAX_VM_UUIDS = 500; + +private void validateVmUuids() { + if (vmUuids != null && vmUuids.size() > MAX_VM_UUIDS) { + throw new ApiException(String.format("vmUuids size cannot exceed %d", MAX_VM_UUIDS)); + } +} @@ public Result call() { + validateVmUuids(); ApiResult res = ZSClient.call(this); return makeResult(res); } @@ public void call(final Completion<Result> completion) { + validateVmUuids(); ZSClient.call(this, new InternalCompletion() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/src/main/java/org/zstack/sdk/CleanupVmInstanceMetadataAction.java` around lines 28 - 29, The vmUuids field in CleanupVmInstanceMetadataAction currently allows empty-string UUIDs and has no batch-size limit; update the `@Param` on the vmUuids field in class CleanupVmInstanceMetadataAction to disallow empty strings (set emptyString = false) and add a sensible maximum list size (e.g., max = 100) to prevent oversized payloads, ensuring existing nonempty = true and nullElements = false remain; if your codebase uses a constant for limits, reference that constant instead of a magic number.header/src/main/java/org/zstack/header/vm/VmInstanceState.java (1)
205-207:⚠️ Potential issue | 🟠 Major请补齐
Registering的兜底迁移，避免状态机抛异常中断。当前只处理
stopped，Registering收到unknown/noState时会走未映射分支并抛CloudRuntimeException。建议补齐常见兜底事件映射。建议修复
Registering.transactions( - new Transaction(VmInstanceStateEvent.stopped, VmInstanceState.Stopped) + new Transaction(VmInstanceStateEvent.stopped, VmInstanceState.Stopped), + new Transaction(VmInstanceStateEvent.noState, VmInstanceState.NoState), + new Transaction(VmInstanceStateEvent.unknown, VmInstanceState.Unknown) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/vm/VmInstanceState.java` around lines 205 - 207, 在 Registering 的事务注册里补充对未映射兜底事件的处理，避免状态机抛出 CloudRuntimeException：在现有 Registering.transactions(...) 区块中新增 Transaction 条目，把 VmInstanceStateEvent.unknown 和 VmInstanceStateEvent.noState 映射到一个安全状态（例如 VmInstanceState.Stopped 或合适的默认状态），确保 Registering 对这两个事件有明确映射以防止未映射分支抛异常。compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java (1)
1366-1399:⚠️ Potential issue | 🟠 Major还需要校验
clusterUuid和zoneUuid的归属关系。现在只校验了主存储挂载到
clusterUuid、host 也在clusterUuid里，但没有约束这个 cluster 是否属于msg.getZoneUuid()。传入跨 Zone 的zoneUuid/clusterUuid组合仍然能通过拦截。建议修改
+ boolean clusterInZone = Q.New(ClusterVO.class) + .eq(ClusterVO_.uuid, msg.getClusterUuid()) + .eq(ClusterVO_.zoneUuid, msg.getZoneUuid()) + .isExists(); + if (!clusterInZone) { + throw new ApiMessageInterceptionException(argerr( + "cluster[uuid:%s] is not in zone[uuid:%s]", + msg.getClusterUuid(), msg.getZoneUuid())); + } + boolean psAttachedToCluster = Q.New(PrimaryStorageClusterRefVO.class) .eq(PrimaryStorageClusterRefVO_.primaryStorageUuid, psUuid) .eq(PrimaryStorageClusterRefVO_.clusterUuid, msg.getClusterUuid()) .isExists();同时需要添加导入：
import org.zstack.header.cluster.ClusterVO; import org.zstack.header.cluster.ClusterVO_;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java` around lines 1366 - 1399, Add a check that the cluster specified by msg.getClusterUuid() belongs to msg.getZoneUuid(): query ClusterVO (using ClusterVO and ClusterVO_) for ClusterVO_.uuid == msg.getClusterUuid() and ClusterVO_.zoneUuid == msg.getZoneUuid() and throw ApiMessageInterceptionException(argerr(...)) if not found; place this validation near the existing primary-storage/host checks (e.g., after psAttachedToCluster or before host availability logic) and add the suggested imports for org.zstack.header.cluster.ClusterVO and ClusterVO_.plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java (2)
3474-3481:⚠️ Potential issue | 🟠 Major断连主机会被静默漏扫。
Line 3474 只枚举
Connectedhost，Line 3529 只回传这些 host 里的失败者。已挂在这块 LocalStorage 上但当前断连的 host 既不在结果里，也不在failedHostUuids里，调用方会拿到“成功 + 不完整结果”。至少要把未扫描的断连 host 并入失败集合；如果结果不能保证完整，建议直接返回错误。Also applies to: 3524-3534
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java` around lines 3474 - 3481, The current query in LocalStorageBase that builds connectedHostUuids (using SQL.New with param psUuid from self.getUuid()) only selects HostStatus.Connected, so hosts attached to this LocalStorage but currently disconnected are omitted from the scan and from failedHostUuids; update the logic that collects connectedHostUuids and failedHostUuids (the variables around that SQL.New call and where failedHostUuids is returned) to also include attached-but-disconnected hosts as failures (e.g., query hosts with status != Available/appropriate set or add a second query for attached hosts regardless of status) or, if you cannot guarantee a complete scan, change the method to return an explicit error instead of a partial success so callers do not assume completeness.
3371-3378:⚠️ Potential issue | 🟠 MajorUpdate/Cleanup 仍然没有落到同一条串行队列。
Line 3372/3377 还是 PS 级 key + level 10，而 Line 3546/3551 已经是另一套 VM 级 key + level 1。这样同一 VM 的元数据刷新和 cleanup 仍会并发改同一个文件，出现回写或误删竞态；两边需要完全一致的按 VM 串行 key 和并发度。
🛠 建议修改
`@Override` public String getSyncSignature() { - return String.format("update-metadata-on-ps-%s", self.getUuid()); + return String.format("vm-metadata-on-ps-%s-vm-%s", self.getUuid(), msg.getVmInstanceUuid()); } `@Override` public int getSyncLevel() { - return 10; + return 1; }`@Override` public String getSyncSignature() { - return String.format("update-metadata-on-ps-%s-%s", self.getUuid(), msg.getVmInstanceUuid()); + return String.format("vm-metadata-on-ps-%s-vm-%s", self.getUuid(), msg.getVmInstanceUuid()); }Also applies to: 3545-3552
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java` around lines 3371 - 3378, The sync signature and level in the metadata update block (methods getSyncSignature() and getSyncLevel() in LocalStorageBase) use a primary-storage (PS) level key and concurrency 10, which differs from the cleanup code that uses a VM-level key and concurrency 1; update the metadata update block so its getSyncSignature() builds the same VM-scoped key used by the cleanup path (use the VM identifier used there) and set getSyncLevel() to the same concurrency value (1) so both update and cleanup serialize on the same VM queue and prevent races.plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorage.java (1)
1953-1962:⚠️ Potential issue | 🟠 Major不要把 NFS 元数据写入绑到可能已失联的 VM host。
getHostUuidFromVolume()对 Running/Paused VM 会直接返回vm.hostUuid；这里没有像下方Get/Cleanup路径那样再校验它是否仍在 connected host 集合里。对 NFS 这种共享存储，只要还有任一 connected host，就不该因为 VM 当前 host 短暂失联而把本可成功的更新直接做失败。🛠 建议修改
- 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); - chain.next(); - return; - } + List<HostInventory> connectedHosts = factory.getConnectedHostForOperation(getSelfInventory()); + String hostUuid = getHostUuidFromVolume(msg.getRootVolumeUuid()); + if (hostUuid == null || hostUuid.isEmpty() + || connectedHosts.stream().noneMatch(h -> h.getUuid().equals(hostUuid))) { + if (connectedHosts.isEmpty()) { + reply.setError(operr("no connected host found for NFS primary storage[uuid:%s]", self.getUuid())); + bus.reply(msg, reply); + chain.next(); + return; + } + hostUuid = connectedHosts.get(0).getUuid(); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorage.java` around lines 1953 - 1962, The code uses getHostUuidFromVolume() and immediately calls getBackendByHostUuid(hostUuid) and backend.handle(...), which can bind metadata writes to a VM's host that may be disconnected; instead, after obtaining hostUuid via getHostUuidFromVolume(), check whether that hostUuid is present in the connected host set (use the existing method that returns connected host UUIDs for the primary storage), and if it is not, choose another connected host (e.g., the first entry) and call getBackendByHostUuid(chosenConnectedHostUuid) before backend.handle(...); only return an error if there are no connected hosts for that primary storage.
🧹 Nitpick comments (6)
header/src/main/java/org/zstack/header/vm/metadata/VmUuidFromApiResolver.java (2)
6-7: 接口及方法缺少 Javadoc 注释。根据编码规范，接口方法必须配有有效的 Javadoc 注释。建议为
VmUuidFromApiResolver接口及其方法添加文档说明，描述其用途、参数含义及返回值。📝 建议添加 Javadoc
package org.zstack.header.vm.metadata; import java.util.ArrayList; import java.util.List; +/** + * Resolver interface for converting API field values to VM UUIDs. + */ public interface VmUuidFromApiResolver { + /** + * Resolves a single API field value to a VM UUID. + * + * `@param` fieldValue the API field value to resolve + * `@return` the resolved VM UUID, or null if resolution fails + */ String resolveVmUuid(String fieldValue);As per coding guidelines, "接口方法不应有多余的修饰符（例如
public），且必须配有有效的 Javadoc 注释。"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/vm/metadata/VmUuidFromApiResolver.java` around lines 6 - 7, Add proper Javadoc for the VmUuidFromApiResolver interface and its resolveVmUuid method describing the purpose, parameter (fieldValue) meaning and expected return value; also remove the redundant "public" modifier from the method declaration per guideline so the interface looks like "interface VmUuidFromApiResolver" with a documented "String resolveVmUuid(String fieldValue)" method.
9-24: 建议使用 Stream API 简化实现。当前实现使用传统 for 循环配合多个 null 检查。根据编码规范建议，可使用 Java Stream 表达式使代码更简洁。
♻️ 使用 Stream API 重构
+ /** + * Batch resolves multiple API field values to VM UUIDs. + * + * `@param` fieldValues the list of API field values to resolve + * `@return` a list of resolved VM UUIDs (excluding null results) + */ default List<String> batchResolveVmUuids(List<String> fieldValues) { - List<String> result = new ArrayList<>(); if (fieldValues == null || fieldValues.isEmpty()) { - return result; + return new ArrayList<>(); } - for (String v : fieldValues) { - if (v == null) { - continue; - } - String vmUuid = resolveVmUuid(v); - if (vmUuid != null) { - result.add(vmUuid); - } - } - return result; + return fieldValues.stream() + .filter(v -> v != null) + .map(this::resolveVmUuid) + .filter(uuid -> uuid != null) + .collect(java.util.stream.Collectors.toList()); }As per coding guidelines, "使用 Java Stream 或 Lambda 表达式代替冗长的循环与条件判断。"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/vm/metadata/VmUuidFromApiResolver.java` around lines 9 - 24, Replace the manual loop in batchResolveVmUuids with a Stream pipeline: start from Optional.ofNullable(fieldValues).orElseGet(Collections::emptyList).stream(), filter out null entries, map each value using resolveVmUuid, filter non-null results, and collect to a List; ensure the method still returns an empty list for null/empty input and preserves behavior of resolveVmUuid (call resolveVmUuid for each non-null value and only include non-null results).header/src/main/java/org/zstack/header/vm/metadata/VmMetadataFlushStateVO.java (2)
31-33: 建议添加显式的@Table(name = "...")注解。当前
@Table注解未指定表名，将依赖 JPA 默认映射。根据 PR 审查建议和项目惯例，应显式指定表名以避免依赖隐式行为。♻️ 建议修改
`@Entity` -@Table +@Table(name = "VmMetadataFlushStateVO") public class VmMetadataFlushStateVO {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/vm/metadata/VmMetadataFlushStateVO.java` around lines 31 - 33, 类 VmMetadataFlushStateVO 上的 `@Table` 注解未指定表名，导致依赖 JPA 默认映射；请在该类（VmMetadataFlushStateVO）将 `@Table` 替换为或补充为显式的 `@Table`(name = "<明确表名>")，使用项目约定的表名以消除隐式行为并保持一致性，确保名称与现有数据库/迁移脚本匹配并在必要时同步注释的实体类与表名。
67-69: 确认metadataSnapshot字段使用 LONGTEXT 的合理性。
@Lob注解在 MySQL 中通常映射为 LONGTEXT（最大 4GB）。根据 PR 讨论，应用层面的vm.metadata.payload.rejectThreshold默认值为 32MB。考虑到实际使用场景，MEDIUMTEXT（16MB）可能已足够，且能减少存储开销。如确需支持超过 16MB 的元数据快照，建议在代码注释或文档中说明选择 LONGTEXT 的理由。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/vm/metadata/VmMetadataFlushStateVO.java` around lines 67 - 69, 字段 metadataSnapshot 在 VmMetadataFlushStateVO 上使用 `@Lob`（MySQL 映射为 LONGTEXT），请确认是否应改为 MEDIUMTEXT 以节省存储：如果应用上限制（vm.metadata.payload.rejectThreshold，默认 32MB）不超过 16MB，请将映射改为 MEDIUMTEXT（例如通过在 metadataSnapshot 上使用 `@Column`(columnDefinition="MEDIUMTEXT") 或等效映射）；若确需支持 >16MB 的快照，请保留 LONGTEXT 并在 VmMetadataFlushStateVO 的类或字段注释中说明选择 LONGTEXT 的理由（包含关联的配置项 vm.metadata.payload.rejectThreshold 和其默认值）。header/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataMsg.java (1)
19-20: 建议为vmUuids添加maxLength约束。当前
vmUuids仅有nonempty = true约束，没有限制列表的最大长度。如 PR 评审中提到的，过大的列表可能导致系统负载过高。建议添加合理的上限（如 100 或 500）。建议修改
- `@APIParam`(resourceType = VmInstanceVO.class, nonempty = true) + `@APIParam`(resourceType = VmInstanceVO.class, nonempty = true, maxLength = 100) private List<String> vmUuids;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataMsg.java` around lines 19 - 20, The API parameter vmUuids in class APIUpdateVmInstanceMetadataMsg lacks a maxLength constraint which can allow unbounded lists and overload the system; update the `@APIParam` annotation on the vmUuids field to include a reasonable maxLength (e.g., 100 or 500) so the framework enforces an upper bound, leaving nonempty = true intact; locate the vmUuids field and its `@APIParam` usage in APIUpdateVmInstanceMetadataMsg and add the maxLength attribute to the annotation.plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageBackend.java (1)
96-104: 接口方法签名与主流模式不一致。新增的
handle(...)方法（第 96-104 行）不包含PrimaryStorageInventory参数，而线路 25-55 的既有handle方法遵循handle(PrimaryStorageInventory inv, MsgType msg, ...)的标准模式。虽然pullSnapshot和commitSnapshot方法（第 88、90 行）已采用类似的(MsgType msg, String hostUuid, ...)模式，但这仍然与接口中的主流设计不一致。建议明确这是否为有意的设计演进（例如，新增的消息类型已包含
primaryStorageUuid或其他标识信息），还是应统一所有handle方法的签名风格。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageBackend.java` around lines 96 - 104, 新增的 handle 方法（handle(UpdateVmInstanceMetadataOnPrimaryStorageMsg ...), handle(GetVmInstanceMetadataFromPrimaryStorageMsg ...), handle(ScanVmInstanceMetadataFromPrimaryStorageMsg ...), handle(CleanupVmInstanceMetadataOnPrimaryStorageMsg ...), handle(RebaseVolumeBackingFileOnPrimaryStorageMsg ...)）与现有主流签名不一致：缺失 PrimaryStorageInventory 参数；请统一签名风格，恢复为 handle(PrimaryStorageInventory inv, <MsgType> msg, String hostUuid, ReturnValueCompletion<...> completion) 或明确在注释/接口文档中说明这是有意变更并确保每个 Msg 类型包含并使用 primaryStorageUuid，随后更新所有实现类和调用方以匹配（调整方法声明、实现类方法签名与调用处）。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@compute/src/main/java/org/zstack/compute/vm/CleanupVmInstanceMetadataOnPrimaryStorageGC.java`:
- Around line 58-80: The requireHost branch currently only checks HostVO
existence; enhance CleanupVmInstanceMetadataOnPrimaryStorageGC so that when
ext.requireHostForCleanup() is true you also verify the host is still mapped to
the target primary storage before sending cleanup; obtain the extension via
pluginRgty.getExtensionFromMap(...) or use the PS helper to check the
host-primary-storage mapping (e.g. validate a PrimaryStorageHostRef or use a
helper on PrimaryStorageVO) and if the host is not mapped/correctly bound to
primaryStorageUuid then call completion.cancel() and return (mirror the existing
log style and messages used around hostUuid checks to explain cancellation).
Ensure the check runs where hostUuid != null && requireHost is evaluated so the
GC won’t keep retrying against a detached host.
In
`@plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorage.java`:
- Around line 1938-1947: The metadata update and the cleanup tasks are not
serialized together: the ChainTask submitted via thdf.chainSubmit (implementing
getSyncSignature and getSyncLevel) uses a PS-level signature and concurrency 10,
while the cleanup flow uses a different path and doesn't join the same ChainTask
queue, causing concurrent writes/deletes on the same NFS file; fix by changing
the sync signature in the update task (getSyncSignature in the ChainTask
submitted by thdf.chainSubmit) to a VM-scoped key (e.g., include vmInstanceUuid
or volumeUuid) and reduce getSyncLevel to 1, and ensure the cleanup code path
also submits its work with thdf.chainSubmit using the identical VM-scoped
syncSignature and getSyncLevel=1 so both update and cleanup serialize on the
same queue.
In
`@plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.java`:
- Around line 2065-2098: The handler method
handle(UpdateVmInstanceMetadataOnPrimaryStorageMsg...) sends
KVMHostAsyncHttpCallMsg directly and therefore drops any capacity info returned
by the agent; updatePrimaryStorageCapacity(...) is never called (unlike
asyncHttpCall()). Fix by invoking
updatePrimaryStorageCapacity(msg.getPrimaryStorageUuid(), rsp) after a
successful WriteVmMetadataRsp/CleanupVmMetadataRsp (i.e. in the success branch
before completion.success(...)), or refactor to reuse the existing
asyncHttpCall(...) helper so the NfsPrimaryStorageAgentResponse-derived capacity
is processed consistently.
---
Duplicate comments:
In `@compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java`:
- Around line 1366-1399: Add a check that the cluster specified by
msg.getClusterUuid() belongs to msg.getZoneUuid(): query ClusterVO (using
ClusterVO and ClusterVO_) for ClusterVO_.uuid == msg.getClusterUuid() and
ClusterVO_.zoneUuid == msg.getZoneUuid() and throw
ApiMessageInterceptionException(argerr(...)) if not found; place this validation
near the existing primary-storage/host checks (e.g., after psAttachedToCluster
or before host availability logic) and add the suggested imports for
org.zstack.header.cluster.ClusterVO and ClusterVO_.
In `@header/src/main/java/org/zstack/header/vm/VmInstanceState.java`:
- Around line 205-207: 在 Registering 的事务注册里补充对未映射兜底事件的处理，避免状态机抛出
CloudRuntimeException：在现有 Registering.transactions(...) 区块中新增 Transaction 条目，把
VmInstanceStateEvent.unknown 和 VmInstanceStateEvent.noState 映射到一个安全状态（例如
VmInstanceState.Stopped 或合适的默认状态），确保 Registering 对这两个事件有明确映射以防止未映射分支抛异常。
In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java`:
- Around line 3474-3481: The current query in LocalStorageBase that builds
connectedHostUuids (using SQL.New with param psUuid from self.getUuid()) only
selects HostStatus.Connected, so hosts attached to this LocalStorage but
currently disconnected are omitted from the scan and from failedHostUuids;
update the logic that collects connectedHostUuids and failedHostUuids (the
variables around that SQL.New call and where failedHostUuids is returned) to
also include attached-but-disconnected hosts as failures (e.g., query hosts with
status != Available/appropriate set or add a second query for attached hosts
regardless of status) or, if you cannot guarantee a complete scan, change the
method to return an explicit error instead of a partial success so callers do
not assume completeness.
- Around line 3371-3378: The sync signature and level in the metadata update
block (methods getSyncSignature() and getSyncLevel() in LocalStorageBase) use a
primary-storage (PS) level key and concurrency 10, which differs from the
cleanup code that uses a VM-level key and concurrency 1; update the metadata
update block so its getSyncSignature() builds the same VM-scoped key used by the
cleanup path (use the VM identifier used there) and set getSyncLevel() to the
same concurrency value (1) so both update and cleanup serialize on the same VM
queue and prevent races.
In
`@plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorage.java`:
- Around line 1953-1962: The code uses getHostUuidFromVolume() and immediately
calls getBackendByHostUuid(hostUuid) and backend.handle(...), which can bind
metadata writes to a VM's host that may be disconnected; instead, after
obtaining hostUuid via getHostUuidFromVolume(), check whether that hostUuid is
present in the connected host set (use the existing method that returns
connected host UUIDs for the primary storage), and if it is not, choose another
connected host (e.g., the first entry) and call
getBackendByHostUuid(chosenConnectedHostUuid) before backend.handle(...); only
return an error if there are no connected hosts for that primary storage.
In `@sdk/src/main/java/org/zstack/sdk/CleanupVmInstanceMetadataAction.java`:
- Around line 28-29: The vmUuids field in CleanupVmInstanceMetadataAction
currently allows empty-string UUIDs and has no batch-size limit; update the
`@Param` on the vmUuids field in class CleanupVmInstanceMetadataAction to disallow
empty strings (set emptyString = false) and add a sensible maximum list size
(e.g., max = 100) to prevent oversized payloads, ensuring existing nonempty =
true and nullElements = false remain; if your codebase uses a constant for
limits, reference that constant instead of a magic number.
---
Nitpick comments:
In
`@header/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataMsg.java`:
- Around line 19-20: The API parameter vmUuids in class
APIUpdateVmInstanceMetadataMsg lacks a maxLength constraint which can allow
unbounded lists and overload the system; update the `@APIParam` annotation on the
vmUuids field to include a reasonable maxLength (e.g., 100 or 500) so the
framework enforces an upper bound, leaving nonempty = true intact; locate the
vmUuids field and its `@APIParam` usage in APIUpdateVmInstanceMetadataMsg and add
the maxLength attribute to the annotation.
In
`@header/src/main/java/org/zstack/header/vm/metadata/VmMetadataFlushStateVO.java`:
- Around line 31-33: 类 VmMetadataFlushStateVO 上的 `@Table` 注解未指定表名，导致依赖 JPA
默认映射；请在该类（VmMetadataFlushStateVO）将 `@Table` 替换为或补充为显式的 `@Table`(name =
"<明确表名>")，使用项目约定的表名以消除隐式行为并保持一致性，确保名称与现有数据库/迁移脚本匹配并在必要时同步注释的实体类与表名。
- Around line 67-69: 字段 metadataSnapshot 在 VmMetadataFlushStateVO 上使用 `@Lob`（MySQL
映射为 LONGTEXT），请确认是否应改为 MEDIUMTEXT
以节省存储：如果应用上限制（vm.metadata.payload.rejectThreshold，默认 32MB）不超过 16MB，请将映射改为
MEDIUMTEXT（例如通过在 metadataSnapshot 上使用 `@Column`(columnDefinition="MEDIUMTEXT")
或等效映射）；若确需支持 >16MB 的快照，请保留 LONGTEXT 并在 VmMetadataFlushStateVO 的类或字段注释中说明选择
LONGTEXT 的理由（包含关联的配置项 vm.metadata.payload.rejectThreshold 和其默认值）。
In
`@header/src/main/java/org/zstack/header/vm/metadata/VmUuidFromApiResolver.java`:
- Around line 6-7: Add proper Javadoc for the VmUuidFromApiResolver interface
and its resolveVmUuid method describing the purpose, parameter (fieldValue)
meaning and expected return value; also remove the redundant "public" modifier
from the method declaration per guideline so the interface looks like "interface
VmUuidFromApiResolver" with a documented "String resolveVmUuid(String
fieldValue)" method.
- Around line 9-24: Replace the manual loop in batchResolveVmUuids with a Stream
pipeline: start from
Optional.ofNullable(fieldValues).orElseGet(Collections::emptyList).stream(),
filter out null entries, map each value using resolveVmUuid, filter non-null
results, and collect to a List; ensure the method still returns an empty list
for null/empty input and preserves behavior of resolveVmUuid (call resolveVmUuid
for each non-null value and only include non-null results).
In
`@plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageBackend.java`:
- Around line 96-104: 新增的 handle
方法（handle(UpdateVmInstanceMetadataOnPrimaryStorageMsg ...),
handle(GetVmInstanceMetadataFromPrimaryStorageMsg ...),
handle(ScanVmInstanceMetadataFromPrimaryStorageMsg ...),
handle(CleanupVmInstanceMetadataOnPrimaryStorageMsg ...),
handle(RebaseVolumeBackingFileOnPrimaryStorageMsg ...)）与现有主流签名不一致：缺失
PrimaryStorageInventory 参数；请统一签名风格，恢复为 handle(PrimaryStorageInventory inv,
<MsgType> msg, String hostUuid, ReturnValueCompletion<...> completion)
或明确在注释/接口文档中说明这是有意变更并确保每个 Msg 类型包含并使用
primaryStorageUuid，随后更新所有实现类和调用方以匹配（调整方法声明、实现类方法签名与调用处）。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 214eaf98-c475-4434-bd81-e2a3fde71d8f
⛔ Files ignored due to path filters (5)
conf/globalConfig/vm.xmlis excluded by!**/*.xmlconf/persistence.xmlis excluded by!**/*.xmlconf/serviceConfig/primaryStorage.xmlis excluded by!**/*.xmlconf/serviceConfig/vmInstance.xmlis excluded by!**/*.xmlconf/springConfigXml/VmInstanceManager.xmlis excluded by!**/*.xml
📒 Files selected for processing (146)
compute/src/main/java/org/zstack/compute/vm/CleanupVmInstanceMetadataOnPrimaryStorageGC.javacompute/src/main/java/org/zstack/compute/vm/VmExpungeMetadataFlow.javacompute/src/main/java/org/zstack/compute/vm/VmGlobalConfig.javacompute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.javacompute/src/main/java/org/zstack/compute/vm/VmSystemTags.javaconf/db/zsv/V5.0.0__schema.sqlheader/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageEvent.javaheader/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageMsgDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/storage/primary/CleanupVmInstanceMetadataOnPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/CleanupVmInstanceMetadataOnPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/storage/primary/GetVmInstanceMetadataFromPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/GetVmInstanceMetadataFromPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/storage/primary/ReadVmInstanceMetadataMsg.javaheader/src/main/java/org/zstack/header/storage/primary/ReadVmInstanceMetadataReply.javaheader/src/main/java/org/zstack/header/storage/primary/RebaseVolumeBackingFileOnPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/RebaseVolumeBackingFileOnPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/storage/primary/ScanVmInstanceMetadataFromPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/ScanVmInstanceMetadataFromPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/storage/primary/VmMetadataScanEntry.javaheader/src/main/java/org/zstack/header/storage/primary/VmMetadataScanEntryDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/storage/snapshot/APIDeleteVolumeSnapshotMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/APIRevertVolumeFromSnapshotMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/APIShrinkVolumeSnapshotMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/APIUpdateVolumeSnapshotMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/group/APIDeleteVolumeSnapshotGroupMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/group/APIRevertVmFromSnapshotGroupMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/group/APIUngroupVolumeSnapshotGroupMsg.javaheader/src/main/java/org/zstack/header/storage/snapshot/group/APIUpdateVolumeSnapshotGroupMsg.javaheader/src/main/java/org/zstack/header/tag/APICreateSystemTagMsg.javaheader/src/main/java/org/zstack/header/tag/APICreateSystemTagsMsg.javaheader/src/main/java/org/zstack/header/tag/APIDeleteTagMsg.javaheader/src/main/java/org/zstack/header/tag/APIUpdateSystemTagMsg.javaheader/src/main/java/org/zstack/header/vm/APIAttachIsoToVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIAttachL3NetworkToVmMsg.javaheader/src/main/java/org/zstack/header/vm/APIAttachVmNicToVmMsg.javaheader/src/main/java/org/zstack/header/vm/APIChangeInstanceOfferingMsg.javaheader/src/main/java/org/zstack/header/vm/APIChangeVmNicNetworkMsg.javaheader/src/main/java/org/zstack/header/vm/APIChangeVmNicStateMsg.javaheader/src/main/java/org/zstack/header/vm/APIConvertTemplatedVmInstanceToVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIConvertVmInstanceToTemplatedVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIDeleteVmBootModeMsg.javaheader/src/main/java/org/zstack/header/vm/APIDeleteVmConsolePasswordMsg.javaheader/src/main/java/org/zstack/header/vm/APIDeleteVmHostnameMsg.javaheader/src/main/java/org/zstack/header/vm/APIDeleteVmSshKeyMsg.javaheader/src/main/java/org/zstack/header/vm/APIDeleteVmStaticIpMsg.javaheader/src/main/java/org/zstack/header/vm/APIDestroyVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIDetachIsoFromVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIDetachL3NetworkFromVmMsg.javaheader/src/main/java/org/zstack/header/vm/APIMigrateVmMsg.javaheader/src/main/java/org/zstack/header/vm/APIRecoverVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIReimageVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmBootModeMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmBootOrderMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmBootVolumeMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmClockTrackMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmConsolePasswordMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmHostnameMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmQxlMemoryMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmSoundTypeMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmSshKeyMsg.javaheader/src/main/java/org/zstack/header/vm/APISetVmStaticIpMsg.javaheader/src/main/java/org/zstack/header/vm/APIUpdateVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/APIUpdateVmNicDriverMsg.javaheader/src/main/java/org/zstack/header/vm/APIUpdateVmPriorityMsg.javaheader/src/main/java/org/zstack/header/vm/VmInstanceState.javaheader/src/main/java/org/zstack/header/vm/cdrom/APISetVmInstanceDefaultCdRomMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataEvent.javaheader/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataMsgDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageEvent.javaheader/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageMsgDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataEvent.javaheader/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataMsgDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataEvent.javaheader/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataMsgDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/metadata/MetadataImpact.javaheader/src/main/java/org/zstack/header/vm/metadata/ResourceMetadata.javaheader/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataOnPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataOnPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataReply.javaheader/src/main/java/org/zstack/header/vm/metadata/VmInstanceMetadataConstants.javaheader/src/main/java/org/zstack/header/vm/metadata/VmInstanceMetadataDTO.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataCategory.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyService.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyVO.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyVO_.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataErrors.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataFlushStateVO.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataFlushStateVO_.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataPathBuildExtensionPoint.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataPathReplacementExtensionPoint.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataResourcePersistExtensionPoint.javaheader/src/main/java/org/zstack/header/vm/metadata/VmUuidFromApiResolver.javaheader/src/main/java/org/zstack/header/vm/metadata/VolumeResourceMetadata.javaheader/src/main/java/org/zstack/header/volume/APIAttachDataVolumeToVmMsg.javaheader/src/main/java/org/zstack/header/volume/APIChangeVolumeStateMsg.javaheader/src/main/java/org/zstack/header/volume/APICreateVolumeSnapshotGroupMsg.javaheader/src/main/java/org/zstack/header/volume/APICreateVolumeSnapshotMsg.javaheader/src/main/java/org/zstack/header/volume/APIDeleteDataVolumeMsg.javaheader/src/main/java/org/zstack/header/volume/APIDetachDataVolumeFromVmMsg.javaheader/src/main/java/org/zstack/header/volume/APIExpungeDataVolumeMsg.javaheader/src/main/java/org/zstack/header/volume/APIFlattenVolumeMsg.javaheader/src/main/java/org/zstack/header/volume/APIRecoverDataVolumeMsg.javaheader/src/main/java/org/zstack/header/volume/APISyncVolumeSizeMsg.javaheader/src/main/java/org/zstack/header/volume/APIUndoSnapshotCreationMsg.javaheader/src/main/java/org/zstack/header/volume/APIUpdateVolumeMsg.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/APILocalStorageMigrateVolumeMsg.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageHypervisorBackend.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmBackend.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageSimulator.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageSimulatorConfig.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageVmMetadataExtension.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorage.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageBackend.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackendCommands.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsVmMetadataExtension.javaresourceconfig/src/main/java/org/zstack/resourceconfig/APIDeleteResourceConfigMsg.javaresourceconfig/src/main/java/org/zstack/resourceconfig/APIUpdateResourceConfigMsg.javasdk/src/main/java/SourceClassMap.javasdk/src/main/java/org/zstack/sdk/CleanupVmInstanceMetadataAction.javasdk/src/main/java/org/zstack/sdk/CleanupVmInstanceMetadataResult.javasdk/src/main/java/org/zstack/sdk/GetVmInstanceMetadataFromPrimaryStorageAction.javasdk/src/main/java/org/zstack/sdk/GetVmInstanceMetadataFromPrimaryStorageResult.javasdk/src/main/java/org/zstack/sdk/RegisterVmInstanceFromMetadataAction.javasdk/src/main/java/org/zstack/sdk/RegisterVmInstanceFromMetadataResult.javasdk/src/main/java/org/zstack/sdk/ScanVmInstanceMetadataFromPrimaryStorageAction.javasdk/src/main/java/org/zstack/sdk/ScanVmInstanceMetadataFromPrimaryStorageResult.javasdk/src/main/java/org/zstack/sdk/UpdateVmInstanceMetadataAction.javasdk/src/main/java/org/zstack/sdk/UpdateVmInstanceMetadataResult.javasdk/src/main/java/org/zstack/sdk/VmMetadataScanEntry.javastorage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.javatestlib/src/main/java/org/zstack/testlib/ApiHelper.groovytestlib/src/main/java/org/zstack/testlib/LocalStorageSpec.groovytestlib/src/main/java/org/zstack/testlib/NfsPrimaryStorageSpec.groovy
✅ Files skipped from review due to trivial changes (83)
- header/src/main/java/org/zstack/header/storage/snapshot/group/APIDeleteVolumeSnapshotGroupMsg.java
- header/src/main/java/org/zstack/header/storage/primary/CleanupVmInstanceMetadataOnPrimaryStorageReply.java
- header/src/main/java/org/zstack/header/vm/APIUpdateVmPriorityMsg.java
- header/src/main/java/org/zstack/header/tag/APIDeleteTagMsg.java
- header/src/main/java/org/zstack/header/storage/primary/VmMetadataScanEntryDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataReply.java
- header/src/main/java/org/zstack/header/storage/primary/RebaseVolumeBackingFileOnPrimaryStorageReply.java
- sdk/src/main/java/org/zstack/sdk/RegisterVmInstanceFromMetadataResult.java
- sdk/src/main/java/org/zstack/sdk/UpdateVmInstanceMetadataResult.java
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataCategory.java
- header/src/main/java/org/zstack/header/storage/primary/ReadVmInstanceMetadataMsg.java
- header/src/main/java/org/zstack/header/storage/primary/ReadVmInstanceMetadataReply.java
- header/src/main/java/org/zstack/header/vm/APIDestroyVmInstanceMsg.java
- header/src/main/java/org/zstack/header/storage/snapshot/group/APIUngroupVolumeSnapshotGroupMsg.java
- header/src/main/java/org/zstack/header/vm/APIUpdateVmInstanceMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataOnPrimaryStorageReply.java
- header/src/main/java/org/zstack/header/vm/APIChangeInstanceOfferingMsg.java
- header/src/main/java/org/zstack/header/vm/APIDeleteVmConsolePasswordMsg.java
- resourceconfig/src/main/java/org/zstack/resourceconfig/APIUpdateResourceConfigMsg.java
- header/src/main/java/org/zstack/header/volume/APISyncVolumeSizeMsg.java
- testlib/src/main/java/org/zstack/testlib/LocalStorageSpec.groovy
- header/src/main/java/org/zstack/header/vm/APIDeleteVmBootModeMsg.java
- header/src/main/java/org/zstack/header/volume/APIFlattenVolumeMsg.java
- sdk/src/main/java/org/zstack/sdk/GetVmInstanceMetadataFromPrimaryStorageResult.java
- header/src/main/java/org/zstack/header/vm/APIChangeVmNicStateMsg.java
- header/src/main/java/org/zstack/header/vm/APIDeleteVmStaticIpMsg.java
- header/src/main/java/org/zstack/header/tag/APICreateSystemTagsMsg.java
- header/src/main/java/org/zstack/header/vm/APISetVmStaticIpMsg.java
- header/src/main/java/org/zstack/header/vm/APISetVmSshKeyMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataEvent.java
- header/src/main/java/org/zstack/header/vm/metadata/VolumeResourceMetadata.java
- header/src/main/java/org/zstack/header/storage/primary/ScanVmInstanceMetadataFromPrimaryStorageReply.java
- header/src/main/java/org/zstack/header/vm/metadata/ResourceMetadata.java
- header/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataEventDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataMsg.java
- header/src/main/java/org/zstack/header/vm/APIAttachIsoToVmInstanceMsg.java
- header/src/main/java/org/zstack/header/tag/APIUpdateSystemTagMsg.java
- header/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageMsg.java
- plugin/localstorage/src/main/java/org/zstack/storage/primary/local/APILocalStorageMigrateVolumeMsg.java
- header/src/main/java/org/zstack/header/vm/APISetVmBootOrderMsg.java
- header/src/main/java/org/zstack/header/storage/primary/CleanupVmInstanceMetadataOnPrimaryStorageMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataEventDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/vm/metadata/MetadataImpact.java
- header/src/main/java/org/zstack/header/vm/APISetVmConsolePasswordMsg.java
- header/src/main/java/org/zstack/header/vm/APISetVmClockTrackMsg.java
- header/src/main/java/org/zstack/header/volume/APIDeleteDataVolumeMsg.java
- compute/src/main/java/org/zstack/compute/vm/VmSystemTags.java
- header/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataEventDoc_zh_cn.groovy
- sdk/src/main/java/org/zstack/sdk/CleanupVmInstanceMetadataResult.java
- sdk/src/main/java/org/zstack/sdk/ScanVmInstanceMetadataFromPrimaryStorageResult.java
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyService.java
- header/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataMsgDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataMsgDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageEventDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataResourcePersistExtensionPoint.java
- header/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageMsgDoc_zh_cn.groovy
- plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageSimulatorConfig.java
- header/src/main/java/org/zstack/header/vm/APIDetachL3NetworkFromVmMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataPathBuildExtensionPoint.java
- header/src/main/java/org/zstack/header/vm/APIConvertTemplatedVmInstanceToVmInstanceMsg.java
- header/src/main/java/org/zstack/header/storage/primary/VmMetadataScanEntry.java
- header/src/main/java/org/zstack/header/volume/APIExpungeDataVolumeMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataEvent.java
- header/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataEvent.java
- header/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataPathReplacementExtensionPoint.java
- header/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageEventDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataMsgDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/vm/metadata/VmInstanceMetadataDTO.java
- plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageSimulator.java
- header/src/main/java/org/zstack/header/vm/metadata/APIGetVmInstanceMetadataFromPrimaryStorageEvent.java
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataFlushStateVO_.java
- header/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageMsgDoc_zh_cn.groovy
- header/src/main/java/org/zstack/header/vm/APISetVmBootModeMsg.java
- header/src/main/java/org/zstack/header/storage/primary/GetVmInstanceMetadataFromPrimaryStorageReply.java
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataErrors.java
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyVO.java
- header/src/main/java/org/zstack/header/vm/APISetVmHostnameMsg.java
- conf/db/zsv/V5.0.0__schema.sql
- header/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyVO_.java
- sdk/src/main/java/org/zstack/sdk/VmMetadataScanEntry.java
- header/src/main/java/org/zstack/header/vm/metadata/UpdateVmInstanceMetadataOnPrimaryStorageMsg.java
- compute/src/main/java/org/zstack/compute/vm/VmGlobalConfig.java
🚧 Files skipped from review as they are similar to previous changes (36)
- header/src/main/java/org/zstack/header/vm/APIReimageVmInstanceMsg.java
- header/src/main/java/org/zstack/header/vm/APISetVmBootVolumeMsg.java
- header/src/main/java/org/zstack/header/storage/snapshot/group/APIRevertVmFromSnapshotGroupMsg.java
- header/src/main/java/org/zstack/header/vm/APIAttachL3NetworkToVmMsg.java
- header/src/main/java/org/zstack/header/volume/APIChangeVolumeStateMsg.java
- header/src/main/java/org/zstack/header/vm/APIDeleteVmHostnameMsg.java
- header/src/main/java/org/zstack/header/storage/snapshot/APIUpdateVolumeSnapshotMsg.java
- header/src/main/java/org/zstack/header/vm/APIConvertVmInstanceToTemplatedVmInstanceMsg.java
- header/src/main/java/org/zstack/header/storage/snapshot/group/APIUpdateVolumeSnapshotGroupMsg.java
- header/src/main/java/org/zstack/header/storage/snapshot/APIShrinkVolumeSnapshotMsg.java
- header/src/main/java/org/zstack/header/vm/APIMigrateVmMsg.java
- header/src/main/java/org/zstack/header/volume/APIDetachDataVolumeFromVmMsg.java
- header/src/main/java/org/zstack/header/vm/APIAttachVmNicToVmMsg.java
- header/src/main/java/org/zstack/header/vm/APIUpdateVmNicDriverMsg.java
- header/src/main/java/org/zstack/header/vm/cdrom/APISetVmInstanceDefaultCdRomMsg.java
- header/src/main/java/org/zstack/header/volume/APIUndoSnapshotCreationMsg.java
- header/src/main/java/org/zstack/header/storage/primary/ScanVmInstanceMetadataFromPrimaryStorageMsg.java
- header/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataMsg.java
- header/src/main/java/org/zstack/header/storage/primary/GetVmInstanceMetadataFromPrimaryStorageMsg.java
- header/src/main/java/org/zstack/header/volume/APIUpdateVolumeMsg.java
- header/src/main/java/org/zstack/header/vm/APIRecoverVmInstanceMsg.java
- header/src/main/java/org/zstack/header/volume/APICreateVolumeSnapshotGroupMsg.java
- header/src/main/java/org/zstack/header/volume/APICreateVolumeSnapshotMsg.java
- resourceconfig/src/main/java/org/zstack/resourceconfig/APIDeleteResourceConfigMsg.java
- header/src/main/java/org/zstack/header/volume/APIAttachDataVolumeToVmMsg.java
- header/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageEvent.java
- header/src/main/java/org/zstack/header/vm/APISetVmQxlMemoryMsg.java
- testlib/src/main/java/org/zstack/testlib/NfsPrimaryStorageSpec.groovy
- sdk/src/main/java/SourceClassMap.java
- header/src/main/java/org/zstack/header/vm/metadata/VmInstanceMetadataConstants.java
- compute/src/main/java/org/zstack/compute/vm/VmExpungeMetadataFlow.java
- plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageHypervisorBackend.java
- plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsVmMetadataExtension.java
- plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageVmMetadataExtension.java
- header/src/main/java/org/zstack/header/volume/APIRecoverDataVolumeMsg.java
- plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmBackend.java
Comment on lines
+58
to
+80
| VmMetadataPathBuildExtensionPoint ext = pluginRgty.getExtensionFromMap(psType, VmMetadataPathBuildExtensionPoint.class); | ||
| boolean requireHost = ext != null && ext.requireHostForCleanup(); | ||
|
|
||
| if (hostUuid == null && requireHost) { | ||
| logger.debug(String.format("[MetadataCleanupGC] hostUuid is null and ps[uuid:%s, type:%s] " + | ||
| "requires host for cleanup, cancel gc for vm[uuid:%s]", | ||
| primaryStorageUuid, psType, vmUuid)); | ||
| completion.cancel(); | ||
| return; | ||
| } | ||
|
|
||
| if (hostUuid != null && !dbf.isExist(hostUuid, HostVO.class)) { | ||
| if (requireHost) { | ||
| logger.debug(String.format("[MetadataCleanupGC] host[uuid:%s] no longer exists " + | ||
| "and ps[uuid:%s, type:%s] requires host for cleanup, cancel gc for vm[uuid:%s]", | ||
| hostUuid, primaryStorageUuid, psType, vmUuid)); | ||
| completion.cancel(); | ||
| return; | ||
| } | ||
|
|
||
| logger.info(String.format("[MetadataCleanupGC] host[uuid:%s] no longer exists for vm[uuid:%s], " + | ||
| "clear hostUuid and let the primary storage backend pick an available host", hostUuid, vmUuid)); | ||
| hostUuid = null; |
There was a problem hiding this comment.
requireHost 路径还缺少“host 仍能访问该主存储”的校验。
当前唯一返回 requireHostForCleanup()==true 的实现是 LocalStorage，而 Line 69 这里只判断 HostVO 是否存在。LocalStorageBase 后续还会在 backend 解析时校验该 host 是否仍绑定这块 PS（Line 3163-3167、Line 3577-3583）；如果 host 还在库里但已经从这块 PS 脱离，这个 GC 会持续发送注定失败的 cleanup 并一直重试。这里最好通过扩展点或 PS helper 在发送前把“host 是否仍映射到目标 PS”也判掉，不满足时直接 cancel。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@compute/src/main/java/org/zstack/compute/vm/CleanupVmInstanceMetadataOnPrimaryStorageGC.java`
around lines 58 - 80, The requireHost branch currently only checks HostVO
existence; enhance CleanupVmInstanceMetadataOnPrimaryStorageGC so that when
ext.requireHostForCleanup() is true you also verify the host is still mapped to
the target primary storage before sending cleanup; obtain the extension via
pluginRgty.getExtensionFromMap(...) or use the PS helper to check the
host-primary-storage mapping (e.g. validate a PrimaryStorageHostRef or use a
helper on PrimaryStorageVO) and if the host is not mapped/correctly bound to
primaryStorageUuid then call completion.cancel() and return (mirror the existing
log style and messages used around hostUuid checks to explain cancellation).
Ensure the check runs where hostUuid != null && requireHost is evaluated so the
GC won’t keep retrying against a detached host.
Comment on lines
+1938
to
+1947
| thdf.chainSubmit(new ChainTask(msg) { | ||
| @Override | ||
| public String getSyncSignature() { | ||
| return String.format("update-metadata-on-ps-%s", self.getUuid()); | ||
| } | ||
|
|
||
| @Override | ||
| public int getSyncLevel() { | ||
| return 10; | ||
| } |
There was a problem hiding this comment.
更新和清理没有进入同一条串行队列。
Line 1941 只按 PS 做串行且并发度是 10，而 Line 2040 的 cleanup 路径完全没进同一条 ChainTask。这样同一 VM 的元数据刷新和 cleanup 会并发改同一个 NFS 文件，出现“删完又被旧快照写回”或“新 flush 被 cleanup 删掉”。建议把 cleanup 也放进同一个按 VM 维度的 syncSignature，并把并发度收敛到 1。
Also applies to: 2040-2066
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorage.java`
around lines 1938 - 1947, The metadata update and the cleanup tasks are not
serialized together: the ChainTask submitted via thdf.chainSubmit (implementing
getSyncSignature and getSyncLevel) uses a PS-level signature and concurrency 10,
while the cleanup flow uses a different path and doesn't join the same ChainTask
queue, causing concurrent writes/deletes on the same NFS file; fix by changing
the sync signature in the update task (getSyncSignature in the ChainTask
submitted by thdf.chainSubmit) to a VM-scoped key (e.g., include vmInstanceUuid
or volumeUuid) and reduce getSyncLevel to 1, and ensure the cleanup code path
also submits its work with thdf.chainSubmit using the identical VM-scoped
syncSignature and getSyncLevel=1 so both update and cleanup serialize on the
same queue.
Comment on lines
+2065
to
+2098
| public void handle(UpdateVmInstanceMetadataOnPrimaryStorageMsg msg, String hostUuid, ReturnValueCompletion<UpdateVmInstanceMetadataOnPrimaryStorageReply> completion) { | ||
| WriteVmMetadataCmd cmd = new WriteVmMetadataCmd(); | ||
| cmd.setUuid(msg.getPrimaryStorageUuid()); | ||
| cmd.metadata = msg.getMetadata(); | ||
| cmd.metadataPath = msg.getMetadataPath(); | ||
| cmd.vmUuid = msg.getVmInstanceUuid(); | ||
| cmd.vmName = msg.getVmInstanceName(); | ||
| cmd.vmCategory = msg.getVmCategory(); | ||
| cmd.architecture = msg.getArchitecture(); | ||
| cmd.schemaVersion = msg.getSchemaVersion(); | ||
|
|
||
| KVMHostAsyncHttpCallMsg hmsg = new KVMHostAsyncHttpCallMsg(); | ||
| hmsg.setCommand(cmd); | ||
| hmsg.setPath(WRITE_VM_METADATA_PATH); | ||
| hmsg.setHostUuid(hostUuid); | ||
| bus.makeTargetServiceIdByResourceUuid(hmsg, HostConstant.SERVICE_ID, hostUuid); | ||
| bus.send(hmsg, new CloudBusCallBack(completion) { | ||
| @Override | ||
| public void run(MessageReply reply) { | ||
| if (!reply.isSuccess()) { | ||
| completion.fail(reply.getError()); | ||
| return; | ||
| } | ||
|
|
||
| WriteVmMetadataRsp rsp = ((KVMHostAsyncHttpCallReply) reply).toResponse(WriteVmMetadataRsp.class); | ||
| if (!rsp.isSuccess()) { | ||
| completion.fail(operr("failed to write vm metadata on nfs via host[uuid:%s]: %s", hostUuid, rsp.getError())); | ||
| return; | ||
| } | ||
|
|
||
| completion.success(new UpdateVmInstanceMetadataOnPrimaryStorageReply()); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
写入/清理元数据后没有刷新主存容量。
这两个分支直接发 KVMHostAsyncHttpCallMsg，绕过了本类 asyncHttpCall() 里的 updatePrimaryStorageCapacity()。WriteVmMetadataRsp / CleanupVmMetadataRsp 本身继承了 NfsPrimaryStorageAgentResponse，如果 agent 回了最新容量，这里会被静默丢掉，容量会一直滞后到下一次别的操作或 ping。建议至少在成功分支补 updatePrimaryStorageCapacity(msg.getPrimaryStorageUuid(), rsp)，或者直接复用统一 helper。
♻️ 建议修改
WriteVmMetadataRsp rsp = ((KVMHostAsyncHttpCallReply) reply).toResponse(WriteVmMetadataRsp.class);
if (!rsp.isSuccess()) {
completion.fail(operr("failed to write vm metadata on nfs via host[uuid:%s]: %s", hostUuid, rsp.getError()));
return;
}
+ updatePrimaryStorageCapacity(msg.getPrimaryStorageUuid(), rsp);
completion.success(new UpdateVmInstanceMetadataOnPrimaryStorageReply());
@@
CleanupVmMetadataRsp rsp = ((KVMHostAsyncHttpCallReply) reply).toResponse(CleanupVmMetadataRsp.class);
if (!rsp.isSuccess()) {
completion.fail(operr("failed to cleanup vm metadata on nfs via host[uuid:%s]: %s", hostUuid, rsp.getError()));
return;
}
+ updatePrimaryStorageCapacity(msg.getPrimaryStorageUuid(), rsp);
CleanupVmInstanceMetadataOnPrimaryStorageReply r = new CleanupVmInstanceMetadataOnPrimaryStorageReply();Also applies to: 2169-2198
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.java`
around lines 2065 - 2098, The handler method
handle(UpdateVmInstanceMetadataOnPrimaryStorageMsg...) sends
KVMHostAsyncHttpCallMsg directly and therefore drops any capacity info returned
by the agent; updatePrimaryStorageCapacity(...) is never called (unlike
asyncHttpCall()). Fix by invoking
updatePrimaryStorageCapacity(msg.getPrimaryStorageUuid(), rsp) after a
successful WriteVmMetadataRsp/CleanupVmMetadataRsp (i.e. in the success branch
before completion.success(...)), or refactor to reuse the existing
asyncHttpCall(...) helper so the NfsPrimaryStorageAgentResponse-derived capacity
is processed consistently.
00998ce to
35a2367
Compare
APIImpact Resolves: ZSV-11559 Change-Id: I6b6a6378627264646d6a76726762736e77787373
MatheMatrix
force-pushed
the
sync/tao.gan/fb-reg-2-ZSV-11559@@3
branch
from
35a2367 to
9b595fd
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
APIImpact
Resolves: ZSV-11559
Change-Id: I6b6a6378627264646d6a76726762736e77787373
sync from gitlab !9513