Skip to content

Commit 1b74c2d

Browse files
abh1sarabh1sar
authored
Fix restore from NAS backup when datadisk is older than the root disk. (#11258)
1 parent 0ebf72d commit 1b74c2d

File tree

5 files changed

+49
-43
lines changed

5 files changed

+49
-43
lines changed

‎plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
import java.text.SimpleDateFormat;
5252
import java.util.ArrayList;
5353
import java.util.Collections;
54+
import java.util.Comparator;
5455
import java.util.Date;
5556
import java.util.List;
5657
import java.util.Locale;
@@ -162,6 +163,7 @@ public boolean takeBackup(final VirtualMachine vm) {
162163

163164
if (VirtualMachine.State.Stopped.equals(vm.getState())) {
164165
List<VolumeVO> vmVolumes = volumeDao.findByInstance(vm.getId());
166+
vmVolumes.sort(Comparator.comparing(Volume::getDeviceId));
165167
List<String> volumePaths = getVolumePaths(vmVolumes);
166168
command.setVolumePaths(volumePaths);
167169
}
@@ -212,7 +214,10 @@ private BackupVO createBackupObject(VirtualMachine vm, String backupPath) {
212214
@Override
213215
public boolean restoreVMFromBackup(VirtualMachine vm, Backup backup) {
214216
List<Backup.VolumeInfo> backedVolumes = backup.getBackedUpVolumes();
215-
List<VolumeVO> volumes = backedVolumes.stream().map(volume -> volumeDao.findByUuid(volume.getUuid())).collect(Collectors.toList());
217+
List<VolumeVO> volumes = backedVolumes.stream()
218+
.map(volume -> volumeDao.findByUuid(volume.getUuid()))
219+
.sorted((v1, v2) -> Long.compare(v1.getDeviceId(), v2.getDeviceId()))
220+
.collect(Collectors.toList());
216221

217222
LOG.debug("Restoring vm {} from backup {} on the NAS Backup Provider", vm, backup);
218223
BackupRepository backupRepository = getBackupRepository(vm, backup);

‎plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java‎

Lines changed: 32 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -62,16 +62,25 @@ public Answer execute(RestoreBackupCommand command, LibvirtComputingResource ser
6262
String restoreVolumeUuid = command.getRestoreVolumeUUID();
6363

6464
String newVolumeId = null;
65-
if (Objects.isNull(vmExists)) {
66-
String volumePath = volumePaths.get(0);
67-
int lastIndex = volumePath.lastIndexOf("/");
68-
newVolumeId = volumePath.substring(lastIndex + 1);
69-
restoreVolume(backupPath, backupRepoType, backupRepoAddress, volumePath, diskType, restoreVolumeUuid,
70-
new Pair<>(vmName, command.getVmState()), mountOptions);
71-
} else if (Boolean.TRUE.equals(vmExists)) {
72-
restoreVolumesOfExistingVM(volumePaths, backupPath, backupRepoType, backupRepoAddress, mountOptions);
73-
} else {
74-
restoreVolumesOfDestroyedVMs(volumePaths, vmName, backupPath, backupRepoType, backupRepoAddress, mountOptions);
65+
try {
66+
if (Objects.isNull(vmExists)) {
67+
String volumePath = volumePaths.get(0);
68+
int lastIndex = volumePath.lastIndexOf("/");
69+
newVolumeId = volumePath.substring(lastIndex + 1);
70+
restoreVolume(backupPath, backupRepoType, backupRepoAddress, volumePath, diskType, restoreVolumeUuid,
71+
new Pair<>(vmName, command.getVmState()), mountOptions);
72+
} else if (Boolean.TRUE.equals(vmExists)) {
73+
restoreVolumesOfExistingVM(volumePaths, backupPath, backupRepoType, backupRepoAddress, mountOptions);
74+
} else {
75+
restoreVolumesOfDestroyedVMs(volumePaths, vmName, backupPath, backupRepoType, backupRepoAddress, mountOptions);
76+
}
77+
} catch (CloudRuntimeException e) {
78+
String errorMessage = "Failed to restore backup for VM: " + vmName + ".";
79+
if (e.getMessage() != null && !e.getMessage().isEmpty()) {
80+
errorMessage += " Details: " + e.getMessage();
81+
}
82+
logger.error(errorMessage);
83+
return new BackupAnswer(command, false, errorMessage);
7584
}
7685

7786
return new BackupAnswer(command, true, newVolumeId);
@@ -86,10 +95,8 @@ private void restoreVolumesOfExistingVM(List<String> volumePaths, String backupP
8695
String volumePath = volumePaths.get(idx);
8796
Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, null);
8897
diskType = "datadisk";
89-
try {
90-
replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
91-
} catch (IOException e) {
92-
throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);
98+
if (!replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first())) {
99+
throw new CloudRuntimeException(String.format("Unable to restore backup for volume [%s].", bkpPathAndVolUuid.second()));
93100
}
94101
}
95102
} finally {
@@ -108,10 +115,8 @@ private void restoreVolumesOfDestroyedVMs(List<String> volumePaths, String vmNam
108115
String volumePath = volumePaths.get(i);
109116
Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, null);
110117
diskType = "datadisk";
111-
try {
112-
replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
113-
} catch (IOException e) {
114-
throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);
118+
if (!replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first())) {
119+
throw new CloudRuntimeException(String.format("Unable to restore backup for volume [%s].", bkpPathAndVolUuid.second()));
115120
}
116121
}
117122
} finally {
@@ -126,15 +131,13 @@ private void restoreVolume(String backupPath, String backupRepoType, String back
126131
Pair<String, String> bkpPathAndVolUuid;
127132
try {
128133
bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, volumeUUID);
129-
try {
130-
replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
131-
if (VirtualMachine.State.Running.equals(vmNameAndState.second())) {
132-
if (!attachVolumeToVm(vmNameAndState.first(), volumePath)) {
133-
throw new CloudRuntimeException(String.format("Failed to attach volume to VM: %s", vmNameAndState.first()));
134-
}
134+
if (!replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first())) {
135+
throw new CloudRuntimeException(String.format("Unable to restore backup for volume [%s].", bkpPathAndVolUuid.second()));
136+
}
137+
if (VirtualMachine.State.Running.equals(vmNameAndState.second())) {
138+
if (!attachVolumeToVm(vmNameAndState.first(), volumePath)) {
139+
throw new CloudRuntimeException(String.format("Failed to attach volume to VM: %s", vmNameAndState.first()));
135140
}
136-
} catch (IOException e) {
137-
throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);
138141
}
139142
} catch (Exception e) {
140143
throw new CloudRuntimeException("Failed to restore volume", e);
@@ -194,8 +197,9 @@ private Pair<String, String> getBackupPath(String mountDirectory, String volumeP
194197
return new Pair<>(bkpPath, volUuid);
195198
}
196199

197-
private void replaceVolumeWithBackup(String volumePath, String backupPath) throws IOException {
198-
Script.runSimpleBashScript(String.format(RSYNC_COMMAND, backupPath, volumePath));
200+
private boolean replaceVolumeWithBackup(String volumePath, String backupPath) {
201+
int exitValue = Script.runSimpleBashScriptForExitValue(String.format(RSYNC_COMMAND, backupPath, volumePath));
202+
return exitValue == 0;
199203
}
200204

201205
private boolean attachVolumeToVm(String vmName, String volumePath) {

‎server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2605,7 +2605,9 @@ public Volume attachVolumeToVM(Long vmId, Long volumeId, Long deviceId, Boolean
26052605

26062606
excludeLocalStorageIfNeeded(volumeToAttach);
26072607

2608-
checkForDevicesInCopies(vmId, vm);
2608+
checkForVMSnapshots(vmId, vm);
2609+
2610+
checkForBackups(vm, true);
26092611

26102612
checkRightsToAttach(caller, volumeToAttach, vm);
26112613

@@ -2707,18 +2709,12 @@ private void checkRightsToAttach(Account caller, VolumeInfo volumeToAttach, User
27072709
}
27082710
}
27092711

2710-
private void checkForDevicesInCopies(Long vmId, UserVmVO vm) {
2712+
private void checkForVMSnapshots(Long vmId, UserVmVO vm) {
27112713
// if target VM has associated VM snapshots
27122714
List<VMSnapshotVO> vmSnapshots = _vmSnapshotDao.findByVm(vmId);
27132715
if (vmSnapshots.size() > 0) {
27142716
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()));
27152717
}
2716-
2717-
// if target VM has backups
2718-
List<Backup> backups = backupDao.listByVmId(vm.getDataCenterId(), vm.getId());
2719-
if (vm.getBackupOfferingId() != null && !backups.isEmpty()) {
2720-
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()));
2721-
}
27222718
}
27232719

27242720
/**
@@ -2818,7 +2814,7 @@ private void checkDeviceId(Long deviceId, VolumeInfo volumeToAttach, UserVmVO vm
28182814
return volumeToAttach;
28192815
}
28202816

2821-
protected void validateIfVmHasBackups(UserVmVO vm, boolean attach) {
2817+
protected void checkForBackups(UserVmVO vm, boolean attach) {
28222818
if ((vm.getBackupOfferingId() == null || CollectionUtils.isEmpty(vm.getBackupVolumeList())) || BooleanUtils.isTrue(BackupManager.BackupEnableAttachDetachVolumes.value())) {
28232819
return;
28242820
}
@@ -3038,7 +3034,7 @@ public Volume detachVolumeFromVM(DetachVolumeCmd cmmd) {
30383034
throw new InvalidParameterValueException("Unable to detach volume, please specify a VM that does not have VM snapshots");
30393035
}
30403036

3041-
validateIfVmHasBackups(vm, false);
3037+
checkForBackups(vm, false);
30423038

30433039
AsyncJobExecutionContext asyncExecutionContext = AsyncJobExecutionContext.getCurrentExecutionContext();
30443040
if (asyncExecutionContext != null) {

‎server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.util.ArrayList;
2020
import java.util.Arrays;
2121
import java.util.Collections;
22+
import java.util.Comparator;
2223
import java.util.Date;
2324
import java.util.HashMap;
2425
import java.util.List;
@@ -277,6 +278,7 @@ public boolean deleteBackupOffering(final Long offeringId) {
277278

278279
public static String createVolumeInfoFromVolumes(List<VolumeVO> vmVolumes) {
279280
List<Backup.VolumeInfo> list = new ArrayList<>();
281+
vmVolumes.sort(Comparator.comparing(VolumeVO::getDeviceId));
280282
for (VolumeVO vol : vmVolumes) {
281283
list.add(new Backup.VolumeInfo(vol.getUuid(), vol.getPath(), vol.getVolumeType(), vol.getSize()));
282284
}

‎server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -666,7 +666,6 @@ public void testResourceLimitCheckForUploadedVolume() throws NoSuchFieldExceptio
666666
when(vm.getState()).thenReturn(State.Running);
667667
when(vm.getDataCenterId()).thenReturn(34L);
668668
when(vm.getBackupOfferingId()).thenReturn(null);
669-
when(backupDaoMock.listByVmId(anyLong(), anyLong())).thenReturn(Collections.emptyList());
670669
when(volumeDaoMock.findByInstanceAndType(anyLong(), any(Volume.Type.class))).thenReturn(new ArrayList<>(10));
671670
when(volumeDataFactoryMock.getVolume(9L)).thenReturn(volumeToAttach);
672671
when(volumeToAttach.getState()).thenReturn(Volume.State.Uploaded);
@@ -1305,7 +1304,7 @@ public void validateIfVmHaveBackupsTestExceptionWhenTryToDetachVolumeFromVMWhich
13051304
try {
13061305
UserVmVO vm = Mockito.mock(UserVmVO.class);
13071306
when(vm.getBackupOfferingId()).thenReturn(1l);
1308-
volumeApiServiceImpl.validateIfVmHasBackups(vm, false);
1307+
volumeApiServiceImpl.checkForBackups(vm, false);
13091308
} catch (Exception e) {
13101309
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());
13111310
}
@@ -1316,7 +1315,7 @@ public void validateIfVmHaveBackupsTestExceptionWhenTryToAttachVolumeFromVMWhich
13161315
try {
13171316
UserVmVO vm = Mockito.mock(UserVmVO.class);
13181317
when(vm.getBackupOfferingId()).thenReturn(1l);
1319-
volumeApiServiceImpl.validateIfVmHasBackups(vm, true);
1318+
volumeApiServiceImpl.checkForBackups(vm, true);
13201319
} catch (Exception e) {
13211320
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());
13221321
}
@@ -1326,7 +1325,7 @@ public void validateIfVmHaveBackupsTestExceptionWhenTryToAttachVolumeFromVMWhich
13261325
public void validateIfVmHaveBackupsTestSuccessWhenVMDontHaveBackupOffering() {
13271326
UserVmVO vm = Mockito.mock(UserVmVO.class);
13281327
when(vm.getBackupOfferingId()).thenReturn(null);
1329-
volumeApiServiceImpl.validateIfVmHasBackups(vm, true);
1328+
volumeApiServiceImpl.checkForBackups(vm, true);
13301329
}
13311330

13321331
@Test

0 commit comments

Comments
 (0)