<feature>[vm]: add vm metadata header definitions#3782
Closed
ZStack-Robot wants to merge 1 commit intozsv_5.0.0from
Closed
<feature>[vm]: add vm metadata header definitions#3782ZStack-Robot wants to merge 1 commit intozsv_5.0.0from
ZStack-Robot wants to merge 1 commit intozsv_5.0.0from
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添加完整的云主机元数据子系统：新的 REST API、消息/回复、注解、扩展点、数据库表与 JPA 实体、主存储（Local/NFS）后端协议与 agent 命令、相关 SDK/测试支持，以及用于写入/读取/扫描/清理/重基路径的实现与 GC/流程组件。 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as APIService
participant DB as Database
participant Plugin as PluginRegistry
participant PS as PrimaryStorageService
participant Host as HostAgent
Client->>API: POST /vm-instances/metadata/register (metadataPath, psUuid, ...)
API->>Plugin: resolve VmMetadataPathBuildExtensionPoint(psType)
Plugin-->>API: extension
API->>PS: GetVmInstanceMetadataFromPrimaryStorageMsg(psUuid, metadataPath[, hostUuid])
PS->>Host: HTTP GET_VM_INSTANCE_METADATA_PATH
Host-->>PS: GetVmInstanceMetadataRsp(metadata)
PS-->>API: GetVmInstanceMetadataFromPrimaryStorageReply(metadata)
API->>DB: create VM (state=Registering)
API->>PS: UpdateVmInstanceMetadataOnPrimaryStorageMsg(write metadata)
PS->>Host: HTTP WRITE_VM_METADATA_PATH
Host-->>PS: WriteVmMetadataRsp(success)
PS-->>API: UpdateVmInstanceMetadataOnPrimaryStorageReply(success)
API->>DB: update VM state -> Running
API-->>Client: APIRegisterVmInstanceFromMetadataEvent (inventory)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (11)
header/src/main/java/org/zstack/header/storage/primary/ReadVmInstanceMetadataReply.java (1)
6-14: 建议统一元数据字段命名以降低接口心智负担。Line 6 使用
vmMetadata，而同批新增回复中多处使用metadata。建议在同一元数据 API 家族内统一字段名，减少 SDK/序列化映射分歧。可选修改示例
public class ReadVmInstanceMetadataReply extends MessageReply { - private String vmMetadata; + private String metadata; - public String getVmMetadata() { - return vmMetadata; + public String getMetadata() { + return metadata; } - public void setVmMetadata(String vmMetadata) { - this.vmMetadata = vmMetadata; + public void setMetadata(String metadata) { + this.metadata = metadata; } }🤖 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/ReadVmInstanceMetadataReply.java` around lines 6 - 14, The field and accessors in ReadVmInstanceMetadataReply (vmMetadata, getVmMetadata, setVmMetadata) use a different name than other replies that use metadata; rename the field and its getters/setters to metadata (or vice versa to match the majority) so the DTO/serialization mapping is consistent, update all usages and imports that reference vmMetadata/getVmMetadata/setVmMetadata, and add/adjust any JSON/XML/serialization annotations (e.g., `@JsonProperty`) on the field or methods if renaming would break external contracts to preserve wire compatibility.header/src/main/java/org/zstack/header/vm/metadata/VmMetadataResourcePersistExtensionPoint.java (1)
7-15: 建议为扩展点接口方法补充 Javadoc。当前扩展点方法缺少契约说明，后续实现方难以统一理解参数语义与调用时机。
♻️ 建议修改
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); + /** + * 注册回滚后的清理回调，默认无操作。 + */ default void afterRegistrationRollback(List<String> resourceUuids) {} }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 - 15, 为接口 VmMetadataResourcePersistExtensionPoint 的每个方法补充规范 Javadoc，分别说明方法职责与调用时机以及各参数语义和返回值（getPrimaryStorageType、afterVolumePersist、afterSnapshotPersist、afterRegistrationRollback），并确保不添加不必要的修饰符；例如在 getPrimaryStorageType 说明返回值含义和格式，在 afterVolumePersist/afterSnapshotPersist 说明何时被调用、primaryStorageUuid/resourceUuid/resourceType/hostUuid 的含义、size 单位和 now 的时间语义，在 afterRegistrationRollback 说明何种情况会回滚及 resourceUuids 含义。确保注释简洁明确以作为实现约定。header/src/main/java/org/zstack/header/vm/metadata/VmMetadataPathReplacementExtensionPoint.java (1)
8-10: 建议补充路径替换扩展点方法的接口文档。该接口是跨存储插件契约，建议明确输入/输出约束，避免实现方语义不一致。
♻️ 建议修改
public interface VmMetadataPathReplacementExtensionPoint { + /** + * `@return` 支持该扩展的主存储类型 + */ String getPrimaryStorageType(); + + /** + * 计算元数据路径替换结果。 + * `@param` targetPsUuid 目标主存储 UUID + * `@param` allOldPaths 旧路径集合 + * `@return` 路径替换结果 + */ PathReplacementResult calculatePathReplacements(String targetPsUuid, List<String> allOldPaths);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/VmMetadataPathReplacementExtensionPoint.java` around lines 8 - 10, 为 VmMetadataPathReplacementExtensionPoint、方法 getPrimaryStorageType()、calculatePathReplacements(String targetPsUuid, List<String> allOldPaths) 和内部类 PathReplacementResult 添加完整的 Javadoc，明确每个方法的输入/输出约束（例如 targetPsUuid 非空性、allOldPaths 元素含义与允许的空/空列表语义）、返回值含义（PathReplacementResult 中各字段的语义、是否可为 null，以及是否表示“无替换”情形）、并指明线程/重入或异常行为；保持接口方法无多余修饰符并在文档中示例典型用例和失败场景以确保跨存储实现语义一致。testlib/src/main/java/org/zstack/testlib/LocalStorageSpec.groovy (1)
604-622: 建议让元数据相关模拟器具备最小状态一致性。当前 5 个新 endpoint 仅返回空响应，未消费请求参数，也未维护写/读/扫/清理之间的状态，测试很难覆盖真实链路。建议至少维护一个内存 map，并在
PREFIX_REBASE_BACKING_FILES_PATH回填rebasedCount。🤖 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 - 622, The five metadata simulators (LocalStorageKvmBackend.WRITE_VM_METADATA_PATH, GET_VM_INSTANCE_METADATA_PATH, SCAN_VM_METADATA_PATH, CLEANUP_VM_METADATA_PATH and PREFIX_REBASE_BACKING_FILES_PATH) currently return empty responses and do not share state; add a simple in-memory map (e.g. Map<String, Map<String,Object>> metadataStore) keyed by vmInstanceUuid to persist writes, have WRITE_VM_METADATA_PATH write incoming metadata into the map, GET_VM_INSTANCE_METADATA_PATH read from it, SCAN_VM_METADATA_PATH iterate/collect matching entries, CLEANUP_VM_METADATA_PATH remove entries, and make PREFIX_REBASE_BACKING_FILES_PATH compute and return a populated PrefixRebaseBackingFilesRsp.rebasedCount based on how many backing files in the map were rebased.header/src/main/java/org/zstack/header/vm/metadata/ResourceMetadata.java (1)
5-22: 建议将vo重命名为更具语义的字段名。
vo语义过于宽泛，后续维护时不利于快速理解数据内容。建议改为能体现载荷含义的名称（例如resourceVoContent/resourceVoJson等），并同步更新 getter/setter。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/ResourceMetadata.java` around lines 5 - 22, Rename the ambiguous field vo in class ResourceMetadata to a descriptive name (e.g., resourceVoContent or resourceVoJson) and update its getter and setter methods (getVo/setVo → getResourceVoContent/setResourceVoContent) accordingly; change the private field declaration (vo) and the accessor method names and signatures, then update all usages/references across the codebase to the new names (including any serialization, JSON property annotations, or reflection-based access) to ensure compatibility.header/src/main/java/org/zstack/header/storage/primary/VmMetadataScanEntryDoc_zh_cn.groovy (1)
7-66: 建议补充字段说明，避免空desc。当前多个字段描述为空，会降低 API 文档可读性。建议至少为关键字段（如
schemaVersion、metadataPath、incomplete）补充简短语义说明。🤖 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/VmMetadataScanEntryDoc_zh_cn.groovy` around lines 7 - 66, Several fields in VmMetadataScanEntryDoc_zh_cn.groovy (notably vmUuid, vmName, vmCategory, architecture, schemaVersion, metadataPath, hostUuid, sizeBytes, lastUpdateTime, incomplete) have empty desc values; update the field blocks for these names to include concise Chinese descriptions (e.g., schemaVersion → “元数据模式版本号”, metadataPath → “元数据在主存储上的路径”, incomplete → “指示元数据是否不完整（true/false）”, vmUuid → “虚拟机唯一标识符”) so API docs are readable and meaningful while keeping types and since unchanged.header/src/main/java/org/zstack/header/vm/metadata/VmMetadataPathBuildExtensionPoint.java (1)
3-11: 给扩展接口补上 Javadoc 契约。这是公开的 extension point，但
buildVmMetadataPath、buildMetadataDir、validateMetadataPath和requireHostForCleanup的语义/返回约定现在只能从实现里反推。建议直接在接口上写清楚 path 格式、validateMetadataPath的null/非 null约定，以及 cleanup 对hostUuid的要求，避免不同主存实现各自解释。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/VmMetadataPathBuildExtensionPoint.java` around lines 3 - 11, Add Javadoc to the VmMetadataPathBuildExtensionPoint interface and each method (getPrimaryStorageType, buildVmMetadataPath, buildMetadataDir, validateMetadataPath, requireHostForCleanup) that clearly states the contract: describe the expected path formats returned by buildVmMetadataPath and buildMetadataDir (examples/prefixes, whether paths are absolute or relative), specify validateMetadataPath return convention (null means valid, non-null returns a descriptive error message), document when and why requireHostForCleanup returns true (i.e., cleanup requires a hostUuid) and what callers should provide, and ensure no unnecessary modifiers are added to interface methods per coding guidelines.header/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyVO.java (1)
5-5: 移除未使用的导入。
VmInstanceVO导入未被使用，外键引用的是VmInstanceEO。♻️ 建议的修改
-import org.zstack.header.vm.VmInstanceVO;🤖 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/VmMetadataDirtyVO.java` at line 5, 当前 VmMetadataDirtyVO 中引入了未使用的类 VmInstanceVO；请删除该 import（import org.zstack.header.vm.VmInstanceVO;），并确认类中引用的外键使用的是 VmInstanceEO（若未导入则添加 import org.zstack.header.vm.VmInstanceEO;）；确保移除后无编译错误并且所有对 VmInstance 实体的引用均指向 VmInstanceEO。plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorage.java (1)
1985-2012: 建议提取重复的 host 选择逻辑
GetVmInstanceMetadataFromPrimaryStorageMsg和CleanupVmInstanceMetadataOnPrimaryStorageMsg的 handler 中有几乎相同的 host 选择逻辑（第 1994-1998 行和第 2048-2052 行）。可以考虑提取为私有辅助方法以减少重复代码。♻️ 建议的重构
private String selectHostUuid(List<HostInventory> connectedHosts, String preferredHostUuid) { if (preferredHostUuid == null || !connectedHosts.stream().anyMatch(h -> h.getUuid().equals(preferredHostUuid))) { return connectedHosts.get(0).getUuid(); } return preferredHostUuid; }然后在两个 handler 中可以简化为：
String hostUuid = selectHostUuid(connectedHosts, msg.getHostUuid());Also applies to: 2039-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 1985 - 2012, Extract the repeated host-selection logic into a private helper method (e.g., selectHostUuid) and use it in both handle(GetVmInstanceMetadataFromPrimaryStorageMsg msg) and handle(CleanupVmInstanceMetadataOnPrimaryStorageMsg msg); the helper should accept List<HostInventory> connectedHosts and a preferred hostUuid and return either the preferred hostUuid if it exists in connectedHosts or connectedHosts.get(0).getUuid() otherwise, then replace the existing inline selection (the code around getHostUuid()/connectedHosts.stream().anyMatch(...)) with a call like String hostUuid = selectHostUuid(connectedHosts, msg.getHostUuid()) before calling getBackendByHostUuid(...) and backend.handle(...).sdk/src/main/java/org/zstack/sdk/RegisterVmInstanceFromMetadataAction.java (1)
46-50: 建议为 List 类型添加泛型参数
systemTags和userTags字段使用了原始类型java.util.List，缺少泛型参数。虽然这可能是代码生成的结果，但使用泛型可以提供更好的类型安全性。♻️ 建议的改进
`@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/RegisterVmInstanceFromMetadataAction.java` around lines 46 - 50, The fields systemTags and userTags in RegisterVmInstanceFromMetadataAction use raw type java.util.List which loses type safety; change both declarations to a parameterized type (e.g., java.util.List<String> if tags are strings) and update any related getters/setters, constructors, or usages to the same generic type to ensure compile-time type safety; ensure imports remain correct and adjust any code that assumed raw List (casts or iterations) to use the typed element type.sdk/src/main/java/org/zstack/sdk/CleanupVmInstanceMetadataAction.java (1)
28-35: 建议使用泛型类型以提高类型安全性
vmUuids、systemTags和userTags字段使用了原始List类型，缺乏泛型约束。建议使用List<String>来提高类型安全性。不过，如果此类是从 API 定义自动生成的，则可以忽略此建议。
♻️ 建议的修改
- `@Param`(required = true, nonempty = true, nullElements = false, emptyString = true, noTrim = false) - public java.util.List vmUuids; + `@Param`(required = true, nonempty = true, nullElements = false, emptyString = true, noTrim = false) + public java.util.List<String> vmUuids; - `@Param`(required = false) - public java.util.List systemTags; + `@Param`(required = false) + public java.util.List<String> systemTags; - `@Param`(required = false) - public java.util.List userTags; + `@Param`(required = false) + 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/CleanupVmInstanceMetadataAction.java` around lines 28 - 35, Change the raw List fields in CleanupVmInstanceMetadataAction to use generics: replace vmUuids, systemTags, and userTags declarations with java.util.List<String> types (keep their existing `@Param` annotations and other modifiers), and add or ensure the appropriate java.util.List import if needed; if this class is auto-generated from API definitions, you may ignore this change.
🤖 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-59: 在 CleanupVmInstanceMetadataOnPrimaryStorageGC 中，当从
pluginRgty 获取 VmMetadataPathBuildExtensionPoint（变量名 ext）为 null 时应立即取消本次
GC，不要继续下发 cleanup 消息；也就是在使用 ext 或基于 requireHost 的逻辑之前加入 ext == null
的分支（直接结束/标记为无需处理并返回成功），确保对不支持 metadata 的主存储不再重复触发失败/重试路径（定位到
VmMetadataPathBuildExtensionPoint ext 及 boolean requireHost = ext != null &&
ext.requireHostForCleanup() 所在处修复）。
- Around line 61-72: The code currently cancels the GC when hostUuid is null for
PS types that require a host, which blocks backend fallback logic; update
CleanupVmInstanceMetadataOnPrimaryStorageGC so that when requireHost is true but
hostUuid is null you do NOT call completion.cancel() or return — instead keep
effectiveHostUuid as null (or leave it unset) and only log the debug message
about missing host (using primaryStorageUuid, psType, vmUuid) so the message can
proceed to the backend which can resolve the host from rootVolumeUuid (do this
by removing the completion.cancel() and return in the branch that checks
effectiveHostUuid == null and leaving effectiveHostUuid null for downstream
handling).
In `@compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java`:
- Around line 1333-1392: The validate(APIRegisterVmInstanceFromMetadataMsg msg)
method misses checking that the cluster belongs to the supplied zoneUuid; add an
explicit check that the ClusterVO with uuid msg.getClusterUuid() exists and
ClusterVO_.zoneUuid equals msg.getZoneUuid(), throwing
ApiMessageInterceptionException(argerr(...)) if not. Reuse that cluster-zone
result in the host validation branch (the HostVO queries that currently only
check clusterUuid should remain but rely on the confirmed cluster-zone binding),
and ensure the error messages reference zoneUuid/clusterUuid consistency when
failing.
In `@header/src/main/java/org/zstack/header/tag/APIDeleteTagMsg.java`:
- Line 16: APIDeleteTagMsg is annotated to use SystemTagUuidToVmUuidResolver but
its API path allows both SystemTagVO and UserTagVO; either make the resolver
generic or restrict the API. Fix by either (A) replacing
SystemTagUuidToVmUuidResolver with a generic resolver such as
TagUuidToVmUuidResolver in APIDeleteTagMsg so UUIDs for both SystemTagVO and
UserTagVO resolve correctly, or (B) restrict APIDeleteTagMsg to system-only tags
by adding `@APIParam`(resourceType = SystemTagVO.class) on the tag parameter to
match SystemTagUuidToVmUuidResolver (consistent with APIUpdateSystemTagMsg).
Ensure the chosen change updates the resolver reference or adds the APIParam
annotation on APIDeleteTagMsg accordingly.
In
`@header/src/main/java/org/zstack/header/vm/metadata/VmUuidFromApiResolver.java`:
- Line 7: Add Javadoc comments to the interface methods resolveVmUuid(String
fieldValue) and batchResolveVmUuids(List<String> fieldValues) in
VmUuidFromApiResolver to specify parameter semantics, exact return values and
types, null/empty input behavior, and exception guarantees; explicitly document
whether methods may return null or an empty collection, whether they throw on
unresolved values or return partial results, and any thread-safety or
performance expectations so callers know the contract. Ensure the Javadoc
includes `@param` and `@return` tags (and `@throws` if applicable) and remove any
unnecessary modifiers per interface style guidelines (do not add public/static)
while keeping method names resolveVmUuid and batchResolveVmUuids unchanged.
In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java`:
- Around line 3503-3551: The log computes successHosts incorrectly by
subtracting failedHosts.size() from connectedHostUuids.size() while failedHosts
already includes disconnectedHostUuids; change the calculation to compute
totalHosts = connectedHostUuids.size() + disconnectedHostUuids.size() (or use
connectedHostUuids.size() + disconnectedHostUuids.size() inline) and set
successHosts = totalHosts - failedHosts.size() when building the warning log
around failedHosts/ successHosts in LocalStorageBase (reference symbols:
failedHosts, connectedHostUuids, disconnectedHostUuids, the While completion
block that sets reply and logs the partial scan failure).
In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageSimulator.java`:
- Around line 375-382: In LocalStorageSimulator, the prefixRebaseBackingFiles
handler currently does not persist the incoming PrefixRebaseBackingFilesCmd to
the simulator config; modify prefixRebaseBackingFiles(HttpEntity<String> entity)
to deserialize the cmd (PrefixRebaseBackingFilesCmd) and store it into the
simulator's config/state the same way other metadata endpoints do (so tests can
assert the call and inspect inputs), then continue to build
PrefixRebaseBackingFilesRsp and reply; ensure you use the same simulator config
key/structure and naming convention used by other handlers in
LocalStorageSimulator for consistency.
In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageVmMetadataExtension.java`:
- Around line 151-168: The createResourceRef method currently checks existence
only by resourceUuid but LocalStorageResourceRefVO uses a composite key of
(resourceUuid, primaryStorageUuid, hostUuid), so update the existence check to
query by all three fields (resourceUuid, primaryStorageUuid, hostUuid) before
deciding to skip creation, and wrap the dbf.persist(ref) call in a try/catch to
handle unique-constraint/duplicate-key exceptions (catch the DB-specific runtime
exception, log a debug/info message including the three-key values and swallow
or return gracefully); reference the createResourceRef method,
LocalStorageResourceRefVO fields resourceUuid/primaryStorageUuid/hostUuid, and
the dbf.persist call and logger for locating where to change logic.
---
Nitpick comments:
In
`@header/src/main/java/org/zstack/header/storage/primary/ReadVmInstanceMetadataReply.java`:
- Around line 6-14: The field and accessors in ReadVmInstanceMetadataReply
(vmMetadata, getVmMetadata, setVmMetadata) use a different name than other
replies that use metadata; rename the field and its getters/setters to metadata
(or vice versa to match the majority) so the DTO/serialization mapping is
consistent, update all usages and imports that reference
vmMetadata/getVmMetadata/setVmMetadata, and add/adjust any
JSON/XML/serialization annotations (e.g., `@JsonProperty`) on the field or methods
if renaming would break external contracts to preserve wire compatibility.
In
`@header/src/main/java/org/zstack/header/storage/primary/VmMetadataScanEntryDoc_zh_cn.groovy`:
- Around line 7-66: Several fields in VmMetadataScanEntryDoc_zh_cn.groovy
(notably vmUuid, vmName, vmCategory, architecture, schemaVersion, metadataPath,
hostUuid, sizeBytes, lastUpdateTime, incomplete) have empty desc values; update
the field blocks for these names to include concise Chinese descriptions (e.g.,
schemaVersion → “元数据模式版本号”, metadataPath → “元数据在主存储上的路径”, incomplete →
“指示元数据是否不完整（true/false）”, vmUuid → “虚拟机唯一标识符”) so API docs are readable and
meaningful while keeping types and since unchanged.
In `@header/src/main/java/org/zstack/header/vm/metadata/ResourceMetadata.java`:
- Around line 5-22: Rename the ambiguous field vo in class ResourceMetadata to a
descriptive name (e.g., resourceVoContent or resourceVoJson) and update its
getter and setter methods (getVo/setVo →
getResourceVoContent/setResourceVoContent) accordingly; change the private field
declaration (vo) and the accessor method names and signatures, then update all
usages/references across the codebase to the new names (including any
serialization, JSON property annotations, or reflection-based access) to ensure
compatibility.
In `@header/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyVO.java`:
- Line 5: 当前 VmMetadataDirtyVO 中引入了未使用的类 VmInstanceVO；请删除该 import（import
org.zstack.header.vm.VmInstanceVO;），并确认类中引用的外键使用的是 VmInstanceEO（若未导入则添加 import
org.zstack.header.vm.VmInstanceEO;）；确保移除后无编译错误并且所有对 VmInstance 实体的引用均指向
VmInstanceEO。
In
`@header/src/main/java/org/zstack/header/vm/metadata/VmMetadataPathBuildExtensionPoint.java`:
- Around line 3-11: Add Javadoc to the VmMetadataPathBuildExtensionPoint
interface and each method (getPrimaryStorageType, buildVmMetadataPath,
buildMetadataDir, validateMetadataPath, requireHostForCleanup) that clearly
states the contract: describe the expected path formats returned by
buildVmMetadataPath and buildMetadataDir (examples/prefixes, whether paths are
absolute or relative), specify validateMetadataPath return convention (null
means valid, non-null returns a descriptive error message), document when and
why requireHostForCleanup returns true (i.e., cleanup requires a hostUuid) and
what callers should provide, and ensure no unnecessary modifiers are added to
interface methods per coding guidelines.
In
`@header/src/main/java/org/zstack/header/vm/metadata/VmMetadataPathReplacementExtensionPoint.java`:
- Around line 8-10: 为 VmMetadataPathReplacementExtensionPoint、方法
getPrimaryStorageType()、calculatePathReplacements(String targetPsUuid,
List<String> allOldPaths) 和内部类 PathReplacementResult 添加完整的
Javadoc，明确每个方法的输入/输出约束（例如 targetPsUuid 非空性、allOldPaths
元素含义与允许的空/空列表语义）、返回值含义（PathReplacementResult 中各字段的语义、是否可为
null，以及是否表示“无替换”情形）、并指明线程/重入或异常行为；保持接口方法无多余修饰符并在文档中示例典型用例和失败场景以确保跨存储实现语义一致。
In
`@header/src/main/java/org/zstack/header/vm/metadata/VmMetadataResourcePersistExtensionPoint.java`:
- Around line 7-15: 为接口 VmMetadataResourcePersistExtensionPoint 的每个方法补充规范
Javadoc，分别说明方法职责与调用时机以及各参数语义和返回值（getPrimaryStorageType、afterVolumePersist、afterSnapshotPersist、afterRegistrationRollback），并确保不添加不必要的修饰符；例如在
getPrimaryStorageType 说明返回值含义和格式，在 afterVolumePersist/afterSnapshotPersist
说明何时被调用、primaryStorageUuid/resourceUuid/resourceType/hostUuid 的含义、size 单位和 now
的时间语义，在 afterRegistrationRollback 说明何种情况会回滚及 resourceUuids 含义。确保注释简洁明确以作为实现约定。
In
`@plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorage.java`:
- Around line 1985-2012: Extract the repeated host-selection logic into a
private helper method (e.g., selectHostUuid) and use it in both
handle(GetVmInstanceMetadataFromPrimaryStorageMsg msg) and
handle(CleanupVmInstanceMetadataOnPrimaryStorageMsg msg); the helper should
accept List<HostInventory> connectedHosts and a preferred hostUuid and return
either the preferred hostUuid if it exists in connectedHosts or
connectedHosts.get(0).getUuid() otherwise, then replace the existing inline
selection (the code around getHostUuid()/connectedHosts.stream().anyMatch(...))
with a call like String hostUuid = selectHostUuid(connectedHosts,
msg.getHostUuid()) before calling getBackendByHostUuid(...) and
backend.handle(...).
In `@sdk/src/main/java/org/zstack/sdk/CleanupVmInstanceMetadataAction.java`:
- Around line 28-35: Change the raw List fields in
CleanupVmInstanceMetadataAction to use generics: replace vmUuids, systemTags,
and userTags declarations with java.util.List<String> types (keep their existing
`@Param` annotations and other modifiers), and add or ensure the appropriate
java.util.List import if needed; if this class is auto-generated from API
definitions, you may ignore this change.
In `@sdk/src/main/java/org/zstack/sdk/RegisterVmInstanceFromMetadataAction.java`:
- Around line 46-50: The fields systemTags and userTags in
RegisterVmInstanceFromMetadataAction use raw type java.util.List which loses
type safety; change both declarations to a parameterized type (e.g.,
java.util.List<String> if tags are strings) and update any related
getters/setters, constructors, or usages to the same generic type to ensure
compile-time type safety; ensure imports remain correct and adjust any code that
assumed raw List (casts or iterations) to use the typed element type.
In `@testlib/src/main/java/org/zstack/testlib/LocalStorageSpec.groovy`:
- Around line 604-622: The five metadata simulators
(LocalStorageKvmBackend.WRITE_VM_METADATA_PATH, GET_VM_INSTANCE_METADATA_PATH,
SCAN_VM_METADATA_PATH, CLEANUP_VM_METADATA_PATH and
PREFIX_REBASE_BACKING_FILES_PATH) currently return empty responses and do not
share state; add a simple in-memory map (e.g. Map<String, Map<String,Object>>
metadataStore) keyed by vmInstanceUuid to persist writes, have
WRITE_VM_METADATA_PATH write incoming metadata into the map,
GET_VM_INSTANCE_METADATA_PATH read from it, SCAN_VM_METADATA_PATH
iterate/collect matching entries, CLEANUP_VM_METADATA_PATH remove entries, and
make PREFIX_REBASE_BACKING_FILES_PATH compute and return a populated
PrefixRebaseBackingFilesRsp.rebasedCount based on how many backing files in the
map were rebased.
🪄 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: 8f42f398-ed9b-4c87-b730-da319eebc37d
⛔ 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 (145)
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/APIScanVmInstanceMetadataFromPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageMsgDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageReplyDoc_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/APICleanupVmInstanceMetadataEvent.javaheader/src/main/java/org/zstack/header/vm/APICleanupVmInstanceMetadataEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/APICleanupVmInstanceMetadataMsg.javaheader/src/main/java/org/zstack/header/vm/APICleanupVmInstanceMetadataMsgDoc_zh_cn.groovyheader/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/APIGetVmInstanceMetadataFromPrimaryStorageEvent.javaheader/src/main/java/org/zstack/header/vm/APIGetVmInstanceMetadataFromPrimaryStorageEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/APIGetVmInstanceMetadataFromPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/vm/APIGetVmInstanceMetadataFromPrimaryStorageMsgDoc_zh_cn.groovyheader/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/APIRegisterVmInstanceFromMetadataEvent.javaheader/src/main/java/org/zstack/header/vm/APIRegisterVmInstanceFromMetadataEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/APIRegisterVmInstanceFromMetadataMsg.javaheader/src/main/java/org/zstack/header/vm/APIRegisterVmInstanceFromMetadataMsgDoc_zh_cn.groovyheader/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/APIUpdateVmInstanceMetadataEvent.javaheader/src/main/java/org/zstack/header/vm/APIUpdateVmInstanceMetadataEventDoc_zh_cn.groovyheader/src/main/java/org/zstack/header/vm/APIUpdateVmInstanceMetadataMsg.javaheader/src/main/java/org/zstack/header/vm/APIUpdateVmInstanceMetadataMsgDoc_zh_cn.groovyheader/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/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/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
| import java.util.List; | ||
|
|
||
| public interface VmUuidFromApiResolver { | ||
| String resolveVmUuid(String fieldValue); |
There was a problem hiding this comment.
请为接口方法补充 Javadoc，明确契约语义。
resolveVmUuid 与 batchResolveVmUuids 当前缺少方法级说明，建议补充参数、返回值及 null 处理约定，避免调用方误用。
建议补丁
public interface VmUuidFromApiResolver {
+ /**
+ * 将 API 字段值解析为 VM UUID。
+ *
+ * `@param` fieldValue API 请求中的字段值
+ * `@return` 解析后的 VM UUID；无法解析时返回 null
+ */
String resolveVmUuid(String fieldValue);
+ /**
+ * 批量解析 VM UUID。
+ * <p>
+ * 会忽略 null 输入项和解析结果为 null 的项，返回仅包含有效 VM UUID 的列表。
+ *
+ * `@param` fieldValues API 请求中的字段值集合
+ * `@return` 有效 VM UUID 列表；当输入为 null 或空时返回空列表
+ */
default List<String> batchResolveVmUuids(List<String> fieldValues) {
List<String> result = new ArrayList<>();
if (fieldValues == null || fieldValues.isEmpty()) {
return result;
}As per coding guidelines, 接口方法不应有多余的修饰符（例如 public），且必须配有有效的 Javadoc 注释。
Also applies to: 9-24
🤖 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`
at line 7, Add Javadoc comments to the interface methods resolveVmUuid(String
fieldValue) and batchResolveVmUuids(List<String> fieldValues) in
VmUuidFromApiResolver to specify parameter semantics, exact return values and
types, null/empty input behavior, and exception guarantees; explicitly document
whether methods may return null or an empty collection, whether they throw on
unresolved values or return partial results, and any thread-safety or
performance expectations so callers know the contract. Ensure the Javadoc
includes `@param` and `@return` tags (and `@throws` if applicable) and remove any
unnecessary modifiers per interface style guidelines (do not add public/static)
while keeping method names resolveVmUuid and batchResolveVmUuids unchanged.
Comment on lines
+375
to
+382
| @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.
把 PrefixRebase 请求也记录到 simulator config。
这个分支是新增 endpoint 里唯一没有保存 cmd 的实现。这样测试侧就没法断言该 HTTP 调用是否真的发生，也拿不到入参内容，和前面几个 metadata 接口的 simulator 行为不一致。
🤖 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 - 382, In LocalStorageSimulator, the prefixRebaseBackingFiles
handler currently does not persist the incoming PrefixRebaseBackingFilesCmd to
the simulator config; modify prefixRebaseBackingFiles(HttpEntity<String> entity)
to deserialize the cmd (PrefixRebaseBackingFilesCmd) and store it into the
simulator's config/state the same way other metadata endpoints do (so tests can
assert the call and inspect inputs), then continue to build
PrefixRebaseBackingFilesRsp and reply; ensure you use the same simulator config
key/structure and naming convention used by other handlers in
LocalStorageSimulator for consistency.
Comment on lines
+151
to
+168
| 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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 搜索调用 afterVolumePersist 和 afterSnapshotPersist 的位置，检查是否有同步保护
rg -n -A5 'afterVolumePersist|afterSnapshotPersist' --type=javaRepository: MatheMatrix/zstack
Length of output: 3246
🏁 Script executed:
rg -n 'VmMetadataResourcePersistExtensionPoint|afterVolumePersist|afterSnapshotPersist' --type=java -B3 -A3 | head -100Repository: MatheMatrix/zstack
Length of output: 6160
🏁 Script executed:
# Find the implementation class that calls these extension points
rg -n 'getExtensionsByInterfaceNames|notifyExtensionList.*VmMetadataResourcePersistExtensionPoint' --type=java -B2 -A5Repository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
# Check the schema for LocalStorageResourceRefVO to see if there are unique constraints
find . -name '*LocalStorageResourceRefVO*' -o -name '*LocalStorageResourceRef*' | grep -E '\.(java|xml)$'Repository: MatheMatrix/zstack
Length of output: 934
🏁 Script executed:
cat plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageResourceRefVO.javaRepository: MatheMatrix/zstack
Length of output: 2732
🏁 Script executed:
# Search for where the extension point methods are called/invoked
rg -n 'afterVolumePersist|afterSnapshotPersist' --type=java -B5 -A2 | head -150Repository: MatheMatrix/zstack
Length of output: 3856
🏁 Script executed:
# Find implementations that call these extension point methods
rg -n 'getExtensionByInterfaceName|for.*VmMetadataResourcePersistExtensionPoint' --type=java -B3 -A10Repository: MatheMatrix/zstack
Length of output: 8627
🏁 Script executed:
rg -n 'VmMetadataResourcePersistExtensionPoint|afterVolumePersist|afterSnapshotPersist' --type=java -B10 -A10 | grep -E '(afterVolumePersist|afterSnapshotPersist|notifyExtension|for.*extension)' | head -50Repository: MatheMatrix/zstack
Length of output: 796
🏁 Script executed:
# Search for who calls the extension points - look for notify/invoke patterns
rg -n 'notifyExtension|invokeExtension|for.*VmMetadataResourcePersistExtensionPoint' --type=java -B5 -A10Repository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
# Check if CompositePrimaryKeyForLocalStorageResourceRefVO reveals the composite key constraint
cat plugin/localstorage/src/main/java/org/zstack/storage/primary/local/CompositePrimaryKeyForLocalStorageResourceRefVO.javaRepository: MatheMatrix/zstack
Length of output: 2084
🏁 Script executed:
# Verify the exact check-then-act pattern and look for exception handling
sed -n '151,168p' plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageVmMetadataExtension.javaRepository: MatheMatrix/zstack
Length of output: 940
🏁 Script executed:
# Check if there's any try-catch or exception handling around dbf.persist() calls
rg -n 'dbf\.persist|catch|throws' plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageVmMetadataExtension.java -B2 -A2Repository: MatheMatrix/zstack
Length of output: 477
🏁 Script executed:
# Look for similar patterns in the codebase to understand ZStack convention
rg -n 'isExists().*resourceUuid.*primaryStorage' --type=java -A5 -B2 | head -60Repository: MatheMatrix/zstack
Length of output: 44
存在性检查与复合主键不匹配，缺少异常处理
createResourceRef 方法的存在性检查仅验证 resourceUuid 一个字段（Line 153），但 LocalStorageResourceRefVO 的复合主键由 (resourceUuid, primaryStorageUuid, hostUuid) 三个字段组成。这导致检查逻辑不完整：
- 逻辑缺陷：若相同
resourceUuid的记录已存在但对应的primaryStorageUuid或hostUuid不同，检查会错误地跳过创建，即使该新组合应该被允许。 - 缺少异常处理：若两个并发调用传入完全相同的三元组，均会通过检查并尝试执行
dbf.persist(ref)（Line 167），数据库约束冲突会导致异常抛出而不被捕获。
建议改用完整的复合键检查，或添加异常处理来优雅地处理约束冲突（如捕获重复键异常并记录）。
🤖 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 - 168, The createResourceRef method currently checks existence
only by resourceUuid but LocalStorageResourceRefVO uses a composite key of
(resourceUuid, primaryStorageUuid, hostUuid), so update the existence check to
query by all three fields (resourceUuid, primaryStorageUuid, hostUuid) before
deciding to skip creation, and wrap the dbf.persist(ref) call in a try/catch to
handle unique-constraint/duplicate-key exceptions (catch the DB-specific runtime
exception, log a debug/info message including the three-key values and swallow
or return gracefully); reference the createResourceRef method,
LocalStorageResourceRefVO fields resourceUuid/primaryStorageUuid/hostUuid, and
the dbf.persist call and logger for locating where to change logic.
There was a problem hiding this comment.
🧩 Analysis chain
I'm unable to view the image in the comment. Let me check the actual key structure of LocalStorageResourceRefVO to understand what tao.gan likely pointed out.
🏁 Script executed:
#!/bin/bash
cat plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageResourceRefVO.java
echo "---"
cat plugin/localstorage/src/main/java/org/zstack/storage/primary/local/CompositePrimaryKeyForLocalStorageResourceRefVO.java
Length of output: 4697
@MatheMatrix, I'm unable to view the image that tao.gan attached. However, after inspecting the actual code, I can confirm that the original concern remains valid:
LocalStorageResourceRefVO is explicitly annotated with @IdClass(CompositePrimaryKeyForLocalStorageResourceRefVO.class), and all three fields — resourceUuid, primaryStorageUuid, and hostUuid — are each annotated with @Id, forming a true composite primary key. CompositePrimaryKeyForLocalStorageResourceRefVO also implements equals/hashCode using all three fields.
The current existence check:
boolean exists = Q.New(LocalStorageResourceRefVO.class)
.eq(LocalStorageResourceRefVO_.resourceUuid, resourceUuid)
.isExists();only filters by resourceUuid. A correct check should include all three key parts:
boolean exists = Q.New(LocalStorageResourceRefVO.class)
.eq(LocalStorageResourceRefVO_.resourceUuid, resourceUuid)
.eq(LocalStorageResourceRefVO_.primaryStorageUuid, primaryStorageUuid)
.eq(LocalStorageResourceRefVO_.hostUuid, hostUuid)
.isExists();Could you share what tao.gan's image was explaining? If there is a business reason why resourceUuid is always unique in practice for this specific call site (e.g., a volume/snapshot UUID is always bound to exactly one host and one primary storage at persist time), that would be worth recording as a learning. Otherwise, the composite-key check fix and adding exception handling around dbf.persist() for concurrent duplicates still seem warranted.
🧠 Learnings used
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 3737
File: compute/src/main/java/org/zstack/compute/vm/devices/VmHostFileCascadeExtension.java:77-113
Timestamp: 2026-04-14T05:45:06.368Z
Learning: In ZStack's VmHostFileCascadeExtension (compute/src/main/java/org/zstack/compute/vm/devices/VmHostFileCascadeExtension.java), deleting ALL VmHostFileVO records for a VM during cascade deletion is correct behavior. VmHostFileVO.vmInstanceUuid has an ON DELETE CASCADE foreign key to VmInstanceEO, so VmHostFileVO records cannot exist independently of the VM's EO record. The 90-day "latest file" retention policy in VmHostFileTracker applies only while VmInstanceEO is in the soft-deleted state — there is no way to preserve a VmHostFileVO record beyond the VM's EO lifecycle. Suggesting to skip the latest record in the cascade extension is incorrect.
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 3665
File: plugin/kvm/src/main/java/org/zstack/kvm/vmfiles/VmHostFileTracker.java:341-347
Timestamp: 2026-04-02T07:45:31.705Z
Learning: In ZStack's VmHostFileTracker (plugin/kvm/src/main/java/org/zstack/kvm/vmfiles/VmHostFileTracker.java), TPM and NvRam host files only require the single latest copy to be retained. For deleted VMs, non-latest VmHostFileVO records have no practical value and can be deleted immediately upon a successful KVM delete command, regardless of their age (no 7-day retention grace period is needed for non-latest files). Only the latest file needs to be retained for up to 90 days.
Learnt from: zstack-robot-2
Repo: MatheMatrix/zstack PR: 2435
File: storage/src/main/java/org/zstack/storage/snapshot/group/VolumeSnapshotGroupBase.java:47-47
Timestamp: 2025-08-14T06:56:19.585Z
Learning: 在VolumeSnapshotGroupBase.java中，VmInstanceResourceMetadataManager的注入和SKIP_RESOURCE_ROLLBACK标记虽然在当前版本中未被使用，但这些导入在大型重构PR中是有意为之的，用于保持代码一致性或为后续功能做准备。
Learnt from: MatheMatrix
Repo: MatheMatrix/zstack PR: 2504
File: plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmBackend.java:1010-1010
Timestamp: 2025-08-25T03:53:30.749Z
Learning: The OFFLINE_COMMIT_PATH ("/localstorage/snapshot/offlinecommit") for local storage snapshot commit operations is properly handled on both the ZStack management node side (in LocalStorageKvmBackend.java) and the KVM Agent side, providing complete end-to-end offline snapshot commit functionality.
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 0
File: :0-0
Timestamp: 2026-04-02T01:40:40.916Z
Learning: 在 ZStack 中，资源创建成功路径（如 `handle(APIAddSdnControllerMsg)` 中）对 `tagMgr.createTagsFromAPICreateMessage(msg, vo.getUuid(), SdnControllerVO.class.getSimpleName())` 和 `dbf.findByUuid(vo.getUuid(), SdnControllerVO.class)` 等后续操作，不需要用 try/catch 来防御"资源不存在"异常。ZStack 的设计哲学是：在资源创建流程中，通过生命周期管理和数据库外键约束来保证一致性，一般假设此类情况不会发生；若需要解决框架层面的一致性问题，应通过声明式 API 等机制统一处理，而不是在每个操作中无休止地判断异常。(plugin/sdnController/src/main/java/org/zstack/sdnController/SdnControllerManagerImpl.java)
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 2504
File: storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotTreeBase.java:775-819
Timestamp: 2025-08-25T03:52:37.301Z
Learning: 在ZStack项目的VolumeSnapshotTreeBase类中，当通过dbf.findByUuid()方法获取VolumeVO对象时，需要进行null检查，因为该方法可能在找不到对应记录时返回null，直接使用可能导致NullPointerException。
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 3716
File: header/src/main/java/org/zstack/header/keyprovider/EncryptedResourceKeyManager.java:32-35
Timestamp: 2026-04-12T05:42:20.685Z
Learning: In ZStack's KvmTpmExtensions.preReleaseVmResource (plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.java), as of PR `#3716`, the `ResourceKeyResult.refExistedBeforeCreate` field has been removed from `EncryptedResourceKeyManager.ResourceKeyResult`. Rollback behavior is now determined solely by `ResourceKeyResult.isCreatedNewKey()`: when true, the implementation deletes the materialized key-tool secret and removes the `EncryptedResourceKeyRef` row; when false, rollback is a no-op. The previously hardcoded `result.setRefExistedBeforeCreate(true)` in `preReleaseVmResource` has also been removed as part of this redesign.
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 3307
File: conf/db/zsv/V5.0.0__schema.sql:38-76
Timestamp: 2026-02-10T08:53:52.891Z
Learning: 在 ZStack 项目中,对于 joined-table inheritance 模式(如 KVMHostVO→HostEO, ApplianceVmVO→VmInstanceEO),子表的 uuid 字段不添加到父表的外键约束。ZStack 依赖 Hibernate/JPA 在应用层管理继承关系和级联操作,外键约束仅在数据库层直接删除时才有效,因此不使用。
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 0
File: :0-0
Timestamp: 2025-08-03T04:10:21.683Z
Learning: ZStack 数据库升级脚本 V4.10.16__schema.sql 中的 UPGRADE_VM_METADATA_TABLES_DIRECT 存储过程使用直接 RENAME TABLE 操作，不具备幂等性。需要通过 information_schema 检查表和约束的存在性来确保脚本可以安全重复执行。
Learnt from: MatheMatrix
Repo: MatheMatrix/zstack PR: 3742
File: plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.java:214-233
Timestamp: 2026-04-14T10:03:17.395Z
Learning: In KvmTpmExtensions.preInstantiateVmResource (plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.java), when context.providerUuid is blank and tpmSpec.getKeyProviderUuid() is also blank, it is intentional to NOT fail early or attempt by-name rebind at that point. The flow must continue to downstream steps (e.g. get-secret-on-host-first) to first check whether cached data is available before deciding to fail. Early failure here would break the NKP restore scenario.
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 2489
File: storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotTreeBase.java:179-179
Timestamp: 2025-08-22T06:31:57.406Z
Learning: 在ZStack项目的VolumeSnapshotAO类中，团队决定不为treeUuid字段添加Index注解，即使这可能会影响查询性能。团队优先考虑代码稳定性而非性能优化。
Learnt from: MatheMatrix
Repo: MatheMatrix/zstack PR: 2410
File: compute/src/main/java/org/zstack/compute/vm/VmInstanceHelper.java:334-356
Timestamp: 2025-08-10T13:42:01.027Z
Learning: 在 ZStack 的 `VmInstanceHelper.validateVmNicParams` 方法中，当 vmNicParams 非空但 l3NetworkUuids 为空时的报错文案 "l3NetworkUuids and vmNicInventories mustn't both be empty or both be set" 需要保持不变，以维持与现有 i18n 的一致性，即使文案中提到的是 vmNicInventories 而非 vmNicParams。
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 3782
File: header/src/main/java/org/zstack/header/tag/APIDeleteTagMsg.java:16-16
Timestamp: 2026-04-17T07:29:41.559Z
Learning: In ZStack, APIDeleteTagMsg (/tags/{uuid}) only handles system tags (SystemTagVO); UserTagVO has been removed/is no longer retained. Therefore, using SystemTagUuidToVmUuidResolver as the MetadataImpact resolver for APIDeleteTagMsg is correct and there is no need to add a generic tag resolver or restrict with APIParam(resourceType = SystemTagVO.class). (header/src/main/java/org/zstack/header/tag/APIDeleteTagMsg.java)
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 3563
File: plugin/kvm/src/main/java/org/zstack/kvm/efi/KvmSecureBootManager.java:328-334
Timestamp: 2026-03-23T09:33:01.215Z
Learning: In ZStack message handlers, ensure your `handleMessage` method is annotated with `MessageSafe` (`org.zstack.core.cloudbus.MessageSafe`). This enables `MessageSafeAspect.aj` to automatically convert exceptions thrown by the handler into appropriate reply errors. Note that `ExceptionSafeAspect.aj` only applies to methods annotated with `ExceptionSafe` and will only log/swallow exceptions without sending a reply—so missing `MessageSafe` can break reply/error semantics for the message bus.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
compute/src/main/java/org/zstack/compute/vm/CleanupVmInstanceMetadataOnPrimaryStorageGC.java (1)
73-87:⚠️ Potential issue | 🟠 Major
hostUuid为空但rootVolumeUuid存在时，仍会被提前取消，回退路径被挡住。在 Line 80，
effectiveHostUuid == null的情况下仍执行dbf.isExist(effectiveHostUuid, HostVO.class)，会导致进入取消分支，消息无法下发到后端通过rootVolumeUuid解析 host。该行为与本 PR 的回退目标冲突。建议修改
if (!requireHost) { effectiveHostUuid = null; } else { if (effectiveHostUuid == null && rootVolumeUuid == null) { logger.debug(String.format("[MetadataCleanupGC] hostUuid and rootVolumeUuid are both null, " + "ps[uuid:%s, type:%s] requires host for cleanup, cancel gc for vm[uuid:%s]", primaryStorageUuid, psType, vmUuid)); completion.cancel(); return; } - if (!dbf.isExist(effectiveHostUuid, HostVO.class)) { + if (effectiveHostUuid != null && !dbf.isExist(effectiveHostUuid, HostVO.class)) { logger.debug(String.format("[MetadataCleanupGC] host[uuid:%s] no longer exists " + "and ps[uuid:%s, type:%s] requires host for cleanup, " + "metadata is unreachable, cancel gc for vm[uuid:%s]", effectiveHostUuid, primaryStorageUuid, psType, vmUuid)); completion.cancel(); return; } }🤖 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 73 - 87, The code cancels GC when effectiveHostUuid is null even though rootVolumeUuid may allow a fallback; update the conditional logic in CleanupVmInstanceMetadataOnPrimaryStorageGC so that you only call dbf.isExist(effectiveHostUuid, HostVO.class) and potentially cancel via completion.cancel() when effectiveHostUuid is non-null, and if effectiveHostUuid is null but rootVolumeUuid is non-null, skip the cancel branch and allow the fallback path that resolves host from rootVolumeUuid to proceed (adjust the if checks around effectiveHostUuid, rootVolumeUuid, dbf.isExist, and completion.cancel accordingly).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@compute/src/main/java/org/zstack/compute/vm/CleanupVmInstanceMetadataOnPrimaryStorageGC.java`:
- Around line 73-87: The code cancels GC when effectiveHostUuid is null even
though rootVolumeUuid may allow a fallback; update the conditional logic in
CleanupVmInstanceMetadataOnPrimaryStorageGC so that you only call
dbf.isExist(effectiveHostUuid, HostVO.class) and potentially cancel via
completion.cancel() when effectiveHostUuid is non-null, and if effectiveHostUuid
is null but rootVolumeUuid is non-null, skip the cancel branch and allow the
fallback path that resolves host from rootVolumeUuid to proceed (adjust the if
checks around effectiveHostUuid, rootVolumeUuid, dbf.isExist, and
completion.cancel accordingly).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 86a9bf7b-2f58-4706-a866-39f06240dacc
📒 Files selected for processing (3)
compute/src/main/java/org/zstack/compute/vm/CleanupVmInstanceMetadataOnPrimaryStorageGC.javacompute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java
✅ Files skipped from review due to trivial changes (1)
- plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java
🚧 Files skipped from review as they are similar to previous changes (1)
- compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java
MatheMatrix
force-pushed
the
sync/tao.gan/vm_metadata-ZSV-11559@@3
branch
from
09a8976 to
bf1c5a6
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 80-87: The host-existence check in
CleanupVmInstanceMetadataOnPrimaryStorageGC incorrectly calls
dbf.isExist(effectiveHostUuid, HostVO.class) when effectiveHostUuid is null but
rootVolumeUuid is present, which can falsely cancel GC; change the logic so that
if requireHost is true and effectiveHostUuid is null but rootVolumeUuid != null
you first resolve the host UUID from rootVolumeUuid (or derive effectiveHostUuid
from the root volume) before calling dbf.isExist, and only call
completion.cancel() when no host can be resolved; update the condition around
effectiveHostUuid, rootVolumeUuid, dbf.isExist(...) and completion.cancel()
accordingly to avoid cancelling when a host can be inferred from rootVolumeUuid.
🪄 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: 7acd22e9-ec40-4f5d-9d35-bab99f132a5f
📒 Files selected for processing (3)
compute/src/main/java/org/zstack/compute/vm/CleanupVmInstanceMetadataOnPrimaryStorageGC.javacompute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java
🚧 Files skipped from review as they are similar to previous changes (1)
- compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java
Comment on lines
+80
to
+87
| if (!dbf.isExist(effectiveHostUuid, HostVO.class)) { | ||
| logger.debug(String.format("[MetadataCleanupGC] host[uuid:%s] no longer exists " + | ||
| "and ps[uuid:%s, type:%s] requires host for cleanup, " + | ||
| "metadata is unreachable, cancel gc for vm[uuid:%s]", | ||
| effectiveHostUuid, primaryStorageUuid, psType, vmUuid)); | ||
| completion.cancel(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
当 effectiveHostUuid 为 null 但 rootVolumeUuid 不为 null 时，主机存在性检查逻辑有误。
根据 Lines 73-79 的逻辑，当 requireHost 为 true 且 effectiveHostUuid == null 但 rootVolumeUuid != null 时，代码会继续执行到 Line 80。此时 dbf.isExist(null, HostVO.class) 会被调用，可能返回 false 导致 GC 被错误取消，而实际上后端可以通过 rootVolumeUuid 推导 host。
建议修改
- if (!dbf.isExist(effectiveHostUuid, HostVO.class)) {
+ if (effectiveHostUuid != null && !dbf.isExist(effectiveHostUuid, HostVO.class)) {
logger.debug(String.format("[MetadataCleanupGC] host[uuid:%s] no longer exists " +
"and ps[uuid:%s, type:%s] requires host for cleanup, " +
"metadata is unreachable, cancel gc for vm[uuid:%s]",
effectiveHostUuid, primaryStorageUuid, psType, vmUuid));
completion.cancel();
return;
}🤖 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 80 - 87, The host-existence check in
CleanupVmInstanceMetadataOnPrimaryStorageGC incorrectly calls
dbf.isExist(effectiveHostUuid, HostVO.class) when effectiveHostUuid is null but
rootVolumeUuid is present, which can falsely cancel GC; change the logic so that
if requireHost is true and effectiveHostUuid is null but rootVolumeUuid != null
you first resolve the host UUID from rootVolumeUuid (or derive effectiveHostUuid
from the root volume) before calling dbf.isExist, and only call
completion.cancel() when no host can be resolved; update the condition around
effectiveHostUuid, rootVolumeUuid, dbf.isExist(...) and completion.cancel()
accordingly to avoid cancelling when a host can be inferred from rootVolumeUuid.
24c1536 to
8ad9325
Compare
Resolves: ZSV-11559 Change-Id: I3cd9fa26b6af7bd47787ab88c5050683a3356059
Collaborator
|
Comment from gitlab: 自上次添加REVIEWED标签(2026-04-17 22:02:32.000Z)后, 有新的COMMIT更新(2026-04-18 15:21:36.274Z), 所以移除了REVIEWED标签 |
MatheMatrix
force-pushed
the
sync/tao.gan/vm_metadata-ZSV-11559@@3
branch
from
8ad9325 to
22f2626
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
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves: ZSV-11559
Change-Id: I3cd9fa26b6af7bd47787ab88c5050683a3356059
sync from gitlab !9651