From 6c73a293a0d6e152a0961a193f84a7e637d0ba3b Mon Sep 17 00:00:00 2001 From: Bryan Lima Date: Wed, 13 Mar 2024 14:23:27 -0300 Subject: [PATCH 1/7] Apply IOPS limitation when resizing/migrating volume --- .../api/storage/MigrateVolumeCommand.java | 9 ++++ .../api/storage/ResizeVolumeCommand.java | 19 ++++++++- .../service/VolumeOrchestrationService.java | 2 +- .../orchestration/VolumeOrchestrator.java | 9 ++-- .../motion/AncientDataMotionStrategy.java | 11 +++++ .../vmware/resource/VmwareResource.java | 41 ++++++++++++++++++- .../CloudStackPrimaryDataStoreDriverImpl.java | 2 + .../cloud/storage/VolumeApiServiceImpl.java | 12 +++--- .../vm/UnmanagedVMsManagerImpl.java | 2 +- 9 files changed, 93 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/com/cloud/agent/api/storage/MigrateVolumeCommand.java b/core/src/main/java/com/cloud/agent/api/storage/MigrateVolumeCommand.java index 9fed0f913e14..f50415c097c4 100644 --- a/core/src/main/java/com/cloud/agent/api/storage/MigrateVolumeCommand.java +++ b/core/src/main/java/com/cloud/agent/api/storage/MigrateVolumeCommand.java @@ -36,6 +36,7 @@ public class MigrateVolumeCommand extends Command { String attachedVmName; Volume.Type volumeType; String hostGuidInTargetCluster; + Long newIops; private DataTO srcData; private DataTO destData; @@ -145,4 +146,12 @@ public int getWaitInMillSeconds() { } public String getChainInfo() { return chainInfo; } + + public void setNewIops(Long newIops) { + this.newIops = newIops; + } + + public Long getNewIops() { + return newIops; + } } diff --git a/core/src/main/java/com/cloud/agent/api/storage/ResizeVolumeCommand.java b/core/src/main/java/com/cloud/agent/api/storage/ResizeVolumeCommand.java index db867698e91e..a8d88ca3124d 100644 --- a/core/src/main/java/com/cloud/agent/api/storage/ResizeVolumeCommand.java +++ b/core/src/main/java/com/cloud/agent/api/storage/ResizeVolumeCommand.java @@ -33,6 +33,8 @@ public class ResizeVolumeCommand extends Command { private boolean shrinkOk; private String vmInstance; private String chainInfo; + private Long newMaxIops; + private Long newMinIops; /* For managed storage */ private boolean managed; @@ -70,7 +72,6 @@ public ResizeVolumeCommand(String path, StorageFilerTO pool, Long currentSize, L public ResizeVolumeCommand(String path, StorageFilerTO pool, Long currentSize, Long newSize, boolean shrinkOk, String vmInstance, boolean isManaged, String iScsiName) { this(path, pool, currentSize, newSize, shrinkOk, vmInstance); - this.iScsiName = iScsiName; this.managed = isManaged; } @@ -120,4 +121,20 @@ public void clearPassphrase() { public boolean executeInSequence() { return false; } + + public Long getNewMaxIops() { + return newMaxIops; + } + + public void setNewMaxIops(Long newMaxIops) { + this.newMaxIops = newMaxIops; + } + + public Long getNewMinIops() { + return newMinIops; + } + + public void setNewMinIops(Long newMinIops) { + this.newMinIops = newMinIops; + } } diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java index 7950dda4d68e..016aaf34e331 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java @@ -98,7 +98,7 @@ VolumeInfo moveVolume(VolumeInfo volume, long destPoolDcId, Long destPoolPodId, VolumeInfo createVolumeFromSnapshot(Volume volume, Snapshot snapshot, UserVm vm) throws StorageUnavailableException; - Volume migrateVolume(Volume volume, StoragePool destPool) throws StorageUnavailableException; + Volume migrateVolume(Volume volume, StoragePool destPool, DiskOffering newDiskOffering) throws StorageUnavailableException; Volume liveMigrateVolume(Volume volume, StoragePool destPool); diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java index 36e281459492..a959664a14b7 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java @@ -310,7 +310,7 @@ public VolumeInfo moveVolume(VolumeInfo volumeInfo, long destPoolDcId, Long dest throw new CloudRuntimeException(String.format("Failed to find a storage pool with enough capacity to move the volume [%s] to.", volumeToString)); } - Volume newVol = migrateVolume(volumeInfo, destPool); + Volume newVol = migrateVolume(volumeInfo, destPool, diskOffering); return volFactory.getVolume(newVol.getId()); } @@ -1343,13 +1343,14 @@ private void checkConcurrentJobsPerDatastoreThreshhold(final StoragePool destPoo @Override @DB - public Volume migrateVolume(Volume volume, StoragePool destPool) throws StorageUnavailableException { + public Volume migrateVolume(Volume volume, StoragePool destPool, DiskOffering newDiskOffering) throws StorageUnavailableException { String volumeToString = getVolumeIdentificationInfos(volume); VolumeInfo vol = volFactory.getVolume(volume.getId()); if (vol == null){ throw new CloudRuntimeException(String.format("Volume migration failed because volume [%s] is null.", volumeToString)); } + vol.addPayload(newDiskOffering); if (destPool == null) { throw new CloudRuntimeException("Volume migration failed because the destination storage pool is not available."); } @@ -1478,7 +1479,7 @@ public boolean storageMigration(VirtualMachineProfile vm, Map entry : volumeStoragePoolMap.entrySet()) { - Volume result = migrateVolume(entry.getKey(), entry.getValue()); + Volume result = migrateVolume(entry.getKey(), entry.getValue(), null); if (result == null) { return false; } @@ -1939,7 +1940,7 @@ public void prepare(VirtualMachineProfile vm, DeployDestination dest) throws Sto } else if (task.type == VolumeTaskType.MIGRATE) { store = (PrimaryDataStore) dataStoreMgr.getDataStore(task.pool.getId(), DataStoreRole.Primary); updateVolumeSize(store, task.volume); - vol = migrateVolume(task.volume, store); + vol = migrateVolume(task.volume, store, null); } else if (task.type == VolumeTaskType.RECREATE) { Pair result = recreateVolume(task.volume, vm, dest); store = (PrimaryDataStore) dataStoreMgr.getDataStore(result.second().getId(), DataStoreRole.Primary); diff --git a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java index 0b0065361d07..bb4703555a8f 100644 --- a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java +++ b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java @@ -69,6 +69,7 @@ import com.cloud.storage.DataStoreRole; import com.cloud.storage.Snapshot.Type; import com.cloud.storage.SnapshotVO; +import com.cloud.storage.DiskOfferingVO; import com.cloud.storage.StorageManager; import com.cloud.storage.Storage.StoragePoolType; import com.cloud.storage.StoragePool; @@ -459,6 +460,16 @@ protected Answer migrateVolumeToPool(DataObject srcData, DataObject destData) { StoragePool srcPool = (StoragePool)dataStoreMgr.getDataStore(srcData.getDataStore().getId(), DataStoreRole.Primary); StoragePool destPool = (StoragePool)dataStoreMgr.getDataStore(destData.getDataStore().getId(), DataStoreRole.Primary); MigrateVolumeCommand command = new MigrateVolumeCommand(volume.getId(), volume.getPath(), destPool, volume.getAttachedVmName(), volume.getVolumeType(), waitInterval, volume.getChainInfo()); + + if (volume.getpayload() instanceof DiskOfferingVO) { + DiskOfferingVO offering = (DiskOfferingVO) volume.getpayload(); + Long newIops = null; + if (offering.getIopsReadRate() != null && offering.getIopsWriteRate() != null) { + newIops = offering.getIopsReadRate() + offering.getIopsWriteRate(); + } + command.setNewIops(newIops); + } + if (srcPool.getParent() != 0) { command.setContextParam(DiskTO.PROTOCOL_TYPE, Storage.StoragePoolType.DatastoreCluster.toString()); } diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java index 6f6ae6f41e76..06c78e19f950 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java @@ -45,6 +45,7 @@ import java.util.UUID; import java.util.stream.Collectors; +import com.vmware.vim25.StorageIOAllocationInfo; import javax.naming.ConfigurationException; import javax.xml.datatype.XMLGregorianCalendar; @@ -75,6 +76,9 @@ import org.apache.commons.lang.math.NumberUtils; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.ThreadContext; +import org.apache.commons.lang3.ObjectUtils; +import org.apache.log4j.Logger; +import org.apache.log4j.NDC; import org.joda.time.Duration; import com.cloud.agent.IAgentControl; @@ -868,6 +872,8 @@ private Answer execute(ResizeVolumeCommand cmd) { boolean managed = cmd.isManaged(); String poolUUID = cmd.getPoolUuid(); String chainInfo = cmd.getChainInfo(); + Long newMinIops = cmd.getNewMinIops(); + Long newMaxIops = cmd.getNewMaxIops(); boolean useWorkerVm = false; VmwareContext context = getServiceContext(); @@ -882,7 +888,7 @@ private Answer execute(ResizeVolumeCommand cmd) { oldSize / Float.valueOf(ResourceType.bytesToMiB), newSize / Float.valueOf(ResourceType.bytesToMiB), vmName); logger.error(errorMsg); throw new Exception(errorMsg); - } else if (newSize == oldSize) { + } else if (newSize == oldSize && ObjectUtils.allNull(newMaxIops, newMinIops)) { return new ResizeVolumeAnswer(cmd, true, "success", newSize * ResourceType.bytesToKiB); } @@ -977,11 +983,21 @@ private Answer execute(ResizeVolumeCommand cmd) { VirtualDisk disk = getDiskAfterResizeDiskValidations(vmMo, path); String vmdkAbsFile = VmwareHelper.getAbsoluteVmdkFile(disk); + StorageIOAllocationInfo limitIops = disk.getStorageIOAllocation(); + if (ObjectUtils.allNotNull(newMinIops, newMaxIops)) { + Long iops = newMinIops + newMaxIops; + s_logger.debug(LogUtils.logGsonWithoutException("Adding [%s] as the new IOPS limit of disk [%s].", iops, disk)); + StorageIOAllocationInfo newIops = new StorageIOAllocationInfo(); + newIops.setLimit(iops); + limitIops = newIops; + } + if (vmdkAbsFile != null && !vmdkAbsFile.isEmpty()) { vmMo.updateAdapterTypeIfRequired(vmdkAbsFile); } disk.setCapacityInKB(newSize); + disk.setStorageIOAllocation(limitIops); VirtualDeviceConfigSpec deviceConfigSpec = new VirtualDeviceConfigSpec(); @@ -5102,6 +5118,29 @@ private Answer execute(MigrateVolumeCommand cmd) { volumePath = vmMo.getVmdkFileBaseName(disk); } } + if (cmd.getNewIops() != null) { + String vmwareDocumentation = "https://kb.vmware.com/s/article/68164"; + Long newIops = cmd.getNewIops(); + VirtualDisk disk = vmMo.getDiskDevice(volumePath, true).first(); + try { + s_logger.debug(LogUtils.logGsonWithoutException("Trying to change disk [%s] IOPS to [%s].", disk, newIops)); + VirtualMachineConfigSpec vmConfigSpec = new VirtualMachineConfigSpec(); + VirtualDeviceConfigSpec deviceConfigSpec = new VirtualDeviceConfigSpec(); + + StorageIOAllocationInfo storageIOAllocation = new StorageIOAllocationInfo(); + storageIOAllocation.setLimit(newIops); + disk.setStorageIOAllocation(storageIOAllocation); + + deviceConfigSpec.setDevice(disk); + deviceConfigSpec.setOperation(VirtualDeviceConfigSpecOperation.EDIT); + vmConfigSpec.getDeviceChange().add(deviceConfigSpec); + vmMo.configureVm(vmConfigSpec); + } catch (Exception e) { + s_logger.error(LogUtils.logGsonWithoutException("Failed to change disk [%s] IOPS to [%s] due to [%s]. This happens " + + "when the disk controller is IDE. Please read this documentation for more information: [%s]. ", disk, newIops, + e.getMessage(), vmwareDocumentation), e); + } + } VirtualMachineDiskInfoBuilder diskInfoBuilder = vmMo.getDiskInfoBuilder(); chainInfo = _gson.toJson(diskInfoBuilder.getDiskInfoByBackingFileBaseName(volumePath, targetDsMo.getName())); MigrateVolumeAnswer answer = new MigrateVolumeAnswer(cmd, true, null, volumePath); diff --git a/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java b/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java index 02a28b6e947d..495639fde206 100644 --- a/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java +++ b/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java @@ -437,6 +437,8 @@ public void resize(DataObject data, AsyncCompletionCallback cal } ResizeVolumeCommand resizeCmd = new ResizeVolumeCommand(vol.getPath(), new StorageFilerTO(pool), vol.getSize(), resizeParameter.newSize, resizeParameter.shrinkOk, resizeParameter.instanceName, vol.getChainInfo(), vol.getPassphrase(), vol.getEncryptFormat()); + resizeCmd.setNewMinIops(resizeParameter.newMinIops); + resizeCmd.setNewMaxIops(resizeParameter.newMaxIops); if (pool.getParent() != 0) { resizeCmd.setContextParam(DiskTO.PROTOCOL_TYPE, Storage.StoragePoolType.DatastoreCluster.toString()); } diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 73a7f7ab5466..b1df9f5f75ee 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -1243,8 +1243,8 @@ public VolumeVO resizeVolume(ResizeVolumeCmd cmd) throws ResourceAllocationExcep validateIops(newMinIops, newMaxIops, volume.getPoolType()); } else { - newMinIops = newDiskOffering.getMinIops(); - newMaxIops = newDiskOffering.getMaxIops(); + newMinIops = newDiskOffering.getMinIops() != null ? newDiskOffering.getMinIops() : newDiskOffering.getIopsReadRate(); + newMaxIops = newDiskOffering.getMaxIops() != null ? newDiskOffering.getMaxIops() : newDiskOffering.getIopsWriteRate(); } // if the hypervisor snapshot reserve value is null, it must remain null (currently only KVM uses null and null is all KVM uses for a value here) @@ -3560,9 +3560,9 @@ private Volume orchestrateMigrateVolume(VolumeVO volume, StoragePool destPool, b Volume newVol = null; try { if (liveMigrateVolume) { - newVol = liveMigrateVolume(volume, destPool); + newVol = liveMigrateVolume(volume, destPool, newDiskOffering); } else { - newVol = _volumeMgr.migrateVolume(volume, destPool); + newVol = _volumeMgr.migrateVolume(volume, destPool, newDiskOffering); } if (newDiskOffering != null) { _volsDao.updateDiskOffering(newVol.getId(), newDiskOffering.getId()); @@ -3578,9 +3578,9 @@ private Volume orchestrateMigrateVolume(VolumeVO volume, StoragePool destPool, b } @DB - protected Volume liveMigrateVolume(Volume volume, StoragePool destPool) throws StorageUnavailableException { + protected Volume liveMigrateVolume(Volume volume, StoragePool destPool, DiskOfferingVO newDiskOffering) throws StorageUnavailableException { VolumeInfo vol = volFactory.getVolume(volume.getId()); - + vol.addPayload(newDiskOffering); DataStore dataStoreTarget = dataStoreMgr.getDataStore(destPool.getId(), DataStoreRole.Primary); AsyncCallFuture future = volService.migrateVolume(vol, dataStoreTarget); try { diff --git a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java index 267faf66dc25..238c61de4d66 100644 --- a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java @@ -1000,7 +1000,7 @@ private UserVm migrateImportedVM(HostVO sourceHost, VirtualMachineTemplate templ if (vm.getState().equals(VirtualMachine.State.Running)) { volume = volumeManager.liveMigrateVolume(volumeVO, storagePool); } else { - volume = volumeManager.migrateVolume(volumeVO, storagePool); + volume = volumeManager.migrateVolume(volumeVO, storagePool, dOffering); } if (volume == null) { String msg = ""; From fd3297400be2cfc878a5113c7b9554c742ef97ae Mon Sep 17 00:00:00 2001 From: SadiJr Date: Tue, 12 Dec 2023 15:41:12 -0300 Subject: [PATCH 2/7] Address Joao review --- .../cloud/hypervisor/vmware/resource/VmwareResource.java | 4 ++-- .../com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java index 06c78e19f950..09cf95b741d3 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java @@ -4389,7 +4389,7 @@ protected GetVolumeStatsAnswer execute(GetVolumeStatsCommand cmd) { for (String diskPath : disks) { DatastoreFile file = new DatastoreFile(diskPath); VirtualMachineMO vmMo = dcMo.findVm(file.getDir()); - Pair vds = vmMo.getDiskDevice(file.getFileName(), true); + Pair vds = vmMo.getDiskDevice(file.getFileName(), true, false); long virtualsize = vds.first().getCapacityInKB() * 1024; long physicalsize = primaryStorageDatastoreMo.fileDiskSize(file.getPath()); if (statEntry.containsKey(chainInfo)) { @@ -5121,7 +5121,7 @@ private Answer execute(MigrateVolumeCommand cmd) { if (cmd.getNewIops() != null) { String vmwareDocumentation = "https://kb.vmware.com/s/article/68164"; Long newIops = cmd.getNewIops(); - VirtualDisk disk = vmMo.getDiskDevice(volumePath, true).first(); + VirtualDisk disk = vmMo.getDiskDevice(volumePath, true, true).first(); try { s_logger.debug(LogUtils.logGsonWithoutException("Trying to change disk [%s] IOPS to [%s].", disk, newIops)); VirtualMachineConfigSpec vmConfigSpec = new VirtualMachineConfigSpec(); diff --git a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java index e0033e55fc6f..d0de9e467ae0 100644 --- a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java +++ b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java @@ -2739,9 +2739,13 @@ public Pair getDiskDevice(String vmdkDatastorePath) throws } // return pair of VirtualDisk and disk device bus name(ide0:0, etc) - public Pair getDiskDevice(String vmdkDatastorePath, boolean matchExactly) throws Exception { + public Pair getDiskDevice(String vmdkDatastorePath, boolean matchExactly, boolean discardDot) throws Exception { List devices = _context.getVimClient().getDynamicProperty(_mor, "config.hardware.device"); + if (discardDot) { + vmdkDatastorePath = vmdkDatastorePath + "."; + } + DatastoreFile dsSrcFile = new DatastoreFile(vmdkDatastorePath); String srcBaseName = dsSrcFile.getFileBaseName(); String trimmedSrcBaseName = VmwareHelper.trimSnapshotDeltaPostfix(srcBaseName); From 936951cfd7ca7eabad920113b9f3aeb3497a3a5b Mon Sep 17 00:00:00 2001 From: Bryan Lima Date: Fri, 23 Aug 2024 13:53:56 -0300 Subject: [PATCH 3/7] Sets IOPS limit to -1 for offerings with no IOPS limitatin --- .../vmware/resource/VmwareResource.java | 82 +++++++++++-------- 1 file changed, 46 insertions(+), 36 deletions(-) diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java index 09cf95b741d3..47ba10b84b54 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java @@ -77,8 +77,6 @@ import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.ThreadContext; import org.apache.commons.lang3.ObjectUtils; -import org.apache.log4j.Logger; -import org.apache.log4j.NDC; import org.joda.time.Duration; import com.cloud.agent.IAgentControl; @@ -888,8 +886,6 @@ private Answer execute(ResizeVolumeCommand cmd) { oldSize / Float.valueOf(ResourceType.bytesToMiB), newSize / Float.valueOf(ResourceType.bytesToMiB), vmName); logger.error(errorMsg); throw new Exception(errorMsg); - } else if (newSize == oldSize && ObjectUtils.allNull(newMaxIops, newMinIops)) { - return new ResizeVolumeAnswer(cmd, true, "success", newSize * ResourceType.bytesToKiB); } if (vmName.equalsIgnoreCase("none")) { @@ -983,21 +979,13 @@ private Answer execute(ResizeVolumeCommand cmd) { VirtualDisk disk = getDiskAfterResizeDiskValidations(vmMo, path); String vmdkAbsFile = VmwareHelper.getAbsoluteVmdkFile(disk); - StorageIOAllocationInfo limitIops = disk.getStorageIOAllocation(); - if (ObjectUtils.allNotNull(newMinIops, newMaxIops)) { - Long iops = newMinIops + newMaxIops; - s_logger.debug(LogUtils.logGsonWithoutException("Adding [%s] as the new IOPS limit of disk [%s].", iops, disk)); - StorageIOAllocationInfo newIops = new StorageIOAllocationInfo(); - newIops.setLimit(iops); - limitIops = newIops; - } + setDiskIops(disk, newMinIops, newMaxIops); if (vmdkAbsFile != null && !vmdkAbsFile.isEmpty()) { vmMo.updateAdapterTypeIfRequired(vmdkAbsFile); } disk.setCapacityInKB(newSize); - disk.setStorageIOAllocation(limitIops); VirtualDeviceConfigSpec deviceConfigSpec = new VirtualDeviceConfigSpec(); @@ -1042,6 +1030,22 @@ private Answer execute(ResizeVolumeCommand cmd) { } } + /** + * Sets the disk IOPS which is the sum of min IOPS and max IOPS; if they are null, the IOPS limit is set to -1 (unlimited). + */ + private void setDiskIops(VirtualDisk disk, Long newMinIops, Long newMaxIops) { + StorageIOAllocationInfo storageIOAllocation = new StorageIOAllocationInfo(); + Long iops = -1L; + + if (ObjectUtils.allNotNull(newMinIops, newMaxIops)) { + iops = newMinIops + newMaxIops; + } + + storageIOAllocation.setLimit(iops); + logger.debug(LogUtils.logGsonWithoutException("Setting [%s] as the IOPS limit of disk [%s].", iops == -1L ? "unlimited" : iops, disk)); + disk.setStorageIOAllocation(storageIOAllocation); + } + private VirtualDisk getDiskAfterResizeDiskValidations(VirtualMachineMO vmMo, String volumePath) throws Exception { Pair vdisk = vmMo.getDiskDevice(volumePath); if (vdisk == null) { @@ -5118,29 +5122,8 @@ private Answer execute(MigrateVolumeCommand cmd) { volumePath = vmMo.getVmdkFileBaseName(disk); } } - if (cmd.getNewIops() != null) { - String vmwareDocumentation = "https://kb.vmware.com/s/article/68164"; - Long newIops = cmd.getNewIops(); - VirtualDisk disk = vmMo.getDiskDevice(volumePath, true, true).first(); - try { - s_logger.debug(LogUtils.logGsonWithoutException("Trying to change disk [%s] IOPS to [%s].", disk, newIops)); - VirtualMachineConfigSpec vmConfigSpec = new VirtualMachineConfigSpec(); - VirtualDeviceConfigSpec deviceConfigSpec = new VirtualDeviceConfigSpec(); - - StorageIOAllocationInfo storageIOAllocation = new StorageIOAllocationInfo(); - storageIOAllocation.setLimit(newIops); - disk.setStorageIOAllocation(storageIOAllocation); - - deviceConfigSpec.setDevice(disk); - deviceConfigSpec.setOperation(VirtualDeviceConfigSpecOperation.EDIT); - vmConfigSpec.getDeviceChange().add(deviceConfigSpec); - vmMo.configureVm(vmConfigSpec); - } catch (Exception e) { - s_logger.error(LogUtils.logGsonWithoutException("Failed to change disk [%s] IOPS to [%s] due to [%s]. This happens " - + "when the disk controller is IDE. Please read this documentation for more information: [%s]. ", disk, newIops, - e.getMessage(), vmwareDocumentation), e); - } - } + + setDiskIops(cmd, vmMo, volumePath); VirtualMachineDiskInfoBuilder diskInfoBuilder = vmMo.getDiskInfoBuilder(); chainInfo = _gson.toJson(diskInfoBuilder.getDiskInfoByBackingFileBaseName(volumePath, targetDsMo.getName())); MigrateVolumeAnswer answer = new MigrateVolumeAnswer(cmd, true, null, volumePath); @@ -5153,6 +5136,33 @@ private Answer execute(MigrateVolumeCommand cmd) { } } + /** + * Sets the disk IOPS limitation, if the {@link MigrateVolumeCommand} did not specify this limitation, then it is set to -1 (unlimited). + */ + private void setDiskIops(MigrateVolumeCommand cmd, VirtualMachineMO vmMo, String volumePath) throws Exception { + Long newIops = cmd.getNewIops() == null ? -1L : cmd.getNewIops(); + VirtualDisk disk = vmMo.getDiskDevice(volumePath, true, true).first(); + + try { + logger.debug(LogUtils.logGsonWithoutException("Trying to change disk [%s] IOPS to [%s].", disk, newIops)); + VirtualMachineConfigSpec vmConfigSpec = new VirtualMachineConfigSpec(); + VirtualDeviceConfigSpec deviceConfigSpec = new VirtualDeviceConfigSpec(); + + StorageIOAllocationInfo storageIOAllocation = new StorageIOAllocationInfo(); + storageIOAllocation.setLimit(newIops); + disk.setStorageIOAllocation(storageIOAllocation); + + deviceConfigSpec.setDevice(disk); + deviceConfigSpec.setOperation(VirtualDeviceConfigSpecOperation.EDIT); + vmConfigSpec.getDeviceChange().add(deviceConfigSpec); + vmMo.configureVm(vmConfigSpec); + } catch (Exception e) { + String vmwareDocumentation = "https://kb.vmware.com/s/article/68164"; + logger.error(LogUtils.logGsonWithoutException("Failed to change disk [%s] IOPS to [%s] due to [%s]. This happens when the disk controller is IDE." + + " Please read this documentation for more information: [%s]. ", disk, newIops, e.getMessage(), vmwareDocumentation), e); + } + } + private Pair getVirtualDiskInfo(VirtualMachineMO vmMo, String srcDiskName) throws Exception { Pair deviceInfo = vmMo.getDiskDevice(srcDiskName); if (deviceInfo == null) { From c3f41a9fd999379ec8f3045da735b5939ce06f44 Mon Sep 17 00:00:00 2001 From: Bryan Lima Date: Fri, 23 Aug 2024 14:10:45 -0300 Subject: [PATCH 4/7] Rename method parameter --- .../java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java index d0de9e467ae0..66142dd10546 100644 --- a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java +++ b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java @@ -2739,10 +2739,10 @@ public Pair getDiskDevice(String vmdkDatastorePath) throws } // return pair of VirtualDisk and disk device bus name(ide0:0, etc) - public Pair getDiskDevice(String vmdkDatastorePath, boolean matchExactly, boolean discardDot) throws Exception { + public Pair getDiskDevice(String vmdkDatastorePath, boolean matchExactly, boolean ignoreDotOnPath) throws Exception { List devices = _context.getVimClient().getDynamicProperty(_mor, "config.hardware.device"); - if (discardDot) { + if (ignoreDotOnPath) { vmdkDatastorePath = vmdkDatastorePath + "."; } From 47f0cd1e84ad205d49d2c484da650e17437c1909 Mon Sep 17 00:00:00 2001 From: Bryan Lima Date: Mon, 2 Sep 2024 14:13:08 -0300 Subject: [PATCH 5/7] Address zero IOPS --- .../com/cloud/hypervisor/vmware/resource/VmwareResource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java index 47ba10b84b54..5716c28e5259 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java @@ -1037,7 +1037,7 @@ private void setDiskIops(VirtualDisk disk, Long newMinIops, Long newMaxIops) { StorageIOAllocationInfo storageIOAllocation = new StorageIOAllocationInfo(); Long iops = -1L; - if (ObjectUtils.allNotNull(newMinIops, newMaxIops)) { + if (ObjectUtils.allNotNull(newMinIops, newMaxIops) && newMinIops > 0 && newMaxIops > 0) { iops = newMinIops + newMaxIops; } From 19deded60801cd01924f70868954089ff056c639 Mon Sep 17 00:00:00 2001 From: Henrique Sato Date: Fri, 4 Apr 2025 13:09:07 -0300 Subject: [PATCH 6/7] Adjusts --- .../cloud/deploy/DeploymentPlanningManagerImpl.java | 2 +- .../java/com/cloud/storage/VolumeApiServiceImpl.java | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java b/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java index 7ccead9f9dc2..e7b926eb4e44 100644 --- a/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java +++ b/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java @@ -31,7 +31,7 @@ import java.util.Set; import java.util.Timer; import java.util.TreeSet; -import java.util.stream.Collectors;Res +import java.util.stream.Collectors; import javax.inject.Inject; import javax.naming.ConfigurationException; diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 30753b383ddd..f7fb37f37051 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -1318,10 +1318,8 @@ public VolumeVO resizeVolume(ResizeVolumeCmd cmd) throws ResourceAllocationExcep volumeMigrateRequired = true; } - boolean volumeResizeRequired = false; - if (currentSize != newSize || !compareEqualsIncludingNullOrZero(newMaxIops, volume.getMaxIops()) || !compareEqualsIncludingNullOrZero(newMinIops, volume.getMinIops())) { - volumeResizeRequired = true; - } + boolean volumeResizeRequired = currentSize != newSize || !compareEqualsIncludingNullOrZero(newMaxIops, volume.getMaxIops()) || !compareEqualsIncludingNullOrZero(newMinIops, volume.getMinIops()) + || !compareEqualsIncludingNullOrZero(newMaxIops, diskOffering.getIopsWriteRate()) || !compareEqualsIncludingNullOrZero(newMinIops, diskOffering.getIopsReadRate()); if (!volumeMigrateRequired && !volumeResizeRequired && newDiskOffering != null) { _volsDao.updateDiskOffering(volume.getId(), newDiskOffering.getId()); volume = _volsDao.findById(volume.getId()); @@ -1386,7 +1384,11 @@ public VolumeVO resizeVolume(ResizeVolumeCmd cmd) throws ResourceAllocationExcep } else if (jobResult instanceof Throwable) { throw new RuntimeException("Unexpected exception", (Throwable) jobResult); } else if (jobResult instanceof Long) { - return _volsDao.findById((Long) jobResult); + Long volumeId = (Long) jobResult; + if (newDiskOffering != null) { + _volsDao.updateDiskOffering(volumeId, newDiskOffering.getId()); + } + return _volsDao.findById(volumeId); } } From 16843475ca3e13403123b054b58ad2c2d0cd817a Mon Sep 17 00:00:00 2001 From: Henrique Sato Date: Fri, 13 Jun 2025 16:44:10 -0300 Subject: [PATCH 7/7] add IOPS to VmwareStorageMotionStrategy copy command --- .../storage/motion/VmwareStorageMotionStrategy.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java b/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java index b0cacf60a176..0e641cad7785 100644 --- a/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java +++ b/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java @@ -27,6 +27,7 @@ import javax.inject.Inject; import com.cloud.agent.api.to.DiskTO; +import com.cloud.storage.DiskOfferingVO; import com.cloud.storage.Storage; import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult; import org.apache.cloudstack.engine.subsystem.api.storage.DataMotionStrategy; @@ -251,6 +252,17 @@ public void copyAsync(DataObject srcData, DataObject destData, Host destHost, As , sourcePool , targetPool , hostIdForVmAndHostGuidInTargetCluster.second(), ((VolumeObjectTO) srcData.getTO()).getChainInfo()); + + VolumeInfo volumeInfo = (VolumeInfo) srcData; + if (volumeInfo.getpayload() instanceof DiskOfferingVO) { + DiskOfferingVO offering = (DiskOfferingVO) volumeInfo.getpayload(); + Long newIops = null; + if (offering.getIopsReadRate() != null && offering.getIopsWriteRate() != null) { + newIops = offering.getIopsReadRate() + offering.getIopsWriteRate(); + } + cmd.setNewIops(newIops); + } + if (sourcePool.getParent() != 0) { cmd.setContextParam(DiskTO.PROTOCOL_TYPE, Storage.StoragePoolType.DatastoreCluster.toString()); }