This is an automated email from the ASF dual-hosted git repository.
gabriel pushed a commit to branch kvm-filesystem-datamotion-strategy
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit 2e5de075d5155e8754234f92cd7965d44f76a71b
Author: GabrielBrascher <gabr...@pcextreme.nl>
AuthorDate: Thu Nov 8 02:48:42 2018 -0200
Allow KVM VM live migration with ROOT volume on file
- Add JUnit tests
---
.../KvmNonManagedStorageDataMotionStrategy.java | 27 ++-
.../motion/StorageSystemDataMotionStrategy.java | 21 +-
.../KvmNonManagedStorageSystemDataMotionTest.java | 256 +++++++++++++++++++++
.../StorageSystemDataMotionStrategyTest.java | 151 ++++++++++--
.../wrapper/LibvirtMigrateCommandWrapper.java | 27 ++-
.../wrapper/LibvirtMigrateCommandWrapperTest.java | 132 ++++++++++-
6 files changed, 568 insertions(+), 46 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 037a89d..9260fb6 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
@@ -30,7 +30,6 @@ import
org.apache.cloudstack.engine.subsystem.api.storage.TemplateDataFactory;
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo;
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
-import org.apache.commons.lang.StringUtils;
import org.springframework.stereotype.Component;
import com.cloud.agent.api.Answer;
@@ -56,10 +55,10 @@ import com.cloud.vm.DiskProfile;
public class KvmNonManagedStorageDataMotionStrategy extends
StorageSystemDataMotionStrategy {
@Inject
- private TemplateDataFactory tmplFactory;
+ private TemplateDataFactory templateDataFactory;
/**
- * Uses the canHandle from the Super class {@link
StorageSystemDataMotionStrategy}. If the storage pool is of file and the
internalCanHandle from {@link StorageSystemDataMotionStrategy} CANT_HANDLE,
returns the HIGHEST strategy priority. otherwise returns CANT_HANDLE.
+ * Uses the canHandle from the Super class {@link
StorageSystemDataMotionStrategy}. If the storage pool is of file and the
internalCanHandle from {@link StorageSystemDataMotionStrategy} CANT_HANDLE,
returns the StrategyPriority.HYPERVISOR strategy priority. otherwise returns
CANT_HANDLE.
* Note that the super implementation (override) is called by {@link
#canHandle(Map, Host, Host)} which ensures that {@link #internalCanHandle(Map)}
will be executed only if the source host is KVM.
*/
@Override
@@ -69,7 +68,7 @@ public class KvmNonManagedStorageDataMotionStrategy extends
StorageSystemDataMot
for (VolumeInfo volumeInfo : volumeInfoSet) {
StoragePoolVO storagePoolVO =
_storagePoolDao.findById(volumeInfo.getPoolId());
- if (storagePoolVO.getPoolType() != StoragePoolType.Filesystem)
{
+ if (storagePoolVO.getPoolType() != StoragePoolType.Filesystem
&& storagePoolVO.getPoolType() != StoragePoolType.NetworkFilesystem) {
return StrategyPriority.CANT_HANDLE;
}
}
@@ -108,21 +107,22 @@ public class KvmNonManagedStorageDataMotionStrategy
extends StorageSystemDataMot
throw new CloudRuntimeException(String.format("Unable to modify
target volume on the host [host id:%s, name:%s]", destHost.getId(),
destHost.getName()));
}
- String libvirtDestImgsPath = StringUtils.EMPTY;
+ String libvirtDestImgsPath = null;
if (rootImageProvisioningAnswer instanceof CreateAnswer) {
- libvirtDestImgsPath =
((CreateAnswer)rootImageProvisioningAnswer).getVolume().getName() +
File.separator;
+ libvirtDestImgsPath =
((CreateAnswer)rootImageProvisioningAnswer).getVolume().getName();
}
- return libvirtDestImgsPath + destVolumeInfo.getUuid();
+ // 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();
}
/**
- * Returns the template UUID of the given {@link VolumeVO}.
+ * Returns the template UUID with the given id. If the template ID is
null, it returns null.
*/
protected String getTemplateUuid(Long templateId) {
if (templateId == null) {
return null;
}
- TemplateInfo templateImage = tmplFactory.getTemplate(templateId,
DataStoreRole.Image);
+ TemplateInfo templateImage =
templateDataFactory.getTemplate(templateId, DataStoreRole.Image);
return templateImage.getUuid();
}
@@ -133,4 +133,13 @@ public class KvmNonManagedStorageDataMotionStrategy
extends StorageSystemDataMot
protected void setVolumePath(VolumeVO volume) {
volume.setPath(volume.getUuid());
}
+
+ /**
+ * Return true if the volume should be migrated. Currently only supports
migrating volumes on storage pool of the type StoragePoolType.Filesystem.
+ * This ensures that volumes on shared storage are not migrated and those
on local storage pools are migrated.
+ */
+ @Override
+ protected boolean shouldMigrateVolume(StoragePoolVO sourceStoragePool,
Host destHost, StoragePoolVO destStoragePool) {
+ return sourceStoragePool.getPoolType() == StoragePoolType.Filesystem;
+ }
}
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 9ecf3f1..948c21c 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
@@ -1687,10 +1687,12 @@ public class StorageSystemDataMotionStrategy implements
DataMotionStrategy {
/**
* For each disk to migrate:
- * Create a volume on the target storage system.
- * Make the newly created volume accessible to the target KVM host.
- * Send a command to the target KVM host to connect to the newly created
volume.
- * Send a command to the source KVM host to migrate the VM and its storage.
+ * <ul>
+ * <li>Create a volume on the target storage system.</li>
+ * <li>Make the newly created volume accessible to the target KVM
host.</li>
+ * <li>Send a command to the target KVM host to connect to the newly
created volume.</li>
+ * <li>Send a command to the source KVM host to migrate the VM and its
storage.</li>
+ * </ul>
*/
@Override
public void copyAsync(Map<VolumeInfo, DataStore> volumeDataStoreMap,
VirtualMachineTO vmTO, Host srcHost, Host destHost,
AsyncCompletionCallback<CopyCommandResult> callback) {
@@ -1718,6 +1720,10 @@ public class StorageSystemDataMotionStrategy implements
DataMotionStrategy {
StoragePoolVO destStoragePool =
_storagePoolDao.findById(destDataStore.getId());
StoragePoolVO sourceStoragePool =
_storagePoolDao.findById(srcVolumeInfo.getPoolId());
+ if (!shouldMigrateVolume(sourceStoragePool, destHost,
destStoragePool)) {
+ continue;
+ }
+
VolumeVO destVolume =
duplicateVolumeOnAnotherStorage(srcVolume, destStoragePool);
VolumeInfo destVolumeInfo =
_volumeDataFactory.getVolume(destVolume.getId(), destDataStore);
@@ -1818,6 +1824,13 @@ public class StorageSystemDataMotionStrategy implements
DataMotionStrategy {
}
/**
+ * Returns true. This method was implemented considering the classes that
extend this {@link StorageSystemDataMotionStrategy} and cannot migrate volumes
from certain types of source storage pools and/or to a different kind of
destiny storage pool.
+ */
+ protected boolean shouldMigrateVolume(StoragePoolVO sourceStoragePool,
Host destHost, StoragePoolVO destStoragePool) {
+ return true;
+ }
+
+ /**
* Returns true if the storage pool type is {@link
StoragePoolType.Filesystem}.
*/
protected boolean isStoragePoolTypeOfFile(StoragePoolVO sourceStoragePool)
{
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
new file mode 100644
index 0000000..320d213
--- /dev/null
+++
b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java
@@ -0,0 +1,256 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.cloudstack.storage.motion;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
+import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
+import org.apache.cloudstack.engine.subsystem.api.storage.TemplateDataFactory;
+import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
+import org.apache.cloudstack.storage.datastore.PrimaryDataStoreImpl;
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.cloudstack.storage.volume.VolumeObject;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import com.cloud.agent.AgentManager;
+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.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.Storage.StoragePoolType;
+import com.cloud.storage.Volume;
+import com.cloud.storage.VolumeVO;
+import com.cloud.storage.dao.DiskOfferingDao;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.vm.DiskProfile;
+
+@RunWith(MockitoJUnitRunner.class)
+public class KvmNonManagedStorageSystemDataMotionTest {
+
+ @Mock
+ private PrimaryDataStoreDao primaryDataStoreDao;
+
+ @Mock
+ private TemplateDataFactory templateFactory;
+
+ @Mock
+ private AgentManager agentMgr;
+
+ @Mock
+ private DiskOfferingDao diskOfferingDao;
+
+ @Spy
+ @InjectMocks
+ private KvmNonManagedStorageDataMotionStrategy
kvmStorageDataMotionStrategy;
+
+ @Test
+ public void canHandleTestExpectHypervisorStrategyForKvm() {
+ canHandleExpectCannotHandle(HypervisorType.KVM, 1,
StrategyPriority.HYPERVISOR);
+ }
+
+ @Test
+ public void canHandleTestExpectCannotHandle() {
+ HypervisorType[] hypervisorTypeArray = HypervisorType.values();
+ for (int i = 0; i < hypervisorTypeArray.length; i++) {
+ HypervisorType ht = hypervisorTypeArray[i];
+ if (ht.equals(HypervisorType.KVM)) {
+ continue;
+ }
+ canHandleExpectCannotHandle(ht, 0, StrategyPriority.CANT_HANDLE);
+ }
+ }
+
+ private void canHandleExpectCannotHandle(HypervisorType hypervisorType,
int times, StrategyPriority expectedStrategyPriority) {
+ HostVO srcHost = new HostVO("sourceHostUuid");
+ srcHost.setHypervisorType(hypervisorType);
+
Mockito.doReturn(StrategyPriority.HYPERVISOR).when(kvmStorageDataMotionStrategy).internalCanHandle(new
HashMap<>());
+
+ StrategyPriority strategyPriority =
kvmStorageDataMotionStrategy.canHandle(new HashMap<>(), srcHost, new
HostVO("destHostUuid"));
+
+ Mockito.verify(kvmStorageDataMotionStrategy,
Mockito.times(times)).internalCanHandle(new HashMap<>());
+ Assert.assertEquals(expectedStrategyPriority, strategyPriority);
+ }
+
+ @Test
+ public void internalCanHandleTestNonManaged() {
+ StoragePoolType[] storagePoolTypeArray = StoragePoolType.values();
+ for (int i = 0; i < storagePoolTypeArray.length; i++) {
+ Map<VolumeInfo, DataStore> volumeMap =
configureTestInternalCanHandle(false, storagePoolTypeArray[i]);
+ StrategyPriority strategyPriority =
kvmStorageDataMotionStrategy.internalCanHandle(volumeMap);
+ if (storagePoolTypeArray[i] == StoragePoolType.Filesystem ||
storagePoolTypeArray[i] == StoragePoolType.NetworkFilesystem) {
+ Assert.assertEquals(StrategyPriority.HYPERVISOR,
strategyPriority);
+ } else {
+ Assert.assertEquals(StrategyPriority.CANT_HANDLE,
strategyPriority);
+ }
+ }
+ }
+
+ @Test
+ public void internalCanHandleTestIsManaged() {
+ StoragePoolType[] storagePoolTypeArray = StoragePoolType.values();
+ for (int i = 0; i < storagePoolTypeArray.length; i++) {
+ Map<VolumeInfo, DataStore> volumeMap =
configureTestInternalCanHandle(true, storagePoolTypeArray[i]);
+ StrategyPriority strategyPriority =
kvmStorageDataMotionStrategy.internalCanHandle(volumeMap);
+ Assert.assertEquals(StrategyPriority.CANT_HANDLE,
strategyPriority);
+ }
+ }
+
+ private Map<VolumeInfo, DataStore> configureTestInternalCanHandle(boolean
isManagedStorage, StoragePoolType storagePoolType) {
+ VolumeObject volumeInfo = Mockito.spy(new VolumeObject());
+ Mockito.doReturn(0l).when(volumeInfo).getPoolId();
+ DataStore ds = Mockito.spy(new PrimaryDataStoreImpl());
+ Mockito.doReturn(0l).when(ds).getId();
+
+ Map<VolumeInfo, DataStore> volumeMap = new HashMap<>();
+ volumeMap.put(volumeInfo, ds);
+
+ StoragePoolVO storagePool = Mockito.spy(new StoragePoolVO());
+ Mockito.doReturn(storagePoolType).when(storagePool).getPoolType();
+
+ Mockito.doReturn(storagePool).when(primaryDataStoreDao).findById(0l);
+ Mockito.doReturn(isManagedStorage).when(storagePool).isManaged();
+ return volumeMap;
+ }
+
+ @Test
+ public void getTemplateUuidTestTemplateIdNotNull() {
+ String expectedTemplateUuid = prepareTestGetTemplateUuid();
+ String templateUuid = kvmStorageDataMotionStrategy.getTemplateUuid(0l);
+ Assert.assertEquals(expectedTemplateUuid, templateUuid);
+ }
+
+ @Test
+ public void getTemplateUuidTestTemplateIdNull() {
+ prepareTestGetTemplateUuid();
+ String templateUuid =
kvmStorageDataMotionStrategy.getTemplateUuid(null);
+ Assert.assertEquals(null, templateUuid);
+ }
+
+ private String prepareTestGetTemplateUuid() {
+ TemplateInfo templateImage = Mockito.mock(TemplateInfo.class);
+ String expectedTemplateUuid = "template uuid";
+ Mockito.when(templateImage.getUuid()).thenReturn(expectedTemplateUuid);
+ Mockito.doReturn(templateImage).when(templateFactory).getTemplate(0l,
DataStoreRole.Image);
+ return expectedTemplateUuid;
+ }
+
+ @Test
+ public void configureMigrateDiskInfoTest() {
+ VolumeObject srcVolumeInfo = Mockito.spy(new VolumeObject());
+ Mockito.doReturn("volume path").when(srcVolumeInfo).getPath();
+ MigrateCommand.MigrateDiskInfo migrateDiskInfo =
kvmStorageDataMotionStrategy.configureMigrateDiskInfo(srcVolumeInfo,
"destPath");
+ Assert.assertEquals(MigrateCommand.MigrateDiskInfo.DiskType.FILE,
migrateDiskInfo.getDiskType());
+ Assert.assertEquals(MigrateCommand.MigrateDiskInfo.DriverType.QCOW2,
migrateDiskInfo.getDriverType());
+ Assert.assertEquals(MigrateCommand.MigrateDiskInfo.Source.FILE,
migrateDiskInfo.getSource());
+ Assert.assertEquals("destPath", migrateDiskInfo.getSourceText());
+ 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(kvmStorageDataMotionStrategy).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(agentMgr).easySend(0l,
rootImageProvisioningCommand);
+ } else {
+ Mockito.doReturn(createAnswer).when(agentMgr).easySend(0l,
rootImageProvisioningCommand);
+ }
+
+ String generatedDestPath =
kvmStorageDataMotionStrategy.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");
+ StoragePoolVO destStoragePool = new StoragePoolVO();
+ StoragePoolType[] storagePoolTypes = StoragePoolType.values();
+ for (int i = 0; i < storagePoolTypes.length; i++) {
+
Mockito.doReturn(storagePoolTypes[i]).when(sourceStoragePool).getPoolType();
+ boolean result =
kvmStorageDataMotionStrategy.shouldMigrateVolume(sourceStoragePool, destHost,
destStoragePool);
+ if (storagePoolTypes[i] == StoragePoolType.Filesystem) {
+ Assert.assertTrue(result);
+ } else {
+ Assert.assertFalse(result);
+ }
+ }
+ }
+}
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 ec85f7d..7e189c8 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
@@ -18,29 +18,43 @@
*/
package org.apache.cloudstack.storage.motion;
-import com.cloud.storage.DataStoreRole;
-import com.cloud.storage.ImageStore;
-import org.apache.cloudstack.engine.subsystem.api.storage.DataMotionStrategy;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.MockitoAnnotations.initMocks;
+
+import java.util.HashMap;
+import java.util.Map;
+
import org.apache.cloudstack.engine.subsystem.api.storage.DataObject;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore;
import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
import org.apache.cloudstack.storage.datastore.PrimaryDataStoreImpl;
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
import org.apache.cloudstack.storage.image.store.ImageStoreImpl;
import org.apache.cloudstack.storage.volume.VolumeObject;
+import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.Spy;
import org.mockito.runners.MockitoJUnitRunner;
-import static org.junit.Assert.assertTrue;
-import static org.mockito.Mockito.doReturn;
-import static org.mockito.Mockito.mock;
-import static org.mockito.MockitoAnnotations.initMocks;
+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;
+import com.cloud.storage.Storage;
+import com.cloud.storage.Volume;
+import com.cloud.storage.Storage.StoragePoolType;
+import com.cloud.storage.VolumeVO;
@RunWith(MockitoJUnitRunner.class)
public class StorageSystemDataMotionStrategyTest {
@@ -54,10 +68,12 @@ public class StorageSystemDataMotionStrategyTest {
@Mock
ImageStore destinationStore;
+ @Spy
@InjectMocks
- DataMotionStrategy strategy = new StorageSystemDataMotionStrategy();
+ StorageSystemDataMotionStrategy storageSystemDataMotionStrategy;
+
@Mock
- PrimaryDataStoreDao _storagePoolDao;
+ PrimaryDataStoreDao storagePoolDao;
@Before public void setUp() throws Exception {
sourceStore = mock(PrimaryDataStoreImpl.class);
@@ -65,7 +81,7 @@ public class StorageSystemDataMotionStrategyTest {
source = mock(VolumeObject.class);
destination = mock(VolumeObject.class);
- initMocks(strategy);
+ initMocks(storageSystemDataMotionStrategy);
}
@Test
@@ -77,8 +93,117 @@ public class StorageSystemDataMotionStrategyTest {
doReturn(sourceStore).when(source).getDataStore();
doReturn(destinationStore).when(destination).getDataStore();
StoragePoolVO storeVO = new StoragePoolVO();
- doReturn(storeVO).when(_storagePoolDao).findById(0l);
+ doReturn(storeVO).when(storagePoolDao).findById(0l);
+
+
assertTrue(storageSystemDataMotionStrategy.canHandle(source,destination) ==
StrategyPriority.CANT_HANDLE);
+ }
+
+ @Test
+ public void internalCanHandleTestAllStoragePoolsAreManaged() {
+ configureAndTestInternalCanHandle(true, true,
StrategyPriority.HIGHEST);
+ }
+
+ @Test
+ public void internalCanHandleTestFirstStoragePoolsIsManaged() {
+ configureAndTestInternalCanHandle(false, true,
StrategyPriority.HIGHEST);
+ }
+
+ @Test
+ public void internalCanHandleTestSecondStoragePoolsIsManaged() {
+ configureAndTestInternalCanHandle(true, false,
StrategyPriority.HIGHEST);
+ }
+
+ @Test
+ public void internalCanHandleTestNoStoragePoolsIsManaged() {
+ configureAndTestInternalCanHandle(false, false,
StrategyPriority.CANT_HANDLE);
+ }
+
+ private void configureAndTestInternalCanHandle(boolean sPool0IsManaged,
boolean sPool1IsManaged, StrategyPriority expectedStrategyPriority) {
+ VolumeObject volumeInfo = Mockito.spy(new VolumeObject());
+ Mockito.doReturn(0l).when(volumeInfo).getPoolId();
+
+ DataStore ds = Mockito.spy(new PrimaryDataStoreImpl());
+ Mockito.doReturn(1l).when(ds).getId();
+
+ Map<VolumeInfo, DataStore> volumeMap = new HashMap<>();
+ volumeMap.put(volumeInfo, ds);
+
+ StoragePoolVO storagePool0 = Mockito.spy(new StoragePoolVO());
+ Mockito.doReturn(sPool0IsManaged).when(storagePool0).isManaged();
+ StoragePoolVO storagePool1 = Mockito.spy(new StoragePoolVO());
+ Mockito.doReturn(sPool1IsManaged).when(storagePool1).isManaged();
+
+ Mockito.doReturn(storagePool0).when(storagePoolDao).findById(0l);
+ Mockito.doReturn(storagePool1).when(storagePoolDao).findById(1l);
- assertTrue(strategy.canHandle(source,destination) ==
StrategyPriority.CANT_HANDLE);
+ StrategyPriority strategyPriority =
storageSystemDataMotionStrategy.internalCanHandle(volumeMap);
+
+ Assert.assertEquals(expectedStrategyPriority, strategyPriority);
+ }
+
+ @Test
+ public void isStoragePoolTypeOfFileTest() {
+ StoragePoolVO sourceStoragePool = Mockito.spy(new StoragePoolVO());
+ StoragePoolType[] storagePoolTypeArray = StoragePoolType.values();
+ for (int i = 0; i < storagePoolTypeArray.length; i++) {
+
Mockito.doReturn(storagePoolTypeArray[i]).when(sourceStoragePool).getPoolType();
+ boolean result =
storageSystemDataMotionStrategy.isStoragePoolTypeOfFile(sourceStoragePool);
+ if (sourceStoragePool.getPoolType() == StoragePoolType.Filesystem)
{
+ Assert.assertTrue(result);
+ } else {
+ Assert.assertFalse(result);
+ }
+ }
+ }
+
+ @Test
+ public void generateDestPathTest() {
+ VolumeObject destVolumeInfo = Mockito.spy(new VolumeObject());
+ HostVO destHost = new HostVO("guid");
+ Mockito.doReturn("iScsiName").when(destVolumeInfo).get_iScsiName();
+ 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);
+
+ Assert.assertEquals(expected, "expected");
+
Mockito.verify(storageSystemDataMotionStrategy).connectHostToVolume(destHost,
0l, "iScsiName");
+ }
+
+ @Test
+ public void configureMigrateDiskInfoTest() {
+ VolumeObject srcVolumeInfo = Mockito.spy(new VolumeObject());
+ Mockito.doReturn("volume path").when(srcVolumeInfo).getPath();
+ MigrateCommand.MigrateDiskInfo migrateDiskInfo =
storageSystemDataMotionStrategy.configureMigrateDiskInfo(srcVolumeInfo,
"destPath");
+ Assert.assertEquals(MigrateCommand.MigrateDiskInfo.DiskType.BLOCK,
migrateDiskInfo.getDiskType());
+ Assert.assertEquals(MigrateCommand.MigrateDiskInfo.DriverType.RAW,
migrateDiskInfo.getDriverType());
+ Assert.assertEquals(MigrateCommand.MigrateDiskInfo.Source.DEV,
migrateDiskInfo.getSource());
+ Assert.assertEquals("destPath", migrateDiskInfo.getSourceText());
+ Assert.assertEquals("volume path", migrateDiskInfo.getSerialNumber());
+ }
+
+ @Test
+ public void setVolumePathTest() {
+ VolumeVO volume = new VolumeVO("name", 0l, 0l, 0l, 0l, 0l, "folder",
"path", Storage.ProvisioningType.THIN, 0l, Volume.Type.ROOT);
+ String volumePath = "iScsiName";
+ volume.set_iScsiName(volumePath);
+
+ storageSystemDataMotionStrategy.setVolumePath(volume);
+
+ Assert.assertEquals(volumePath, volume.getPath());
+ }
+
+ @Test
+ public void shouldMigrateVolumeTest() {
+ StoragePoolVO sourceStoragePool = Mockito.spy(new StoragePoolVO());
+ HostVO destHost = new HostVO("guid");
+ StoragePoolVO destStoragePool = new StoragePoolVO();
+ StoragePoolType[] storagePoolTypes = StoragePoolType.values();
+ for (int i = 0; i < storagePoolTypes.length; i++) {
+
Mockito.doReturn(storagePoolTypes[i]).when(sourceStoragePool).getPoolType();
+ boolean result =
storageSystemDataMotionStrategy.shouldMigrateVolume(sourceStoragePool,
destHost, destStoragePool);
+ Assert.assertTrue(result);
+ }
}
-}
\ No newline at end of file
+}
diff --git
a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java
b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java
index 174c836..2e3bd20 100644
---
a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java
+++
b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java
@@ -207,14 +207,7 @@ public final class LibvirtMigrateCommandWrapper extends
CommandWrapper<MigrateCo
destDomain = migrateThread.get(10, TimeUnit.SECONDS);
if (destDomain != null) {
- for (DiskDef disk : disks) {
- MigrateDiskInfo migrateDiskInfo =
searchDiskDefOnMigrateDiskInfoList(migrateDiskInfoList, disk);
- if (migrateDiskInfo != null &&
migrateDiskInfo.isSourceDiskOnStorageFileSystem()) {
- deleteLocalVolume(disk.getDiskPath());
- } else {
- libvirtComputingResource.cleanupDisk(disk);
- }
- }
+ deleteOrDisconnectDisksOnSourcePool(libvirtComputingResource,
migrateDiskInfoList, disks);
}
} catch (final LibvirtException e) {
@@ -289,7 +282,23 @@ public final class LibvirtMigrateCommandWrapper extends
CommandWrapper<MigrateCo
}
/**
- * Deletes the local volume from the storage pool
+ * In case of a local file, it deletes the file on the source host/storage
pool. Otherwise (for instance iScsi) it disconnects the disk on the source
storage pool. </br>
+ * This method must be executed after a successful migration to a target
storage pool, cleaning up the source storage.
+ */
+ protected void deleteOrDisconnectDisksOnSourcePool(final
LibvirtComputingResource libvirtComputingResource, final List<MigrateDiskInfo>
migrateDiskInfoList,
+ List<DiskDef> disks) {
+ for (DiskDef disk : disks) {
+ MigrateDiskInfo migrateDiskInfo =
searchDiskDefOnMigrateDiskInfoList(migrateDiskInfoList, disk);
+ if (migrateDiskInfo != null &&
migrateDiskInfo.isSourceDiskOnStorageFileSystem()) {
+ deleteLocalVolume(disk.getDiskPath());
+ } else {
+ libvirtComputingResource.cleanupDisk(disk);
+ }
+ }
+ }
+
+ /**
+ * Deletes the local volume from the storage pool.
*/
protected void deleteLocalVolume(String localPath) {
try {
diff --git
a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapperTest.java
b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapperTest.java
index da71e40..f703a4a 100644
---
a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapperTest.java
+++
b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapperTest.java
@@ -21,11 +21,31 @@ package com.cloud.hypervisor.kvm.resource.wrapper;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.junit.Assert;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.libvirt.Connect;
+import org.libvirt.StorageVol;
+import org.mockito.InOrder;
+import org.mockito.Mockito;
+import org.powermock.api.mockito.PowerMockito;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+import com.cloud.agent.api.MigrateCommand.MigrateDiskInfo;
+import com.cloud.agent.api.MigrateCommand.MigrateDiskInfo.DiskType;
+import com.cloud.agent.api.MigrateCommand.MigrateDiskInfo.DriverType;
+import com.cloud.agent.api.MigrateCommand.MigrateDiskInfo.Source;
import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.hypervisor.kvm.resource.LibvirtConnection;
+import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.DiskDef;
import com.cloud.utils.exception.CloudRuntimeException;
+@RunWith(PowerMockRunner.class)
+@PrepareForTest({LibvirtConnection.class, LibvirtMigrateCommandWrapper.class})
public class LibvirtMigrateCommandWrapperTest {
String fullfile =
"<domain type='kvm' id='4'>\n" +
@@ -252,11 +272,12 @@ public class LibvirtMigrateCommandWrapperTest {
" </devices>\n" +
"</domain>";
+ LibvirtMigrateCommandWrapper libvirtMigrateCmdWrapper = new
LibvirtMigrateCommandWrapper();
+
@Test
public void testReplaceIpForVNCInDescFile() {
final String targetIp = "192.168.22.21";
- final LibvirtMigrateCommandWrapper lw = new
LibvirtMigrateCommandWrapper();
- final String result = lw.replaceIpForVNCInDescFile(fullfile, targetIp);
+ final String result =
libvirtMigrateCmdWrapper.replaceIpForVNCInDescFile(fullfile, targetIp);
assertTrue("transformation does not live up to expectation:\n" +
result, targetfile.equals(result));
}
@@ -279,8 +300,7 @@ public class LibvirtMigrateCommandWrapperTest {
" </devices>" +
"</domain>";
final String targetIp = "10.10.10.10";
- final LibvirtMigrateCommandWrapper lw = new
LibvirtMigrateCommandWrapper();
- final String result = lw.replaceIpForVNCInDescFile(xmlDesc, targetIp);
+ final String result =
libvirtMigrateCmdWrapper.replaceIpForVNCInDescFile(xmlDesc, targetIp);
assertTrue("transformation does not live up to expectation:\n" +
result, expectedXmlDesc.equals(result));
}
@@ -303,26 +323,116 @@ public class LibvirtMigrateCommandWrapperTest {
" </devices>" +
"</domain>";
final String targetIp = "localhost.localdomain";
- final LibvirtMigrateCommandWrapper lw = new
LibvirtMigrateCommandWrapper();
- final String result = lw.replaceIpForVNCInDescFile(xmlDesc, targetIp);
+ final String result =
libvirtMigrateCmdWrapper.replaceIpForVNCInDescFile(xmlDesc, targetIp);
assertTrue("transformation does not live up to expectation:\n" +
result, expectedXmlDesc.equals(result));
}
@Test
public void testMigrationUri() {
final String ip = "10.1.1.1";
- LibvirtMigrateCommandWrapper lw = new LibvirtMigrateCommandWrapper();
LibvirtComputingResource lcr = new LibvirtComputingResource();
if (lcr.isHostSecured()) {
- assertEquals(lw.createMigrationURI(ip, lcr),
String.format("qemu+tls://%s/system", ip));
+ assertEquals(libvirtMigrateCmdWrapper.createMigrationURI(ip, lcr),
String.format("qemu+tls://%s/system", ip));
} else {
- assertEquals(lw.createMigrationURI(ip, lcr),
String.format("qemu+tcp://%s/system", ip));
+ assertEquals(libvirtMigrateCmdWrapper.createMigrationURI(ip, lcr),
String.format("qemu+tcp://%s/system", ip));
}
}
@Test(expected = CloudRuntimeException.class)
public void testMigrationUriException() {
- LibvirtMigrateCommandWrapper lw = new LibvirtMigrateCommandWrapper();
- lw.createMigrationURI(null, new LibvirtComputingResource());
+ libvirtMigrateCmdWrapper.createMigrationURI(null, new
LibvirtComputingResource());
+ }
+
+ @Test
+ public void deleteLocalVolumeTest() throws Exception {
+ PowerMockito.mockStatic(LibvirtConnection.class);
+ Connect conn = Mockito.mock(Connect.class);
+
+ PowerMockito.doReturn(conn).when(LibvirtConnection.class,
"getConnection");
+
+ StorageVol storageVolLookupByPath = Mockito.mock(StorageVol.class);
+
Mockito.when(conn.storageVolLookupByPath("localPath")).thenReturn(storageVolLookupByPath);
+
+ libvirtMigrateCmdWrapper.deleteLocalVolume("localPath");
+
+ PowerMockito.verifyStatic(Mockito.times(1));
+ LibvirtConnection.getConnection();
+ InOrder inOrder = Mockito.inOrder(conn, storageVolLookupByPath);
+ inOrder.verify(conn,
Mockito.times(1)).storageVolLookupByPath("localPath");
+ inOrder.verify(storageVolLookupByPath, Mockito.times(1)).delete(0);
}
+
+ @Test
+ public void searchDiskDefOnMigrateDiskInfoListTest() {
+
configureAndVerifyTestSearchDiskDefOnMigrateDiskInfoList("f3d49ecc-870c-475a-89fa-fd0124420a9b",
"/var/lib/libvirt/images/f3d49ecc-870c-475a-89fa-fd0124420a9b", false);
+ }
+
+ @Test
+ public void searchDiskDefOnMigrateDiskInfoListTestExpectNull() {
+
configureAndVerifyTestSearchDiskDefOnMigrateDiskInfoList("f3d49ecc-870c-475a-89fa-fd0124420a9b",
"/var/lib/libvirt/images/f3d49ecc-870c-89fa-fd0124420a9b", true);
+ }
+
+ private void
configureAndVerifyTestSearchDiskDefOnMigrateDiskInfoList(String serialNumber,
String diskPath, boolean isExpectedDiskInfoNull) {
+ MigrateDiskInfo migrateDiskInfo = new MigrateDiskInfo(serialNumber,
DiskType.FILE, DriverType.QCOW2, Source.FILE, "sourceText");
+ List<MigrateDiskInfo> migrateDiskInfoList = new ArrayList<>();
+ migrateDiskInfoList.add(migrateDiskInfo);
+
+ DiskDef disk = new DiskDef();
+ disk.setDiskPath(diskPath);
+
+ MigrateDiskInfo returnedMigrateDiskInfo =
libvirtMigrateCmdWrapper.searchDiskDefOnMigrateDiskInfoList(migrateDiskInfoList,
disk);
+
+ if (isExpectedDiskInfoNull)
+ Assert.assertEquals(null, returnedMigrateDiskInfo);
+ else
+ Assert.assertEquals(migrateDiskInfo, returnedMigrateDiskInfo);
+ }
+
+ @Test
+ public void deleteOrDisconnectDisksOnSourcePoolTest() {
+ LibvirtMigrateCommandWrapper spyLibvirtMigrateCmdWrapper =
PowerMockito.spy(libvirtMigrateCmdWrapper);
+
Mockito.doNothing().when(spyLibvirtMigrateCmdWrapper).deleteLocalVolume("volPath");
+
+ List<MigrateDiskInfo> migrateDiskInfoList = new ArrayList<>();
+ MigrateDiskInfo migrateDiskInfo0 = createMigrateDiskInfo(true);
+ MigrateDiskInfo migrateDiskInfo2 = createMigrateDiskInfo(false);
+
+ List<DiskDef> disks = new ArrayList<>();
+ DiskDef diskDef0 = new DiskDef();
+ DiskDef diskDef1 = new DiskDef();
+ DiskDef diskDef2 = new DiskDef();
+
+ diskDef0.setDiskPath("volPath");
+ disks.add(diskDef0);
+ disks.add(diskDef1);
+ disks.add(diskDef2);
+
+ LibvirtComputingResource libvirtComputingResource = Mockito.spy(new
LibvirtComputingResource());
+
Mockito.doReturn(true).when(libvirtComputingResource).cleanupDisk(diskDef1);
+
+
Mockito.doReturn(migrateDiskInfo0).when(spyLibvirtMigrateCmdWrapper).searchDiskDefOnMigrateDiskInfoList(migrateDiskInfoList,
diskDef0);
+
Mockito.doReturn(null).when(spyLibvirtMigrateCmdWrapper).searchDiskDefOnMigrateDiskInfoList(migrateDiskInfoList,
diskDef1);
+
Mockito.doReturn(migrateDiskInfo2).when(spyLibvirtMigrateCmdWrapper).searchDiskDefOnMigrateDiskInfoList(migrateDiskInfoList,
diskDef2);
+
+
spyLibvirtMigrateCmdWrapper.deleteOrDisconnectDisksOnSourcePool(libvirtComputingResource,
migrateDiskInfoList, disks);
+
+ InOrder inOrder = Mockito.inOrder(spyLibvirtMigrateCmdWrapper,
libvirtComputingResource);
+ inOrderVerifyDeleteOrDisconnect(inOrder, spyLibvirtMigrateCmdWrapper,
libvirtComputingResource, migrateDiskInfoList, diskDef0, 1, 0);
+ inOrderVerifyDeleteOrDisconnect(inOrder, spyLibvirtMigrateCmdWrapper,
libvirtComputingResource, migrateDiskInfoList, diskDef1, 0, 1);
+ inOrderVerifyDeleteOrDisconnect(inOrder, spyLibvirtMigrateCmdWrapper,
libvirtComputingResource, migrateDiskInfoList, diskDef2, 0, 1);
+ }
+
+ private MigrateDiskInfo createMigrateDiskInfo(boolean
isSourceDiskOnStorageFileSystem) {
+ MigrateDiskInfo migrateDiskInfo = new MigrateDiskInfo("serialNumber",
DiskType.FILE, DriverType.QCOW2, Source.FILE, "sourceText");
+
migrateDiskInfo.setSourceDiskOnStorageFileSystem(isSourceDiskOnStorageFileSystem);
+ return migrateDiskInfo;
+ }
+
+ private void inOrderVerifyDeleteOrDisconnect(InOrder inOrder,
LibvirtMigrateCommandWrapper lw, LibvirtComputingResource virtResource,
List<MigrateDiskInfo> diskInfoList,
+ DiskDef disk, int timesDelete, int timesCleanup) {
+ inOrder.verify(lw).searchDiskDefOnMigrateDiskInfoList(diskInfoList,
disk);
+ inOrder.verify(lw,
Mockito.times(timesDelete)).deleteLocalVolume("volPath");
+ inOrder.verify(virtResource,
Mockito.times(timesCleanup)).cleanupDisk(disk);
+ }
+
}
