<feature>[kvm]: support clone for TPM Vm#3473
Open
zstack-robot-1 wants to merge 4 commits intofeature-zsv-5.0.0-vm-support-vtpm-and-secucebootfrom
Open
<feature>[kvm]: support clone for TPM Vm#3473zstack-robot-1 wants to merge 4 commits intofeature-zsv-5.0.0-vm-support-vtpm-and-secucebootfrom
zstack-robot-1 wants to merge 4 commits intofeature-zsv-5.0.0-vm-support-vtpm-and-secucebootfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough新增 VmHostBackupFileVO 表并调整 VmHostFileContentVO 的外键指向 ResourceVO；移除 VmHostFileType 中两个备份类型；新增主机文件与 TPM 克隆相关消息、KVM 管理逻辑、TPM 密钥后端接口与 dummy 实现；KvmSecureBootManager 转为服务并增加克隆处理流程。 Changes
Sequence Diagram(s)sequenceDiagram
participant API as 调用方
participant KSB as KvmSecureBootManager
participant DB as DatabaseFacade
participant Host as HostAgent
participant Resource as ResourceVO
rect rgba(200,230,255,0.5)
API->>KSB: CloneVmHostFileMsg(srcVmUuid, dstVmUuidList, resetTpm)
end
KSB->>DB: 查询源 VM 的 VmHostFile/VmHostBackupFile 与 Resource 引用
DB-->>KSB: 返回文件元数据与 Resource 列表
loop 对每个源主机/文件
KSB->>Host: RPC 读取文件内容
Host-->>KSB: 返回文件二进制或错误
end
KSB->>DB: 创建 VmHostBackupFileVO 记录并写入 VmHostFileContentVO（关联 ResourceVO.uuid）
DB-->>KSB: 持久化确认
KSB->>API: CloneVmHostFileReply(success/failure)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 诗歌
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
plugin/kvm/src/main/java/org/zstack/kvm/tpm/TpmEncryptedResourceKeyBackend.java (1)
5-12: 给这个扩展点补上 Javadoc。这是跨模块的 SPI，当前看不出
srcTpmUuid、dstTpmUuid、resetTpm的约束，也看不出completion何时必须回调成功/失败；后续实现很容易各写各的。As per coding guidelines "接口方法不应有多余的修饰符（例如 public），且必须配有有效的 Javadoc 注释。"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/kvm/src/main/java/org/zstack/kvm/tpm/TpmEncryptedResourceKeyBackend.java` around lines 5 - 12, 为跨模块 SPI TpmEncryptedResourceKeyBackend 补充完整的 Javadoc：在接口上说明用途和可见性约束；为内置静态类 CloneEncryptedResourceKeyContext 及其字段 srcTpmUuid、dstTpmUuid、resetTpm 分别注明含义、是否允许为 null/空、格式约束（例如 UUID 格式）、语义（src 为原始 TPM，dst 为目标 TPM，resetTpm 表示是否在克隆后重置目标 TPM）和谁负责校验；为方法 cloneEncryptedResourceKey(CloneEncryptedResourceKeyContext context, Completion completion) 增加 Javadoc 说明何时必须调用 completion.success() / completion.fail(Throwable)，在何种错误条件下应返回失败、是否允许异步返回、线程/重试语义以及调用方和实现方的责任；同时确认方法签名不包含多余的访问修饰符并在注释中给出典型使用示例或引用调用约定。
🤖 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/zsv/V5.0.0__schema.sql`:
- Around line 15-17: Changing hostUuid to nullable breaks the
ukVmHostFileVO(vmInstanceUuid, hostUuid, type) uniqueness for backup types
(NvRamBackup/TpmStateBackup) because MySQL allows multiple NULLs; add a new
uniqueness constraint that covers backup records without hostUuid (for example a
unique index on (vmInstanceUuid, type) filtered/conditional for type IN
('NvRamBackup','TpmStateBackup')), or alternatively model backups in a separate
table or enforce a non-null placeholder value for hostUuid for backup rows;
update schema DDL around hostUuid and the ukVmHostFileVO definition to implement
one of these options and ensure inserts/queries use the new constraint.
In `@header/src/main/java/org/zstack/header/vm/additions/VmHostFileType.java`:
- Around line 10-18: The helper method VmHostFileType.backupType(VmHostFileType
type) currently does a switch(type) which will throw NullPointerException if
type is null; add an early null check at the start of backupType and return null
immediately when type == null so the method preserves its existing "unable to
map => null" behavior (keep the existing switch for cases NvRam/NvRamBackup and
TpmState/TpmStateBackup).
In `@plugin/kvm/src/main/java/org/zstack/kvm/efi/CloneVmHostFileMsg.java`:
- Around line 10-33: The resetTpm field in CloneVmHostFileMsg is a nullable
Boolean which can cause NPEs; change the field from Boolean resetTpm to
primitive boolean resetTpm (or initialize it to false) and update the
getter/setter signatures (getResetTpm/setResetTpm) accordingly so callers and
any auto-unboxing won't encounter null; ensure any places constructing
CloneVmHostFileMsg use the primitive or rely on the default false value.
In `@plugin/kvm/src/main/java/org/zstack/kvm/efi/KvmSecureBootManager.java`:
- Around line 234-242: WhileDoneCompletion.done() always calls logger.warn which
produces misleading warnings even when no errors occurred; update the done
implementation in KvmSecureBootManager (the WhileDoneCompletion.done method
handling errorCodeList) to first check that errorCodeList is non-null and that
errorCodeList.getCauses() is not empty (or has errors) before calling
logger.warn (use
transform/errorCodeList.getCauses()/ErrorCode::getReadableDetails to build the
message); always call trigger.next() afterwards regardless of logging.
- Around line 244-297: The flow chain created for copying host file DB never
gets started and lacks completion handlers; add FlowDoneHandler and
FlowErrorHandler to the SimpleFlowChain instance (the chain that adds the
"copy-host-content-database" NoRollbackFlow) to send a CloneVmHostFileReply on
success and send an error reply on failure, import
org.zstack.header.core.workflow.FlowDoneHandler and FlowErrorHandler, and
finally call .start() on the chain so the chain executes and the caller receives
a response.
In `@plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmManager.java`:
- Around line 316-355: The method handle(CloneVmTpmMsg msg) can NPE when
tpmKeyBackend (injected with `@Autowired`(required = false)) is null; add a null
check before calling tpmKeyBackend.cloneEncryptedResourceKey: if tpmKeyBackend
is null create an appropriate ErrorCode on the CloneVmTpmReply and
bus.reply(msg, reply) immediately (or fail the Completion) so you never call
cloneEncryptedResourceKey on a null reference; update behavior around
TpmEncryptedResourceKeyBackend.CloneEncryptedResourceKeyContext accordingly so
the early-return path mirrors the failure handling used in the async completion.
---
Nitpick comments:
In
`@plugin/kvm/src/main/java/org/zstack/kvm/tpm/TpmEncryptedResourceKeyBackend.java`:
- Around line 5-12: 为跨模块 SPI TpmEncryptedResourceKeyBackend 补充完整的
Javadoc：在接口上说明用途和可见性约束；为内置静态类 CloneEncryptedResourceKeyContext 及其字段
srcTpmUuid、dstTpmUuid、resetTpm 分别注明含义、是否允许为 null/空、格式约束（例如 UUID 格式）、语义（src 为原始
TPM，dst 为目标 TPM，resetTpm 表示是否在克隆后重置目标 TPM）和谁负责校验；为方法
cloneEncryptedResourceKey(CloneEncryptedResourceKeyContext context, Completion
completion) 增加 Javadoc 说明何时必须调用 completion.success() /
completion.fail(Throwable)，在何种错误条件下应返回失败、是否允许异步返回、线程/重试语义以及调用方和实现方的责任；同时确认方法签名不包含多余的访问修饰符并在注释中给出典型使用示例或引用调用约定。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5f253829-9deb-44a3-831d-002cca67e95d
⛔ Files ignored due to path filters (2)
conf/springConfigXml/Kvm.xmlis excluded by!**/*.xmltest/src/test/resources/springConfigXml/Kvm.xmlis excluded by!**/*.xml
📒 Files selected for processing (11)
conf/db/zsv/V5.0.0__schema.sqlheader/src/main/java/org/zstack/header/vm/VmInstanceConstant.javaheader/src/main/java/org/zstack/header/vm/additions/VmHostFileType.javaplugin/kvm/src/main/java/org/zstack/kvm/efi/CloneVmHostFileMsg.javaplugin/kvm/src/main/java/org/zstack/kvm/efi/CloneVmHostFileReply.javaplugin/kvm/src/main/java/org/zstack/kvm/efi/KvmSecureBootManager.javaplugin/kvm/src/main/java/org/zstack/kvm/tpm/CloneVmTpmMsg.javaplugin/kvm/src/main/java/org/zstack/kvm/tpm/CloneVmTpmReply.javaplugin/kvm/src/main/java/org/zstack/kvm/tpm/DummyTpmEncryptedResourceKeyBackend.javaplugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmManager.javaplugin/kvm/src/main/java/org/zstack/kvm/tpm/TpmEncryptedResourceKeyBackend.java
Comment on lines
+10
to
+18
| public static VmHostFileType backupType(VmHostFileType type) { | ||
| switch (type) { | ||
| case NvRam: case NvRamBackup: | ||
| return NvRamBackup; | ||
| case TpmState: case TpmStateBackup: | ||
| return TpmStateBackup; | ||
| default: | ||
| return null; | ||
| } |
There was a problem hiding this comment.
先处理 null 入参。
Line 11 直接 switch (type) 在 type == null 时会抛 NullPointerException。既然这个 helper 对无法映射的值本来就返回 null，这里最好也早返回，避免把一个工具方法变成运行时炸点。
🔧 建议修改
public static VmHostFileType backupType(VmHostFileType type) {
+ if (type == null) {
+ return null;
+ }
switch (type) {
case NvRam: case NvRamBackup:
return NvRamBackup;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public static VmHostFileType backupType(VmHostFileType type) { | |
| switch (type) { | |
| case NvRam: case NvRamBackup: | |
| return NvRamBackup; | |
| case TpmState: case TpmStateBackup: | |
| return TpmStateBackup; | |
| default: | |
| return null; | |
| } | |
| public static VmHostFileType backupType(VmHostFileType type) { | |
| if (type == null) { | |
| return null; | |
| } | |
| switch (type) { | |
| case NvRam: case NvRamBackup: | |
| return NvRamBackup; | |
| case TpmState: case TpmStateBackup: | |
| return TpmStateBackup; | |
| default: | |
| return null; | |
| } | |
| } |
🤖 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/additions/VmHostFileType.java`
around lines 10 - 18, The helper method VmHostFileType.backupType(VmHostFileType
type) currently does a switch(type) which will throw NullPointerException if
type is null; add an early null check at the start of backupType and return null
immediately when type == null so the method preserves its existing "unable to
map => null" behavior (keep the existing switch for cases NvRam/NvRamBackup and
TpmState/TpmStateBackup).
Comment on lines
+10
to
+33
| private Boolean resetTpm; | ||
|
|
||
| public String getSrcVmUuid() { | ||
| return srcVmUuid; | ||
| } | ||
|
|
||
| public void setSrcVmUuid(String srcVmUuid) { | ||
| this.srcVmUuid = srcVmUuid; | ||
| } | ||
|
|
||
| public List<String> getDstVmUuidList() { | ||
| return dstVmUuidList; | ||
| } | ||
|
|
||
| public void setDstVmUuidList(List<String> dstVmUuidList) { | ||
| this.dstVmUuidList = dstVmUuidList; | ||
| } | ||
|
|
||
| public Boolean getResetTpm() { | ||
| return resetTpm; | ||
| } | ||
|
|
||
| public void setResetTpm(Boolean resetTpm) { | ||
| this.resetTpm = resetTpm; |
There was a problem hiding this comment.
resetTpm 不要做成可空三态。
这里的语义看起来只有“重置 / 不重置”两种，Boolean 会额外引入 null；下游只要有一次自动拆箱，或者直接写 if (msg.getResetTpm())，就会在未赋值时报 NPE。更稳妥的是直接改成 boolean，或者至少在字段上默认成 false。
🔧 建议修改
- private Boolean resetTpm;
+ private boolean resetTpm;
@@
- public Boolean getResetTpm() {
+ public boolean getResetTpm() {
return resetTpm;
}
@@
- public void setResetTpm(Boolean resetTpm) {
+ public void setResetTpm(boolean resetTpm) {
this.resetTpm = resetTpm;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/kvm/src/main/java/org/zstack/kvm/efi/CloneVmHostFileMsg.java` around
lines 10 - 33, The resetTpm field in CloneVmHostFileMsg is a nullable Boolean
which can cause NPEs; change the field from Boolean resetTpm to primitive
boolean resetTpm (or initialize it to false) and update the getter/setter
signatures (getResetTpm/setResetTpm) accordingly so callers and any
auto-unboxing won't encounter null; ensure any places constructing
CloneVmHostFileMsg use the primitive or rely on the default false value.
Comment on lines
+316
to
+355
| private void handle(CloneVmTpmMsg msg) { | ||
| CloneVmTpmReply reply = new CloneVmTpmReply(); | ||
|
|
||
| String originTpmUuid = Q.New(TpmVO.class) | ||
| .eq(TpmVO_.vmInstanceUuid, msg.getSrcVmUuid()) | ||
| .select(TpmVO_.uuid) | ||
| .findValue(); | ||
| if (originTpmUuid == null) { | ||
| bus.reply(msg, reply); | ||
| return; | ||
| } | ||
|
|
||
| TpmVO dstTpm = vmTpmManager.persistTpmVO(null, msg.getDstVmUuid()); | ||
|
|
||
| boolean resetTpm; | ||
| if (msg.getResetTpm() == null) { | ||
| ResourceConfig resourceConfig = resourceConfigFacade.getResourceConfig(RESET_TPM_AFTER_VM_CLONE.getIdentity()); | ||
| resetTpm = resourceConfig.getResourceConfigValue(msg.getSrcVmUuid(), Boolean.class); | ||
| } else { | ||
| resetTpm = msg.getResetTpm(); | ||
| } | ||
|
|
||
| TpmEncryptedResourceKeyBackend.CloneEncryptedResourceKeyContext context = | ||
| new TpmEncryptedResourceKeyBackend.CloneEncryptedResourceKeyContext(); | ||
| context.srcTpmUuid = originTpmUuid; | ||
| context.dstTpmUuid = dstTpm.getUuid(); | ||
| context.resetTpm = resetTpm; | ||
| tpmKeyBackend.cloneEncryptedResourceKey(context, new Completion(msg) { | ||
| @Override | ||
| public void success() { | ||
| bus.reply(msg, reply); | ||
| } | ||
|
|
||
| @Override | ||
| public void fail(ErrorCode errorCode) { | ||
| reply.setError(errorCode); | ||
| bus.reply(msg, reply); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
tpmKeyBackend 可能为 null，存在 NPE 风险。
tpmKeyBackend 使用 @Autowired(required = false) 注入，意味着它可能为 null。在 Line 343 调用 tpmKeyBackend.cloneEncryptedResourceKey() 之前没有进行空值检查，如果该 bean 未配置，将导致 NullPointerException。
🛡️ 建议添加空值检查
+ if (tpmKeyBackend == null) {
+ logger.warn("TpmEncryptedResourceKeyBackend is not available, skip cloning TPM key");
+ bus.reply(msg, reply);
+ return;
+ }
+
TpmEncryptedResourceKeyBackend.CloneEncryptedResourceKeyContext context =
new TpmEncryptedResourceKeyBackend.CloneEncryptedResourceKeyContext();
context.srcTpmUuid = originTpmUuid;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmManager.java` around lines
316 - 355, The method handle(CloneVmTpmMsg msg) can NPE when tpmKeyBackend
(injected with `@Autowired`(required = false)) is null; add a null check before
calling tpmKeyBackend.cloneEncryptedResourceKey: if tpmKeyBackend is null create
an appropriate ErrorCode on the CloneVmTpmReply and bus.reply(msg, reply)
immediately (or fail the Completion) so you never call cloneEncryptedResourceKey
on a null reference; update behavior around
TpmEncryptedResourceKeyBackend.CloneEncryptedResourceKeyContext accordingly so
the early-return path mirrors the failure handling used in the async completion.
MatheMatrix
force-pushed
the
sync/wenhao.zhang/zsv-ldap@@2
branch
from
b2c1689 to
e82f91e
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 `@plugin/kvm/src/main/java/org/zstack/kvm/efi/KvmSecureBootManager.java`:
- Around line 204-215: The comparison using
file.getHostUuid().equals(item.hostUuid) can NPE if file.getHostUuid() is null;
update the matching logic in the loop that resolves
SyncVmHostFilesFromHostContext (the call to findOneOrNull and the lambda
referencing file.getHostUuid() and item.hostUuid) to perform a null-safe
comparison (e.g., compare item.hostUuid to file.getHostUuid() in a null-safe way
or use Objects.equals(item.hostUuid, file.getHostUuid())) so that null hostUuid
values are handled without throwing; keep assigning nvRamPath/tpmStateFolder on
the resolved SyncVmHostFilesFromHostContext as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a276fb91-1e1f-44c6-885d-f158bda2d4e5
📥 Commits
Reviewing files that changed from the base of the PR and between b2c1689 and e82f91ecc6a2dc0be6b27101e90515838ab13c3f.
⛔ Files ignored due to path filters (2)
conf/springConfigXml/Kvm.xmlis excluded by!**/*.xmltest/src/test/resources/springConfigXml/Kvm.xmlis excluded by!**/*.xml
📒 Files selected for processing (11)
conf/db/zsv/V5.0.0__schema.sqlheader/src/main/java/org/zstack/header/vm/VmInstanceConstant.javaheader/src/main/java/org/zstack/header/vm/additions/VmHostFileType.javaplugin/kvm/src/main/java/org/zstack/kvm/efi/CloneVmHostFileMsg.javaplugin/kvm/src/main/java/org/zstack/kvm/efi/CloneVmHostFileReply.javaplugin/kvm/src/main/java/org/zstack/kvm/efi/KvmSecureBootManager.javaplugin/kvm/src/main/java/org/zstack/kvm/tpm/CloneVmTpmMsg.javaplugin/kvm/src/main/java/org/zstack/kvm/tpm/CloneVmTpmReply.javaplugin/kvm/src/main/java/org/zstack/kvm/tpm/DummyTpmEncryptedResourceKeyBackend.javaplugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmManager.javaplugin/kvm/src/main/java/org/zstack/kvm/tpm/TpmEncryptedResourceKeyBackend.java
🚧 Files skipped from review as they are similar to previous changes (5)
- plugin/kvm/src/main/java/org/zstack/kvm/tpm/TpmEncryptedResourceKeyBackend.java
- plugin/kvm/src/main/java/org/zstack/kvm/efi/CloneVmHostFileMsg.java
- plugin/kvm/src/main/java/org/zstack/kvm/efi/CloneVmHostFileReply.java
- plugin/kvm/src/main/java/org/zstack/kvm/tpm/CloneVmTpmReply.java
- plugin/kvm/src/main/java/org/zstack/kvm/tpm/CloneVmTpmMsg.java
plugin/kvm/src/main/java/org/zstack/kvm/efi/KvmSecureBootManager.java
Outdated
Show resolved
Hide resolved
MatheMatrix
force-pushed
the
sync/wenhao.zhang/zsv-ldap@@2
branch
from
e82f91e to
6af440d
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/zsv/V5.0.0__schema.sql`:
- Line 37: 在 VmHostBackupFileVO 表的唯一键定义中，把重复使用的约束名 `ukVmHostFileVO` 重命名为
`ukVmHostBackupFileVO`：定位 V5.0.0__schema.sql 中包含 `UNIQUE KEY `ukVmHostFileVO`
(`vmInstanceUuid`, `type`)` 的行并将键名改为
`ukVmHostBackupFileVO`，同时检查并更新同一文件或迁移脚本中任何引用该唯一键名称的地方以保持一致性。
- Line 41: Fix the typo in the column comment for `uuid`: update the comment
string that currently references "VmHostFileVO.uuid or VmHostFileBackupVO.uuid"
to the correct table/class name "VmHostBackupFileVO" so it reads
"'VmHostFileVO.uuid or VmHostBackupFileVO.uuid'"; the change should be made
where the `uuid` column definition appears (the line defining `uuid` char(32)
NOT NULL UNIQUE COMMENT ...).
In `@header/src/main/java/org/zstack/header/vm/additions/VmHostBackupFileVO.java`:
- Around line 26-36: The entity VmHostBackupFileVO is missing its primary key
and base resource inheritance; add a private String uuid field annotated with
`@Id` (and `@Column`) and make the class extend ResourceVO so it aligns with
VmHostBackupFileVO_ metamodel; ensure imports/annotations for
javax.persistence.Id (and any ResourceVO constructor/fields) are adjusted and
remove any conflicting fields so Hibernate recognizes uuid as the PK.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 66a4b10f-a47a-4dc0-904e-2a05f341c704
📥 Commits
Reviewing files that changed from the base of the PR and between e82f91ecc6a2dc0be6b27101e90515838ab13c3f and 6af440d.
⛔ Files ignored due to path filters (1)
conf/persistence.xmlis excluded by!**/*.xml
📒 Files selected for processing (4)
conf/db/zsv/V5.0.0__schema.sqlheader/src/main/java/org/zstack/header/vm/additions/VmHostBackupFileVO.javaheader/src/main/java/org/zstack/header/vm/additions/VmHostBackupFileVO_.javaheader/src/main/java/org/zstack/header/vm/additions/VmHostFileType.java
💤 Files with no reviewable changes (1)
- header/src/main/java/org/zstack/header/vm/additions/VmHostFileType.java
Comment on lines
+26
to
+36
| public class VmHostBackupFileVO { | ||
| @Column | ||
| @ForeignKey(parentEntityClass = VmInstanceEO.class, onDeleteAction = ForeignKey.ReferenceOption.CASCADE) | ||
| private String vmInstanceUuid; | ||
| @Column | ||
| @Enumerated(EnumType.STRING) | ||
| private VmHostFileType type; | ||
| @Column | ||
| private Timestamp createDate; | ||
| @Column | ||
| private Timestamp lastOpDate; |
There was a problem hiding this comment.
实体缺少主键字段 uuid 和 @Id 注解，且未继承 ResourceVO。
根据 SQL schema，VmHostBackupFileVO 表以 uuid 为主键。但当前实体类：
- 缺少
uuid字段和@Id注解 - 未继承
ResourceVO（而 metamodelVmHostBackupFileVO_继承了ResourceVO_）
这会导致 Hibernate 运行时失败。
🐛 建议修复
+import org.zstack.header.vo.ResourceVO;
+
`@Entity`
`@Table`
`@EntityGraph`(
friends = {
`@EntityGraph.Neighbour`(type = VmInstanceEO.class, myField = "vmInstanceUuid", targetField = "uuid"),
}
)
-public class VmHostBackupFileVO {
+public class VmHostBackupFileVO extends ResourceVO {
`@Column`
`@ForeignKey`(parentEntityClass = VmInstanceEO.class, onDeleteAction = ForeignKey.ReferenceOption.CASCADE)
private String vmInstanceUuid;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public class VmHostBackupFileVO { | |
| @Column | |
| @ForeignKey(parentEntityClass = VmInstanceEO.class, onDeleteAction = ForeignKey.ReferenceOption.CASCADE) | |
| private String vmInstanceUuid; | |
| @Column | |
| @Enumerated(EnumType.STRING) | |
| private VmHostFileType type; | |
| @Column | |
| private Timestamp createDate; | |
| @Column | |
| private Timestamp lastOpDate; | |
| public class VmHostBackupFileVO extends ResourceVO { | |
| `@Column` | |
| `@ForeignKey`(parentEntityClass = VmInstanceEO.class, onDeleteAction = ForeignKey.ReferenceOption.CASCADE) | |
| private String vmInstanceUuid; | |
| `@Column` | |
| `@Enumerated`(EnumType.STRING) | |
| private VmHostFileType type; | |
| `@Column` | |
| private Timestamp createDate; | |
| `@Column` | |
| private Timestamp lastOpDate; |
🤖 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/additions/VmHostBackupFileVO.java`
around lines 26 - 36, The entity VmHostBackupFileVO is missing its primary key
and base resource inheritance; add a private String uuid field annotated with
`@Id` (and `@Column`) and make the class extend ResourceVO so it aligns with
VmHostBackupFileVO_ metamodel; ensure imports/annotations for
javax.persistence.Id (and any ResourceVO constructor/fields) are adjusted and
remove any conflicting fields so Hibernate recognizes uuid as the PK.
a19754f to
a575eda
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
conf/db/zsv/V5.0.0__schema.sql (1)
41-41:⚠️ Potential issue | 🟡 Minor注释里的实体名仍然拼错了。
这里写的是
VmHostFileBackupVO.uuid，应为VmHostBackupFileVO.uuid，否则和实际表/实体名对不上。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@conf/db/zsv/V5.0.0__schema.sql` at line 41, The column comment for `uuid` currently references the wrong entity name "VmHostFileBackupVO.uuid"; update that comment to the correct entity "VmHostBackupFileVO.uuid" so the `uuid` column comment matches the actual entity/table name (look for the `uuid` char(32) NOT NULL UNIQUE COMMENT line and replace the mistyped entity name).
🤖 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/zsv/V5.0.0__schema.sql`:
- Around line 28-47: Add an idempotent migration that (1) conditionally CREATE
TABLE VmHostBackupFileVO if not exists (preserving PRIMARY KEY uuid, UNIQUE
ukVmHostFileVO and FK fkVmHostBackupFileVOVmInstanceVO), (2) MOVE existing
backup rows from the old VmHostFileVO into VmHostBackupFileVO mapping old types
(e.g. NvRamBackup -> NvRam, TpmStateBackup -> TpmState), preserving uuid,
vmInstanceUuid, createDate/lastOpDate and skipping duplicates, (3) update
VmHostFileContentVO foreign key: check information_schema for existing FK
pointing to VmHostFileVO and, if present, DROP that FK and ADD CONSTRAINT
fkVmHostFileContentVOResourceVO to reference ResourceVO(uuid) (do this
conditionally so repeated runs are safe), and (4) ensure all data rows in
VmHostFileContentVO whose uuid belonged to moved backup files remain consistent
(adjust uuids or copy records as needed) and wrap the whole migration in
transactions to be idempotent and safe for upgrades.
In `@plugin/kvm/src/main/java/org/zstack/kvm/efi/KvmSecureBootManager.java`:
- Around line 156-165: The logic is inverted: when RESET_TPM_AFTER_VM_CLONE /
resetTpm is true we should not copy the TPM state; change the condition so
VmHostFileType.TpmState is added only when resetTpm is false. In
KvmSecureBootManager, locate the block that reads msg.getResetTpm() (and uses
resourceConfigFacade.getResourceConfig(RESET_TPM_AFTER_VM_CLONE...)) and replace
the final if (resetTpm) { needClone.add(VmHostFileType.TpmState); } with a check
that adds TpmState only when !resetTpm.
- Around line 197-208: The code creates multiple SyncVmHostFilesFromHostContext
entries for the same host because it builds syncContexts from hostUuids list
that can contain duplicates; change KvmSecureBootManager so syncContexts
contains one context per unique hostUuid (e.g., dedupe hostUuids or use a
Map<String,SyncVmHostFilesFromHostContext> keyed by hostUuid), set
syncContext.hostUuid and syncContext.vmUuid when creating the single context,
and then when iterating over files use findOneOrNull (or map lookup) to populate
that single context’s file path fields before calling syncVmHostFilesFromHost();
this prevents duplicate/empty contexts for the same host.
In `@plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmManager.java`:
- Around line 328-353: persistTpmVO() currently creates the dstTpm record before
calling tpmKeyBackend.cloneEncryptedResourceKey(), leaving a half-initialized
TPM if clone fails; change this by either deferring persistence until
cloneEncryptedResourceKey() succeeds or adding cleanup in the failure path:
capture dstTpm.getUuid() and in the Completion.fail(ErrorCode) callback call the
vmTpmManager method that deletes or marks the dstTpm (e.g.,
vmTpmManager.deleteTpmVO/deleteOrMarkFailed by UUID) so the dangling record is
removed/flagged, then set the reply error and bus.reply(msg, reply).
In
`@plugin/kvm/src/main/java/org/zstack/kvm/tpm/TpmEncryptedResourceKeyBackend.java`:
- Around line 5-12: 为接口添加 Javadoc：在 TpmEncryptedResourceKeyBackend
接口上补充总体说明，在嵌套类 CloneEncryptedResourceKeyContext 上为每个字段（特别是
srcTpmUuid、dstTpmUuid、resetTpm）添加注释，明确 resetTpm 的语义是“在目标 TPM
上重置/重新生成密钥（true）还是仅复制源 TPM 上已有密钥（false）”，并在
cloneEncryptedResourceKey(CloneEncryptedResourceKeyContext context, Completion
completion) 方法上补充参数与行为说明（包括成功/失败回调 Completion 的语义和异常/错误处理约定）；同时移除接口方法多余的 public
修饰符（保持包级可见性约定）。
---
Duplicate comments:
In `@conf/db/zsv/V5.0.0__schema.sql`:
- Line 41: The column comment for `uuid` currently references the wrong entity
name "VmHostFileBackupVO.uuid"; update that comment to the correct entity
"VmHostBackupFileVO.uuid" so the `uuid` column comment matches the actual
entity/table name (look for the `uuid` char(32) NOT NULL UNIQUE COMMENT line and
replace the mistyped entity name).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 95cc198b-563f-456b-9263-13a18488f13e
⛔ Files ignored due to path filters (3)
conf/persistence.xmlis excluded by!**/*.xmlconf/springConfigXml/Kvm.xmlis excluded by!**/*.xmltest/src/test/resources/springConfigXml/Kvm.xmlis excluded by!**/*.xml
📒 Files selected for processing (13)
conf/db/zsv/V5.0.0__schema.sqlheader/src/main/java/org/zstack/header/vm/VmInstanceConstant.javaheader/src/main/java/org/zstack/header/vm/additions/VmHostBackupFileVO.javaheader/src/main/java/org/zstack/header/vm/additions/VmHostBackupFileVO_.javaheader/src/main/java/org/zstack/header/vm/additions/VmHostFileType.javaplugin/kvm/src/main/java/org/zstack/kvm/efi/CloneVmHostFileMsg.javaplugin/kvm/src/main/java/org/zstack/kvm/efi/CloneVmHostFileReply.javaplugin/kvm/src/main/java/org/zstack/kvm/efi/KvmSecureBootManager.javaplugin/kvm/src/main/java/org/zstack/kvm/tpm/CloneVmTpmMsg.javaplugin/kvm/src/main/java/org/zstack/kvm/tpm/CloneVmTpmReply.javaplugin/kvm/src/main/java/org/zstack/kvm/tpm/DummyTpmEncryptedResourceKeyBackend.javaplugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmManager.javaplugin/kvm/src/main/java/org/zstack/kvm/tpm/TpmEncryptedResourceKeyBackend.java
💤 Files with no reviewable changes (1)
- header/src/main/java/org/zstack/header/vm/additions/VmHostFileType.java
🚧 Files skipped from review as they are similar to previous changes (1)
- header/src/main/java/org/zstack/header/vm/additions/VmHostBackupFileVO_.java
Comment on lines
+28
to
+47
| CREATE TABLE IF NOT EXISTS `zstack`.`VmHostBackupFileVO` ( | ||
| `uuid` char(32) NOT NULL UNIQUE, | ||
| `vmInstanceUuid` char(32) NOT NULL, | ||
| `type` varchar(64) NOT NULL COMMENT 'NvRam, TpmState', | ||
| `lastOpDate` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, | ||
| `createDate` timestamp NOT NULL DEFAULT '1999-12-31 23:59:59', | ||
| PRIMARY KEY (`uuid`), | ||
| INDEX `idxVmHostBackupFileVOVmInstanceUuid` (`vmInstanceUuid`), | ||
| CONSTRAINT `fkVmHostBackupFileVOVmInstanceVO` FOREIGN KEY (`vmInstanceUuid`) REFERENCES `VmInstanceEO` (`uuid`) ON DELETE CASCADE, | ||
| UNIQUE KEY `ukVmHostFileVO` (`vmInstanceUuid`, `type`) | ||
| ) ENGINE=InnoDB DEFAULT CHARSET=utf8; | ||
|
|
||
| CREATE TABLE IF NOT EXISTS `zstack`.`VmHostFileContentVO` ( | ||
| `uuid` char(32) NOT NULL UNIQUE COMMENT 'VmHostFileVO.uuid or VmHostFileBackupVO.uuid', | ||
| `content` MEDIUMBLOB DEFAULT NULL, | ||
| `format` varchar(64) NOT NULL COMMENT 'Raw, TarballGzip', | ||
| `lastOpDate` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, | ||
| `createDate` timestamp NOT NULL DEFAULT '1999-12-31 23:59:59', | ||
| PRIMARY KEY (`uuid`), | ||
| CONSTRAINT `fkVmHostFileContentVOVmHostFileVO` FOREIGN KEY (`uuid`) REFERENCES `VmHostFileVO` (`uuid`) ON DELETE CASCADE | ||
| CONSTRAINT `fkVmHostFileContentVOResourceVO` FOREIGN KEY (`uuid`) REFERENCES `ResourceVO` (`uuid`) ON DELETE CASCADE |
There was a problem hiding this comment.
缺少升级迁移，老库上的备份文件会卡在旧模型里。
这里新增了 VmHostBackupFileVO，同时把 VmHostFileContentVO 的关联语义切到了 ResourceVO，但当前写法只会影响全新库。对已升级环境，已存在的 VmHostFileContentVO 不会被重建，外键仍然指向旧的 VmHostFileVO；另外旧表里的 NvRamBackup/TpmStateBackup 历史记录也没有迁到新表。结果就是升级后备份文件元数据/内容可能一部分还留在旧模型里，新的 clone/TPM 路径会出现查不到或插入失败。
建议补一段幂等迁移：条件式创建新表、把旧 VmHostFileVO 中的 backup 类型搬到 VmHostBackupFileVO（并做类型映射）、再按 information_schema 条件式重建 VmHostFileContentVO 的外键到 ResourceVO。
As per coding guidelines, **/*.sql: Review the SQL code, make sure has no errors and confirm that: Upgrading scene has been carefully handled.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@conf/db/zsv/V5.0.0__schema.sql` around lines 28 - 47, Add an idempotent
migration that (1) conditionally CREATE TABLE VmHostBackupFileVO if not exists
(preserving PRIMARY KEY uuid, UNIQUE ukVmHostFileVO and FK
fkVmHostBackupFileVOVmInstanceVO), (2) MOVE existing backup rows from the old
VmHostFileVO into VmHostBackupFileVO mapping old types (e.g. NvRamBackup ->
NvRam, TpmStateBackup -> TpmState), preserving uuid, vmInstanceUuid,
createDate/lastOpDate and skipping duplicates, (3) update VmHostFileContentVO
foreign key: check information_schema for existing FK pointing to VmHostFileVO
and, if present, DROP that FK and ADD CONSTRAINT fkVmHostFileContentVOResourceVO
to reference ResourceVO(uuid) (do this conditionally so repeated runs are safe),
and (4) ensure all data rows in VmHostFileContentVO whose uuid belonged to moved
backup files remain consistent (adjust uuids or copy records as needed) and wrap
the whole migration in transactions to be idempotent and safe for upgrades.
plugin/kvm/src/main/java/org/zstack/kvm/efi/KvmSecureBootManager.java
Outdated
Show resolved
Hide resolved
plugin/kvm/src/main/java/org/zstack/kvm/efi/KvmSecureBootManager.java
Outdated
Show resolved
Hide resolved
plugin/kvm/src/main/java/org/zstack/kvm/tpm/TpmEncryptedResourceKeyBackend.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
plugin/kvm/src/main/java/org/zstack/kvm/efi/KvmSecureBootManager.java (2)
156-165:⚠️ Potential issue | 🟠 Major
resetTpm的分支方向写反了。按
RESET_TPM_AFTER_VM_CLONE/msg.getResetTpm()的命名，true表示“克隆后重置 TPM”。Line 163-165 却在true时把TpmState加入复制列表，这会把源 VM 的 TPM 状态带到目标 VM；这里只应在resetTpm == false时复制TpmState。🔧 建议修改
- if (resetTpm) { + if (!resetTpm) { needClone.add(VmHostFileType.TpmState); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/kvm/src/main/java/org/zstack/kvm/efi/KvmSecureBootManager.java` around lines 156 - 165, In KvmSecureBootManager.java the branch using resetTpm (computed from msg.getResetTpm() or RESET_TPM_AFTER_VM_CLONE via resourceConfigFacade and resourceConfig.getResourceConfigValue(...)) is inverted: currently when resetTpm is true the code adds VmHostFileType.TpmState to needClone, which incorrectly preserves TPM state; change the condition to add VmHostFileType.TpmState only when resetTpm is false (i.e., if (!resetTpm) needClone.add(VmHostFileType.TpmState)) so TPM state is copied only when not resetting TPM after clone.
197-208:⚠️ Potential issue | 🟠 Major同一个 host 会生成多条同步上下文。
当
files同时包含NvRam和TpmState且它们都在同一台 host 上时，Line 197-203 会插入多个相同hostUuid的SyncVmHostFilesFromHostContext。Line 206-208 的匹配因此变成多匹配的不确定行为，并且至少会留下一个空 context 进入后续同步。这里应先按hostUuid去重，再回填路径。🔧 建议修改
- List<String> hostUuids = transform(files, VmHostFileVO::getHostUuid); - for (String hostUuid : hostUuids) { - SyncVmHostFilesFromHostContext syncContext = new SyncVmHostFilesFromHostContext(); - syncContext.hostUuid = hostUuid; - syncContext.vmUuid = msg.getSrcVmUuid(); - syncContexts.add(syncContext); - } + Map<String, SyncVmHostFilesFromHostContext> contextMap = new java.util.LinkedHashMap<>(); + for (VmHostFileVO file : files) { + contextMap.computeIfAbsent(file.getHostUuid(), hostUuid -> { + SyncVmHostFilesFromHostContext syncContext = new SyncVmHostFilesFromHostContext(); + syncContext.hostUuid = hostUuid; + syncContext.vmUuid = msg.getSrcVmUuid(); + return syncContext; + }); + } + syncContexts.addAll(contextMap.values()); for (VmHostFileVO file : files) { - SyncVmHostFilesFromHostContext syncContext = findOneOrNull(syncContexts, - item -> file.getHostUuid().equals(item.hostUuid)); + SyncVmHostFilesFromHostContext syncContext = contextMap.get(file.getHostUuid()); DebugUtils.Assert(syncContext != null, "syncContext must be not null"); if (file.getType() == VmHostFileType.NvRam) { syncContext.nvRamPath = file.getPath();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/kvm/src/main/java/org/zstack/kvm/efi/KvmSecureBootManager.java` around lines 197 - 208, The code creates multiple SyncVmHostFilesFromHostContext entries for the same host when files (VmHostFileVO) contain multiple types; deduplicate host UUIDs before constructing contexts: derive a unique set from transform(files, VmHostFileVO::getHostUuid) (or use a Map<String,SyncVmHostFilesFromHostContext> keyed by hostUuid), create one SyncVmHostFilesFromHostContext per unique hostUuid with vmUuid = msg.getSrcVmUuid(), then when iterating files use that map/findOneOrNull to backfill paths so each file matches exactly one context and no empty/duplicate contexts remain.conf/db/zsv/V5.0.0__schema.sql (1)
28-47:⚠️ Potential issue | 🟠 Major这组 DDL 只覆盖新库，升级库还会停在旧模型。
Line 28-47 只有
CREATE TABLE IF NOT EXISTS，不会把历史VmHostFileVO里的NvRamBackup/TpmStateBackup迁到VmHostBackupFileVO，也不会把已存在VmHostFileContentVO的外键从VmHostFileVO切到ResourceVO。升级环境里 clone/TPM 仍会读到旧数据或旧约束；这里还需要补一段基于information_schema的幂等迁移脚本。 As per coding guidelines,**/*.sql: Review the SQL code, make sure has no errors and confirm that: Upgrading scene has been carefully handled.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@conf/db/zsv/V5.0.0__schema.sql` around lines 28 - 47, 当前 SQL 仅在新库建表但未处理升级路径：需要在迁移脚本中添加基于 information_schema 的幂等迁移步骤，先检测旧表/列/约束（VmHostFileVO、VmHostFileContentVO、fk 指向 VmHostFileVO 等）是否存在，再把 VmHostFileVO 中 type 为 'NvRamBackup'/'TpmStateBackup' 的行迁移到新表 VmHostBackupFileVO（保持 uuid、vmInstanceUuid、type、createDate、lastOpDate），并为可能存在的 VmHostFileContentVO 行将外键从 VmHostFileVO.uuid 切换到 ResourceVO.uuid：具体操作包括 1) 在 information_schema 确认目标表/列/约束不存在后才创建 VmHostBackupFileVO/VmHostFileContentVO 的新约束；2) 以事务做数据复制（INSERT ... SELECT 或等价手段），确保不重复插入（按 uuid 或 ukVmHostBackupFileVO 唯一键去重）；3) 在确认数据复制成功并无孤立引用后，安全地删除旧外键并建立新外键指向 ResourceVO；4) 全部步骤应幂等（存在即跳过）并记录可重试的错误处理。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugin/kvm/src/main/java/org/zstack/kvm/efi/KvmSecureBootManager.java`:
- Around line 267-289: The code currently creates and adds a VmHostBackupFileVO
to filesNeedPersists before verifying the corresponding VmHostFileContentVO
exists, which can leave orphaned backup metadata and block the unique
(vmInstanceUuid,type) key; change the flow in the loop over
msg.getDstVmUuidList() and filesAfterSyncing so you first locate srcContent via
findOneOrNull(contents, ...) and if srcContent is non-null then create and add
the VmHostBackupFileVO and its VmHostFileContentVO to filesNeedPersists and
contentsNeedPersists respectively (alternatively implement an upsert on
VmHostBackupFileVO when adding to filesNeedPersists so missing content can be
filled later); update references in this block (VmHostBackupFileVO,
VmHostFileContentVO, filesAfterSyncing, contentsNeedPersists, filesNeedPersists)
accordingly.
---
Duplicate comments:
In `@conf/db/zsv/V5.0.0__schema.sql`:
- Around line 28-47: 当前 SQL 仅在新库建表但未处理升级路径：需要在迁移脚本中添加基于 information_schema
的幂等迁移步骤，先检测旧表/列/约束（VmHostFileVO、VmHostFileContentVO、fk 指向 VmHostFileVO 等）是否存在，再把
VmHostFileVO 中 type 为 'NvRamBackup'/'TpmStateBackup' 的行迁移到新表
VmHostBackupFileVO（保持 uuid、vmInstanceUuid、type、createDate、lastOpDate），并为可能存在的
VmHostFileContentVO 行将外键从 VmHostFileVO.uuid 切换到 ResourceVO.uuid：具体操作包括 1) 在
information_schema 确认目标表/列/约束不存在后才创建 VmHostBackupFileVO/VmHostFileContentVO
的新约束；2) 以事务做数据复制（INSERT ... SELECT 或等价手段），确保不重复插入（按 uuid 或 ukVmHostBackupFileVO
唯一键去重）；3) 在确认数据复制成功并无孤立引用后，安全地删除旧外键并建立新外键指向 ResourceVO；4)
全部步骤应幂等（存在即跳过）并记录可重试的错误处理。
In `@plugin/kvm/src/main/java/org/zstack/kvm/efi/KvmSecureBootManager.java`:
- Around line 156-165: In KvmSecureBootManager.java the branch using resetTpm
(computed from msg.getResetTpm() or RESET_TPM_AFTER_VM_CLONE via
resourceConfigFacade and resourceConfig.getResourceConfigValue(...)) is
inverted: currently when resetTpm is true the code adds VmHostFileType.TpmState
to needClone, which incorrectly preserves TPM state; change the condition to add
VmHostFileType.TpmState only when resetTpm is false (i.e., if (!resetTpm)
needClone.add(VmHostFileType.TpmState)) so TPM state is copied only when not
resetting TPM after clone.
- Around line 197-208: The code creates multiple SyncVmHostFilesFromHostContext
entries for the same host when files (VmHostFileVO) contain multiple types;
deduplicate host UUIDs before constructing contexts: derive a unique set from
transform(files, VmHostFileVO::getHostUuid) (or use a
Map<String,SyncVmHostFilesFromHostContext> keyed by hostUuid), create one
SyncVmHostFilesFromHostContext per unique hostUuid with vmUuid =
msg.getSrcVmUuid(), then when iterating files use that map/findOneOrNull to
backfill paths so each file matches exactly one context and no empty/duplicate
contexts remain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ad0caa2c-ee66-49d7-942f-9fafa3dca22d
⛔ Files ignored due to path filters (3)
conf/persistence.xmlis excluded by!**/*.xmlconf/springConfigXml/Kvm.xmlis excluded by!**/*.xmltest/src/test/resources/springConfigXml/Kvm.xmlis excluded by!**/*.xml
📒 Files selected for processing (13)
conf/db/zsv/V5.0.0__schema.sqlheader/src/main/java/org/zstack/header/vm/VmInstanceConstant.javaheader/src/main/java/org/zstack/header/vm/additions/VmHostBackupFileVO.javaheader/src/main/java/org/zstack/header/vm/additions/VmHostBackupFileVO_.javaheader/src/main/java/org/zstack/header/vm/additions/VmHostFileType.javaplugin/kvm/src/main/java/org/zstack/kvm/efi/CloneVmHostFileMsg.javaplugin/kvm/src/main/java/org/zstack/kvm/efi/CloneVmHostFileReply.javaplugin/kvm/src/main/java/org/zstack/kvm/efi/KvmSecureBootManager.javaplugin/kvm/src/main/java/org/zstack/kvm/tpm/CloneVmTpmMsg.javaplugin/kvm/src/main/java/org/zstack/kvm/tpm/CloneVmTpmReply.javaplugin/kvm/src/main/java/org/zstack/kvm/tpm/DummyTpmEncryptedResourceKeyBackend.javaplugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmManager.javaplugin/kvm/src/main/java/org/zstack/kvm/tpm/TpmEncryptedResourceKeyBackend.java
💤 Files with no reviewable changes (1)
- header/src/main/java/org/zstack/header/vm/additions/VmHostFileType.java
🚧 Files skipped from review as they are similar to previous changes (5)
- plugin/kvm/src/main/java/org/zstack/kvm/tpm/DummyTpmEncryptedResourceKeyBackend.java
- plugin/kvm/src/main/java/org/zstack/kvm/efi/CloneVmHostFileReply.java
- header/src/main/java/org/zstack/header/vm/additions/VmHostBackupFileVO_.java
- plugin/kvm/src/main/java/org/zstack/kvm/tpm/CloneVmTpmMsg.java
- header/src/main/java/org/zstack/header/vm/VmInstanceConstant.java
MatheMatrix
force-pushed
the
sync/wenhao.zhang/zsv-ldap@@2
branch
4 times, most recently
from
efaafc2 to
11c84b5
Compare
Resolves: ZSV-11439 Related: ZSV-11310 Change-Id: I63706e6a7a75616366716574646a677a6e646276
MatheMatrix
force-pushed
the
sync/wenhao.zhang/zsv-ldap@@2
branch
from
11c84b5 to
d2d2ea6
Compare
Collaborator
|
Comment from ye.zou: Code Review: ZSV-11439 — TPM VM Clone (zstack)Verdict: 🚫 BLOCK 🔴 Critical1. Backup content 写了但永远不会被消费 —
|
Collaborator
|
Comment from wenhao.zhang: 这个 CloneVmHostFileMsg 还能消息重试？ |
Owner
|
Comment from wenhao.zhang:
这个问题刚刚确定，因为失败会导致 env.delete() 扫描发现表残留。问题已经修复 |
Collaborator
Author
|
Comment from wenhao.zhang: VmHostBackupFileVO 后面一个补丁 （支持快照）还需要把 vmInstanceUuid 改成 resourceUuid （因为会支持快照组 uuid） |
MatheMatrix
force-pushed
the
sync/wenhao.zhang/zsv-ldap@@2
branch
2 times, most recently
from
604eaaa to
b326400
Compare
added 3 commits
March 13, 2026 16:54
* Added support for cross-VM cloning of host files (NvRam, TpmState) and TPM encryption key cloning * KvmSecureBootManager transitioned from a Component to a service * Introducing new message/response types, workflows, and TPM key backend interfaces along with their virtual implementations Resolves: ZSV-11439 Related: ZSV-11310 Change-Id: I796b77716b6c64657469697a7a797a7268636e6e
Resolves: ZSV-11439 Related: ZSV-11310 Change-Id: I626c6775707a67657570736778766d6b75736570
Resolves: ZSV-11439 Related: ZSV-11310 Change-Id: I68696b7776656f78677679686370707a70616665
MatheMatrix
force-pushed
the
sync/wenhao.zhang/zsv-ldap@@2
branch
from
b326400 to
30f42a7
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
4 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-11439
Related: ZSV-11310
Change-Id: I796b77716b6c64657469697a7a797a7268636e6e
sync from gitlab !9333