apache · SadiJr · Feb 14, 2023 · Feb 14, 2023 · Feb 14, 2023 · Feb 21, 2023
diff --git a/api/src/main/java/com/cloud/hypervisor/HypervisorGuru.java b/api/src/main/java/com/cloud/hypervisor/HypervisorGuru.java
@@ -95,7 +95,7 @@ public interface HypervisorGuru extends Adapter {
     VirtualMachine importVirtualMachineFromBackup(long zoneId, long domainId, long accountId, long userId,
                                                   String vmInternalName, Backup backup) throws Exception;
 
-    boolean attachRestoredVolumeToVirtualMachine(long zoneId, String location, Backup.VolumeInfo volumeInfo,
+    boolean attachRestoredVolumeToVirtualMachine(long zoneId, String restoredVolumeLocation, Backup.VolumeInfo volumeInfo,
                                                  VirtualMachine vm, long poolId, Backup backup) throws Exception;
     /**
      * Will generate commands to migrate a vm to a pool. For now this will only work for stopped VMs on Vmware.

diff --git a/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java b/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java
@@ -135,6 +135,12 @@ public interface BackupManager extends BackupService, Configurable, PluggableSer
 
     /**
      * Restore a backed up volume and attach it to a VM
+     *
+     * @param backedUpVolumeUuid volume to be restored
+     * @param backupId           backup containing the volume to be restored
+     * @param vmId               VM to attach restored volume
+     * @return returns operation success
+     * @throws Exception
      */
     boolean restoreBackupVolumeAndAttachToVM(final String backedUpVolumeUuid, final Long backupId, final Long vmId) throws Exception;
 

diff --git a/api/src/main/java/org/apache/cloudstack/backup/BackupProvider.java b/api/src/main/java/org/apache/cloudstack/backup/BackupProvider.java
@@ -93,7 +93,7 @@ public interface BackupProvider {
     /**
      * Restore a volume from a backup
      */
-    Pair<Boolean, String> restoreBackedUpVolume(Backup backup, String volumeUuid, String hostIp, String dataStoreUuid);
+    Pair<Boolean, String> restoreBackedUpVolume(Backup backup, String volumeUuid, String host, String dataStore, VirtualMachine vm);
 
     /**
      * Returns backup metrics for a list of VMs in a zone

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
@@ -76,7 +76,7 @@ public boolean restoreVMFromBackup(VirtualMachine vm, Backup backup) {
     }
 
     @Override
-    public Pair<Boolean, String> restoreBackedUpVolume(Backup backup, String volumeUuid, String hostIp, String dataStoreUuid) {
+    public Pair<Boolean, String> restoreBackedUpVolume(Backup backup, String volumeUuid, String host, String dataStore, VirtualMachine vm) {
         logger.debug("Restoring volume " + volumeUuid + "from backup " + backup.getUuid() + " on the Dummy Backup Provider");
         throw new CloudRuntimeException("Dummy plugin does not support this feature");
     }

diff --git a/.../backup/networker/src/main/java/org/apache/cloudstack/backup/NetworkerBackupProvider.java b/.../backup/networker/src/main/java/org/apache/cloudstack/backup/NetworkerBackupProvider.java
@@ -372,12 +372,12 @@ public boolean restoreVMFromBackup(VirtualMachine vm, Backup backup) {
     }
 
     @Override
-    public Pair<Boolean, String> restoreBackedUpVolume(Backup backup, String volumeUuid, String hostIp, String dataStoreUuid) {
+    public Pair<Boolean, String> restoreBackedUpVolume(Backup backup, String volumeUuid, String host, String dataStore, VirtualMachine vm) {
         String networkerServer;
         VolumeVO volume = volumeDao.findByUuid(volumeUuid);
         VMInstanceVO backupSourceVm = vmInstanceDao.findById(backup.getVmId());
-        StoragePoolHostVO dataStore = storagePoolHostDao.findByUuid(dataStoreUuid);
-        HostVO hostVO = hostDao.findByIp(hostIp);
+        StoragePoolHostVO datastore = storagePoolHostDao.findByUuid(dataStore);
+        HostVO hostVO = hostDao.findByIp(host);
 
         final Long zoneId = backup.getZoneId();
         final String externalBackupId = backup.getExternalId();
@@ -440,7 +440,7 @@ public Pair<Boolean, String> restoreBackedUpVolume(Backup backup, String volumeU
         script.add("-n");
         script.add(restoredVolume.getUuid());
         script.add("-p");
-        script.add(dataStore.getLocalPath());
+        script.add(datastore.getLocalPath());
         script.add("-a");
         script.add(volume.getUuid());
 

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
@@ -27,6 +27,7 @@
 import java.util.Objects;
 import java.util.stream.Collectors;
 
+import com.google.gson.Gson;
 import javax.inject.Inject;
 
 import org.apache.cloudstack.api.ApiCommandResourceType;
@@ -37,7 +38,9 @@
 import org.apache.cloudstack.backup.veeam.api.Job;
 import org.apache.cloudstack.framework.config.ConfigKey;
 import org.apache.cloudstack.framework.config.Configurable;
+import org.apache.cloudstack.utils.volume.VirtualMachineDiskInfo;
 import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.BooleanUtils;
 
 import com.cloud.agent.AgentManager;
@@ -51,6 +54,9 @@
 import com.cloud.dc.dao.VmwareDatacenterDao;
 import com.cloud.hypervisor.vmware.dao.VmwareDatacenterZoneMapDao;
 import com.cloud.user.User;
+import com.cloud.serializer.GsonHelper;
+import com.cloud.storage.VolumeVO;
+import com.cloud.storage.dao.VolumeDao;
 import com.cloud.utils.Pair;
 import com.cloud.utils.component.AdapterBase;
 import com.cloud.utils.db.Transaction;
@@ -64,6 +70,7 @@
 
 public class VeeamBackupProvider extends AdapterBase implements BackupProvider, Configurable {
 
+    private static final Gson GSON = GsonHelper.getGson();
     public static final String BACKUP_IDENTIFIER = "-CSBKP-";
 
     public ConfigKey<String> VeeamUrl = new ConfigKey<>("Advanced", String.class,
@@ -109,6 +116,8 @@ public class VeeamBackupProvider extends AdapterBase implements BackupProvider,
     private AgentManager agentMgr;
     @Inject
     private VirtualMachineManager virtualMachineManager;
+    @Inject
+    private VolumeDao volumeDao;
 
     protected VeeamClient getClient(final Long zoneId) {
         try {
@@ -291,10 +300,31 @@ private void prepareForBackupRestoration(VirtualMachine vm) {
     }
 
     @Override
-    public Pair<Boolean, String> restoreBackedUpVolume(Backup backup, String volumeUuid, String hostIp, String dataStoreUuid) {
+    public Pair<Boolean, String> restoreBackedUpVolume(Backup backup, String volumeUuid, String host, String dataStore, VirtualMachine vm) {
+        Pair<Boolean, String> result = new Pair<>(false, "");
         final Long zoneId = backup.getZoneId();
         final String restorePointId = backup.getExternalId();
-        return getClient(zoneId).restoreVMToDifferentLocation(restorePointId, hostIp, dataStoreUuid);
+
+        VMInstanceVO vmVO = vmInstanceDao.findById(backup.getVmId());
+        VolumeVO volumeVO = volumeDao.findByUuid(volumeUuid);
+        long totalDeviceIds = volumeDao.findByInstance(vm.getId()).stream().mapToLong(VolumeVO::getDeviceId).max().orElse(0L);
+        long newDeviceId = totalDeviceIds + 1;
+        logger.debug("VM [{}] has [{}] deviceIds. Trying to restore volume [{}] using restorePoint [{}] and with [{}] as the new deviceId.", vm.getUuid(),
+                totalDeviceIds, volumeUuid, restorePointId, newDeviceId);
+
+        VirtualMachineDiskInfo fromJson = GSON.fromJson(volumeVO.getChainInfo(), VirtualMachineDiskInfo.class);
+        String type = fromJson.getControllerFromDeviceBusName().toUpperCase();
+        String virtualDeviceNode = StringUtils.substringAfter(fromJson.getDiskDeviceBusName(), ":");
+        for (String name : fromJson.getDiskChain()) {
+            String diskName = StringUtils.substringAfter(name, "/");
+            try {
+                result = getClient(zoneId).restoreVolume(volumeUuid, vmVO.getUuid(), restorePointId, host, dataStore, type, virtualDeviceNode, diskName, newDeviceId, vm);
+            } catch (Exception e) {
+                logger.error("Failed to restore volume [{}] in VM [{}], with type [{}], node [{}] and disk name [{}], using target host [{}] and datastore [{}] due to [{}].",
+                        volumeUuid, vmVO.getUuid(), type, virtualDeviceNode, diskName, host, dataStore, e.getMessage(), e);
+            }
+        }
+        return result;
     }
 
     @Override

diff --git a/plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/veeam/VeeamClient.java b/plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/veeam/VeeamClient.java
@@ -84,9 +84,11 @@
 
 import com.cloud.utils.NumbersUtil;
 import com.cloud.utils.Pair;
+import com.cloud.utils.UuidUtils;
 import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.utils.nio.TrustAllManager;
 import com.cloud.utils.ssh.SshHelper;
+import com.cloud.vm.VirtualMachine;
 import com.fasterxml.jackson.databind.DeserializationFeature;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.dataformat.xml.XmlMapper;
@@ -913,14 +915,25 @@ private Date formatDate(String date) throws ParseException {
         return dateFormat.parse(StringUtils.substring(date, 0, 19));
     }
 
-    public Pair<Boolean, String> restoreVMToDifferentLocation(String restorePointId, String hostIp, String dataStoreUuid) {
+    protected String removeDashesIfDatastoreNameIsUuid(String datastore) {
+        if (!UuidUtils.isUuid(datastore)) {
+            return datastore;
+        }
+        logger.trace("Removing the dash symbol of datastore name [{}] because this name is a valid UUID used by ACS. This happens because when a new NFS storage is created via ACS, "
+                + "this storage is created in vCenter with a name that uses the UUID in ACS, but removing the dashes. "
+                + "However, if the storage is created first in vCenter (e.g. VMFS), and its name contains dashes, if this name is not a valid UUID, the dashes will be keeped. But, if for some "
+                + "reason the name is a valid UUID, this causes ACS to remove the dashes, and then the restore backup fails, because Veeam does not find the datastore.", datastore);
+        return datastore.replace("-","");
+    }
+
+    public Pair<Boolean, String> restoreVMToDifferentLocation(String restorePointId, String host, String datastore) {
         final String restoreLocation = RESTORE_VM_SUFFIX + UUID.randomUUID().toString();
-        final String datastoreId = dataStoreUuid.replace("-","");
+        datastore = removeDashesIfDatastoreNameIsUuid(datastore);
         final List<String> cmds = Arrays.asList(
                 "$points = Get-VBRRestorePoint",
                 String.format("foreach($point in $points) { if ($point.Id -eq '%s') { break; } }", restorePointId),
-                String.format("$server = Get-VBRServer -Name \"%s\"", hostIp),
-                String.format("$ds = Find-VBRViDatastore -Server:$server -Name \"%s\"", datastoreId),
+                String.format("$server = Get-VBRServer -Name \"%s\"", host),
+                String.format("$ds = Find-VBRViDatastore -Server:$server -Name \"%s\"", datastore),
                 String.format("$job = Start-VBRRestoreVM -RestorePoint:$point -Server:$server -Datastore:$ds -VMName \"%s\" -RunAsync", restoreLocation),
                 "while (-not (Get-VBRRestoreSession -Id $job.Id).IsCompleted) { Start-Sleep -Seconds 10 }"
         );
@@ -934,4 +947,43 @@ public Pair<Boolean, String> restoreVMToDifferentLocation(String restorePointId,
     private boolean isLegacyServer() {
         return this.veeamServerVersion != null && (this.veeamServerVersion > 0 && this.veeamServerVersion < 11);
     }
+
+    public Pair<Boolean, String> restoreVolume(String volumeUuid, String vmUuid, String restorePointId, String host, String datastore, String diskType, String diskDeviceNode,
+            String diskName, long device, VirtualMachine vm) {
+        datastore = removeDashesIfDatastoreNameIsUuid(datastore);
+        if (diskType.equals("IDE")) {
+            String veeamLink = "https://www.veeam.com/kb1100";
+            logger.warn("Veeam does not support restore of IDE virtual devices disks. We will use the type SCSI instead. "
+                    + "For more information, please see this link [{}].", veeamLink);
+        }
+        final List<String> cmds = Arrays.asList(
+                String.format("$point = Get-VBRRestorePoint -Id '%s'", restorePointId),
+                "if ($point) {",
+                    String.format("$vm = Find-VBRViEntity -Name '%s'", vm.getInstanceName()),
+                    String.format("$server = Get-VBRServer -Name \"%s\"", host),
+                    String.format("$ds = Find-VBRViDatastore -Server:$server -Name \"%s\"", datastore),
+                    String.format("$disk = Get-VBRViVirtualDevice -RestorePoint:$point ^| Where-Object { $_.Type -eq '%s' -and $_.VirtualDeviceNode -eq '%s' -and $_.Name -eq '%s' }", diskType, diskDeviceNode, diskName),
+                    "if ($disk -and $ds -and $server -and $vm) { ",
+                        String.format("$newdisk = Set-VBRViVirtualDevice -VirtualDevice $disk -VirtualDeviceNode %s -Type %s", device, diskType.equals("IDE") ? "SCSI" : diskType),
+                        "$mapping = New-VBRViVirtualDeviceMappingRule -SourceVirtualDevice:$newdisk -Datastore:$ds",
+                        "$job = Start-VBRViVirtualDiskRestore -RestorePoint:$point -VirtualDeviceMapping:$mapping -TargetVM $vm -RunAsync",
+                        "while (-not (Get-VBRRestoreSession -Id $job.Id).IsCompleted) { Start-Sleep -Seconds 10 }",
+                        "Write-Output $disk.Name",
+                    "} else { ",
+                        "Write-Output 'Cannot find disk to restore' ",
+                        "Exit 1",
+                    "}",
+                "} else { ",
+                    "Write-Output 'Cannot find any restore point with this id'",
+                    "Exit 1",
+                "}"
+        );
+        Pair<Boolean, String> result = executePowerShellCommands(cmds);
+        if (result != null && result.first()) {
+            return new Pair<>(result.first(), result.second().split("\r\n")[0]);
+        }
+        logger.error("Failed to restore volume [uuid: {}, name: {}, type: {}, deviceId: {}] of VM [{}] to VM [{}] using restore point [%s], host [%s] and datastore [%s].",
+                volumeUuid, diskName, diskType, diskDeviceNode, vmUuid, vm.getUuid(), restorePointId, host, datastore);
+        throw new CloudRuntimeException(String.format("Failed to restore volume [%s] of VM [%s] to VM [%s] using restore point [%s],", volumeUuid, vmUuid, vm.getUuid(), restorePointId));
+    }
 }
diff --git a/plugins/backup/veeam/src/test/java/org/apache/cloudstack/backup/veeam/VeeamClientTest.java b/plugins/backup/veeam/src/test/java/org/apache/cloudstack/backup/veeam/VeeamClientTest.java
@@ -24,30 +24,24 @@
 import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor;
 import static com.github.tomakehurst.wiremock.client.WireMock.urlMatching;
 import static com.github.tomakehurst.wiremock.client.WireMock.verify;
-import static org.junit.Assert.fail;
-import static org.mockito.Mockito.times;
 
 import java.io.ByteArrayInputStream;
-import java.io.IOException;
 import java.io.InputStream;
 import java.text.SimpleDateFormat;
 import java.util.List;
 import java.util.Map;
+import java.util.UUID;
 
+import com.cloud.utils.Pair;
 import org.apache.cloudstack.backup.Backup;
 import org.apache.cloudstack.backup.BackupOffering;
-import org.apache.cloudstack.backup.veeam.api.RestoreSession;
-import org.apache.http.HttpResponse;
 import org.apache.logging.log4j.core.Logger;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.mockito.Mockito;
-import org.springframework.test.util.ReflectionTestUtils;
 
-import com.cloud.utils.Pair;
-import com.cloud.utils.exception.CloudRuntimeException;
 import com.github.tomakehurst.wiremock.client.BasicCredentials;
 import com.github.tomakehurst.wiremock.junit.WireMockRule;
 
@@ -105,71 +99,25 @@ public void testVeeamJobs() {
     }
 
     @Test
-    public void getRepositoryNameFromJobTestExceptionCmdWithoutResult() throws Exception {
-        String backupName = "TEST-BACKUP";
-        try {
-            Mockito.doReturn(null).when(mockClient).executePowerShellCommands(Mockito.anyList());
-            mockClient.getRepositoryNameFromJob(backupName);
-            fail();
-        } catch (Exception e) {
-            Assert.assertEquals(CloudRuntimeException.class, e.getClass());
-            Assert.assertEquals("Failed to get Repository Name from Job [name: TEST-BACKUP].", e.getMessage());
-        }
-    }
-
-    @Test
-    public void getRepositoryNameFromJobTestExceptionCmdWithFalseResult() {
-        String backupName = "TEST-BACKUP2";
-        Pair<Boolean, String> response = new Pair<Boolean, String>(Boolean.FALSE, "");
-        Mockito.doReturn(response).when(mockClient).executePowerShellCommands(Mockito.anyList());
-        try {
-            mockClient.getRepositoryNameFromJob(backupName);
-            fail();
-        } catch (Exception e) {
-            Assert.assertEquals(CloudRuntimeException.class, e.getClass());
-            Assert.assertEquals("Failed to get Repository Name from Job [name: TEST-BACKUP2].", e.getMessage());
-        }
-    }
-
-    @Test
-    public void getRepositoryNameFromJobTestExceptionWhenResultIsInWrongFormat() {
-        String backupName = "TEST-BACKUP3";
-        Pair<Boolean, String> response = new Pair<Boolean, String>(Boolean.TRUE, "\nName:\n\nName-test");
-        Mockito.doReturn(response).when(mockClient).executePowerShellCommands(Mockito.anyList());
-        try {
-            mockClient.getRepositoryNameFromJob(backupName);
-            fail();
-        } catch (Exception e) {
-            Assert.assertEquals(CloudRuntimeException.class, e.getClass());
-            Assert.assertEquals("Can't find any repository name for Job [name: TEST-BACKUP3].", e.getMessage());
-        }
+    public void removeDashesIfDatastoreNameIsUuidTestValidUuid() {
+        String validUuid = UUID.randomUUID().toString();
+        String expected = validUuid.replace("-","");
+        String result = client.removeDashesIfDatastoreNameIsUuid(validUuid);
+        Assert.assertEquals(expected, result);
     }
 
     @Test
-    public void getRepositoryNameFromJobTestSuccess() throws Exception {
-        String backupName = "TEST-BACKUP3";
-        Pair<Boolean, String> response = new Pair<Boolean, String>(Boolean.TRUE, "\r\nName : test");
-        Mockito.doReturn(response).when(mockClient).executePowerShellCommands(Mockito.anyList());
-        String repositoryNameFromJob = mockClient.getRepositoryNameFromJob(backupName);
-        Assert.assertEquals("test", repositoryNameFromJob);
+    public void removeDashesIfDatastoreNameIsUuidTestNameWithDashesButIsNotUuid() {
+        String datastore = UUID.randomUUID().toString() + "-test-extra-name";
+        String result = client.removeDashesIfDatastoreNameIsUuid(datastore);
+        Assert.assertEquals(datastore, result);
     }
 
     @Test
-    public void checkIfRestoreSessionFinishedTestTimeoutException() throws IOException {
-        try {
-            ReflectionTestUtils.setField(mockClient, "restoreTimeout", 10);
-            RestoreSession restoreSession = Mockito.mock(RestoreSession.class);
-            HttpResponse httpResponse = Mockito.mock(HttpResponse.class);
-            Mockito.when(mockClient.get(Mockito.anyString())).thenReturn(httpResponse);
-            Mockito.when(mockClient.parseRestoreSessionResponse(httpResponse)).thenReturn(restoreSession);
-            Mockito.when(restoreSession.getResult()).thenReturn("No Success");
-            Mockito.when(mockClient.checkIfRestoreSessionFinished(Mockito.eq("RestoreTest"), Mockito.eq("any"))).thenCallRealMethod();
-            mockClient.checkIfRestoreSessionFinished("RestoreTest", "any");
-            fail();
-        } catch (Exception e) {
-            Assert.assertEquals("Related job type: RestoreTest was not successful", e.getMessage());
-        }
-        Mockito.verify(mockClient, times(10)).get(Mockito.anyString());
+    public void removeDashesIfDatastoreNameIsUuidTestNotUuidName() {
+        String name = "simple-datastore-name";
+        String result = client.removeDashesIfDatastoreNameIsUuid(name);
+        Assert.assertEquals(name, result);
     }
 
     private void verifyBackupMetrics(Map<String, Backup.Metric> metrics) {