Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,62 @@ 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().getInventory();
if (image != null && 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();
Comment on lines +157 to +161
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

数据盘缓存容量的索引基准和卷创建流程不一致。

Line 157-161 这里按 spec.getDataDiskOfferings().get(index) 取大小,但同一份 dataDiskCacheConfigOnIndexcompute/src/main/java/org/zstack/compute/vm/VmAllocateVolumeFlow.java 的 Line 77-86 是按 dataVolumeSpecs 套用的。两边一旦出现非 offering 数据卷或索引不完全一致,这里就会把 requiredSize 算错,甚至直接 IndexOutOfBoundsException

🤖 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 157 - 161, The code in VmAllocateHostFlow sums data disk sizes using
spec.getDataDiskOfferings().get(index).getDiskSize() while the allocation logic
in VmAllocateVolumeFlow uses spec.getDataVolumeSpecs() as the index basis; this
mismatch can produce wrong requiredSize or IndexOutOfBoundsException. Fix by
reading sizes from spec.getDataVolumeSpecs().get(index).getDiskSize() (or
otherwise map the same index space used by dataVolumeSpecs) when iterating
spec.getDataDiskCacheConfigOnIndex(), and add defensive checks that the
referenced dataVolumeSpecs entry exists and is non-null before accessing
getDiskSize() (fall back or skip if missing) so both flows use the same index
basis and avoid OOB errors.

}

}
return 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
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

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

空指针会让未配置数据盘缓存的建机流程直接失败。

Line 76 这里先对 spec.getDataDiskCacheConfigOnIndex()isEmpty(),但同一 PR 的 compute/src/main/java/org/zstack/compute/vm/VmAllocateHostFlow.java Line 157-158 已经按 nullable 处理了这个 getter。这里一旦返回 null,建机会在 volume allocation 阶段直接 NPE。

🛠 建议修改
-        if (!spec.getDataDiskCacheConfigOnIndex().isEmpty() && !CollectionUtils.isEmpty(spec.getDataDiskOfferings())) {
+        if (!MapUtils.isEmpty(spec.getDataDiskCacheConfigOnIndex()) && !CollectionUtils.isEmpty(spec.getDataDiskOfferings())) {
🤖 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
spec.getDataDiskCacheConfigOnIndex().isEmpty() can NPE if the getter returns
null; change the guard to use a null-safe check (e.g.
!CollectionUtils.isEmpty(spec.getDataDiskCacheConfigOnIndex()) &&
!CollectionUtils.isEmpty(spec.getDataDiskOfferings())) before iterating, and
ensure the IntStream index loop only runs up to dataVolumeSpecs.size() and the
cache config list size (safeguard by checking both list sizes or using
min(dataVolumeSpecs.size(), spec.getDataDiskCacheConfigOnIndex().size())); keep
the existing null check for each config inside the loop.


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());
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());
}
}
msg.setSystemTags(new ArrayList<>(tags));
msg.setDiskOfferingUuid(vspec.getDiskOfferingUuid());
msg.setSize(vspec.getSize());
Expand Down
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
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
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
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;
}

@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
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