<feature>[storage]: ZSV-12664 support ZBS volume encryption conversion#4571
Closed
MatheMatrix wants to merge 1 commit into
Closed
Conversation
Walkthrough新增 ZBS/CBD 卷加密双向转换流程，涵盖接口契约、目标卷生命周期、KVM 转换、密钥材料准备、CBD 路径处理、错误清理及端到端测试。 ChangesZBS 卷加密转换
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant API
participant ExternalPrimaryStorage
participant ZbsStorageController
participant ZbsVolumeEncryptionConverter
participant KVMHost
API->>ExternalPrimaryStorage: 提交卷加密转换
ExternalPrimaryStorage->>ZbsStorageController: 转发转换消息
ZbsStorageController->>ZbsVolumeEncryptionConverter: 执行转换
ZbsVolumeEncryptionConverter->>KVMHost: 发送LUKS转换命令
KVMHost-->>ZbsVolumeEncryptionConverter: 返回actualSize或转换结果
ZbsVolumeEncryptionConverter-->>API: 返回转换回复
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![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
🧹 Nitpick comments (3)
header/src/main/java/org/zstack/header/storage/addon/primary/ZbsVolumeEncryptionBackend.java (1)
24-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win请补充接口 Javadoc，并用枚举替代
targetEncrypted。这些方法需要说明路径所有权、创建结果和清理回调语义；建议将布尔参数替换为
ENCRYPTED/PLAIN枚举，避免调用方传反。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, 为 ZbsVolumeEncryptionBackend 接口的 validateConversionPaths、createConversionTarget 和 deleteConversionTarget 补充有效 Javadoc，说明路径所有权、创建结果及清理回调语义；将 createConversionTarget 的 targetEncrypted 布尔参数替换为表达 ENCRYPTED/PLAIN 的枚举类型，并同步更新相关调用方以使用该枚举。Source: Path instructions
storage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionMaterialFactory.java (1)
46-86: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win缓存密钥资源字段并将
kpUuid改为完整名称。建议使用
keyResourceType、keyResourceUuid和keyProviderUuid局部变量，避免重复读取spec并提升这段密钥选择逻辑的可审计性。As per path instructions, “不允许使用不必要的缩写”，命名应使用完整单词表达意图。
🤖 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 `@storage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionMaterialFactory.java` around lines 46 - 86, 在 prepareVolumeEncryption 中先将 spec 的加密资源类型、资源 UUID 和密钥提供者 UUID 缓存到 keyResourceType、keyResourceUuid、keyProviderUuid 局部变量，后续校验、上下文设置及错误信息统一使用这些变量；同时将 kpUuid 重命名为完整的 keyProviderUuid，避免不必要缩写并减少重复读取 spec。Source: Path instructions
test/src/test/groovy/org/zstack/test/integration/storage/primary/addon/zbs/ZbsVolumeEncryptionCase.groovy (1)
216-233: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win不要依赖缺失的 KVM handler 让密钥复用测试失败。
该用例当前以
outcome.error != null为预期，实际只证明材料准备发生在后续失败之前。请显式模拟 KVM 成功并断言命令中的encryptedDek与成功结果；或者直接测试材料工厂并重命名用例。🤖 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 `@test/src/test/groovy/org/zstack/test/integration/storage/primary/addon/zbs/ZbsVolumeEncryptionCase.groovy` around lines 216 - 233, 更新 ZbsVolumeEncryptionExtension.createEncryptedEmptyVolume 用例，显式 stub KVM handler 返回成功，避免通过后续缺失 handler 的失败来验证材料复用；在 success 回调中断言结果及命令中的 encryptedDek，并相应移除 outcome.error 非空断言，保留必要的材料与删除路径验证。
🤖 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
`@plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsVolumeEncryptionBackendImpl.java`:
- Around line 200-261: Update the path-mismatch cleanup in
createConversionTarget so cleanupInstallPath uses createdInstallPath, the actual
path returned by ZBS, instead of targetInstallPath. Preserve the existing error
reporting and completion behavior while ensuring deleteConversionTarget removes
the created orphan volume.
---
Nitpick comments:
In
`@header/src/main/java/org/zstack/header/storage/addon/primary/ZbsVolumeEncryptionBackend.java`:
- Around line 24-29: 为 ZbsVolumeEncryptionBackend 接口的
validateConversionPaths、createConversionTarget 和 deleteConversionTarget 补充有效
Javadoc，说明路径所有权、创建结果及清理回调语义；将 createConversionTarget 的 targetEncrypted 布尔参数替换为表达
ENCRYPTED/PLAIN 的枚举类型，并同步更新相关调用方以使用该枚举。
In
`@storage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionMaterialFactory.java`:
- Around line 46-86: 在 prepareVolumeEncryption 中先将 spec 的加密资源类型、资源 UUID 和密钥提供者
UUID 缓存到 keyResourceType、keyResourceUuid、keyProviderUuid
局部变量，后续校验、上下文设置及错误信息统一使用这些变量；同时将 kpUuid 重命名为完整的 keyProviderUuid，避免不必要缩写并减少重复读取
spec。
In
`@test/src/test/groovy/org/zstack/test/integration/storage/primary/addon/zbs/ZbsVolumeEncryptionCase.groovy`:
- Around line 216-233: 更新
ZbsVolumeEncryptionExtension.createEncryptedEmptyVolume 用例，显式 stub KVM handler
返回成功，避免通过后续缺失 handler 的失败来验证材料复用；在 success 回调中断言结果及命令中的 encryptedDek，并相应移除
outcome.error 非空断言，保留必要的材料与删除路径验证。
🪄 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: 970dc29e-d67b-42cb-99b2-f982048cc1ae
⛔ Files ignored due to path filters (1)
conf/springConfigXml/VolumeManager.xmlis excluded by!**/*.xml
📒 Files selected for processing (16)
header/src/main/java/org/zstack/header/storage/addon/primary/CreateVolumeSpec.javaheader/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/ZbsEncryptedEmptyVolumeCreator.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/encrypt/ZbsVolumeEncryptionMaterialFactory.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
+200
to
+261
| @Override | ||
| public void createConversionTarget(String targetInstallPath, long virtualSize, boolean targetEncrypted, | ||
| ReturnValueCompletion<String> completion) { | ||
| CbdPath target; | ||
| try { | ||
| target = parseConversionCbdPath(targetInstallPath); | ||
| } catch (OperationFailureException e) { | ||
| completion.fail(e.getErrorCode()); | ||
| return; | ||
| } | ||
|
|
||
| ZbsStorageController.CreateVolumeCmd cmd = new ZbsStorageController.CreateVolumeCmd(); | ||
| cmd.setLogicalPool(target.logicalPool); | ||
| cmd.setVolume(target.volume); | ||
| cmd.setUnit(getSizeUnit(addonInfo.getClusterInfo().getVersion())); | ||
| long allocatedSize = targetEncrypted ? luksBackingSize(virtualSize) : virtualSize; | ||
| cmd.setSize(alignSizeTo(allocatedSize, cmd.getUnit())); | ||
| cmd.setSkipIfExisting(false); | ||
|
|
||
| controller.httpCall(ZbsStorageController.CREATE_VOLUME_PATH, cmd, ZbsStorageController.CreateVolumeRsp.class, | ||
| new ReturnValueCompletion<ZbsStorageController.CreateVolumeRsp>(completion) { | ||
| @Override | ||
| public void success(ZbsStorageController.CreateVolumeRsp returnValue) { | ||
| 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) { | ||
| @Override | ||
| public void success() { | ||
| completion.fail(error); | ||
| } | ||
|
|
||
| @Override | ||
| public void fail(ErrorCode cleanupError) { | ||
| logger.warn(String.format( | ||
| "failed to cleanup unexpected ZBS conversion target[installPath:%s] after create path mismatch: %s", | ||
| cleanupInstallPath, cleanupError)); | ||
| completion.fail(error); | ||
| } | ||
| }); | ||
| } catch (Exception e) { | ||
| logger.warn(String.format( | ||
| "failed to cleanup unexpected ZBS conversion target[installPath:%s] after create path mismatch: %s", | ||
| cleanupInstallPath, e.getMessage())); | ||
| completion.fail(error); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public void fail(ErrorCode errorCode) { | ||
| completion.fail(errorCode); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
路径不一致时清理了错误的路径，导致泄漏卷无法被回收。
当创建目标卷成功但返回的 createdInstallPath 与请求的 targetInstallPath 不一致时，cleanupInstallPath 被赋值为 targetInstallPath（该路径实际从未被创建），而真正被 ZBS 创建出来的卷位于 createdInstallPath。对 targetInstallPath 执行删除无法清理真正泄漏的卷，会在 ZBS 侧留下一个无主卷；且由于创建时 skipIfExisting=false，后续对同一转换目标的重试还会因路径已存在而持续失败。
🐛 建议修复：清理实际创建出来的路径
ErrorCode error = operr(
"ZBS volume encryption conversion target[%s] was created at unexpected path[%s]",
targetInstallPath, createdInstallPath);
- String cleanupInstallPath = targetInstallPath;
+ String cleanupInstallPath = createdInstallPath;
try {
deleteConversionTarget(cleanupInstallPath, new Completion(completion) {📝 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
| @Override | |
| public void createConversionTarget(String targetInstallPath, long virtualSize, boolean targetEncrypted, | |
| ReturnValueCompletion<String> completion) { | |
| CbdPath target; | |
| try { | |
| target = parseConversionCbdPath(targetInstallPath); | |
| } catch (OperationFailureException e) { | |
| completion.fail(e.getErrorCode()); | |
| return; | |
| } | |
| ZbsStorageController.CreateVolumeCmd cmd = new ZbsStorageController.CreateVolumeCmd(); | |
| cmd.setLogicalPool(target.logicalPool); | |
| cmd.setVolume(target.volume); | |
| cmd.setUnit(getSizeUnit(addonInfo.getClusterInfo().getVersion())); | |
| long allocatedSize = targetEncrypted ? luksBackingSize(virtualSize) : virtualSize; | |
| cmd.setSize(alignSizeTo(allocatedSize, cmd.getUnit())); | |
| cmd.setSkipIfExisting(false); | |
| controller.httpCall(ZbsStorageController.CREATE_VOLUME_PATH, cmd, ZbsStorageController.CreateVolumeRsp.class, | |
| new ReturnValueCompletion<ZbsStorageController.CreateVolumeRsp>(completion) { | |
| @Override | |
| public void success(ZbsStorageController.CreateVolumeRsp returnValue) { | |
| 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) { | |
| @Override | |
| public void success() { | |
| completion.fail(error); | |
| } | |
| @Override | |
| public void fail(ErrorCode cleanupError) { | |
| logger.warn(String.format( | |
| "failed to cleanup unexpected ZBS conversion target[installPath:%s] after create path mismatch: %s", | |
| cleanupInstallPath, cleanupError)); | |
| completion.fail(error); | |
| } | |
| }); | |
| } catch (Exception e) { | |
| logger.warn(String.format( | |
| "failed to cleanup unexpected ZBS conversion target[installPath:%s] after create path mismatch: %s", | |
| cleanupInstallPath, e.getMessage())); | |
| completion.fail(error); | |
| } | |
| } | |
| @Override | |
| public void fail(ErrorCode errorCode) { | |
| completion.fail(errorCode); | |
| } | |
| }); | |
| } | |
| ErrorCode error = operr( | |
| "ZBS volume encryption conversion target[%s] was created at unexpected path[%s]", | |
| targetInstallPath, createdInstallPath); | |
| String cleanupInstallPath = createdInstallPath; | |
| try { | |
| deleteConversionTarget(cleanupInstallPath, new Completion(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 200 - 261, Update the path-mismatch cleanup in
createConversionTarget so cleanupInstallPath uses createdInstallPath, the actual
path returned by ZBS, instead of targetInstallPath. Preserve the existing error
reporting and completion behavior while ensuring deleteConversionTarget removes
the created orphan volume.
Collaborator
There was a problem hiding this comment.
Comment from 周众:
ZBS 创建卷的实际标识由请求中的 logicalPool/volume 决定，CreateVolumeRsp.installPath 只是基于相同请求字段拼装的响应值；删除接口同样按 logicalPool/volume 删除。路径不一致说明响应字段异常，不能信任它执行删除，否则返回源卷或其他卷路径时可能误删数据。当前逻辑及回归测试有意只清理请求的 targetInstallPath。
There was a problem hiding this comment.
@ZStack-Robot 感谢周众的解释，这个说明是合理的。既然 ZBS 创建卷时实际使用的标识是请求中的 logicalPool/volume（即 target 解析出的字段），而 CreateVolumeRsp.installPath 只是基于同样字段拼装出的回显值，一旦该字段异常，也不能保证其指向的就是真正创建出来的卷；而删除接口同样是按 logicalPool/volume 定位卷。若信任这个不可靠的回显字段去执行删除，反而有误删源卷或其他卷的风险。因此当前只清理 targetInstallPath（对应实际用于创建的请求参数）是更安全的选择，我撤回此前的建议。
Collaborator
|
Comment from 周众: 第二个Critical问题不在这个patch解，后面会单独提patch解。 |
1 similar comment
Owner
Author
|
Comment from 周众: 第二个Critical问题不在这个patch解，后面会单独提patch解。 |
MatheMatrix
force-pushed
the
sync/zstackio/codex/zbs-change-volume-encryption@@2
branch
from
July 15, 2026 11:20
2e62674 to
b6690f9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@test/src/test/groovy/org/zstack/test/integration/storage/primary/addon/zbs/ZbsVolumeEncryptionCase.groovy`:
- Around line 193-249: Update testEncryptedEmptyVolumeCreatorReusesImageKey to
configure secretHelper with a non-empty sealedDek so the sealing step succeeds,
then assert successful stats creation and no deletedPaths. Remove the
failure/error assertions from this reuse-success test; cover cleanup behavior
separately if needed, while preserving the existing key reuse request
assertions.
🪄 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: 1da40e80-e791-4253-8776-1706f47738f7
⛔ Files ignored due to path filters (1)
conf/springConfigXml/VolumeManager.xmlis excluded by!**/*.xml
📒 Files selected for processing (16)
header/src/main/java/org/zstack/header/storage/addon/primary/CreateVolumeSpec.javaheader/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/ZbsEncryptedEmptyVolumeCreator.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/encrypt/ZbsVolumeEncryptionMaterialFactory.javastorage/src/main/java/org/zstack/storage/volume/VolumeBase.javatest/src/test/groovy/org/zstack/test/integration/storage/primary/addon/zbs/ZbsVolumeEncryptionCase.groovy
🚧 Files skipped from review as they are similar to previous changes (14)
- storage/src/main/java/org/zstack/storage/encrypt/ZbsEncryptedEmptyVolumeCreator.java
- header/src/main/java/org/zstack/header/storage/addon/primary/PrimaryStorageControllerSvc.java
- plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java
- storage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionExtension.java
- storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorage.java
- header/src/main/java/org/zstack/header/storage/addon/primary/ZbsVolumeEncryptionExtensionPoint.java
- storage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionCommands.java
- header/src/main/java/org/zstack/header/storage/addon/primary/CreateVolumeSpec.java
- plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.java
- plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsVolumeEncryptionBackendImpl.java
- storage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionMaterialFactory.java
- storage/src/main/java/org/zstack/storage/volume/VolumeBase.java
- header/src/main/java/org/zstack/header/storage/addon/primary/ZbsVolumeEncryptionBackend.java
- storage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionConverter.java
MatheMatrix
force-pushed
the
sync/zstackio/codex/zbs-change-volume-encryption@@2
branch
2 times, most recently
from
July 15, 2026 13:33
73a5dfe to
32cdc5a
Compare
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
MatheMatrix
force-pushed
the
sync/zstackio/codex/zbs-change-volume-encryption@@2
branch
2 times, most recently
from
July 15, 2026 14:52
68ff7b0 to
de66f44
Compare
Route ZBS conversion through a dedicated backend and KVM handler. Tests: cbok zsv groovy_test ZbsVolumeEncryptionCase Resolves: ZSV-12664 Change-Id: Ibd30f74dd17cb4dc817e57c713b0f32915487e7c
MatheMatrix
force-pushed
the
sync/zstackio/codex/zbs-change-volume-encryption@@2
branch
from
July 15, 2026 15:42
de66f44 to
f78c317
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
header/src/main/java/org/zstack/header/storage/addon/primary/ZbsVolumeEncryptionBackend.java (1)
24-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win新增接口方法缺少 Javadoc 注释。
validateConversionPaths、createConversionTarget、deleteConversionTarget均无 Javadoc，无法在文档层面表达异常契约（如validateConversionPaths何时抛出异常）及各参数含义（如targetEncrypted）。📝 建议补充说明
+ /** + * 校验转换源路径与目标路径是否合法（例如是否位于同一物理/逻辑池）。 + * + * `@throws` org.zstack.header.exception.OperationFailureException 当路径不合法时抛出 + */ void validateConversionPaths(String sourceInstallPath, String targetInstallPath); + /** + * 在转换流程中创建目标卷。 + * + * `@param` targetEncrypted 目标卷是否为加密卷 + */ void createConversionTarget(String targetInstallPath, long virtualSize, boolean targetEncrypted, ReturnValueCompletion<String> completion); + /** + * 删除转换失败或清理场景下的目标卷。 + */ void deleteConversionTarget(String targetInstallPath, Completion completion);As per path instructions, "接口方法不应有多余的修饰符（例如
public），且必须配有有效的 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 - 30, 为接口中的 validateConversionPaths、createConversionTarget 和 deleteConversionTarget 补充有效的 Javadoc，说明各参数含义、异步 completion 回调行为，以及 validateConversionPaths 可能抛出的异常或失败条件；保持方法签名不变，不添加多余的 public 等修饰符。Source: Path instructions
🤖 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.
Nitpick comments:
In
`@header/src/main/java/org/zstack/header/storage/addon/primary/ZbsVolumeEncryptionBackend.java`:
- Around line 24-30: 为接口中的 validateConversionPaths、createConversionTarget 和
deleteConversionTarget 补充有效的 Javadoc，说明各参数含义、异步 completion 回调行为，以及
validateConversionPaths 可能抛出的异常或失败条件；保持方法签名不变，不添加多余的 public 等修饰符。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b66960aa-0f1b-4b58-a213-824904b11cfe
⛔ Files ignored due to path filters (1)
conf/springConfigXml/VolumeManager.xmlis excluded by!**/*.xml
📒 Files selected for processing (16)
header/src/main/java/org/zstack/header/storage/addon/primary/CreateVolumeSpec.javaheader/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/ZbsEncryptedEmptyVolumeCreator.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/encrypt/ZbsVolumeEncryptionMaterialFactory.javastorage/src/main/java/org/zstack/storage/volume/VolumeBase.javatest/src/test/groovy/org/zstack/test/integration/storage/primary/addon/zbs/ZbsVolumeEncryptionCase.groovy
🚧 Files skipped from review as they are similar to previous changes (14)
- storage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionConstants.java
- storage/src/main/java/org/zstack/storage/encrypt/ZbsEncryptedEmptyVolumeCreator.java
- storage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionExtension.java
- storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorage.java
- header/src/main/java/org/zstack/header/storage/addon/primary/CreateVolumeSpec.java
- header/src/main/java/org/zstack/header/storage/addon/primary/PrimaryStorageControllerSvc.java
- plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java
- header/src/main/java/org/zstack/header/storage/addon/primary/ZbsVolumeEncryptionExtensionPoint.java
- plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.java
- storage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionCommands.java
- storage/src/main/java/org/zstack/storage/volume/VolumeBase.java
- plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsVolumeEncryptionBackendImpl.java
- storage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionConverter.java
- test/src/test/groovy/org/zstack/test/integration/storage/primary/addon/zbs/ZbsVolumeEncryptionCase.groovy
Collaborator
|
Comment from 周众: 1.createConversionTarget mismatch 清理路径 |
ZStack-Robot
deleted the
sync/zstackio/codex/zbs-change-volume-encryption@@2
branch
July 16, 2026 10:17
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.
Summary
为 ZBS/CBD 云盘增加明文与加密格式双向转换，转换语义参考 RBD；云盘存在主存储快照时拒绝转换。
Changes
Testing
Resolves: ZSV-12664
sync from gitlab !10539