You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by da...@apache.org on 2026/03/16 16:42:02 UTC
(cloudstack) 01/02: review comments
This is an automated email from the ASF dual-hosted git repository.
dahn pushed a commit to branch 4.20
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit 23b19a9776ded761f5c36b5e8fb367450fb62d2f
Author: Abhisar Sinha <63...@users.noreply.github.com>
AuthorDate: Fri Mar 13 16:28:30 2026 +0530
review comments
---
.../com/cloud/projects/ProjectManagerImpl.java | 41 ++++++++++------------
.../cloud/storage/ImageStoreUploadMonitorImpl.java | 2 +-
.../cloud/storage/download/DownloadListener.java | 7 ++--
.../com/cloud/template/TemplateManagerImpl.java | 14 ++++----
4 files changed, 33 insertions(+), 31 deletions(-)
diff --git a/server/src/main/java/com/cloud/projects/ProjectManagerImpl.java b/server/src/main/java/com/cloud/projects/ProjectManagerImpl.java
index 43efccd04f9..6942400c82c 100644
--- a/server/src/main/java/com/cloud/projects/ProjectManagerImpl.java
+++ b/server/src/main/java/com/cloud/projects/ProjectManagerImpl.java
@@ -604,16 +604,16 @@ public class ProjectManagerImpl extends ManagerBase implements ProjectManager, C
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 class ProjectManagerImpl extends ManagerBase implements ProjectManager, C
}
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);
diff --git a/server/src/main/java/com/cloud/storage/ImageStoreUploadMonitorImpl.java b/server/src/main/java/com/cloud/storage/ImageStoreUploadMonitorImpl.java
index b56e5b56213..a63b0a6f5ec 100755
--- a/server/src/main/java/com/cloud/storage/ImageStoreUploadMonitorImpl.java
+++ b/server/src/main/java/com/cloud/storage/ImageStoreUploadMonitorImpl.java
@@ -360,7 +360,7 @@ public class ImageStoreUploadMonitorImpl extends ManagerBase implements ImageSto
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");
diff --git a/server/src/main/java/com/cloud/storage/download/DownloadListener.java b/server/src/main/java/com/cloud/storage/download/DownloadListener.java
index 058881fdb54..695b1c060e4 100644
--- a/server/src/main/java/com/cloud/storage/download/DownloadListener.java
+++ b/server/src/main/java/com/cloud/storage/download/DownloadListener.java
@@ -280,7 +280,7 @@ public class DownloadListener implements Listener {
}
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();
@@ -288,7 +288,7 @@ public class DownloadListener implements Listener {
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 @@ public class DownloadListener implements Listener {
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)) {
diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java
index 1f3eb567c6f..34996710bdb 100755
--- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java
+++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java
@@ -1035,12 +1035,14 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager,
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());
+ }
}
}
}