[GitHub] rafaelweingartner commented on a change in pull request #2499: Updates to capacity management

[GitBox <gi...@apache.org>]

rafaelweingartner commented on a change in pull request #2499: Updates to capacity management
URL: https://github.com/apache/cloudstack/pull/2499#discussion_r179507086
 
 

 ##########
 File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
 ##########
 @@ -1833,67 +1839,88 @@ public boolean storagePoolHasEnoughSpace(List<Volume> volumes, StoragePool pool,
                     allocatedSizeWithTemplate = _capacityMgr.getAllocatedPoolCapacity(poolVO, tmpl);
                 }
             }
-            // A ready state volume is already allocated in a pool. so the asking size is zero for it.
-            // In case the volume is moving across pools or is not ready yet, the asking size has to be computed
+
             if (s_logger.isDebugEnabled()) {
-                s_logger.debug("pool id for the volume with id: " + volumeVO.getId() + " is " + volumeVO.getPoolId());
+                s_logger.debug("Pool ID for the volume with ID " + volumeVO.getId() + " is " + volumeVO.getPoolId());
             }
+
+            // A ready-state volume is already allocated in a pool, so the asking size is zero for it.
+            // In case the volume is moving across pools or is not ready yet, the asking size has to be computed.
             if ((volumeVO.getState() != Volume.State.Ready) || (volumeVO.getPoolId() != pool.getId())) {
-                if (ScopeType.ZONE.equals(poolVO.getScope()) && volumeVO.getTemplateId() != null) {
+                totalAskingSize += getDataObjectSizeIncludingHypervisorSnapshotReserve(volumeVO, poolVO);
+
+                if (poolVO.isManaged() && volumeVO.getTemplateId() != null) {
                     VMTemplateVO tmpl = _templateDao.findByIdIncludingRemoved(volumeVO.getTemplateId());
 
                     if (tmpl != null && !ImageFormat.ISO.equals(tmpl.getFormat())) {
-                        // Storage plug-ins for zone-wide primary storage can be designed in such a way as to store a template on the
-                        // primary storage once and make use of it in different clusters (via cloning).
+                        // Storage plug-ins for managed storage can be designed in such a way as to store a template on the
+                        // primary storage once and make use of it via storage-side cloning.
                         // This next call leads to CloudStack asking how many more bytes it will need for the template (if the template is
                         // already stored on the primary storage, then the answer is 0).
 
-                        if (clusterId != null && _clusterDao.getSupportsResigning(clusterId)) {
-                            totalAskingSize += getBytesRequiredForTemplate(tmpl, pool);
+                        if (clusterId != null) {
 
 Review comment:
   Would you mind extracting the block of this IF condition to a method? This would allow proper documentation an unit tests.
   
   If the resigning is not supported this method can return 0 as the value to be added to the `totalAskingSize` variable.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services