This is an automated email from the ASF dual-hosted git repository.

vishesh pushed a commit to branch fixup-limits-exceed-scaleio
in repository https://gitbox.apache.org/repos/asf/cloudstack.git

commit 33ed96ca673da1027b644c46a3f8b042e7114a23
Author: Vishesh <vishes...@gmail.com>
AuthorDate: Thu Apr 25 17:45:48 2024 +0530

    Fix exceeding of resource limits with powerflex
---
 .../java/com/cloud/user/ResourceLimitService.java  |  2 +
 .../service/VolumeOrchestrationService.java        |  3 +-
 .../subsystem/api/storage/DataStoreDriver.java     |  1 +
 .../api/storage/PrimaryDataStoreDriver.java        | 16 ++++++++
 .../com/cloud/vm/VirtualMachineManagerImpl.java    |  3 +-
 .../engine/orchestration/VolumeOrchestrator.java   | 39 ++++++++++++++++--
 .../driver/ScaleIOPrimaryDataStoreDriver.java      | 10 +++++
 .../driver/ScaleIOPrimaryDataStoreDriverTest.java  | 33 +++++++++++++++
 .../resourcelimit/ResourceLimitManagerImpl.java    | 13 ++++++
 .../com/cloud/storage/VolumeApiServiceImpl.java    | 18 ++++++++
 .../main/java/com/cloud/vm/UserVmManagerImpl.java  | 48 +++++++++++++++++-----
 .../java/com/cloud/vm/UserVmManagerImplTest.java   | 33 +++++++++++++++
 .../cloud/vpc/MockResourceLimitManagerImpl.java    |  6 +++
 13 files changed, 209 insertions(+), 16 deletions(-)

