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

bernardodemarco · 2025-09-11T12:54:35Z

Description

The vmsnapshot.max global setting is used to control the maximum number of VM snapshots that can be taken for a single VM. However, its scope is global and, thus, when configured, all accounts of the ACS environment will have to comply with the same limitation.

This PR proposes to change the setting's scope to the account level, enabling for the cloud operators to define different limitations for different accounts.

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)
build/CI
test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Defined the vmsnapshot.max global value as 2
Checked that the limitation was correctly enforced for all accounts
Defined the vmsnapshot.max value to 1 for a user account
Checked that the above limitation was correctly enforced for the user account
Checked that the global limitation was still enforced for the others accounts

codecov · 2025-09-11T13:02:43Z

Codecov Report

❌ Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 17.56%. Comparing base (4cdcde2) to head (53ca91f).
⚠️ Report is 34 commits behind head on main.

Files with missing lines	Patch %	Lines
...a/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java	50.00%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##              main   #11616       +/-   ##
============================================
+ Coverage     3.58%   17.56%   +13.98%     
- Complexity       0    15544    +15544     
============================================
  Files          445     5909     +5464     
  Lines        37532   529061   +491529     
  Branches      6901    64619    +57718     
============================================
+ Hits          1346    92940    +91594     
- Misses       36022   425664   +389642     
- Partials       164    10457    +10293

