Skip to content

<feature>[storage]: ZSV-12664 support ZBS volume encryption conversion#4571

Open
MatheMatrix wants to merge 1 commit into
feature-zsv-5.1.0-encryptionfrom
sync/zstackio/codex/zbs-change-volume-encryption@@2
Open

<feature>[storage]: ZSV-12664 support ZBS volume encryption conversion#4571
MatheMatrix wants to merge 1 commit into
feature-zsv-5.1.0-encryptionfrom
sync/zstackio/codex/zbs-change-volume-encryption@@2

Conversation

@MatheMatrix

Copy link
Copy Markdown
Owner

Summary

为 ZBS/CBD 云盘增加明文与加密格式双向转换,转换语义参考 RBD;云盘存在主存储快照时拒绝转换。

Changes

  • 新增 ZBS 加密转换器、主存储后端和 KVM 路由。
  • 使用同一物理池与逻辑池的新目标路径完成转换,并安全清理失败目标。
  • 增加快照拒绝、双向转换、路径与清理安全性自动化覆盖。

Testing

  • cbok zsv groovy_test:ZbsVolumeEncryptionCase 通过(1/1)
  • cbok zsv compile --no-deploy:header/storage/zbs/expon BUILD SUCCESS
  • CI pipeline

Resolves: ZSV-12664

sync from gitlab !10539

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 5 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 822fc732-7a6d-43d2-9d76-f28472897a25

📥 Commits

Reviewing files that changed from the base of the PR and between b6690f9 and 73a5dfe.

⛔ Files ignored due to path filters (1)
  • conf/springConfigXml/VolumeManager.xml is excluded by !**/*.xml
📒 Files selected for processing (16)
  • header/src/main/java/org/zstack/header/storage/addon/primary/CreateVolumeSpec.java
  • header/src/main/java/org/zstack/header/storage/addon/primary/PrimaryStorageControllerSvc.java
  • header/src/main/java/org/zstack/header/storage/addon/primary/ZbsVolumeEncryptionBackend.java
  • header/src/main/java/org/zstack/header/storage/addon/primary/ZbsVolumeEncryptionExtensionPoint.java
  • plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.java
  • plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java
  • plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsVolumeEncryptionBackendImpl.java
  • storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorage.java
  • storage/src/main/java/org/zstack/storage/encrypt/ZbsEncryptedEmptyVolumeCreator.java
  • storage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionCommands.java
  • storage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionConstants.java
  • storage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionConverter.java
  • storage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionExtension.java
  • storage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionMaterialFactory.java
  • storage/src/main/java/org/zstack/storage/volume/VolumeBase.java
  • test/src/test/groovy/org/zstack/test/integration/storage/primary/addon/zbs/ZbsVolumeEncryptionCase.groovy

Walkthrough

新增 ZBS/CBD 卷加密双向转换流程,涵盖接口契约、目标卷生命周期、KVM 转换、密钥材料准备、CBD 路径处理、错误清理及端到端测试。

Changes

ZBS 卷加密转换

Layer / File(s) Summary
转换契约与加密材料
header/.../primary/*, storage/src/main/java/org/zstack/storage/encrypt/*
新增密钥资源字段、转换接口、KVM 命令结构和常量;支持从卷或临时快照镜像解析 key provider 并准备加密材料。
主存储路由与后端目标管理
storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorage.java, plugin/zbs/..., plugin/expon/...
接入转换消息处理;ZBS 控制器委托扩展点执行转换,后端校验 CBD 路径、创建目标卷并在路径不匹配时清理。
转换执行与失败清理
storage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionConverter.java
校验请求,创建目标,调用 KVM LUKS 转换;根据直接结果或 stats 返回实际大小,并在失败时删除目标。
CBD 路径与集成验证
storage/src/main/java/org/zstack/storage/volume/VolumeBase.java, test/src/test/.../ZbsVolumeEncryptionCase.groovy
增加 CBD 安装路径转换逻辑及动态存储类型校验;测试成功转换、输入拒绝、密钥处理、版本限制、统计回退和清理范围。

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: 返回转换回复
Loading

Possibly related PRs

Poem

小兔挥爪改密钥,
CBD 路径蹦蹦跳。
KVM 转换灯亮起,
失败目标及时消。
明文密文来回换,
测试萝卜堆得高!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed 标题准确概括了该 PR 的核心变更:支持 ZBS 云盘加密转换。
Description check ✅ Passed 描述与变更一致,准确说明了双向加密转换、快照拒绝、路径与清理安全性及测试覆盖。
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch sync/zstackio/codex/zbs-change-volume-encryption@@2

Comment @coderabbitai help to get the list of available commands.

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details
putComment timed out

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 改为完整名称。

建议使用 keyResourceTypekeyResourceUuidkeyProviderUuid 局部变量,避免重复读取 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

📥 Commits

Reviewing files that changed from the base of the PR and between 27aafe6 and 2e62674.

⛔ Files ignored due to path filters (1)
  • conf/springConfigXml/VolumeManager.xml is excluded by !**/*.xml
