Skip to content

<doc>[docs]: <description#3945

Open
zstack-robot-2 wants to merge 5 commits into
zsv_5.0.1from
sync/tao.gan/5.1.0-ZSV-10538@@3
Open

<doc>[docs]: <description#3945
zstack-robot-2 wants to merge 5 commits into
zsv_5.0.1from
sync/tao.gan/5.1.0-ZSV-10538@@3

Conversation

@zstack-robot-2
Copy link
Copy Markdown
Collaborator

Resolves: ZSV-10538

Change-Id: I766c706a657a63746c766b6a63626a7367636771

sync from gitlab !9836

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 13, 2026

Review Change Stack

Walkthrough

新增并细化数据面校验规范文档:定义 L1–L4 校验模板与工具接口、在线/离线操作的 pre/post 验证、qemu-img 检查约束、blockJob 状态机加固、启动恢复流程与相关配置项。

Changes

数据面校验与恢复规范

Layer / File(s) Summary
工具与接口规范
docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/02-data-plane-validation.md
定义 ChainSnapshot、take_chain_snapshot、verify_post_op、qcow2_get_backing_chain_strict、qemu_img_check 等工具/接口契约与接入格式。
L1:操作前快照(pre-op)模板
docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/02-data-plane-validation.md
给出在线 block_commit 的 pre-op/post-op 模板、离线 commit/pull/fullRebase 的临时文件登记与 mv/验证接入规范及恢复文件生命周期。
L2:操作后自检与 SharedBlock 接入
docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/02-data-plane-validation.md
规范 backing 关系与 size 校验(阈值/容差为 warn)、SharedBlock LV 处理、以及 PostOpVerifyError → FlowChain/reconciler 的上报链路。
L3:qemu-img 检查异常路径
docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/02-data-plane-validation.md
列出 qemu-img check 的触发场景与错误语义,明确仅检测不自动修复,-r 仅在控制面批准时使用。
L4:blockJob 状态机加固
docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/02-data-plane-validation.md
定义 active commit 的 READY/PIVOT 验证、超时/取消语义与基于 domain XML 的二次终态确认。
启动恢复与配置项
docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/02-data-plane-validation.md
启动时 recovery dump 扫描与诊断流程说明,以及 [snapshot_recovery] 配置项(开关、超时、阈值)定义。

🎯 3 (Moderate) | ⏱️ ~20 minutes

"我是一只爱挖洞的兔,文件与链条都能嗅;
L1 到 L4 我细细巡查,保护快照不外露。
校验模板我记牢,恢复流程我看好;
若有异常我去报,别让孤儿文件跑。
🐰✨"

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch sync/tao.gan/5.1.0-ZSV-10538@@3

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 13

🧹 Nitpick comments (4)
docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/01-control-plane-reconciler.md (4)

31-42: ⚡ Quick win

为伪代码块添加语言标识符。

建议为此代码围栏添加语言标识符(如 textpseudocode)以改善渲染效果。

📝 建议的修复
-```
+```text
 reconcile(treeUuid, volumeUuid, trigger):
   1. 读 DB:Q.New(VolumeSnapshotVO).eq(treeUuid).list()
      若结果为空(dst 是树根的 commit 已完成切换到新 treeUuid 场景)
🤖 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/specs/2026-05-13-snapshot-single-delete-hardening/01-control-plane-reconciler.md`
around lines 31 - 42, The fenced pseudocode for reconcile(treeUuid, volumeUuid,
trigger) lacks a language identifier which hurts rendering; update the fenced
code block to include a language tag like ```text or ```pseudocode so the block
renders correctly (i.e., change the opening fence from ``` to ```text and keep
the existing contents), referencing the reconcile function header to locate the
block.

151-157: ⚡ Quick win

为日志示例代码块添加语言标识符。

建议为日志示例围栏添加语言标识符(如 textlog)以改善渲染效果。

📝 建议的修复
-```
+```text
 [VolumeSnapshotTreeReconciler] tree=<uuid> trigger=AfterCommitSuccess
   inconsistencies: I3(snap-a parentUuid mismatch), I2(orphan-file /xxx.qcow2)
   applied: UPDATE_DB_PARENT_UUID(snap-a), SCHEDULE_GC_ORPHAN_FILE(/xxx.qcow2)
🤖 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/specs/2026-05-13-snapshot-single-delete-hardening/01-control-plane-reconciler.md`
around lines 151 - 157, The fenced log example for the
VolumeSnapshotTreeReconciler snippet should include a language identifier to
improve rendering; update the markdown block that contains the log lines
starting with "[VolumeSnapshotTreeReconciler] tree=<uuid>
trigger=AfterCommitSuccess" to use a suitable info string such as `text` or
`log` (e.g., replace the opening ``` with ```text) so the lines including
"inconsistencies: I3(...)" and "applied: UPDATE_DB_PARENT_UUID(...)" render
correctly.

135-137: ⚡ Quick win

为流程图代码块添加语言标识符。

建议为流程图围栏添加语言标识符(如 textmermaid)以改善渲染效果。

📝 建议的修复
-```
+```text
 vm 队列 ──► chainSubmit ──► commit/pull flow ──► done/error ──► reconciler ──► comp.success/fail ──► chainSubmit 释放 ──► vm 队列释放
🤖 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/specs/2026-05-13-snapshot-single-delete-hardening/01-control-plane-reconciler.md`
around lines 135 - 137, The fenced flow diagram block containing the exact line
"vm 队列 ──► chainSubmit ──► commit/pull flow ──► done/error ──► reconciler ──►
comp.success/fail ──► chainSubmit 释放 ──► vm 队列释放" should add a language
identifier after the opening backticks (for example ```text or ```mermaid) so
the renderer applies proper formatting; update that code fence to ```text (or
another appropriate language) and keep the diagram content unchanged.

54-54: ⚡ Quick win

考虑将 I3b 的三种子情形提取为独立子章节。

I3b 的描述包含三种复杂子情形,当前以单个表格单元格呈现可读性较差。建议将其提取为 5.3.1 子章节,使用项目符号列表详细阐述每种子情形的检测条件与修复动作。

🤖 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/specs/2026-05-13-snapshot-single-delete-hardening/01-control-plane-reconciler.md`
at line 54, Refactor the I3b row into its own subsection (e.g., 5.3.1) so the
three subcases become separate bullet items: clearly list detection conditions
and remediation for each subcase referencing the existing symbols (I3b,
parentUuid, db.parentUuid, Q(VolumeSnapshotVO).eq(uuid, parentUuid),
physical.backing, UPDATE_DB_PARENT_UUID, remaining, stepDelete); for (a)
describe detection “physical.backing can be reverse-resolved to an alive VO” and
action “UPDATE_DB_PARENT_UUID = that VO.uuid”; for (b) describe detection
“physical.backing == null (rebase to volume base)” and action
“UPDATE_DB_PARENT_UUID(null)”; for (c) describe detection “physical.backing
exists but cannot resolve to any alive VO (points to stepDeleted VO path,
physical rebase pending)” and action “leave DB unchanged and mark remaining for
retry after physical rebase.”
🤖 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 `@docs/snapshot-single-delete/00-overview.md`:
- Around line 32-44: The fenced code block in
docs/snapshot-single-delete/00-overview.md (the ASCII diagram showing "[祖父] ──
[待删节点 X] ── [子 Y] ...") is missing a language tag and triggers MD040; update the
opening fence from ``` to ```text (or ```mermaid if you convert it to a
renderable diagram) so the block is annotated and the linter passes, ensuring
the diagram remains unchanged.

In `@docs/snapshot-single-delete/02-call-chain.md`:
- Around line 5-20: The code block in the call chain snippet lacks a language
tag which triggers markdownlint MD040; update the opening fence from ``` to
```text so the block is explicitly marked as plain text (affecting the snippet
that shows APIDeleteVolumeSnapshotGroupMsg, VolumeSnapshotGroupBase.handle(),
handleDelete(), deleteChainFlows(), deleteSingleFlows(), stepDelete(), etc.),
ensuring consistent rendering and silencing the MD040 warning.

In `@docs/snapshot-single-delete/05-commit-db-swap.md`:
- Around line 5-22: Update the fenced code block in the commit DB swap example
to include an explicit language marker by changing the opening backticks from
``` to ```text so the block starts with ```text and ends with ```, preventing
MD040 and ensuring consistent rendering; apply this to the block that shows
"Commit 前:" / "blockCommit(top=src, base=dst) 完成后:" etc.

In `@docs/snapshot-single-delete/06-pull-db-rewrite.md`:
- Around line 5-19: The fenced code block starting with ``` (containing "Pull
前:", "qemu-img rebase", and "DB 改写:") lacks a language tag which triggers MD040;
fix it by adding a language identifier (e.g. `text`) after the opening backticks
so the block begins with ```text, leaving the block contents unchanged and
ensuring markdownlint passes.

In `@docs/snapshot-single-delete/08-hypervisor-online-commit.md`:
- Around line 89-101: The fenced code block containing the Phase 1/Phase 2
sequence (showing blockCommit(), blockJobInfo,
blockJobAbort(VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT), check_overlay_file, etc.) is
missing a language marker and triggers MD040; update the opening fence from ```
to a declared language such as ```text (or ```bash) so the code block is
lint-compliant and the doc renders consistently.

In `@docs/snapshot-single-delete/11-sibling-rebase.md`:
- Around line 7-18: The fenced code block showing the 分叉链示例 is missing a
language tag and triggers MD040; add an explicit language identifier (e.g., use
"text" or "no-highlight") to the opening fence that contains the snippet (the
block starting with "分叉链示例:" and the ASCII tree) so the markdown linter stops
warning and document style stays consistent.

In `@docs/snapshot-single-delete/scenarios/01-multi-children-stepDelete.md`:
- Around line 10-16: The three fenced code blocks that show the flow diagram,
the VolumeSnapshotVO example (vos = Q(VolumeSnapshotVO).eq(treeUuid).list()) and
the tree diagram around the stepDelete() / deleteSingleFlows() notes are missing
language tags and trigger MD040; update each fenced block to use a language tag
(use text for diagrams/flow and for the VO query snippet if not actual code) —
specifically add ```text before the flow containing deleteSingleFlows() /
stepDelete(), before the block with the vos = Q(VolumeSnapshotVO)... example,
and before the tree diagram (currentRoot etc.), and close each with ``` so all
occurrences (including the other similar blocks mentioned) use ```text fenced
code blocks.

In
`@docs/snapshot-single-delete/scenarios/02-local-running-delete-mid-with-3-children.md`:
- Around line 16-214: The file triggers MD040 because many fenced code blocks
lack a language hint; update each triple-backtick block in this document to
include an appropriate language tag (e.g., text for ASCII diagrams, java for
Java snippets like the commit/children code, python for agent/plugin examples)
so linting passes; search for all ``` blocks in this markdown and append the
correct language (suggest text/java/python) consistent with the block content.
- Around line 128-146: Summary: This doc uses inconsistent online-commit flag
semantics (VIR_DOMAIN_BLOCK_COMMIT_ACTIVE | SHALLOW and
VIR_DOMAIN_BLOCK_COMMIT_DELETE) that conflict with the canonical mapping in
08-hypervisor-online-commit.md (ACTIVE | RELATIVE, DELETE only for non-active
commits). Fix: update the virDomainBlockCommit example to use the canonical
flags (replace VIR_DOMAIN_BLOCK_COMMIT_ACTIVE | SHALLOW with the same ACTIVE |
RELATIVE combination used in 08-hypervisor-online-commit.md) and adjust the
post-pivot deletion sentence so it does not attribute deletion to
VIR_DOMAIN_BLOCK_COMMIT_DELETE for an active commit (either state that pivoting
does not use DELETE or match the canonical behavior described in
08-hypervisor-online-commit.md); reference the symbols
VIR_DOMAIN_BLOCK_COMMIT_ACTIVE, SHALLOW, RELATIVE, and
VIR_DOMAIN_BLOCK_COMMIT_DELETE and align wording to the other doc for
consistency.

In
`@docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/00-overview.md`:
- Around line 54-87: The fenced ASCII diagram block is missing a language
specifier which triggers MD040; add a language token (e.g., ```text) to the code
fence that contains the architecture diagram (the block showing
VolumeSnapshotTreeBase, VolumeSnapshotTreeReconciler,
GetVolumeBackingChainFromPrimaryStorageMsg and vm_plugin.py / *_plugin.py) so
the markdown linter treats it as plain text and rendering is consistent.

