From f01e018c3d9defb296a10f3f5b4e3d4f28b7b370 Mon Sep 17 00:00:00 2001 From: "xiangheng.zhao" Date: Thu, 6 Aug 2026 18:43:16 +0800 Subject: [PATCH] [localstorage]: ZSTAC-84163 check capacity by target primary storage Root Cause: LocalStorageAllocatorFactory used HostAllocatorSpec.getDiskSize() to check every local storage host ref. The value was the total size of root and data disks, so disks assigned to non-local primary storage were also counted against local storage and could incorrectly filter out a valid host. Solution: Carry root and data disk capacity requirements as primary-storage UUID and size tuples. Local storage sums requirements assigned to the current local primary storage. When the host cluster only attaches local storage, add only the capacity of disks whose primary storage is undetermined, instead of counting disks already assigned to another primary storage. Migration uses the same per-primary-storage calculation because its producer records the destination primary storage with the migration capacity. Test: Update CreateVmHostAllocateCase to cover insufficient all-local capacity, mixed local/NFS placement, and partially undetermined primary storage. Verified with: Not run as requested. Resolves: ZSTAC-84163 Change-Id: I3f9895c605819a570a580cedfeaf1d80db300c80 --- .../zstack/compute/vm/VmAllocateHostFlow.java | 25 +++---- .../header/allocator/AllocateHostMsg.java | 60 ++++++++++++++- .../header/allocator/HostAllocatorSpec.java | 22 ++++-- .../local/LocalStorageAllocatorFactory.java | 24 +++++- .../host/CreateVmHostAllocateCase.groovy | 75 +++++++++++++++++++ 5 files changed, 180 insertions(+), 26 deletions(-) diff --git a/compute/src/main/java/org/zstack/compute/vm/VmAllocateHostFlow.java b/compute/src/main/java/org/zstack/compute/vm/VmAllocateHostFlow.java index 8993b278e9b..73d389d5807 100755 --- a/compute/src/main/java/org/zstack/compute/vm/VmAllocateHostFlow.java +++ b/compute/src/main/java/org/zstack/compute/vm/VmAllocateHostFlow.java @@ -52,33 +52,23 @@ public class VmAllocateHostFlow implements Flow { @Autowired protected VmInstanceExtensionPointEmitter extEmitter; - private long getTotalDataDiskSize(VmInstanceSpec spec) { - long size = 0; - for (DiskOfferingInventory dinv : spec.getDataDiskOfferings()) { - size += dinv.getDiskSize(); - } - return size; - } - protected AllocateHostMsg prepareMsg(VmInstanceSpec spec) { DesignatedAllocateHostMsg msg = new DesignatedAllocateHostMsg(); List diskOfferings = new ArrayList<>(); ImageInventory image = spec.getImageSpec().getInventory(); - long diskSize; + long rootDiskSize; if (image == null || (image.getMediaType() != null && image.getMediaType().equals(ImageMediaType.ISO.toString()))) { DiskOfferingVO dvo = dbf.findByUuid(spec.getRootDiskOffering().getUuid(), DiskOfferingVO.class); - diskSize = dvo.getDiskSize(); + rootDiskSize = dvo.getDiskSize(); diskOfferings.add(DiskOfferingInventory.valueOf(dvo)); } else { - diskSize = image.getSize(); + rootDiskSize = image.getSize(); } - diskSize += getTotalDataDiskSize(spec); diskOfferings.addAll(spec.getDataDiskOfferings()); msg.setSoftAvoidHostUuids(spec.getSoftAvoidHostUuids()); msg.setAvoidHostUuids(spec.getAvoidHostUuids()); msg.setDiskOfferings(diskOfferings); - msg.setDiskSize(diskSize); msg.setCpuCapacity(spec.getVmInventory().getCpuNum()); msg.setMemoryCapacity(spec.getVmInventory().getMemorySize()); msg.setClusterUuids(spec.getRequiredClusterUuids()); @@ -136,6 +126,15 @@ public String call(L3NetworkInventory arg) { msg.getRequiredPrimaryStorageUuids().addAll(spec.getDiskAOs().stream() .map(APICreateVmInstanceMsg.DiskAO::getPrimaryStorageUuid).filter(Objects::nonNull).collect(Collectors.toList())); } + String rootPsUuid = spec.getCandidatePrimaryStorageUuidsForRootVolume().size() == 1 ? + spec.getCandidatePrimaryStorageUuidsForRootVolume().get(0) : null; + msg.addRequiredDiskCapacity(rootPsUuid, rootDiskSize); + + String dataPsUuid = spec.getCandidatePrimaryStorageUuidsForDataVolume().size() == 1 ? + spec.getCandidatePrimaryStorageUuidsForDataVolume().get(0) : null; + for (DiskOfferingInventory dinv : spec.getDataDiskOfferings()) { + msg.addRequiredDiskCapacity(dataPsUuid, dinv.getDiskSize()); + } return msg; } diff --git a/header/src/main/java/org/zstack/header/allocator/AllocateHostMsg.java b/header/src/main/java/org/zstack/header/allocator/AllocateHostMsg.java index 132637b8635..8bc7fa4f4cd 100755 --- a/header/src/main/java/org/zstack/header/allocator/AllocateHostMsg.java +++ b/header/src/main/java/org/zstack/header/allocator/AllocateHostMsg.java @@ -5,12 +5,13 @@ import org.zstack.header.message.NeedReplyMessage; import org.zstack.header.vm.VmInstanceInventory; +import javax.persistence.Tuple; +import javax.persistence.TupleElement; import java.util.*; public class AllocateHostMsg extends NeedReplyMessage { private long cpuCapacity; private long memoryCapacity; - private long diskSize; private String allocatorStrategy; private List avoidHostUuids; private List softAvoidHostUuids; @@ -27,6 +28,7 @@ public class AllocateHostMsg extends NeedReplyMessage { private Set requiredPrimaryStorageUuids = new HashSet<>(); // for each set in the list, the primary storage inside is optional private final List> optionalPrimaryStorageUuids = new ArrayList<>(); + private final List requiredDiskCapacities = new ArrayList<>(); private boolean fullAllocate = true; private long oldMemoryCapacity = 0; private AllocationScene allocationScene; @@ -70,6 +72,14 @@ public void addRequiredPrimaryStorageUuid(String requiredPrimaryStorageUuid) { this.requiredPrimaryStorageUuids.add(requiredPrimaryStorageUuid); } + public List getRequiredDiskCapacities() { + return requiredDiskCapacities; + } + + public void addRequiredDiskCapacity(String primaryStorageUuid, long size) { + requiredDiskCapacities.add(newRequiredDiskCapacity(primaryStorageUuid, size)); + } + public String getRequiredBackupStorageUuid() { return requiredBackupStorageUuid; } @@ -143,11 +153,55 @@ public void setMemoryCapacity(long memoryCapacity) { } public long getDiskSize() { - return diskSize; + return requiredDiskCapacities.stream().mapToLong(it -> it.get(1, Long.class)).sum(); } + // Compatibility entry for callers that cannot determine primary storage yet. public void setDiskSize(long diskSize) { - this.diskSize = diskSize; + requiredDiskCapacities.clear(); + requiredDiskCapacities.add(newRequiredDiskCapacity(null, diskSize)); + } + + private Tuple newRequiredDiskCapacity(String primaryStorageUuid, long size) { + return new Tuple() { + @Override + public X get(TupleElement tupleElement) { + throw new UnsupportedOperationException(); + } + + @Override + public Object get(String alias) { + throw new UnsupportedOperationException(); + } + + @Override + public X get(String alias, Class type) { + throw new UnsupportedOperationException(); + } + + @Override + public Object get(int i) { + if (i == 0) { + return primaryStorageUuid; + } + return size; + } + + @Override + public X get(int i, Class type) { + return type.cast(get(i)); + } + + @Override + public Object[] toArray() { + return new Object[]{primaryStorageUuid, size}; + } + + @Override + public List> getElements() { + return Collections.emptyList(); + } + }; } public String getAllocatorStrategy() { diff --git a/header/src/main/java/org/zstack/header/allocator/HostAllocatorSpec.java b/header/src/main/java/org/zstack/header/allocator/HostAllocatorSpec.java index 33d39896c82..a49f2c659e7 100755 --- a/header/src/main/java/org/zstack/header/allocator/HostAllocatorSpec.java +++ b/header/src/main/java/org/zstack/header/allocator/HostAllocatorSpec.java @@ -4,6 +4,7 @@ import org.zstack.header.image.ImageInventory; import org.zstack.header.vm.VmInstanceInventory; +import javax.persistence.Tuple; import java.util.*; /** @@ -14,7 +15,6 @@ public class HostAllocatorSpec { private long cpuCapacity; private long memoryCapacity; private List l3NetworkUuids; - private long diskSize; private String hypervisorType; private String allocatorStrategy; private VmInstanceInventory vmInstance; @@ -29,6 +29,7 @@ public class HostAllocatorSpec { private Set requiredPrimaryStorageUuids = new HashSet<>(); // for each set in the list, the primary storage inside is optional private final List> optionalPrimaryStorageUuids = new ArrayList<>(); + private final List requiredDiskCapacities = new ArrayList<>(); private Map> backupStoragePrimaryStorageMetrics; private boolean dryRun; private List systemTags; @@ -89,6 +90,17 @@ public Set getRequiredPrimaryStorageUuids() { return requiredPrimaryStorageUuids; } + public List getRequiredDiskCapacities() { + return requiredDiskCapacities; + } + + public void setRequiredDiskCapacities(List requiredDiskCapacities) { + this.requiredDiskCapacities.clear(); + if (requiredDiskCapacities != null) { + this.requiredDiskCapacities.addAll(requiredDiskCapacities); + } + } + public List> getOptionalPrimaryStorageUuids() { return optionalPrimaryStorageUuids; } @@ -192,11 +204,7 @@ public void setL3NetworkUuids(List l3NetworkUuids) { } public long getDiskSize() { - return diskSize; - } - - public void setDiskSize(long diskSize) { - this.diskSize = diskSize; + return requiredDiskCapacities.stream().mapToLong(it -> it.get(1, Long.class)).sum(); } public String getHypervisorType() { @@ -261,7 +269,6 @@ public static HostAllocatorSpec fromAllocationMsg(AllocateHostMsg msg) { spec.setAvoidHostUuids(msg.getAvoidHostUuids()); spec.setSoftAvoidHostUuids(msg.getSoftAvoidHostUuids()); spec.setCpuCapacity(msg.getCpuCapacity()); - spec.setDiskSize(msg.getDiskSize()); spec.setListAllHosts(msg.isListAllHosts()); spec.setDryRun(msg.isDryRun()); spec.setFullAllocate(msg.isFullAllocate()); @@ -280,6 +287,7 @@ public static HostAllocatorSpec fromAllocationMsg(AllocateHostMsg msg) { spec.setAllowNoL3Networks(msg.isAllowNoL3Networks()); spec.setRequiredBackupStorageUuid(msg.getRequiredBackupStorageUuid()); spec.setRequiredPrimaryStorageUuids(msg.getRequiredPrimaryStorageUuids()); + spec.setRequiredDiskCapacities(msg.getRequiredDiskCapacities()); msg.getOptionalPrimaryStorageUuids().forEach(spec::addOptionalPrimaryStorageUuids); spec.setAllocationScene(msg.getAllocationScene()); spec.setArchitecture(msg.getArchitecture()); diff --git a/plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageAllocatorFactory.java b/plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageAllocatorFactory.java index 0c90fd3d9bb..feee4825f68 100755 --- a/plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageAllocatorFactory.java +++ b/plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageAllocatorFactory.java @@ -109,7 +109,8 @@ public List filterHostCandidates(List candidates, HostAllocatorS long reservedCapacity = SizeUtils.sizeStringToBytes(PrimaryStorageGlobalConfig.RESERVED_CAPACITY.value()); if (VmOperation.NewCreate.toString().equals(spec.getVmOperation()) || VmOperation.MigrateVolume.toString().equals(spec.getVmOperation())) { - List huuids = getNeedCheckHostLocalStorageList(candidates, spec); + Set onlyLocalStorageHostUuids = new HashSet<>(); + List huuids = getNeedCheckHostLocalStorageList(candidates, spec, onlyLocalStorageHostUuids); if (huuids.isEmpty()) { return candidates; } @@ -126,10 +127,15 @@ public List filterHostCandidates(List candidates, HostAllocatorS for (LocalStorageHostRefVO ref : refs) { String huuid = ref.getHostUuid(); String psUuid = ref.getPrimaryStorageUuid(); + long requiredSize = getRequiredLocalStorageSize(spec, psUuid, onlyLocalStorageHostUuids.contains(huuid)); + if (requiredSize == 0) { + continue; + } + // check primary storage capacity and host physical capacity boolean capacityChecked = PrimaryStorageCapacityChecker.New(psUuid, ref.getAvailableCapacity(), ref.getTotalPhysicalCapacity(), ref.getAvailablePhysicalCapacity()) - .checkRequiredSize(spec.getDiskSize()); + .checkRequiredSize(requiredSize); if (!capacityChecked) { addHostPrimaryStorageBlacklist(huuid, psUuid, spec); @@ -178,6 +184,14 @@ else if (VmOperation.Migrate.toString().equals(spec.getVmOperation())) { return candidates; } + private long getRequiredLocalStorageSize(HostAllocatorSpec spec, String psUuid, boolean onlyAttachedLocalStorage) { + return spec.getRequiredDiskCapacities().stream() + .filter(it -> psUuid.equals(it.get(0, String.class)) || + (onlyAttachedLocalStorage && it.get(0, String.class) == null)) + .mapToLong(it -> it.get(1, Long.class)) + .sum(); + } + private void checkLocalStorageForVmStart(VmInstanceInventory vm, List candidates) { final List localPS = Q.New(PrimaryStorageVO.class) .select(PrimaryStorageVO_.uuid) @@ -221,7 +235,8 @@ private void checkLocalStorageForVmStart(VmInstanceInventory vm, List ca * Negative impact * In the case of local + non-local and no ps specified (non-local is Disconnected/Disabled, or non-local capacity not enough), the allocated host may not have enough disks */ - private List getNeedCheckHostLocalStorageList(List candidates, HostAllocatorSpec spec) { + private List getNeedCheckHostLocalStorageList(List candidates, HostAllocatorSpec spec, + Set onlyLocalStorageHostUuids) { boolean isRequireNonLocalStorage = spec.getRequiredPrimaryStorageUuids() .stream().noneMatch(LocalStorageUtils::isLocalStorage); Map> grouped = candidates.stream().collect( @@ -239,6 +254,9 @@ private List getNeedCheckHostLocalStorageList(List candidates, H } result.addAll(entry.getValue()); + if (isOnlyAttachedLocalStorage) { + onlyLocalStorageHostUuids.addAll(entry.getValue()); + } } return result; diff --git a/test/src/test/groovy/org/zstack/test/integration/storage/primary/local_nfs/allocator/host/CreateVmHostAllocateCase.groovy b/test/src/test/groovy/org/zstack/test/integration/storage/primary/local_nfs/allocator/host/CreateVmHostAllocateCase.groovy index 36709557942..f639e14938c 100644 --- a/test/src/test/groovy/org/zstack/test/integration/storage/primary/local_nfs/allocator/host/CreateVmHostAllocateCase.groovy +++ b/test/src/test/groovy/org/zstack/test/integration/storage/primary/local_nfs/allocator/host/CreateVmHostAllocateCase.groovy @@ -116,6 +116,8 @@ class CreateVmHostAllocateCase extends SubCase { env.create { testGetCandidateZonesClustersHostsForCreatingVm() + testCreateVmAssignLocalAndNfs() + testCreateVmAssignNfs() } } @@ -142,6 +144,60 @@ class CreateVmHostAllocateCase extends SubCase { assert 2 == hosts.size() } + void testCreateVmAssignLocalAndNfs() { + InstanceOfferingInventory instanceOffering = env.inventoryByName("instanceOffering") as InstanceOfferingInventory + DiskOfferingInventory diskOffering = env.inventoryByName("diskOffering") as DiskOfferingInventory + ImageInventory image = env.inventoryByName("image") as ImageInventory + L3NetworkInventory l3 = env.inventoryByName("l3") as L3NetworkInventory + HostInventory host = env.inventoryByName("kvm") + PrimaryStorageInventory nfs = env.inventoryByName("nfs") + PrimaryStorageInventory local = env.inventoryByName("local") + + CreateVmInstanceAction rootAndDataLocalAction = new CreateVmInstanceAction( + name : "rootAndDataLocalVm", + instanceOfferingUuid : instanceOffering.uuid, + imageUuid : image.uuid, + l3NetworkUuids : [l3.uuid], + hostUuid : host.uuid, + dataDiskOfferingUuids : [diskOffering.uuid], + primaryStorageUuidForRootVolume : local.uuid, + systemTags : [VmSystemTags.PRIMARY_STORAGE_UUID_FOR_DATA_VOLUME.instantiateTag([(VmSystemTags.PRIMARY_STORAGE_UUID_FOR_DATA_VOLUME_TOKEN): local.uuid])], + sessionId : currentEnvSpec.session.uuid + ) + assert null != rootAndDataLocalAction.call().error + + CreateVmInstanceAction rootLocalDataNfsAction = new CreateVmInstanceAction( + name : "rootLocalDataNfsVm", + instanceOfferingUuid : instanceOffering.uuid, + imageUuid : image.uuid, + l3NetworkUuids : [l3.uuid], + hostUuid : host.uuid, + dataDiskOfferingUuids : [diskOffering.uuid], + primaryStorageUuidForRootVolume : local.uuid, + systemTags : [VmSystemTags.PRIMARY_STORAGE_UUID_FOR_DATA_VOLUME.instantiateTag([(VmSystemTags.PRIMARY_STORAGE_UUID_FOR_DATA_VOLUME_TOKEN): nfs.uuid])], + sessionId : currentEnvSpec.session.uuid + ) + CreateVmInstanceAction.Result rootLocalDataNfsResult = rootLocalDataNfsAction.call() + assert null == rootLocalDataNfsResult.error + checkVmRootDiskPs(rootLocalDataNfsResult.value.inventory, local.uuid) + checkVmDataDiskPs(rootLocalDataNfsResult.value.inventory, nfs.uuid) + + CreateVmInstanceAction rootLocalDataUnspecifiedAction = new CreateVmInstanceAction( + name : "rootLocalDataUnspecifiedVm", + instanceOfferingUuid : instanceOffering.uuid, + imageUuid : image.uuid, + l3NetworkUuids : [l3.uuid], + hostUuid : host.uuid, + dataDiskOfferingUuids : [diskOffering.uuid], + primaryStorageUuidForRootVolume : local.uuid, + sessionId : currentEnvSpec.session.uuid + ) + CreateVmInstanceAction.Result rootLocalDataUnspecifiedResult = rootLocalDataUnspecifiedAction.call() + assert null == rootLocalDataUnspecifiedResult.error + checkVmRootDiskPs(rootLocalDataUnspecifiedResult.value.inventory, local.uuid) + checkVmDataDiskPs(rootLocalDataUnspecifiedResult.value.inventory, nfs.uuid) + } + void testCreateVmAssignNfs(){ InstanceOfferingInventory instanceOffering = env.inventoryByName("instanceOffering") as InstanceOfferingInventory DiskOfferingInventory diskOffering = env.inventoryByName("diskOffering") as DiskOfferingInventory @@ -184,4 +240,23 @@ class CreateVmHostAllocateCase extends SubCase { ) assert null != createVmInstanceAction.call().error } + + void checkVmRootDiskPs(VmInstanceInventory vm, String psUuid) { + assert vm.allVolumes.size() > 0 + for (VolumeInventory disk : vm.allVolumes) { + if (disk.uuid == vm.rootVolumeUuid) { + assert psUuid == disk.primaryStorageUuid + return + } + } + } + + void checkVmDataDiskPs(VmInstanceInventory vm, String psUuid) { + assert vm.allVolumes.size() > 1 + for (VolumeInventory disk : vm.allVolumes) { + if (disk.uuid != vm.rootVolumeUuid) { + assert psUuid == disk.primaryStorageUuid + } + } + } }