From 2542cf6950c2289e502b7cbfe4effdf5940df8bf 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 only requirements assigned to the current local primary storage. When all primary storages are undetermined, retain the original total-size check for compatibility. When only some requirements are undetermined, check the determined local requirements and leave the undetermined requirements to primary storage allocation. Volume migration retains the total-size check because all migrated volumes share one destination primary storage. 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 | 22 +++++- .../host/CreateVmHostAllocateCase.groovy | 75 +++++++++++++++++++ 5 files changed, 180 insertions(+), 24 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..441cc06f25f 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 @@ -126,10 +126,15 @@ public List filterHostCandidates(List candidates, HostAllocatorS for (LocalStorageHostRefVO ref : refs) { String huuid = ref.getHostUuid(); String psUuid = ref.getPrimaryStorageUuid(); + long requiredSize = getRequiredLocalStorageSize(spec, psUuid); + 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 +183,21 @@ else if (VmOperation.Migrate.toString().equals(spec.getVmOperation())) { return candidates; } + private long getRequiredLocalStorageSize(HostAllocatorSpec spec, String psUuid) { + if (VmOperation.MigrateVolume.toString().equals(spec.getVmOperation())) { + return spec.getDiskSize(); + } + + long requiredSize = spec.getRequiredDiskCapacities().stream() + .filter(it -> psUuid.equals(it.get(0, String.class))) + .mapToLong(it -> it.get(1, Long.class)) + .sum(); + + boolean allPrimaryStorageUndetermined = spec.getRequiredDiskCapacities().stream() + .allMatch(it -> it.get(0, String.class) == null); + return allPrimaryStorageUndetermined ? spec.getDiskSize() : requiredSize; + } + private void checkLocalStorageForVmStart(VmInstanceInventory vm, List candidates) { final List localPS = Q.New(PrimaryStorageVO.class) .select(PrimaryStorageVO_.uuid) 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 + } + } + } }