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/06/29 13:33:52 UTC
(cloudstack) 08/24: Check resource reservation on volume snapshot creation
This is an automated email from the ASF dual-hosted git repository.
DaanHoogland pushed a commit to branch 4.22
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit 9a4f3415507835c88ed1720a0a2a08907405bad8
Author: dahn <da...@onecht.net>
AuthorDate: Fri Feb 20 17:29:25 2026 +0100
Check resource reservation on volume snapshot creation
---
.../storage/snapshot/SnapshotManagerImpl.java | 37 ++++++++++++++--------
.../storage/snapshot/SnapshotManagerTest.java | 11 +++++--
2 files changed, 31 insertions(+), 17 deletions(-)
diff --git a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java
index 19cde4da0f1..d5475948c59 100755
--- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java
+++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java
@@ -32,6 +32,7 @@ import java.util.stream.Collectors;
import javax.inject.Inject;
import javax.naming.ConfigurationException;
+import com.cloud.resourcelimit.CheckedReservation;
import org.apache.cloudstack.acl.SecurityChecker;
import com.cloud.api.ApiDBUtils;
import org.apache.cloudstack.annotation.AnnotationService;
@@ -67,6 +68,7 @@ import org.apache.cloudstack.framework.config.ConfigKey;
import org.apache.cloudstack.framework.config.Configurable;
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.reservation.dao.ReservationDao;
import org.apache.cloudstack.resourcedetail.SnapshotPolicyDetailVO;
import org.apache.cloudstack.resourcedetail.dao.SnapshotPolicyDetailsDao;
import org.apache.cloudstack.snapshot.SnapshotHelper;
@@ -240,6 +242,8 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement
@Inject
private AnnotationDao annotationDao;
+ @Inject
+ private ReservationDao reservationDao;
@Inject
protected SnapshotHelper snapshotHelper;
@Inject
@@ -1705,20 +1709,6 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement
Type snapshotType = getSnapshotType(policyId);
Account owner = _accountMgr.getAccount(volume.getAccountId());
- ResourceType storeResourceType = getStoreResourceType(volume.getDataCenterId(), locationType);
- try {
- _resourceLimitMgr.checkResourceLimit(owner, ResourceType.snapshot);
- _resourceLimitMgr.checkResourceLimit(owner, storeResourceType, volume.getSize());
- } catch (ResourceAllocationException e) {
- if (snapshotType != Type.MANUAL) {
- String msg = String.format("Snapshot resource limit exceeded for account %s. Failed to create recurring snapshots", owner);
- logger.warn(msg);
- _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_UPDATE_RESOURCE_COUNT, 0L, 0L, msg, "Snapshot resource limit exceeded for account id : " + owner.getId()
- + ". Failed to create recurring snapshots; please use updateResourceLimit to increase the limit");
- }
- throw e;
- }
-
// Determine the name for this snapshot
// Snapshot Name: VMInstancename + volumeName + timeString
String timeString = DateUtil.getDateDisplayString(DateUtil.GMT_TIMEZONE, new Date(), DateUtil.YYYYMMDD_FORMAT);
@@ -1750,6 +1740,14 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement
hypervisorType = volume.getHypervisorType();
}
+ ResourceType storeResourceType = ResourceType.secondary_storage;
+ if (!isBackupSnapshotToSecondaryForZone(volume.getDataCenterId()) ||
+ Snapshot.LocationType.PRIMARY.equals(locationType)) {
+ storeResourceType = ResourceType.primary_storage;
+ }
+
+ try (CheckedReservation volumeSnapshotReservation = new CheckedReservation(owner, ResourceType.snapshot, null, null, 1L, reservationDao, _resourceLimitMgr);
+ CheckedReservation storageReservation = new CheckedReservation(owner, storeResourceType, null, null, volume.getSize(), reservationDao, _resourceLimitMgr)) {
SnapshotVO snapshotVO = new SnapshotVO(volume.getDataCenterId(), volume.getAccountId(), volume.getDomainId(), volume.getId(), volume.getDiskOfferingId(), snapshotName,
(short)snapshotType.ordinal(), snapshotType.name(), volume.getSize(), volume.getMinIops(), volume.getMaxIops(), hypervisorType, locationType);
@@ -1761,6 +1759,17 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement
_resourceLimitMgr.incrementResourceCount(volume.getAccountId(), ResourceType.snapshot);
_resourceLimitMgr.incrementResourceCount(volume.getAccountId(), storeResourceType, volume.getSize());
return snapshot;
+ } catch (Exception e) {
+ if (e instanceof ResourceAllocationException) {
+ if (snapshotType != Type.MANUAL) {
+ String msg = String.format("Snapshot resource limit exceeded for account id : %s. Failed to create recurring snapshots", owner.getId());
+ logger.warn(msg);
+ _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_UPDATE_RESOURCE_COUNT, 0L, 0L, msg, msg + ". Please, use updateResourceLimit to increase the limit");
+ }
+ throw (ResourceAllocationException) e;
+ }
+ throw new CloudRuntimeException(e);
+ }
}
@Override
diff --git a/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java b/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java
index 28903c72cc3..5513536ab75 100755
--- a/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java
+++ b/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java
@@ -36,6 +36,7 @@ import java.util.UUID;
import com.cloud.api.ApiDBUtils;
import com.cloud.exception.PermissionDeniedException;
+import com.cloud.resourcelimit.CheckedReservation;
import com.cloud.storage.Storage;
import org.apache.cloudstack.api.command.user.snapshot.ExtractSnapshotCmd;
import org.apache.cloudstack.context.CallContext;
@@ -65,6 +66,7 @@ import org.junit.runner.RunWith;
import org.mockito.BDDMockito;
import org.mockito.InjectMocks;
import org.mockito.Mock;
+import org.mockito.MockedConstruction;
import org.mockito.MockedStatic;
import org.mockito.Mockito;
import org.mockito.junit.MockitoJUnitRunner;
@@ -233,8 +235,6 @@ public class SnapshotManagerTest {
when(_storageStrategyFactory.getSnapshotStrategy(Mockito.any(SnapshotVO.class), Mockito.eq(SnapshotOperation.BACKUP))).thenReturn(snapshotStrategy);
when(_storageStrategyFactory.getSnapshotStrategy(Mockito.any(SnapshotVO.class), Mockito.eq(SnapshotOperation.REVERT))).thenReturn(snapshotStrategy);
- doNothing().when(_resourceLimitMgr).checkResourceLimit(any(Account.class), any(ResourceType.class));
- doNothing().when(_resourceLimitMgr).checkResourceLimit(any(Account.class), any(ResourceType.class), anyLong());
doNothing().when(_resourceLimitMgr).decrementResourceCount(anyLong(), any(ResourceType.class), anyLong());
doNothing().when(_resourceLimitMgr).incrementResourceCount(anyLong(), any(ResourceType.class));
doNothing().when(_resourceLimitMgr).incrementResourceCount(anyLong(), any(ResourceType.class), anyLong());
@@ -317,7 +317,12 @@ public class SnapshotManagerTest {
when(mockList2.size()).thenReturn(0);
when(_vmSnapshotDao.listByInstanceId(TEST_VM_ID, VMSnapshot.State.Creating, VMSnapshot.State.Reverting, VMSnapshot.State.Expunging)).thenReturn(mockList2);
when(_snapshotDao.persist(any(SnapshotVO.class))).thenReturn(snapshotMock);
- _snapshotMgr.allocSnapshot(TEST_VOLUME_ID, Snapshot.MANUAL_POLICY_ID, null, null);
+
+ try (MockedConstruction<CheckedReservation> mockCheckedReservation = Mockito.mockConstruction(CheckedReservation.class)) {
+ _snapshotMgr.allocSnapshot(TEST_VOLUME_ID, Snapshot.MANUAL_POLICY_ID, null, null);
+ } catch (ResourceAllocationException e) {
+ Assert.fail(String.format("Failure with exception: %s", e.getMessage()));
+ }
}
@Test(expected = InvalidParameterValueException.class)