<feature>[storage]: ZSV-12664 support ZBS volume encryption conversion - #4570
Closed
ZStack-Robot wants to merge 1 commit into
Closed
Conversation
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in:7 seconds Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (16)
Walkthrough新增 ZBS/CBD 卷明文与 LUKS 加密双向转换能力，包含消息路由、目标卷创建、KVM 转换、快照门禁、容量回填、失败清理及集成测试。 ChangesZBS 卷加密转换
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ExternalPrimaryStorage
participant ZbsStorageController
participant ZbsVolumeEncryptionConverter
participant ZbsVolumeEncryptionBackend
participant KVMHost
ExternalPrimaryStorage->>ZbsStorageController: 转发转换消息
ZbsStorageController->>ZbsVolumeEncryptionConverter: 执行转换
ZbsVolumeEncryptionConverter->>ZbsVolumeEncryptionBackend: 创建目标卷
ZbsVolumeEncryptionConverter->>KVMHost: 执行 LUKS 转换
KVMHost-->>ZbsVolumeEncryptionConverter: 返回实际容量或错误
ZbsVolumeEncryptionConverter->>ZbsVolumeEncryptionBackend: 失败时删除目标卷
ZbsVolumeEncryptionConverter-->>ExternalPrimaryStorage: 返回转换结果
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
docs/superpowers/plans/2026-07-15-zbs-change-volume-encryption-implementation.md (1)
37-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win不要把开发者本机路径写进执行步骤。
命令硬编码
/Users/mizar/...worktree 和固定/tmp/...目录，其他开发者或 CI 无法直接复现，且可能误用错误分支。请统一使用环境变量或仓库相对路径，并让groovy_test与最终compile复用同一组变量。Also applies to: 131-135, 194-202, 271-288, 376-405
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/plans/2026-07-15-zbs-change-volume-encryption-implementation.md` around lines 37 - 40, 移除执行步骤中的开发者本机 worktree 路径和固定临时目录，改用仓库相对路径或统一定义的环境变量；让 groovy_test、最终 compile 及相关命令复用同一组路径变量，并同步更新所列其他步骤，确保开发者和 CI 可直接复现且使用当前仓库版本。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@header/src/main/java/org/zstack/header/storage/addon/primary/PrimaryStorageControllerSvc.java`:
- Around line 39-40: 为 PrimaryStorageControllerSvc.convertVolumeEncryption 和
ZbsVolumeEncryptionExtensionPoint 中对应的新增接口方法补充有效
Javadoc；前者说明主存储控制器的转换请求、ReturnValueCompletion 回调结果及失败行为，后者说明 ZBS
后端的输入约束和异步回调语义。涉及文件
header/src/main/java/org/zstack/header/storage/addon/primary/PrimaryStorageControllerSvc.java（39-40）和
header/src/main/java/org/zstack/header/storage/addon/primary/ZbsVolumeEncryptionExtensionPoint.java（28-29），分别直接添加方法契约文档，保持现有接口签名不变。
In
`@header/src/main/java/org/zstack/header/storage/addon/primary/ZbsVolumeEncryptionBackend.java`:
- Around line 24-29: 为接口中的 validateConversionPaths、createConversionTarget 和
deleteConversionTarget 方法补充有效 Javadoc：说明源路径与目标路径的约束，描述 targetEncrypted
对分配大小的影响，明确 createConversionTarget 回调返回路径的语义，并说明 deleteConversionTarget
仅在目标成功删除时触发成功回调。
In
`@plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsVolumeEncryptionBackendImpl.java`:
- Around line 223-234: Update the cleanup path in the conversion error branch of
ZbsVolumeEncryptionBackendImpl to pass createdInstallPath to
deleteConversionTarget instead of targetInstallPath, ensuring the unexpectedly
created volume is removed without affecting data at the expected path.
---
Nitpick comments:
In
`@docs/superpowers/plans/2026-07-15-zbs-change-volume-encryption-implementation.md`:
- Around line 37-40: 移除执行步骤中的开发者本机 worktree 路径和固定临时目录，改用仓库相对路径或统一定义的环境变量；让
groovy_test、最终 compile 及相关命令复用同一组路径变量，并同步更新所列其他步骤，确保开发者和 CI 可直接复现且使用当前仓库版本。
🪄 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: fb301811-c9b3-430c-b8b6-1915cf66ab73
⛔ Files ignored due to path filters (1)
conf/springConfigXml/VolumeManager.xmlis excluded by!**/*.xml
📒 Files selected for processing (15)
docs/superpowers/plans/2026-07-15-zbs-change-volume-encryption-implementation.mddocs/superpowers/specs/2026-07-15-zbs-change-volume-encryption-design.mdheader/src/main/java/org/zstack/header/storage/addon/primary/PrimaryStorageControllerSvc.javaheader/src/main/java/org/zstack/header/storage/addon/primary/ZbsVolumeEncryptionBackend.javaheader/src/main/java/org/zstack/header/storage/addon/primary/ZbsVolumeEncryptionExtensionPoint.javaplugin/expon/src/main/java/org/zstack/expon/ExponStorageController.javaplugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.javaplugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsVolumeEncryptionBackendImpl.javastorage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorage.javastorage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionCommands.javastorage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionConstants.javastorage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionConverter.javastorage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionExtension.javastorage/src/main/java/org/zstack/storage/volume/VolumeBase.javatest/src/test/groovy/org/zstack/test/integration/storage/primary/addon/zbs/ZbsVolumeEncryptionCase.groovy
Comment on lines
+39
to
+40
| void convertVolumeEncryption(ConvertVolumeEncryptionOnPrimaryStorageMsg msg, | ||
| ReturnValueCompletion<ConvertVolumeEncryptionOnPrimaryStorageReply> completion); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
为两个新增接口方法补充有效 Javadoc。
这两个新增接口方法共同缺少契约说明，违反接口方法文档规范：
header/src/main/java/org/zstack/header/storage/addon/primary/PrimaryStorageControllerSvc.java#L39-L40：说明主存储控制器的转换请求、回调结果及失败行为。header/src/main/java/org/zstack/header/storage/addon/primary/ZbsVolumeEncryptionExtensionPoint.java#L28-L29：说明 ZBS 后端转换扩展点的输入约束和异步回调语义。
根据路径规范，接口方法必须配有有效的 Javadoc。
📍 Affects 2 files
header/src/main/java/org/zstack/header/storage/addon/primary/PrimaryStorageControllerSvc.java#L39-L40(this comment)header/src/main/java/org/zstack/header/storage/addon/primary/ZbsVolumeEncryptionExtensionPoint.java#L28-L29
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@header/src/main/java/org/zstack/header/storage/addon/primary/PrimaryStorageControllerSvc.java`
around lines 39 - 40, 为 PrimaryStorageControllerSvc.convertVolumeEncryption 和
ZbsVolumeEncryptionExtensionPoint 中对应的新增接口方法补充有效
Javadoc；前者说明主存储控制器的转换请求、ReturnValueCompletion 回调结果及失败行为，后者说明 ZBS
后端的输入约束和异步回调语义。涉及文件
header/src/main/java/org/zstack/header/storage/addon/primary/PrimaryStorageControllerSvc.java（39-40）和
header/src/main/java/org/zstack/header/storage/addon/primary/ZbsVolumeEncryptionExtensionPoint.java（28-29），分别直接添加方法契约文档，保持现有接口签名不变。
Source: Path instructions
Comment on lines
+24
to
+29
| void validateConversionPaths(String sourceInstallPath, String targetInstallPath); | ||
| void createConversionTarget(String targetInstallPath, long virtualSize, boolean targetEncrypted, | ||
| ReturnValueCompletion<String> completion); | ||
| void deleteConversionTarget(String targetInstallPath, Completion completion); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
请为新增接口方法补充有效的 Javadoc。
至少应说明路径约束、targetEncrypted 对分配大小的影响、返回路径语义，以及删除回调的成功条件。
As per path instructions, “接口方法必须配有有效的 Javadoc 注释”。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@header/src/main/java/org/zstack/header/storage/addon/primary/ZbsVolumeEncryptionBackend.java`
around lines 24 - 29, 为接口中的 validateConversionPaths、createConversionTarget 和
deleteConversionTarget 方法补充有效 Javadoc：说明源路径与目标路径的约束，描述 targetEncrypted
对分配大小的影响，明确 createConversionTarget 回调返回路径的语义，并说明 deleteConversionTarget
仅在目标成功删除时触发成功回调。
Source: Path instructions
Comment on lines
+223
to
+234
| String createdInstallPath = returnValue.getInstallPath(); | ||
| if (targetInstallPath.equals(createdInstallPath)) { | ||
| completion.success(createdInstallPath); | ||
| return; | ||
| } | ||
| ErrorCode error = operr( | ||
| "ZBS volume encryption conversion target[%s] was created at unexpected path[%s]", | ||
| targetInstallPath, createdInstallPath); | ||
| String cleanupInstallPath = targetInstallPath; | ||
| try { | ||
| deleteConversionTarget(cleanupInstallPath, new Completion(completion) { |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
路径不匹配时应删除实际创建的目标。
当前清理 targetInstallPath，而不是 createdInstallPath。这会遗留意外创建的卷，并可能误删预期路径上已有的数据。
建议修改
String createdInstallPath = returnValue.getInstallPath();
...
-String cleanupInstallPath = targetInstallPath;+String cleanupInstallPath = createdInstallPath;📝 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
| StringcreatedInstallPath = returnValue.getInstallPath(); | |
| if (targetInstallPath.equals(createdInstallPath)) { | |
| completion.success(createdInstallPath); | |
| return; | |
| } | |
| ErrorCodeerror = operr( | |
| "ZBS volume encryption conversion target[%s] was created at unexpected path[%s]", | |
| targetInstallPath, createdInstallPath); | |
| StringcleanupInstallPath = targetInstallPath; | |
| try { | |
| deleteConversionTarget(cleanupInstallPath, newCompletion(completion) { | |
| StringcreatedInstallPath = returnValue.getInstallPath(); | |
| if (targetInstallPath.equals(createdInstallPath)) { | |
| completion.success(createdInstallPath); | |
| return; | |
| } | |
| ErrorCodeerror = operr( | |
| "ZBS volume encryption conversion target[%s] was created at unexpected path[%s]", | |
| targetInstallPath, createdInstallPath); | |
| StringcleanupInstallPath = createdInstallPath; | |
| try { | |
| deleteConversionTarget(cleanupInstallPath, newCompletion(completion) { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsVolumeEncryptionBackendImpl.java`
around lines 223 - 234, Update the cleanup path in the conversion error branch
of ZbsVolumeEncryptionBackendImpl to pass createdInstallPath to
deleteConversionTarget instead of targetInstallPath, ensuring the unexpectedly
created volume is removed without affecting data at the expected path.
MatheMatrixforce-pushed
the
sync/zstackio/codex/zbs-change-volume-encryption
branch
2 times, most recently
from
July 15, 2026 05:06
c47cbed to
b29ef3bCompare
Route ZBS conversion through a dedicated backend and KVM handler. Tests: cbok zsv groovy_test ZbsVolumeEncryptionCase Resolves: ZSV-12664 Change-Id: Ibd30f74dd17cb4dc817e57c713b0f32915487e7c
MatheMatrixforce-pushed
the
sync/zstackio/codex/zbs-change-volume-encryption
branch
from
July 15, 2026 08:00
b29ef3b to
2e62674Comparezstack-robot-2
deleted the
sync/zstackio/codex/zbs-change-volume-encryption
branch
July 15, 2026 08:09
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
Summary
为 ZBS/CBD 云盘增加明文与加密格式双向转换，转换语义参考 RBD；云盘存在主存储快照时拒绝转换。
Changes
Testing
Resolves: ZSV-12664
sync from gitlab !10538