In
`@docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/02-data-plane-validation.md`:
- Around line 212-222: The fenced code block containing the state-machine
diagram should include a language tag to avoid MD040; change the opening
backticks from ``` to ```text so the block becomes ```text ... ```; update the
specific block that starts with "NOT_STARTED → RUNNING ──(timeout)──► FAILED →
raise" and ensure the closing backticks remain, leaving the diagram content
unchanged.

In
`@docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/04-testing-strategy.md`:
- Around line 5-13: The fenced code block uses triple backticks with no language
specifier, triggering MD040; update the opening fence from ``` to ```text for
the block that contains the pyramid ASCII (the lines starting with
"┌──────────────────────┐" and showing "E2E (~5 cases)", "Integration (~30)",
"Unit (~100)") so the code block is explicitly marked as plain text and the
MD040 lint warning is resolved.

In
`@docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/05-rollout-plan.md`:
- Around line 5-18: The fenced code block that starts with "Phase 1 (周 1):默认
false 上线" should declare a language to satisfy markdownlint MD040; update the
opening fence from ``` to ```text so the block becomes ```text ... ```
preserving all lines (Phase 1..Phase 4) unchanged, which will stop the
"fenced-code-language" warning.

---

Nitpick comments:
In
`@docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/01-control-plane-reconciler.md`:
- Around line 31-42: The fenced pseudocode for reconcile(treeUuid, volumeUuid,
trigger) lacks a language identifier which hurts rendering; update the fenced
code block to include a language tag like ```text or ```pseudocode so the block
renders correctly (i.e., change the opening fence from ``` to ```text and keep
the existing contents), referencing the reconcile function header to locate the
block.
- Around line 151-157: The fenced log example for the
VolumeSnapshotTreeReconciler snippet should include a language identifier to
improve rendering; update the markdown block that contains the log lines
starting with "[VolumeSnapshotTreeReconciler] tree=<uuid>
trigger=AfterCommitSuccess" to use a suitable info string such as `text` or
`log` (e.g., replace the opening ``` with ```text) so the lines including
"inconsistencies: I3(...)" and "applied: UPDATE_DB_PARENT_UUID(...)" render
correctly.
- Around line 135-137: The fenced flow diagram block containing the exact line
"vm 队列 ──► chainSubmit ──► commit/pull flow ──► done/error ──► reconciler ──►
comp.success/fail ──► chainSubmit 释放 ──► vm 队列释放" should add a language
identifier after the opening backticks (for example ```text or ```mermaid) so
the renderer applies proper formatting; update that code fence to ```text (or
another appropriate language) and keep the diagram content unchanged.
- Line 54: Refactor the I3b row into its own subsection (e.g., 5.3.1) so the
three subcases become separate bullet items: clearly list detection conditions
and remediation for each subcase referencing the existing symbols (I3b,
parentUuid, db.parentUuid, Q(VolumeSnapshotVO).eq(uuid, parentUuid),
physical.backing, UPDATE_DB_PARENT_UUID, remaining, stepDelete); for (a)
describe detection “physical.backing can be reverse-resolved to an alive VO” and
action “UPDATE_DB_PARENT_UUID = that VO.uuid”; for (b) describe detection
“physical.backing == null (rebase to volume base)” and action
“UPDATE_DB_PARENT_UUID(null)”; for (c) describe detection “physical.backing
exists but cannot resolve to any alive VO (points to stepDeleted VO path,
physical rebase pending)” and action “leave DB unchanged and mark remaining for
retry after physical rebase.”
🪄 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: abd5cdab-f4cf-4f82-a56e-c82f214faee1

📥 Commits

Reviewing files that changed from the base of the PR and between 5cfabbf and 30d2503.

📒 Files selected for processing (25)
  • docs/snapshot-single-delete/00-overview.md
  • docs/snapshot-single-delete/01-api-and-fields.md
  • docs/snapshot-single-delete/02-call-chain.md
  • docs/snapshot-single-delete/03-direction-resolution.md
  • docs/snapshot-single-delete/04-scope-and-stepDelete.md
  • docs/snapshot-single-delete/05-commit-db-swap.md
  • docs/snapshot-single-delete/06-pull-db-rewrite.md
  • docs/snapshot-single-delete/07-group-passthrough.md
  • docs/snapshot-single-delete/08-hypervisor-online-commit.md
  • docs/snapshot-single-delete/09-agent-qemu-img.md
  • docs/snapshot-single-delete/10-storage-backend-matrix.md
  • docs/snapshot-single-delete/11-sibling-rebase.md
  • docs/snapshot-single-delete/12-fullrebase-and-cleanup.md
  • docs/snapshot-single-delete/13-premium-and-cdp.md
  • docs/snapshot-single-delete/14-limitations-and-todos.md
  • docs/snapshot-single-delete/scenarios/00-index.md
  • docs/snapshot-single-delete/scenarios/01-multi-children-stepDelete.md
  • docs/snapshot-single-delete/scenarios/02-local-running-delete-mid-with-3-children.md
  • docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/00-overview.md
  • docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/01-control-plane-reconciler.md
  • docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/02-data-plane-validation.md
  • docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/03-flowchain-recovery.md
  • docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/04-testing-strategy.md
  • docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/05-rollout-plan.md
  • docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/06-invariants-and-scope.md

Comment on lines +32 to +44
```
[祖父] ── [待删节点 X] ── [子 Y] ── ...
┌──────────┴───────────┐
│ scope=single │
│ direction=commit │ 在线VM 且 X≠latest
│ → Y 差量写入 X 文件 │
│ → DB: 互换 path, Y.parent=X.parent
│ direction=pull │ 离线 或 X=latest
│ → 祖父+X 合并入 Y(rebase)
│ → DB: Y.parent = X.parent
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

为代码块补充语言标识以通过 Markdown lint。

