Skip to content

<feature>[vm_local_volume_cache]: vm local volume cache imp #3484

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: feature-5.5.6-local-cache
Choose a base branch
from
+5,004 −58
Open

<feature>[vm_local_volume_cache]: vm local volume cache imp #3484

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
22 changes: 22 additions & 0 deletions compute/src/main/java/org/zstack/compute/host/HostBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,8 @@ protected void handleApiMessage(APIMessage msg) {
handle((APIPowerResetHostMsg) msg);
} else if (msg instanceof APIGetHostPowerStatusMsg) {
handle((APIGetHostPowerStatusMsg) msg);
} else if (msg instanceof APIGetBlockDevicesMsg) {
handle((APIGetBlockDevicesMsg) msg);
} else {
bus.dealWithUnknownMessage(msg);
}
Expand All @@ -214,6 +216,26 @@ public void run(MessageReply reply) {
});
}

private void handle(APIGetBlockDevicesMsg msg) {
APIGetBlockDevicesEvent event = new APIGetBlockDevicesEvent(msg.getId());
GetBlockDevicesOnHostMsg gmsg = new GetBlockDevicesOnHostMsg();
gmsg.setHostUuid(msg.getHostUuid());
bus.makeTargetServiceIdByResourceUuid(gmsg, HostConstant.SERVICE_ID, msg.getHostUuid());
bus.send(gmsg, new CloudBusCallBack(msg) {
@Override
public void run(MessageReply reply) {
if (!reply.isSuccess()) {
event.setSuccess(false);
event.setError(reply.getError());
} else {
GetBlockDevicesOnHostReply r = reply.castReply();
event.setBlockDevices(r.getBlockDevices());
}
bus.publish(event);
}
});
}

