Skip to content

Commit 4a453d0

Browse files
bernardodemarcoowsferraro
authored andcommitted
Change vmsnapshot.max setting scope to the account level (apache#11616)
1 parent 936341c commit 4a453d0

File tree

3 files changed

+20
-4
lines changed

3 files changed

+20
-4
lines changed

engine/components-api/src/main/java/com/cloud/vm/snapshot/VMSnapshotManager.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public interface VMSnapshotManager extends VMSnapshotService, Manager {
3131
static final ConfigKey<Integer> VMSnapshotExpireInterval = new ConfigKey<Integer>("Advanced", Integer.class, "vmsnapshot.expire.interval", "-1",
3232
"VM Snapshot expire interval in hours", true, ConfigKey.Scope.Account);
3333

34-
ConfigKey<Integer> VMSnapshotMax = new ConfigKey<Integer>("Advanced", Integer.class, "vmsnapshot.max", "10", "Maximum vm snapshots for a single vm", true, ConfigKey.Scope.Global);
34+
ConfigKey<Integer> VMSnapshotMax = new ConfigKey<Integer>("Advanced", Integer.class, "vmsnapshot.max", "10", "Maximum VM snapshots for a single VM", true, ConfigKey.Scope.Account);
3535

3636
/**
3737
* Delete all VM snapshots belonging to one VM

server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -406,10 +406,11 @@ public VMSnapshot allocVMSnapshot(Long vmId, String vsDisplayName, String vsDesc
406406
_accountMgr.checkAccess(caller, null, true, userVmVo);
407407

408408
// check max snapshot limit for per VM
409-
int vmSnapshotMax = VMSnapshotManager.VMSnapshotMax.value();
410-
409+
boolean vmBelongsToProject = _accountMgr.getAccount(userVmVo.getAccountId()).getType() == Account.Type.PROJECT;
410+
long accountIdToRetrieveConfigurationValueFrom = vmBelongsToProject ? caller.getId() : userVmVo.getAccountId();
411+
int vmSnapshotMax = VMSnapshotManager.VMSnapshotMax.valueIn(accountIdToRetrieveConfigurationValueFrom);
411412
if (_vmSnapshotDao.findByVm(vmId).size() >= vmSnapshotMax) {
412-
throw new CloudRuntimeException("Creating Instance Snapshot failed due to a Instance can just have : " + vmSnapshotMax + " Instance Snapshots. Please delete old ones");
413+
throw new CloudRuntimeException(String.format("Each VM can have at most [%s] VM snapshots.", vmSnapshotMax));
413414
}
414415

415416
// check if there are active volume snapshots tasks

server/src/test/java/com/cloud/vm/snapshot/VMSnapshotManagerTest.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import com.cloud.storage.dao.VolumeDao;
4242
import com.cloud.user.Account;
4343
import com.cloud.user.AccountManager;
44+
import com.cloud.user.AccountVO;
4445
import com.cloud.user.dao.AccountDao;
4546
import com.cloud.user.dao.UserDao;
4647
import com.cloud.uservm.UserVm;
@@ -141,6 +142,8 @@ public class VMSnapshotManagerTest {
141142
VMSnapshotDetailsDao _vmSnapshotDetailsDao;
142143
@Mock
143144
UserVmManager _userVmManager;
145+
@Mock
146+
private AccountVO accountVOMock;
144147

145148
private static final long TEST_VM_ID = 3L;
146149
private static final long SERVICE_OFFERING_ID = 1L;
@@ -290,8 +293,12 @@ public void testCreateVMSnapshotF3() throws AgentUnavailableException, Operation
290293
@SuppressWarnings("unchecked")
291294
@Test(expected = CloudRuntimeException.class)
292295
public void testAllocVMSnapshotF4() throws ResourceAllocationException {
296+
long accountId = 1L;
293297
List<VMSnapshotVO> mockList = mock(List.class);
294298
when(mockList.size()).thenReturn(10);
299+
when(_userVMDao.findById(TEST_VM_ID)).thenReturn(vmMock);
300+
when(userVm.getAccountId()).thenReturn(accountId);
301+
when(_accountMgr.getAccount(accountId)).thenReturn(accountVOMock);
295302
when(_vmSnapshotDao.findByVm(TEST_VM_ID)).thenReturn(mockList);
296303
_vmSnapshotMgr.allocVMSnapshot(TEST_VM_ID, "", "", true);
297304
}
@@ -300,15 +307,23 @@ public void testAllocVMSnapshotF4() throws ResourceAllocationException {
300307
@SuppressWarnings("unchecked")
301308
@Test(expected = CloudRuntimeException.class)
302309
public void testAllocVMSnapshotF5() throws ResourceAllocationException {
310+
long accountId = 1L;
303311
List<SnapshotVO> mockList = mock(List.class);
304312
when(mockList.size()).thenReturn(1);
313+
when(_userVMDao.findById(TEST_VM_ID)).thenReturn(vmMock);
314+
when(userVm.getAccountId()).thenReturn(accountId);
315+
when(_accountMgr.getAccount(accountId)).thenReturn(accountVOMock);
305316
when(_snapshotDao.listByInstanceId(TEST_VM_ID, Snapshot.State.Creating, Snapshot.State.CreatedOnPrimary, Snapshot.State.BackingUp)).thenReturn(mockList);
306317
_vmSnapshotMgr.allocVMSnapshot(TEST_VM_ID, "", "", true);
307318
}
308319

309320
// successful creation case
310321
@Test
311322
public void testCreateVMSnapshot() throws AgentUnavailableException, OperationTimedoutException, ResourceAllocationException, NoTransitionException {
323+
long accountId = 1L;
324+
when(_userVMDao.findById(TEST_VM_ID)).thenReturn(vmMock);
325+
when(userVm.getAccountId()).thenReturn(accountId);
326+
when(_accountMgr.getAccount(accountId)).thenReturn(accountVOMock);
312327
when(vmMock.getState()).thenReturn(State.Running);
313328
_vmSnapshotMgr.allocVMSnapshot(TEST_VM_ID, "", "", true);
314329
}

0 commit comments

Comments
 (0)