<fix>[storage]: ZSTAC-86038 recover stale XInfini image cache#4538
Open
zstack-robot-2 wants to merge 1 commit into
Open
<fix>[storage]: ZSTAC-86038 recover stale XInfini image cache#4538zstack-robot-2 wants to merge 1 commit into
zstack-robot-2 wants to merge 1 commit into
Conversation
|
Warning
|
| Layer / File(s) | Summary |
|---|---|
存储资源统计契约 header/src/main/java/org/zstack/header/storage/addon/StorageResource.java, header/src/main/java/org/zstack/header/storage/addon/primary/PrimaryStorageControllerSvc.java, header/src/main/java/org/zstack/header/storage/snapshot/VolumeSnapshotStats.java, header/src/main/java/org/zstack/header/volume/VolumeStats.java |
新增统一的 StorageResource 抽象类型，卷和快照统计类改为继承该类型，统计接口返回类型改为 StorageResource。 |
控制器统计分流 plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.java, plugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniStorageController.java, plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java, plugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniApiHelper.java |
各存储控制器根据安装路径区分卷与快照统计；XInfini 对已删除资源返回资源不存在错误。 |
外部主存储缓存与容量处理 storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorage.java |
镜像缓存、backing chain、卷大小同步和模板估算改用 StorageResource，并处理缺失资源时的删除与重试。 |
集成测试与模拟器校验 test/src/test/groovy/org/zstack/test/integration/storage/primary/addon/xinfini/XinfiniPrimaryStorageCase.groovy, testlib/src/main/java/org/zstack/testlib/ExternalPrimaryStorageSpec.groovy |
新增缺失快照、reused 快照保护及卷快照统计测试；模拟器在克隆源快照不存在时返回 404，并继承源快照大小。 |
Estimated code review effort: 4 (Complex) | ~45 minutes
Sequence Diagram(s)
sequenceDiagram
participant ExternalPrimaryStorage
participant PrimaryStorageControllerSvc
participant StorageController
participant ImageCacheVO
ExternalPrimaryStorage->>ImageCacheVO: 选择候选镜像缓存
ExternalPrimaryStorage->>PrimaryStorageControllerSvc: stats(installPath)
PrimaryStorageControllerSvc->>StorageController: 查询卷或快照统计
StorageController-->>PrimaryStorageControllerSvc: StorageResource
PrimaryStorageControllerSvc-->>ExternalPrimaryStorage: 返回统计结果
alt 资源不存在
ExternalPrimaryStorage->>StorageController: 删除卷和快照
ExternalPrimaryStorage->>ImageCacheVO: 删除缓存记录并重试
end
Poem
我是小兔，抱来资源新框，
卷与快照共用一张网。
缺失快照不乱跑，
缓存重建有方向。
测试萝卜咔嚓响！
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | 标题准确概括了修复 XInfini 旧 ImageCache 失效后重建缓存的主要变更。 |
| Description check | ✅ Passed | 描述与代码变更一致，涵盖 stale cache 重建、404 处理、快照复用保护和回归测试。 |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
sync/jin.ma/fix/ZSTAC-86038
Comment @coderabbitai help to get the list of available commands.
MatheMatrix
force-pushed
the
sync/jin.ma/fix/ZSTAC-86038
branch
from
50d955c to
049cc6e
Compare
Validate cached XInfini snapshots before reuse. Rebuild missing regular caches and preserve user-owned snapshot reuse sources. Resolves: ZSTAC-86038 Change-Id: I88b8ac1fa8740194d735aa2c0dd139cadbd119ff
MatheMatrix
force-pushed
the
sync/jin.ma/fix/ZSTAC-86038
branch
from
049cc6e to
c18971f
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.java`:
- Around line 1149-1172: 在 ExponStorageController.stats 的快照分支中补充 parentUri：使用
snap.getPoolName() 和 snap.getVolumeId() 还原父卷路径并设置到 VolumeSnapshotStats，确保
getParentChain() 能继续遍历父卷；保持现有快照统计字段和普通卷分支不变。
🪄 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: 2578f0ea-9c91-4e95-9bba-97fcac899fb1
📒 Files selected for processing (11)
header/src/main/java/org/zstack/header/storage/addon/StorageResource.javaheader/src/main/java/org/zstack/header/storage/addon/primary/PrimaryStorageControllerSvc.javaheader/src/main/java/org/zstack/header/storage/snapshot/VolumeSnapshotStats.javaheader/src/main/java/org/zstack/header/volume/VolumeStats.javaplugin/expon/src/main/java/org/zstack/expon/ExponStorageController.javaplugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniApiHelper.javaplugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniStorageController.javaplugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.javastorage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorage.javatest/src/test/groovy/org/zstack/test/integration/storage/primary/addon/xinfini/XinfiniPrimaryStorageCase.groovytestlib/src/main/java/org/zstack/testlib/ExternalPrimaryStorageSpec.groovy
Comment on lines
1149
to
1172
| @Override | ||
| public void stats(String installPath, ReturnValueCompletion<VolumeStats> comp) { | ||
| public void stats(String installPath, ReturnValueCompletion<StorageResource> comp) { | ||
| if (installPath.contains("@")) { | ||
| VolumeSnapshotModule snap = apiHelper.getVolumeSnapshot(getSnapIdFromPath(installPath)); | ||
| VolumeSnapshotStats stats = new VolumeSnapshotStats(); | ||
| stats.setInstallPath(installPath); | ||
| stats.setSize(snap.getSnapSize()); | ||
| stats.setActualSize(snap.getDataSize()); | ||
| comp.success(stats); | ||
| return; | ||
| } | ||
|
|
||
| comp.success(getVolumeStats(installPath)); | ||
| } | ||
|
|
||
| private VolumeStats getVolumeStats(String installPath) { | ||
| VolumeModule vol = apiHelper.getVolume(getVolIdFromPath(installPath)); | ||
| VolumeStats stats = new VolumeStats(); | ||
| stats.setInstallPath(installPath); | ||
| stats.setSize(vol.getVolumeSize()); | ||
| stats.setActualSize(vol.getDataSize()); | ||
| stats.setFormat(VolumeConstant.VOLUME_FORMAT_RAW); | ||
| comp.success(stats); | ||
| return stats; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether Expon's VolumeModule exposes a backing snapshot id equivalent to XInfini's getBsSnapId()
ast-grep run --pattern 'class VolumeModule { $$$ }' --lang java plugin/expon
rg -n 'class VolumeModule' -A40 plugin/exponRepository: MatheMatrix/zstack
Length of output: 4089
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect Expon volume/snapshot models and the parent-chain logic referenced by the comment.
files=(
plugin/expon/src/main/java/org/zstack/expon/sdk/volume/VolumeModule.java
plugin/expon/src/main/java/org/zstack/expon/sdk/volume/snapshot/VolumeSnapshotModule.java
plugin/expon/src/main/java/org/zstack/storage/primary/ExternalPrimaryStorage.java
plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.java
plugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniStorageController.java
)
for f in "${files[@]}"; do
echo "===== $f ====="
if [ -f "$f" ]; then
wc -l "$f"
echo "--- outline ---"
ast-grep outline "$f" --view expanded | sed -n '1,220p'
else
echo "MISSING"
fi
echo
doneRepository: MatheMatrix/zstack
Length of output: 9477
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for any parentUri / backing snapshot related fields or accessors in Expon SDK and controller code.
rg -n "parentUri|getParentUri|getBsSnapId|backing|snapshot" plugin/expon/src/main/java plugin/xinfini/src/main/java plugin/storage/src/main/java 2>/dev/null || trueRepository: MatheMatrix/zstack
Length of output: 11651
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the exact implementation slices around the relevant methods.
sed -n '1138,1180p' plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.java
printf '\n===== XInfini =====\n'
sed -n '1120,1185p' plugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniStorageController.java
printf '\n===== ExternalPrimaryStorage =====\n'
rg -n "getParentChain|parentUri" plugin/storage/src/main/java/org/zstack/storage/primary/ExternalPrimaryStorage.javaRepository: MatheMatrix/zstack
Length of output: 2964
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the parent-chain implementation and any callers to see whether Expon stats are expected to provide parentUri.
rg -n "getParentChain\\(|parentUri == null|setParentUri\\(|getParentUri\\(" plugin -g '*.java'Repository: MatheMatrix/zstack
Length of output: 1594
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the parent-chain traversal logic and the storage stats interface.
files=(
plugin/storage/src/main/java/org/zstack/storage/primary/ExternalPrimaryStorage.java
plugin/storage/src/main/java/org/zstack/storage/primary/ExternalPrimaryStorageFactory.java
plugin/storage/src/main/java/org/zstack/storage/primary/ExternalPrimaryStorageBase.java
)
for f in "${files[@]}"; do
if [ -f "$f" ]; then
echo "===== $f ====="
wc -l "$f"
rg -n "getParentChain|parentUri|StorageResource|VolumeStats|VolumeSnapshotStats" "$f"
fi
doneRepository: MatheMatrix/zstack
Length of output: 156
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file that contains the parent-chain logic.
fd -a 'ExternalPrimaryStorage.java|ExternalPrimaryStorageBase.java|ExternalPrimaryStorageFactory.java' pluginRepository: MatheMatrix/zstack
Length of output: 156
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search the entire repository for the parent-chain implementation and related stats logic.
rg -n "getParentChain\\(|parentUri == null|setParentUri\\(|getParentUri\\(" .Repository: MatheMatrix/zstack
Length of output: 2600
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect Expon snapshot/volume SDK models and the API helper getter to see whether a backing parent can be derived.
for f in \
plugin/expon/src/main/java/org/zstack/expon/sdk/volume/VolumeSnapshotModule.java \
plugin/expon/src/main/java/org/zstack/expon/sdk/volume/VolumeModule.java \
plugin/expon/src/main/java/org/zstack/expon/ExponApiHelper.java
do
echo "===== $f ====="
wc -l "$f"
echo "--- outline ---"
ast-grep outline "$f" --view expanded | sed -n '1,220p'
echo
doneRepository: MatheMatrix/zstack
Length of output: 10359
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the relevant Expon slices around getVolumeStats(), stats(), and any snapshot/volume parent handling nearby.
sed -n '1138,1180p' plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.java
printf '\n===== VolumeSnapshotModule =====\n'
sed -n '1,220p' plugin/expon/src/main/java/org/zstack/expon/sdk/volume/VolumeSnapshotModule.java
printf '\n===== ExponApiHelper#getVolumeSnapshot =====\n'
rg -n "public VolumeSnapshotModule getVolumeSnapshot|public VolumeModule getVolume\\(" -A25 -B5 plugin/expon/src/main/java/org/zstack/expon/ExponApiHelper.javaRepository: MatheMatrix/zstack
Length of output: 7474
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for any Expon field or method that could identify a volume's backing snapshot / clone source.
rg -n "bsSnapId|backing|clone|source snapshot|sourceSnapshot|parentUri" plugin/expon/src/main/java/org/zstack/expon -g '*.java'Repository: MatheMatrix/zstack
Length of output: 2237
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the Expon volume/snapshot API payload shapes to see whether source/backing info is available anywhere else.
sed -n '1,220p' plugin/expon/src/main/java/org/zstack/expon/sdk/volume/GetVolumeResponse.java
printf '\n===== GetVolumeSnapshotResponse =====\n'
sed -n '1,220p' plugin/expon/src/main/java/org/zstack/expon/sdk/volume/GetVolumeSnapshotResponse.java
printf '\n===== GetVolumeSnapshotRequest =====\n'
sed -n '1,220p' plugin/expon/src/main/java/org/zstack/expon/sdk/volume/GetVolumeSnapshotRequest.javaRepository: MatheMatrix/zstack
Length of output: 3825
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect Expon path builders and whether snapshot stats could be linked to a volume path directly.
rg -n "buildExponVolumePath|buildExponSnapshotPath|getSnapIdFromPath|getVolIdFromPath|setParentUri\\(" plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.java
sed -n '1,260p' plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.java | sed -n '1,220p'Repository: MatheMatrix/zstack
Length of output: 11971
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the helper methods around install path construction and path parsing in ExponStorageController.
sed -n '1,220p' plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.javaRepository: MatheMatrix/zstack
Length of output: 9374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether Expon already has a volume-path helper and how clone/snapshot methods populate parentUri.
rg -n "buildExponVolumePath|buildExponSnapshotPath|setParentUri\\(" plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.java
printf '\n===== clone / snapshot slices =====\n'
sed -n '1096,1338p' plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.javaRepository: MatheMatrix/zstack
Length of output: 10106
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the parent-chain traversal to confirm what parentUri is used for.
sed -n '1688,1745p' storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorage.javaRepository: MatheMatrix/zstack
Length of output: 2508
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether Expon's snapshot response fields can be mapped to a parent storage path.
rg -n "getVolumeId\\(|buildExponPath\\(|buildExponSnapshotPath\\(" plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.java plugin/expon/src/main/java/org/zstack/expon/ExponNameHelper.java
sed -n '1,220p' plugin/expon/src/main/java/org/zstack/expon/ExponNameHelper.javaRepository: MatheMatrix/zstack
Length of output: 3853
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for any helper that can reconstruct a volume path from a volume ID, which would be needed to set parentUri for snapshots.
rg -n "buildExponPath\\(|buildExponSnapshotPath\\(" plugin/expon/src/main/java/org/zstack/expon -g '*.java'Repository: MatheMatrix/zstack
Length of output: 1567
补上快照 parentUri
stats() 的快照分支已经能从 snap.getPoolName() / snap.getVolumeId() 还原父卷路径，但现在没设置 parentUri，getParentChain() 会在这一层直接截断。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.java`
around lines 1149 - 1172, 在 ExponStorageController.stats 的快照分支中补充 parentUri：使用
snap.getPoolName() 和 snap.getVolumeId() 还原父卷路径并设置到 VolumeSnapshotStats，确保
getParentChain() 能继续遍历父卷；保持现有快照统计字段和普通卷分支不变。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
修复 XInfini 主存储的 ImageCacheVO 仍存在、但其后端 BsSnap 已丢失时，创建 vhost-root 云主机直接复用 stale cache 并报
BsSnap not found的问题。Changes
volumeSnapshotReuse://：复用快照缺失时明确失败，不删除用户源卷BsSnap not found行为，并增加 stale cache 自愈与复用快照安全回归场景Testing
mvn compile -pl header,storage,plugin/xinfini -am -DskipTests（24/24 modules）XinfiniPrimaryStorageCase：1 test, 0 failures, 0 errors, 0 skippedxinfini://2/17239@5640检查到 404 后重建为xinfini://2/17302@5648，VM 成功进入 RunningResolves: ZSTAC-86038
sync from gitlab !10500