📒 Files selected for processing (16)
  • header/src/main/java/org/zstack/header/storage/addon/primary/CreateVolumeSpec.java
  • header/src/main/java/org/zstack/header/storage/addon/primary/PrimaryStorageControllerSvc.java
  • header/src/main/java/org/zstack/header/storage/addon/primary/ZbsVolumeEncryptionBackend.java
  • header/src/main/java/org/zstack/header/storage/addon/primary/ZbsVolumeEncryptionExtensionPoint.java
  • plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.java
  • plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java
  • plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsVolumeEncryptionBackendImpl.java
  • storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorage.java
  • storage/src/main/java/org/zstack/storage/encrypt/ZbsEncryptedEmptyVolumeCreator.java
  • storage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionCommands.java
  • storage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionConstants.java
  • storage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionConverter.java
  • storage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionExtension.java
  • storage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionMaterialFactory.java
  • storage/src/main/java/org/zstack/storage/volume/VolumeBase.java
  • test/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);
}
});
}

@coderabbitai coderabbitai Bot Jul 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment from 周众:

ZBS 创建卷的实际标识由请求中的 logicalPool/volume 决定,CreateVolumeRsp.installPath 只是基于相同请求字段拼装的响应值;删除接口同样按 logicalPool/volume 删除。路径不一致说明响应字段异常,不能信任它执行删除,否则返回源卷或其他卷路径时可能误删数据。当前逻辑及回归测试有意只清理请求的 targetInstallPath。

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ZStack-Robot 感谢周众的解释,这个说明是合理的。既然 ZBS 创建卷时实际使用的标识是请求中的 logicalPool/volume(即 target 解析出的字段),而 CreateVolumeRsp.installPath 只是基于同样字段拼装出的回显值,一旦该字段异常,也不能保证其指向的就是真正创建出来的卷;而删除接口同样是按 logicalPool/volume 定位卷。若信任这个不可靠的回显字段去执行删除,反而有误删源卷或其他卷的风险。因此当前只清理 targetInstallPath(对应实际用于创建的请求参数)是更安全的选择,我撤回此前的建议。

@zstack-robot-2

Copy link
Copy Markdown
Collaborator

Comment from 周众:

第二个Critical问题不在这个patch解,后面会单独提patch解。

1 similar comment
@MatheMatrix

Copy link
Copy Markdown
Owner Author

Comment from 周众:

第二个Critical问题不在这个patch解,后面会单独提patch解。

@MatheMatrix MatheMatrix force-pushed the sync/zstackio/codex/zbs-change-volume-encryption@@2 branch from 2e62674 to b6690f9 Compare July 15, 2026 11:20

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 2e62674 and b6690f9.

⛔ Files ignored due to path filters (1)
  • conf/springConfigXml/VolumeManager.xml is excluded by !**/*.xml
📒 Files selected for processing (16)
  • header/src/main/java/org/zstack/header/storage/addon/primary/CreateVolumeSpec.java
  • header/src/main/java/org/zstack/header/storage/addon/primary/PrimaryStorageControllerSvc.java
  • header/src/main/java/org/zstack/header/storage/addon/primary/ZbsVolumeEncryptionBackend.java
  • header/src/main/java/org/zstack/header/storage/addon/primary/ZbsVolumeEncryptionExtensionPoint.java
  • plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.java
  • plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java
  • plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsVolumeEncryptionBackendImpl.java
  • storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorage.java
  • storage/src/main/java/org/zstack/storage/encrypt/ZbsEncryptedEmptyVolumeCreator.java
  • storage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionCommands.java
  • storage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionConstants.java
  • storage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionConverter.java
  • storage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionExtension.java
  • storage/src/main/java/org/zstack/storage/encrypt/ZbsVolumeEncryptionMaterialFactory.java
  • storage/src/main/java/org/zstack/storage/volume/VolumeBase.java
  • test/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

Route ZBS conversion through a dedicated backend and KVM handler.

Tests: cbok zsv groovy_test ZbsVolumeEncryptionCase

Resolves: ZSV-12664

Change-Id: Ibd30f74dd17cb4dc817e57c713b0f32915487e7c
@MatheMatrix MatheMatrix force-pushed the sync/zstackio/codex/zbs-change-volume-encryption@@2 branch from b6690f9 to 73a5dfe Compare July 15, 2026 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants