Fix of deployment VM from a copied snapshot in another zone #11351

slavkap · 2025-07-30T12:27:51Z

Description

This PR fixes the deployment of a VM with a snapshot that is copied to another zone.
Fix for creating StorPool volume from a snapshot if the size in the offering is bigger than the snapshot size

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)

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Manually tested

codecov · 2025-07-30T12:36:01Z

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 17.35%. Comparing base (d7b7bd5) to head (2d2990d).
⚠️ Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
...rapper/StorPoolDownloadTemplateCommandWrapper.java	0.00%	1 Missing ⚠️
.../src/main/java/com/cloud/vm/UserVmManagerImpl.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #11351      +/-   ##
============================================
- Coverage     17.35%   17.35%   -0.01%     
+ Complexity    15231    15229       -2     
============================================
  Files          5885     5885              
  Lines        525611   525617       +6     
  Branches      64160    64157       -3     
============================================
- Hits          91224    91214      -10     
- Misses       424090   424107      +17     
+ Partials      10297    10296       -1

Flag	Coverage Δ
uitests	`3.63% <ø> (ø)`
unittests	`18.39% <0.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.

Copilot

Pull Request Overview

This PR fixes VM deployment issues when using snapshots from different zones, specifically addressing StorPool volume creation from snapshots when the offering size exceeds the snapshot size.

Removed complex volume size determination logic in favor of standardized disk size configuration
Eliminated override disk offering assignments for both volume and snapshot-based deployments
Fixed StorPool template resizing to only apply to template objects, not all storage objects

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
UserVmManagerImpl.java	Simplified volume size calculation and removed disk offering overrides for cross-zone deployments
StorPoolDownloadTemplateCommandWrapper.java	Moved StorPool resize operation to only execute for template objects

Copilot · 2025-07-30T13:08:39Z

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

-            volumesSize = configureCustomRootDiskSize(customParameters, template, hypervisorType, rootDiskOffering);
-        }
+        long volumesSize = configureCustomRootDiskSize(customParameters, template, hypervisorType, rootDiskOffering);



The simplified logic now ignores the actual size of existing volumes and snapshots, which could lead to data loss or corruption when the configured size is smaller than the source volume/snapshot size. Consider adding validation to ensure the configured size is at least as large as the source.

Suggested change

// Validate that the configured size is at least as large as the source volume or snapshot size

if (snapshot != null) {

long sourceSize = snapshot.getSize(); // Assuming snapshot.getSize() returns the size in bytes

if (volumesSize < sourceSize) {

throw new InvalidParameterValueException("The configured root disk size (" + volumesSize +

" bytes) is smaller than the source snapshot size (" + sourceSize + " bytes).");

}

} else if (volume != null) {

long sourceSize = volume.getSize(); // Assuming volume.getSize() returns the size in bytes

if (volumesSize < sourceSize) {

throw new InvalidParameterValueException("The configured root disk size (" + volumesSize +

" bytes) is smaller than the source volume size (" + sourceSize + " bytes).");

}

}

Copilot · 2025-07-30T13:08:40Z

...n/java/com/cloud/hypervisor/kvm/resource/wrapper/StorPoolDownloadTemplateCommandWrapper.java

+                StorPoolStorageAdaptor.resize( Long.toString(srcDisk.getVirtualSize()), dst.getPath());
+


The resize operation is now only called for TemplateObjectTO instances, but there's no validation that srcDisk.getVirtualSize() is appropriate for the destination. Consider adding size validation before calling resize.

Suggested change

StorPoolStorageAdaptor.resize( Long.toString(srcDisk.getVirtualSize()), dst.getPath());

if (dst.getSize() != null && dst.getSize() < srcDisk.getVirtualSize()) {

final String error = "Destination size " + dst.getSize() + " is smaller than source virtual size " + srcDisk.getVirtualSize();

SP_LOG(error);

return new CopyCmdAnswer(error);

}

StorPoolStorageAdaptor.resize(Long.toString(srcDisk.getVirtualSize()), dst.getPath());

very sensible comment @Copilot, but it has nothing to do with the changes at hand.

sureshanaparti · 2025-07-30T13:12:04Z

@blueorangutan package

sureshanaparti · 2025-07-30T13:12:51Z

...n/java/com/cloud/hypervisor/kvm/resource/wrapper/StorPoolDownloadTemplateCommandWrapper.java

-            StorPoolStorageAdaptor.resize( Long.toString(srcDisk.getVirtualSize()), dst.getPath());

            if (dst instanceof TemplateObjectTO) {
+                StorPoolStorageAdaptor.resize( Long.toString(srcDisk.getVirtualSize()), dst.getPath());


Suggested change

StorPoolStorageAdaptor.resize( Long.toString(srcDisk.getVirtualSize()), dst.getPath());

StorPoolStorageAdaptor.resize(Long.toString(srcDisk.getVirtualSize()), dst.getPath());

DaanHoogland

clgtm, the second copilot remark seems as mute as the first one, I imaging that configureCustomRootDiskSize already does the checks sugested.

nvazquez

Code LGTM (haven't tested it though)

nvazquez · 2025-07-30T23:42:11Z

@blueorangutan package

blueorangutan · 2025-07-30T23:44:04Z

@nvazquez 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-07-31T00:55:27Z

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

nvazquez · 2025-07-31T01:12:51Z

@blueorangutan test

blueorangutan · 2025-07-31T01:14:04Z

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

blueorangutan · 2025-07-31T01:17:26Z

[SF] Trillian Build Failed (tid-13981)

sureshanaparti · 2025-07-31T09:35:23Z

@blueorangutan test

blueorangutan · 2025-07-31T09:36:03Z

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

blueorangutan · 2025-08-01T02:24:43Z

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

Test	Result	Time (s)	Test File

fix of deploy VM with a snapshot that is copied to another zone fix of creating StorPool volume from a snapshot if the size in the offering is bigger than the snapshot size

sureshanaparti · 2025-08-06T17:23:03Z

@blueorangutan package

blueorangutan · 2025-08-06T17:24:03Z

@sureshanaparti 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-08-06T18:49:15Z

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

DaanHoogland · 2025-08-07T06:09:57Z

@blueorangutan test

blueorangutan · 2025-08-07T06:12:04Z

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

blueorangutan · 2025-08-07T20:32:18Z

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

Test	Result	Time (s)	Test File

…1351) * Fix of deploy VM with a snapshot that is copied to another zone * Fix of creating StorPool volume from a snapshot if the size in the offering is bigger than the snapshot size

boring-cyborg bot added component:primary-storage component:storage labels Jul 30, 2025

DaanHoogland requested a review from Copilot July 30, 2025 13:08

Copilot AI reviewed Jul 30, 2025

View reviewed changes

sureshanaparti reviewed Jul 30, 2025

View reviewed changes

DaanHoogland approved these changes Jul 30, 2025

View reviewed changes

nvazquez approved these changes Jul 30, 2025

View reviewed changes

slavkap added 2 commits August 5, 2025 11:42

Fix of deployment VM from a copied snapshot in another zone

48170e1

fix of deploy VM with a snapshot that is copied to another zone fix of creating StorPool volume from a snapshot if the size in the offering is bigger than the snapshot size

addressed comments

2d2990d

slavkap force-pushed the fix-deploy-vm-from-copied-snapshot branch from 2652739 to 2d2990d Compare August 5, 2025 11:26

sureshanaparti added this to the 4.21.0 milestone Aug 6, 2025

sureshanaparti added this to Apache CloudStack 4.21.0 Aug 7, 2025

sureshanaparti moved this to In Progress in Apache CloudStack 4.21.0 Aug 7, 2025

sureshanaparti merged commit dc5e475 into apache:main Aug 7, 2025
25 of 26 checks passed

github-project-automation bot moved this from In Progress to Done in Apache CloudStack 4.21.0 Aug 7, 2025

+        // Validate that the configured size is at least as large as the source volume or snapshot size
+        if (snapshot != null) {
+            long sourceSize = snapshot.getSize(); // Assuming snapshot.getSize() returns the size in bytes
+            if (volumesSize < sourceSize) {
+                throw new InvalidParameterValueException("The configured root disk size (" + volumesSize +
+                        " bytes) is smaller than the source snapshot size (" + sourceSize + " bytes).");
+            }
+        } else if (volume != null) {
+            long sourceSize = volume.getSize(); // Assuming volume.getSize() returns the size in bytes
+            if (volumesSize < sourceSize) {
+                throw new InvalidParameterValueException("The configured root disk size (" + volumesSize +
+                        " bytes) is smaller than the source volume size (" + sourceSize + " bytes).");
+            }
+        }

		StorPoolStorageAdaptor.resize( Long.toString(srcDisk.getVirtualSize()), dst.getPath());

-                StorPoolStorageAdaptor.resize( Long.toString(srcDisk.getVirtualSize()), dst.getPath());
+                if (dst.getSize() != null && dst.getSize() < srcDisk.getVirtualSize()) {
+                    final String error = "Destination size " + dst.getSize() + " is smaller than source virtual size " + srcDisk.getVirtualSize();
+                    SP_LOG(error);
+                    return new CopyCmdAnswer(error);
+                }
+                StorPoolStorageAdaptor.resize(Long.toString(srcDisk.getVirtualSize()), dst.getPath());

	StorPoolStorageAdaptor.resize( Long.toString(srcDisk.getVirtualSize()), dst.getPath());
	StorPoolStorageAdaptor.resize(Long.toString(srcDisk.getVirtualSize()), dst.getPath());

Fix of deployment VM from a copied snapshot in another zone #11351

Fix of deployment VM from a copied snapshot in another zone #11351

Uh oh!

Conversation

slavkap commented Jul 30, 2025

Description

Types of changes

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

DaanHoogland Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

sureshanaparti commented Jul 30, 2025

Uh oh!

sureshanaparti Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

nvazquez left a comment

Choose a reason for hiding this comment

Uh oh!

nvazquez commented Jul 30, 2025

Uh oh!

blueorangutan commented Jul 30, 2025

Uh oh!

blueorangutan commented Jul 31, 2025

Uh oh!

nvazquez commented Jul 31, 2025

Uh oh!

blueorangutan commented Jul 31, 2025

Uh oh!

blueorangutan commented Jul 31, 2025

Uh oh!

sureshanaparti commented Jul 31, 2025

Uh oh!

blueorangutan commented Jul 31, 2025

Uh oh!

blueorangutan commented Aug 1, 2025

Uh oh!

sureshanaparti commented Aug 6, 2025

Uh oh!

blueorangutan commented Aug 6, 2025

Uh oh!

blueorangutan commented Aug 6, 2025

Uh oh!

DaanHoogland commented Aug 7, 2025

Uh oh!

blueorangutan commented Aug 7, 2025

Uh oh!

Uh oh!

blueorangutan commented Aug 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov bot commented Jul 30, 2025 •

edited

Loading