diff --git a/api/src/main/java/com/cloud/user/ResourceLimitService.java 
b/api/src/main/java/com/cloud/user/ResourceLimitService.java
index d0aa9f69f84..ba19719ea8d 100644
--- a/api/src/main/java/com/cloud/user/ResourceLimitService.java
+++ b/api/src/main/java/com/cloud/user/ResourceLimitService.java
@@ -243,6 +243,8 @@ public interface ResourceLimitService {
     void checkVolumeResourceLimitForDiskOfferingChange(Account owner, Boolean 
display, Long currentSize, Long newSize,
             DiskOffering currentOffering, DiskOffering newOffering) throws 
ResourceAllocationException;
 
+    void checkPrimaryStorageResourceLimit(Account owner, Boolean display, Long 
size, DiskOffering diskOffering) throws ResourceAllocationException;
+
     void incrementVolumeResourceCount(long accountId, Boolean display, Long 
size, DiskOffering diskOffering);
     void decrementVolumeResourceCount(long accountId, Boolean display, Long 
size, DiskOffering diskOffering);
 
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 c3525466ce1..7950dda4d68 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
@@ -22,6 +22,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+import com.cloud.exception.ResourceAllocationException;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataObject;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
 import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
@@ -126,7 +127,7 @@ public interface VolumeOrchestrationService {
 
     void prepareForMigration(VirtualMachineProfile vm, DeployDestination dest);
 
-    void prepare(VirtualMachineProfile vm, DeployDestination dest) throws 
StorageUnavailableException, InsufficientStorageCapacityException, 
ConcurrentOperationException, StorageAccessException;
+    void prepare(VirtualMachineProfile vm, DeployDestination dest) throws 
StorageUnavailableException, InsufficientStorageCapacityException, 
ConcurrentOperationException, StorageAccessException, 
ResourceAllocationException;
 
     boolean canVmRestartOnAnotherServer(long vmId);
 
diff --git 
a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreDriver.java
 
b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreDriver.java
index b197afad863..6ddf4a2e5e0 100644
--- 
a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreDriver.java
+++ 
b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreDriver.java
@@ -45,4 +45,5 @@ public interface DataStoreDriver {
     boolean canCopy(DataObject srcData, DataObject destData);
 
     void resize(DataObject data, AsyncCompletionCallback<CreateCmdResult> 
callback);
+
 }
diff --git 
a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreDriver.java
 
b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreDriver.java
index 2c7d3c60278..dbe67e6cca5 100644
--- 
a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreDriver.java
+++ 
b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreDriver.java
@@ -157,4 +157,20 @@ public interface PrimaryDataStoreDriver extends 
DataStoreDriver {
     default boolean zoneWideVolumesAvailableWithoutClusterMotion() {
         return false;
     }
+
+    /**
+     * This method returns the actual size required on the pool for a volume.
+     *
+     * @param volumeSize
+     *         Size of volume to be created on the store
+     * @param templateSize
+     *         Size of template, if any, which will be used to create the 
volume
+     * @param isEncryptionRequired
+     *         true if volume is encrypted
+     *
+     * @return the size required on the pool for the volume
+     */
+    default long getVolumeSizeRequiredOnPool(long volumeSize, Long 
templateSize, boolean isEncryptionRequired) {
+        return volumeSize;
+    }
 }
diff --git 
a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
 
b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
index 824b9f5f45d..0613b9586ff 100755
--- 
a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
+++ 
b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
@@ -52,6 +52,7 @@ import javax.persistence.EntityExistsException;
 import com.cloud.configuration.Resource;
 import com.cloud.domain.Domain;
 import com.cloud.domain.dao.DomainDao;
+import com.cloud.exception.ResourceAllocationException;
 import com.cloud.network.vpc.VpcVO;
 import com.cloud.network.vpc.dao.VpcDao;
 import com.cloud.user.dao.AccountDao;
@@ -1403,7 +1404,7 @@ public class VirtualMachineManagerImpl extends 
ManagerBase implements VirtualMac
                             logger.warn("unexpected 
InsufficientCapacityException : {}", e.getScope().getName(), e);
                         }
                     }
-                } catch (ExecutionException | NoTransitionException e) {
+                } catch (ExecutionException | NoTransitionException | 
ResourceAllocationException e) {
                     logger.error("Failed to start instance {}", vm, e);
                     throw new AgentUnavailableException("Unable to start 
instance due to " + e.getMessage(), destHostId, e);
                 } catch (final StorageAccessException e) {
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 ff9da6bccc2..54f634e701d 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
@@ -38,6 +38,10 @@ import java.util.stream.Collectors;
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.storage.VMTemplateVO;
+import com.cloud.storage.dao.VMTemplateDao;
+import com.cloud.user.AccountManager;
 import org.apache.cloudstack.api.ApiCommandResourceType;
 import org.apache.cloudstack.api.ApiConstants.IoDriverPolicy;
 import org.apache.cloudstack.api.command.admin.vm.MigrateVMCmd;
@@ -180,6 +184,8 @@ public class VolumeOrchestrator extends ManagerBase 
implements VolumeOrchestrati
     }
 
 
+    @Inject
+    private AccountManager _accountMgr;
     @Inject
     EntityManager _entityMgr;
     @Inject
@@ -195,6 +201,8 @@ public class VolumeOrchestrator extends ManagerBase 
implements VolumeOrchestrati
     @Inject
     protected VolumeDao _volumeDao;
     @Inject
+    protected VMTemplateDao _templateDao;
+    @Inject
     protected SnapshotDao _snapshotDao;
     @Inject
     protected SnapshotDataStoreDao _snapshotDataStoreDao;
@@ -1677,7 +1685,7 @@ public class VolumeOrchestrator extends ManagerBase 
implements VolumeOrchestrati
         }
     }
 
-    private Pair<VolumeVO, DataStore> recreateVolume(VolumeVO vol, 
VirtualMachineProfile vm, DeployDestination dest) throws 
StorageUnavailableException, StorageAccessException {
+    private Pair<VolumeVO, DataStore> recreateVolume(VolumeVO vol, 
VirtualMachineProfile vm, DeployDestination dest) throws 
StorageUnavailableException, StorageAccessException, 
ResourceAllocationException {
         String volToString = getReflectOnlySelectedFields(vol);
 
         VolumeVO newVol;
@@ -1710,6 +1718,7 @@ public class VolumeOrchestrator extends ManagerBase 
implements VolumeOrchestrati
             }
             logger.debug("Created new volume [{}] from old volume [{}].", 
newVolToString, volToString);
         }
