Address reviews

Author: hsato03

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

@@ -36,6 +36,11 @@ public class MigrateVolumeCommand extends Command {
     String attachedVmName;
     Volume.Type volumeType;
     String hostGuidInTargetCluster;
+<<<<<<< HEAD
+=======
+    Long newMaxIops;
+    Long newMinIops;
+>>>>>>> cb43664102 (Address reviews)
 
     private DataTO srcData;
     private DataTO destData;
@@ -146,6 +151,25 @@ public int getWaitInMillSeconds() {
 
     public String getChainInfo() { return chainInfo; }
 
+<<<<<<< HEAD
+=======
+    public Long getNewMinIops() {
+        return newMinIops;
+    }
+
+    public void setNewMinIops(Long newMinIops) {
+        this.newMinIops = newMinIops;
+    }
+
+    public Long getNewMaxIops() {
+        return newMaxIops;
+    }
+
+    public void setNewMaxIops(Long newMaxIops) {
+        this.newMaxIops = newMaxIops;
+    }
+
+>>>>>>> cb43664102 (Address reviews)
     @Override
     public boolean isReconcile() {
         return true;

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

@@ -546,6 +546,10 @@ 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());
+<<<<<<< HEAD
+=======
+
+>>>>>>> cb43664102 (Address reviews)
         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

@@ -5191,6 +5191,43 @@ private Answer execute(MigrateVolumeCommand cmd) {
         }
     }
 
+<<<<<<< HEAD
+=======
+    /**
+     * 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 = -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 {
+            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);
+        }
+    }
+
+>>>>>>> cb43664102 (Address reviews)
     private Pair<VirtualDisk, String> getVirtualDiskInfo(VirtualMachineMO vmMo, String srcDiskName) throws Exception {
         Pair<VirtualDisk, String> deviceInfo = vmMo.getDiskDevice(srcDiskName);
         if (deviceInfo == null) {

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

@@ -38,6 +38,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;
@@ -251,6 +252,28 @@ public void copyAsync(DataObject srcData, DataObject destData, Host destHost, As
                 , sourcePool
                 , targetPool
                 , hostIdForVmAndHostGuidInTargetCluster.second(), ((VolumeObjectTO) srcData.getTO()).getChainInfo());
+<<<<<<< HEAD
+=======
+
+        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.setNewMinIops(minIops);
+            cmd.setNewMaxIops(maxIops);
+        }
+
+>>>>>>> cb43664102 (Address reviews)
         if (sourcePool.getParent() != 0) {
             cmd.setContextParam(DiskTO.PROTOCOL_TYPE, Storage.StoragePoolType.DatastoreCluster.toString());
         }

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

@@ -1266,8 +1266,25 @@ public VolumeVO resizeVolume(ResizeVolumeCmd cmd) throws ResourceAllocationExcep
 
                 validateIops(newMinIops, newMaxIops, volume.getPoolType());
             } else {
+<<<<<<< HEAD
                 newMinIops = newDiskOffering.getMinIops();
                 newMaxIops = newDiskOffering.getMaxIops();
+=======
+                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);
+                }
+>>>>>>> cb43664102 (Address reviews)
             }
 
             // 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,10 +1352,14 @@ public VolumeVO resizeVolume(ResizeVolumeCmd cmd) throws ResourceAllocationExcep
             volumeMigrateRequired = true;
         }
 
+<<<<<<< HEAD
         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());
+>>>>>>> cb43664102 (Address reviews)
         if (!volumeMigrateRequired && !volumeResizeRequired && newDiskOffering != null) {
             _volsDao.updateDiskOffering(volume.getId(), newDiskOffering.getId());
             volume = _volsDao.findById(volume.getId());
@@ -1403,7 +1424,18 @@ public VolumeVO resizeVolume(ResizeVolumeCmd cmd) throws ResourceAllocationExcep
                     } else if (jobResult instanceof Throwable) {
                         throw new RuntimeException("Unexpected exception", (Throwable) jobResult);
                     } else if (jobResult instanceof Long) {
+<<<<<<< HEAD
                         return _volsDao.findById((Long) jobResult);
+=======
+                        Long volumeId = (Long) jobResult;
+                        if (newDiskOffering != null) {
+                            _volsDao.updateDiskOffering(volumeId, newDiskOffering.getId());
+                        }
+                        volume.setMinIops(newMinIops);
+                        volume.setMinIops(newMaxIops);
+                        _volsDao.update(volumeId, volume);
+                        return _volsDao.findById(volumeId);
+>>>>>>> cb43664102 (Address reviews)
                     }
                 }
 

setup/dev/demo.cfg

@@ -0,0 +1,21 @@
+{
+    "dbSvr": {
+        "dbSvr": "192.168.104.160",
+        "passwd": "password",
+        "db": "cloud",
+        "port": 3306,
+        "user": "cloud"
+    },
+    "logger": {
+         "LogFolderPath": "/tmp/"
+    },
+    "mgtSvr": [
+        {
+            "mgtSvrIp": "192.168.103.150",
+            "port": 8096,
+            "user": "root",
+            "passwd": "password",
+            "hypervisor": "kvm"
+        }
+    ]
+}