private void handle(APIPowerResetHostMsg msg) {
final APIPowerResetHostEvent event = new APIPowerResetHostEvent(msg.getId());
RebootHostMsg rebootHostMsg = new RebootHostMsg();
Expand Down
55 changes: 55 additions & 0 deletions compute/src/main/java/org/zstack/compute/vm/InstantiateVmFromNewCreatedStruct.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ public class InstantiateVmFromNewCreatedStruct {
private Map<String, List<String>> dataVolumeSystemTagsOnIndex;
private List<String> disableL3Networks;
private List<String> sshKeyPairUuids;
private Boolean enableRootVolumeCache;
private String cacheMode;
private String rootVolumeCachePoolUuid;
private String rootVolumeCacheMode;
private Map<Integer, APICreateVmInstanceMsg.VolumeCacheConfig> dataDiskCacheConfigOnIndex;
private final List<String> candidatePrimaryStorageUuidsForRootVolume = new ArrayList<>();
private final List<String> candidatePrimaryStorageUuidsForDataVolume = new ArrayList<>();

Expand Down Expand Up @@ -142,6 +147,11 @@ public static InstantiateVmFromNewCreatedStruct fromMessage(InstantiateNewCreate
struct.setDataVolumeSystemTagsOnIndex(msg.getDataVolumeSystemTagsOnIndex());
struct.setDisableL3Networks(msg.getDisableL3Networks());
struct.setDiskAOs(msg.getDiskAOs());
struct.setEnableRootVolumeCache(msg.getEnableRootVolumeCache());
struct.setCacheMode(msg.getCacheMode());
struct.setRootVolumeCachePoolUuid(msg.getRootVolumeCachePoolUuid());
struct.setRootVolumeCacheMode(msg.getRootVolumeCacheMode());
struct.setDataDiskCacheConfigOnIndex(msg.getDataDiskCacheConfigOnIndex());
return struct;
}

Expand All @@ -161,6 +171,11 @@ public static InstantiateVmFromNewCreatedStruct fromMessage(CreateVmInstanceMsg
struct.setDataVolumeSystemTagsOnIndex(msg.getDataVolumeSystemTagsOnIndex());
struct.setDisableL3Networks(msg.getDisableL3Networks());
struct.setDiskAOs(msg.getDiskAOs());
struct.setEnableRootVolumeCache(msg.getEnableRootVolumeCache());
struct.setCacheMode(msg.getCacheMode());
struct.setRootVolumeCachePoolUuid(msg.getRootVolumeCachePoolUuid());
struct.setRootVolumeCacheMode(msg.getRootVolumeCacheMode());
struct.setDataDiskCacheConfigOnIndex(msg.getDataDiskCacheConfigOnIndex());
return struct;
}

Expand Down Expand Up @@ -243,4 +258,44 @@ public List<String> getSshKeyPairUuids() {
public void setSshKeyPairUuids(List<String> sshKeyPairUuids) {
this.sshKeyPairUuids = sshKeyPairUuids;
}

public Boolean getEnableRootVolumeCache() {
return enableRootVolumeCache;
}

public void setEnableRootVolumeCache(Boolean enableRootVolumeCache) {
this.enableRootVolumeCache = enableRootVolumeCache;
}

public String getCacheMode() {
return cacheMode;
}

public void setCacheMode(String cacheMode) {
this.cacheMode = cacheMode;
}

public String getRootVolumeCachePoolUuid() {
return rootVolumeCachePoolUuid;
}

public void setRootVolumeCachePoolUuid(String rootVolumeCachePoolUuid) {
this.rootVolumeCachePoolUuid = rootVolumeCachePoolUuid;
}

public String getRootVolumeCacheMode() {
return rootVolumeCacheMode;
}

public void setRootVolumeCacheMode(String rootVolumeCacheMode) {
this.rootVolumeCacheMode = rootVolumeCacheMode;
}

public Map<Integer, APICreateVmInstanceMsg.VolumeCacheConfig> getDataDiskCacheConfigOnIndex() {
return dataDiskCacheConfigOnIndex;
}

public void setDataDiskCacheConfigOnIndex(Map<Integer, APICreateVmInstanceMsg.VolumeCacheConfig> dataDiskCacheConfigOnIndex) {
this.dataDiskCacheConfigOnIndex = dataDiskCacheConfigOnIndex;
}
}
54 changes: 54 additions & 0 deletions compute/src/main/java/org/zstack/compute/vm/VmAllocateHostFlow.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,63 @@ public String call(L3NetworkInventory arg) {
msg.getRequiredPrimaryStorageUuids().addAll(spec.getDiskAOs().stream()
.map(APICreateVmInstanceMsg.DiskAO::getPrimaryStorageUuid).filter(Objects::nonNull).collect(Collectors.toList()));
}

// Add cache pool requirements as system tags for host allocation filtering
addCacheSystemTags(spec, msg);

return msg;
}

private long getTotalCacheSize(VmInstanceSpec spec) {
long totalCacheSize = 0;
boolean enableRootVolumeCache = Boolean.TRUE.equals(spec.getEnableRootVolumeCache());
if (enableRootVolumeCache) {
ImageInventory image = spec.getImageSpec() == null ? null : spec.getImageSpec().getInventory();
boolean nonIsoImage = image != null && !ImageMediaType.ISO.toString().equals(image.getMediaType());
if (nonIsoImage && image.getSize() > 0) {
totalCacheSize += image.getSize();
} else if (spec.getRootDiskOffering() != null) {
totalCacheSize += spec.getRootDiskOffering().getDiskSize();
}
}
Map<Integer, APICreateVmInstanceMsg.VolumeCacheConfig> dataDiskCacheConfigs = spec.getDataDiskCacheConfigOnIndex();
if (dataDiskCacheConfigs != null && !dataDiskCacheConfigs.isEmpty()) {
for (Integer index : dataDiskCacheConfigs.keySet()) {
APICreateVmInstanceMsg.VolumeCacheConfig dataDiskCacheConfig = dataDiskCacheConfigs.get(index);
totalCacheSize += spec.getDataDiskOfferings().get(index).getDiskSize();
}

}
return totalCacheSize;
Comment on lines +159 to +166
Copy link

@coderabbitai coderabbitai bot Mar 13, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

潜在的 IndexOutOfBoundsException 风险

代码使用 dataDiskCacheConfigs.keySet() 中的 index 直接访问 spec.getDataDiskOfferings().get(index)，但没有验证 index 是否在 DataDiskOfferings 列表的有效范围内。如果用户配置的 cache 索引超出了实际数据盘数量，将抛出 IndexOutOfBoundsException。

此外，第 161 行声明的变量 dataDiskCacheConfig 未被使用，属于无用代码。

🛡️ 建议添加边界检查并移除无用变量 
         Map<Integer, APICreateVmInstanceMsg.VolumeCacheConfig> dataDiskCacheConfigs = spec.getDataDiskCacheConfigOnIndex();
         if (dataDiskCacheConfigs != null && !dataDiskCacheConfigs.isEmpty()) {
+            List<DiskOfferingInventory> dataDisks = spec.getDataDiskOfferings();
             for (Integer index : dataDiskCacheConfigs.keySet()) {
-                APICreateVmInstanceMsg.VolumeCacheConfig dataDiskCacheConfig = dataDiskCacheConfigs.get(index);
-                totalCacheSize += spec.getDataDiskOfferings().get(index).getDiskSize();
+                if (index >= 0 && index < dataDisks.size()) {
+                    totalCacheSize += dataDisks.get(index).getDiskSize();
+                }
             }
-            
         }
📝 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
if (dataDiskCacheConfigs != null && !dataDiskCacheConfigs.isEmpty()) {
for (Integer index : dataDiskCacheConfigs.keySet()) {
APICreateVmInstanceMsg.VolumeCacheConfig dataDiskCacheConfig = dataDiskCacheConfigs.get(index);
totalCacheSize += spec.getDataDiskOfferings().get(index).getDiskSize();
}
}
return totalCacheSize;
if (dataDiskCacheConfigs != null && !dataDiskCacheConfigs.isEmpty()) {
List<DiskOfferingInventory> dataDisks = spec.getDataDiskOfferings();
for (Integer index : dataDiskCacheConfigs.keySet()) {
if (index >= 0 && index < dataDisks.size()) {
totalCacheSize += dataDisks.get(index).getDiskSize();
}
}
}
return totalCacheSize;
🤖 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/VmAllocateHostFlow.java` around
lines 159 - 166, The loop over dataDiskCacheConfigs may use keys that are out of
range for spec.getDataDiskOfferings() and also declares an unused variable
dataDiskCacheConfig; update the loop in VmAllocateHostFlow so for each Integer
index from dataDiskCacheConfigs.keySet() you first check that index >= 0 and
index < spec.getDataDiskOfferings().size() before accessing
spec.getDataDiskOfferings().get(index), skip or log invalid indexes, and remove
the unused local APICreateVmInstanceMsg.VolumeCacheConfig dataDiskCacheConfig
variable to eliminate dead code while still accumulating totalCacheSize.

}
private List<String> getSpecifiedCachePoolUuids(VmInstanceSpec spec) {
List<String> poolUuids = new ArrayList<>();
if (spec.getRootVolumeCachePoolUuid() != null) {
poolUuids.add(spec.getRootVolumeCachePoolUuid());
}
Map<Integer, APICreateVmInstanceMsg.VolumeCacheConfig> dataDiskCacheConfigs = spec.getDataDiskCacheConfigOnIndex();
if (dataDiskCacheConfigs != null && !dataDiskCacheConfigs.isEmpty()) {
for (Integer index : dataDiskCacheConfigs.keySet()) {
APICreateVmInstanceMsg.VolumeCacheConfig dataDiskCacheConfig = dataDiskCacheConfigs.get(index);
if (dataDiskCacheConfig.getCachePoolUuid() != null) {
poolUuids.add(dataDiskCacheConfig.getCachePoolUuid());
}
}
}
return poolUuids;
}

private void addCacheSystemTags(VmInstanceSpec spec, AllocateHostMsg msg) {
long totalCacheSize = getTotalCacheSize(spec);
if (totalCacheSize > 0) {
msg.addSystemTag("volumeCache::requiredSize::" + totalCacheSize);
}
List<String> specifiedCachePoolUuids = getSpecifiedCachePoolUuids(spec);
for (String poolUuid : specifiedCachePoolUuids) {
msg.addSystemTag("volumeCache::poolUuid::" + poolUuid);
}
}

@Override
public void run(final FlowTrigger chain, Map data) {
taskProgress("allocate candidate hosts");
Expand Down
29 changes: 24 additions & 5 deletions compute/src/main/java/org/zstack/compute/vm/VmAllocateVolumeFlow.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,8 @@
import org.zstack.header.image.ImageConstant;
import org.zstack.header.image.ImageConstant.ImageMediaType;
import org.zstack.header.message.MessageReply;
import org.zstack.header.vm.VmInstanceConstant;
import org.zstack.header.vm.VmInstanceSpec;
import org.zstack.header.vm.*;
import org.zstack.header.vm.VmInstanceSpec.VolumeSpec;
import org.zstack.header.vm.VmInstanceVO;
import org.zstack.header.vm.VmInstanceVO_;
import org.zstack.header.volume.*;
import org.zstack.header.volume.VolumeDeletionPolicyManager.VolumeDeletionPolicy;
import org.zstack.identity.AccountManager;
Expand Down Expand Up @@ -76,6 +73,19 @@ protected List<CreateVolumeMsg> prepareMsg(Map<String, Object> ctx) {
}
});
}
if (!spec.getDataDiskCacheConfigOnIndex().isEmpty() && !CollectionUtils.isEmpty(spec.getDataDiskOfferings())) {
List<VolumeSpec> dataVolumeSpecs = spec.getVolumeSpecs().stream()
.filter(s -> s.getType().equals(VolumeType.Data.toString())).collect(Collectors.toList());

IntStream.range(0, dataVolumeSpecs.size()).forEach(index -> {
APICreateVmInstanceMsg.VolumeCacheConfig config = spec.getDataDiskCacheConfigOnIndex().get(index);
if (config != null) {
dataVolumeSpecs.get(index).setEnableVolumeCache(true);
dataVolumeSpecs.get(index).setCachePoolUuid(config.getCachePoolUuid());
dataVolumeSpecs.get(index).setCacheMode(config.getCacheMode());
}
});
}
Comment on lines +76 to +88
Copy link

@coderabbitai coderabbitai bot Mar 13, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

给 dataDiskCacheConfigOnIndex 加空值保护。

Line 76 直接对新 map 调 isEmpty()，和 Lines 65-75 对同类可选参数的空安全处理不一致。调用方不传这个字段时，这里会在 volume 分配前直接 NPE。

💡 建议修改 
-        if (!spec.getDataDiskCacheConfigOnIndex().isEmpty() && !CollectionUtils.isEmpty(spec.getDataDiskOfferings())) {
+        Map<Integer, APICreateVmInstanceMsg.VolumeCacheConfig> dataDiskCacheConfigs = spec.getDataDiskCacheConfigOnIndex();
+        if (dataDiskCacheConfigs != null && !dataDiskCacheConfigs.isEmpty()
+                && !CollectionUtils.isEmpty(spec.getDataDiskOfferings())) {
             List<VolumeSpec> dataVolumeSpecs = spec.getVolumeSpecs().stream()
                     .filter(s -> s.getType().equals(VolumeType.Data.toString())).collect(Collectors.toList());

             IntStream.range(0, dataVolumeSpecs.size()).forEach(index -> {
-                APICreateVmInstanceMsg.VolumeCacheConfig config = spec.getDataDiskCacheConfigOnIndex().get(index);
+                APICreateVmInstanceMsg.VolumeCacheConfig config = dataDiskCacheConfigs.get(index);
                 if (config != null) {
                     dataVolumeSpecs.get(index).setEnableVolumeCache(true);
                     dataVolumeSpecs.get(index).setCachePoolUuid(config.getCachePoolUuid());
🤖 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/VmAllocateVolumeFlow.java` around
lines 76 - 88, In VmAllocateVolumeFlow, the call to
spec.getDataDiskCacheConfigOnIndex().isEmpty() can NPE when
getDataDiskCacheConfigOnIndex() is null; update the conditional to null-safe
check (e.g., use CollectionUtils.isEmpty(spec.getDataDiskCacheConfigOnIndex())
or check spec.getDataDiskCacheConfigOnIndex() != null &&
!spec.getDataDiskCacheConfigOnIndex().isEmpty()) before iterating, keeping the
rest of the logic that updates dataVolumeSpecs (dataVolumeSpecs,
setEnableVolumeCache, setCachePoolUuid, setCacheMode) unchanged.


List<VolumeSpec> volumeSpecs = spec.getVolumeSpecs();
List<CreateVolumeMsg> msgs = new ArrayList<>(volumeSpecs.size());
Expand All @@ -93,6 +103,9 @@ protected List<CreateVolumeMsg> prepareMsg(Map<String, Object> ctx) {
DebugUtils.Assert(vspec.getType() != null, "VolumeType can not be null!");

if (VolumeType.Root.toString().equals(vspec.getType())) {
vspec.setEnableVolumeCache(spec.getEnableRootVolumeCache());
vspec.setCachePoolUuid(spec.getRootVolumeCachePoolUuid());
vspec.setCacheMode(spec.getRootVolumeCacheMode());
Comment on lines +106 to +108
Copy link

@coderabbitai coderabbitai bot Mar 13, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
fd 'VmInstanceSpec\.java$' | while read -r f; do
  echo "== $f =="
  rg -n -C3 'Boolean\s+enableRootVolumeCache|getEnableRootVolumeCache|setEnableVolumeCache\(' "$f"
done

Repository: MatheMatrix/zstack

Length of output: 1027

🏁 Script executed:

#!/bin/bash
# Find VmAllocateVolumeFlow.java and check lines around 106-108
fd 'VmAllocateVolumeFlow\.java$' | while read -r f; do
  echo "== $f =="
  sed -n '100,115p' "$f"
  echo ""
  echo "== Checking VolumeSpec setter methods =="
done

# Also find VolumeSpec to check all setter parameter types
echo "=== VolumeSpec class ==="
fd 'VolumeSpec\.java$' | while read -r f; do
  echo "== $f =="
  rg -n -A1 'setCachePoolUuid|setCacheMode|setEnableVolumeCache' "$f"
done

Repository: MatheMatrix/zstack

Length of output: 1130

🏁 Script executed:

#!/bin/bash
# Find VolumeSpec more broadly
fd 'VolumeSpec' --type f | grep -E '\.java$' | head -20

# Search for setCachePoolUuid and setCacheMode definitions
echo "=== Searching for setCachePoolUuid and setCacheMode ==="
rg -n 'setCachePoolUuid|setCacheMode' --type java -A2 | head -50

Repository: MatheMatrix/zstack

Length of output: 4345

🏁 Script executed:

#!/bin/bash
# Search for getRootVolumeCachePoolUuid and getRootVolumeCacheMode getters in VmInstanceSpec
rg -n 'getRootVolumeCachePoolUuid|getRootVolumeCacheMode|rootVolumeCachePoolUuid|rootVolumeCacheMode' header/src/main/java/org/zstack/header/vm/VmInstanceSpec.java -A2

Repository: MatheMatrix/zstack

Length of output: 788

避免在根盘默认路径上对 Boolean 自动拆箱。

Line 106 中，spec.getEnableRootVolumeCache() 返回 Boolean（装箱类型），但 vspec.setEnableVolumeCache() 期望 boolean（原始类型）。若值为 null，此行会抛出 NullPointerException。应显式归一化为 false：

-                vspec.setEnableVolumeCache(spec.getEnableRootVolumeCache());
+                vspec.setEnableVolumeCache(Boolean.TRUE.equals(spec.getEnableRootVolumeCache()));

Line 107 与 108 的 setCachePoolUuid() 和 setCacheMode() 接收 String 类型，无此风险。

🤖 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/VmAllocateVolumeFlow.java` around
lines 106 - 108, In VmAllocateVolumeFlow update the call that passes
spec.getEnableRootVolumeCache() into vspec.setEnableVolumeCache() to guard
against Boolean null-unboxing: explicitly normalize the boxed Boolean from
spec.getEnableRootVolumeCache() to a primitive (e.g. treat null as false) before
calling vspec.setEnableVolumeCache(), leaving setCachePoolUuid(...) and
setCacheMode(...) unchanged.

msg.setResourceUuid((String) ctx.get("uuid"));
msg.setName("ROOT-for-" + spec.getVmInventory().getName());
msg.setDescription(String.format("Root volume for VM[uuid:%s]", spec.getVmInventory().getUuid()));
Expand All @@ -116,7 +129,13 @@ protected List<CreateVolumeMsg> prepareMsg(Map<String, Object> ctx) {
} else {
continue;
}

if (vspec.getEnableVolumeCache()) {
tags.add("volumeCache::enable");
tags.add("volumeCache::cacheMode::" + vspec.getCacheMode());
if(vspec.getCachePoolUuid() != null) {
tags.add("volumeCache::poolUuid::" + vspec.getCachePoolUuid());
}
}
Comment on lines +132 to +138
Copy link

@coderabbitai coderabbitai bot Mar 13, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

不要把未配置的 cacheMode 持久化成坏 tag。

Line 134 对 cacheMode 没有空值保护。当前分支已经对 cachePoolUuid 做了空判，但这里会把缺省值写成 volumeCache::cacheMode::null，后续读取方拿到的是持久化坏数据，而不是“未配置”。

💡 建议修改 
             if (vspec.getEnableVolumeCache()) {
                 tags.add("volumeCache::enable");
-                tags.add("volumeCache::cacheMode::" + vspec.getCacheMode());
+                if (vspec.getCacheMode() == null || vspec.getCacheMode().isEmpty()) {
+                    throw new CloudRuntimeException(String.format(
+                            "cacheMode cannot be empty when volume cache is enabled, vmUuid:%s, volumeType:%s",
+                            spec.getVmInventory().getUuid(), vspec.getType()));
+                }
+                tags.add("volumeCache::cacheMode::" + vspec.getCacheMode());
                 if(vspec.getCachePoolUuid() != null) {
                     tags.add("volumeCache::poolUuid::" + vspec.getCachePoolUuid());
                 }
🤖 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/VmAllocateVolumeFlow.java` around
lines 132 - 138, In VmAllocateVolumeFlow where you add volume cache tags (the
block using vspec.getEnableVolumeCache()), avoid persisting a "null" cacheMode
tag: only add the "volumeCache::cacheMode::<value>" tag if vspec.getCacheMode()
is non-null/non-empty (similar to the existing null check for
vspec.getCachePoolUuid()); update the logic around vspec.getCacheMode() so it
conditionally appends that tag (or omits it) to prevent writing
"volumeCache::cacheMode::null".

msg.setSystemTags(new ArrayList<>(tags));
msg.setDiskOfferingUuid(vspec.getDiskOfferingUuid());
msg.setSize(vspec.getSize());
Expand Down
5 changes: 5 additions & 0 deletions compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7355,6 +7355,11 @@ public DiskOfferingVO call(DiskOfferingVO arg) {
}

spec.setDiskAOs(struct.getDiskAOs());
spec.setEnableRootVolumeCache(struct.getEnableRootVolumeCache());
spec.setCacheMode(struct.getCacheMode());
spec.setRootVolumeCachePoolUuid(struct.getRootVolumeCachePoolUuid());
spec.setRootVolumeCacheMode(struct.getRootVolumeCacheMode());
spec.setDataDiskCacheConfigOnIndex(struct.getDataDiskCacheConfigOnIndex());

List<CdRomSpec> cdRomSpecs = buildVmCdRomSpecsForNewCreated(spec);
spec.setCdRomSpecs(cdRomSpecs);
Expand Down
5 changes: 5 additions & 0 deletions compute/src/main/java/org/zstack/compute/vm/VmInstanceManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1323,6 +1323,11 @@ public void run(FlowTrigger trigger, Map data) {
smsg.setDataVolumeSystemTags(msg.getDataVolumeSystemTags());
smsg.setDataVolumeSystemTagsOnIndex(msg.getDataVolumeSystemTagsOnIndex());
smsg.setDiskAOs(msg.getDiskAOs());
smsg.setEnableRootVolumeCache(msg.getEnableRootVolumeCache());
smsg.setCacheMode(msg.getCacheMode());
smsg.setRootVolumeCachePoolUuid(msg.getRootVolumeCachePoolUuid());
smsg.setRootVolumeCacheMode(msg.getRootVolumeCacheMode());
smsg.setDataDiskCacheConfigOnIndex(msg.getDataDiskCacheConfigOnIndex());
bus.makeTargetServiceIdByResourceUuid(smsg, VmInstanceConstant.SERVICE_ID, finalVo.getUuid());
bus.send(smsg, new CloudBusCallBack(smsg) {
@Override
Expand Down
5 changes: 5 additions & 0 deletions compute/src/main/java/org/zstack/compute/vm/VmInstanceUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ public static CreateVmInstanceMsg fromAPICreateVmInstanceMsg(APICreateVmInstance
cmsg.setStrategy(msg.getStrategy());

cmsg.setDiskAOs(msg.getDiskAOs());
cmsg.setEnableRootVolumeCache(msg.getEnableRootVolumeCache());
cmsg.setCacheMode(msg.getCacheMode());
cmsg.setRootVolumeCachePoolUuid(msg.getRootVolumeCachePoolUuid());
cmsg.setRootVolumeCacheMode(msg.getRootVolumeCacheMode());
cmsg.setDataDiskCacheConfigOnIndex(msg.getDataDiskCacheConfigOnIndex());

if (CollectionUtils.isNotEmpty(msg.getDataDiskOfferingUuids()) || CollectionUtils.isNotEmpty(msg.getDataDiskSizes())) {
cmsg.setPrimaryStorageUuidForDataVolume(getPSUuidForDataVolume(msg.getSystemTags()));
Expand Down
36 changes: 36 additions & 0 deletions compute/src/main/java/org/zstack/compute/vm/VmInstantiateOtherDiskFlow.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Configurable;
import org.zstack.compute.allocator.HostAllocatorManager;
import org.zstack.core.Platform;
import org.zstack.core.cloudbus.CloudBus;
import org.zstack.core.cloudbus.CloudBusCallBack;
import org.zstack.core.componentloader.PluginRegistry;
Expand All @@ -18,6 +19,7 @@
import org.zstack.header.errorcode.ErrorCode;
import org.zstack.header.image.ImageBackupStorageRefVO;
import org.zstack.header.image.ImageVO;
import org.zstack.header.localVolumeCache.*;
import org.zstack.header.message.MessageReply;
import org.zstack.header.storage.backup.BackupStorageVO;
import org.zstack.header.storage.backup.BackupStorageVO_;
Expand Down Expand Up @@ -60,6 +62,23 @@ public class VmInstantiateOtherDiskFlow implements Flow {
this.diskAO = diskAO;
}

private VmLocalVolumeCacheVO initCacheRecord() {
VmLocalVolumeCacheVO existing = Q.New(VmLocalVolumeCacheVO.class)
.eq(VmLocalVolumeCacheVO_.volumeUuid, volumeInventory.getUuid())
.find();
if (existing != null) {
return existing;
}
VmLocalVolumeCacheVO cacheVO = new VmLocalVolumeCacheVO();
cacheVO.setUuid(Platform.getUuid());
cacheVO.setVolumeUuid(volumeInventory.getUuid());
cacheVO.setState(VmLocalVolumeCacheState.Uninstantiated);
cacheVO.setCacheMode(VmLocalVolumeCacheMode.valueOf(diskAO.getCacheMode()));
cacheVO.setPoolUuid(diskAO.getCachePoolUuid());
dbf.persist(cacheVO);
return cacheVO;
}
Comment on lines +65 to +80
Copy link

@coderabbitai coderabbitai bot Mar 13, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

缓存记录创建缺少回滚清理，失败路径会遗留脏数据。

initCacheRecord() 会落库，但放在 NoRollbackFlow 中；后续任一环节失败触发外层回滚时，卷可能被删除而缓存记录仍保留，形成孤儿数据。

🧹 建议修复（记录“本次是否新建”，并在 rollback 清理） 
@@
-    VolumeInventory volumeInventory;
+    VolumeInventory volumeInventory;
+    String createdCacheRecordUuid;
@@
     private VmLocalVolumeCacheVO initCacheRecord() {
         VmLocalVolumeCacheVO existing = Q.New(VmLocalVolumeCacheVO.class)
                 .eq(VmLocalVolumeCacheVO_.volumeUuid, volumeInventory.getUuid())
                 .find();
         if (existing != null) {
             return existing;
         }
         VmLocalVolumeCacheVO cacheVO = new VmLocalVolumeCacheVO();
         cacheVO.setUuid(Platform.getUuid());
@@
         cacheVO.setPoolUuid(diskAO.getCachePoolUuid());
         dbf.persist(cacheVO);
+        createdCacheRecordUuid = cacheVO.getUuid();
         return cacheVO;
     }
// 建议补充到 rollback(...) 开头（位于当前变更段之外）
if (createdCacheRecordUuid != null) {
    VmLocalVolumeCacheVO cacheVO = Q.New(VmLocalVolumeCacheVO.class)
            .eq(VmLocalVolumeCacheVO_.uuid, createdCacheRecordUuid)
            .find();
    if (cacheVO != null) {
        dbf.remove(cacheVO);
    }
}

Also applies to: 421-436

🤖 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/VmInstantiateOtherDiskFlow.java`
around lines 65 - 80, initCacheRecord() creates a VmLocalVolumeCacheVO and
persists it inside a NoRollbackFlow without tracking whether it was newly
created; if later steps fail the persisted cache can become an orphan. Fix:
record the created cache UUID (e.g. createdCacheRecordUuid) when
initCacheRecord() persists a new VmLocalVolumeCacheVO, and in the flow's
rollback(...) handler check that flag and remove the persisted
VmLocalVolumeCacheVO via
dbf.remove(Q.New(VmLocalVolumeCacheVO.class).eq(VmLocalVolumeCacheVO_.uuid,
createdCacheRecordUuid).find()) to ensure cleanup; apply the same pattern for
the other occurrence mentioned (lines 421-436).


@Override
public void run(FlowTrigger trigger, Map data) {
VmInstanceInventory instantiateVm = (VmInstanceInventory) data.get(VmInstanceInventory.class.getSimpleName());
Expand Down Expand Up @@ -398,6 +417,23 @@ public void run(MessageReply reply) {
});
}
});

// Enable volume cache if requested for this DiskAO
if (Boolean.TRUE.equals(diskAO.getEnableCache())) {
flow(new NoRollbackFlow() {
String __name__ = String.format("create-cache-record-for-diskAO-volume-on-vm-%s", vmUuid);

@Override
public void run(final FlowTrigger innerTrigger, Map data) {
if (volumeInventory == null) {
innerTrigger.next();
return;
}
initCacheRecord();
innerTrigger.next();
}
});
}
}

private void setupAttachOtherDiskFlows() {
Expand Down
46 changes: 46 additions & 0 deletions conf/db/upgrade/V5.5.12__schema.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
CREATE TABLE IF NOT EXISTS `VmLocalVolumeCachePoolVO` (
`uuid` VARCHAR(32) NOT NULL,
`hostUuid` VARCHAR(32) NOT NULL,
`name` VARCHAR(255) DEFAULT NULL,
`description` VARCHAR(2048) DEFAULT NULL,
`metadata` VARCHAR(2048) DEFAULT NULL,
`state` VARCHAR(32) NOT NULL,
`status` VARCHAR(32) NOT NULL,
`createDate` TIMESTAMP NOT NULL DEFAULT '0000-00-00 00:00:00',
`lastOpDate` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
PRIMARY KEY (`uuid`),
CONSTRAINT `fkVmLocalVolumeCachePoolVOHostEO`
FOREIGN KEY (`hostUuid`) REFERENCES `HostEO` (`uuid`)
ON DELETE CASCADE
) ENGINE = InnoDB DEFAULT CHARSET = utf8;

CREATE TABLE IF NOT EXISTS `VmLocalVolumeCachePoolCapacityVO` (
`uuid` VARCHAR(32) NOT NULL,
`totalCapacity` BIGINT NOT NULL DEFAULT 0,
`availableCapacity` BIGINT NOT NULL DEFAULT 0,
`allocated` BIGINT NOT NULL DEFAULT 0,
`dirty` BIGINT NOT NULL DEFAULT 0,
PRIMARY KEY (`uuid`),
CONSTRAINT `fkVmLocalVolumeCachePoolCapacityVOVmLocalVolumeCachePoolVO`
FOREIGN KEY (`uuid`) REFERENCES `VmLocalVolumeCachePoolVO` (`uuid`)
ON DELETE CASCADE
) ENGINE = InnoDB DEFAULT CHARSET = utf8;

CREATE TABLE IF NOT EXISTS `VmLocalVolumeCacheVO` (
`uuid` VARCHAR(32) NOT NULL,
`volumeUuid` VARCHAR(32) NOT NULL,
`poolUuid` VARCHAR(32) DEFAULT NULL,
`installPath` VARCHAR(2048) DEFAULT NULL,
`cacheMode` VARCHAR(32) NOT NULL,
`state` VARCHAR(32) NOT NULL,
`createDate` TIMESTAMP NOT NULL DEFAULT '0000-00-00 00:00:00',
`lastOpDate` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
PRIMARY KEY (`uuid`),
UNIQUE KEY `uniVmLocalVolumeCacheVOVolumeUuid` (`volumeUuid`),
CONSTRAINT `fkVmLocalVolumeCacheVOVolumeEO`
FOREIGN KEY (`volumeUuid`) REFERENCES `VolumeEO` (`uuid`)
ON DELETE CASCADE,
CONSTRAINT `fkVmLocalVolumeCacheVOPoolUuid`
FOREIGN KEY (`poolUuid`) REFERENCES `VmLocalVolumeCachePoolVO` (`uuid`)
ON DELETE SET NULL
) ENGINE = InnoDB DEFAULT CHARSET = utf8;
Loading