Change `vmsnapshot.max` setting scope to the account level by bernardodemarco · Pull Request #11616 · apache/cloudstack

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.76%. Comparing base (2811217) to head (86297b0).
⚠️ Report is 116 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 17.76% 17.76% - Complexity 15862 15863 +1 
=========================================
Files 5923 5923 Lines 530553 530555 +2 Branches 64829 64831 +2 =========================================
+ Hits 94260 94263 +3 + Misses 425747 425745 -2 - Partials 10546 10547 +1

Flag	Coverage Δ
uitests	`3.57% <ø> (ø)`
unittests	`18.85% <60.00%> (+<0.01%)`	⬆️

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

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

github-actions · 2025-12-22T13:28:24Z

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

…apshotManager.java Co-authored-by: Lucas Martins <56271185+lucas-a-martins@users.noreply.github.com>

bernardodemarco · 2026-01-09T17:05:31Z

@blueorangutan package

blueorangutan · 2026-01-09T17:06:05Z

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

blueorangutan · 2026-01-09T18:05:53Z

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

rajujith · 2026-01-12T07:19:10Z

@bernardodemarco Since this is for the 4.22.1 release, could you retarget the PR to the 4.22 branch?

bernardodemarco · 2026-01-12T12:34:13Z

@rajujith, sorry, I had forgotten to update the milestone to 4.23 after 4.22 was released

vladimirpetrov

LGTM based on manual testing.

vladimirpetrov · 2026-01-28T16:11:16Z

@blueorangutan test

blueorangutan · 2026-01-28T16:12:04Z

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

bernardodemarco · 2026-01-28T16:33:55Z

@vladimirpetrov, thanks for testing!

blueorangutan · 2026-01-29T08:29:01Z

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

Test	Result	Time (s)	Test File
test_03_deploy_and_scale_kubernetes_cluster	`Failure`	29.77	test_kubernetes_clusters.py

)

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

Comment threadserver/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java Outdated

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 force-pushed the change-vmsnapshotmax-scope branch from a399f91 to 8daad16Compare October 20, 2025 22:50

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

Comment threadengine/components-api/src/main/java/com/cloud/vm/snapshot/VMSnapshotManager.java Outdated

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

github-actionsBot added the status:has-conflicts label Dec 22, 2025

rajujith added this to Apache CloudStack 4.22.1 Jan 5, 2026

rajujith moved this to In Progress in Apache CloudStack 4.22.1 Jan 7, 2026

bernardodemarcoand others added 4 commits January 9, 2026 13:57

change vmsnapshotmax config scope
3ba5e39

apply suggestion
8c68a67

fix unit tests
282aa19

Update engine/components-api/src/main/java/com/cloud/vm/snapshot/VMSn…
86297b0
…apshotManager.java Co-authored-by: Lucas Martins <56271185+lucas-a-martins@users.noreply.github.com>

bernardodemarco force-pushed the change-vmsnapshotmax-scope branch from 53ca91f to 86297b0Compare January 9, 2026 17:04

github-actionsBot removed the status:has-conflicts label Jan 9, 2026

bernardodemarco modified the milestones: 4.22.1, 4.23.0 Jan 12, 2026

DaanHoogland removed this from Apache CloudStack 4.22.1 Jan 12, 2026

RosiKyu assigned RosiKyu and unassigned RosiKyu Jan 26, 2026

vladimirpetrov self-assigned this Jan 28, 2026

vladimirpetrov approved these changes Jan 28, 2026
View reviewed changes

vladimirpetrov removed the status:needs-testing label Jan 28, 2026

bernardodemarco added the status:waiting-for-smoketest-results label Jan 28, 2026

DaanHoogland merged commit c681d0d into apache:mainJan 29, 2026
28 checks passed

sandeeplocharla pushed a commit to NetApp/cloudstack that referenced this pull request Feb 6, 2026
Changevmsnapshot.maxsetting scope to the account level (apache#11616
374fe1e
)

dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Mar 3, 2026
Changevmsnapshot.maxsetting scope to the account level (apache#11616
dc82a31
)

owsferraro pushed a commit to owsferraro/cloudstack that referenced this pull request Mar 25, 2026
Changevmsnapshot.maxsetting scope to the account level (apache#11616
4a453d0
)

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!

codecovBot 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!

github-actionsBot commented Dec 22, 2025

Uh oh!

bernardodemarco commented Jan 9, 2026

Uh oh!

blueorangutan commented Jan 9, 2026

Uh oh!

blueorangutan commented Jan 9, 2026

Uh oh!

rajujith commented Jan 12, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bernardodemarco commented Jan 12, 2026

Uh oh!

vladimirpetrov left a comment

Choose a reason for hiding this comment

Uh oh!

vladimirpetrov commented Jan 28, 2026

Uh oh!

blueorangutan commented Jan 28, 2026

Uh oh!

bernardodemarco commented Jan 28, 2026

Uh oh!

blueorangutan commented Jan 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

codecovBot commented Sep 11, 2025•
edited
Loading

rajujith commented Jan 12, 2026•
edited
Loading