Flag	Coverage Δ
uitests	`3.58% <ø> (-0.01%)`	⬇️
unittests	`18.63% <60.00%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

DaanHoogland · 2025-09-15T09:43:37Z

functionaly sane change, I am just wondering if a domain level maximum should be enforced as well? If we do it would require some functional definition (is it a default, is it possible to go beyond the domain level maximum, etc) Not in this PR.

DaanHoogland

clgtm

harikrishna-patnala

code LGTM

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

bernardodemarco · 2025-10-20T22:52:55Z

@blueorangutan package

blueorangutan · 2025-10-20T22:54:04Z

@bernardodemarco a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2025-10-20T23:10:55Z

Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 15507

DaanHoogland · 2025-10-21T08:51:51Z

@bernardodemarco , ignore the GHA for now as per #11873, but look at the below from the backend, I think these are related:

01:04:12 [INFO] Running com.cloud.vm.snapshot.VMSnapshotManagerTest
01:04:12 23:04:13.850 [main] ERROR com.cloud.vm.snapshot.VMSnapshotManagerImpl - Couldn't change service offering for vm userVm to serviceOffering
01:04:13 [ERROR] Tests run: 19, Failures: 0, Errors: 3, Skipped: 0, Time elapsed: 1.141 s <<< FAILURE! - in com.cloud.vm.snapshot.VMSnapshotManagerTest
01:04:14 [ERROR] testAllocVMSnapshotF4(com.cloud.vm.snapshot.VMSnapshotManagerTest)  Time elapsed: 0.034 s  <<< ERROR!
01:04:14 java.lang.Exception: Unexpected exception, expected<com.cloud.utils.exception.CloudRuntimeException> but was<java.lang.NullPointerException>
01:04:14 	at com.cloud.vm.snapshot.VMSnapshotManagerTest.testAllocVMSnapshotF4(VMSnapshotManagerTest.java:291)
01:04:14 
01:04:14 [ERROR] testAllocVMSnapshotF5(com.cloud.vm.snapshot.VMSnapshotManagerTest)  Time elapsed: 0.016 s  <<< ERROR!
01:04:14 java.lang.Exception: Unexpected exception, expected<com.cloud.utils.exception.CloudRuntimeException> but was<java.lang.NullPointerException>
01:04:14 	at com.cloud.vm.snapshot.VMSnapshotManagerTest.testAllocVMSnapshotF5(VMSnapshotManagerTest.java:301)
01:04:14 
01:04:14 [ERROR] testCreateVMSnapshot(com.cloud.vm.snapshot.VMSnapshotManagerTest)  Time elapsed: 0.007 s  <<< ERROR!
01:04:14 java.lang.NullPointerException
01:04:14 	at com.cloud.vm.snapshot.VMSnapshotManagerTest.testCreateVMSnapshot(VMSnapshotManagerTest.java:308)
01:04:14

bernardodemarco · 2025-10-21T11:43:28Z

@DaanHoogland, yes, they seem to be related. I'll soon try to take a look at them

bernardodemarco · 2025-10-23T22:53:52Z

@blueorangutan package

blueorangutan · 2025-10-23T22:56:04Z

@bernardodemarco a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2025-10-24T00:10:37Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15533

DaanHoogland · 2025-10-24T09:43:14Z

@blueorangutan test

blueorangutan · 2025-10-24T09:44:04Z

@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

blueorangutan · 2025-10-25T05:18:39Z

[SF] Trillian test result (tid-14718)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 66398 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr11616-t14718-kvm-ol8.zip
Smoke tests completed. 146 look OK, 1 have errors, 2 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_02_list_cpvm_vm	`Failure`	0.07	test_ssvm.py
test_04_cpvm_internals	`Failure`	0.07	test_ssvm.py
all_test_human_readable_logs	`Skipped`	---	test_human_readable_logs.py
all_test_image_store_object_migration	`Skipped`	---	test_image_store_object_migration.py

lucas-a-martins

CLGTM

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

…apshotManager.java Co-authored-by: Lucas Martins <[email protected]>

bernardodemarco added this to the 4.22.0 milestone Sep 11, 2025

DaanHoogland approved these changes Sep 15, 2025

View reviewed changes

harikrishna-patnala approved these changes Sep 23, 2025

View reviewed changes

harikrishna-patnala self-requested a review September 23, 2025 06:39

harikrishna-patnala reviewed Sep 23, 2025

View reviewed changes

server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java Outdated Show resolved Hide resolved

harikrishna-patnala added this to Apache CloudStack 4.22.0 Sep 23, 2025

harikrishna-patnala self-requested a review September 23, 2025 06:44

bernardodemarco added 2 commits October 17, 2025 15:47

change vmsnapshotmax config scope

7cb7042

apply suggestion

8daad16

bernardodemarco force-pushed the change-vmsnapshotmax-scope branch from a399f91 to 8daad16 Compare October 20, 2025 22:50

fix unit tests

3820805

lucas-a-martins approved these changes Nov 6, 2025

View reviewed changes

engine/components-api/src/main/java/com/cloud/vm/snapshot/VMSnapshotManager.java Outdated Show resolved Hide resolved

Update engine/components-api/src/main/java/com/cloud/vm/snapshot/VMSn…

53ca91f

…apshotManager.java Co-authored-by: Lucas Martins <[email protected]>

bernardodemarco added the status:needs-testing label Nov 6, 2025

harikrishna-patnala modified the milestones: 4.22.0, 4.22.1 Nov 12, 2025

harikrishna-patnala removed this from Apache CloudStack 4.22.0 Nov 12, 2025

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

Are you sure you want to change the base?

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

Uh oh!

Conversation

bernardodemarco commented Sep 11, 2025

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

codecov bot commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

DaanHoogland commented Sep 15, 2025

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

harikrishna-patnala left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bernardodemarco commented Oct 20, 2025

Uh oh!

blueorangutan commented Oct 20, 2025

Uh oh!

blueorangutan commented Oct 20, 2025

Uh oh!

DaanHoogland commented Oct 21, 2025

Uh oh!

bernardodemarco commented Oct 21, 2025

Uh oh!

bernardodemarco commented Oct 23, 2025

Uh oh!

blueorangutan commented Oct 23, 2025

Uh oh!

blueorangutan commented Oct 24, 2025

Uh oh!

DaanHoogland commented Oct 24, 2025

Uh oh!

blueorangutan commented Oct 24, 2025

Uh oh!

blueorangutan commented Oct 25, 2025

Uh oh!

lucas-a-martins left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

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

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

codecov bot commented Sep 11, 2025 •

edited

Loading