Skip to content

<fix>[storage]: ZSV-12469 support volume encryption conversion rollback#4494

Open
ZStack-Robot wants to merge 2 commits into
feature-zsv-5.1.0-encryptionfrom
sync/zstackio/dev/encrypted-volume-snapshot-conversion-rollback@@3
Open

<fix>[storage]: ZSV-12469 support volume encryption conversion rollback#4494
ZStack-Robot wants to merge 2 commits into
feature-zsv-5.1.0-encryptionfrom
sync/zstackio/dev/encrypted-volume-snapshot-conversion-rollback@@3

Conversation

@ZStack-Robot

Copy link
Copy Markdown
Collaborator

Summary

Split volume/snapshot encryption conversion rollback support into an independent base branch for ZSV-12469.

Changes

  • Add rollback message/reply for volume encryption conversion on primary storage.
  • Route conversion rollback through LocalStorage, NFS, and Ceph backends.
  • Restore source bits from sourceTrash on rollback instead of only deleting converted target bits.
  • Avoid deterministic sourceTrash install path conflicts across repeated conversion attempts.

Testing

  • git diff --check
  • cbok/default-env verification
  • CI pipeline

Resolves: ZSV-12469

sync from gitlab !10455

@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown

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: 37 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: c51dd490-ff2f-4b4f-82bd-0dacc3dd72c5

📥 Commits

Reviewing files that changed from the base of the PR and between 3fe3999 and 763f2db.

📒 Files selected for processing (12)
  • header/src/main/java/org/zstack/header/storage/primary/RollbackVolumeEncryptionOnPrimaryStorageMsg.java
  • header/src/main/java/org/zstack/header/storage/primary/RollbackVolumeEncryptionOnPrimaryStorageReply.java
  • plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageBase.java
  • plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java
  • plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageHypervisorBackend.java
  • plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmBackend.java
  • plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorage.java
  • plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageBackend.java
  • plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.java
  • plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackendCommands.java
  • storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java
  • storage/src/main/java/org/zstack/storage/volume/VolumeBase.java
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch sync/zstackio/dev/encrypted-volume-snapshot-conversion-rollback@@3

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

@MatheMatrix MatheMatrix force-pushed the sync/zstackio/dev/encrypted-volume-snapshot-conversion-rollback@@3 branch 6 times, most recently from f08acda to c1f218b Compare July 8, 2026 07:33
Resolves: ZSV-12469

Change-Id: I7940bd9f7ac4207a0694a3b52e8c8dafbae538c2
@MatheMatrix MatheMatrix force-pushed the sync/zstackio/dev/encrypted-volume-snapshot-conversion-rollback@@3 branch from c1f218b to 9952275 Compare July 8, 2026 07:45
@zstack-robot-2

Copy link
Copy Markdown
Collaborator

Comment from yaohua.wu:

Review: MR !10455 — ZSV-12469

Background

  • Jira: ZSV-12469 — 【后端】【磁盘加密】加密属性转换
  • Bug summary: 本组 MR 为磁盘/快照加密属性转换增加失败回滚能力,避免转换后只清理 target 而未恢复 sourceTrash 中的原始 bits。Jira 描述较少,主要上下文来自 3 个 MR 的 Summary/Changes。
  • Intent & scope: zstack 侧新增 rollback message/reply,并把 VolumeBase 的转换失败回滚路由到具体 PrimaryStorage;premium 侧补 SharedBlock Java dispatch;zstack-utility 侧补 KVM agent rollback endpoints。
  • Round 1 initial findings: 0 × Critical, 1 × Warning in zstack Ceph rollback command schema;另有 1 × Warning in zstack-utility agent rollback delete safety。
  • Suggested fix direction: 对齐 Java command 与 Python agent schema,并在 agent rollback 删除 target 前复用现有 active-use / watcher 防护。

关联 MR

MR 关系
zstackio/premium !14652 SharedBlock 主存储 Java dispatch,依赖本 MR 新增的 RollbackVolumeEncryptionOnPrimaryStorageMsg
zstackio/zstack-utility !7465 KVM/Ceph/Local/NFS/SharedBlock agent rollback endpoints,必须与本 MR 的 command schema 同步合入。

Findings

Severity File:Line Issue Fix
🟡 Warning plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageBase.java:506 / :3711 Ceph rollback 的 Java command 没有 volumeUuid,但 zstack-utility 的 cephagent.py:1280 在 rollback 失败时会读取 cmd.volumeUuid 组装错误。实际失败场景下会先触发 AttributeError,导致原始 rollback 失败原因被覆盖,管理节点只能看到错误格式化失败后的泛化错误。 RollbackLuksRbdCmd 增加 public String volumeUuid;,并在构造 rcmd 时设置 rcmd.volumeUuid = msg.getVolume().getUuid();;同时建议 utility 侧用 getattr(cmd, 'volumeUuid', cmd.installPath) 做兼容兜底。

Cross-Repo Notes

Check Result
Command path alignment zstack Local/NFS/Ceph path 与 zstack-utility endpoint 名称一致;premium SharedBlock path 与 utility SharedBlock endpoint 一致。
Schema alignment Local/NFS/SharedBlock 的 volumeUuid/items schema 对齐;Ceph 缺 volumeUuid,见上方 Warning。
Joint merge requirement 这 3 个 MR 使用同一 @@3 分支语义,不能只合入 zstack 侧,否则新消息会路由到未实现 agent endpoint。

Coverage

  • Local diff source: git diff 3fe399986d0b0cc90017395f08d548f451632f29..99522755c099d45ed18c046e5f4073957dd0948a
  • Generated-file filter: 0 files skipped (kept 12 / total 12)
  • Upstream freshness: target branch ref equals MR base SHA;MR metadata merge_status=can_be_merged
  • Verification run: git diff --check passed for all 3 repos
  • Testing gap: 未本地运行 cbok/default-env 或集成测试;建议覆盖 Local/NFS/SharedBlock/Ceph rollback failure path。

Verdict: REVISION_REQUIRED

Ceph rollback schema 需要与 zstack-utility 侧补齐后再合入;同时本 MR 需与两个关联 MR 联合合并。


🤖 Robot Reviewer

@MatheMatrix MatheMatrix force-pushed the sync/zstackio/dev/encrypted-volume-snapshot-conversion-rollback@@3 branch from 6ce9818 to d5a1db3 Compare July 9, 2026 12:12
Resolves: ZSV-12469

Tests: covered by premium EncryptVolumeConversionCase.

Change-Id: I9b79e68662d0e6abccaa9a87929e6b8d2c1fa123
@MatheMatrix MatheMatrix force-pushed the sync/zstackio/dev/encrypted-volume-snapshot-conversion-rollback@@3 branch from d5a1db3 to 763f2db Compare July 9, 2026 12:16
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.

2 participants