Preserve device Ids of data volumes in the new instance.

abh1sar · abh1sar · commit f4f0655eaa3f · 2025-01-15T00:06:22.000+05:30
diff --git a/api/src/main/java/com/cloud/offering/DiskOfferingInfo.java b/api/src/main/java/com/cloud/offering/DiskOfferingInfo.java
@@ -23,6 +23,7 @@ public class DiskOfferingInfo {
     private Long _size;
     private Long _minIops;
     private Long _maxIops;
+    private Long _deviceId;
 
     public DiskOfferingInfo() {
     }
@@ -38,6 +39,11 @@ public DiskOfferingInfo(DiskOffering diskOffering, Long size, Long minIops, Long
         _maxIops = maxIops;
     }
 
+    public DiskOfferingInfo(DiskOffering diskOffering, Long size, Long minIops, Long maxIops, Long deviceId) {
+        this(diskOffering, size, minIops, maxIops);
+        _deviceId = deviceId;
+    }
+
     public void setDiskOffering(DiskOffering diskOffering) {
         _diskOffering = diskOffering;
     }
@@ -69,4 +75,12 @@ public void setMaxIops(Long maxIops) {
     public Long getMaxIops() {
         return _maxIops;
     }
+
+    public void setDeviceId(Long deviceId) {
+        _deviceId = deviceId;
+    }
+
+    public Long getDeviceId() {
+        return _deviceId;
+    }
 }
diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
@@ -146,6 +146,7 @@ public class ApiConstants {
     public static final String DESTINATION_ZONE_ID = "destzoneid";
     public static final String DETAILS = "details";
     public static final String DEVICE_ID = "deviceid";
+    public static final String DEVICE_IDS = "deviceids";
     public static final String DIRECT_DOWNLOAD = "directdownload";
     public static final String DISK = "disk";
     public static final String DISK_OFFERING_ID = "diskofferingid";
diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java
@@ -547,6 +547,7 @@ public List<DiskOfferingInfo> getDataDiskOfferingsInfo() {
             Long size = null;
             Long minIops = null;
             Long maxIops = null;
+            Long deviceId = Long.parseLong(dataDisk.get(ApiConstants.DEVICE_ID));
             if (diskOffering.isCustomized()) {
                 if (dataDisk.get(ApiConstants.SIZE) == null) {
                     throw new InvalidParameterValueException("Size is required for custom disk offering");
@@ -565,7 +566,7 @@ public List<DiskOfferingInfo> getDataDiskOfferingsInfo() {
                 minIops = Long.parseLong(dataDisk.get(ApiConstants.MIN_IOPS));
                 maxIops = Long.parseLong(dataDisk.get(ApiConstants.MAX_IOPS));
             }
-            DiskOfferingInfo diskOfferingInfo = new DiskOfferingInfo(diskOffering, size, minIops, maxIops);
+            DiskOfferingInfo diskOfferingInfo = new DiskOfferingInfo(diskOffering, size, minIops, maxIops, deviceId);
             diskOfferingInfoList.add(diskOfferingInfo);
         }
         this.dataDiskOfferingsInfo = diskOfferingInfoList;
diff --git a/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java b/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java
@@ -161,7 +161,7 @@ public interface BackupManager extends BackupService, Configurable, PluggableSer
 
     List<DiskOfferingInfo> getDataDiskOfferingListFromBackup(Backup backup);
 
-    Map<String, String> getBackupVmDetails(VirtualMachine vm);
+    Map<String, String> getVmDetailsForBackup(VirtualMachine vm);
 
-    Map<String, String> getBackupDiskOfferingDetails(Long vmId);
+    Map<String, String> getDiskOfferingDetailsForBackup(Long vmId);
 }
diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
@@ -520,22 +520,25 @@ public void allocate(final String vmInstanceName, final VirtualMachineTemplate t
             // Create new Volume context and inject event resource type, id and details to generate VOLUME.CREATE event for the ROOT disk.
             CallContext volumeContext = CallContext.register(CallContext.current(), ApiCommandResourceType.Volume);
             try {
-                int diskNumber = 1;
+                Long nextDiskNumber = 1L;
                 if (dataDiskOfferings != null) {
                     for (final DiskOfferingInfo dataDiskOfferingInfo : dataDiskOfferings) {
-                        volumeMgr.allocateRawVolume(Type.DATADISK, "DATA-" + persistedVm.getId() + "-" + String.valueOf(diskNumber), dataDiskOfferingInfo.getDiskOffering(), dataDiskOfferingInfo.getSize(),
-                                dataDiskOfferingInfo.getMinIops(), dataDiskOfferingInfo.getMaxIops(), persistedVm, template, owner, Long.valueOf(diskNumber));
-                        diskNumber++;
+                        Long deviceId = dataDiskOfferingInfo.getDeviceId();
+                        volumeMgr.allocateRawVolume(Type.DATADISK, "DATA-" + persistedVm.getId() + "-" + String.valueOf(deviceId), dataDiskOfferingInfo.getDiskOffering(), dataDiskOfferingInfo.getSize(),
+                                dataDiskOfferingInfo.getMinIops(), dataDiskOfferingInfo.getMaxIops(), persistedVm, template, owner, deviceId);
+                        if (nextDiskNumber <= deviceId) {
+                            nextDiskNumber = deviceId + 1;
+                        }
                     }
                 }
                 if (datadiskTemplateToDiskOfferingMap != null && !datadiskTemplateToDiskOfferingMap.isEmpty()) {
                     for (Entry<Long, DiskOffering> dataDiskTemplateToDiskOfferingMap : datadiskTemplateToDiskOfferingMap.entrySet()) {
                         DiskOffering diskOffering = dataDiskTemplateToDiskOfferingMap.getValue();
                         long diskOfferingSize = diskOffering.getDiskSize() / (1024 * 1024 * 1024);
                         VMTemplateVO dataDiskTemplate = _templateDao.findById(dataDiskTemplateToDiskOfferingMap.getKey());
-                        volumeMgr.allocateRawVolume(Type.DATADISK, "DATA-" + persistedVm.getId() + "-" + String.valueOf(diskNumber), diskOffering, diskOfferingSize, null, null,
-                                persistedVm, dataDiskTemplate, owner, Long.valueOf(diskNumber));
-                        diskNumber++;
+                        volumeMgr.allocateRawVolume(Type.DATADISK, "DATA-" + persistedVm.getId() + "-" + String.valueOf(nextDiskNumber), diskOffering, diskOfferingSize, null, null,
+                                persistedVm, dataDiskTemplate, owner, nextDiskNumber);
+                        nextDiskNumber++;
                     }
                 }
             } finally {
diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/CloudOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/CloudOrchestrator.java
@@ -226,6 +226,7 @@ public VirtualMachineEntity createVirtualMachine(String id, String owner, String
                 DiskOfferingInfo dataDiskOfferingInfo = new DiskOfferingInfo();
 
                 dataDiskOfferingInfo.setDiskOffering(diskOffering);
+                dataDiskOfferingInfo.setDeviceId(1L);
                 dataDiskOfferingInfo.setSize(size);
 
                 if (diskOffering.isCustomizedIops() != null && diskOffering.isCustomizedIops()) {
diff --git a/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupDaoImpl.java
@@ -239,6 +239,7 @@ public BackupResponse newBackupResponse(Backup backup) {
             vmDetails.put(ApiConstants.SERVICE_OFFERING_ID, details.get(ApiConstants.SERVICE_OFFERING_ID));
             vmDetails.put(ApiConstants.NETWORK_IDS, details.get(ApiConstants.NETWORK_IDS));
             vmDetails.put(ApiConstants.DISK_OFFERING_IDS, details.get(ApiConstants.DISK_OFFERING_IDS));
+            vmDetails.put(ApiConstants.DEVICE_IDS, details.get(ApiConstants.DEVICE_IDS));
             vmDetails.put(ApiConstants.DISK_SIZES, details.get(ApiConstants.DISK_SIZES));
             vmDetails.put(ApiConstants.MIN_IOPS, details.get(ApiConstants.MIN_IOPS));
             vmDetails.put(ApiConstants.MAX_IOPS, details.get(ApiConstants.MAX_IOPS));
diff --git a/plugins/backup/dummy/src/main/java/org/apache/cloudstack/backup/DummyBackupProvider.java b/plugins/backup/dummy/src/main/java/org/apache/cloudstack/backup/DummyBackupProvider.java
@@ -130,9 +130,9 @@ public boolean takeBackup(VirtualMachine vm) {
         backup.setDomainId(vm.getDomainId());
         backup.setZoneId(vm.getDataCenterId());
         backup.setBackedUpVolumes(BackupManagerImpl.createVolumeInfoFromVolumes(volumeDao.findByInstance(vm.getId())));
-        Map<String, String> details = backupManager.getBackupVmDetails(vm);
+        Map<String, String> details = backupManager.getVmDetailsForBackup(vm);
         backup.setDetails(details);
-        Map<String, String> diskOfferingDetails = backupManager.getBackupDiskOfferingDetails(vm.getId());
+        Map<String, String> diskOfferingDetails = backupManager.getDiskOfferingDetailsForBackup(vm.getId());
         backup.addDetails(diskOfferingDetails);
 
         return backupDao.persist(backup) != null;
diff --git a/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java b/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java
@@ -183,7 +183,7 @@ public boolean takeBackup(final VirtualMachine vm) {
             backupVO.setSize(answer.getSize());
             backupVO.setStatus(Backup.Status.BackedUp);
             backupVO.setBackedUpVolumes(BackupManagerImpl.createVolumeInfoFromVolumes(volumeDao.findByInstance(vm.getId())));
-            Map<String, String> details = backupManager.getBackupDiskOfferingDetails(vm.getId());
+            Map<String, String> details = backupManager.getDiskOfferingDetailsForBackup(vm.getId());
             backupVO.addDetails(details);
             return backupDao.update(backupVO.getId(), backupVO);
         } else {
@@ -211,7 +211,7 @@ private BackupVO createBackupObject(VirtualMachine vm, String backupPath) {
         backup.setAccountId(vm.getAccountId());
         backup.setDomainId(vm.getDomainId());
         backup.setZoneId(vm.getDataCenterId());
-        Map<String, String> details = backupManager.getBackupVmDetails(vm);
+        Map<String, String> details = backupManager.getVmDetailsForBackup(vm);
         backup.setDetails(details);
 
         return backupDao.persist(backup);
diff --git a/plugins/backup/networker/src/main/java/org/apache/cloudstack/backup/NetworkerBackupProvider.java b/plugins/backup/networker/src/main/java/org/apache/cloudstack/backup/NetworkerBackupProvider.java
@@ -524,9 +524,9 @@ public boolean takeBackup(VirtualMachine vm) {
         BackupVO backup = getClient(vm.getDataCenterId()).registerBackupForVm(vm, backupJobStart, saveTime);
         if (backup != null) {
             backup.setBackedUpVolumes(BackupManagerImpl.createVolumeInfoFromVolumes(volumeDao.findByInstance(vm.getId())));
-            Map<String, String> details = backupManager.getBackupVmDetails(vm);
+            Map<String, String> details = backupManager.getVmDetailsForBackup(vm);
             backup.setDetails(details);
-            details = backupManager.getBackupDiskOfferingDetails(vm.getId());
+            details = backupManager.getDiskOfferingDetailsForBackup(vm.getId());
             backup.addDetails(details);
             backupDao.persist(backup);
             return true;
diff --git a/plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/VeeamBackupProvider.java b/plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/VeeamBackupProvider.java
@@ -384,9 +384,9 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
                         backup.setDomainId(vm.getDomainId());
                         backup.setZoneId(vm.getDataCenterId());
                         backup.setBackedUpVolumes(BackupManagerImpl.createVolumeInfoFromVolumes(volumeDao.findByInstance(vm.getId())));
-                        Map<String, String> details = backupManager.getBackupVmDetails(vm);
+                        Map<String, String> details = backupManager.getVmDetailsForBackup(vm);
                         backup.setDetails(details);
-                        details = backupManager.getBackupDiskOfferingDetails(vm.getId());
+                        details = backupManager.getDiskOfferingDetailsForBackup(vm.getId());
                         backup.addDetails(details);
 
                         logger.debug("Creating a new entry in backups: [id: {}, uuid: {}, name: {}, vm_id: {}, external_id: {}, type: {}, date: {}, backup_offering_id: {}, account_id: {}, "
diff --git a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java
@@ -298,7 +298,7 @@ public boolean deleteBackupOffering(final Long offeringId) {
     }
 
     @Override
-    public Map<String, String> getBackupVmDetails(VirtualMachine vm) {
+    public Map<String, String> getVmDetailsForBackup(VirtualMachine vm) {
         HashMap<String, String> details = new HashMap<>();
         details.put(ApiConstants.HYPERVISOR, vm.getHypervisorType().toString());
         ServiceOffering serviceOffering =  entityManager.findById(ServiceOffering.class, vm.getServiceOfferingId());
@@ -319,28 +319,32 @@ public Map<String, String> getBackupVmDetails(VirtualMachine vm) {
     }
 
     @Override
-    public Map<String, String> getBackupDiskOfferingDetails(Long vmId) {
+    public Map<String, String> getDiskOfferingDetailsForBackup(Long vmId) {
         List<VolumeVO> volumes = volumeDao.findByInstance(vmId);
         List<String> diskOfferingIds = new ArrayList<>();
         List<Long> diskSizes = new ArrayList<>();
         List<Long> minIops = new ArrayList<>();
         List<Long> maxIops = new ArrayList<>();
+        List<Long> deviceIds = new ArrayList<>();
         Map<String, String> details = new HashMap<>();
+
         for (Volume vol : volumes) {
-            if (vol.getVolumeType() != Volume.Type.DATADISK) {
+            if (vol.getVolumeType() != Volume.Type.ROOT && vol.getVolumeType() != Volume.Type.DATADISK) {
                 continue;
             }
             DiskOffering diskOffering = diskOfferingDao.findById(vol.getDiskOfferingId());
             diskOfferingIds.add(diskOffering.getUuid());
             diskSizes.add(vol.getSize());
             minIops.add(vol.getMinIops());
             maxIops.add(vol.getMaxIops());
+            deviceIds.add(vol.getDeviceId());
         }
         if (!diskOfferingIds.isEmpty()) {
             details.put(ApiConstants.DISK_OFFERING_IDS, String.join(",", diskOfferingIds));
             details.put(ApiConstants.DISK_SIZES, String.join(",", diskSizes.stream().map(String::valueOf).collect(Collectors.toList())));
             details.put(ApiConstants.MIN_IOPS, String.join(",", minIops.stream().map(String::valueOf).collect(Collectors.toList())));
             details.put(ApiConstants.MAX_IOPS, String.join(",", maxIops.stream().map(String::valueOf).collect(Collectors.toList())));
+            details.put(ApiConstants.DEVICE_IDS, String.join(",", deviceIds.stream().map(String::valueOf).collect(Collectors.toList())));
         }
         return details;
     }
@@ -823,6 +827,7 @@ public List<DiskOfferingInfo> getDataDiskOfferingListFromBackup(Backup backup) {
         List<Long> diskSizes;
         List<Long> minIopsList;
         List<Long> maxIopsList;
+        List<Long> deviceIds;
 
         String diskOfferingIds = backup.getDetail(ApiConstants.DISK_OFFERING_IDS);
         if (diskOfferingIds == null) {
@@ -841,16 +846,24 @@ public List<DiskOfferingInfo> getDataDiskOfferingListFromBackup(Backup backup) {
         maxIopsList = Stream.of(backup.getDetail(ApiConstants.MAX_IOPS).split(","))
                 .map(s -> "null".equals(s) ? null : Long.valueOf(s))
                 .collect(Collectors.toList());
+        deviceIds = Stream.of(backup.getDetail(ApiConstants.DEVICE_IDS).split(","))
+                .map(Long::valueOf)
+                .collect(Collectors.toList());
 
         int index = 0;
         for (DiskOfferingVO diskOffering : diskOfferings) {
+            Long deviceId = deviceIds.get(index);
+            if (deviceId == 0) {
+                index++;
+                continue;
+            }
             Long size = diskOffering.isCustomized() ? diskSizes.get(index) : diskOffering.getDiskSize();
             size = size / (1024 * 1024 * 1024);
             Long minIops = (diskOffering.isCustomizedIops() != null && diskOffering.isCustomizedIops()) ?
                     minIopsList.get(index) : null;
             Long maxIops = (diskOffering.isCustomizedIops() != null && diskOffering.isCustomizedIops()) ?
                     maxIopsList.get(index) : null;
-            diskOfferingInfoList.add(new DiskOfferingInfo(diskOffering, size, minIops, maxIops));
+            diskOfferingInfoList.add(new DiskOfferingInfo(diskOffering, size, minIops, maxIops, deviceId));
             index++;
         }
 
diff --git a/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java b/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java
diff --git a/ui/src/views/compute/DeployVMFromBackup.vue b/ui/src/views/compute/DeployVMFromBackup.vue
diff --git a/ui/src/views/compute/wizard/VolumeDiskOfferingMap.vue b/ui/src/views/compute/wizard/VolumeDiskOfferingMap.vue
diff --git a/ui/src/views/storage/CreateVMFromBackup.vue b/ui/src/views/storage/CreateVMFromBackup.vue

Original file line number	Diff line number	Diff line change
`@@ -23,6 +23,7 @@ public class DiskOfferingInfo {`
`23`	`23`	`private Long _size;`
`24`	`24`	`private Long _minIops;`
`25`	`25`	`private Long _maxIops;`
	`26`	`+ private Long _deviceId;`
`26`	`27`
`27`	`28`	`public DiskOfferingInfo() {`
`28`	`29`	`}`
`@@ -38,6 +39,11 @@ public DiskOfferingInfo(DiskOffering diskOffering, Long size, Long minIops, Long`
`38`	`39`	`_maxIops = maxIops;`
`39`	`40`	`}`
`40`	`41`
	`42`	`+ public DiskOfferingInfo(DiskOffering diskOffering, Long size, Long minIops, Long maxIops, Long deviceId) {`
	`43`	`+ this(diskOffering, size, minIops, maxIops);`
	`44`	`+ _deviceId = deviceId;`
	`45`	`+ }`
	`46`	`+`
`41`	`47`	`public void setDiskOffering(DiskOffering diskOffering) {`
`42`	`48`	`_diskOffering = diskOffering;`
`43`	`49`	`}`
`@@ -69,4 +75,12 @@ public void setMaxIops(Long maxIops) {`
`69`	`75`	`public Long getMaxIops() {`
`70`	`76`	`return _maxIops;`
`71`	`77`	`}`
	`78`	`+`
	`79`	`+ public void setDeviceId(Long deviceId) {`
	`80`	`+ _deviceId = deviceId;`
	`81`	`+ }`
	`82`	`+`
	`83`	`+ public Long getDeviceId() {`
	`84`	`+ return _deviceId;`
	`85`	`+ }`
`72`	`86`	`}`
Original file line number	Diff line number	Diff line change
`@@ -161,7 +161,7 @@ public interface BackupManager extends BackupService, Configurable, PluggableSer`
`161`	`161`
`162`	`162`	`List<DiskOfferingInfo> getDataDiskOfferingListFromBackup(Backup backup);`
`163`	`163`
`164`		`- Map<String, String> getBackupVmDetails(VirtualMachine vm);`
	`164`	`+ Map<String, String> getVmDetailsForBackup(VirtualMachine vm);`
`165`	`165`
`166`		`- Map<String, String> getBackupDiskOfferingDetails(Long vmId);`
	`166`	`+ Map<String, String> getDiskOfferingDetailsForBackup(Long vmId);`
`167`	`167`	`}`