Skip to content

Commit a626760

Browse files
abh1sarPearl1594
authored andcommitted
Fix backup path (after migration) for single volume restore and refactor
1 parent 6f23a2d commit a626760

File tree

5 files changed

+77
-90
lines changed

5 files changed

+77
-90
lines changed

core/src/main/java/org/apache/cloudstack/backup/RestoreBackupCommand.java

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -23,17 +23,17 @@
2323
import com.cloud.agent.api.LogLevel;
2424
import com.cloud.vm.VirtualMachine;
2525

26-
import java.util.Map;
26+
import java.util.List;
2727

2828
public class RestoreBackupCommand extends Command {
2929
private String vmName;
3030
private String backupPath;
3131
private String backupRepoType;
3232
private String backupRepoAddress;
33-
private Map<String, String> volumePathsAndUuids;
33+
private List<String> volumePaths;
34+
private List<String> backupFiles;
3435
private String diskType;
3536
private Boolean vmExists;
36-
private String restoreVolumeUUID;
3737
private VirtualMachine.State vmState;
3838

3939
protected RestoreBackupCommand() {
@@ -72,12 +72,20 @@ public void setBackupRepoAddress(String backupRepoAddress) {
7272
this.backupRepoAddress = backupRepoAddress;
7373
}
7474

75-
public Map<String, String> getVolumePathsAndUuids() {
76-
return volumePathsAndUuids;
75+
public List<String> getVolumePaths() {
76+
return volumePaths;
7777
}
7878

79-
public void setVolumePathsAndUuids(Map<String, String> volumePathsAndUuids) {
80-
this.volumePathsAndUuids = volumePathsAndUuids;
79+
public void setVolumePaths(List<String> volumePaths) {
80+
this.volumePaths = volumePaths;
81+
}
82+
83+
public List<String> getBackupFiles() {
84+
return backupFiles;
85+
}
86+
87+
public void setBackupFiles(List<String> backupFiles) {
88+
this.backupFiles = backupFiles;
8189
}
8290

8391
public Boolean isVmExists() {
@@ -104,14 +112,6 @@ public void setMountOptions(String mountOptions) {
104112
this.mountOptions = mountOptions;
105113
}
106114

107-
public String getRestoreVolumeUUID() {
108-
return restoreVolumeUUID;
109-
}
110-
111-
public void setRestoreVolumeUUID(String restoreVolumeUUID) {
112-
this.restoreVolumeUUID = restoreVolumeUUID;
113-
}
114-
115115
public VirtualMachine.State getVmState() {
116116
return vmState;
117117
}

core/src/main/java/org/apache/cloudstack/backup/TakeBackupCommand.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,14 @@
2222
import com.cloud.agent.api.Command;
2323
import com.cloud.agent.api.LogLevel;
2424

25-
import java.util.Map;
25+
import java.util.List;
2626

2727
public class TakeBackupCommand extends Command {
2828
private String vmName;
2929
private String backupPath;
3030
private String backupRepoType;
3131
private String backupRepoAddress;
32-
private Map<String, String> volumePathsAndUuids;
32+
private List<String> volumePaths;
3333
@LogLevel(LogLevel.Log4jLevel.Off)
3434
private String mountOptions;
3535

@@ -79,12 +79,12 @@ public void setMountOptions(String mountOptions) {
7979
this.mountOptions = mountOptions;
8080
}
8181

82-
public Map<String, String> getVolumePathsAndUuids() {
83-
return volumePathsAndUuids;
82+
public List<String> getVolumePaths() {
83+
return volumePaths;
8484
}
8585

86-
public void setVolumePathsAndUuids(Map<String, String> volumePathsAndUuids) {
87-
this.volumePathsAndUuids = volumePathsAndUuids;
86+
public void setVolumePaths(List<String> volumePaths) {
87+
this.volumePaths = volumePaths;
8888
}
8989

9090
@Override

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

Lines changed: 22 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@
5858
import java.util.Map;
5959
import java.util.HashMap;
6060
import java.util.Objects;
61-
import java.util.Optional;
6261
import java.util.UUID;
6362
import java.util.stream.Collectors;
6463

@@ -164,8 +163,8 @@ public boolean takeBackup(final VirtualMachine vm) {
164163
if (VirtualMachine.State.Stopped.equals(vm.getState())) {
165164
List<VolumeVO> vmVolumes = volumeDao.findByInstance(vm.getId());
166165
vmVolumes.sort(Comparator.comparing(Volume::getDeviceId));
167-
Map<String, String> volumePaths = getVolumePaths(vmVolumes, Collections.emptyList());
168-
command.setVolumePathsAndUuids(volumePaths);
166+
List<String> volumePaths = getVolumePaths(vmVolumes);
167+
command.setVolumePaths(volumePaths);
169168
}
170169

171170
BackupAnswer answer = null;
@@ -229,7 +228,8 @@ public boolean restoreVMFromBackup(VirtualMachine vm, Backup backup) {
229228
restoreCommand.setBackupRepoAddress(backupRepository.getAddress());
230229
restoreCommand.setMountOptions(backupRepository.getMountOptions());
231230
restoreCommand.setVmName(vm.getName());
232-
restoreCommand.setVolumePathsAndUuids(getVolumePaths(volumes, backedVolumes));
231+
restoreCommand.setVolumePaths(getVolumePaths(volumes));
232+
restoreCommand.setBackupFiles(getBackupFiles(backedVolumes));
233233
restoreCommand.setVmExists(vm.getRemoved() == null);
234234
restoreCommand.setVmState(vm.getState());
235235

@@ -244,8 +244,16 @@ public boolean restoreVMFromBackup(VirtualMachine vm, Backup backup) {
244244
return answer.getResult();
245245
}
246246

247-
private Map<String, String> getVolumePaths(List<VolumeVO> volumes, List<Backup.VolumeInfo> backedVolumes) {
248-
Map<String, String> volumePaths = new HashMap<>();
247+
private List<String> getBackupFiles(List<Backup.VolumeInfo> backedVolumes) {
248+
List<String> backupFiles = new ArrayList<>();
249+
for (Backup.VolumeInfo backedVolume : backedVolumes) {
250+
backupFiles.add(backedVolume.getPath());
251+
}
252+
return backupFiles;
253+
}
254+
255+
private List<String> getVolumePaths(List<VolumeVO> volumes) {
256+
List<String> volumePaths = new ArrayList<>();
249257
for (VolumeVO volume : volumes) {
250258
StoragePoolVO storagePool = primaryDataStoreDao.findById(volume.getPoolId());
251259
if (Objects.isNull(storagePool)) {
@@ -259,27 +267,8 @@ private Map<String, String> getVolumePaths(List<VolumeVO> volumes, List<Backup.V
259267
} else {
260268
volumePathPrefix = String.format("/mnt/%s", storagePool.getUuid());
261269
}
262-
// Build current volume path (destination for restore)
263-
String currentVolumePath = String.format("%s/%s", volumePathPrefix, volume.getPath());
264-
265-
// Find backed volume path (used for backup filename lookup)
266-
String backedVolumePath = volume.getPath();
267-
boolean hasBackedVolumes = backedVolumes != null && !backedVolumes.isEmpty();
268-
if (hasBackedVolumes) {
269-
Optional<Backup.VolumeInfo> opt = backedVolumes.stream()
270-
.filter(bv -> bv.getUuid().equals(volume.getUuid())).findFirst();
271-
if (opt.isPresent()) {
272-
Backup.VolumeInfo backedVolume = opt.get();
273-
if (backedVolume.getPath() != null && !backedVolume.getPath().isEmpty()) {
274-
// Use the backed volume path (path at time of backup) for filename lookup
275-
backedVolumePath = backedVolume.getPath();
276-
}
277-
}
278-
}
279-
280-
volumePaths.put(currentVolumePath, backedVolumePath);
270+
volumePaths.add(String.format("%s/%s", volumePathPrefix, volume.getPath()));
281271
}
282-
283272
return volumePaths;
284273
}
285274

@@ -290,8 +279,8 @@ public Pair<Boolean, String> restoreBackedUpVolume(Backup backup, String volumeU
290279
final StoragePoolHostVO dataStore = storagePoolHostDao.findByUuid(dataStoreUuid);
291280
final HostVO hostVO = hostDao.findByIp(hostIp);
292281

293-
Optional<Backup.VolumeInfo> matchingVolume = getBackedUpVolumeInfo(backupSourceVm.getBackupVolumeList(), volumeUuid);
294-
Long backedUpVolumeSize = matchingVolume.isPresent() ? matchingVolume.get().getSize() : 0L;
282+
Backup.VolumeInfo matchingVolume = getBackedUpVolumeInfo(backupSourceVm.getBackupVolumeList(), volumeUuid);
283+
Long backedUpVolumeSize = matchingVolume.getSize() != null ? matchingVolume.getSize() : 0L;
295284

296285
LOG.debug("Restoring vm volume {} from backup {} on the NAS Backup Provider", volume, backup);
297286
BackupRepository backupRepository = getBackupRepository(backupSourceVm, backup);
@@ -318,12 +307,12 @@ public Pair<Boolean, String> restoreBackedUpVolume(Backup backup, String volumeU
318307
restoreCommand.setBackupRepoType(backupRepository.getType());
319308
restoreCommand.setBackupRepoAddress(backupRepository.getAddress());
320309
restoreCommand.setVmName(vmNameAndState.first());
321-
restoreCommand.setVolumePathsAndUuids(Collections.singletonMap(String.format("%s/%s", dataStore.getLocalPath(), volumeUUID), volumeUUID));
310+
restoreCommand.setVolumePaths(Collections.singletonList(String.format("%s/%s", dataStore.getLocalPath(), volumeUUID)));
311+
restoreCommand.setBackupFiles(Collections.singletonList(matchingVolume.getPath()));
322312
restoreCommand.setDiskType(volume.getVolumeType().name().toLowerCase(Locale.ROOT));
323313
restoreCommand.setMountOptions(backupRepository.getMountOptions());
324314
restoreCommand.setVmExists(null);
325315
restoreCommand.setVmState(vmNameAndState.second());
326-
restoreCommand.setRestoreVolumeUUID(volumeUuid);
327316

328317
BackupAnswer answer = null;
329318
try {
@@ -358,10 +347,11 @@ private BackupRepository getBackupRepository(VirtualMachine vm, Backup backup) {
358347
return backupRepository;
359348
}
360349

361-
private Optional<Backup.VolumeInfo> getBackedUpVolumeInfo(List<Backup.VolumeInfo> backedUpVolumes, String volumeUuid) {
350+
private Backup.VolumeInfo getBackedUpVolumeInfo(List<Backup.VolumeInfo> backedUpVolumes, String volumeUuid) {
362351
return backedUpVolumes.stream()
363352
.filter(v -> v.getUuid().equals(volumeUuid))
364-
.findFirst();
353+
.findFirst()
354+
.orElse(null);
365355
}
366356

367357
@Override

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

Lines changed: 30 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,11 @@
3131
import org.apache.cloudstack.backup.RestoreBackupCommand;
3232
import org.apache.commons.lang3.RandomStringUtils;
3333

34-
import java.io.File;
3534
import java.io.IOException;
3635
import java.nio.file.Files;
3736
import java.nio.file.Paths;
37+
import java.util.List;
3838
import java.util.Locale;
39-
import java.util.Map;
4039
import java.util.Objects;
4140

4241
@ResourceWrapper(handles = RestoreBackupCommand.class)
@@ -58,22 +57,22 @@ public Answer execute(RestoreBackupCommand command, LibvirtComputingResource ser
5857
String mountOptions = command.getMountOptions();
5958
Boolean vmExists = command.isVmExists();
6059
String diskType = command.getDiskType();
61-
Map<String, String> volumePathsAndUuids = command.getVolumePathsAndUuids();
62-
String restoreVolumeUuid = command.getRestoreVolumeUUID();
60+
List<String> volumePaths = command.getVolumePaths();
61+
List<String> backupFiles = command.getBackupFiles();
6362

6463
String newVolumeId = null;
6564
try {
6665
if (Objects.isNull(vmExists)) {
67-
Map.Entry<String, String> firstEntry = volumePathsAndUuids.entrySet().iterator().next();
68-
String volumePath = firstEntry.getKey();
66+
String volumePath = volumePaths.get(0);
67+
String backupFile = backupFiles.get(0);
6968
int lastIndex = volumePath.lastIndexOf("/");
7069
newVolumeId = volumePath.substring(lastIndex + 1);
71-
restoreVolume(backupPath, backupRepoType, backupRepoAddress, volumePath, diskType, restoreVolumeUuid,
70+
restoreVolume(backupPath, backupRepoType, backupRepoAddress, volumePath, diskType, backupFile,
7271
new Pair<>(vmName, command.getVmState()), mountOptions);
7372
} else if (Boolean.TRUE.equals(vmExists)) {
74-
restoreVolumesOfExistingVM(volumePathsAndUuids, backupPath, backupRepoType, backupRepoAddress, mountOptions);
73+
restoreVolumesOfExistingVM(volumePaths, backupPath, backupFiles, backupRepoType, backupRepoAddress, mountOptions);
7574
} else {
76-
restoreVolumesOfDestroyedVMs(volumePathsAndUuids, vmName, backupPath, backupRepoType, backupRepoAddress, mountOptions);
75+
restoreVolumesOfDestroyedVMs(volumePaths, vmName, backupPath, backupFiles, backupRepoType, backupRepoAddress, mountOptions);
7776
}
7877
} catch (CloudRuntimeException e) {
7978
String errorMessage = "Failed to restore backup for VM: " + vmName + ".";
@@ -87,18 +86,18 @@ public Answer execute(RestoreBackupCommand command, LibvirtComputingResource ser
8786
return new BackupAnswer(command, true, newVolumeId);
8887
}
8988

90-
private void restoreVolumesOfExistingVM(Map<String,String> volumePaths, String backupPath,
89+
private void restoreVolumesOfExistingVM(List<String> volumePaths, String backupPath, List<String> backupFiles,
9190
String backupRepoType, String backupRepoAddress, String mountOptions) {
9291
String diskType = "root";
9392
String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType, mountOptions);
9493
try {
95-
for (Map.Entry<String, String> entry : volumePaths.entrySet()) {
96-
String currentVolumePath = entry.getKey();
97-
String backedVolumePath = entry.getValue();
98-
Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, backedVolumePath, backupPath, diskType, null);
94+
for (int idx = 0; idx < volumePaths.size(); idx++) {
95+
String volumePath = volumePaths.get(idx);
96+
String backupFile = backupFiles.get(idx);
97+
String bkpPath = getBackupPath(mountDirectory, backupPath, backupFile, diskType);
9998
diskType = "datadisk";
100-
if (!replaceVolumeWithBackup(currentVolumePath, bkpPathAndVolUuid.first())) {
101-
throw new CloudRuntimeException(String.format("Unable to restore backup for volume [%s].", bkpPathAndVolUuid.second()));
99+
if (!replaceVolumeWithBackup(volumePath, bkpPath)) {
100+
throw new CloudRuntimeException(String.format("Unable to restore backup from volume [%s].", volumePath));
102101
}
103102
}
104103
} finally {
@@ -108,18 +107,18 @@ private void restoreVolumesOfExistingVM(Map<String,String> volumePaths, String b
108107

109108
}
110109

111-
private void restoreVolumesOfDestroyedVMs(Map<String, String> volumePaths, String vmName, String backupPath,
110+
private void restoreVolumesOfDestroyedVMs(List<String> volumePaths, String vmName, String backupPath, List<String> backupFiles,
112111
String backupRepoType, String backupRepoAddress, String mountOptions) {
113112
String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType, mountOptions);
114113
String diskType = "root";
115114
try {
116-
for (Map.Entry<String, String> entry : volumePaths.entrySet()) {
117-
String currentVolumePath = entry.getKey();
118-
String backedVolumePath = entry.getValue();
119-
Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, backedVolumePath, backupPath, diskType, null);
115+
for (int idx = 0; idx < volumePaths.size(); idx++) {
116+
String volumePath = volumePaths.get(idx);
117+
String backupFile = backupFiles.get(idx);
118+
String bkpPath = getBackupPath(mountDirectory, backupPath, backupFile, diskType);
120119
diskType = "datadisk";
121-
if (!replaceVolumeWithBackup(currentVolumePath, bkpPathAndVolUuid.first())) {
122-
throw new CloudRuntimeException(String.format("Unable to restore backup for volume [%s].", bkpPathAndVolUuid.second()));
120+
if (!replaceVolumeWithBackup(volumePath, bkpPath)) {
121+
throw new CloudRuntimeException(String.format("Unable to restore backup from volume [%s].", volumePath));
123122
}
124123
}
125124
} finally {
@@ -129,13 +128,13 @@ private void restoreVolumesOfDestroyedVMs(Map<String, String> volumePaths, Strin
129128
}
130129

131130
private void restoreVolume(String backupPath, String backupRepoType, String backupRepoAddress, String volumePath,
132-
String diskType, String volumeUUID, Pair<String, VirtualMachine.State> vmNameAndState, String mountOptions) {
131+
String diskType, String backupFile, Pair<String, VirtualMachine.State> vmNameAndState, String mountOptions) {
133132
String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType, mountOptions);
134-
Pair<String, String> bkpPathAndVolUuid;
133+
String bkpPath;
135134
try {
136-
bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, volumeUUID);
137-
if (!replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first())) {
138-
throw new CloudRuntimeException(String.format("Unable to restore backup for volume [%s].", bkpPathAndVolUuid.second()));
135+
bkpPath = getBackupPath(mountDirectory, backupPath, backupFile, diskType);
136+
if (!replaceVolumeWithBackup(volumePath, bkpPath)) {
137+
throw new CloudRuntimeException(String.format("Unable to restore backup from volume [%s].", volumePath));
139138
}
140139
if (VirtualMachine.State.Running.equals(vmNameAndState.second())) {
141140
if (!attachVolumeToVm(vmNameAndState.first(), volumePath)) {
@@ -191,13 +190,11 @@ private void deleteTemporaryDirectory(String backupDirectory) {
191190
}
192191
}
193192

194-
private Pair<String, String> getBackupPath(String mountDirectory, String volumePath, String backupPath, String diskType, String volumeUuid) {
193+
private String getBackupPath(String mountDirectory, String backupPath, String backupFile, String diskType) {
195194
String bkpPath = String.format(FILE_PATH_PLACEHOLDER, mountDirectory, backupPath);
196-
int lastIndex = volumePath.lastIndexOf(File.separator);
197-
String volUuid = Objects.isNull(volumeUuid) ? volumePath.substring(lastIndex + 1) : volumeUuid;
198-
String backupFileName = String.format("%s.%s.qcow2", diskType.toLowerCase(Locale.ROOT), volUuid);
195+
String backupFileName = String.format("%s.%s.qcow2", diskType.toLowerCase(Locale.ROOT), backupFile);
199196
bkpPath = String.format(FILE_PATH_PLACEHOLDER, bkpPath, backupFileName);
200-
return new Pair<>(bkpPath, volUuid);
197+
return bkpPath;
201198
}
202199

203200
private boolean replaceVolumeWithBackup(String volumePath, String backupPath) {

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
package com.cloud.hypervisor.kvm.resource.wrapper;
2121

22+
import com.amazonaws.util.CollectionUtils;
2223
import com.cloud.agent.api.Answer;
2324
import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
2425
import com.cloud.resource.CommandWrapper;
@@ -31,7 +32,6 @@
3132
import java.util.ArrayList;
3233
import java.util.Arrays;
3334
import java.util.List;
34-
import java.util.Map;
3535
import java.util.Objects;
3636

3737
@ResourceWrapper(handles = TakeBackupCommand.class)
@@ -43,7 +43,7 @@ public Answer execute(TakeBackupCommand command, LibvirtComputingResource libvir
4343
final String backupRepoType = command.getBackupRepoType();
4444
final String backupRepoAddress = command.getBackupRepoAddress();
4545
final String mountOptions = command.getMountOptions();
46-
final Map<String, String> diskPathsAndUuids = command.getVolumePathsAndUuids();
46+
final List<String> diskPaths = command.getVolumePaths();
4747

4848
List<String[]> commands = new ArrayList<>();
4949
commands.add(new String[]{
@@ -54,7 +54,7 @@ public Answer execute(TakeBackupCommand command, LibvirtComputingResource libvir
5454
"-s", backupRepoAddress,
5555
"-m", Objects.nonNull(mountOptions) ? mountOptions : "",
5656
"-p", backupPath,
57-
"-d", (Objects.nonNull(diskPathsAndUuids) && !diskPathsAndUuids.isEmpty()) ? String.join(",", diskPathsAndUuids.keySet()) : ""
57+
"-d", (Objects.nonNull(diskPaths) && !diskPaths.isEmpty()) ? String.join(",", diskPaths) : ""
5858
});
5959

6060
Pair<Integer, String> result = Script.executePipedCommands(commands, libvirtComputingResource.getCmdsTimeout());
@@ -65,7 +65,7 @@ public Answer execute(TakeBackupCommand command, LibvirtComputingResource libvir
6565
}
6666

6767
long backupSize = 0L;
68-
if (diskPathsAndUuids == null || diskPathsAndUuids.isEmpty()) {
68+
if (CollectionUtils.isNullOrEmpty(diskPaths)) {
6969
List<String> outputLines = Arrays.asList(result.second().trim().split("\n"));
7070
if (!outputLines.isEmpty()) {
7171
backupSize = Long.parseLong(outputLines.get(outputLines.size() - 1).trim());

0 commit comments

Comments
 (0)