Line 32 的 fenced code block 缺少语言类型,当前会触发 MD040。建议改为 ```text(或 mermaid,若后续改成可渲染图)。

🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 32-32: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 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/snapshot-single-delete/00-overview.md` around lines 32 - 44, The fenced
code block in docs/snapshot-single-delete/00-overview.md (the ASCII diagram
showing "[祖父] ── [待删节点 X] ── [子 Y] ...") is missing a language tag and triggers
MD040; update the opening fence from ``` to ```text (or ```mermaid if you
convert it to a renderable diagram) so the block is annotated and the linter
passes, ensuring the diagram remains unchanged.

Comment on lines +5 to +20
```
APIDeleteVolumeSnapshotGroupMsg
└─ VolumeSnapshotGroupBase.handle() GroupBase.java:163
└─ handleDelete() GroupBase.java:187
└─ DeleteVolumeSnapshotGroupInnerMsg (携带 scope/direction)
└─ While 循环每个 VolumeSnapshotVO GroupBase.java:212
└─ DeleteVolumeSnapshotMsg(scope,direction)
└─ VolumeSnapshotTreeBase
└─ deletion() TreeBase.java:358
├─ scope=chain → deleteChainFlows() :487
└─ scope=single → deleteSingleFlows() :828
└─ stepDelete() :875
├─ 叶节点 → deleteVolumeSnapshotAndSyncVolumeSize
├─ 单子节点 → resolveDirection → commit() / pull()
└─ 多子节点 → pull() (强制)
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

处理链路代码块缺少语言声明(MD040)。

Line 5 的代码围栏建议改成 ```text,避免 markdownlint 报警并提升渲染一致性。

🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 5-5: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 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/snapshot-single-delete/02-call-chain.md` around lines 5 - 20, The code
block in the call chain snippet lacks a language tag which triggers markdownlint
MD040; update the opening fence from ``` to ```text so the block is explicitly
marked as plain text (affecting the snippet that shows
APIDeleteVolumeSnapshotGroupMsg, VolumeSnapshotGroupBase.handle(),
handleDelete(), deleteChainFlows(), deleteSingleFlows(), stepDelete(), etc.),
ensuring consistent rendering and silencing the MD040 warning.

Comment on lines +5 to +22
```
Commit 前:
dst.qcow2 ←backing— src.qcow2 ←backing— grandchild.qcow2
父 子 孙

blockCommit(top=src, base=dst) 完成后:
dst.qcow2 内容 = 原 src 内容(src 的 delta 已 flush 进 dst 文件)
src.qcow2 已被 DELETE(VIR_DOMAIN_BLOCK_COMMIT_DELETE)或将被回收

期望逻辑:
src(保留) ← grandchild ← 但 uuid 不变,所以用 path 互换实现:

DB 互换:
dst.installPath ← src 旧 path (dst 记录"指"已合并的文件)
src.installPath ← dst 旧 path (src 记录"指"待回收的文件)
src.parentUuid ← dst.parentUuid (跨过 dst)
src.distance -= 1
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

为时序示意代码块添加语言标记。

该代码块建议使用 text 标注,避免 MD040 告警并提升渲染一致性。

建议修改示例
-```
+```text
 Commit 前:
   dst.qcow2  ←backing—  src.qcow2  ←backing—  grandchild.qcow2
 ...
-```
+```
📝 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
```
Commit 前:
dst.qcow2 ←backing— src.qcow2 ←backing— grandchild.qcow2
父 子 孙
blockCommit(top=src, base=dst) 完成后:
dst.qcow2 内容 = 原 src 内容(src 的 delta 已 flush 进 dst 文件)
src.qcow2 已被 DELETE(VIR_DOMAIN_BLOCK_COMMIT_DELETE)或将被回收
期望逻辑:
src(保留) ← grandchild ← 但 uuid 不变,所以用 path 互换实现:
DB 互换:
dst.installPath ← src 旧 path (dst 记录"指"已合并的文件)
src.installPath ← dst 旧 path (src 记录"指"待回收的文件)
src.parentUuid ← dst.parentUuid (跨过 dst)
src.distance -= 1
```
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 5-5: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 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/snapshot-single-delete/05-commit-db-swap.md` around lines 5 - 22, Update
the fenced code block in the commit DB swap example to include an explicit
language marker by changing the opening backticks from ``` to ```text so the
block starts with ```text and ends with ```, preventing MD040 and ensuring
consistent rendering; apply this to the block that shows "Commit 前:" /
"blockCommit(top=src, base=dst) 完成后:" etc.

Comment on lines +5 to +19
```
Pull 前:
grandparent ← src(待删) ← dst(子) ← descendants

qemu-img rebase(dst → grandparent) 完成后:
dst.qcow2 文件中数据 = 原 dst delta + 原 src delta(合并)
dst 的 backing file = grandparent
src.qcow2 待删除

DB 改写:
dst.parentUuid ← src.parentUuid (跨过 src)
dst.distance -= 1
dst.size = 合并后的实际大小
所有后代 distance -1
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

补充代码块 language 以通过 markdownlint。

Line 5 的 fenced code block 未标注语言,建议加 text(或更具体语言)以修复 MD040。

🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 5-5: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 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/snapshot-single-delete/06-pull-db-rewrite.md` around lines 5 - 19, The
fenced code block starting with ``` (containing "Pull 前:", "qemu-img rebase",
and "DB 改写:") lacks a language tag which triggers MD040; fix it by adding a
language identifier (e.g. `text`) after the opening backticks so the block
begins with ```text, leaving the block contents unchanged and ensuring
markdownlint passes.

Comment thread docs/snapshot-single-delete/08-hypervisor-online-commit.md
Comment on lines +128 to +146
1. virDomainBlockCommit(disk, base=2.qcow2, top=5.qcow2,
flags=VIR_DOMAIN_BLOCK_COMMIT_ACTIVE | SHALLOW)
→ libvirt 把 5 中尚未在 2 的数据 flush 到 2.qcow2
→ 进入 READY 态(active commit 特征)
2. _wait_for_block_job → READY
3. virDomainBlockJobAbort(disk, flags=VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT)
→ VM disk source 从 5.qcow2 → 切到 2.qcow2
4. for child in srcChildrenInstallPathInDb=[vol.installPath]:
if qcow2_get_backing_file(child) != base:
qcow2_rebase_no_check(base, child)
→ 本例 vol 即 5.qcow2 自身,pivot 后 VM 已切到 2.qcow2,通常 noop
```

完成后物理:

```
2.qcow2 内容:原 5 的全部数据已合并进来
5.qcow2 物理文件:libvirt 在 pivot 时删除(VIR_DOMAIN_BLOCK_COMMIT_DELETE)
VM 活跃盘 source:2.qcow2
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

在线 commit flag 描述与同 PR 其他文档冲突,需统一。

Line 129 写的是 VIR_DOMAIN_BLOCK_COMMIT_ACTIVE | SHALLOW,且 Line 145 将删除归因于 VIR_DOMAIN_BLOCK_COMMIT_DELETE。但在 08-hypervisor-online-commit.md 中,active commit 描述为 ACTIVE | RELATIVEDELETE 仅用于非 active commit。这里建议统一为同一套语义,否则会误导排障和实现对齐。

建议修订示例
-                          flags=VIR_DOMAIN_BLOCK_COMMIT_ACTIVE | SHALLOW)
+                          flags=VIR_DOMAIN_BLOCK_COMMIT_ACTIVE | RELATIVE)
...
-5.qcow2 物理文件:libvirt 在 pivot 时删除(VIR_DOMAIN_BLOCK_COMMIT_DELETE)
+5.qcow2 物理文件:active commit 通过 pivot 脱离活跃链后由后续清理流程处理(非 DELETE flag 语义)
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 143-143: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 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/snapshot-single-delete/scenarios/02-local-running-delete-mid-with-3-children.md`
around lines 128 - 146, Summary: This doc uses inconsistent online-commit flag
semantics (VIR_DOMAIN_BLOCK_COMMIT_ACTIVE | SHALLOW and
VIR_DOMAIN_BLOCK_COMMIT_DELETE) that conflict with the canonical mapping in
08-hypervisor-online-commit.md (ACTIVE | RELATIVE, DELETE only for non-active
commits). Fix: update the virDomainBlockCommit example to use the canonical
flags (replace VIR_DOMAIN_BLOCK_COMMIT_ACTIVE | SHALLOW with the same ACTIVE |
RELATIVE combination used in 08-hypervisor-online-commit.md) and adjust the
post-pivot deletion sentence so it does not attribute deletion to
VIR_DOMAIN_BLOCK_COMMIT_DELETE for an active commit (either state that pivoting
does not use DELETE or match the canonical behavior described in
08-hypervisor-online-commit.md); reference the symbols
VIR_DOMAIN_BLOCK_COMMIT_ACTIVE, SHALLOW, RELATIVE, and
VIR_DOMAIN_BLOCK_COMMIT_DELETE and align wording to the other doc for
consistency.

Comment on lines +54 to +87
```
┌─────────────────────────────────────────────┐
用户 / API │ 控制面(zstack management) │
APIDeleteVolumeSnapshotMsg │
│ │ ┌─────────────────────────┐ │
▼ │ │ VolumeSnapshotTreeBase │ │
VolumeSnapshotTreeBase │ │ deletion() │ │
│ │ │ stepDelete() │ │
│ commit/pull/del │ └────────┬─────────────────┘ │
▼ │ │ success/fail │
FlowChain │ ▼ │
│ │ ┌─────────────────────────┐ │
│ each step ends │ │ VolumeSnapshotTreeReconciler (新) │
└────────────────►│ │ reconcile(treeUuid) │
│ │ 1) 拉物理 backing chain │
│ │ 2) 与 DB 比对 │
│ │ 3) 输出 fix actions(受限动作集) │
│ │ 4) 顺序执行;记 remaining │
│ └────────┬─────────────────┘ │
│ │ │
└───────────┼────────────────────────────────────┘
│ GetVolumeBackingChainFromPrimaryStorageMsg
┌─────────────────────────────────────────────┐
│ 数据面(kvm agent) │
│ │
│ vm_plugin.py / *_plugin.py │
│ ├─ L1 操作前 dump chain → recovery file │
│ ├─ qemu-img commit/rebase 主操作 │
│ ├─ L2 操作后 verify_backing_chain │
│ ├─ L3 异常路径 qemu-img check │
│ └─ L4 _wait_for_block_job 状态机加固 │
└─────────────────────────────────────────────┘
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

架构图代码块建议声明语言。

Line 54 的代码块缺少语言标识,建议加 text,避免 MD040 并提升渲染一致性。

🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 54-54: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 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/specs/2026-05-13-snapshot-single-delete-hardening/00-overview.md`
around lines 54 - 87, The fenced ASCII diagram block is missing a language
specifier which triggers MD040; add a language token (e.g., ```text) to the code
fence that contains the architecture diagram (the block showing
VolumeSnapshotTreeBase, VolumeSnapshotTreeReconciler,
GetVolumeBackingChainFromPrimaryStorageMsg and vm_plugin.py / *_plugin.py) so
the markdown linter treats it as plain text and rendering is consistent.

Comment on lines +212 to +222
```
NOT_STARTED → RUNNING ──(timeout)──► FAILED → raise
│ ready event
READY ──(timeout)──► FAILED → blockJobAbort(no pivot) → CANCELLED → raise
│ blockJobAbort(PIVOT)
PIVOTED ──verify domain XML source==base──► COMPLETED
│ no
▼ FAILED → raise
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

状态机示意代码块建议补充语言标记。

这里建议使用 text 作为 fenced code language,避免 MD040 告警。

建议修改示例
-```
+```text
 NOT_STARTED → RUNNING ──(timeout)──► FAILED → raise
 ...
-```
+```
📝 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
```
NOT_STARTED → RUNNING ──(timeout)──► FAILED → raise
│ ready event
READY ──(timeout)──► FAILED → blockJobAbort(no pivot) → CANCELLED → raise
│ blockJobAbort(PIVOT)
PIVOTED ──verify domain XML source==base──► COMPLETED
│ no
▼ FAILED → raise
```
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 212-212: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 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/specs/2026-05-13-snapshot-single-delete-hardening/02-data-plane-validation.md`
around lines 212 - 222, The fenced code block containing the state-machine
diagram should include a language tag to avoid MD040; change the opening
backticks from ``` to ```text so the block becomes ```text ... ```; update the
specific block that starts with "NOT_STARTED → RUNNING ──(timeout)──► FAILED →
raise" and ensure the closing backticks remain, leaving the diagram content
unchanged.

Comment on lines +5 to +13
```
┌──────────────────────┐
│ E2E (~5 cases) │
├──────────────────────┤
│ Integration (~30) │
├──────────────────────┤
│ Unit (~100) │
└──────────────────────┘
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

测试金字塔图示代码块缺少语言标识。

Line 5 建议将围栏改为 ```text,可消除 MD040 并保持文档规范一致。

🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 5-5: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 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/specs/2026-05-13-snapshot-single-delete-hardening/04-testing-strategy.md`
around lines 5 - 13, The fenced code block uses triple backticks with no
language specifier, triggering MD040; update the opening fence from ``` to
```text for the block that contains the pyramid ASCII (the lines starting with
"┌──────────────────────┐" and showing "E2E (~5 cases)", "Integration (~30)",
"Unit (~100)") so the code block is explicitly marked as plain text and the
MD040 lint warning is resolved.

Comment on lines +5 to +18
```
Phase 1 (周 1):默认 false 上线
- 仅日志旁路:reconcile 跑但不执行 FixAction
- 验证检测准确率

Phase 2 (周 2):测试环境开启
- 全测试集群 enabled=true,跑 E2E + 压力

Phase 3 (周 3-4):开发/UAT 集群灰度
- 一台真实业务集群打开,观察一周

Phase 4 (周 5+):默认开启
- release notes,保留 GlobalConfig 关闭通道
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

灰度阶段代码块建议声明语言,避免 MD040。

