Address reviews

Author: hsato03

core/src/main/java/com/cloud/agent/api/storage/MigrateVolumeCommand.java

@@ -36,7 +36,8 @@ public class MigrateVolumeCommand extends Command {
     String attachedVmName;
     Volume.Type volumeType;
     String hostGuidInTargetCluster;
-    Long newIops;
+    Long newMaxIops;
+    Long newMinIops;
 
     private DataTO srcData;
     private DataTO destData;
@@ -147,12 +148,20 @@ public int getWaitInMillSeconds() {
 
     public String getChainInfo() { return chainInfo; }
 
-    public void setNewIops(Long newIops) {
-        this.newIops = newIops;
+    public Long getNewMinIops() {
+        return newMinIops;
     }
 
-    public Long getNewIops() {
-        return newIops;
+    public void setNewMinIops(Long newMinIops) {
+        this.newMinIops = newMinIops;
+    }
+
+    public Long getNewMaxIops() {
+        return newMaxIops;
+    }
+
+    public void setNewMaxIops(Long newMaxIops) {
+        this.newMaxIops = newMaxIops;
     }
 
     @Override

engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java

@@ -74,7 +74,6 @@
 import com.cloud.storage.ScopeType;
 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;
@@ -548,15 +547,6 @@ protected Answer migrateVolumeToPool(DataObject srcData, DataObject destData) {
         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());
         }

plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java

@@ -5217,7 +5217,14 @@ 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();
+        Long newIops = -1L;
+        Long newMinIops = cmd.getNewMinIops();
+        Long newMaxIops = cmd.getNewMaxIops();
+
+        if (ObjectUtils.allNotNull(newMinIops, newMaxIops) && newMinIops > 0 && newMaxIops > 0) {
+            newIops = newMinIops + newMaxIops;
+        }
+
         VirtualDisk disk = vmMo.getDiskDevice(volumePath, true, true).first();
 
         try {

plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java

@@ -39,6 +39,7 @@
 import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
 import org.apache.cloudstack.storage.to.VolumeObjectTO;
 import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang3.ObjectUtils;
 import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.LogManager;
 import org.springframework.stereotype.Component;
@@ -253,14 +254,22 @@ public void copyAsync(DataObject srcData, DataObject destData, Host destHost, As
                 , 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();
+        VolumeInfo volume = (VolumeInfo) srcData;
+        if (volume.getpayload() instanceof DiskOfferingVO) {
+            DiskOfferingVO offering = (DiskOfferingVO) volume.getpayload();
+
+            Long offeringIopsReadRate = offering.getIopsReadRate();
+            Long offeringIopsWriteRate = offering.getIopsWriteRate();
+
+            Long minIops = null;
+            Long maxIops = null;
+            if (ObjectUtils.allNotNull(offeringIopsReadRate, offeringIopsWriteRate)) {
+                minIops = Math.min(offeringIopsReadRate, offeringIopsWriteRate);
+                maxIops = Math.max(offeringIopsReadRate, offeringIopsWriteRate);
             }
-            cmd.setNewIops(newIops);
+
+            cmd.setNewMinIops(minIops);
+            cmd.setNewMaxIops(maxIops);
         }
 
         if (sourcePool.getParent() != 0) {

server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java

@@ -1266,8 +1266,20 @@ public VolumeVO resizeVolume(ResizeVolumeCmd cmd) throws ResourceAllocationExcep
 
                 validateIops(newMinIops, newMaxIops, volume.getPoolType());
             } else {
-                newMinIops = newDiskOffering.getMinIops() != null ? newDiskOffering.getMinIops() : newDiskOffering.getIopsReadRate();
-                newMaxIops = newDiskOffering.getMaxIops() != null ? newDiskOffering.getMaxIops() : newDiskOffering.getIopsWriteRate();
+                if (newDiskOffering.getMinIops() != null) {
+                    newMinIops = newDiskOffering.getMinIops();
+                }
+
+                if (newDiskOffering.getMaxIops() != null) {
+                    newMaxIops = newDiskOffering.getMaxIops();
+                }
+
+                Long newDiskOfferingIopsReadRate = newDiskOffering.getIopsReadRate();
+                Long newDiskOfferingIopsWriteRate = newDiskOffering.getIopsWriteRate();
+                if (ObjectUtils.allNull(newMinIops, newMaxIops) && ObjectUtils.allNotNull(newDiskOfferingIopsReadRate, newDiskOfferingIopsWriteRate)) {
+                    newMaxIops = Math.max(newDiskOfferingIopsReadRate, newDiskOfferingIopsWriteRate);
+                    newMinIops = Math.min(newDiskOfferingIopsReadRate, newDiskOfferingIopsWriteRate);
+                }
             }
 
             // 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)
@@ -1335,8 +1347,7 @@ public VolumeVO resizeVolume(ResizeVolumeCmd cmd) throws ResourceAllocationExcep
             volumeMigrateRequired = true;
         }
 
-        boolean volumeResizeRequired = currentSize != newSize || !compareEqualsIncludingNullOrZero(newMaxIops, volume.getMaxIops()) || !compareEqualsIncludingNullOrZero(newMinIops, volume.getMinIops())
-                || !compareEqualsIncludingNullOrZero(newMaxIops, diskOffering.getIopsWriteRate()) || !compareEqualsIncludingNullOrZero(newMinIops, diskOffering.getIopsReadRate());
+        boolean volumeResizeRequired = currentSize != newSize || !compareEqualsIncludingNullOrZero(newMaxIops, volume.getMaxIops()) || !compareEqualsIncludingNullOrZero(newMinIops, volume.getMinIops());
         if (!volumeMigrateRequired && !volumeResizeRequired && newDiskOffering != null) {
             _volsDao.updateDiskOffering(volume.getId(), newDiskOffering.getId());
             volume = _volsDao.findById(volume.getId());
@@ -1405,6 +1416,9 @@ public VolumeVO resizeVolume(ResizeVolumeCmd cmd) throws ResourceAllocationExcep
                         if (newDiskOffering != null) {
                             _volsDao.updateDiskOffering(volumeId, newDiskOffering.getId());
                         }
+                        volume.setMinIops(newMinIops);
+                        volume.setMinIops(newMaxIops);
+                        _volsDao.update(volumeId, volume);
                         return _volsDao.findById(volumeId);
                     }
                 }