+        updateVolumeSize(destPool, newVol);
         VolumeInfo volume = volFactory.getVolume(newVol.getId(), destPool);
         Long templateId = newVol.getTemplateId();
         for (int i = 0; i < 2; i++) {
@@ -1841,8 +1850,29 @@ public class VolumeOrchestrator extends ManagerBase 
implements VolumeOrchestrati
         }
     }
 
+    /**
+     * This method checks if size of volume on the data store would be 
different.
+     * If it's different it verifies the resource limits and updates the 
volume's size
+     */
+    protected void updateVolumeSize(DataStore store, VolumeVO vol) throws 
ResourceAllocationException {
+        if (store.getDriver() instanceof PrimaryDataStoreDriver) {
+            VMTemplateVO template = vol.getTemplateId() != null ? 
_templateDao.findById(vol.getTemplateId()) : null;
+            PrimaryDataStoreDriver driver = (PrimaryDataStoreDriver) 
store.getDriver();
+            long newSize = driver.getVolumeSizeRequiredOnPool(vol.getSize(), 
template == null ? null : template.getSize(), vol.getPassphraseId() != null);
+            if (newSize != vol.getSize()) {
+                if (newSize > vol.getSize()) {
+                    _resourceLimitMgr.checkPrimaryStorageResourceLimit(
+                            
_accountMgr.getActiveAccountById(vol.getAccountId()), vol.isDisplay(), newSize 
- vol.getSize(),
+                            
diskOfferingDao.findByIdIncludingRemoved(vol.getDiskOfferingId()));
+                }
+                vol.setSize(newSize);
+                _volsDao.persist(vol);
+            }
+        }
+    }
+
     @Override
