Skip to content

<fix>[volume]: optimization of Local and NFS Storage Provisioning #2990

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MatheMatrix wants to merge 1 commit into
base: zsv_4.10.28
Choose a base branch
from
Open

<fix>[volume]: optimization of Local and NFS Storage Provisioning #2990

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions ...in/java/org/zstack/header/volume/CreateQcow2VolumeProvisioningStrategyExtensionPoint.java
View file
Open in desktop

This file was deleted.

4 changes: 4 additions & 0 deletions header/src/main/java/org/zstack/header/volume/VolumeConstant.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ public interface VolumeConstant {
int DEFAULT_MAX_DATA_VOLUME_NUMBER = 24;
String BLOCK_VOLUME_TYPE = "blockVolume";

String NONE_PREALLOCTION = "none";
String FULL_PREALLOCTION = "full";
String FALLOC_PREALLOCTION = "falloc";
Comment on lines +15 to +17
Copy link

@coderabbitai coderabbitai bot Dec 2, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

预分配常量命名存在拼写错误, 建议在本 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.


enum Capability {
MigrationInCurrentPrimaryStorage,
MigrationToOtherPrimaryStorage
Expand Down
25 changes: 20 additions & 5 deletions plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@
import org.zstack.header.vm.VmInstanceConstant.VmOperation;
import org.zstack.header.volume.*;
import org.zstack.kvm.KVMConstant;
import org.zstack.resourceconfig.ResourceConfig;
import org.zstack.resourceconfig.ResourceConfigFacade;
import org.zstack.storage.primary.PrimaryStorageCapacityChecker;
Comment on lines +50 to 52
Copy link

@coderabbitai coderabbitai bot Dec 2, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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

import org.zstack.storage.snapshot.PostMarkRootVolumeAsSnapshotExtension;
import org.zstack.storage.snapshot.reference.VolumeSnapshotReferenceUtils;
Expand All @@ -60,6 +62,7 @@
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;

import static org.zstack.compute.vm.VmCpuVendor.AuthenticAMD;
import static org.zstack.core.Platform.argerr;
import static org.zstack.core.Platform.err;
import static org.zstack.core.Platform.operr;
Expand Down Expand Up @@ -101,6 +104,8 @@ public boolean isSupportVmLiveMigration() {
private CloudBus bus;
@Autowired
private ErrorFacade errf;
@Autowired
private ResourceConfigFacade rcf;

private Map<String, LocalStorageBackupStorageMediator> backupStorageMediatorMap = new HashMap<String, LocalStorageBackupStorageMediator>();

Expand Down Expand Up @@ -261,11 +266,21 @@ public void afterInstantiateVolume(InstantiateVolumeOnPrimaryStorageMsg msg) {
hasBackingFile = true;
}
}

VolumeInventory volume = msg.getVolume();
volume.setPrimaryStorageUuid(msg.getPrimaryStorageUuid());
for (CreateQcow2VolumeProvisioningStrategyExtensionPoint exp : pluginRgty.getExtensionList(CreateQcow2VolumeProvisioningStrategyExtensionPoint.class)) {
exp.saveQcow2VolumeProvisioningStrategy(volume, hasBackingFile);

String preallocation = rcf.getResourceConfigValueByResourceType(LocalStoragePrimaryStorageGlobalConfig.QCOW2_ALLOCATION,
msg.getVolume().getUuid(), VolumeVO.class.getSimpleName(), String.class);
if (preallocation != null) {
return;
}

if (hasBackingFile) {
ResourceConfig rc = rcf.getResourceConfig(LocalStoragePrimaryStorageGlobalConfig.QCOW2_ALLOCATION.getIdentity());
String psPreallocation = rcf.getResourceConfigValueByResourceType(LocalStoragePrimaryStorageGlobalConfig.QCOW2_ALLOCATION,
msg.getVolume().getPrimaryStorageUuid(), PrimaryStorageVO.class.getSimpleName(), String.class);
if (psPreallocation == null) {
psPreallocation = LocalStoragePrimaryStorageGlobalConfig.QCOW2_ALLOCATION.getDefaultValue();
}
rc.updateValue(msg.getVolume().getUuid(), psPreallocation);
}
}

Expand Down
3 changes: 2 additions & 1 deletion ...rc/main/java/org/zstack/storage/primary/local/LocalStoragePrimaryStorageGlobalConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import org.zstack.core.config.GlobalConfigDefinition;
import org.zstack.core.config.GlobalConfigValidation;
import org.zstack.header.storage.primary.PrimaryStorageVO;
import org.zstack.header.volume.VolumeVO;
import org.zstack.resourceconfig.BindResourceConfig;

/**
Expand All @@ -17,6 +18,6 @@ public class LocalStoragePrimaryStorageGlobalConfig {
public static GlobalConfig ALLOW_LIVE_MIGRATION = new GlobalConfig(CATEGORY, "liveMigrationWithStorage.allow");

@GlobalConfigValidation(validValues = {"none", "metadata", "falloc", "full"})
@BindResourceConfig({PrimaryStorageVO.class})
@BindResourceConfig({VolumeVO.class, PrimaryStorageVO.class})
public static GlobalConfig QCOW2_ALLOCATION = new GlobalConfig(CATEGORY, "qcow2.allocation");
}
22 changes: 18 additions & 4 deletions ...PrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@
import org.zstack.header.vo.ResourceVO;
import org.zstack.header.volume.*;
import org.zstack.kvm.KVMConstant;
import org.zstack.resourceconfig.ResourceConfig;
import org.zstack.resourceconfig.ResourceConfigFacade;
import org.zstack.storage.primary.ChangePrimaryStorageStatusMsg;
Comment on lines +43 to 45
Copy link

@coderabbitai coderabbitai bot Dec 2, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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.

import org.zstack.storage.primary.PrimaryStorageCapacityUpdater;
import org.zstack.storage.primary.PrimaryStorageSystemTags;
Expand Down Expand Up @@ -80,6 +82,8 @@ public class NfsPrimaryStorageFactory implements NfsPrimaryStorageManager, Prima
private RESTFacade restf;
@Autowired
protected EventFacade evtf;
@Autowired
private ResourceConfigFacade rcf;

private Map<String, NfsPrimaryStorageBackend> backends = new HashMap<String, NfsPrimaryStorageBackend>();
private Map<String, Map<String, NfsPrimaryToBackupStorageMediator>> mediators =
Expand Down Expand Up @@ -793,10 +797,20 @@ public void afterInstantiateVolume(InstantiateVolumeOnPrimaryStorageMsg msg) {
}
}

VolumeInventory volume = msg.getVolume();
volume.setPrimaryStorageUuid(msg.getPrimaryStorageUuid());
for (CreateQcow2VolumeProvisioningStrategyExtensionPoint exp : pluginRgty.getExtensionList(CreateQcow2VolumeProvisioningStrategyExtensionPoint.class)) {
exp.saveQcow2VolumeProvisioningStrategy(volume, hasBackingFile);
String preallocation = rcf.getResourceConfigValueByResourceType(NfsPrimaryStorageGlobalConfig.QCOW2_ALLOCATION,
msg.getVolume().getUuid(), VolumeVO.class.getSimpleName(), String.class);
if (preallocation != null) {
return;
}

if (hasBackingFile) {
ResourceConfig rc = rcf.getResourceConfig(NfsPrimaryStorageGlobalConfig.QCOW2_ALLOCATION.getIdentity());
String psPreallocation = rcf.getResourceConfigValueByResourceType(NfsPrimaryStorageGlobalConfig.QCOW2_ALLOCATION,
msg.getVolume().getPrimaryStorageUuid(), PrimaryStorageVO.class.getSimpleName(), String.class);
if (psPreallocation == null) {
psPreallocation = NfsPrimaryStorageGlobalConfig.QCOW2_ALLOCATION.getDefaultValue();
}
rc.updateValue(msg.getVolume().getUuid(), psPreallocation);
}
}
}
3 changes: 2 additions & 1 deletion ...ryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageGlobalConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import org.zstack.core.config.GlobalConfigDefinition;
import org.zstack.core.config.GlobalConfigValidation;
import org.zstack.header.storage.primary.PrimaryStorageVO;
import org.zstack.header.volume.VolumeVO;
import org.zstack.resourceconfig.BindResourceConfig;

/**
Expand All @@ -19,6 +20,6 @@ public class NfsPrimaryStorageGlobalConfig {
public static GlobalConfig GC_INTERVAL = new GlobalConfig(CATEGORY, "deletion.gcInterval");

@GlobalConfigValidation(validValues = {"none", "metadata", "falloc", "full"})
@BindResourceConfig({PrimaryStorageVO.class})
@BindResourceConfig({VolumeVO.class, PrimaryStorageVO.class})
public static GlobalConfig QCOW2_ALLOCATION = new GlobalConfig(CATEGORY, "qcow2.allocation");
}
22 changes: 18 additions & 4 deletions ...PrimaryStorage/src/main/java/org/zstack/storage/primary/smp/SMPPrimaryStorageFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
import org.zstack.header.storage.snapshot.CreateTemplateFromVolumeSnapshotExtensionPoint;
import org.zstack.header.storage.snapshot.VolumeSnapshotInventory;
import org.zstack.header.volume.*;
import org.zstack.resourceconfig.ResourceConfig;
import org.zstack.resourceconfig.ResourceConfigFacade;
Comment on lines +31 to +32
Copy link

@coderabbitai coderabbitai bot Dec 2, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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.

import org.zstack.storage.snapshot.PostMarkRootVolumeAsSnapshotExtension;
import org.zstack.utils.Utils;
import org.zstack.utils.logging.CLogger;
Expand Down Expand Up @@ -62,6 +64,8 @@ public class SMPPrimaryStorageFactory implements PrimaryStorageFactory, CreateTe
private ErrorFacade errf;
@Autowired
private PluginRegistry pluginRgty;
@Autowired
private ResourceConfigFacade rcf;

@Override
public PrimaryStorageType getPrimaryStorageType() {
Expand Down Expand Up @@ -422,10 +426,20 @@ public void afterInstantiateVolume(InstantiateVolumeOnPrimaryStorageMsg msg) {
}
}

VolumeInventory volume = msg.getVolume();
volume.setPrimaryStorageUuid(msg.getPrimaryStorageUuid());
for (CreateQcow2VolumeProvisioningStrategyExtensionPoint exp : pluginRgty.getExtensionList(CreateQcow2VolumeProvisioningStrategyExtensionPoint.class)) {
exp.saveQcow2VolumeProvisioningStrategy(volume, hasBackingFile);
String preallocation = rcf.getResourceConfigValueByResourceType(SMPPrimaryStorageGlobalConfig.QCOW2_ALLOCATION,
msg.getVolume().getUuid(), VolumeVO.class.getSimpleName(), String.class);
if (preallocation != null) {
return;
}

if (hasBackingFile) {
ResourceConfig rc = rcf.getResourceConfig(SMPPrimaryStorageGlobalConfig.QCOW2_ALLOCATION.getIdentity());
String psPreallocation = rcf.getResourceConfigValueByResourceType(SMPPrimaryStorageGlobalConfig.QCOW2_ALLOCATION,
msg.getVolume().getPrimaryStorageUuid(), PrimaryStorageVO.class.getSimpleName(), String.class);
if (psPreallocation == null) {
psPreallocation = SMPPrimaryStorageGlobalConfig.QCOW2_ALLOCATION.getDefaultValue();
}
rc.updateValue(msg.getVolume().getUuid(), psPreallocation);
}
}
}
3 changes: 2 additions & 1 deletion ...ryStorage/src/main/java/org/zstack/storage/primary/smp/SMPPrimaryStorageGlobalConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import org.zstack.core.config.GlobalConfigDefinition;
import org.zstack.core.config.GlobalConfigValidation;
import org.zstack.header.storage.primary.PrimaryStorageVO;
import org.zstack.header.volume.VolumeVO;
import org.zstack.resourceconfig.BindResourceConfig;

/**
Expand All @@ -17,6 +18,6 @@ public class SMPPrimaryStorageGlobalConfig {
public static GlobalConfig GC_INTERVAL = new GlobalConfig(CATEGORY, "deletion.gcInterval");

@GlobalConfigValidation(validValues = {"none", "metadata", "falloc", "full"})
@BindResourceConfig({PrimaryStorageVO.class})
@BindResourceConfig({VolumeVO.class, PrimaryStorageVO.class})
public static GlobalConfig QCOW2_ALLOCATION = new GlobalConfig(CATEGORY, "qcow2.allocation");
}