<fix>[vm]: improve error messages for backup storage selection failures#3396
Open
zstack-robot-1 wants to merge 1 commit into5.5.12from
Open
<fix>[vm]: improve error messages for backup storage selection failures#3396zstack-robot-1 wants to merge 1 commit into5.5.12from
zstack-robot-1 wants to merge 1 commit into5.5.12from
Conversation
selection failures during ISO attach and VM creation When VM image or ISO backup storage selection fails, the error messages previously only stated that no suitable backup storage was found, without providing diagnostic details about the actual state of backup storage resources. 1. Why is this change necessary? Users encountering backup storage selection failures had no visibility into the root cause. The error messages did not indicate which backup storages the image/ISO was on, their connection status, or which backup storages were attached to the zone, making troubleshooting difficult. 2. How does it address the problem? Added diagnostic helper methods to both VmDownloadIsoFlow and VmImageSelectBackupStorageFlow that query and format backup storage state info. Error messages now include: (a) which backup storages hold the image/ISO with their status, (b) which backup storages are attached to the zone with their status, and (c) actionable suggestions. Also added dedicated error codes with i18n support for all 10 locales. 3. Are there any side effects? None. Changes only affect error-path diagnostics. Normal backup storage selection logic is unchanged. # Summary of changes (by module): - compute: add diagnostic info to error messages in VmDownloadIsoFlow and VmImageSelectBackupStorageFlow - conf/i18n: add 4 new error codes (10084-10087) across all 10 locale JSON files Related: ZSTAC-65467 Change-Id: Idbfdaa2b97792a46a29c25b077d990a51f6e1df8
概述在两个虚拟机工作流类中添加了私有辅助方法，用于查询备份存储映射关系并生成格式化的状态信息字符串，同时使用这些信息增强了备份存储不可用场景下的错误提示信息，提供更详细的诊断上下文。 变更内容
预估审核工作量🎯 2 (Simple) | ⏱️ ~12 分钟 庆祝诗
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Collaborator
|
Comment from yaohua.wu: Review: MR !9244 — ZSTAC-65467Jira: 创建vm时镜像位置错误，创建vm失败报错需要优化 SummaryMR enriches error messages in 4 backup storage selection failure paths ( Review FindingsNo Critical or Warning issues found. Suggestion
Checklist
Verdict: APPROVED🤖 Robot Reviewer |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@compute/src/main/java/org/zstack/compute/vm/VmDownloadIsoFlow.java`:
- Around line 54-55: The new code in VmDownloadIsoFlow creates a
TypedQuery<Tuple> directly (dbf.getEntityManager().createQuery(sql, Tuple.class)
and q.setParameter("imageUuid", imageUuid)); replace these direct TypedQuery
usages with the project's unified DB query wrapper (e.g., SimpleQuery/SQLBatch
or the DbFacade helper methods used elsewhere) so queries follow the
repository/DB facade pattern: locate the sql and imageUuid usage in
VmDownloadIsoFlow and rewrite the two query sites (the Tuple-typed query at both
additions) to use the project's standard query helper (passing the SQL and
parameter map) instead of instantiating TypedQuery directly.
In
`@compute/src/main/java/org/zstack/compute/vm/VmImageSelectBackupStorageFlow.java`:
- Around line 167-168: Replace the direct JPA TypedQuery usage (the local
variable q created with dbf.getEntityManager().createQuery and
q.setParameter("imageUuid", ...)) inside VmImageSelectBackupStorageFlow with the
project's Q/QueryMore query API; build a query starting from
Q.New(BackupStorageVO.class, ImageBackupStorageRefVO.class) (or equivalent
QueryMore) to add the join/filter on ImageBackupStorageRefVO.imageUuid, select
the same columns (or tuples) previously requested, and call listTuple() (or the
QueryMore equivalent) to fetch results—remove the TypedQuery instantiation and
parameter binding and mirror the original selection/where logic using
Q/QueryMore methods.
ℹ️ Review info
Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (10)
conf/i18n/globalErrorCodeMapping/global-error-de-DE.jsonis excluded by!**/*.jsonconf/i18n/globalErrorCodeMapping/global-error-en_US.jsonis excluded by!**/*.jsonconf/i18n/globalErrorCodeMapping/global-error-fr-FR.jsonis excluded by!**/*.jsonconf/i18n/globalErrorCodeMapping/global-error-id-ID.jsonis excluded by!**/*.jsonconf/i18n/globalErrorCodeMapping/global-error-ja-JP.jsonis excluded by!**/*.jsonconf/i18n/globalErrorCodeMapping/global-error-ko-KR.jsonis excluded by!**/*.jsonconf/i18n/globalErrorCodeMapping/global-error-ru-RU.jsonis excluded by!**/*.jsonconf/i18n/globalErrorCodeMapping/global-error-th-TH.jsonis excluded by!**/*.jsonconf/i18n/globalErrorCodeMapping/global-error-zh_CN.jsonis excluded by!**/*.jsonconf/i18n/globalErrorCodeMapping/global-error-zh_TW.jsonis excluded by!**/*.json
📒 Files selected for processing (2)
compute/src/main/java/org/zstack/compute/vm/VmDownloadIsoFlow.javacompute/src/main/java/org/zstack/compute/vm/VmImageSelectBackupStorageFlow.java
Comment on lines
+54
to
+55
| TypedQuery<Tuple> q = dbf.getEntityManager().createQuery(sql, Tuple.class); | ||
| q.setParameter("imageUuid", imageUuid); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the specific file
fd -t f "VmDownloadIsoFlow.java" --exec wc -l {} \;Repository: MatheMatrix/zstack
Length of output: 134
🏁 Script executed:
# Read the file content, focusing on lines around 54-55 and 70-71
fd -t f "VmDownloadIsoFlow.java" --exec cat -n {} \; | head -100Repository: MatheMatrix/zstack
Length of output: 5244
🏁 Script executed:
# Search for TypedQuery usage patterns in the codebase to understand if it's discouraged
rg "TypedQuery" -t java --max-count 20 -B 2 -A 2Repository: MatheMatrix/zstack
Length of output: 50375
🏁 Script executed:
# Look for repository query encapsulation patterns
rg "repository|Repository" -t java compute/src/main/java/org/zstack/compute/vm/ --max-count 10 -A 3Repository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
# Search for repository or query helper patterns in compute module
rg "Query.*Tuple|repository.*query" compute/src/main/java/org/zstack/compute/ -t java -A 3 --max-count 15Repository: MatheMatrix/zstack
Length of output: 10417
🏁 Script executed:
# Look for alternative query patterns - checking how similar data fetching is done
rg "BackupStorage|getEntityManager.*createQuery" compute/src/main/java/org/zstack/compute/vm/ -t java -B 2 -A 2Repository: MatheMatrix/zstack
Length of output: 50374
🏁 Script executed:
# Check if there are repository classes or query builders for BackupStorageVO
fd -t f ".*Repository.*|.*Query.*" compute/src/main/java/org/zstack/compute/ --exec basename {} \;Repository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
# Search for how multiple field selection queries are handled elsewhere
rg "select.*from.*where.*and.*status|name.*status" storage/src/main/java/ -t java --max-count 5 -B 1 -A 3Repository: MatheMatrix/zstack
Length of output: 11670
避免在新增代码中直接使用 TypedQuery。
第 54-55 行与第 70-71 行新增了 TypedQuery<Tuple> 的直接使用。建议改用统一的数据库查询封装方式（如仓库模式），以保持代码风格一致。
Also applies to: 70-71
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@compute/src/main/java/org/zstack/compute/vm/VmDownloadIsoFlow.java` around
lines 54 - 55, The new code in VmDownloadIsoFlow creates a TypedQuery<Tuple>
directly (dbf.getEntityManager().createQuery(sql, Tuple.class) and
q.setParameter("imageUuid", imageUuid)); replace these direct TypedQuery usages
with the project's unified DB query wrapper (e.g., SimpleQuery/SQLBatch or the
DbFacade helper methods used elsewhere) so queries follow the repository/DB
facade pattern: locate the sql and imageUuid usage in VmDownloadIsoFlow and
rewrite the two query sites (the Tuple-typed query at both additions) to use the
project's standard query helper (passing the SQL and parameter map) instead of
instantiating TypedQuery directly.
Comment on lines
+167
to
+168
| TypedQuery<Tuple> q = dbf.getEntityManager().createQuery(sql, Tuple.class); | ||
| q.setParameter("imageUuid", imageUuid); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and read the relevant lines
if [ -f "compute/src/main/java/org/zstack/compute/vm/VmImageSelectBackupStorageFlow.java" ]; then
echo "=== File exists, checking lines 160-195 for context ==="
sed -n '160,195p' compute/src/main/java/org/zstack/compute/vm/VmImageSelectBackupStorageFlow.java
else
echo "File not found at specified path"
find . -name "VmImageSelectBackupStorageFlow.java" 2>/dev/null
fiRepository: MatheMatrix/zstack
Length of output: 1719
🏁 Script executed:
# Search for Q and QueryMore usage patterns in the project
echo "=== Searching for Q usage pattern ==="
rg "new Q\(" --type java -B 2 -A 2 | head -50
echo -e "\n=== Searching for QueryMore usage pattern ==="
rg "QueryMore" --type java -B 2 -A 2 | head -50Repository: MatheMatrix/zstack
Length of output: 3651
🏁 Script executed:
# Check for TypedQuery usage patterns to understand current practice
echo "=== Checking TypedQuery usage in codebase ==="
rg "TypedQuery" --type java | head -20
echo -e "\n=== Checking for similar query patterns in compute module ==="
rg "dbf\.getEntityManager\(\)" compute/ --type java | head -10Repository: MatheMatrix/zstack
Length of output: 4596
🏁 Script executed:
# Check QueryMore implementation for select/projection support
echo "=== Checking QueryMore for select method ==="
rg "public.*select" core/src/main/java/org/zstack/core/db/QueryMore.java -A 3
echo -e "\n=== Checking for Tuple usage with Q/QueryMore ==="
rg "QueryMore.*select" --type java -B 2 -A 3 | head -40
echo -e "\n=== Check Q usage examples with multiple columns ==="
rg "Q\.New.*select|QueryMore.*list\|find" --type java -B 2 -A 5 | head -60Repository: MatheMatrix/zstack
Length of output: 11052
🏁 Script executed:
# Check how to construct queries with Q/QueryMore for Tuple-like results
echo "=== Checking QueryMore source for select method signature ==="
sed -n '1,100p' core/src/main/java/org/zstack/core/db/QueryMore.java | grep -A 10 "select"Repository: MatheMatrix/zstack
Length of output: 554
🏁 Script executed:
# Check how QueryMore.list() returns results when multiple columns are selected
echo "=== Checking QueryMore.list() method signature and return type ==="
rg "public.*list\(\)" core/src/main/java/org/zstack/core/db/QueryMore.java -A 5
echo -e "\n=== Check examples of multi-attribute selects with list/find methods ==="
rg "\.select\([^)]*,\s*[^)]*\)" --type java -B 2 -A 5 | head -80Repository: MatheMatrix/zstack
Length of output: 9535
🏁 Script executed:
# Check if there are examples using QueryMore to select multiple attributes
echo "=== Search for QueryMore usage with 2+ select parameters ==="
grep -r "select(" compute/ --include="*.java" -A 5 | head -40Repository: MatheMatrix/zstack
Length of output: 4845
🏁 Script executed:
# Check the QueryMore list method to understand return type
sed -n '1,500p' core/src/main/java/org/zstack/core/db/QueryMore.java | grep -A 15 "public.*List.*list()"Repository: MatheMatrix/zstack
Length of output: 768
避免使用 TypedQuery，改用项目统一查询封装 Q/QueryMore
第 167-168 行和第 184-185 行的新增查询直接使用了 TypedQuery，违反了编码规范要求。应改为项目统一的 Q/QueryMore 查询框架，以保持一致性。
参考示例：
Q.New(BackupStorageVO.class, ImageBackupStorageRefVO.class)
.select(BackupStorageVO_.name, BackupStorageVO_.status)
// 添加关联条件
.listTuple();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@compute/src/main/java/org/zstack/compute/vm/VmImageSelectBackupStorageFlow.java`
around lines 167 - 168, Replace the direct JPA TypedQuery usage (the local
variable q created with dbf.getEntityManager().createQuery and
q.setParameter("imageUuid", ...)) inside VmImageSelectBackupStorageFlow with the
project's Q/QueryMore query API; build a query starting from
Q.New(BackupStorageVO.class, ImageBackupStorageRefVO.class) (or equivalent
QueryMore) to add the join/filter on ImageBackupStorageRefVO.imageUuid, select
the same columns (or tuples) previously requested, and call listTuple() (or the
QueryMore equivalent) to fetch results—remove the TypedQuery instantiation and
parameter binding and mirror the original selection/where logic using
Q/QueryMore methods.
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
3 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.
sync from gitlab !9244