<fix>[kvm]: backup TPM state before virsh undefine#3852
Open
zstack-robot-1 wants to merge 1 commit intozsv_5.0.0from
Open
<fix>[kvm]: backup TPM state before virsh undefine#3852zstack-robot-1 wants to merge 1 commit intozsv_5.0.0from
zstack-robot-1 wants to merge 1 commit intozsv_5.0.0from
Conversation
virsh undefine deletes /var/lib/libvirt/swtpm/{uuid}/
before releaseVmResource() can sync TPM state to DB.
Add tpmBackupJobs to StopVmCmd so the agent backs up
TPM files between virsh shutdown and virsh undefine.
On sync, if the primary path read fails, fall back to
the backup path, merge with NvRam results, persist to
DB, then clean up the backup directory.
KvmSecureBootExtensions now implements
KVMStopVmExtensionPoint to set backup jobs and
fallback folder. KvmSecureBootManager.handle(
SyncVmHostFilesFromHostMsg) is refactored into a
SimpleFlowChain with named steps for readability.
Related: ZSV-11310
Resolves: ZSV-12030
Change-Id: I66787076646270756c7971707561687462627772
Walkthrough该更改为KVM插件添加TPM状态备份和故障转移功能。通过在虚拟机停止前备份TPM数据到快照支持的位置，并在主路径读取失败时支持从备份位置回退读取，增强了安全启动和TPM状态管理的可靠性。 Changes
Sequence Diagram(s)sequenceDiagram
participant VM as 虚拟机<br/>实例
participant Ext as SecureBootExt<br/>扩展点
participant Agent as KVMAgent<br/>命令
participant Manager as SecureBootMgr<br/>管理器
participant Host as KVM主机
participant DB as 数据库/<br/>存储
rect rgba(200, 150, 255, 0.5)
Note over VM,DB: TPM备份阶段（虚拟机停止前）
VM->>Ext: beforeStopVmOnKvm()
Ext->>DB: 查询主机上是否存在TPM状态
DB-->>Ext: VmHostFileVO(TpmState)
Ext->>Ext: 构建TPM备份路径
Ext->>Agent: 配置StopVmCmd.tpmBackupJobs
Agent-->>Ext: 命令准备完毕
end
rect rgba(150, 200, 255, 0.5)
Note over VM,Host: TPM备份执行阶段
Agent->>Host: 执行停止命令+备份TPM
Host->>Host: 将TPM复制到快照备份位置
Host-->>Agent: 执行成功
Agent->>Ext: stopVmOnKvmSuccess()
end
rect rgba(200, 255, 150, 0.5)
Note over Manager,Host: TPM同步与故障转移阶段
Manager->>Host: 读取主路径上的NvRam+TPM
Host-->>Manager: 返回数据或失败
alt 主路径失败且存在备份路径
Manager->>Host: 尝试从备份路径读取TPM
Host-->>Manager: 返回备份TPM数据
Manager->>Manager: 合并：非TPM文件用主路径<br/>TPM文件用备份路径
Manager->>Manager: 重写备份TPM路径为规范路径
else 主路径成功
Manager->>Manager: 直接使用主路径数据
end
Manager->>DB: 持久化主机文件记录
Manager->>Host: 清理备份TPM(异步/最佳努力)
Host-->>Manager: 清理完成或忽略失败
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 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 505-513: The cleanup currently runs only when ctx.tpmFallbackUsed
is true, leaving stale TPM backups when the normal path succeeded; change the
condition in the "cleanup-tpm-backup" trigger to check for the presence of the
fallback backup folder (use msg.getTpmStateFallbackFolder() /
ctx.tpmStateFallbackFolder or a host-side existence check) instead of
ctx.tpmFallbackUsed, and call cleanupTpmBackupOnHost(msg.getHostUuid(),
msg.getVmUuid(), msg.getTpmStateFallbackFolder()) only when that fallback folder
actually exists.
🪄 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: 787a08ef-c5ef-42d3-8339-11de2d47d516
⛔ 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 (4)
plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.javaplugin/kvm/src/main/java/org/zstack/kvm/efi/KvmSecureBootExtensions.javaplugin/kvm/src/main/java/org/zstack/kvm/efi/KvmSecureBootManager.javaplugin/kvm/src/main/java/org/zstack/kvm/vmfiles/message/SyncVmHostFilesFromHostMsg.java
Comment on lines
+505
to
+513
| .then("cleanup-tpm-backup", trigger -> { | ||
| if (!ctx.tpmFallbackUsed) { | ||
| trigger.next(); | ||
| return; | ||
| } | ||
|
|
||
| cleanupTpmBackupOnHost(msg.getHostUuid(), msg.getVmUuid(), | ||
| msg.getTpmStateFallbackFolder()); | ||
| trigger.next(); |
There was a problem hiding this comment.
这里只在真正走了 fallback 后才清理备份目录，会留下陈旧 TPM 备份。
如果本次主路径读取成功，这个备份目录会一直留在 host 上。下一次 stop/undefine 流程一旦新备份没刷进去、但 fallback 又被启用，就可能把上一次停机留下的旧 TPM 状态重新写回 DB。
清理条件应该跟“是否存在 fallback 备份目录”绑定，而不是跟“这次是否实际用了 fallback”绑定。
可选修正方向
.then("cleanup-tpm-backup", trigger -> {
- if (!ctx.tpmFallbackUsed) {
+ if (msg.getTpmStateFallbackFolder() == null) {
trigger.next();
return;
}
cleanupTpmBackupOnHost(msg.getHostUuid(), msg.getVmUuid(),
msg.getTpmStateFallbackFolder());
trigger.next();
})🤖 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 505 - 513, The cleanup currently runs only when ctx.tpmFallbackUsed is
true, leaving stale TPM backups when the normal path succeeded; change the
condition in the "cleanup-tpm-backup" trigger to check for the presence of the
fallback backup folder (use msg.getTpmStateFallbackFolder() /
ctx.tpmStateFallbackFolder or a host-side existence check) instead of
ctx.tpmFallbackUsed, and call cleanupTpmBackupOnHost(msg.getHostUuid(),
msg.getVmUuid(), msg.getTpmStateFallbackFolder()) only when that fallback folder
actually exists.
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
1 participant
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.
virsh undefine deletes /var/lib/libvirt/swtpm/{uuid}/
before releaseVmResource() can sync TPM state to DB.
Add tpmBackupJobs to StopVmCmd so the agent backs up
TPM files between virsh shutdown and virsh undefine.
On sync, if the primary path read fails, fall back to
the backup path, merge with NvRam results, persist to
DB, then clean up the backup directory.
KvmSecureBootExtensions now implements
KVMStopVmExtensionPoint to set backup jobs and
fallback folder. KvmSecureBootManager.handle(
SyncVmHostFilesFromHostMsg) is refactored into a
SimpleFlowChain with named steps for readability.
Related: ZSV-11310
Resolves: ZSV-12030
Change-Id: I66787076646270756c7971707561687462627772
sync from gitlab !9724