<fix>[storage]: ZSV-12469 support volume encryption conversion rollback#4494
Open
ZStack-Robot wants to merge 3 commits into
Conversation
Walkthrough本次变更新增卷加密转换回滚消息，重构转换路径和数据库更新逻辑，并为 Ceph、LocalStorage、NFS 主存储接入回滚处理及对应测试模拟器。 Changes卷加密转换与回滚
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant VolumeBase
participant PrimaryStorage
participant StorageAgent
VolumeBase->>PrimaryStorage: 发送回滚消息与转换条目
PrimaryStorage->>StorageAgent: 下发卷加密回滚命令
StorageAgent-->>PrimaryStorage: 返回执行结果
PrimaryStorage-->>VolumeBase: 返回回滚回复
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.44.1)plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageBase.javaast-grep timed out on this file plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.javaast-grep retry budget exhausted before isolating this batch plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageHypervisorBackend.javaast-grep retry budget exhausted before isolating this batch
Comment |
MatheMatrix
force-pushed
the
sync/zstackio/dev/encrypted-volume-snapshot-conversion-rollback@@3
branch
7 times, most recently
from
c1f218b to
9952275
Compare
Collaborator
|
Comment from yaohua.wu: Review: MR !10455 — ZSV-12469Background
关联 MR
Findings
Cross-Repo Notes
Coverage
Verdict: REVISION_REQUIREDCeph rollback schema 需要与 zstack-utility 侧补齐后再合入；同时本 MR 需与两个关联 MR 联合合并。 🤖 Robot Reviewer |
MatheMatrix
force-pushed
the
sync/zstackio/dev/encrypted-volume-snapshot-conversion-rollback@@3
branch
3 times, most recently
from
4e34602 to
763f2db
Compare
added 2 commits
July 11, 2026 13:10
…sion Resolves: ZSV-12469 Tests: covered by premium EncryptVolumeConversionCase. Change-Id: I625c1b26de3f8bb2b3c8520c936425b9d4457dcb
Related: ZSV-12469 Change-Id: I943b6c54f5cef6fafaa899c89709265b94ff7224
MatheMatrix
force-pushed
the
sync/zstackio/dev/encrypted-volume-snapshot-conversion-rollback@@3
branch
from
763f2db to
67e59ab
Compare
Resolves: ZSV-12469 Tests: covered by premium EncryptVolumeConversionCase. Change-Id: I49312b0d373c696f86f79adb6274e6648cdede74
MatheMatrix
force-pushed
the
sync/zstackio/dev/encrypted-volume-snapshot-conversion-rollback@@3
branch
from
67e59ab to
ab4f2b5
Compare
![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)
plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageBase.java (1)
3649-3692: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win三处
RollbackVolumeEncryptionOnPrimaryStorageMsg处理方法均是对同类中既有ConvertVolumeEncryptionOnPrimaryStorageMsg处理方法的复制粘贴，共享同一根因：新增回滚处理逻辑未与转换处理逻辑抽取公共校验/委托代码。
plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageBase.java#L3649-L3692：抽取公共私有方法处理 items 数量校验、resourceType 校验及路径合法性校验（与上方handle(ConvertVolumeEncryptionOnPrimaryStorageMsg)共用）。plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java#L964-L1005：抽取公共私有方法处理hostUuid解析（优先msg.getHostUuid()，否则getHostUuidByResourceUuid）逻辑，供 Convert 与 Rollback 处理方法共用。plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorage.java#L174-L197：抽取公共私有方法处理getUsableBackend()为空时的报错与委托流程，供 Convert 与 Rollback 处理方法共用。🤖 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/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageBase.java` around lines 3649 - 3692, 抽取 Convert 与 Rollback 处理逻辑中的重复公共流程：在 plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageBase.java:3649-3692 中新增私有方法，统一处理 items 数量、resourceType 和目标路径校验，并由两个 handle 方法复用；在 plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java:964-1005 中抽取 hostUuid 解析逻辑，保持优先使用 msg.getHostUuid()、否则调用 getHostUuidByResourceUuid；在 plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorage.java:174-197 中抽取 getUsableBackend() 为空时的报错与委托流程，供 Convert 和 Rollback 处理方法共同调用。storage/src/main/java/org/zstack/storage/volume/VolumeBase.java (2)
3733-3749: 🧹 Nitpick | 🔵 Trivial回滚失败仅记录 warn 日志，缺少进一步的可观测性。
rollbackVolumeEncryptionOnPrimaryStorage在主存储侧回滚消息失败时，仅打印 warn 日志便继续trigger.rollback()。这意味着"加密转换失败 + 主存储侧回滚也失败"的双重失败场景下，运维人员仅能通过日志感知，没有额外的指标或告警渠道，可能导致目标残留文件长期未被清理却难以被发现。建议为该失败路径补充可观测性手段（如打点、事件或专门的 GC 巡检），便于及时发现并处理这类残留。
🤖 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/volume/VolumeBase.java` around lines 3733 - 3749, 在 rollbackVolumeEncryptionOnPrimaryStorage 的 reply 失败分支中，除现有 warn 日志外增加项目既有的可观测性机制（如失败指标、告警事件或残留 GC 巡检登记），并携带 volume UUID、primary storage UUID 及错误详情。保持 trigger.rollback() 仍被调用，确保双重失败可被后续运维及时发现和处理。
3479-3496: 🧹 Nitpick | 🔵 Trivial旧卷位（trash）记录失败被静默吞掉，可能造成存储泄漏。
record-old-volume-bits-in-trash步骤将trash.createTrash包裹在 try/catch 中，失败时仅 warn 日志继续执行。由于此时数据库已经指向新路径，一旦 trash 记录失败，旧的卷/快照文件将不再被任何清理机制追踪，长期可能造成存储空间泄漏且难以被发现。建议在失败时补充告警或将失败项记录到可巡检的位置（如系统标签/专用表），以便后续人工或 GC 任务介入清理。
🤖 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/volume/VolumeBase.java` around lines 3479 - 3496, 更新“record-old-volume-bits-in-trash”流程的异常处理，避免仅通过 logger.warn 后继续执行而静默丢失失败项；在 trash.createTrash 失败时，使用现有的可巡检机制（如系统标签或专用记录表）持久化卷/快照标识及错误信息，确保后续人工或 GC 可清理。保留必要告警，并确保卷和各快照的失败记录都能被追踪。
🤖 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/primary/RollbackVolumeEncryptionOnPrimaryStorageMsg.java`:
- Around line 14-17: 在 RollbackVolumeEncryptionOnPrimaryStorageMsg 的
getPrimaryStorageUuid() 中增加 volume 为空时的保护，避免路由阶段直接抛出 NullPointerException；未设置
volume 时返回可表示缺失的结果，并保留已设置 volume 时通过 getPrimaryStorageUuid() 获取主存储 UUID 的行为。
---
Nitpick comments:
In
`@plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageBase.java`:
- Around line 3649-3692: 抽取 Convert 与 Rollback 处理逻辑中的重复公共流程：在
plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageBase.java:3649-3692
中新增私有方法，统一处理 items 数量、resourceType 和目标路径校验，并由两个 handle 方法复用；在
plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java:964-1005
中抽取 hostUuid 解析逻辑，保持优先使用 msg.getHostUuid()、否则调用 getHostUuidByResourceUuid；在
plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorage.java:174-197
中抽取 getUsableBackend() 为空时的报错与委托流程，供 Convert 和 Rollback 处理方法共同调用。
In `@storage/src/main/java/org/zstack/storage/volume/VolumeBase.java`:
- Around line 3733-3749: 在 rollbackVolumeEncryptionOnPrimaryStorage 的 reply
失败分支中，除现有 warn 日志外增加项目既有的可观测性机制（如失败指标、告警事件或残留 GC 巡检登记），并携带 volume UUID、primary
storage UUID 及错误详情。保持 trigger.rollback() 仍被调用，确保双重失败可被后续运维及时发现和处理。
- Around line 3479-3496: 更新“record-old-volume-bits-in-trash”流程的异常处理，避免仅通过
logger.warn 后继续执行而静默丢失失败项；在 trash.createTrash
失败时，使用现有的可巡检机制（如系统标签或专用记录表）持久化卷/快照标识及错误信息，确保后续人工或 GC
可清理。保留必要告警，并确保卷和各快照的失败记录都能被追踪。
🪄 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: 2fe25c24-c9d9-4626-90f9-262c481dd4d6
📒 Files selected for processing (16)
header/src/main/java/org/zstack/header/storage/primary/ConvertVolumeEncryptionOnPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/RollbackVolumeEncryptionOnPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/RollbackVolumeEncryptionOnPrimaryStorageReply.javaplugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageBase.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageHypervisorBackend.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmBackend.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorage.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageBackend.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackendCommands.javastorage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.javastorage/src/main/java/org/zstack/storage/volume/VolumeBase.javatestlib/src/main/java/org/zstack/testlib/CephPrimaryStorageSpec.groovytestlib/src/main/java/org/zstack/testlib/LocalStorageSpec.groovytestlib/src/main/java/org/zstack/testlib/NfsPrimaryStorageSpec.groovy
💤 Files with no reviewable changes (1)
- header/src/main/java/org/zstack/header/storage/primary/ConvertVolumeEncryptionOnPrimaryStorageMsg.java
Comment on lines
+14
to
+17
| @Override | ||
| public String getPrimaryStorageUuid() { | ||
| return volume.getPrimaryStorageUuid(); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
getPrimaryStorageUuid() 未对 volume 做空值校验，可能在消息路由阶段抛出 NPE
若调用方在未设置 volume 的情况下发送该消息，消息总线路由时调用 getPrimaryStorageUuid() 会直接抛出 NullPointerException，此时 handle() 方法中已有的空值校验根本不会被执行到（路由发生在 handle() 之前）。建议在此处补充空值保护。
🤖 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/primary/RollbackVolumeEncryptionOnPrimaryStorageMsg.java`
around lines 14 - 17, 在 RollbackVolumeEncryptionOnPrimaryStorageMsg 的
getPrimaryStorageUuid() 中增加 volume 为空时的保护，避免路由阶段直接抛出 NullPointerException；未设置
volume 时返回可表示缺失的结果，并保留已设置 volume 时通过 getPrimaryStorageUuid() 获取主存储 UUID 的行为。
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
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
Split volume/snapshot encryption conversion rollback support into an independent base branch for ZSV-12469.
Changes
Testing
Resolves: ZSV-12469
sync from gitlab !10455