Line 5 的围栏代码块建议使用 ```text,这样 markdownlint 不会报 fenced-code-language。

🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 5-5: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 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/specs/2026-05-13-snapshot-single-delete-hardening/05-rollout-plan.md`
around lines 5 - 18, The fenced code block that starts with "Phase 1 (周 1):默认
false 上线" should declare a language to satisfy markdownlint MD040; update the
opening fence from ``` to ```text so the block becomes ```text ... ```
preserving all lines (Phase 1..Phase 4) unchanged, which will stop the
"fenced-code-language" warning.

@MatheMatrix MatheMatrix force-pushed the sync/tao.gan/5.1.0-ZSV-10538@@3 branch from 30d2503 to 88374a6 Compare May 14, 2026 02:52
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

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
`@docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/01-control-plane-reconciler.md`:
- Around line 31-42: Three fenced code blocks are using bare ``` which triggers
MD040; update each block opener to ```text so they are language-marked.
Specifically, change the block that begins with "reconcile(treeUuid, volumeUuid,
trigger):" to start with "```text", the block containing "vm 队列 ──► chainSubmit
──► commit/pull flow ──► done/error ──► reconciler ──► comp.success/fail ──►
chainSubmit 释放 ──► vm 队列释放" to "```text", and the block beginning with
"[VolumeSnapshotTreeReconciler] tree=<uuid> trigger=AfterCommitSuccess" to
"```text"; also apply the same change to the other similar fenced blocks later
in the file that show the same flow and log examples (the blocks around those
vm‑queue and reconciler log examples).
🪄 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: a73a408e-2fdf-4489-8725-fea0947b312e

📥 Commits

Reviewing files that changed from the base of the PR and between 30d2503 and 88374a6.

📒 Files selected for processing (27)
  • docs/snapshot-single-delete/00-overview.md
  • docs/snapshot-single-delete/01-api-and-fields.md
  • docs/snapshot-single-delete/02-call-chain.md
  • docs/snapshot-single-delete/03-direction-resolution.md
  • docs/snapshot-single-delete/04-scope-and-stepDelete.md
  • docs/snapshot-single-delete/05-commit-db-swap.md
  • docs/snapshot-single-delete/06-pull-db-rewrite.md
  • docs/snapshot-single-delete/07-group-passthrough.md
  • docs/snapshot-single-delete/08-hypervisor-online-commit.md
  • docs/snapshot-single-delete/09-agent-qemu-img.md
  • docs/snapshot-single-delete/10-storage-backend-matrix.md
  • docs/snapshot-single-delete/11-sibling-rebase.md
  • docs/snapshot-single-delete/12-fullrebase-and-cleanup.md
  • docs/snapshot-single-delete/13-premium-and-cdp.md
  • docs/snapshot-single-delete/14-limitations-and-todos.md
  • docs/snapshot-single-delete/scenarios/00-index.md
  • docs/snapshot-single-delete/scenarios/01-multi-children-stepDelete.md
  • docs/snapshot-single-delete/scenarios/02-local-running-delete-mid-with-3-children.md
  • docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/00-overview.md
  • docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/01-control-plane-reconciler.md
  • docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/02-data-plane-validation.md
  • docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/03-flowchain-recovery.md
  • docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/04-testing-strategy.md
  • docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/05-rollout-plan.md
  • docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/06-invariants-and-scope.md
  • storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotTreeBase.java
  • storage/src/main/java/org/zstack/storage/snapshot/VolumeTree.java
✅ Files skipped from review due to trivial changes (5)
  • docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/06-invariants-and-scope.md
  • docs/snapshot-single-delete/03-direction-resolution.md
  • docs/snapshot-single-delete/14-limitations-and-todos.md
  • docs/snapshot-single-delete/01-api-and-fields.md
  • docs/snapshot-single-delete/04-scope-and-stepDelete.md
🚧 Files skipped from review as they are similar to previous changes (3)
  • docs/snapshot-single-delete/09-agent-qemu-img.md
  • docs/snapshot-single-delete/07-group-passthrough.md
  • docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/03-flowchain-recovery.md

Comment on lines +31 to +42
```
reconcile(treeUuid, volumeUuid, trigger):
1. 读 DB:Q.New(VolumeSnapshotVO).eq(treeUuid).list()
若结果为空(dst 是树根的 commit 已完成切换到新 treeUuid 场景)
→ 通过 volumeUuid 查 latest VO,反推真实 treeUuid 重新加载
2. 读物理:对每个 alive 叶节点发 GetVolumeBackingChainFromPrimaryStorageMsg
(分叉链时多发,合并去重得到全树物理 chains)
+ GetSnapshotInstalledPathExistenceMsg
3. 比对 → InconsistencyReport[]
4. 翻译为 FixAction(受限动作集)
5. 顺序执行;失败的进 remaining
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

请为 3 处 fenced code block 增加 language 标记以消除 MD040。

当前三个代码块都使用了裸的 ```,建议分别改成 ```text(流程/时序/日志示意都适合)。

建议修改
-```
+```text
 reconcile(treeUuid, volumeUuid, trigger):
   1. 读 DB:Q.New(VolumeSnapshotVO).eq(treeUuid).list()
   ...
   5. 顺序执行;失败的进 remaining

@@
- +text
vm 队列 ──► chainSubmit ──► commit/pull flow ──► done/error ──► reconciler ──► comp.success/fail ──► chainSubmit 释放 ──► vm 队列释放

@@
-```
+```text
[VolumeSnapshotTreeReconciler] tree=<uuid> trigger=AfterCommitSuccess
  inconsistencies: I3(snap-a parentUuid mismatch), I2(orphan-file /xxx.qcow2)
  applied: UPDATE_DB_PARENT_UUID(snap-a), SCHEDULE_GC_ORPHAN_FILE(/xxx.qcow2)
  remaining: []
  duration_ms: 152
</details>


Also applies to: 135-137, 151-157

<details>
<summary>🧰 Tools</summary>

<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>

[warning] 31-31: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

</details>

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

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/specs/2026-05-13-snapshot-single-delete-hardening/01-control-plane-reconciler.md
around lines 31 - 42, Three fenced code blocks are using bare which triggers MD040; update each block opener totext so they are language-marked.
Specifically, change the block that begins with "reconcile(treeUuid, volumeUuid,
trigger):" to start with "text", the block containing "vm 队列 ──► chainSubmit ──► commit/pull flow ──► done/error ──► reconciler ──► comp.success/fail ──► chainSubmit 释放 ──► vm 队列释放" to "text", and the block beginning with
"[VolumeSnapshotTreeReconciler] tree= trigger=AfterCommitSuccess" to
"```text"; also apply the same change to the other similar fenced blocks later
in the file that show the same flow and log examples (the blocks around those
vm‑queue and reconciler log examples).


</details>

<!-- fingerprinting:phantom:triton:hawk -->

<!-- This is an auto-generated comment by CodeRabbit -->

@MatheMatrix MatheMatrix force-pushed the sync/tao.gan/5.1.0-ZSV-10538@@3 branch 2 times, most recently from d2df6e6 to 489c558 Compare May 14, 2026 10:38
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 8

🧹 Nitpick comments (1)
storage/src/main/java/org/zstack/storage/snapshot/group/VolumeSnapshotGroupCascadeExtension.java (1)

121-123: ⚡ Quick win

异常日志建议保留完整堆栈。

这里只拼接 t.getMessage() 会丢失关键定位信息,建议直接传 Throwable 给 logger。

🔧 建议修复
-            logger.warn("eoCleanup VolumeSnapshotGroupVO failed: " + t.getMessage());
+            logger.warn("eoCleanup VolumeSnapshotGroupVO failed", t);
🤖 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/snapshot/group/VolumeSnapshotGroupCascadeExtension.java`
around lines 121 - 123, The catch block in VolumeSnapshotGroupCascadeExtension
(around the eoCleanup handling) only logs t.getMessage(), losing stack trace;
change the logger call to pass the Throwable itself (e.g.,
logger.warn("eoCleanup VolumeSnapshotGroupVO failed", t)) so the full exception
and stack trace are preserved for debugging.
🤖 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 `@docs/snapshot-single-delete/bugs.md`:
- Around line 188-198: The fenced code block containing the Chinese diagnostic
text should include a language identifier to satisfy markdownlint MD040; edit
the triple-backtick opening fence for that block (the one before "物理:") to
```text (or another appropriate language) so the block becomes a labeled code
block, leaving the content unchanged.

