This is an automated email from the ASF dual-hosted git repository. gabriel pushed a commit to branch null-file-created-during-migration-fixed in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit 3ce9f4665398c8f2255aaaf5450336e9f37fcb89 Author: GabrielBrascher <gabr...@pcextreme.nl> AuthorDate: Wed Mar 13 11:36:20 2019 -0300 Remove code that generated /var/lib/libvirt/images/null on target host --- .../KvmNonManagedStorageDataMotionStrategy.java | 29 +--------- .../motion/StorageSystemDataMotionStrategy.java | 4 +- .../KvmNonManagedStorageSystemDataMotionTest.java | 67 ---------------------- .../StorageSystemDataMotionStrategyTest.java | 4 +- 4 files changed, 5 insertions(+), 99 deletions(-) diff --git a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java index 2cf236d..1bbe177 100644 --- a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java +++ b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java @@ -40,15 +40,11 @@ import org.apache.log4j.Logger; import com.cloud.agent.api.Answer; import com.cloud.agent.api.MigrateCommand; import com.cloud.agent.api.MigrateCommand.MigrateDiskInfo; -import com.cloud.agent.api.storage.CreateAnswer; -import com.cloud.agent.api.storage.CreateCommand; -import com.cloud.agent.api.to.VirtualMachineTO; import com.cloud.exception.AgentUnavailableException; import com.cloud.exception.OperationTimedoutException; import com.cloud.host.Host; import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.storage.DataStoreRole; -import com.cloud.storage.DiskOfferingVO; import com.cloud.storage.Storage.StoragePoolType; import com.cloud.storage.StorageManager; import com.cloud.storage.StoragePool; @@ -57,7 +53,6 @@ import com.cloud.storage.VMTemplateStorageResourceAssoc; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.VMTemplatePoolDao; import com.cloud.utils.exception.CloudRuntimeException; -import com.cloud.vm.DiskProfile; import com.cloud.vm.VirtualMachineManager; /** @@ -111,28 +106,8 @@ public class KvmNonManagedStorageDataMotionStrategy extends StorageSystemDataMot * Example: /var/lib/libvirt/images/f3d49ecc-870c-475a-89fa-fd0124420a9b */ @Override - protected String generateDestPath(VirtualMachineTO vmTO, VolumeVO srcVolume, Host destHost, StoragePoolVO destStoragePool, VolumeInfo destVolumeInfo) { - DiskOfferingVO diskOffering = _diskOfferingDao.findById(srcVolume.getDiskOfferingId()); - DiskProfile diskProfile = new DiskProfile(destVolumeInfo, diskOffering, HypervisorType.KVM); - String templateUuid = getTemplateUuid(destVolumeInfo.getTemplateId()); - CreateCommand rootImageProvisioningCommand = new CreateCommand(diskProfile, templateUuid, destStoragePool, true); - - Answer rootImageProvisioningAnswer = agentManager.easySend(destHost.getId(), rootImageProvisioningCommand); - - if (rootImageProvisioningAnswer == null) { - throw new CloudRuntimeException(String.format("Migration with storage of vm [%s] failed while provisioning root image", vmTO.getName())); - } - - if (!rootImageProvisioningAnswer.getResult()) { - throw new CloudRuntimeException(String.format("Unable to modify target volume on the host [host id:%s, name:%s]", destHost.getId(), destHost.getName())); - } - - String libvirtDestImgsPath = null; - if (rootImageProvisioningAnswer instanceof CreateAnswer) { - libvirtDestImgsPath = ((CreateAnswer)rootImageProvisioningAnswer).getVolume().getName(); - } - // File.getAbsolutePath is used to keep the file separator as it should be and eliminate a verification to check if exists a file separator in the last character of libvirtDestImgsPath. - return new File(libvirtDestImgsPath, destVolumeInfo.getUuid()).getAbsolutePath(); + protected String generateDestPath(Host destHost, StoragePoolVO destStoragePool, VolumeInfo destVolumeInfo) { + return new File(destStoragePool.getPath(), destVolumeInfo.getUuid()).getAbsolutePath(); } /** diff --git a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java index 6bcaebe..1f3368f 100644 --- a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java +++ b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java @@ -1764,7 +1764,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { _volumeService.grantAccess(destVolumeInfo, destHost, destDataStore); - String destPath = generateDestPath(vmTO, srcVolume, destHost, destStoragePool, destVolumeInfo); + String destPath = generateDestPath(destHost, destStoragePool, destVolumeInfo); MigrateCommand.MigrateDiskInfo migrateDiskInfo = configureMigrateDiskInfo(srcVolumeInfo, destPath); migrateDiskInfo.setSourceDiskOnStorageFileSystem(isStoragePoolTypeOfFile(sourceStoragePool)); @@ -1855,7 +1855,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { /** * Returns the iScsi connection path. */ - protected String generateDestPath(VirtualMachineTO vmTO, VolumeVO srcVolume, Host destHost, StoragePoolVO destStoragePool, VolumeInfo destVolumeInfo) { + protected String generateDestPath(Host destHost, StoragePoolVO destStoragePool, VolumeInfo destVolumeInfo) { return connectHostToVolume(destHost, destVolumeInfo.getPoolId(), destVolumeInfo.get_iScsiName()); } diff --git a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java index c0d8ad3..89a2783 100644 --- a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java +++ b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java @@ -50,10 +50,6 @@ import org.mockito.runners.MockitoJUnitRunner; import com.cloud.agent.AgentManager; import com.cloud.agent.api.Answer; import com.cloud.agent.api.MigrateCommand; -import com.cloud.agent.api.storage.CreateAnswer; -import com.cloud.agent.api.storage.CreateCommand; -import com.cloud.agent.api.to.VirtualMachineTO; -import com.cloud.agent.api.to.VolumeTO; import com.cloud.exception.AgentUnavailableException; import com.cloud.exception.CloudException; import com.cloud.exception.OperationTimedoutException; @@ -61,18 +57,13 @@ import com.cloud.host.Host; import com.cloud.host.HostVO; import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.storage.DataStoreRole; -import com.cloud.storage.DiskOfferingVO; -import com.cloud.storage.Storage; import com.cloud.storage.StoragePool; import com.cloud.storage.VMTemplateStoragePoolVO; import com.cloud.storage.Storage.ImageFormat; import com.cloud.storage.Storage.StoragePoolType; -import com.cloud.storage.Volume; -import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.DiskOfferingDao; import com.cloud.storage.dao.VMTemplatePoolDao; import com.cloud.utils.exception.CloudRuntimeException; -import com.cloud.vm.DiskProfile; import com.cloud.vm.VirtualMachineManager; @RunWith(MockitoJUnitRunner.class) @@ -203,64 +194,6 @@ public class KvmNonManagedStorageSystemDataMotionTest { } @Test - public void generateDestPathTest() { - configureAndVerifygenerateDestPathTest(true, false); - } - - @Test(expected = CloudRuntimeException.class) - public void generateDestPathTestExpectCloudRuntimeException() { - configureAndVerifygenerateDestPathTest(false, false); - } - - @Test(expected = CloudRuntimeException.class) - public void generateDestPathTestExpectCloudRuntimeException2() { - configureAndVerifygenerateDestPathTest(false, true); - } - - private void configureAndVerifygenerateDestPathTest(boolean answerResult, boolean answerIsNull) { - String uuid = "f3d49ecc-870c-475a-89fa-fd0124420a9b"; - String destPath = "/var/lib/libvirt/images/"; - - VirtualMachineTO vmTO = Mockito.mock(VirtualMachineTO.class); - Mockito.when(vmTO.getName()).thenReturn("vmName"); - - VolumeVO srcVolume = Mockito.spy(new VolumeVO("name", 0l, 0l, 0l, 0l, 0l, "folder", "path", Storage.ProvisioningType.THIN, 0l, Volume.Type.ROOT)); - StoragePoolVO destStoragePool = Mockito.spy(new StoragePoolVO()); - - VolumeInfo destVolumeInfo = Mockito.spy(new VolumeObject()); - Mockito.doReturn(0l).when(destVolumeInfo).getTemplateId(); - Mockito.doReturn(0l).when(destVolumeInfo).getId(); - Mockito.doReturn(Volume.Type.ROOT).when(destVolumeInfo).getVolumeType(); - Mockito.doReturn("name").when(destVolumeInfo).getName(); - Mockito.doReturn(0l).when(destVolumeInfo).getSize(); - Mockito.doReturn(uuid).when(destVolumeInfo).getUuid(); - - DiskOfferingVO diskOffering = Mockito.spy(new DiskOfferingVO()); - Mockito.doReturn(0l).when(diskOffering).getId(); - Mockito.doReturn(diskOffering).when(diskOfferingDao).findById(0l); - DiskProfile diskProfile = Mockito.spy(new DiskProfile(destVolumeInfo, diskOffering, HypervisorType.KVM)); - - String templateUuid = Mockito.doReturn("templateUuid").when(kvmNonManagedStorageDataMotionStrategy).getTemplateUuid(0l); - CreateCommand rootImageProvisioningCommand = new CreateCommand(diskProfile, templateUuid, destStoragePool, true); - CreateAnswer createAnswer = Mockito.spy(new CreateAnswer(rootImageProvisioningCommand, "details")); - Mockito.doReturn(answerResult).when(createAnswer).getResult(); - - VolumeTO volumeTo = Mockito.mock(VolumeTO.class); - Mockito.doReturn(destPath).when(volumeTo).getName(); - Mockito.doReturn(volumeTo).when(createAnswer).getVolume(); - - if (answerIsNull) { - Mockito.doReturn(null).when(agentManager).easySend(0l, rootImageProvisioningCommand); - } else { - Mockito.doReturn(createAnswer).when(agentManager).easySend(0l, rootImageProvisioningCommand); - } - - String generatedDestPath = kvmNonManagedStorageDataMotionStrategy.generateDestPath(vmTO, srcVolume, new HostVO("sourceHostUuid"), destStoragePool, destVolumeInfo); - - Assert.assertEquals(destPath + uuid, generatedDestPath); - } - - @Test public void shouldMigrateVolumeTest() { StoragePoolVO sourceStoragePool = Mockito.spy(new StoragePoolVO()); HostVO destHost = new HostVO("guid"); diff --git a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategyTest.java b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategyTest.java index d76ff27..197e66c 100644 --- a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategyTest.java +++ b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategyTest.java @@ -47,7 +47,6 @@ import org.mockito.Spy; import org.mockito.runners.MockitoJUnitRunner; import com.cloud.agent.api.MigrateCommand; -import com.cloud.agent.api.to.VirtualMachineTO; import com.cloud.host.HostVO; import com.cloud.storage.DataStoreRole; import com.cloud.storage.ImageStore; @@ -164,8 +163,7 @@ public class StorageSystemDataMotionStrategyTest { Mockito.doReturn(0l).when(destVolumeInfo).getPoolId(); Mockito.doReturn("expected").when(storageSystemDataMotionStrategy).connectHostToVolume(destHost, 0l, "iScsiName"); - String expected = storageSystemDataMotionStrategy.generateDestPath(Mockito.mock(VirtualMachineTO.class), Mockito.mock(VolumeVO.class), destHost, - Mockito.mock(StoragePoolVO.class), destVolumeInfo); + String expected = storageSystemDataMotionStrategy.generateDestPath(destHost, Mockito.mock(StoragePoolVO.class), destVolumeInfo); Assert.assertEquals(expected, "expected"); Mockito.verify(storageSystemDataMotionStrategy).connectHostToVolume(destHost, 0l, "iScsiName");