<fix>[crypto]: Fix the issue where VMs fail to boot after deleting NKP and reimporting them.#3690
Open
zstack-robot-2 wants to merge 1 commit intofeature-zsv-5.0.0-vm-support-vtpm-and-secucebootfrom
Conversation
…P and reimporting them.
总览本次变更为 TPM 加密资源密钥管理添加了数据一致性修复机制。更新了 变更清单
序列图sequenceDiagram
participant VM as VM 启动
participant KTE as KvmTpmExtensions
participant Repair as KvmTpmEncryptedResourceKeyRefJdbcRepair
participant DB as 数据库
participant Backend as TpmEncryptedResourceKeyBackend
VM->>KTE: beforeStartVmOnKvm(tpmUuid)
KTE->>Repair: repairOrphanTpmKeyRefPlaceholders(tpmUuid)
Repair->>DB: DELETE 孤儿占位符行<br/>(providerUuid NOT NULL AND kekRef IS NULL)
DB-->>Repair: 返回影响行数
KTE->>DB: safeFindKeyProviderUuidByTpm(tpmUuid)
DB-->>KTE: 返回 providerUuid (可能为 null)
alt providerUuid 为空
KTE->>KTE: tryRebindKeyProviderByName(tpmUuid)
KTE->>DB: 按名称查询提供者
DB-->>KTE: 返回提供者信息
KTE->>Repair: applyProviderUuidOnRowWithKek(tpmUuid, providerUuid)
Repair->>DB: UPDATE 设置 providerUuid
DB-->>Repair: 返回影响行数
end
KTE->>Backend: attachKeyProviderToTpm(tpmUuid, providerUuid)
Backend->>DB: 确保 DB 一致性
DB-->>Backend: 完成
KTE-->>VM: 返回配置后的启动命令
代码审查工作量估计🎯 4 (复杂) | ⏱️ ~45 分钟 诗歌
🚥 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 docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.java (1)
425-432: 建议:使用更健壮的异常类型检查。当前通过类名后缀字符串匹配来检测
NonUniqueResultException,虽然能兼容不同 JPA 实现,但较为脆弱。♻️ 可选改进:显式检查已知异常类型
private static boolean isNonUniqueResultException(Throwable e) { for (Throwable t = e; t != null; t = t.getCause()) { - if (t.getClass().getName().endsWith("NonUniqueResultException")) { + if (t instanceof javax.persistence.NonUniqueResultException + || t.getClass().getName().equals("org.hibernate.NonUniqueResultException")) { return true; } } return false; }注:若项目明确仅使用 Hibernate,可直接
instanceof检查org.hibernate.NonUniqueResultException。🤖 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/KvmTpmExtensions.java` around lines 425 - 432, The current isNonUniqueResultException(Throwable e) uses fragile class-name suffix matching; update it to first explicitly check known exception classes (e.g., org.hibernate.NonUniqueResultException via instanceof if that dependency is present, and any other project-known NonUniqueResultException types) while still walking causes, and only fall back to the name-suffix string check if no explicit instanceof matches; keep the same Throwable cause traversal logic and preserve method signature so callers of isNonUniqueResultException behave identically.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.java`:
- Around line 425-432: The current isNonUniqueResultException(Throwable e) uses
fragile class-name suffix matching; update it to first explicitly check known
exception classes (e.g., org.hibernate.NonUniqueResultException via instanceof
if that dependency is present, and any other project-known
NonUniqueResultException types) while still walking causes, and only fall back
to the name-suffix string check if no explicit instanceof matches; keep the same
Throwable cause traversal logic and preserve method signature so callers of
isNonUniqueResultException behave identically.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: df691ea4-0010-4fb3-b8ed-10143545c926
⛔ Files ignored due to path filters (1)
conf/springConfigXml/Kvm.xmlis excluded by!**/*.xml
📒 Files selected for processing (3)
compute/src/main/java/org/zstack/compute/vm/devices/TpmEncryptedResourceKeyBackend.javaplugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmEncryptedResourceKeyRefJdbcRepair.javaplugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.java
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
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.
sync from gitlab !9552