-    public void prepare(VirtualMachineProfile vm, DeployDestination dest) 
throws StorageUnavailableException, InsufficientStorageCapacityException, 
ConcurrentOperationException, StorageAccessException {
+    public void prepare(VirtualMachineProfile vm, DeployDestination dest) 
throws StorageUnavailableException, InsufficientStorageCapacityException, 
ConcurrentOperationException, StorageAccessException, 
ResourceAllocationException {
         if (dest == null) {
             String msg = String.format("Unable to prepare volumes for the VM 
[%s] because DeployDestination is null.", vm.getVirtualMachine());
             logger.error(msg);
@@ -1865,7 +1895,7 @@ public class VolumeOrchestrator extends ManagerBase 
implements VolumeOrchestrati
 
                 String volToString = getReflectOnlySelectedFields(vol);
 
-                store = 
(PrimaryDataStore)dataStoreMgr.getDataStore(task.pool.getId(), 
DataStoreRole.Primary);
+                store = (PrimaryDataStore) 
dataStoreMgr.getDataStore(task.pool.getId(), DataStoreRole.Primary);
 
                 // For zone-wide managed storage, it is possible that the VM 
can be started in another
                 // cluster. In that case, make sure that the volume is in the 
right access group.
@@ -1876,6 +1906,8 @@ public class VolumeOrchestrator extends ManagerBase 
implements VolumeOrchestrati
                     long lastClusterId = lastHost == null || 
lastHost.getClusterId() == null ? -1 : lastHost.getClusterId();
                     long clusterId = host == null || host.getClusterId() == 
null ? -1 : host.getClusterId();
 
+                    updateVolumeSize(store, (VolumeVO) vol);
+
                     if (lastClusterId != clusterId) {
                         if (lastHost != null) {
                             
storageMgr.removeStoragePoolFromCluster(lastHost.getId(), vol.get_iScsiName(), 
store);
@@ -1895,6 +1927,7 @@ public class VolumeOrchestrator extends ManagerBase 
implements VolumeOrchestrati
                 }
             } else if (task.type == VolumeTaskType.MIGRATE) {
                 store = (PrimaryDataStore) 
dataStoreMgr.getDataStore(task.pool.getId(), DataStoreRole.Primary);
+                updateVolumeSize(store, task.volume);
                 vol = migrateVolume(task.volume, store);
             } else if (task.type == VolumeTaskType.RECREATE) {
                 Pair<VolumeVO, DataStore> result = recreateVolume(task.volume, 
vm, dest);
diff --git 
a/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java
 
b/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java
index 31308a429da..817b263e9b4 100644
--- 
a/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java
+++ 
b/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java
@@ -1340,6 +1340,16 @@ public class ScaleIOPrimaryDataStoreDriver implements 
PrimaryDataStoreDriver {
         }
     }
 
+    @Override
+    public long getVolumeSizeRequiredOnPool(long volumeSize, Long 
templateSize, boolean isEncryptionRequired) {
+        long newSizeInGB = volumeSize / (1024 * 1024 * 1024);
+        if (templateSize != null && isEncryptionRequired && 
needsExpansionForEncryptionHeader(templateSize, volumeSize)) {
+            newSizeInGB = (volumeSize + (1<<30)) / (1024 * 1024 * 1024);
+        }
+        long newSizeIn8gbBoundary = (long) (Math.ceil(newSizeInGB / 8.0) * 
8.0);
+        return newSizeIn8gbBoundary * (1024 * 1024 * 1024);
+    }
+
     @Override
     public void handleQualityOfServiceForVolumeMigration(VolumeInfo 
volumeInfo, QualityOfServiceState qualityOfServiceState) {
     }
diff --git 
a/plugins/storage/volume/scaleio/src/test/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriverTest.java
 
b/plugins/storage/volume/scaleio/src/test/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriverTest.java
index de5e4d44c02..b8a799a4e07 100644
--- 
a/plugins/storage/volume/scaleio/src/test/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriverTest.java
+++ 
b/plugins/storage/volume/scaleio/src/test/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriverTest.java
@@ -542,4 +542,37 @@ public class ScaleIOPrimaryDataStoreDriverTest {
 
         Assert.assertEquals(false, answer.getResult());
     }
+
+    @Test
+    public void testGetVolumeSizeRequiredOnPool() {
+        Assert.assertEquals(16L * (1024 * 1024 * 1024),
+                scaleIOPrimaryDataStoreDriver.getVolumeSizeRequiredOnPool(
+                        10L * (1024 * 1024 * 1024),
+                        null,
+                        true));
+
+        Assert.assertEquals(16L * (1024 * 1024 * 1024),
+                scaleIOPrimaryDataStoreDriver.getVolumeSizeRequiredOnPool(
+                        10L * (1024 * 1024 * 1024),
+                        null,
+                        false));
+
+        Assert.assertEquals(16L * (1024 * 1024 * 1024),
+                scaleIOPrimaryDataStoreDriver.getVolumeSizeRequiredOnPool(
+                        16L * (1024 * 1024 * 1024),
+                        null,
+                        false));
+
+        Assert.assertEquals(16L * (1024 * 1024 * 1024),
+                scaleIOPrimaryDataStoreDriver.getVolumeSizeRequiredOnPool(
+                        16L * (1024 * 1024 * 1024),
+                        16L * (1024 * 1024 * 1024),
+                        false));
+
+        Assert.assertEquals(24L * (1024 * 1024 * 1024),
+                scaleIOPrimaryDataStoreDriver.getVolumeSizeRequiredOnPool(
+                        16L * (1024 * 1024 * 1024),
+                        16L * (1024 * 1024 * 1024),
+                        true));
+    }
 }
diff --git 
a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java 
b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java
index f040b526a6e..4455c472113 100644
--- a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java
+++ b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java
@@ -1641,6 +1641,19 @@ public class ResourceLimitManagerImpl extends 
ManagerBase implements ResourceLim
         }
     }
 
+    @Override
+    public void checkPrimaryStorageResourceLimit(Account owner, Boolean 
display, Long size, DiskOffering diskOffering) throws 
ResourceAllocationException {
+        List<String> tags = 
getResourceLimitStorageTagsForResourceCountOperation(display, diskOffering);
+        if (CollectionUtils.isEmpty(tags)) {
+            return;
+        }
+        if (size != null) {
+            for (String tag : tags) {
+                checkResourceLimitWithTag(owner, ResourceType.primary_storage, 
tag, size);
+            }
+        }
+    }
+
     @Override
     public void checkVolumeResourceLimitForDiskOfferingChange(Account owner, 
Boolean display, Long currentSize, Long newSize,
             DiskOffering currentOffering, DiskOffering newOffering
diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java 
b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
index e79e9b9941d..ac6cdea7e0d 100644
--- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
+++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
@@ -1240,6 +1240,16 @@ public class VolumeApiServiceImpl extends ManagerBase 
implements VolumeApiServic
         }
 
         long currentSize = volume.getSize();
+        VolumeInfo volInfo = volFactory.getVolume(volume.getId());
+        boolean isEncryptionRequired = volume.getPassphraseId() != null;
+        if (newDiskOffering != null) {
+            isEncryptionRequired = newDiskOffering.getEncrypt();
+        }
+
+        DataStore dataStore = volInfo.getDataStore();
+        if (dataStore != null && dataStore.getDriver() instanceof 
PrimaryDataStoreDriver) {
+            newSize = ((PrimaryDataStoreDriver) 
dataStore.getDriver()).getVolumeSizeRequiredOnPool(newSize, null, 
isEncryptionRequired);
+        }
         validateVolumeResizeWithSize(volume, currentSize, newSize, shrinkOk, 
diskOffering, newDiskOffering);
 
         // Note: The storage plug-in in question should perform validation on 
the IOPS to check if a sufficient number of IOPS is available to perform
@@ -1982,6 +1992,14 @@ public class VolumeApiServiceImpl extends ManagerBase 
implements VolumeApiServic
         newMaxIops = updateNewMaxIops[0];
         newHypervisorSnapshotReserve = updateNewHypervisorSnapshotReserve[0];
         long currentSize = volume.getSize();
+
+        VolumeInfo volInfo = volFactory.getVolume(volume.getId());
+
+        DataStore dataStore = volInfo.getDataStore();
+        if (dataStore != null && dataStore.getDriver() instanceof 
PrimaryDataStoreDriver) {
+            newSize = ((PrimaryDataStoreDriver) 
dataStore.getDriver()).getVolumeSizeRequiredOnPool(newSize, null, 
newDiskOffering.getEncrypt());
+        }
+
         validateVolumeResizeWithSize(volume, currentSize, newSize, shrinkOk, 
existingDiskOffering, newDiskOffering);
 
         /* If this volume has never been beyond allocated state, short circuit 
everything and simply update the database. */
diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java 
b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
index c924a124fa1..1bb1f22b9a5 100644
--- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
+++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
@@ -7980,7 +7980,7 @@ public class UserVmManagerImpl extends ManagerBase 
implements UserVmManager, Vir
         }
 
         for (VolumeVO root : rootVols) {
-            if ( !Volume.State.Allocated.equals(root.getState()) || 
newTemplateId != null ) {
+            if ( !Volume.State.Allocated.equals(root.getState()) || 
newTemplateId != null || diskOffering != null) {
                 _volumeService.validateDestroyVolume(root, caller, 
Volume.State.Allocated.equals(root.getState()) || expunge, false);
                 final UserVmVO userVm = vm;
                 Pair<UserVmVO, Volume> vmAndNewVol = Transaction.execute(new 
TransactionCallbackWithException<Pair<UserVmVO, Volume>, 
CloudRuntimeException>() {
@@ -8185,7 +8185,7 @@ public class UserVmManagerImpl extends ManagerBase 
implements UserVmManager, Vir
      * @param template template
      * @throws InvalidParameterValueException if restore is not possible
      */
-    private void checkRestoreVmFromTemplate(UserVmVO vm, VMTemplateVO 
template, List<VolumeVO> volumes, DiskOffering newDiskOffering, 
Map<String,String> details) throws ResourceAllocationException {
+    private void checkRestoreVmFromTemplate(UserVmVO vm, VMTemplateVO 
template, List<VolumeVO> rootVolumes, DiskOffering newDiskOffering, 
Map<String,String> details) throws ResourceAllocationException {
         TemplateDataStoreVO tmplStore;
         if (!template.isDirectDownload()) {
             tmplStore = 
_templateStoreDao.findByTemplateZoneReady(template.getId(), 
vm.getDataCenterId());
@@ -8206,19 +8206,45 @@ public class UserVmManagerImpl extends ManagerBase 
implements UserVmManager, Vir
             _resourceLimitMgr.checkVmResourceLimitsForTemplateChange(owner, 
vm.isDisplay(), serviceOffering, currentTemplate, template);
         }
 
-        Long newSize = newDiskOffering != null ? newDiskOffering.getDiskSize() 
: null;
-        if (MapUtils.isNotEmpty(details) && 
StringUtils.isNumeric(details.get(VmDetailConstants.ROOT_DISK_SIZE))) {
-            newSize = 
Long.parseLong(details.get(VmDetailConstants.ROOT_DISK_SIZE)) * GiB_TO_BYTES;
+        Long newSize = getRootVolumeSizeForVmTemplateRestore(vm, template, 
newDiskOffering, details);
+        for (Volume vol : rootVolumes) {
+            if (newSize == null) {
+                newSize = vol.getSize();
+            }
+            if (newDiskOffering != null || !vol.getSize().equals(newSize)) {
+                DiskOffering currentOffering = 
_diskOfferingDao.findById(vol.getDiskOfferingId());
+                
_resourceLimitMgr.checkVolumeResourceLimitForDiskOfferingChange(owner, 
vol.isDisplay(),
+                        vol.getSize(), newSize, currentOffering, 
newDiskOffering);
+            }
         }
-        if (newDiskOffering != null || newSize != null) {
-            for (Volume vol : volumes) {
-                if (newDiskOffering != null || !vol.getSize().equals(newSize)) 
{
-                    DiskOffering currentOffering = 
_diskOfferingDao.findById(vol.getDiskOfferingId());
-                    
_resourceLimitMgr.checkVolumeResourceLimitForDiskOfferingChange(owner, 
vol.isDisplay(),
-                            vol.getSize(), newSize, currentOffering, 
newDiskOffering);
+    }
+
+    protected Long getRootVolumeSizeForVmTemplateRestore(
+            UserVmVO userVm, VMTemplateVO template, DiskOffering diskOffering, 
Map<String, String> details
+    ) {
+        Long size = null;
+        if (template != null && template.getSize() != null) {
+            UserVmDetailVO vmRootDiskSizeDetail = 
userVmDetailsDao.findDetail(userVm.getId(), VmDetailConstants.ROOT_DISK_SIZE);
+            if (vmRootDiskSizeDetail == null) {
+                size = template.getSize();
+            } else {
+                long rootDiskSize = 
Long.parseLong(vmRootDiskSizeDetail.getValue()) * GiB_TO_BYTES;
+                if (template.getSize() >= rootDiskSize) {
+                    size = template.getSize();
+                } else {
+                    size = rootDiskSize;
                 }
             }
         }
+
+        if (diskOffering != null && !diskOffering.isCustomized()) {
+            size = diskOffering.getDiskSize();
+        }
+
+        if (MapUtils.isNotEmpty(details) && 
StringUtils.isNumeric(details.get(VmDetailConstants.ROOT_DISK_SIZE))) {
+            size = 
Long.parseLong(details.get(VmDetailConstants.ROOT_DISK_SIZE)) * GiB_TO_BYTES;
+        }
+        return size;
     }
 
     private void handleManagedStorage(UserVmVO vm, VolumeVO root) {
diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java 
b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
index c464af6385b..f97334d87c1 100644
--- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
+++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
@@ -1563,4 +1563,37 @@ public class UserVmManagerImplTest {
             Assert.fail(e.getMessage());
         }
     }
+
+    @Test
+    public void testGetRootVolumeSizeForVmTemplateRestore() {
+        VMTemplateVO template = Mockito.mock(VMTemplateVO.class);
+        Mockito.when(template.getSize()).thenReturn(10L * GiB_TO_BYTES);
+        UserVmVO userVm = Mockito.mock(UserVmVO.class);
+        Mockito.when(userVm.getId()).thenReturn(1L);
+        DiskOffering diskOffering = Mockito.mock(DiskOffering.class);
+        Mockito.when(diskOffering.isCustomized()).thenReturn(false);
+        Mockito.when(diskOffering.getDiskSize()).thenReturn(8L * GiB_TO_BYTES);
+        Map<String, String> details = new HashMap<>();
+        details.put(VmDetailConstants.ROOT_DISK_SIZE, "16");
+        UserVmDetailVO vmRootDiskSizeDetail = 
Mockito.mock(UserVmDetailVO.class);
+        Mockito.when(vmRootDiskSizeDetail.getValue()).thenReturn("20");
+        Mockito.when(userVmDetailsDao.findDetail(1L, 
VmDetailConstants.ROOT_DISK_SIZE)).thenReturn(vmRootDiskSizeDetail);
+        Long actualSize = 
userVmManagerImpl.getRootVolumeSizeForVmTemplateRestore(userVm, template, 
diskOffering, details);
+        Assert.assertEquals(16 * GiB_TO_BYTES, actualSize.longValue());
+    }
+
+    @Test
+    public void 
testGetRootVolumeSizeForVmTemplateRestoreNullDiskOfferingAndEmptyDetails() {
+        VMTemplateVO template = Mockito.mock(VMTemplateVO.class);
+        Mockito.when(template.getSize()).thenReturn(10L * GiB_TO_BYTES);
+        UserVmVO userVm = Mockito.mock(UserVmVO.class);
+        Mockito.when(userVm.getId()).thenReturn(1L);
+        DiskOffering diskOffering = null;
+        Map<String, String> details = new HashMap<>();
+        UserVmDetailVO vmRootDiskSizeDetail = 
Mockito.mock(UserVmDetailVO.class);
+        Mockito.when(vmRootDiskSizeDetail.getValue()).thenReturn("20");
+        Mockito.when(userVmDetailsDao.findDetail(1L, 
VmDetailConstants.ROOT_DISK_SIZE)).thenReturn(vmRootDiskSizeDetail);
+        Long actualSize = 
userVmManagerImpl.getRootVolumeSizeForVmTemplateRestore(userVm, template, 
diskOffering, details);
+        Assert.assertEquals(20 * GiB_TO_BYTES, actualSize.longValue());
+    }
 }
diff --git 
a/server/src/test/java/com/cloud/vpc/MockResourceLimitManagerImpl.java 
b/server/src/test/java/com/cloud/vpc/MockResourceLimitManagerImpl.java
index 74e2a7e6545..3f3220d0934 100644
--- a/server/src/test/java/com/cloud/vpc/MockResourceLimitManagerImpl.java
+++ b/server/src/test/java/com/cloud/vpc/MockResourceLimitManagerImpl.java
@@ -278,6 +278,12 @@ public class MockResourceLimitManagerImpl extends 
ManagerBase implements Resourc
 
     }
 
+    @Override
+    public void checkPrimaryStorageResourceLimit(Account owner, Boolean 
display, Long size,
+            DiskOffering diskOffering) {
+
+    }
+
     @Override
     public void incrementVolumeResourceCount(long accountId, Boolean display, 
Long size, DiskOffering diskOffering) {
 

Reply via email to