<fix>[volume]: optimization of Local and NFS Storage Provisioning #2990
Conversation
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
GlobalConfigImpact Resolves: ZSV-7904 Change-Id: I776162727a786f6764706a736c6768756a63766e
概览该变更移除了QCOW2卷预配置策略扩展点接口，并在三个存储工厂实现中将其替换为基于ResourceConfig的配置驱动方案。同时新增三个预分配类型常量，并扩展QCOW2_ALLOCATION配置以同时绑定VolumeVO和PrimaryStorageVO资源类型。 变更内容
代码审查工作量估算🎯 3 (中等) | ⏱️ ~20-25 分钟 需特别关注的区域：
诗歌
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
header/src/main/java/org/zstack/header/volume/CreateQcow2VolumeProvisioningStrategyExtensionPoint.java(0 hunks)header/src/main/java/org/zstack/header/volume/VolumeConstant.java(1 hunks)plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageFactory.java(4 hunks)plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStoragePrimaryStorageGlobalConfig.java(2 hunks)plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageFactory.java(3 hunks)plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageGlobalConfig.java(2 hunks)plugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/SMPPrimaryStorageFactory.java(3 hunks)plugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/SMPPrimaryStorageGlobalConfig.java(2 hunks)
💤 Files with no reviewable changes (1)
- header/src/main/java/org/zstack/header/volume/CreateQcow2VolumeProvisioningStrategyExtensionPoint.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java
⚙️ CodeRabbit configuration file
**/*.java: ## 1. API 设计要求
- API 命名：
- API 名称必须唯一，不能重复。
- API 消息类需要继承
APIMessage；其返回类必须继承APIReply或APIEvent，并在注释中用@RestResponse进行标注。- API 消息上必须添加注解
@RestRequest，并满足如下规范：
path:
- 针对资源使用复数形式。
- 当 path 中引用消息类变量时，使用
{variableName}格式。- HTTP 方法对应：
- 查询操作 →
HttpMethod.GET- 更新操作 →
HttpMethod.PUT- 创建操作 →
HttpMethod.POST- 删除操作 →
HttpMethod.DELETE- API 类需要实现
__example__方法以便生成 API 文档，并确保生成对应的 Groovy API Template 与 API Markdown 文件。
2. 命名与格式规范
类名：
- 使用 UpperCamelCase 风格。
- 特殊情况：
- VO/AO/EO 类型类除外。
- 抽象类采用
Abstract或Base前缀/后缀。- 异常类应以
Exception结尾。- 测试类需要以
Test或Case结尾。方法名、参数名、成员变量和局部变量：
- 使用 lowerCamelCase 风格。
常量命名：
- 全部大写，使用下划线分隔单词。
- 要求表达清楚，避免使用含糊或不准确的名称。
包名：
- 统一使用小写，使用点分隔符，每个部分应是一个具有自然语义的英文单词（参考 Spring 框架的结构）。
命名细节：
- 避免在父子类或同一代码块中出现相同名字的成员或局部变量，防止混淆。
- 命名缩写：
- 不允许使用不必要的缩写，如：
AbsSchedulerJob、condi、Fu等。应使用完整单词提升可读性。
3. 编写自解释代码
意图表达：
- 避免使用布尔型参数造成含义不明确。例如：
- 对于
stopAgent(boolean ignoreError)，建议拆分为不同函数（如stopAgentIgnoreError()），或使用枚举表达操作类型。- 命名应尽量用完整的单词组合表达意图，并在名称中体现数据类型或用途（例如在常量与变量名称中，将类型词放在末尾）。
注释：
- 代码应尽量做到自解释，对少于两行的说明可以直接写在代码中。
- 对于较长的注释，需要仔细校对并随代码更新，确保内容正确。
- 接口方法不应有多余的修饰符（例如
public），且必须配有有效的 Javadoc 注释。
4. 流程控制和结构优化
if...else 的使用：
- 应尽量减少 if...else 结构的使用，建议：
- 限制嵌套层级最多为两层，且内层不应再出现
else分支。- 尽早返回（Early Return），将条件判断中的处理逻辑提前结束或抽成独立方法。
- 使用 Java Stream 或 Lambda 表达式代替冗长的循环与条件判断。
条件判断：
- if 条件表达不宜过长或过于复杂，必要时可以将条件抽成 boolean 变量描述。
代码块长度：
- 单个 if 代码块不宜超过一屏显示，以提高可读性和后续维护性。
5. 异常处理与日志
- 捕获异常的原则：
- 对于可以通过预检查避免的 RuntimeException（如
NullPointerException、IndexOutOfBoundsException等），不建议使用 try-catch 来进行处理。- 捕获异常应仅用于处理真正的意外情况，不应将异常逻辑当作正常流程控制。
- 在必要时，应继续抛出异常，使上层业务处理者可以转换为用户友好的错误提示。
- 使用 try-with-resources 语法管理资源，确保在 finally 块中正确关闭资源，并避免在 finally 中返回值。
...
Files:
plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageGlobalConfig.javaplugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/SMPPrimaryStorageGlobalConfig.javaheader/src/main/java/org/zstack/header/volume/VolumeConstant.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageFactory.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStoragePrimaryStorageGlobalConfig.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageFactory.javaplugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/SMPPrimaryStorageFactory.java
🧠 Learnings (5)
📓 Common learnings
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 2496
File: storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotTreeBase.java:1218-1224
Timestamp: 2025-08-24T06:33:10.771Z
Learning: ZStack团队在容量管理相关问题上响应迅速，当发现Pull快照流程中申请容量与释放容量不匹配的问题时，开发人员会及时进行修复以确保主存储容量核算的准确性。
📚 Learning: 2025-08-14T06:56:19.585Z
Learnt from: zstack-robot-2
Repo: MatheMatrix/zstack PR: 2435
File: storage/src/main/java/org/zstack/storage/snapshot/group/VolumeSnapshotGroupBase.java:47-47
Timestamp: 2025-08-14T06:56:19.585Z
Learning: 在VolumeSnapshotGroupBase.java中，VmInstanceResourceMetadataManager的注入和SKIP_RESOURCE_ROLLBACK标记虽然在当前版本中未被使用，但这些导入在大型重构PR中是有意为之的，用于保持代码一致性或为后续功能做准备。
Applied to files:
plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageFactory.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageFactory.javaplugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/SMPPrimaryStorageFactory.java
📚 Learning: 2025-07-22T02:30:46.123Z
Learnt from: MatheMatrix
Repo: MatheMatrix/zstack PR: 2311
File: plugin/sdnController/src/main/java/org/zstack/sdnController/hardwareVxlan/KVMRealizeHardwareVxlanNetworkBackend.java:52-53
Timestamp: 2025-07-22T02:30:46.123Z
Learning: In ZStack, backend realization code (such as KVMRealizeHardwareVxlanNetworkBackend) can safely assume the existence of the VO for the resource being operated on (e.g., HardwareL2VxlanNetworkVO for a given L2NetworkInventory), due to system lifecycle management and strong foreign key constraints in the database schema. Explicit null checks after findByUuid are not required in these contexts.
Applied to files:
plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageFactory.java
📚 Learning: 2025-08-24T07:56:17.047Z
Learnt from: MatheMatrix
Repo: MatheMatrix/zstack PR: 2496
File: storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotCascadeExtension.java:71-73
Timestamp: 2025-08-24T07:56:17.047Z
Learning: In VolumeSnapshotCascadeExtension, the createActionForChildResource method always wraps the context in VolumeSnapshotDeletionStructs when creating child actions, so no backward compatibility with List<VolumeSnapshotInventory> is needed in handleDeletionCleanup.
Applied to files:
plugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/SMPPrimaryStorageFactory.java
📚 Learning: 2024-06-10T19:31:27.994Z
Learnt from: AlanJager
Repo: MatheMatrix/zstack PR: 175
File: storage/src/main/java/org/zstack/storage/volume/VolumeBase.java:31-38
Timestamp: 2024-06-10T19:31:27.994Z
Learning: The user has clarified that the `MemorySnapshotGroupExtensionPoint` has been removed and its implementation has been moved to `VolumeSnapshotCreationExtensionPoint`.
Applied to files:
plugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/SMPPrimaryStorageFactory.java
🔇 Additional comments (3)
plugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/SMPPrimaryStorageGlobalConfig.java (1)
7-7: QCOW2 配置绑定扩展到 VolumeVO 与其他存储保持一致
QCOW2_ALLOCATION现在绑定到{VolumeVO.class, PrimaryStorageVO.class}，与 Local/NFS 的对应 GlobalConfig 一致，便于后续按卷级别覆写预分配策略，当前实现本身没有逻辑问题。Also applies to: 21-22
plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageGlobalConfig.java (1)
7-7: NFS QCOW2 配置绑定扩展到卷与主存储, 行为合理绑定对象扩展为
{VolumeVO.class, PrimaryStorageVO.class}，与其他存储插件保持一致，有利于通过 ResourceConfig 在卷和主存储两个维度控制预分配策略，当前改动本身无明显风险。Also applies to: 23-24
plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStoragePrimaryStorageGlobalConfig.java (1)
7-7: LocalStorage QCOW2 全局配置绑定扩展正确
QCOW2_ALLOCATION同样绑定到卷和主存储两个 VO，上层通过 ResourceConfigFacade 读取时可以精细到卷级/主存储级，和 NFS/SMP 的实现对齐，没有额外逻辑风险。Also applies to: 21-22
Comment on lines
+15
to
+17
| String NONE_PREALLOCTION = "none"; | ||
| String FULL_PREALLOCTION = "full"; | ||
| String FALLOC_PREALLOCTION = "falloc"; |
There was a problem hiding this comment.
预分配常量命名存在拼写错误, 建议在本 PR 即时修正
这里三个常量名中的 PREALLOCTION 少了一个字母 A（应为 PREALLOCATION），不影响运行时行为，但会降低可读性且不利于后续搜索/维护，建议在引入阶段就修正，同时统一修改所有引用处：
- String NONE_PREALLOCTION = "none";
- String FULL_PREALLOCTION = "full";
- String FALLOC_PREALLOCTION = "falloc";
+ String NONE_PREALLOCATION = "none";
+ String FULL_PREALLOCATION = "full";
+ String FALLOC_PREALLOCATION = "falloc";另外, GlobalConfig 的 validValues 还包含 "metadata"，如希望完全避免 magic string，也可以考虑后续补充一个 METADATA_PREALLOCATION 常量（可选优化）。
📝 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
| String NONE_PREALLOCTION = "none"; | |
| String FULL_PREALLOCTION = "full"; | |
| String FALLOC_PREALLOCTION = "falloc"; | |
| String NONE_PREALLOCATION = "none"; | |
| String FULL_PREALLOCATION = "full"; | |
| String FALLOC_PREALLOCATION = "falloc"; |
🤖 Prompt for AI Agents
In header/src/main/java/org/zstack/header/volume/VolumeConstant.java around
lines 15-17, the three constants are misspelled as PREALLOCTION; rename them to
NONE_PREALLOCATION, FULL_PREALLOCATION, and FALLOC_PREALLOCATION and update
every usage across the codebase (imports/usages, tests, docs) to the new names
to avoid breakage. Ensure you run a project-wide search-and-replace and compile
to catch remaining references, update any JavaDocs/comments mentioning the old
names, and add a new METADATA_PREALLOCATION constant and replace the literal
"metadata" in GlobalConfig.validValues with this constant if you want to
eliminate the magic string.
Comment on lines
+50
to
52
| import org.zstack.resourceconfig.ResourceConfig; | ||
| import org.zstack.resourceconfig.ResourceConfigFacade; | ||
| import org.zstack.storage.primary.PrimaryStorageCapacityChecker; |
There was a problem hiding this comment.
LocalStorage qcow2 预分配改为 ResourceConfig 驱动, fallback 建议改用运行时 GlobalConfig 值
整体逻辑没问题：
- 只在本地主存储且非内存卷时处理；
- 先检查卷级 ResourceConfig，有值则尊重现有配置直接返回；
- 仅对从 RootVolumeTemplate 创建且有 backing file 的场景, 通过主存储级配置初始化卷级配置。
需要注意的是 fallback 分支当前使用的是编译期默认值：
String psPreallocation = rcf.getResourceConfigValueByResourceType(
LocalStoragePrimaryStorageGlobalConfig.QCOW2_ALLOCATION,
msg.getVolume().getPrimaryStorageUuid(), PrimaryStorageVO.class.getSimpleName(), String.class);
if (psPreallocation == null) {
psPreallocation = LocalStoragePrimaryStorageGlobalConfig.QCOW2_ALLOCATION.getDefaultValue();
}这会忽略管理员在运行时通过 GlobalConfig API 修改 qcow2.allocation 的全局值（global-level），在 PS 级 ResourceConfig 未单独设置时直接退回到编译期默认，属于 GlobalConfig 行为变更，且本 PR 已标记 GlobalConfigImpact。
建议这里改为使用运行时全局值作为最终 fallback，例如：
- if (psPreallocation == null) {
- psPreallocation = LocalStoragePrimaryStorageGlobalConfig.QCOW2_ALLOCATION.getDefaultValue();
- }
+ if (psPreallocation == null) {
+ // 使用运行时全局配置值作为最终 fallback, 保持与现有 GlobalConfig 语义一致
+ psPreallocation = LocalStoragePrimaryStorageGlobalConfig.QCOW2_ALLOCATION.value(String.class);
+ }同时建议在 NFS/SMP 的对应逻辑中做同样调整，以保证三种主存储的 qcow2 预分配在 “卷级 > PS 级 > 全局级” 三层 fallback 语义上一致。
Also applies to: 107-108, 270-284
Comment on lines
+43
to
45
| import org.zstack.resourceconfig.ResourceConfig; | ||
| import org.zstack.resourceconfig.ResourceConfigFacade; | ||
| import org.zstack.storage.primary.ChangePrimaryStorageStatusMsg; |
There was a problem hiding this comment.
NFS qcow2 预分配改为 ResourceConfig 流程合理, 建议修正 fallback 以尊重运行时全局配置
在 NFS factory 中引入 ResourceConfigFacade 并在 afterInstantiateVolume 中按“卷级优先, 再退回 PS 级”的思路设置 qcow2 预分配, 与 Local/SMP 的实现保持一致, 这是不错的统一。
需要注意的是, 这里的 PS 级 fallback 同样使用了默认值：
String psPreallocation = rcf.getResourceConfigValueByResourceType(
NfsPrimaryStorageGlobalConfig.QCOW2_ALLOCATION,
msg.getVolume().getPrimaryStorageUuid(), PrimaryStorageVO.class.getSimpleName(), String.class);
if (psPreallocation == null) {
psPreallocation = NfsPrimaryStorageGlobalConfig.QCOW2_ALLOCATION.getDefaultValue();
}这会忽略运行时通过 GlobalConfig 修改 qcow2.allocation 的场景，使行为回退到编译期默认。建议与 Local/SMP 一致，使用运行时全局值作为最终兜底：
- if (psPreallocation == null) {
- psPreallocation = NfsPrimaryStorageGlobalConfig.QCOW2_ALLOCATION.getDefaultValue();
- }
+ if (psPreallocation == null) {
+ psPreallocation = NfsPrimaryStorageGlobalConfig.QCOW2_ALLOCATION.value(String.class);
+ }这样三类主存储在预分配配置上的优先级和 GlobalConfig 语义就完全一致了。
Also applies to: 85-86, 800-813
🤖 Prompt for AI Agents
In
plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageFactory.java
around lines 43-45 (and also apply to 85-86 and 800-813), the code falls back to
the compile-time default when PS-level ResourceConfig is missing which ignores
runtime changes to the global qcow2.allocation; change the fallback so it uses
the runtime GlobalConfig value instead of getDefaultValue(): if the
ResourceConfigFacade call returns null, set the variable from
NfsPrimaryStorageGlobalConfig.QCOW2_ALLOCATION.value() (or the equivalent
runtime getter), and apply the same fix to the other occurrences so volume-level
-> PS-level -> runtime-global fallback order is preserved.
Comment on lines
+31
to
+32
| import org.zstack.resourceconfig.ResourceConfig; | ||
| import org.zstack.resourceconfig.ResourceConfigFacade; |
There was a problem hiding this comment.
SMP qcow2 预分配逻辑与 Local 一致, 但 fallback 同样应尊重运行时 GlobalConfig
新增的 ResourceConfig 驱动逻辑整体合理：
- 只处理 SMP 类型的主存储, 并跳过内存卷；
- 先查卷级 ResourceConfig, 有值则不覆盖；
- 仅 RootVolumeTemplate 且有 backing file 时, 用 PS 级/默认值初始化卷级配置。
和 LocalStorageFactory 一样, 这里的 fallback 目前使用的是编译期默认值：
String psPreallocation = rcf.getResourceConfigValueByResourceType(
SMPPrimaryStorageGlobalConfig.QCOW2_ALLOCATION,
msg.getVolume().getPrimaryStorageUuid(), PrimaryStorageVO.class.getSimpleName(), String.class);
if (psPreallocation == null) {
psPreallocation = SMPPrimaryStorageGlobalConfig.QCOW2_ALLOCATION.getDefaultValue();
}这会忽略运行时通过 GlobalConfig 调整过的 qcow2.allocation 全局配置。建议改为使用运行时全局值, 与 Local/NFS 三处统一：
- if (psPreallocation == null) {
- psPreallocation = SMPPrimaryStorageGlobalConfig.QCOW2_ALLOCATION.getDefaultValue();
- }
+ if (psPreallocation == null) {
+ psPreallocation = SMPPrimaryStorageGlobalConfig.QCOW2_ALLOCATION.value(String.class);
+ }这样可以保证预分配策略的优先级为：卷级 ResourceConfig > PS 级 ResourceConfig > 运行时全局 GlobalConfig。
Also applies to: 67-68, 429-442
🤖 Prompt for AI Agents
In
plugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/SMPPrimaryStorageFactory.java
around lines 31-32 (and also at lines 67-68 and 429-442), the code falls back to
the compile-time default for qcow2 allocation when PS-level ResourceConfig is
null; change the fallback to read the runtime GlobalConfig value instead of
using getDefaultValue(): after attempting to get the PS-level ResourceConfig, if
it is null call the GlobalConfig provider to fetch
SMPPrimaryStorageGlobalConfig.QCOW2_ALLOCATION's current value (the runtime
value) and use that as the fallback so behavior matches Local/NFS and respects
runtime configuration changes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
GlobalConfigImpact
Resolves: ZSV-7904
Change-Id: I776162727a786f6764706a736c6768756a63766e
sync from gitlab !8799