mirror of
				https://github.com/apache/cloudstack.git
				synced 2025-10-26 08:42:29 +01:00 
			
		
		
		
	Fix restore from NAS backup when datadisk is older than the root disk. (#11258)
This commit is contained in:
		
							parent
							
								
									0ebf72df0f
								
							
						
					
					
						commit
						1b74c2dd3f
					
				| @ -51,6 +51,7 @@ import javax.inject.Inject; | ||||
| import java.text.SimpleDateFormat; | ||||
| import java.util.ArrayList; | ||||
| import java.util.Collections; | ||||
| import java.util.Comparator; | ||||
| import java.util.Date; | ||||
| import java.util.List; | ||||
| import java.util.Locale; | ||||
| @ -162,6 +163,7 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co | ||||
| 
 | ||||
|         if (VirtualMachine.State.Stopped.equals(vm.getState())) { | ||||
|             List<VolumeVO> vmVolumes = volumeDao.findByInstance(vm.getId()); | ||||
|             vmVolumes.sort(Comparator.comparing(Volume::getDeviceId)); | ||||
|             List<String> volumePaths = getVolumePaths(vmVolumes); | ||||
|             command.setVolumePaths(volumePaths); | ||||
|         } | ||||
| @ -212,7 +214,10 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co | ||||
|     @Override | ||||
|     public boolean restoreVMFromBackup(VirtualMachine vm, Backup backup) { | ||||
|         List<Backup.VolumeInfo> backedVolumes = backup.getBackedUpVolumes(); | ||||
|         List<VolumeVO> volumes = backedVolumes.stream().map(volume -> volumeDao.findByUuid(volume.getUuid())).collect(Collectors.toList()); | ||||
|         List<VolumeVO> volumes = backedVolumes.stream() | ||||
|                 .map(volume -> volumeDao.findByUuid(volume.getUuid())) | ||||
|                 .sorted((v1, v2) -> Long.compare(v1.getDeviceId(), v2.getDeviceId())) | ||||
|                 .collect(Collectors.toList()); | ||||
| 
 | ||||
|         LOG.debug("Restoring vm {} from backup {} on the NAS Backup Provider", vm, backup); | ||||
|         BackupRepository backupRepository = getBackupRepository(vm, backup); | ||||
|  | ||||
| @ -62,6 +62,7 @@ public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper<RestoreBa | ||||
|         String restoreVolumeUuid = command.getRestoreVolumeUUID(); | ||||
| 
 | ||||
|         String newVolumeId = null; | ||||
|         try { | ||||
|             if (Objects.isNull(vmExists)) { | ||||
|                 String volumePath = volumePaths.get(0); | ||||
|                 int lastIndex = volumePath.lastIndexOf("/"); | ||||
| @ -73,6 +74,14 @@ public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper<RestoreBa | ||||
|             } else { | ||||
|                 restoreVolumesOfDestroyedVMs(volumePaths, vmName, backupPath, backupRepoType, backupRepoAddress, mountOptions); | ||||
|             } | ||||
|         } catch (CloudRuntimeException e) { | ||||
|             String errorMessage = "Failed to restore backup for VM: " + vmName + "."; | ||||
|             if (e.getMessage() != null && !e.getMessage().isEmpty()) { | ||||
|                 errorMessage += " Details: " + e.getMessage(); | ||||
|             } | ||||
|             logger.error(errorMessage); | ||||
|             return new BackupAnswer(command, false, errorMessage); | ||||
|         } | ||||
| 
 | ||||
|         return new BackupAnswer(command, true, newVolumeId); | ||||
|     } | ||||
| @ -86,10 +95,8 @@ public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper<RestoreBa | ||||
|                 String volumePath = volumePaths.get(idx); | ||||
|                 Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, null); | ||||
|                 diskType = "datadisk"; | ||||
|                 try { | ||||
|                     replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first()); | ||||
|                 } catch (IOException e) { | ||||
|                     throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e); | ||||
|                 if (!replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first())) { | ||||
|                     throw new CloudRuntimeException(String.format("Unable to restore backup for volume [%s].", bkpPathAndVolUuid.second())); | ||||
|                 } | ||||
|             } | ||||
|         } finally { | ||||
| @ -108,10 +115,8 @@ public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper<RestoreBa | ||||
|                 String volumePath = volumePaths.get(i); | ||||
|                 Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, null); | ||||
|                 diskType = "datadisk"; | ||||
|                 try { | ||||
|                     replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first()); | ||||
|                 } catch (IOException e) { | ||||
|                     throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e); | ||||
|                 if (!replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first())) { | ||||
|                     throw new CloudRuntimeException(String.format("Unable to restore backup for volume [%s].", bkpPathAndVolUuid.second())); | ||||
|                 } | ||||
|             } | ||||
|         } finally { | ||||
| @ -126,16 +131,14 @@ public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper<RestoreBa | ||||
|         Pair<String, String> bkpPathAndVolUuid; | ||||
|         try { | ||||
|             bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, volumeUUID); | ||||
|             try { | ||||
|                 replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first()); | ||||
|             if (!replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first())) { | ||||
|                 throw new CloudRuntimeException(String.format("Unable to restore backup for volume [%s].", bkpPathAndVolUuid.second())); | ||||
|             } | ||||
|             if (VirtualMachine.State.Running.equals(vmNameAndState.second())) { | ||||
|                 if (!attachVolumeToVm(vmNameAndState.first(), volumePath)) { | ||||
|                     throw new CloudRuntimeException(String.format("Failed to attach volume to VM: %s", vmNameAndState.first())); | ||||
|                 } | ||||
|             } | ||||
|             } catch (IOException e) { | ||||
|                 throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e); | ||||
|             } | ||||
|         } catch (Exception e) { | ||||
|             throw new CloudRuntimeException("Failed to restore volume", e); | ||||
|         } finally { | ||||
| @ -194,8 +197,9 @@ public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper<RestoreBa | ||||
|         return new Pair<>(bkpPath, volUuid); | ||||
|     } | ||||
| 
 | ||||
