Skip to content

Commit 9b3e82d

Browse files
abh1sarDajeong-Park
authored andcommitted
Fix restore from NAS backup when datadisk is older than the root disk. (apache#11258)
1 parent 9b6a991 commit 9b3e82d

File tree

3 files changed

+44
-39
lines changed

3 files changed

+44
-39
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
@@ -2715,7 +2715,9 @@ public Volume attachVolumeToVM(Long vmId, Long volumeId, Long deviceId, Boolean
27152715

27162716
excludeLocalStorageIfNeeded(volumeToAttach);
27172717

2718-
checkForDevicesInCopies(vmId, vm);
2718+
checkForVMSnapshots(vmId, vm);
2719+
2720+
checkForBackups(vm, true);
27192721

27202722
checkRightsToAttach(caller, volumeToAttach, vm);
27212723

@@ -2814,18 +2816,12 @@ private void checkRightsToAttach(Account caller, VolumeInfo volumeToAttach, User
28142816
}
28152817
}
28162818

2817-
private void checkForDevicesInCopies(Long vmId, UserVmVO vm) {
2819+
private void checkForVMSnapshots(Long vmId, UserVmVO vm) {
28182820
// if target VM has associated VM snapshots
28192821
List<VMSnapshotVO> vmSnapshots = _vmSnapshotDao.findByVm(vmId);
28202822
if (vmSnapshots.size() > 0) {
28212823
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()));
28222824
}
2823-
2824-
// if target VM has backups
2825-
List<Backup> backups = backupDao.listByVmId(vm.getDataCenterId(), vm.getId());
2826-
if (vm.getBackupOfferingId() != null && !backups.isEmpty()) {
2827-
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()));
2828-
}
28292825
}
28302826

28312827
/**
@@ -2925,7 +2921,7 @@ private void checkDeviceId(Long deviceId, VolumeInfo volumeToAttach, UserVmVO vm
29252921
return volumeToAttach;
29262922
}
29272923

2928-
protected void validateIfVmHasBackups(UserVmVO vm, boolean attach) {
2924+
protected void checkForBackups(UserVmVO vm, boolean attach) {
29292925
if ((vm.getBackupOfferingId() == null || CollectionUtils.isEmpty(vm.getBackupVolumeList())) || BooleanUtils.isTrue(BackupManager.BackupEnableAttachDetachVolumes.value())) {
29302926
return;
29312927
}
@@ -3169,7 +3165,7 @@ public Volume detachVolumeFromVM(DetachVolumeCmd cmmd) {
31693165
throw new InvalidParameterValueException("Unable to detach volume, please specify a VM that does not have VM snapshots");
31703166
}
31713167

3172-
validateIfVmHasBackups(vm, false);
3168+
checkForBackups(vm, false);
31733169

31743170
AsyncJobExecutionContext asyncExecutionContext = AsyncJobExecutionContext.getCurrentExecutionContext();
31753171
if (asyncExecutionContext != null) {

0 commit comments

Comments
 (0)