<fix>[vm]: add MetadataImpact#3634
Open
zstack-robot-2 wants to merge 1 commit intofeature-zsv-5.0.0-vm-registrationfrom
Open
<fix>[vm]: add MetadataImpact#3634zstack-robot-2 wants to merge 1 commit intofeature-zsv-5.0.0-vm-registrationfrom
zstack-robot-2 wants to merge 1 commit intofeature-zsv-5.0.0-vm-registrationfrom
Conversation
APIImpact Resolves: ZSV-11559 Change-Id: I6b6a6378627264646d6a76726762736e77787373
简述此次变更为虚拟机系统引入了元数据管理框架，包括数据库表定义、API接口、垃圾回收机制、业务流程、存储适配实现及SDK支持，涵盖元数据的注册、清理、扫描、更新等全生命周期操作。 变更总览
预估代码审查工作量🎯 4 (复杂) | ⏱️ ~60 分钟 诗意赞颂
✨ Finishing Touches📝 Generate docstrings
🧪 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: 5
🧹 Nitpick comments (17)
header/src/main/java/org/zstack/header/vm/metadata/VmUuidFromApiResolver.java (1)
9-24: 可考虑使用 Stream 简化批量处理逻辑根据编码规范建议使用 Java Stream 或 Lambda 表达式代替冗长的循环与条件判断。当前实现正确，但可以更简洁。
♻️ 使用 Stream 的简化版本
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()); }🤖 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 method uses an explicit loop and null checks that can be simplified with Java Streams: replace the for-loop in batchResolveVmUuids with a stream pipeline on fieldValues that filters out nulls, maps each value via resolveVmUuid, filters out null results, and collects to a List<String>; ensure you still return an empty list when fieldValues is null or empty and keep the method signature and resolveVmUuid call unchanged.header/src/main/java/org/zstack/header/vm/metadata/ResourceMetadata.java (1)
5-5: 字段命名vo含义不够明确根据编码规范，应避免使用含糊的缩写。
vo可能表示 "Value Object" 或其他含义，建议使用更具描述性的名称，如valueObject或根据实际业务含义命名（例如voJson、voSerialized等）。♻️ 建议的命名改进
- private String vo; + private String voJson; // 或其他更具描述性的名称🤖 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` at line 5, The field name vo in class ResourceMetadata is ambiguous; rename it to a descriptive identifier (e.g., valueObject, voJson, or voSerialized depending on its actual content) and update all related symbols: the private field, its getter/setter methods (e.g., getVo()/setVo() -> getValueObject()/setValueObject()), any constructors, builders, equals/hashCode, toString, serialization annotations, and usages across the codebase to match the new name to avoid breaking references; also update persistence/JSON mapping annotations or column names if present so external contracts remain consistent.header/src/main/java/org/zstack/header/storage/snapshot/APIShrinkVolumeSnapshotMsg.java (1)
24-24: 考虑添加updateOnFailure = true以保持一致性。
APIDeleteVolumeSnapshotMsg和APIRevertVolumeFromSnapshotMsg都设置了updateOnFailure = true，但此处的APIShrinkVolumeSnapshotMsg未设置。如果 shrink 操作失败时也需要更新元数据状态，建议添加该属性以保持 snapshot 相关 API 的行为一致。建议的修改
-@MetadataImpact(value = MetadataImpact.Impact.STORAGE, resolver = "SnapshotUuidToVmUuidResolver", field = "uuid") +@MetadataImpact(value = MetadataImpact.Impact.STORAGE, resolver = "SnapshotUuidToVmUuidResolver", field = "uuid", 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/storage/snapshot/APIShrinkVolumeSnapshotMsg.java` at line 24, Add updateOnFailure = true to the `@MetadataImpact` annotation on APIShrinkVolumeSnapshotMsg so its metadata update behavior matches APIDeleteVolumeSnapshotMsg and APIRevertVolumeFromSnapshotMsg; locate the `@MetadataImpact` on class APIShrinkVolumeSnapshotMsg and include the updateOnFailure = true attribute (i.e., `@MetadataImpact`(value = MetadataImpact.Impact.STORAGE, resolver = "SnapshotUuidToVmUuidResolver", field = "uuid", updateOnFailure = true)).testlib/src/main/java/org/zstack/testlib/NfsPrimaryStorageSpec.groovy (1)
525-539: 建议为元数据模拟接口增加最小状态行为。目前四个接口都返回固定成功响应，难以覆盖元数据“写-读-扫-清理”的链路一致性校验。建议后续补一个轻量内存态或 VFS hook，提升回归检测有效性。
🤖 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/NfsPrimaryStorageSpec.groovy` around lines 525 - 539, The four simulator handlers for NfsPrimaryStorageKVMBackend (WRITE_VM_METADATA_PATH, GET_VM_INSTANCE_METADATA_PATH, SCAN_VM_METADATA_PATH, CLEANUP_VM_METADATA_PATH) currently always return static success responses which prevents testing the write-read-scan-cleanup metadata lifecycle; implement a minimal in-memory metadata store (or VFS hook) inside the test harness and update the simulator blocks to mutate/read that store: have the WRITE handler record metadata keyed by vm/instance id, GET return stored metadata or not-found, SCAN enumerate stored entries, and CLEANUP remove entries so the full chain can be validated during tests.header/src/main/java/org/zstack/header/vm/metadata/VmMetadataErrors.java (1)
18-22: 建议将code设为不可变字段。该值仅在构造时赋值，改为
final更安全也更符合枚举常量语义。可选修改
- private String code; + private final String code;🤖 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, 将 VmMetadataErrors 中的可变字段 code 改为不可变，修改字段声明为 private final String code，并在构造器 VmMetadataErrors(int id) 中按原有逻辑赋值（code = String.format("VM_METADATA.%s", id)）；确保没有其它地方尝试重新赋值该字段（若有，改为使用新的常量或方法返回值替代）。sdk/src/main/java/org/zstack/sdk/ScanVmInstanceMetadataFromPrimaryStorageResult.java (1)
6-11: 建议审查 SDK 代码生成器，为 Result 类字段添加泛型支持。该 Result 类中
vmInstanceMetadata字段使用了原始类型java.util.List，而对应的源类（ScanVmInstanceMetadataFromPrimaryStorageReply）已明确使用了List<VmMetadataScanEntry>。这是 SDK 代码生成器的一致问题——整个生成的 Result 类均采用原始类型，导致编译期类型检查和 IDE 自动完成功能的缺失。建议修复生成器以保留泛型信息，具体改为
java.util.List<VmMetadataScanEntry>。🤖 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 generated Result class ScanVmInstanceMetadataFromPrimaryStorageResult uses a raw java.util.List for the field vmInstanceMetadata and its accessor methods; change the field and the signatures of getVmInstanceMetadata and setVmInstanceMetadata to use the generic type java.util.List<VmMetadataScanEntry> (and add/import VmMetadataScanEntry as needed) so callers get compile-time type safety and IDE completion; update the SDK code generator that emits this class to preserve generic type parameters from the source reply (ScanVmInstanceMetadataFromPrimaryStorageReply -> List<VmMetadataScanEntry>) when emitting fields and getter/setter method signatures.compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java (1)
37-37: 移除未使用的导入
VmInstanceMetadataConstants在此文件中未被使用，应该删除该导入语句。♻️ 建议的修改
-import org.zstack.header.vm.metadata.VmInstanceMetadataConstants;🤖 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` at line 37, Remove the unused import of VmInstanceMetadataConstants from VmInstanceApiInterceptor: locate the import statement "import org.zstack.header.vm.metadata.VmInstanceMetadataConstants;" and delete it so there are no unused imports remaining in the VmInstanceApiInterceptor class.header/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataMsg.java (2)
17-19: 建议在@APIParam中添加checkExistence = true验证与
APIUpdateVmInstanceMetadataMsg类似，建议添加checkExistence = true以在 API 层面验证 VM UUID 的存在性。♻️ 建议的修改
- `@APIParam`(resourceType = VmInstanceVO.class, nonempty = true) + `@APIParam`(resourceType = VmInstanceVO.class, nonempty = true, checkExistence = true) 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/APICleanupVmInstanceMetadataMsg.java` around lines 17 - 19, The API parameter vmUuids in APICleanupVmInstanceMetadataMsg lacks existence validation; update the `@APIParam` on the vmUuids field in the APICleanupVmInstanceMetadataMsg class to include checkExistence = true so the API layer verifies VM UUIDs exist (match the behavior used in APIUpdateVmInstanceMetadataMsg) by adjusting the `@APIParam` annotation on the private List<String> vmUuids field.
29-33: 建议简化__example__()中的导入引用可以在文件顶部添加
import java.util.Arrays;然后直接使用Arrays.asList()，或者使用已导入的java.util.List配合List.of()（Java 9+）。♻️ 建议的修改
在文件顶部添加导入：
import java.util.Arrays;然后修改
__example__()方法：public static APICleanupVmInstanceMetadataMsg __example__() { APICleanupVmInstanceMetadataMsg msg = new APICleanupVmInstanceMetadataMsg(); - msg.vmUuids = java.util.Arrays.asList(uuid(), uuid()); + msg.vmUuids = Arrays.asList(uuid(), uuid()); return msg; }🤖 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/APICleanupVmInstanceMetadataMsg.java` around lines 29 - 33, The __example__ method in APICleanupVmInstanceMetadataMsg uses fully qualified java.util.Arrays.asList; simplify by adding an import (import java.util.Arrays;) at the top and replace java.util.Arrays.asList(...) with Arrays.asList(...), or alternatively import java.util.List and use List.of(...) to construct vmUuids more concisely; update the APICleanupVmInstanceMetadataMsg.__example__ method to assign vmUuids using the chosen simplified factory method.header/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataMsg.java (1)
18-20: 建议在@APIParam中添加checkExistence = true验证当前
vmUuids参数仅设置了nonempty = true，但未验证 UUID 是否实际存在于数据库中。建议添加checkExistence = true以确保在 API 层面提前校验 VM 实例的存在性。♻️ 建议的修改
- `@APIParam`(resourceType = VmInstanceVO.class, nonempty = true) + `@APIParam`(resourceType = VmInstanceVO.class, nonempty = true, checkExistence = true) 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 18 - 20, APIUpdateVmInstanceMetadataMsg 中的 vmUuids 字段仅使用了 `@APIParam`(nonempty = true) 而未验证数据库中是否存在对应 VM 实例，建议在该注解上添加 checkExistence = true；请定位类 APIUpdateVmInstanceMetadataMsg 中的私有字段 vmUuids 并将其 `@APIParam` 注解更新为包含 checkExistence = true（保留 resourceType = VmInstanceVO.class 和 nonempty = true），以在 API 层提前校验 VM UUID 的存在性。header/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageEvent.java (1)
29-32: 建议在__example__()中添加示例数据当前的
__example__()方法返回空的事件对象，建议添加示例数据以便生成更有意义的 API 文档。📝 建议的修改
public static APIScanVmInstanceMetadataFromPrimaryStorageEvent __example__() { APIScanVmInstanceMetadataFromPrimaryStorageEvent evt = new APIScanVmInstanceMetadataFromPrimaryStorageEvent(); + VmMetadataScanEntry entry = new VmMetadataScanEntry(); + entry.setVmUuid(uuid()); + entry.setVmName("example-vm"); + evt.setVmInstanceMetadata(java.util.Collections.singletonList(entry)); return evt; }🤖 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/APIScanVmInstanceMetadataFromPrimaryStorageEvent.java` around lines 29 - 32, The __example__() method in APIScanVmInstanceMetadataFromPrimaryStorageEvent returns an empty event; populate it with representative sample data to improve generated API docs by setting relevant fields on the APIScanVmInstanceMetadataFromPrimaryStorageEvent instance (e.g., set a sample primaryStorageUuid, a list or count of scanned VM metadata, status or success flag if available, and any child objects like VmInstanceMetadata entries) using the class's setters or public fields so the example shows realistic content; locate the APIScanVmInstanceMetadataFromPrimaryStorageEvent.__example__ method and add example values for its identifiable properties (primaryStorageUuid, metadata list/count, status) before returning the evt.sdk/src/main/java/org/zstack/sdk/VmMetadataScanEntry.java (1)
83-85: 布尔类型的 getter 方法命名建议使用is前缀按照 Java Bean 规范，
boolean类型的 getter 方法应使用is前缀而非get前缀。这可能会影响某些序列化框架的兼容性。♻️ 建议的修改
- public boolean getIncomplete() { + public boolean isIncomplete() { return this.incomplete; }🤖 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/VmMetadataScanEntry.java` around lines 83 - 85, The boolean getter is named getIncomplete which violates JavaBean conventions; rename the method getIncomplete() to isIncomplete() in class VmMetadataScanEntry and update all usages/tests/serializers that reference getIncomplete to call isIncomplete instead; ensure the existing private boolean field incomplete remains unchanged and, if necessary for serialization compatibility, add a `@JsonProperty`("incomplete") annotation to the new isIncomplete() or keep a delegating getIncomplete() that calls isIncomplete() to preserve binary/JSON compatibility.header/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataEvent.java (1)
12-12: 建议初始化failedVmUuids字段以避免潜在的空指针问题
failedVmUuids字段未初始化，调用getFailedVmUuids()可能返回null。建议与APIScanVmInstanceMetadataFromPrimaryStorageEvent保持一致，初始化为空列表。🛡️ 建议的修改
private Integer totalCleaned; private Integer totalFailed; - private List<String> failedVmUuids; + private List<String> failedVmUuids = new java.util.ArrayList<>();或者在 setter 中添加空值保护：
public void setFailedVmUuids(List<String> failedVmUuids) { - this.failedVmUuids = failedVmUuids; + this.failedVmUuids = failedVmUuids == null ? new java.util.ArrayList<>() : failedVmUuids; }🤖 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/APICleanupVmInstanceMetadataEvent.java` at line 12, APICleanupVmInstanceMetadataEvent's field failedVmUuids is not initialized and can be null; initialize it to an empty list (e.g. new ArrayList<>()) where the field is declared or add null-protection in its setter/getFailedVmUuids() to return an empty list to match APIScanVmInstanceMetadataFromPrimaryStorageEvent; update the field declaration or the setter/getter in class APICleanupVmInstanceMetadataEvent accordingly so callers never receive null.header/src/main/java/org/zstack/header/vm/metadata/VmMetadataPathReplacementExtensionPoint.java (1)
7-9: 建议为接口方法添加 Javadoc 注释根据编码规范，接口方法应配有有效的 Javadoc 注释，以便其他开发者理解扩展点的契约。
📝 建议添加 Javadoc
public interface VmMetadataPathReplacementExtensionPoint { + /** + * Returns the primary storage type this extension handles. + * `@return` primary storage type identifier + */ String getPrimaryStorageType(); + + /** + * Calculates path replacements for VM metadata migration. + * `@param` targetPsUuid target primary storage UUID + * `@param` allOldPaths list of old metadata paths to be replaced + * `@return` result containing path mappings and prefix information + */ PathReplacementResult calculatePathReplacements(String targetPsUuid, List<String> allOldPaths);🤖 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 7 - 9, 为接口 VmMetadataPathReplacementExtensionPoint 的两个方法添加 Javadoc：在 getPrimaryStorageType() 的注释中说明返回值含义（应返回哪个主存储类型标识、不能为 null 的约束等）；在 calculatePathReplacements(String targetPsUuid, List<String> allOldPaths) 的注释中说明每个参数的含义（targetPsUuid 表示目标主存储 UUID，allOldPaths 为需替换的原始路径列表）、预期的返回值类型 PathReplacementResult 的语义（例如如何表示替换映射或无替换情况）、异常场景或线程安全/并发调用约束以及允许的空值处理策略；确保描述清晰以形成扩展点契约。plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageSimulator.java (1)
375-383:prefixRebaseBackingFiles模拟器端点缺少命令记录。其他模拟器端点（如
writeVmMetadata、scanVmMetadata等）都会将命令添加到config中以供测试验证，但prefixRebaseBackingFiles没有这样做。这可能导致测试无法验证该命令是否被正确调用。♻️ 建议添加命令记录
`@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; }🤖 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 simulator endpoint LocalStorageSimulator.prefixRebaseBackingFiles currently constructs a PrefixRebaseBackingFilesCmd but does not record it for test verification; update prefixRebaseBackingFiles to add the received PrefixRebaseBackingFilesCmd (the cmd variable) into the simulator's command log/config used by other endpoints (same mechanism used by writeVmMetadata/scanVmMetadata) before replying with PrefixRebaseBackingFilesRsp so tests can assert the command was invoked.plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsVmMetadataExtension.java (2)
37-37: 路径末尾斜杠处理不一致。
resolveBaseDir方法（第 37 行）移除末尾斜杠，而calculatePathReplacements中的newPrefix（第 101 行）总是添加末尾斜杠：// resolveBaseDir - 移除末尾斜杠 return baseDir.endsWith("/") ? baseDir.substring(0, baseDir.length() - 1) : baseDir; // calculatePathReplacements - 添加末尾斜杠 String newPrefix = baseDir.endsWith("/") ? baseDir : baseDir + "/";这种不一致可能是有意为之（因为
buildMetadataDir使用resolveBaseDir后会在String.format中添加斜杠），但建议添加注释说明设计意图，以避免后续维护时产生混淆。Also applies to: 101-101
🤖 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` at line 37, The code treats trailing slashes inconsistently: resolveBaseDir removes a trailing '/' while calculatePathReplacements derives newPrefix that ensures a trailing '/', which is confusing; update the implementation to be explicit by either (a) making both resolveBaseDir and newPrefix consistently include or exclude the trailing slash, or (b) keep the current behavior but add a clear comment in resolveBaseDir, calculatePathReplacements (and buildMetadataDir) explaining the intended convention (e.g., resolveBaseDir returns a no-trailing-slash base and other callers append '/' as needed), referencing the resolveBaseDir, calculatePathReplacements, newPrefix and buildMetadataDir symbols so future maintainers understand the design intent.
29-38: 建议提取公共逻辑以减少代码重复。
resolveBaseDir方法（第 29-38 行）与calculatePathReplacements中的逻辑（第 91-100 行）存在重复：两处都从PrimaryStorageVO获取mountPath或url。虽然错误处理策略不同（一个抛异常，一个记录警告并返回空），但可以考虑提取公共部分：
♻️ 可选重构建议
+ private String resolveBaseDirOrNull(String primaryStorageUuid) { + String baseDir = Q.New(PrimaryStorageVO.class).select(PrimaryStorageVO_.mountPath) + .eq(PrimaryStorageVO_.uuid, primaryStorageUuid).findValue(); + if (baseDir == null) { + baseDir = Q.New(PrimaryStorageVO.class).select(PrimaryStorageVO_.url) + .eq(PrimaryStorageVO_.uuid, primaryStorageUuid).findValue(); + } + return baseDir; + } + private String resolveBaseDir(String primaryStorageUuid) { - String baseDir = Q.New(PrimaryStorageVO.class).select(PrimaryStorageVO_.mountPath).eq(PrimaryStorageVO_.uuid, primaryStorageUuid).findValue(); - if (baseDir == null) { - baseDir = Q.New(PrimaryStorageVO.class).select(PrimaryStorageVO_.url).eq(PrimaryStorageVO_.uuid, primaryStorageUuid).findValue(); - } + String baseDir = resolveBaseDirOrNull(primaryStorageUuid); if (baseDir == null) { throw new CloudRuntimeException(String.format("cannot find mountPath or url for NFS primary storage[uuid:%s]", primaryStorageUuid)); } return baseDir.endsWith("/") ? baseDir.substring(0, baseDir.length() - 1) : baseDir; }Also applies to: 91-100
🤖 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 29 - 38, Extract the duplicated logic that queries PrimaryStorageVO for mountPath or url into a single helper (e.g., a private method like fetchPrimaryStorageBase(String primaryStorageUuid) or getMountOrUrl) that uses Q.New(PrimaryStorageVO.class).select(PrimaryStorageVO_.mountPath).eq(PrimaryStorageVO_.uuid, primaryStorageUuid).findValue() and falls back to selecting PrimaryStorageVO_.url; have resolveBaseDir call this helper and throw the CloudRuntimeException if it returns null, and have calculatePathReplacements call the same helper and handle a null return by logging a warning and returning null/empty as it currently does—this removes the duplicated query while preserving each caller's error handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@conf/db/upgrade/V5.0.0__schema.sql`:
- Around line 12-13: The FOREIGN KEY REFERENCES clauses for the constraints
fkVmMetadataDirtyVOVmInstanceEO and fkVmMetadataDirtyVOManagementNodeVO (and the
other FOREIGN KEY at the same section) must explicitly qualify the referenced
tables with the zstack schema; update the REFERENCES targets from VmInstanceEO
(`uuid`) and ManagementNodeVO (`uuid`) (and the third referenced table at the
same block) to zstack.VmInstanceEO (`uuid`) and zstack.ManagementNodeVO (`uuid`)
(and zstack.<ThirdTable>) so the foreign key clauses consistently include the
zstack schema qualifier.
In
`@header/src/main/java/org/zstack/header/vm/metadata/APIRegisterVmInstanceFromMetadataMsgDoc_zh_cn.groovy`:
- Around line 108-112: Add a trailing newline at the end of the file so the file
ends with a newline character (POSIX compliance); locate the file block that
ends with the APIRegisterVmInstanceFromMetadataEvent.class response and the
closing braces and ensure there is a single blank line (newline) after the final
"}" so the file terminates with a newline.
In `@header/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyVO.java`:
- Around line 24-25: The dirtyVersion field in VmMetadataDirtyVO is currently an
uninitialized primitive long (defaults to 0) which causes JPA to persist 0
instead of the DB default 1; initialize dirtyVersion to 1L at the entity level
(either via a field initializer or in the VmMetadataDirtyVO constructor) so the
value matches the database default and JPA will persist 1 instead of 0; update
the declaration of dirtyVersion and/or the class constructor accordingly.
In `@header/src/main/java/org/zstack/header/volume/APIDeleteDataVolumeMsg.java`:
- Line 7: The project references a Spring bean named VolumeUuidToVmUuidResolver
via the `@MetadataImpact` annotation but that bean is missing; implement a class
VolumeUuidToVmUuidResolver that implements the VmUuidFromApiResolver interface
(or change the `@MetadataImpact` usage to point to an existing resolver) so Spring
can autowire it at startup; ensure the new class lives on the component-scan
path, is annotated appropriately (e.g., `@Component` or registered as a bean),
implements the required methods from VmUuidFromApiResolver, and is referenced by
APIDeleteDataVolumeMsg's `@MetadataImpact` configuration.
In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java`:
- Around line 3471-3490: The calls to getHypervisorBackendFactoryByHostUuid(...)
and f.getHypervisorBackend(self) can throw if the host is detached concurrently,
which bypasses the per-host error handling; wrap the lines that obtain
LocalStorageHypervisorFactory and LocalStorageHypervisorBackend (the calls to
getHypervisorBackendFactoryByHostUuid(...) and f.getHypervisorBackend(self)) in
a try/catch inside the While callback, and on exception invoke com.addError(...)
with a converted ErrorCode (or build one), call com.done(), and log the failure,
then return so bkd.handle(...) is not called for that host; keep existing
success/fail behavior in bkd.handle(...) unchanged.
---
Nitpick comments:
In `@compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java`:
- Line 37: Remove the unused import of VmInstanceMetadataConstants from
VmInstanceApiInterceptor: locate the import statement "import
org.zstack.header.vm.metadata.VmInstanceMetadataConstants;" and delete it so
there are no unused imports remaining in the VmInstanceApiInterceptor class.
In
`@header/src/main/java/org/zstack/header/storage/primary/APIScanVmInstanceMetadataFromPrimaryStorageEvent.java`:
- Around line 29-32: The __example__() method in
APIScanVmInstanceMetadataFromPrimaryStorageEvent returns an empty event;
populate it with representative sample data to improve generated API docs by
setting relevant fields on the APIScanVmInstanceMetadataFromPrimaryStorageEvent
instance (e.g., set a sample primaryStorageUuid, a list or count of scanned VM
metadata, status or success flag if available, and any child objects like
VmInstanceMetadata entries) using the class's setters or public fields so the
example shows realistic content; locate the
APIScanVmInstanceMetadataFromPrimaryStorageEvent.__example__ method and add
example values for its identifiable properties (primaryStorageUuid, metadata
list/count, status) before returning the evt.
In
`@header/src/main/java/org/zstack/header/storage/snapshot/APIShrinkVolumeSnapshotMsg.java`:
- Line 24: Add updateOnFailure = true to the `@MetadataImpact` annotation on
APIShrinkVolumeSnapshotMsg so its metadata update behavior matches
APIDeleteVolumeSnapshotMsg and APIRevertVolumeFromSnapshotMsg; locate the
`@MetadataImpact` on class APIShrinkVolumeSnapshotMsg and include the
updateOnFailure = true attribute (i.e., `@MetadataImpact`(value =
MetadataImpact.Impact.STORAGE, resolver = "SnapshotUuidToVmUuidResolver", field
= "uuid", updateOnFailure = true)).
In
`@header/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataEvent.java`:
- Line 12: APICleanupVmInstanceMetadataEvent's field failedVmUuids is not
initialized and can be null; initialize it to an empty list (e.g. new
ArrayList<>()) where the field is declared or add null-protection in its
setter/getFailedVmUuids() to return an empty list to match
APIScanVmInstanceMetadataFromPrimaryStorageEvent; update the field declaration
or the setter/getter in class APICleanupVmInstanceMetadataEvent accordingly so
callers never receive null.
In
`@header/src/main/java/org/zstack/header/vm/metadata/APICleanupVmInstanceMetadataMsg.java`:
- Around line 17-19: The API parameter vmUuids in
APICleanupVmInstanceMetadataMsg lacks existence validation; update the `@APIParam`
on the vmUuids field in the APICleanupVmInstanceMetadataMsg class to include
checkExistence = true so the API layer verifies VM UUIDs exist (match the
behavior used in APIUpdateVmInstanceMetadataMsg) by adjusting the `@APIParam`
annotation on the private List<String> vmUuids field.
- Around line 29-33: The __example__ method in APICleanupVmInstanceMetadataMsg
uses fully qualified java.util.Arrays.asList; simplify by adding an import
(import java.util.Arrays;) at the top and replace java.util.Arrays.asList(...)
with Arrays.asList(...), or alternatively import java.util.List and use
List.of(...) to construct vmUuids more concisely; update the
APICleanupVmInstanceMetadataMsg.__example__ method to assign vmUuids using the
chosen simplified factory method.
In
`@header/src/main/java/org/zstack/header/vm/metadata/APIUpdateVmInstanceMetadataMsg.java`:
- Around line 18-20: APIUpdateVmInstanceMetadataMsg 中的 vmUuids 字段仅使用了
`@APIParam`(nonempty = true) 而未验证数据库中是否存在对应 VM 实例，建议在该注解上添加 checkExistence =
true；请定位类 APIUpdateVmInstanceMetadataMsg 中的私有字段 vmUuids 并将其 `@APIParam` 注解更新为包含
checkExistence = true（保留 resourceType = VmInstanceVO.class 和 nonempty = true），以在
API 层提前校验 VM UUID 的存在性。
In `@header/src/main/java/org/zstack/header/vm/metadata/ResourceMetadata.java`:
- Line 5: The field name vo in class ResourceMetadata is ambiguous; rename it to
a descriptive identifier (e.g., valueObject, voJson, or voSerialized depending
on its actual content) and update all related symbols: the private field, its
getter/setter methods (e.g., getVo()/setVo() ->
getValueObject()/setValueObject()), any constructors, builders, equals/hashCode,
toString, serialization annotations, and usages across the codebase to match the
new name to avoid breaking references; also update persistence/JSON mapping
annotations or column names if present so external contracts remain consistent.
In `@header/src/main/java/org/zstack/header/vm/metadata/VmMetadataErrors.java`:
- Around line 18-22: 将 VmMetadataErrors 中的可变字段 code 改为不可变，修改字段声明为 private final
String code，并在构造器 VmMetadataErrors(int id) 中按原有逻辑赋值（code =
String.format("VM_METADATA.%s", id)）；确保没有其它地方尝试重新赋值该字段（若有，改为使用新的常量或方法返回值替代）。
In
`@header/src/main/java/org/zstack/header/vm/metadata/VmMetadataPathReplacementExtensionPoint.java`:
- Around line 7-9: 为接口 VmMetadataPathReplacementExtensionPoint 的两个方法添加 Javadoc：在
getPrimaryStorageType() 的注释中说明返回值含义（应返回哪个主存储类型标识、不能为 null 的约束等）；在
calculatePathReplacements(String targetPsUuid, List<String> allOldPaths)
的注释中说明每个参数的含义（targetPsUuid 表示目标主存储 UUID，allOldPaths 为需替换的原始路径列表）、预期的返回值类型
PathReplacementResult
的语义（例如如何表示替换映射或无替换情况）、异常场景或线程安全/并发调用约束以及允许的空值处理策略；确保描述清晰以形成扩展点契约。
In
`@header/src/main/java/org/zstack/header/vm/metadata/VmUuidFromApiResolver.java`:
- Around line 9-24: The batchResolveVmUuids method uses an explicit loop and
null checks that can be simplified with Java Streams: replace the for-loop in
batchResolveVmUuids with a stream pipeline on fieldValues that filters out
nulls, maps each value via resolveVmUuid, filters out null results, and collects
to a List<String>; ensure you still return an empty list when fieldValues is
null or empty and keep the method signature and resolveVmUuid call unchanged.
In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageSimulator.java`:
- Around line 375-383: The simulator endpoint
LocalStorageSimulator.prefixRebaseBackingFiles currently constructs a
PrefixRebaseBackingFilesCmd but does not record it for test verification; update
prefixRebaseBackingFiles to add the received PrefixRebaseBackingFilesCmd (the
cmd variable) into the simulator's command log/config used by other endpoints
(same mechanism used by writeVmMetadata/scanVmMetadata) before replying with
PrefixRebaseBackingFilesRsp so tests can assert the command was invoked.
In
`@plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsVmMetadataExtension.java`:
- Line 37: The code treats trailing slashes inconsistently: resolveBaseDir
removes a trailing '/' while calculatePathReplacements derives newPrefix that
ensures a trailing '/', which is confusing; update the implementation to be
explicit by either (a) making both resolveBaseDir and newPrefix consistently
include or exclude the trailing slash, or (b) keep the current behavior but add
a clear comment in resolveBaseDir, calculatePathReplacements (and
buildMetadataDir) explaining the intended convention (e.g., resolveBaseDir
returns a no-trailing-slash base and other callers append '/' as needed),
referencing the resolveBaseDir, calculatePathReplacements, newPrefix and
buildMetadataDir symbols so future maintainers understand the design intent.
- Around line 29-38: Extract the duplicated logic that queries PrimaryStorageVO
for mountPath or url into a single helper (e.g., a private method like
fetchPrimaryStorageBase(String primaryStorageUuid) or getMountOrUrl) that uses
Q.New(PrimaryStorageVO.class).select(PrimaryStorageVO_.mountPath).eq(PrimaryStorageVO_.uuid,
primaryStorageUuid).findValue() and falls back to selecting
PrimaryStorageVO_.url; have resolveBaseDir call this helper and throw the
CloudRuntimeException if it returns null, and have calculatePathReplacements
call the same helper and handle a null return by logging a warning and returning
null/empty as it currently does—this removes the duplicated query while
preserving each caller's error handling.
In
`@sdk/src/main/java/org/zstack/sdk/ScanVmInstanceMetadataFromPrimaryStorageResult.java`:
- Around line 6-11: The generated Result class
ScanVmInstanceMetadataFromPrimaryStorageResult uses a raw java.util.List for the
field vmInstanceMetadata and its accessor methods; change the field and the
signatures of getVmInstanceMetadata and setVmInstanceMetadata to use the generic
type java.util.List<VmMetadataScanEntry> (and add/import VmMetadataScanEntry as
needed) so callers get compile-time type safety and IDE completion; update the
SDK code generator that emits this class to preserve generic type parameters
from the source reply (ScanVmInstanceMetadataFromPrimaryStorageReply ->
List<VmMetadataScanEntry>) when emitting fields and getter/setter method
signatures.
In `@sdk/src/main/java/org/zstack/sdk/VmMetadataScanEntry.java`:
- Around line 83-85: The boolean getter is named getIncomplete which violates
JavaBean conventions; rename the method getIncomplete() to isIncomplete() in
class VmMetadataScanEntry and update all usages/tests/serializers that reference
getIncomplete to call isIncomplete instead; ensure the existing private boolean
field incomplete remains unchanged and, if necessary for serialization
compatibility, add a `@JsonProperty`("incomplete") annotation to the new
isIncomplete() or keep a delegating getIncomplete() that calls isIncomplete() to
preserve binary/JSON compatibility.
In `@testlib/src/main/java/org/zstack/testlib/NfsPrimaryStorageSpec.groovy`:
- Around line 525-539: The four simulator handlers for
NfsPrimaryStorageKVMBackend (WRITE_VM_METADATA_PATH,
GET_VM_INSTANCE_METADATA_PATH, SCAN_VM_METADATA_PATH, CLEANUP_VM_METADATA_PATH)
currently always return static success responses which prevents testing the
write-read-scan-cleanup metadata lifecycle; implement a minimal in-memory
metadata store (or VFS hook) inside the test harness and update the simulator
blocks to mutate/read that store: have the WRITE handler record metadata keyed
by vm/instance id, GET return stored metadata or not-found, SCAN enumerate
stored entries, and CLEANUP remove entries so the full chain can be validated
during tests.
🪄 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: c130435e-a80e-45b3-a777-90085b931d7b
⛔ 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/upgrade/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/VmMetadataFingerprintVO.javaheader/src/main/java/org/zstack/header/vm/metadata/VmMetadataFingerprintVO_.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
Comment on lines
+12
to
+13
| CONSTRAINT `fkVmMetadataDirtyVOVmInstanceEO` FOREIGN KEY (`vmInstanceUuid`) REFERENCES `VmInstanceEO` (`uuid`) ON DELETE CASCADE, | ||
| CONSTRAINT `fkVmMetadataDirtyVOManagementNodeVO` FOREIGN KEY (`managementNodeUuid`) REFERENCES `ManagementNodeVO` (`uuid`) ON DELETE SET NULL |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 只读核验：检查 upgrade 目录中 REFERENCES 的 schema 使用习惯
rg -n --type=sql 'REFERENCES\s+`[^`]+`' conf/db/upgrade
rg -n --type=sql 'REFERENCES\s+`zstack`\.`[^`]+`' conf/db/upgradeRepository: MatheMatrix/zstack
Length of output: 50374
🏁 Script executed:
cat conf/db/upgrade/V5.0.0__schema.sqlRepository: MatheMatrix/zstack
Length of output: 1565
外键约束应显式包含 zstack schema 限定符以保持代码风格一致。
第 12-13 行和第 25 行的 FOREIGN KEY REFERENCES 子句缺少 zstack schema 限定符。虽然在当前 zstack 默认库上下文下仍能解析，但与同目录其他升级脚本的风格不一致，且在非 zstack 默认库上下文执行时可能引发解析风险。建议补全 schema 限定符。
建议修改
- CONSTRAINT `fkVmMetadataDirtyVOVmInstanceEO` FOREIGN KEY (`vmInstanceUuid`) REFERENCES `VmInstanceEO` (`uuid`) ON DELETE CASCADE,
- CONSTRAINT `fkVmMetadataDirtyVOManagementNodeVO` FOREIGN KEY (`managementNodeUuid`) REFERENCES `ManagementNodeVO` (`uuid`) ON DELETE SET NULL
+ CONSTRAINT `fkVmMetadataDirtyVOVmInstanceEO` FOREIGN KEY (`vmInstanceUuid`) REFERENCES `zstack`.`VmInstanceEO` (`uuid`) ON DELETE CASCADE,
+ CONSTRAINT `fkVmMetadataDirtyVOManagementNodeVO` FOREIGN KEY (`managementNodeUuid`) REFERENCES `zstack`.`ManagementNodeVO` (`uuid`) ON DELETE SET NULL
@@
- CONSTRAINT `fkVmMetadataFingerprintVOVmInstanceEO` FOREIGN KEY (`vmInstanceUuid`) REFERENCES `VmInstanceEO` (`uuid`) ON DELETE CASCADE
+ CONSTRAINT `fkVmMetadataFingerprintVOVmInstanceEO` FOREIGN KEY (`vmInstanceUuid`) REFERENCES `zstack`.`VmInstanceEO` (`uuid`) ON DELETE CASCADE📝 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
| CONSTRAINT `fkVmMetadataDirtyVOVmInstanceEO` FOREIGN KEY (`vmInstanceUuid`) REFERENCES `VmInstanceEO` (`uuid`) ON DELETE CASCADE, | |
| CONSTRAINT `fkVmMetadataDirtyVOManagementNodeVO` FOREIGN KEY (`managementNodeUuid`) REFERENCES `ManagementNodeVO` (`uuid`) ON DELETE SET NULL | |
| CONSTRAINT `fkVmMetadataDirtyVOVmInstanceEO` FOREIGN KEY (`vmInstanceUuid`) REFERENCES `zstack`.`VmInstanceEO` (`uuid`) ON DELETE CASCADE, | |
| CONSTRAINT `fkVmMetadataDirtyVOManagementNodeVO` FOREIGN KEY (`managementNodeUuid`) REFERENCES `zstack`.`ManagementNodeVO` (`uuid`) ON DELETE SET NULL |
Suggested change
| CONSTRAINT `fkVmMetadataDirtyVOVmInstanceEO` FOREIGN KEY (`vmInstanceUuid`) REFERENCES `VmInstanceEO` (`uuid`) ON DELETE CASCADE, | |
| CONSTRAINT `fkVmMetadataDirtyVOManagementNodeVO` FOREIGN KEY (`managementNodeUuid`) REFERENCES `ManagementNodeVO` (`uuid`) ON DELETE SET NULL | |
| CONSTRAINT `fkVmMetadataFingerprintVOVmInstanceEO` FOREIGN KEY (`vmInstanceUuid`) REFERENCES `zstack`.`VmInstanceEO` (`uuid`) ON DELETE CASCADE |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@conf/db/upgrade/V5.0.0__schema.sql` around lines 12 - 13, The FOREIGN KEY
REFERENCES clauses for the constraints fkVmMetadataDirtyVOVmInstanceEO and
fkVmMetadataDirtyVOManagementNodeVO (and the other FOREIGN KEY at the same
section) must explicitly qualify the referenced tables with the zstack schema;
update the REFERENCES targets from VmInstanceEO (`uuid`) and ManagementNodeVO
(`uuid`) (and the third referenced table at the same block) to
zstack.VmInstanceEO (`uuid`) and zstack.ManagementNodeVO (`uuid`) (and
zstack.<ThirdTable>) so the foreign key clauses consistently include the zstack
schema qualifier.
Comment on lines
+108
to
+112
| response { | ||
| clz APIRegisterVmInstanceFromMetadataEvent.class | ||
| } | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
文件末尾缺少换行符。
按照 POSIX 标准，文本文件应以换行符结尾。建议在第 112 行后添加一个空行。
🤖 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/APIRegisterVmInstanceFromMetadataMsgDoc_zh_cn.groovy`
around lines 108 - 112, Add a trailing newline at the end of the file so the
file ends with a newline character (POSIX compliance); locate the file block
that ends with the APIRegisterVmInstanceFromMetadataEvent.class response and the
closing braces and ensure there is a single blank line (newline) after the final
"}" so the file terminates with a newline.
Comment on lines
+24
to
+25
| @Column | ||
| private long dirtyVersion; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 只读核验：定位实体创建点及 dirtyVersion 显式赋值情况
rg -n --type=java '\bnew\s+VmMetadataDirtyVO\s*\('
rg -n --type=java '\.setDirtyVersion\s*\('Repository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
cat -n header/src/main/java/org/zstack/header/vm/metadata/VmMetadataDirtyVO.javaRepository: MatheMatrix/zstack
Length of output: 3718
🏁 Script executed:
# Search for database migrations or schema files related to VmMetadata
fd -e sql -e groovy | xargs rg -l "VmMetadata|vm_metadata" -iRepository: MatheMatrix/zstack
Length of output: 479
🏁 Script executed:
# Look for Hibernate/JPA configuration or mapping files
fd -e xml -e properties | xargs rg -l "VmMetadataDirtyVO|vm_metadata" -iRepository: MatheMatrix/zstack
Length of output: 82
🏁 Script executed:
# Check the database schema for vm_metadata_dirty table
rg -A 20 "vm_metadata_dirty" conf/db/upgrade/V5.0.0__schema.sql conf/db/upgrade/V4.10.16__schema.sqlRepository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
# Read persistence.xml to understand JPA configuration
cat -n conf/persistence.xml | head -100Repository: MatheMatrix/zstack
Length of output: 7477
🏁 Script executed:
# Search for VmMetadataDirtyVO in persistence.xml
rg "VmMetadataDirtyVO" conf/persistence.xmlRepository: MatheMatrix/zstack
Length of output: 132
🏁 Script executed:
# Search for any mention of metadata_dirty in schema files
rg -i "metadata" conf/db/upgrade/*.sql | head -50Repository: MatheMatrix/zstack
Length of output: 6613
🏁 Script executed:
# Look for Groovy schema definitions
fd -e groovy "schema|migration" | xargs rg -l "metadata_dirty|VmMetadata" -iRepository: MatheMatrix/zstack
Length of output: 46
🏁 Script executed:
# Get the complete CREATE TABLE statement for VmMetadataDirtyVO
rg -A 30 "CREATE TABLE IF NOT EXISTS.*VmMetadataDirtyVO" conf/db/upgrade/V5.0.0__schema.sqlRepository: MatheMatrix/zstack
Length of output: 1565
🏁 Script executed:
# Also check V4.10.16 version
rg -A 30 "VmMetadataDirtyVO" conf/db/upgrade/V4.10.16__schema.sqlRepository: MatheMatrix/zstack
Length of output: 44
初始化 dirtyVersion 为 1，避免 JPA 持久化时与数据库默认值不一致。
当前 dirtyVersion 为未初始化的 primitive long 类型。在 Java 中，primitive 类型的默认值为 0，JPA 在持久化时会显式写入 0 值，而不是依赖数据库的 DEFAULT 1。这导致实体层的初始值与数据库定义的默认值不一致。
建议在实体层初始化为 1L：
修改建议
- private long dirtyVersion;
+ private long dirtyVersion = 1L;📝 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
| @Column | |
| private long dirtyVersion; | |
| `@Column` | |
| private long dirtyVersion = 1L; |
🤖 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`
around lines 24 - 25, The dirtyVersion field in VmMetadataDirtyVO is currently
an uninitialized primitive long (defaults to 0) which causes JPA to persist 0
instead of the DB default 1; initialize dirtyVersion to 1L at the entity level
(either via a field initializer or in the VmMetadataDirtyVO constructor) so the
value matches the database default and JPA will persist 1 instead of 0; update
the declaration of dirtyVersion and/or the class constructor accordingly.
| import org.zstack.header.message.APIDeleteMessage; | ||
| import org.zstack.header.message.APIParam; | ||
| import org.zstack.header.rest.RestRequest; | ||
| import org.zstack.header.vm.metadata.MetadataImpact; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 验证 VolumeUuidToVmUuidResolver 实现是否存在
rg -n "VolumeUuidToVmUuidResolver" --type javaRepository: MatheMatrix/zstack
Length of output: 2491
🏁 Script executed:
#!/bin/bash
# Search for the class definition of VolumeUuidToVmUuidResolver
rg "class VolumeUuidToVmUuidResolver" --type javaRepository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Search for resolver implementations to understand the pattern
rg "Resolver.*class|implements.*Resolver" --type java | head -20Repository: MatheMatrix/zstack
Length of output: 260
🏁 Script executed:
#!/bin/bash
# Search for MetadataImpact class definition
rg "class MetadataImpact|interface MetadataImpact" --type java -A 10Repository: MatheMatrix/zstack
Length of output: 1199
🏁 Script executed:
#!/bin/bash
# Search for VmUuidFromApiResolver to understand the interface
rg "VmUuidFromApiResolver|VolumeUuidToVmUuidResolver" --type java -A 5 -B 2Repository: MatheMatrix/zstack
Length of output: 14871
🏁 Script executed:
#!/bin/bash
# Search for bean configuration or component definition of VolumeUuidToVmUuidResolver
rg "@Bean.*VolumeUuidToVmUuidResolver|@Component.*VolumeUuidToVmUuidResolver|new VolumeUuidToVmUuidResolver|implements.*VmUuidFromApiResolver" --type java -B 3 -A 3Repository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Search for any file containing VolumeUuidToVmUuidResolver
fd -type f | xargs rg -l "VolumeUuidToVmUuidResolver" 2>/dev/null | head -20Repository: MatheMatrix/zstack
Length of output: 1184
🏁 Script executed:
#!/bin/bash
# Search for other resolver implementations
rg "class.*Resolver.*implements.*VmUuidFromApiResolver" --type javaRepository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Search for Spring XML configuration files
find . -name "*.xml" -type f | grep -i spring | head -10Repository: MatheMatrix/zstack
Length of output: 565
🏁 Script executed:
#!/bin/bash
# Search for VolumeUuidToVmUuidResolver in XML files
rg "VolumeUuidToVmUuidResolver" --type xmlRepository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Search for any *Resolver classes in storage/volume modules
find . -path "*/storage/*" -o -path "*/volume/*" | xargs rg "class.*Resolver" --type java 2>/dev/null | grep -v "test" | head -20Repository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Check the full APIDeleteDataVolumeMsg.java file
cat header/src/main/java/org/zstack/header/volume/APIDeleteDataVolumeMsg.javaRepository: MatheMatrix/zstack
Length of output: 2127
缺少 VolumeUuidToVmUuidResolver 实现，无法进行 Spring 装配
@MetadataImpact 注解引用的 Spring bean VolumeUuidToVmUuidResolver 在代码库中不存在。该 resolver 需要实现 VmUuidFromApiResolver 接口，否则运行时将抛出 NoSuchBeanDefinitionException。请实现该 resolver 类或调整注解配置。
🤖 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/APIDeleteDataVolumeMsg.java` at
line 7, The project references a Spring bean named VolumeUuidToVmUuidResolver
via the `@MetadataImpact` annotation but that bean is missing; implement a class
VolumeUuidToVmUuidResolver that implements the VmUuidFromApiResolver interface
(or change the `@MetadataImpact` usage to point to an existing resolver) so Spring
can autowire it at startup; ensure the new class lives on the component-scan
path, is annotated appropriately (e.g., `@Component` or registered as a bean),
implements the required methods from VmUuidFromApiResolver, and is referenced by
APIDeleteDataVolumeMsg's `@MetadataImpact` configuration.
Comment on lines
+3471
to
+3490
| new While<>(connectedHostUuids).all((hostUuid, com) -> { | ||
| LocalStorageHypervisorFactory f = getHypervisorBackendFactoryByHostUuid(hostUuid); | ||
| LocalStorageHypervisorBackend bkd = f.getHypervisorBackend(self); | ||
| bkd.handle(msg, hostUuid, new ReturnValueCompletion<ScanVmInstanceMetadataFromPrimaryStorageReply>(com) { | ||
| @Override | ||
| public void success(ScanVmInstanceMetadataFromPrimaryStorageReply returnValue) { | ||
| if (returnValue.getVmInstanceMetadata() != null) { | ||
| allSummaries.addAll(returnValue.getVmInstanceMetadata()); | ||
| } | ||
| com.done(); | ||
| } | ||
|
|
||
| @Override | ||
| public void fail(ErrorCode errorCode) { | ||
| logger.warn(String.format("failed to scan vm metadata from host[uuid:%s] on local primary storage[uuid:%s]: %s", | ||
| hostUuid, self.getUuid(), errorCode)); | ||
| com.addError(errorCode); | ||
| com.done(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
把同步 host 解析失败也收敛到单机失败路径。
这里在 bkd.handle(...) 之前同步执行 getHypervisorBackendFactoryByHostUuid() / getHypervisorBackend()。如果 host 在查询后被并发 detach/delete，这里会直接抛异常，绕过 com.addError(...); com.done()，把当前方法的“部分失败继续扫描”退化成整次失败。建议在这两步外层补 try/catch，按单 host 记错后继续。
🤖 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 3471 - 3490, The calls to
getHypervisorBackendFactoryByHostUuid(...) and f.getHypervisorBackend(self) can
throw if the host is detached concurrently, which bypasses the per-host error
handling; wrap the lines that obtain LocalStorageHypervisorFactory and
LocalStorageHypervisorBackend (the calls to
getHypervisorBackendFactoryByHostUuid(...) and f.getHypervisorBackend(self)) in
a try/catch inside the While callback, and on exception invoke com.addError(...)
with a converted ErrorCode (or build one), call com.done(), and log the failure,
then return so bkd.handle(...) is not called for that host; keep existing
success/fail behavior in bkd.handle(...) unchanged.
Collaborator
|
Comment from yaohua.wu: Review: MR !9494 — ZSV-11559
|
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 !9494