apache/cloudstack
1
0
Fork 0
mirror of https://github.com/apache/cloudstack.git synced 2025-11-03 04:12:31 +01:00
Code Issues Packages Projects Releases Wiki Activity

kvm: Remove code that generated /var/lib/libvirt/images/null on target host (#3280)

Browse Source 
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.
This commit is contained in:
Gabriel Beims Bräscher 2019-05-27 09:45:29 -03:00 committed by Rohit Yadav
parent e8c1deb25d
commit 25c4f7fc08
4 changed files with 5 additions and 99 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java
View File

@ -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();
}
/**

4
engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
View File

@ -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());
}

67
engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java
View File

@ -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)
@ -202,64 +193,6 @@ public class KvmNonManagedStorageSystemDataMotionTest {
Assert.assertEquals("volume path", migrateDiskInfo.getSerialNumber());
}
@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());

4
engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategyTest.java
View File

@ -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");