[Veeam] Fix create new instance from backup using VSAN storage#13203
[Veeam] Fix create new instance from backup using VSAN storage#13203nvazquez wants to merge 1 commit into
Conversation
|
@blueorangutan package |
|
@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. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.22 #13203 +/- ##
=========================================
Coverage 17.67% 17.67%
- Complexity 15788 15789 +1
=========================================
Files 5922 5922
Lines 533121 533131 +10
Branches 65201 65203 +2
=========================================
+ Hits 94240 94246 +6
- Misses 428236 428239 +3
- Partials 10645 10646 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17950 |
There was a problem hiding this comment.
Pull request overview
This PR fixes Veeam-based VM restore/create-from-backup flows on VMware when the target datastore must be identified by datastore name (not the CloudStack UUID), which is required for vSAN/VMFS scenarios (issue #11888).
Changes:
- Add
BackupManagerImpl.getDatastorePossibleValues(...)to prefer datastore name for VMFS pools and keep UUID+name fallbacks for other pool types. - Update VM restore paths to use the new datastore identifier selection logic.
- Adjust Veeam PowerShell datastore identifier handling to only strip hyphens when the input is an actual UUID; add unit tests for datastore selection.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java |
Uses datastore-name-first logic for VMFS and centralizes datastore identifier selection for restore operations. |
server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java |
Adds tests for the new datastore-possible-values behavior. |
plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/veeam/VeeamClient.java |
Prevents unintended mutation of datastore names (e.g., removing -) while keeping UUID normalization. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| final String datastoreId = UuidUtils.isUuid(dataStoreUuid) ? | ||
| dataStoreUuid.replace("-","") : | ||
| dataStoreUuid; |
|
|
||
| /** | ||
| * For VMFS datastores, the identifier to be used for Veeam restore is the datastore name. | ||
| * Otherwise, possible values are the datastore UUID and the datastore name.. |
| @Test | ||
| public void testGetDatastorePossibleValuesVSAN() { | ||
| String poolName = "VSAN-Pool-Name"; | ||
|
|
||
| StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); | ||
| when(pool.getPoolType()).thenReturn(Storage.StoragePoolType.VMFS); | ||
| when(pool.getName()).thenReturn(poolName); |
|
@nvazquez please check if the copilot's comments are valid or not. |
Description
This PR fixes creation of VM from backup using VSAN storage.
Fixes: #11888
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?