|     private void replaceVolumeWithBackup(String volumePath, String backupPath) throws IOException { | ||||
|         Script.runSimpleBashScript(String.format(RSYNC_COMMAND, backupPath, volumePath)); | ||||
|     private boolean replaceVolumeWithBackup(String volumePath, String backupPath) { | ||||
|         int exitValue = Script.runSimpleBashScriptForExitValue(String.format(RSYNC_COMMAND, backupPath, volumePath)); | ||||
|         return exitValue == 0; | ||||
|     } | ||||
| 
 | ||||
|     private boolean attachVolumeToVm(String vmName, String volumePath) { | ||||
|  | ||||
| @ -2605,7 +2605,9 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic | ||||
| 
 | ||||
|         excludeLocalStorageIfNeeded(volumeToAttach); | ||||
| 
 | ||||
|         checkForDevicesInCopies(vmId, vm); | ||||
|         checkForVMSnapshots(vmId, vm); | ||||
| 
 | ||||
|         checkForBackups(vm, true); | ||||
| 
 | ||||
|         checkRightsToAttach(caller, volumeToAttach, vm); | ||||
| 
 | ||||
| @ -2707,18 +2709,12 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic | ||||
|         } | ||||
|     } | ||||
| 
 | ||||
|     private void checkForDevicesInCopies(Long vmId, UserVmVO vm) { | ||||
|     private void checkForVMSnapshots(Long vmId, UserVmVO vm) { | ||||
|         // if target VM has associated VM snapshots | ||||
|         List<VMSnapshotVO> vmSnapshots = _vmSnapshotDao.findByVm(vmId); | ||||
|         if (vmSnapshots.size() > 0) { | ||||
|             throw new InvalidParameterValueException(String.format("Unable to attach volume to VM %s/%s, please specify a VM that does not have VM snapshots", vm.getName(), vm.getUuid())); | ||||
|         } | ||||
| 
 | ||||
|         // if target VM has backups | ||||
|         List<Backup> backups = backupDao.listByVmId(vm.getDataCenterId(), vm.getId()); | ||||
|         if (vm.getBackupOfferingId() != null && !backups.isEmpty()) { | ||||
|             throw new InvalidParameterValueException(String.format("Unable to attach volume to VM %s/%s, please specify a VM that does not have any backups", vm.getName(), vm.getUuid())); | ||||
|         } | ||||
|     } | ||||
| 
 | ||||
|     /** | ||||
| @ -2818,7 +2814,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic | ||||
|         return volumeToAttach; | ||||
|     } | ||||
| 
 | ||||
|     protected void validateIfVmHasBackups(UserVmVO vm, boolean attach) { | ||||
|     protected void checkForBackups(UserVmVO vm, boolean attach) { | ||||
|         if ((vm.getBackupOfferingId() == null || CollectionUtils.isEmpty(vm.getBackupVolumeList())) || BooleanUtils.isTrue(BackupManager.BackupEnableAttachDetachVolumes.value())) { | ||||
|             return; | ||||
|         } | ||||
| @ -3038,7 +3034,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic | ||||
|             throw new InvalidParameterValueException("Unable to detach volume, please specify a VM that does not have VM snapshots"); | ||||
|         } | ||||
| 
 | ||||
|         validateIfVmHasBackups(vm, false); | ||||
|         checkForBackups(vm, false); | ||||
| 
 | ||||
|         AsyncJobExecutionContext asyncExecutionContext = AsyncJobExecutionContext.getCurrentExecutionContext(); | ||||
|         if (asyncExecutionContext != null) { | ||||
|  | ||||
| @ -19,6 +19,7 @@ package org.apache.cloudstack.backup; | ||||
| import java.util.ArrayList; | ||||
| import java.util.Arrays; | ||||
| import java.util.Collections; | ||||
| import java.util.Comparator; | ||||
| import java.util.Date; | ||||
| import java.util.HashMap; | ||||
| import java.util.List; | ||||
| @ -277,6 +278,7 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { | ||||
| 
 | ||||
|     public static String createVolumeInfoFromVolumes(List<VolumeVO> vmVolumes) { | ||||
|         List<Backup.VolumeInfo> list = new ArrayList<>(); | ||||
|         vmVolumes.sort(Comparator.comparing(VolumeVO::getDeviceId)); | ||||
|         for (VolumeVO vol : vmVolumes) { | ||||
|             list.add(new Backup.VolumeInfo(vol.getUuid(), vol.getPath(), vol.getVolumeType(), vol.getSize())); | ||||
|         } | ||||
|  | ||||
| @ -666,7 +666,6 @@ public class VolumeApiServiceImplTest { | ||||
|         when(vm.getState()).thenReturn(State.Running); | ||||
|         when(vm.getDataCenterId()).thenReturn(34L); | ||||
|         when(vm.getBackupOfferingId()).thenReturn(null); | ||||
|         when(backupDaoMock.listByVmId(anyLong(), anyLong())).thenReturn(Collections.emptyList()); | ||||
|         when(volumeDaoMock.findByInstanceAndType(anyLong(), any(Volume.Type.class))).thenReturn(new ArrayList<>(10)); | ||||
|         when(volumeDataFactoryMock.getVolume(9L)).thenReturn(volumeToAttach); | ||||
|         when(volumeToAttach.getState()).thenReturn(Volume.State.Uploaded); | ||||
| @ -1305,7 +1304,7 @@ public class VolumeApiServiceImplTest { | ||||
|         try { | ||||
|             UserVmVO vm = Mockito.mock(UserVmVO.class); | ||||
|             when(vm.getBackupOfferingId()).thenReturn(1l); | ||||
|             volumeApiServiceImpl.validateIfVmHasBackups(vm, false); | ||||
|             volumeApiServiceImpl.checkForBackups(vm, false); | ||||
|         } catch (Exception e) { | ||||
|             Assert.assertEquals("Unable to detach volume, cannot detach volume from a VM that has backups. First remove the VM from the backup offering or set the global configuration 'backup.enable.attach.detach.of.volumes' to true.", e.getMessage()); | ||||
|         } | ||||
| @ -1316,7 +1315,7 @@ public class VolumeApiServiceImplTest { | ||||
|         try { | ||||
|             UserVmVO vm = Mockito.mock(UserVmVO.class); | ||||
|             when(vm.getBackupOfferingId()).thenReturn(1l); | ||||
|             volumeApiServiceImpl.validateIfVmHasBackups(vm, true); | ||||
|             volumeApiServiceImpl.checkForBackups(vm, true); | ||||
|         } catch (Exception e) { | ||||
|             Assert.assertEquals("Unable to attach volume, please specify a VM that does not have any backups or set the global configuration 'backup.enable.attach.detach.of.volumes' to true.", e.getMessage()); | ||||
|         } | ||||
| @ -1326,7 +1325,7 @@ public class VolumeApiServiceImplTest { | ||||
|     public void validateIfVmHaveBackupsTestSuccessWhenVMDontHaveBackupOffering() { | ||||
|         UserVmVO vm = Mockito.mock(UserVmVO.class); | ||||
|         when(vm.getBackupOfferingId()).thenReturn(null); | ||||
|         volumeApiServiceImpl.validateIfVmHasBackups(vm, true); | ||||
|         volumeApiServiceImpl.checkForBackups(vm, true); | ||||
|     } | ||||
| 
 | ||||
|     @Test | ||||
|  | ||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user