review comments

Author: abh1sar

server/src/main/java/com/cloud/projects/ProjectManagerImpl.java

@@ -604,16 +604,16 @@ public boolean addUserToProject(Long projectId, String username, String email, L
 
             boolean shouldIncrementResourceCount = projectRole != null && Role.Admin == projectRole;
             try (CheckedReservation cr = new CheckedReservation(userAccount, ResourceType.project, shouldIncrementResourceCount ? 1L : 0L, reservationDao, _resourceLimitMgr)) {
-            if (assignUserToProject(project, user.getId(), user.getAccountId(), projectRole,
-                    Optional.ofNullable(role).map(ProjectRole::getId).orElse(null)) != null) {
-                if (shouldIncrementResourceCount) {
-                    _resourceLimitMgr.incrementResourceCount(userAccount.getId(), ResourceType.project);
+                if (assignUserToProject(project, user.getId(), user.getAccountId(), projectRole,
+                        Optional.ofNullable(role).map(ProjectRole::getId).orElse(null)) != null) {
+                    if (shouldIncrementResourceCount) {
+                        _resourceLimitMgr.incrementResourceCount(userAccount.getId(), ResourceType.project);
+                    }
+                    return true;
+                } else {
+                    logger.warn("Failed to add user to project: {}", project);
+                    return false;
                 }
-                return true;
-            } else {
-                logger.warn("Failed to add user to project: {}", project);
-                return false;
-            }
             }
         }
     }
@@ -721,19 +721,16 @@ public void doInTransactionWithoutResult(TransactionStatus status) throws Resour
                         }
 
                         try (CheckedReservation checkedReservation = new CheckedReservation(futureOwnerAccount, ResourceType.project, null, null, 1L, reservationDao, _resourceLimitMgr)) {
-
-                        _resourceLimitMgr.checkResourceLimit(_accountMgr.getAccount(futureOwnerAccount.getId()), ResourceType.project);
-
-                        //unset the role for the old owner
-                        ProjectAccountVO currentOwner = _projectAccountDao.findByProjectIdAccountId(projectId, currentOwnerAccount.getId());
-                        currentOwner.setAccountRole(Role.Regular);
-                        _projectAccountDao.update(currentOwner.getId(), currentOwner);
-                        _resourceLimitMgr.decrementResourceCount(currentOwnerAccount.getId(), ResourceType.project);
-
-                        //set new owner
-                        futureOwner.setAccountRole(Role.Admin);
-                        _projectAccountDao.update(futureOwner.getId(), futureOwner);
-                        _resourceLimitMgr.incrementResourceCount(futureOwnerAccount.getId(), ResourceType.project);
+                            //unset the role for the old owner
+                            ProjectAccountVO currentOwner = _projectAccountDao.findByProjectIdAccountId(projectId, currentOwnerAccount.getId());
+                            currentOwner.setAccountRole(Role.Regular);
+                            _projectAccountDao.update(currentOwner.getId(), currentOwner);
+                            _resourceLimitMgr.decrementResourceCount(currentOwnerAccount.getId(), ResourceType.project);
+
+                            //set new owner
+                            futureOwner.setAccountRole(Role.Admin);
+                            _projectAccountDao.update(futureOwner.getId(), futureOwner);
+                            _resourceLimitMgr.incrementResourceCount(futureOwnerAccount.getId(), ResourceType.project);
                         }
                     } else {
                         logger.trace("Future owner {}is already the owner of the project {}", newOwnerName, project);

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

@@ -360,7 +360,7 @@ private Boolean checkAndUpdateVolumeResourceLimit(VolumeVO volume, VolumeDataSto
             boolean success = true;
             Long currentSize = answer.getVirtualSize() != 0 ? answer.getVirtualSize() : answer.getPhysicalSize();
             Long lastSize = volume.getSize() != null ? volume.getSize() : 0L;
-            if (!checkAndUpdateSecondaryStorageResourceLimit(volume.getAccountId(), volume.getSize(), currentSize)) {
+            if (!checkAndUpdateSecondaryStorageResourceLimit(volume.getAccountId(), lastSize, currentSize)) {
                 volumeDataStore.setDownloadState(VMTemplateStorageResourceAssoc.Status.DOWNLOAD_ERROR);
                 volumeDataStore.setState(State.Failed);
                 volumeDataStore.setErrorString("Storage Limit Reached");

server/src/main/java/com/cloud/storage/download/DownloadListener.java

@@ -280,15 +280,15 @@ private Long getAccountIdForDataObject() {
     }
 
     private Long getSizeFromDB() {
-        Long lastSize = 0L;
+        Long lastSize = null;
         if (DataObjectType.TEMPLATE.equals(object.getType())) {
             TemplateDataStoreVO t = _templateDataStoreDao.findByStoreTemplate(object.getDataStore().getId(), object.getId());
             lastSize = t.getSize();
         } else if (DataObjectType.VOLUME.equals(object.getType())) {
             VolumeVO v = _volumeDao.findById(object.getId());
             lastSize = v.getSize();
         }
-        return lastSize;
+        return lastSize == null ? 0L : lastSize;
     }
 
     private Boolean checkAndUpdateResourceLimits(DownloadAnswer answer) {
@@ -297,6 +297,9 @@ private Boolean checkAndUpdateResourceLimits(DownloadAnswer answer) {
 
         if (currentSize > lastSize) {
             Long accountId = getAccountIdForDataObject();
+            if (accountId == null) {
+                return true;
+            }
             Account account = _accountMgr.getAccount(accountId);
             Long usage = currentSize - lastSize;
             try (CheckedReservation secStorageReservation = new CheckedReservation(account, Resource.ResourceType.secondary_storage, usage, _reservationDao, _resourceLimitMgr)) {

server/src/main/java/com/cloud/template/TemplateManagerImpl.java

@@ -1035,12 +1035,14 @@ public VirtualMachineTemplate copyTemplate(CopyTemplateCmd cmd) throws StorageUn
                     logger.debug("There is Template {} in secondary storage {} in zone {} , don't need to copy", template, dstSecStore, dataCenterVOs.get(destZoneId));
                     continue;
                 }
-                try (CheckedReservation secondaryStorageReservation = new CheckedReservation(templateOwner, ResourceType.secondary_storage, null, null, template.getSize(), reservationDao, _resourceLimitMgr)) {
-                if (!copy(userId, template, srcSecStore, dataCenterVOs.get(destZoneId))) {
-                    failedZones.add(dataCenterVOs.get(destZoneId).getName());
-                    continue;
-                }
-                _resourceLimitMgr.incrementResourceCount(templateOwner.getId(), ResourceType.secondary_storage, template.getSize());
+                if (template.getSize() != null) {
+                    try (CheckedReservation secondaryStorageReservation = new CheckedReservation(templateOwner, ResourceType.secondary_storage, null, null, template.getSize(), reservationDao, _resourceLimitMgr)) {
+                        if (!copy(userId, template, srcSecStore, dataCenterVOs.get(destZoneId))) {
+                            failedZones.add(dataCenterVOs.get(destZoneId).getName());
+                            continue;
+                        }
+                        _resourceLimitMgr.incrementResourceCount(templateOwner.getId(), ResourceType.secondary_storage, template.getSize());
+                    }
                 }
             }
         }