Skip to content

Change vmsnapshot.max setting scope to the account level #11616

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Change vmsnapshot.max setting scope to the account level #11616

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7cb7042
change vmsnapshotmax config scope
bernardodemarco Sep 10, 2025
8daad16
apply suggestion
bernardodemarco Oct 20, 2025
3820805
fix unit tests
bernardodemarco Oct 23, 2025
53ca91f
Update engine/components-api/src/main/java/com/cloud/vm/snapshot/VMSn…
bernardodemarco Nov 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion engine/components-api/src/main/java/com/cloud/vm/snapshot/VMSnapshotManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public interface VMSnapshotManager extends VMSnapshotService, Manager {
static final ConfigKey<Integer> VMSnapshotExpireInterval = new ConfigKey<Integer>("Advanced", Integer.class, "vmsnapshot.expire.interval", "-1",
"VM Snapshot expire interval in hours", true, ConfigKey.Scope.Account);

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

/**
* Delete all VM snapshots belonging to one VM
Expand Down
7 changes: 4 additions & 3 deletions server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,10 +400,11 @@ public VMSnapshot allocVMSnapshot(Long vmId, String vsDisplayName, String vsDesc
_accountMgr.checkAccess(caller, null, true, userVmVo);

// check max snapshot limit for per VM
int vmSnapshotMax = VMSnapshotManager.VMSnapshotMax.value();

boolean vmBelongsToProject = _accountMgr.getAccount(userVmVo.getAccountId()).getType() == Account.Type.PROJECT;
long accountIdToRetrieveConfigurationValueFrom = vmBelongsToProject ? caller.getId() : userVmVo.getAccountId();
int vmSnapshotMax = VMSnapshotManager.VMSnapshotMax.valueIn(accountIdToRetrieveConfigurationValueFrom);
if (_vmSnapshotDao.findByVm(vmId).size() >= vmSnapshotMax) {
throw new CloudRuntimeException("Creating vm snapshot failed due to a VM can just have : " + vmSnapshotMax + " VM snapshots. Please delete old ones");
throw new CloudRuntimeException(String.format("Each VM can have at most [%s] VM snapshots.", vmSnapshotMax));
}

// check if there are active volume snapshots tasks
Expand Down
15 changes: 15 additions & 0 deletions server/src/test/java/com/cloud/vm/snapshot/VMSnapshotManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import com.cloud.storage.dao.VolumeDao;
import com.cloud.user.Account;
import com.cloud.user.AccountManager;
import com.cloud.user.AccountVO;
import com.cloud.user.dao.AccountDao;
import com.cloud.user.dao.UserDao;
import com.cloud.uservm.UserVm;
Expand Down Expand Up @@ -136,6 +137,8 @@ public class VMSnapshotManagerTest {
VMSnapshotDetailsDao _vmSnapshotDetailsDao;
@Mock
UserVmManager _userVmManager;
@Mock
private AccountVO accountVOMock;

private static final long TEST_VM_ID = 3L;
private static final long SERVICE_OFFERING_ID = 1L;
Expand Down Expand Up @@ -285,8 +288,12 @@ public void testCreateVMSnapshotF3() throws AgentUnavailableException, Operation
@SuppressWarnings("unchecked")
@Test(expected = CloudRuntimeException.class)
public void testAllocVMSnapshotF4() throws ResourceAllocationException {
long accountId = 1L;
List<VMSnapshotVO> mockList = mock(List.class);
when(mockList.size()).thenReturn(10);
when(_userVMDao.findById(TEST_VM_ID)).thenReturn(vmMock);
when(userVm.getAccountId()).thenReturn(accountId);
when(_accountMgr.getAccount(accountId)).thenReturn(accountVOMock);
when(_vmSnapshotDao.findByVm(TEST_VM_ID)).thenReturn(mockList);
_vmSnapshotMgr.allocVMSnapshot(TEST_VM_ID, "", "", true);
}
Expand All @@ -295,15 +302,23 @@ public void testAllocVMSnapshotF4() throws ResourceAllocationException {
@SuppressWarnings("unchecked")
@Test(expected = CloudRuntimeException.class)
public void testAllocVMSnapshotF5() throws ResourceAllocationException {
long accountId = 1L;
List<SnapshotVO> mockList = mock(List.class);
when(mockList.size()).thenReturn(1);
when(_userVMDao.findById(TEST_VM_ID)).thenReturn(vmMock);
when(userVm.getAccountId()).thenReturn(accountId);
when(_accountMgr.getAccount(accountId)).thenReturn(accountVOMock);
when(_snapshotDao.listByInstanceId(TEST_VM_ID, Snapshot.State.Creating, Snapshot.State.CreatedOnPrimary, Snapshot.State.BackingUp)).thenReturn(mockList);
_vmSnapshotMgr.allocVMSnapshot(TEST_VM_ID, "", "", true);
}

// successful creation case
@Test
public void testCreateVMSnapshot() throws AgentUnavailableException, OperationTimedoutException, ResourceAllocationException, NoTransitionException {
long accountId = 1L;
when(_userVMDao.findById(TEST_VM_ID)).thenReturn(vmMock);
when(userVm.getAccountId()).thenReturn(accountId);
when(_accountMgr.getAccount(accountId)).thenReturn(accountVOMock);
when(vmMock.getState()).thenReturn(State.Running);
_vmSnapshotMgr.allocVMSnapshot(TEST_VM_ID, "", "", true);
}
Expand Down
Loading