This is an automated email from the ASF dual-hosted git repository.
rohit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/master by this push:
new 25c4f7f kvm: Remove code that generated /var/lib/libvirt/images/null
on target host (#3280)
25c4f7f is described below
commit 25c4f7fc088499565eb6df7ead0b57304c78be42
Author: Gabriel Beims Bräscher <[email protected]>
AuthorDate: Mon May 27 09:45:29 2019 -0300
kvm: Remove code that generated /var/lib/libvirt/images/null on target host
(#3280)
This commit simplifies the generateDestPath method and fixes an issue where
an extra file, named as 'null', was created on the target storage pool during
VM local storage volume migration. Without this fix, the VM is migrated and
there is no data loss; however, 193 KB is allocated for the unused file named
as 'null' and the file stays on the target storage.
---
.../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");