In `@docs/snapshot-single-delete/scenarios/_query_tree.py`:
- Around line 85-88: The code builds a shell command by interpolating external
root_path into run(...), causing injection and space-handling issues; update the
call that runs qemu-img (the run function invocation around "qemu-img info
--backing-chain {root_path}") to avoid shell interpolation—either pass arguments
as a list to run so it spawns the process without a shell, or sanitize root_path
with shlex.quote before formatting into the command string; ensure the changed
call still prints the same header and captures out/err as before.
- Around line 5-8: 当前文件中存在硬编码 SSH 凭据(HOST, USER, PWD,
VM_UUID);请将这些常量替换为从环境变量或命令行参数读取(例如使用 os.environ 或
argparse),并在所需变量缺失时立即抛出异常或退出进程以避免默认回退;确保引用到的标识符 HOST, USER, PWD, VM_UUID
被替换为新的配置读取调用并在文档或启动脚本中说明必需的环境变量名。
- Around line 24-26: 不要使用 AutoAddPolicy 来自动信任主机密钥;改为在 paramiko.SSHClient 实例
client 上调用 load_system_host_keys()(或 load_host_keys(known_hosts_path))并将
set_missing_host_key_policy 改为 paramiko.RejectPolicy(),然后在调用 client.connect(...)
时捕获并处理
paramiko.ssh_exception.SSHException/paramiko.ssh_exception.BadHostKeyException/paramiko.ssh_exception.AuthenticationException
以便在主机密钥不匹配或未知时安全失败并记录清晰错误信息。

In
`@docs/snapshot-single-delete/scenarios/03-local-stopped-delete-mid-with-3-children.md`:
- Line 10: 修复 markdownlint 警告:去掉 blockquote 中的空白行以保持连续的 “>” 段落(即合并分散的 >
行,不在引用内留空行),并为所有 fenced code blocks 添加语言标识(在每个 ``` 后加上适当的语言,如 ```text 或 ```bash
等),检查文档中所有类似 blockquote 和代码块位置并统一应用这两项更改。

In
`@docs/snapshot-single-delete/scenarios/04-deleteSingleFlows-online-offline-decision.md`:
- Line 14: 文档中存在多个未声明语言的 fenced code block 导致 markdownlint MD040
警告;请将所有无语言标注的代码块统一改为使用 ```text 作为开头(即把 ``` 替换为 ```text),确保每个 fenced
block(包括评论中提到的那些示例)都带上语言标注以消除 MD040 警告。

In
`@docs/snapshot-single-delete/scenarios/05-local-stopped-direction-commit-actual.md`:
- Line 43: The markdown in
docs/snapshot-single-delete/scenarios/05-local-stopped-direction-commit-actual.md
contains multiple fenced code blocks without language identifiers; update each
opening triple-backtick to include an appropriate language hint (e.g., text,
bash, java) so markdownlint stops flagging them—apply this to the blocks
referenced (lines around 43, 81, 98, 108, 142, 160, 186, 194, 260) and pick the
closest semantic language for each snippet.

In
`@storage/src/main/java/org/zstack/storage/snapshot/group/VolumeSnapshotGroupCascadeExtension.java`:
- Around line 65-103: The handleDeletion method can throw at several steps
(vmUuidsFromAction, Q.New(...).listValues, SQL.New(...).delete,
vidm.deleteArchiveVmInstanceResourceMetadataGroup,
cleanVmHostBackupFilesForGroup, dbf.removeByPrimaryKeys) and currently may exit
without invoking the Completion callback; wrap the method body in a try-catch
that catches Throwable, log the exception with context (include
vmUuids/groupUuids and the exception) and ensure the Completion is always
invoked (call completion.success() on the normal path and completion.fail(e)
inside the catch), so the cascade always receives a terminal callback even on
errors.

---

Nitpick comments:
In
`@storage/src/main/java/org/zstack/storage/snapshot/group/VolumeSnapshotGroupCascadeExtension.java`:
- Around line 121-123: The catch block in VolumeSnapshotGroupCascadeExtension
(around the eoCleanup handling) only logs t.getMessage(), losing stack trace;
change the logger call to pass the Throwable itself (e.g.,
logger.warn("eoCleanup VolumeSnapshotGroupVO failed", t)) so the full exception
and stack trace are preserved for debugging.
🪄 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: b042a2d0-ea74-4665-b186-ce4c18acedb9

📥 Commits

Reviewing files that changed from the base of the PR and between 88374a6 and 489c558.

⛔ Files ignored due to path filters (1)
  • conf/springConfigXml/volumeSnapshot.xml is excluded by !**/*.xml
📒 Files selected for processing (13)
  • docs/snapshot-single-delete/bugs.md
  • docs/snapshot-single-delete/proposals/group-disband-symmetry-and-integrity-check.md
  • docs/snapshot-single-delete/proposals/scope-direction-api-redesign.md
  • docs/snapshot-single-delete/scenarios/03-local-stopped-delete-mid-with-3-children.md
  • docs/snapshot-single-delete/scenarios/04-deleteSingleFlows-online-offline-decision.md
  • docs/snapshot-single-delete/scenarios/05-local-stopped-direction-commit-actual.md
  • docs/snapshot-single-delete/scenarios/_query_tree.py
  • header/src/main/java/org/zstack/header/storage/snapshot/group/APIDeleteVolumeSnapshotGroupMsg.java
  • storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotTreeBase.java
  • storage/src/main/java/org/zstack/storage/snapshot/group/VolumeSnapshotGroupBase.java
  • storage/src/main/java/org/zstack/storage/snapshot/group/VolumeSnapshotGroupCascadeExtension.java
  • storage/src/main/java/org/zstack/storage/snapshot/group/VolumeSnapshotGroupChecker.java
  • storage/src/main/java/org/zstack/storage/volume/VolumeApiInterceptor.java
✅ Files skipped from review due to trivial changes (1)
  • header/src/main/java/org/zstack/header/storage/snapshot/group/APIDeleteVolumeSnapshotGroupMsg.java

Comment on lines +188 to +198
```
物理:
vol.qcow2 头部 backing = 2.qcow2(aa72…e70c)
2.qcow2 含 5+2 合并数据
5.qcow2 已被抽空但文件未删(轮 4 还没执行)

DB(仍是互换前状态):
VO_2.installPath = 2.qcow2 → 仍存在
VO_5.installPath = 5.qcow2 → 指向已被抽空的文件
vol.installPath = 5.qcow2(DB 字段一直不变)
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

为该代码块补充语言标注。

这里的 fenced code block 未指定语言,会持续触发 markdownlint MD040。建议标注为 text(或更贴切的语言)。

建议修改
-```
+```text
 物理:
   vol.qcow2 头部 backing = 2.qcow2(aa72…e70c)
   2.qcow2 含 5+2 合并数据
   5.qcow2 已被抽空但文件未删(轮 4 还没执行)

 DB(仍是互换前状态):
   VO_2.installPath = 2.qcow2  → 仍存在
   VO_5.installPath = 5.qcow2  → 指向已被抽空的文件
   vol.installPath  = 5.qcow2(DB 字段一直不变)
</details>

<!-- suggestion_start -->

<details>
<summary>📝 Committable suggestion</summary>

> ‼️ **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.

```suggestion

🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 188-188: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 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/snapshot-single-delete/bugs.md` around lines 188 - 198, The fenced code
block containing the Chinese diagnostic text should include a language
identifier to satisfy markdownlint MD040; edit the triple-backtick opening fence
for that block (the one before "物理:") to ```text (or another appropriate
language) so the block becomes a labeled code block, leaving the content
unchanged.

Comment on lines +5 to +8
HOST = "172.26.53.180"
USER = "root"
PWD = "admin@123"
VM_UUID = "fa51c9637c024d94a556dd474a5cd74e"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

请移除仓库中的明文 SSH 凭据。

硬编码 root 凭据会导致凭据泄露和横向风险,建议改为环境变量/命令行参数注入,并在缺失时直接失败。

建议修改
+import os
+
-HOST = "172.26.53.180"
-USER = "root"
-PWD = "admin@123"
-VM_UUID = "fa51c9637c024d94a556dd474a5cd74e"
+HOST = os.getenv("ZSTACK_TRACE_HOST")
+USER = os.getenv("ZSTACK_TRACE_USER", "root")
+PWD = os.getenv("ZSTACK_TRACE_PASSWORD")
+VM_UUID = os.getenv("ZSTACK_TRACE_VM_UUID")
+
+if not HOST or not PWD or not VM_UUID:
+    raise SystemExit("missing required env: ZSTACK_TRACE_HOST/ZSTACK_TRACE_PASSWORD/ZSTACK_TRACE_VM_UUID")
🧰 Tools
🪛 Ruff (0.15.12)

[error] 7-7: Possible hardcoded password assigned to: "PWD"

(S105)

🤖 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/snapshot-single-delete/scenarios/_query_tree.py` around lines 5 - 8,
当前文件中存在硬编码 SSH 凭据(HOST, USER, PWD, VM_UUID);请将这些常量替换为从环境变量或命令行参数读取(例如使用
os.environ 或 argparse),并在所需变量缺失时立即抛出异常或退出进程以避免默认回退;确保引用到的标识符 HOST, USER, PWD,
VM_UUID 被替换为新的配置读取调用并在文档或启动脚本中说明必需的环境变量名。

Comment on lines +24 to +26
client = paramiko.SSHClient()
client.set_missing_host_key_policy(paramiko.AutoAddPolicy())
client.connect(HOST, username=USER, password=PWD, timeout=10)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# 只读验证:定位是否仍存在自动信任策略
rg -n "AutoAddPolicy|set_missing_host_key_policy" docs/snapshot-single-delete/scenarios/_query_tree.py

Repository: MatheMatrix/zstack

Length of output: 130


避免自动信任未知主机密钥。

AutoAddPolicy 会无条件信任首次连接主机,存在中间人攻击风险。改为加载已知主机并使用 RejectPolicy

修改建议
 client = paramiko.SSHClient()
-client.set_missing_host_key_policy(paramiko.AutoAddPolicy())
+client.load_system_host_keys()
+client.set_missing_host_key_policy(paramiko.RejectPolicy())
📝 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
client = paramiko.SSHClient()
client.set_missing_host_key_policy(paramiko.AutoAddPolicy())
client.connect(HOST, username=USER, password=PWD, timeout=10)
client = paramiko.SSHClient()
client.load_system_host_keys()
client.set_missing_host_key_policy(paramiko.RejectPolicy())
client.connect(HOST, username=USER, password=PWD, timeout=10)
🧰 Tools
🪛 Ruff (0.15.12)

[error] 25-25: Paramiko call with policy set to automatically trust the unknown host key

(S507)

🤖 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/snapshot-single-delete/scenarios/_query_tree.py` around lines 24 - 26,
不要使用 AutoAddPolicy 来自动信任主机密钥;改为在 paramiko.SSHClient 实例 client 上调用
load_system_host_keys()(或 load_host_keys(known_hosts_path))并将
set_missing_host_key_policy 改为 paramiko.RejectPolicy(),然后在调用 client.connect(...)
时捕获并处理
paramiko.ssh_exception.SSHException/paramiko.ssh_exception.BadHostKeyException/paramiko.ssh_exception.AuthenticationException
以便在主机密钥不匹配或未知时安全失败并记录清晰错误信息。

Comment on lines +85 to +88
if root_path:
print(f"\n--- qemu-img info --backing-chain {root_path} ---")
out, err = run(client, f"qemu-img info --backing-chain {root_path} 2>&1")
print(out)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

root_path 做 shell 转义后再执行命令。

root_path 来自外部数据源(数据库),直接拼接到 shell 命令会带来注入风险,也可能在路径含空格时执行失败。

建议修改
+import shlex
 ...
 if root_path:
     print(f"\n--- qemu-img info --backing-chain {root_path} ---")
-    out, err = run(client, f"qemu-img info --backing-chain {root_path} 2>&1")
+    safe_root_path = shlex.quote(root_path)
+    out, err = run(client, f"qemu-img info --backing-chain {safe_root_path} 2>&1")
     print(out)
🤖 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/snapshot-single-delete/scenarios/_query_tree.py` around lines 85 - 88,
The code builds a shell command by interpolating external root_path into
run(...), causing injection and space-handling issues; update the call that runs
qemu-img (the run function invocation around "qemu-img info --backing-chain
{root_path}") to avoid shell interpolation—either pass arguments as a list to
run so it spawns the process without a shell, or sanitize root_path with
shlex.quote before formatting into the command string; ensure the changed call
still prints the same header and captures out/err as before.

> - `VolumeTree.java` 行 364-392(resolveDirection / isOnline)
> - `LocalStorageKvmBackend.java` 行 3845-3865(PullVolumeSnapshotOnPrimaryStorageMsg → OFFLINE_MERGE_PATH)
> - `localstorage.py` 行 834-856(`offline_merge_snapshot`)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

清理 markdownlint 告警(MD028 + MD040)。

  • Line 10:去掉 blockquote 内空白行(保持连续 > 段落)。
  • 多处代码块:补充 language 标识。
🔧 示例修复
-> - `localstorage.py` 行 834-856(`offline_merge_snapshot`)
->
+> - `localstorage.py` 行 834-856(`offline_merge_snapshot`)

-```
+```text
  快照1
   └─ 快照2
 ...
-```
+```

Also applies to: 27-27, 37-37, 97-97, 129-129, 143-143, 162-162, 177-177, 207-207, 241-241

🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 10-10: Blank line inside blockquote

(MD028, no-blanks-blockquote)

🤖 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/snapshot-single-delete/scenarios/03-local-stopped-delete-mid-with-3-children.md`
at line 10, 修复 markdownlint 警告:去掉 blockquote 中的空白行以保持连续的 “>” 段落(即合并分散的 >
行,不在引用内留空行),并为所有 fenced code blocks 添加语言标识(在每个 ``` 后加上适当的语言,如 ```text 或 ```bash
等),检查文档中所有类似 blockquote 和代码块位置并统一应用这两项更改。


### 极简决策图

```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

为这些文本图示代码块统一补充语言标注。

当前多个 fenced code block 未声明语言,markdownlint MD040 会持续告警。建议统一改为 ```text

Also applies to: 54-54, 78-78, 155-155, 285-285

🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 14-14: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 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/snapshot-single-delete/scenarios/04-deleteSingleFlows-online-offline-decision.md`
at line 14, 文档中存在多个未声明语言的 fenced code block 导致 markdownlint MD040
警告;请将所有无语言标注的代码块统一改为使用 ```text 作为开头(即把 ``` 替换为 ```text),确保每个 fenced
block(包括评论中提到的那些示例)都带上语言标注以消除 MD040 警告。


### 2.2 物理 backing chain(操作前)

```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

建议为所有代码块补充语言标识,避免 markdownlint 告警。

这些 fenced code block 当前都未指定 language,建议统一补齐(可用 text/bash/java 等最接近语义的类型)。

🔧 示例修复
-```
+```text
 imagecache/template/e4e3cca9…e5c.qcow2   (镜像基线,只读)
    ↑
 ...
-```
+```

Also applies to: 81-81, 98-98, 108-108, 142-142, 160-160, 186-186, 194-194, 260-260

🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 43-43: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 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/snapshot-single-delete/scenarios/05-local-stopped-direction-commit-actual.md`
at line 43, The markdown in
docs/snapshot-single-delete/scenarios/05-local-stopped-direction-commit-actual.md
contains multiple fenced code blocks without language identifiers; update each
opening triple-backtick to include an appropriate language hint (e.g., text,
bash, java) so markdownlint stops flagging them—apply this to the blocks
referenced (lines around 43, 81, 98, 108, 142, 160, 186, 194, 260) and pick the
closest semantic language for each snippet.

Comment on lines +65 to +103
private void handleDeletion(CascadeAction action, Completion completion) {
if (!VmInstanceVO.class.getSimpleName().equals(action.getParentIssuer())) {
completion.success();
return;
}

List<String> vmUuids = vmUuidsFromAction(action);
if (vmUuids.isEmpty()) {
completion.success();
return;
}

List<String> groupUuids = Q.New(VolumeSnapshotGroupVO.class)
.select(VolumeSnapshotGroupVO_.uuid)
.in(VolumeSnapshotGroupVO_.vmInstanceUuid, vmUuids)
.listValues();
if (groupUuids.isEmpty()) {
completion.success();
return;
}

logger.debug(String.format("VM destroy cascade: force-removing %d snapshot group(s) %s for vm(s) %s " +
"(includes any incomplete groups from prior single-snapshot deletions)",
groupUuids.size(), groupUuids, vmUuids));

// 1. drop all refs first (FK-like constraint via business logic)
SQL.New(VolumeSnapshotGroupRefVO.class)
.in(VolumeSnapshotGroupRefVO_.volumeSnapshotGroupUuid, groupUuids)
.delete();

// 2. clean associated metadata + backup files
groupUuids.forEach(vidm::deleteArchiveVmInstanceResourceMetadataGroup);
cleanVmHostBackupFilesForGroup(groupUuids);

// 3. remove group VOs
dbf.removeByPrimaryKeys(groupUuids, VolumeSnapshotGroupVO.class);

completion.success();
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

handleDeletion 缺少异常兜底,可能导致 completion 回调未执行。

当前清理链路中任一步抛出运行时异常时,方法会提前退出,completion.success() 不会执行,可能影响级联删除流程稳定性。建议至少保证回调必达,并记录异常。

🛠️ 建议修复
 private void handleDeletion(CascadeAction action, Completion completion) {
@@
-        // 1. drop all refs first (FK-like constraint via business logic)
-        SQL.New(VolumeSnapshotGroupRefVO.class)
-                .in(VolumeSnapshotGroupRefVO_.volumeSnapshotGroupUuid, groupUuids)
-                .delete();
-
-        // 2. clean associated metadata + backup files
-        groupUuids.forEach(vidm::deleteArchiveVmInstanceResourceMetadataGroup);
-        cleanVmHostBackupFilesForGroup(groupUuids);
-
-        // 3. remove group VOs
-        dbf.removeByPrimaryKeys(groupUuids, VolumeSnapshotGroupVO.class);
-
-        completion.success();
+        try {
+            // 1. drop all refs first (FK-like constraint via business logic)
+            SQL.New(VolumeSnapshotGroupRefVO.class)
+                    .in(VolumeSnapshotGroupRefVO_.volumeSnapshotGroupUuid, groupUuids)
+                    .delete();
+
+            // 2. clean associated metadata + backup files
+            groupUuids.forEach(vidm::deleteArchiveVmInstanceResourceMetadataGroup);
+            cleanVmHostBackupFilesForGroup(groupUuids);
+
+            // 3. remove group VOs
+            dbf.removeByPrimaryKeys(groupUuids, VolumeSnapshotGroupVO.class);
+        } catch (Throwable t) {
+            logger.warn(String.format("VM destroy cascade cleanup failed for vm(s) %s, group(s) %s", vmUuids, groupUuids), t);
+        } finally {
+            completion.success();
+        }
 }
🤖 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/snapshot/group/VolumeSnapshotGroupCascadeExtension.java`
around lines 65 - 103, The handleDeletion method can throw at several steps
(vmUuidsFromAction, Q.New(...).listValues, SQL.New(...).delete,
vidm.deleteArchiveVmInstanceResourceMetadataGroup,
cleanVmHostBackupFilesForGroup, dbf.removeByPrimaryKeys) and currently may exit
without invoking the Completion callback; wrap the method body in a try-catch
that catches Throwable, log the exception with context (include
vmUuids/groupUuids and the exception) and ensure the Completion is always
invoked (call completion.success() on the normal path and completion.fail(e)
inside the catch), so the cascade always receives a terminal callback even on
errors.

taogan21 added 5 commits May 14, 2026 18:51
…arios, P0 design)

Add the snapshot-single-delete documentation tree under docs/:

  00-overview.md .. 14-limitations-and-todos.md - top-level walkthrough
      of the single snapshot delete path: APIs/fields, call chain,
      direction resolution, scope/stepDelete, commit DB swap, pull DB
      rewrite, group passthrough, hypervisor online commit, agent
      qemu-img, storage backend matrix, sibling rebase, full rebase &
      cleanup, premium/CDP, limitations & TODOs.

  scenarios/00-index.md, 01-multi-children-stepDelete.md,
  02-local-running-delete-mid-with-3-children.md - end-to-end traces
      keyed on real ZSV captures.

  P0-fix-isOnline-decoupling/ - design notes for the P0 fix that
      decouples alive-chain membership from vmState in VolumeTree:
      00-overview, 01-control-plane-reconciler, 02-data-plane-validation,
      03-flowchain-recovery, 04-testing-strategy, 05-rollout-plan,
      06-invariants-and-scope.

Resolves: ZSV-10538

Change-Id: I75666b6a646a64796163756f6f766c7368777569
…eTree

VolumeTree previously conflated two independent concepts in isOnline /
resolveDirection:
  (a) is the snapshot on the volume's live backing chain (a structural
      property of the tree)
  (b) is the VM currently routed through the hypervisor (Running/Paused)

Two consequences:
  1. In VolumeSnapshotTreeBase.stepDelete, the multi-children "defer
     the alive-chain child to the final round" guard used isOnline,
     which returned false for every child when the VM was Stopped.
     The guard silently disappeared and child selection fell back to
     children.get(0), which could land on the volume's backing chain
     and corrupt the live chain on rebase failure.
  2. resolveDirection's shouldUseCommitStrategy was likewise gated on
     vmState, so direction=Auto + Stopped degraded to Pull (writing N
     copies of the (target - parent) delta into each child file)
     instead of a single-file Commit.

Split into two predicates:
  - isOnAliveChain(snapshotUuid): pure tree-structure query, used by
    stepDelete to identify the alive child regardless of vmState.
  - isHypervisorOperation(vmState): pure run-state predicate, used to
    pick libvirt vs primary-storage agent.

isOnline is rewritten as the compound (treeIsCurrent &&
isHypervisorOperation(vmState) && both endpoints on alive chain),
preserving its existing semantics for current callers.

resolveDirection now derives shouldUseCommitStrategy purely from tree
structure, so Auto + Stopped + alive-chain target now picks Commit.

stepDelete renames onlineChild to aliveChild and queries
isOnAliveChain, so the alive-child deferral protection now applies to
Stopped VMs as well.

Resolves: ZSV-10538

Change-Id: I6e63786d6d6b616f7a766b6c7463617572786969
Add the snapshot-single-delete documentation set under docs/:

scenarios/
  03-local-stopped-delete-mid-with-3-children.md - LocalStorage,
      VM Stopped, mid-tree delete with 3 children; full offline pull
      walk-through.
  04-deleteSingleFlows-online-offline-decision.md - decision matrix
      for deleteSingleFlows / stepDelete / resolveDirection / isOnline,
      mapping vmState x direction x scope to agent entry points.
  05-local-stopped-direction-commit-actual.md - real ZSV environment
      capture for VM Stopped + scope=single + direction=Commit on a
      5-leaf tree; corrects 3 source-reading mistakes.
  _query_tree.py - helper for inspecting snapshot tree state.

bugs.md - consolidated bug ledger for the single-delete path
  (Bug 0..7 plus newly added Bug 8/9).

proposals/
  scope-direction-api-redesign.md - API parameter cleanup for
      APIDeleteVolumeSnapshot[Group]Msg (scope/direction defaults,
      enum normalize, internal msg defaults, dead-code warn).
  group-disband-symmetry-and-integrity-check.md - design behind
      the symmetric-disband (A) and VM integrity-check (C) fixes.

Resolves: ZSV-10538

Change-Id: I7468736c7763656d62666d727971716e6d646375
ungroupAfterDeleted now mirrors ungroupAfterDeleteSingleSnapshot:
regardless of root/data volume type, a snapshot group VO is only
disbanded after ALL of its refs reach snapshotDeleted=true.

Previously the root-volume path immediately removed the group VO once
the root chain finished, leaving data-volume refs as orphans pointing
to a non-existent group. After this change the group lives until every
ref is marked deleted, which is the symmetric behavior with the
single-snapshot path and the precondition the integrity check (landed
in a separate commit) relies on.

Resolves: ZSV-10538

Change-Id: I77707a78706271697463676a756c617a796f6c61
Once symmetric disband is in place, an incomplete snapshot group (some
refs deleted, some still alive on the VM) is sticky and must not be
silently ignored by subsequent operations on the same VM.

Block on the following entries when the VM has any incomplete group
(the incomplete group itself is excluded from the check, so users
always have a path to clear the debt):

  - APIDeleteVolumeSnapshotGroupMsg (other groups): blocked unless
    a new boolean force=true is passed. force is added to the API
    as the operator bypass; other entries deliberately do NOT expose
    force.
  - APICreateVolumeSnapshotGroupMsg: blocked.
  - APIAttachDataVolumeToVmMsg / APIDetachDataVolumeFromVmMsg: blocked.

APIDestroyVmInstanceMsg is intentionally NOT blocked: the new
VolumeSnapshotGroupCascadeExtension keys on VmInstanceVO and
force-cleans every group VO (refs -> archived metadata -> host
backup files -> group VO) when the VM is destroyed, so VM teardown
is never wedged by leftover incomplete groups.

Single-snapshot APIs are also exempt by design.

Resolves: ZSV-10538

Change-Id: I717363667067726a796467787568756d6f7a706e
@MatheMatrix MatheMatrix force-pushed the sync/tao.gan/5.1.0-ZSV-10538@@3 branch from 489c558 to e02de7a Compare May 15, 2026 01:44
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (8)
docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/02-data-plane-validation.md (1)

212-222: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

状态机示意代码块需补 language tag 以消除 MD040。

这里建议把代码块起始从 ``` 改为 ```text,内容可保持不变。

🤖 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/specs/2026-05-13-snapshot-single-delete-hardening/02-data-plane-validation.md`
around lines 212 - 222, The Markdown code block containing the state machine
diagram (starting with the line "NOT_STARTED → RUNNING ──(timeout)──► FAILED →
raise") is missing a language tag; change the opening fence from ``` to ```text
to satisfy MD040 (e.g., replace the code fence that precedes the block with
"```text" and leave the block content unchanged) so the diagram is treated as
plain text.
docs/snapshot-single-delete/08-hypervisor-online-commit.md (1)

89-101: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Phase 说明代码块仍缺少语言标注

Line 89 当前仍建议改为 ```text(或 ```bash),否则会继续触发 MD040。

🤖 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/snapshot-single-delete/08-hypervisor-online-commit.md` around lines 89 -
101, The code fence containing the Phase 1/Phase 2 description should include a
language tag to satisfy MD040; update the triple-backtick start from ``` to
```text (or ```bash) for the block that begins with "Phase 1(数据同步):" and
includes symbols like blockCommit(),
blockJobAbort(VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT) and check_overlay_file so the
linter recognizes it as a text/code block.
docs/snapshot-single-delete/00-overview.md (1)

32-44: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

补充围栏代码块语言标识(MD040)

Line 32 的代码块起始围栏建议改为 ```text,避免 markdownlint 持续告警。

建议修改
-```
+```text
 [祖父] ── [待删节点 X] ── [子 Y] ── ...
               │
    ┌──────────┴───────────┐
@@
-```
+```
🤖 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/snapshot-single-delete/00-overview.md` around lines 32 - 44, Update the
fenced diagram block so the opening fence includes a language tag to silence
markdownlint (change the opening "```" to "```text" for the ASCII diagram in the
overview), leaving the closing fence as "```"; this targets the diagram block
that starts with the lines showing "[祖父] ── [待删节点 X] ── [子 Y] ── ..." and the
subsequent scope/direction bullets.
docs/snapshot-single-delete/bugs.md (1)

188-198: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

诊断示例代码块缺少 language 标识

Line 188 的围栏建议改为 ```text,避免 MD040 持续告警。

🤖 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/snapshot-single-delete/bugs.md` around lines 188 - 198, The fenced code
block starting at the example diagnostic (the block that currently begins with
``` on the snippet showing physical/DB state) is missing a language tag and
triggers MD040; update the opening fence to include a plain text language
identifier (e.g., change the opening fence to ```text) so the linter recognizes
it as a non-code text block and stops flagging MD040; locate the block by its
contents (the lines showing "物理:" and "DB(仍是互换前状态):" in
docs/snapshot-single-delete/bugs.md) and only modify the opening fence to add
"text".
docs/snapshot-single-delete/06-pull-db-rewrite.md (1)

5-19: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

补充代码块 language 标识以通过 MD040

Line 5 的 fenced block 建议使用 ```text 起始,内容可保持不变。

🤖 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/snapshot-single-delete/06-pull-db-rewrite.md` around lines 5 - 19, The
fenced code block lacks a language tag; update the opening fence to include a
language identifier (e.g., use ```text) so the block containing the terms
grandparent, src, dst, qemu-img rebase, and the DB rewrite notes
(dst.parentUuid, dst.distance, dst.size, and "所有后代 distance -1") is fenced as
```text to satisfy the MD040 rule while keeping the content unchanged.
docs/snapshot-single-delete/02-call-chain.md (1)

5-20: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

处理链路示意代码块缺少语言声明

Line 5 建议将围栏从 ``` 改为 ```text,以通过 MD040 并保持渲染一致性。

🤖 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/snapshot-single-delete/02-call-chain.md` around lines 5 - 20, The fenced
code block containing the call chain diagram (starting with
"APIDeleteVolumeSnapshotGroupMsg" and showing symbols like
VolumeSnapshotGroupBase.handle(), handleDelete(),
VolumeSnapshotTreeBase.deletion(), deleteChainFlows(), deleteSingleFlows(),
stepDelete(), deleteVolumeSnapshotAndSyncVolumeSize, resolveDirection, commit,
pull) is missing a language tag; change the opening fence from ``` to ```text to
satisfy MD040 and ensure consistent rendering.
docs/snapshot-single-delete/05-commit-db-swap.md (1)

5-22: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

时序示意代码块请显式标注语言

Line 5 的围栏建议改为 ```text,可直接消除 markdownlint MD040。

🤖 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/snapshot-single-delete/05-commit-db-swap.md` around lines 5 - 22, Change
the fenced code block that currently starts with ``` to explicitly declare the
language as text (i.e. replace the opening ``` with ```text) in the
documentation block showing the commit/db-swap example so the block is treated
as plain text and markdownlint MD040 is satisfied.
docs/snapshot-single-delete/11-sibling-rebase.md (1)

7-18: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

分叉链示例代码块建议添加语言声明

Line 7 的起始围栏建议改为 ```text,可消除 MD040 告警并统一文档风格。

🤖 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/snapshot-single-delete/11-sibling-rebase.md` around lines 7 - 18, The
fenced code block starting with the lines showing "分叉链示例:" should include a
language label to silence MD040 and match docs style; change the opening fence
from "```" to "```text" for the block containing the ASCII diagram and the lines
"X (待删) ... commit(src → X) 完成后:" so the renderer treats it as plain text and
the linter stops warning.
🧹 Nitpick comments (1)
docs/snapshot-single-delete/proposals/group-disband-symmetry-and-integrity-check.md (1)

295-295: ⚡ Quick win

补全或标注 SQL 示例为伪代码。

Line 295 的 SQL 检测脚本包含嵌套子查询且以 ... 结尾,语法不完整。建议:

  • 补全完整可执行的 SQL(如使用 EXISTS 子查询或 JOIN)
  • 或明确标注为"伪代码示例,需根据实际 schema 调整"
💡 补全示例
-| 升级公告 | 必须有 | 升级前需提供 SQL 检测脚本:`SELECT vmInstanceUuid, volumeSnapshotGroupUuid FROM VolumeSnapshotGroupRefVO WHERE snapshotDeleted=1 GROUP BY volumeSnapshotGroupUuid HAVING COUNT(*) < (SELECT COUNT(*) FROM VolumeSnapshotGroupRefVO r2 WHERE r2.volumeSnapshotGroupUuid=...)` |
+| 升级公告 | 必须有 | 升级前需提供 SQL 检测脚本(示例):<br>`SELECT g.vmInstanceUuid, g.uuid as groupUuid`<br>`FROM VolumeSnapshotGroupVO g`<br>`WHERE EXISTS (`<br>`  SELECT 1 FROM VolumeSnapshotGroupRefVO r`<br>`  WHERE r.volumeSnapshotGroupUuid = g.uuid AND r.snapshotDeleted = 1`<br>`) AND (`<br>`  SELECT COUNT(*) FROM VolumeSnapshotGroupRefVO r2`<br>`  WHERE r2.volumeSnapshotGroupUuid = g.uuid AND r2.snapshotDeleted = 0`<br>`) > 0` |
🤖 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/snapshot-single-delete/proposals/group-disband-symmetry-and-integrity-check.md`
at line 295, The SQL example in the docs uses an incomplete nested query ending
with "..."; update the line so it either contains a runnable SQL sample or is
explicitly marked as pseudocode: e.g., replace the fragment with a complete
query using EXISTS or JOIN that references VolumeSnapshotGroupRefVO and its
columns (vmInstanceUuid, volumeSnapshotGroupUuid, snapshotDeleted), for example
using EXISTS to compare counts per volumeSnapshotGroupUuid, or add a clear note
like "示例伪代码—根据实际 schema 调整" if you prefer to keep a non-executable example.
🤖 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 `@docs/snapshot-single-delete/scenarios/01-multi-children-stepDelete.md`:
- Line 114: Update the heading "## 15.6 资料 children 顺序的依赖" to use the suggested
phrasing by replacing "资料 children 顺序的依赖" with "关于 children 顺序的依赖" so the title
reads "## 15.6 关于 children 顺序的依赖".

In
`@docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/02-data-plane-validation.md`:
- Around line 16-23: 在 ChainSnapshotSet 的数据结构中补齐 metadata
字段契约:在类定义(ChainSnapshotSet)中新增 metadata 字段并明确类型与序列化约束(例如 metadata: dict[str,
str] 或 metadata: dict[str, JSONSerializable],确保能存放 pre_snap.metadata['tmp_path']
这样的字符串),并在 dump_to_file / load_from_file 实现中保证对 metadata 的正确序列化/反序列化,确保所有使用
pre_snap.metadata[...] 的代码与此约定一致;同时更新相关文档段落(6.1 数据结构)描述该字段的类型与约束。

---

Duplicate comments:
In `@docs/snapshot-single-delete/00-overview.md`:
- Around line 32-44: Update the fenced diagram block so the opening fence
includes a language tag to silence markdownlint (change the opening "```" to
"```text" for the ASCII diagram in the overview), leaving the closing fence as
"```"; this targets the diagram block that starts with the lines showing "[祖父]
── [待删节点 X] ── [子 Y] ── ..." and the subsequent scope/direction bullets.

In `@docs/snapshot-single-delete/02-call-chain.md`:
- Around line 5-20: The fenced code block containing the call chain diagram
(starting with "APIDeleteVolumeSnapshotGroupMsg" and showing symbols like
VolumeSnapshotGroupBase.handle(), handleDelete(),
VolumeSnapshotTreeBase.deletion(), deleteChainFlows(), deleteSingleFlows(),
stepDelete(), deleteVolumeSnapshotAndSyncVolumeSize, resolveDirection, commit,
pull) is missing a language tag; change the opening fence from ``` to ```text to
satisfy MD040 and ensure consistent rendering.

In `@docs/snapshot-single-delete/05-commit-db-swap.md`:
- Around line 5-22: Change the fenced code block that currently starts with ```
to explicitly declare the language as text (i.e. replace the opening ``` with
```text) in the documentation block showing the commit/db-swap example so the
block is treated as plain text and markdownlint MD040 is satisfied.

In `@docs/snapshot-single-delete/06-pull-db-rewrite.md`:
- Around line 5-19: The fenced code block lacks a language tag; update the
opening fence to include a language identifier (e.g., use ```text) so the block
containing the terms grandparent, src, dst, qemu-img rebase, and the DB rewrite
notes (dst.parentUuid, dst.distance, dst.size, and "所有后代 distance -1") is fenced
as ```text to satisfy the MD040 rule while keeping the content unchanged.

In `@docs/snapshot-single-delete/08-hypervisor-online-commit.md`:
- Around line 89-101: The code fence containing the Phase 1/Phase 2 description
should include a language tag to satisfy MD040; update the triple-backtick start
from ``` to ```text (or ```bash) for the block that begins with "Phase 1(数据同步):"
and includes symbols like blockCommit(),
blockJobAbort(VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT) and check_overlay_file so the
linter recognizes it as a text/code block.

In `@docs/snapshot-single-delete/11-sibling-rebase.md`:
- Around line 7-18: The fenced code block starting with the lines showing
"分叉链示例:" should include a language label to silence MD040 and match docs style;
change the opening fence from "```" to "```text" for the block containing the
ASCII diagram and the lines "X (待删) ... commit(src → X) 完成后:" so the renderer
treats it as plain text and the linter stops warning.

In `@docs/snapshot-single-delete/bugs.md`:
- Around line 188-198: The fenced code block starting at the example diagnostic
(the block that currently begins with ``` on the snippet showing physical/DB
state) is missing a language tag and triggers MD040; update the opening fence to
include a plain text language identifier (e.g., change the opening fence to
```text) so the linter recognizes it as a non-code text block and stops flagging
MD040; locate the block by its contents (the lines showing "物理:" and
"DB(仍是互换前状态):" in docs/snapshot-single-delete/bugs.md) and only modify the
opening fence to add "text".

In
`@docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/02-data-plane-validation.md`:
- Around line 212-222: The Markdown code block containing the state machine
diagram (starting with the line "NOT_STARTED → RUNNING ──(timeout)──► FAILED →
raise") is missing a language tag; change the opening fence from ``` to ```text
to satisfy MD040 (e.g., replace the code fence that precedes the block with
"```text" and leave the block content unchanged) so the diagram is treated as
plain text.

---

Nitpick comments:
In
`@docs/snapshot-single-delete/proposals/group-disband-symmetry-and-integrity-check.md`:
- Line 295: The SQL example in the docs uses an incomplete nested query ending
with "..."; update the line so it either contains a runnable SQL sample or is
explicitly marked as pseudocode: e.g., replace the fragment with a complete
query using EXISTS or JOIN that references VolumeSnapshotGroupRefVO and its
columns (vmInstanceUuid, volumeSnapshotGroupUuid, snapshotDeleted), for example
using EXISTS to compare counts per volumeSnapshotGroupUuid, or add a clear note
like "示例伪代码—根据实际 schema 调整" if you prefer to keep a non-executable example.
🪄 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: 096b73a5-8391-480b-9bd5-77910e71fa33

📥 Commits

Reviewing files that changed from the base of the PR and between 489c558 and e02de7a.

⛔ Files ignored due to path filters (1)
  • conf/springConfigXml/volumeSnapshot.xml is excluded by !**/*.xml
📒 Files selected for processing (39)
  • docs/snapshot-single-delete/00-overview.md
  • docs/snapshot-single-delete/01-api-and-fields.md
  • docs/snapshot-single-delete/02-call-chain.md
  • docs/snapshot-single-delete/03-direction-resolution.md
  • docs/snapshot-single-delete/04-scope-and-stepDelete.md
  • docs/snapshot-single-delete/05-commit-db-swap.md
  • docs/snapshot-single-delete/06-pull-db-rewrite.md
  • docs/snapshot-single-delete/07-group-passthrough.md
  • docs/snapshot-single-delete/08-hypervisor-online-commit.md
  • docs/snapshot-single-delete/09-agent-qemu-img.md
  • docs/snapshot-single-delete/10-storage-backend-matrix.md
  • docs/snapshot-single-delete/11-sibling-rebase.md
  • docs/snapshot-single-delete/12-fullrebase-and-cleanup.md
  • docs/snapshot-single-delete/13-premium-and-cdp.md
  • docs/snapshot-single-delete/14-limitations-and-todos.md
  • docs/snapshot-single-delete/bugs.md
  • docs/snapshot-single-delete/proposals/group-disband-symmetry-and-integrity-check.md
  • docs/snapshot-single-delete/proposals/scope-direction-api-redesign.md
  • docs/snapshot-single-delete/scenarios/00-index.md
  • docs/snapshot-single-delete/scenarios/01-multi-children-stepDelete.md
  • docs/snapshot-single-delete/scenarios/02-local-running-delete-mid-with-3-children.md
  • docs/snapshot-single-delete/scenarios/03-local-stopped-delete-mid-with-3-children.md
  • docs/snapshot-single-delete/scenarios/04-deleteSingleFlows-online-offline-decision.md
  • docs/snapshot-single-delete/scenarios/05-local-stopped-direction-commit-actual.md
  • docs/snapshot-single-delete/scenarios/_query_tree.py
  • docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/00-overview.md
  • docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/01-control-plane-reconciler.md
  • docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/02-data-plane-validation.md
  • docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/03-flowchain-recovery.md
  • docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/04-testing-strategy.md
  • docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/05-rollout-plan.md
  • docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/06-invariants-and-scope.md
  • header/src/main/java/org/zstack/header/storage/snapshot/group/APIDeleteVolumeSnapshotGroupMsg.java
  • storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotTreeBase.java
  • storage/src/main/java/org/zstack/storage/snapshot/VolumeTree.java
  • storage/src/main/java/org/zstack/storage/snapshot/group/VolumeSnapshotGroupBase.java
  • storage/src/main/java/org/zstack/storage/snapshot/group/VolumeSnapshotGroupCascadeExtension.java
  • storage/src/main/java/org/zstack/storage/snapshot/group/VolumeSnapshotGroupChecker.java
  • storage/src/main/java/org/zstack/storage/volume/VolumeApiInterceptor.java
✅ Files skipped from review due to trivial changes (9)
  • docs/snapshot-single-delete/01-api-and-fields.md
  • docs/snapshot-single-delete/03-direction-resolution.md
  • docs/snapshot-single-delete/10-storage-backend-matrix.md
  • docs/snapshot-single-delete/09-agent-qemu-img.md
  • docs/snapshot-single-delete/07-group-passthrough.md
  • docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/06-invariants-and-scope.md
  • docs/snapshot-single-delete/12-fullrebase-and-cleanup.md
  • docs/superpowers/specs/2026-05-13-snapshot-single-delete-hardening/03-flowchain-recovery.md
  • docs/snapshot-single-delete/04-scope-and-stepDelete.md
🚧 Files skipped from review as they are similar to previous changes (8)
  • header/src/main/java/org/zstack/header/storage/snapshot/group/APIDeleteVolumeSnapshotGroupMsg.java
  • storage/src/main/java/org/zstack/storage/snapshot/group/VolumeSnapshotGroupCascadeExtension.java
  • storage/src/main/java/org/zstack/storage/snapshot/group/VolumeSnapshotGroupChecker.java
  • docs/snapshot-single-delete/13-premium-and-cdp.md
  • storage/src/main/java/org/zstack/storage/volume/VolumeApiInterceptor.java
  • storage/src/main/java/org/zstack/storage/snapshot/VolumeTree.java
  • storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotTreeBase.java
  • storage/src/main/java/org/zstack/storage/snapshot/group/VolumeSnapshotGroupBase.java


---

## 15.6 资料 children 顺序的依赖
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

修正标题用词。

"资料 children 顺序的依赖"应为"关于 children 顺序的依赖"。当前用词"资料"(materials/data)在此上下文中不通顺。

🔧 建议修改
-## 15.6 资料 children 顺序的依赖
+## 15.6 关于 children 顺序的依赖
📝 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
## 15.6 资料 children 顺序的依赖
## 15.6 关于 children 顺序的依赖
🤖 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/snapshot-single-delete/scenarios/01-multi-children-stepDelete.md` at
line 114, Update the heading "## 15.6 资料 children 顺序的依赖" to use the suggested
phrasing by replacing "资料 children 顺序的依赖" with "关于 children 顺序的依赖" so the title
reads "## 15.6 关于 children 顺序的依赖".

Comment on lines +16 to +23
class ChainSnapshotSet:
operation: str
timestamp: float
snapshots: dict[str, ChainSnapshot]
def dump_to_file(self, path): ...
@staticmethod
def load_from_file(path) -> "ChainSnapshotSet": ...

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

补齐 ChainSnapshotSetmetadata 契约定义。

当前结构定义未包含 metadata,但 Line 123 已使用 pre_snap.metadata['tmp_path']。建议在 6.1 的数据结构里显式声明该字段(类型与序列化约束),避免接入方实现不一致。

Also applies to: 123-123

🤖 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/specs/2026-05-13-snapshot-single-delete-hardening/02-data-plane-validation.md`
around lines 16 - 23, 在 ChainSnapshotSet 的数据结构中补齐 metadata
字段契约:在类定义(ChainSnapshotSet)中新增 metadata 字段并明确类型与序列化约束(例如 metadata: dict[str,
str] 或 metadata: dict[str, JSONSerializable],确保能存放 pre_snap.metadata['tmp_path']
这样的字符串),并在 dump_to_file / load_from_file 实现中保证对 metadata 的正确序列化/反序列化,确保所有使用
pre_snap.metadata[...] 的代码与此约定一致;同时更新相关文档段落(6.1 数据结构)描述该字段的类型与约束。

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