Skip to content

Commit 2e5de07

Browse files
author
GabrielBrascher
committed
Allow KVM VM live migration with ROOT volume on file
- Add JUnit tests
1 parent dec6ba7 commit 2e5de07

File tree

6 files changed

+568
-46
lines changed

6 files changed

+568
-46
lines changed

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

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo;
3131
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
3232
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
33-
import org.apache.commons.lang.StringUtils;
3433
import org.springframework.stereotype.Component;
3534

3635
import com.cloud.agent.api.Answer;
@@ -56,10 +55,10 @@
5655
public class KvmNonManagedStorageDataMotionStrategy extends StorageSystemDataMotionStrategy {
5756

5857
@Inject
59-
private TemplateDataFactory tmplFactory;
58+
private TemplateDataFactory templateDataFactory;
6059

6160
/**
62-
* 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.
61+
* 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.
6362
* 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.
6463
*/
6564
@Override
@@ -69,7 +68,7 @@ protected StrategyPriority internalCanHandle(Map<VolumeInfo, DataStore> volumeMa
6968

7069
for (VolumeInfo volumeInfo : volumeInfoSet) {
7170
StoragePoolVO storagePoolVO = _storagePoolDao.findById(volumeInfo.getPoolId());
72-
if (storagePoolVO.getPoolType() != StoragePoolType.Filesystem) {
71+
if (storagePoolVO.getPoolType() != StoragePoolType.Filesystem && storagePoolVO.getPoolType() != StoragePoolType.NetworkFilesystem) {
7372
return StrategyPriority.CANT_HANDLE;
7473
}
7574
}
@@ -108,21 +107,22 @@ protected String generateDestPath(VirtualMachineTO vmTO, VolumeVO srcVolume, Hos
108107
throw new CloudRuntimeException(String.format("Unable to modify target volume on the host [host id:%s, name:%s]", destHost.getId(), destHost.getName()));
109108
}
110109

111-
String libvirtDestImgsPath = StringUtils.EMPTY;
110+
String libvirtDestImgsPath = null;
112111
if (rootImageProvisioningAnswer instanceof CreateAnswer) {
113-
libvirtDestImgsPath = ((CreateAnswer)rootImageProvisioningAnswer).getVolume().getName() + File.separator;
112+
libvirtDestImgsPath = ((CreateAnswer)rootImageProvisioningAnswer).getVolume().getName();
114113
}
115-
return libvirtDestImgsPath + destVolumeInfo.getUuid();
114+
// 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.
115+
return new File(libvirtDestImgsPath, destVolumeInfo.getUuid()).getAbsolutePath();
116116
}
117117

118118
/**
119-
* Returns the template UUID of the given {@link VolumeVO}.
119+
* Returns the template UUID with the given id. If the template ID is null, it returns null.
120120
*/
121121
protected String getTemplateUuid(Long templateId) {
122122
if (templateId == null) {
123123
return null;
124124
}
125-
TemplateInfo templateImage = tmplFactory.getTemplate(templateId, DataStoreRole.Image);
125+
TemplateInfo templateImage = templateDataFactory.getTemplate(templateId, DataStoreRole.Image);
126126
return templateImage.getUuid();
127127
}
128128

@@ -133,4 +133,13 @@ protected String getTemplateUuid(Long templateId) {
133133
protected void setVolumePath(VolumeVO volume) {
134134
volume.setPath(volume.getUuid());
135135
}
136+
137+
/**
138+
* Return true if the volume should be migrated. Currently only supports migrating volumes on storage pool of the type StoragePoolType.Filesystem.
139+
* This ensures that volumes on shared storage are not migrated and those on local storage pools are migrated.
140+
*/
141+
@Override
142+
protected boolean shouldMigrateVolume(StoragePoolVO sourceStoragePool, Host destHost, StoragePoolVO destStoragePool) {
143+
return sourceStoragePool.getPoolType() == StoragePoolType.Filesystem;
144+
}
136145
}

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

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1687,10 +1687,12 @@ private SnapshotDetailsVO handleSnapshotDetails(long csSnapshotId, String value)
16871687

16881688
/**
16891689
* For each disk to migrate:
1690-
* Create a volume on the target storage system.
1691-
* Make the newly created volume accessible to the target KVM host.
1692-
* Send a command to the target KVM host to connect to the newly created volume.
1693-
* Send a command to the source KVM host to migrate the VM and its storage.
1690+
* <ul>
1691+
* <li>Create a volume on the target storage system.</li>
1692+
* <li>Make the newly created volume accessible to the target KVM host.</li>
1693+
* <li>Send a command to the target KVM host to connect to the newly created volume.</li>
1694+
* <li>Send a command to the source KVM host to migrate the VM and its storage.</li>
1695+
* </ul>
16941696
*/
16951697
@Override
16961698
public void copyAsync(Map<VolumeInfo, DataStore> volumeDataStoreMap, VirtualMachineTO vmTO, Host srcHost, Host destHost, AsyncCompletionCallback<CopyCommandResult> callback) {
@@ -1718,6 +1720,10 @@ public void copyAsync(Map<VolumeInfo, DataStore> volumeDataStoreMap, VirtualMach
17181720
StoragePoolVO destStoragePool = _storagePoolDao.findById(destDataStore.getId());
17191721
StoragePoolVO sourceStoragePool = _storagePoolDao.findById(srcVolumeInfo.getPoolId());
17201722

1723+
if (!shouldMigrateVolume(sourceStoragePool, destHost, destStoragePool)) {
1724+
continue;
1725+
}
1726+
17211727
VolumeVO destVolume = duplicateVolumeOnAnotherStorage(srcVolume, destStoragePool);
17221728
VolumeInfo destVolumeInfo = _volumeDataFactory.getVolume(destVolume.getId(), destDataStore);
17231729

@@ -1817,6 +1823,13 @@ public void copyAsync(Map<VolumeInfo, DataStore> volumeDataStoreMap, VirtualMach
18171823
}
18181824
}
18191825

1826+
/**
1827+
* 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.
1828+
*/
1829+
protected boolean shouldMigrateVolume(StoragePoolVO sourceStoragePool, Host destHost, StoragePoolVO destStoragePool) {
1830+
return true;
1831+
}
1832+
18201833
/**
18211834
* Returns true if the storage pool type is {@link StoragePoolType.Filesystem}.
18221835
*/
Lines changed: 256 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.cloudstack.storage.motion;
20+
21+
import java.util.HashMap;
22+
import java.util.Map;
23+
24+
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
25+
import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
26+
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateDataFactory;
27+
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo;
28+
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
29+
import org.apache.cloudstack.storage.datastore.PrimaryDataStoreImpl;
30+
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
31+
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
32+
import org.apache.cloudstack.storage.volume.VolumeObject;
33+
import org.junit.Assert;
34+
import org.junit.Test;
35+
import org.junit.runner.RunWith;
36+
import org.mockito.InjectMocks;
37+
import org.mockito.Mock;
38+
import org.mockito.Mockito;
39+
import org.mockito.Spy;
40+
import org.mockito.runners.MockitoJUnitRunner;
41+
42+
import com.cloud.agent.AgentManager;
43+
import com.cloud.agent.api.MigrateCommand;
44+
import com.cloud.agent.api.storage.CreateAnswer;
45+
import com.cloud.agent.api.storage.CreateCommand;
46+
import com.cloud.agent.api.to.VirtualMachineTO;
47+
import com.cloud.agent.api.to.VolumeTO;
48+
import com.cloud.host.HostVO;
49+
import com.cloud.hypervisor.Hypervisor.HypervisorType;
50+
import com.cloud.storage.DataStoreRole;
51+
import com.cloud.storage.DiskOfferingVO;
52+
import com.cloud.storage.Storage;
53+
import com.cloud.storage.Storage.StoragePoolType;
54+
import com.cloud.storage.Volume;
55+
import com.cloud.storage.VolumeVO;
56+
import com.cloud.storage.dao.DiskOfferingDao;
57+
import com.cloud.utils.exception.CloudRuntimeException;
58+
import com.cloud.vm.DiskProfile;
59+
60+
@RunWith(MockitoJUnitRunner.class)
61+
public class KvmNonManagedStorageSystemDataMotionTest {
62+
63+
@Mock
64+
private PrimaryDataStoreDao primaryDataStoreDao;
65+
66+
@Mock
67+
private TemplateDataFactory templateFactory;
68+
69+
@Mock
70+
private AgentManager agentMgr;
71+
72+
@Mock
73+
private DiskOfferingDao diskOfferingDao;
74+
75+
@Spy
76+
@InjectMocks
77+
private KvmNonManagedStorageDataMotionStrategy kvmStorageDataMotionStrategy;
78+
79+
@Test
80+
public void canHandleTestExpectHypervisorStrategyForKvm() {
81+
canHandleExpectCannotHandle(HypervisorType.KVM, 1, StrategyPriority.HYPERVISOR);
82+
}
83+
84+
@Test
85+
public void canHandleTestExpectCannotHandle() {
86+
HypervisorType[] hypervisorTypeArray = HypervisorType.values();
87+
for (int i = 0; i < hypervisorTypeArray.length; i++) {
88+
HypervisorType ht = hypervisorTypeArray[i];
89+
if (ht.equals(HypervisorType.KVM)) {
90+
continue;
91+
}
92+
canHandleExpectCannotHandle(ht, 0, StrategyPriority.CANT_HANDLE);
93+
}
94+
}
95+
96+
private void canHandleExpectCannotHandle(HypervisorType hypervisorType, int times, StrategyPriority expectedStrategyPriority) {
97+
HostVO srcHost = new HostVO("sourceHostUuid");
98+
srcHost.setHypervisorType(hypervisorType);
99+
Mockito.doReturn(StrategyPriority.HYPERVISOR).when(kvmStorageDataMotionStrategy).internalCanHandle(new HashMap<>());
100+
101+
StrategyPriority strategyPriority = kvmStorageDataMotionStrategy.canHandle(new HashMap<>(), srcHost, new HostVO("destHostUuid"));
102+
103+
Mockito.verify(kvmStorageDataMotionStrategy, Mockito.times(times)).internalCanHandle(new HashMap<>());
104+
Assert.assertEquals(expectedStrategyPriority, strategyPriority);
105+
}
106+
107+
@Test
108+
public void internalCanHandleTestNonManaged() {
109+
StoragePoolType[] storagePoolTypeArray = StoragePoolType.values();
110+
for (int i = 0; i < storagePoolTypeArray.length; i++) {
111+
Map<VolumeInfo, DataStore> volumeMap = configureTestInternalCanHandle(false, storagePoolTypeArray[i]);
112+
StrategyPriority strategyPriority = kvmStorageDataMotionStrategy.internalCanHandle(volumeMap);
113+
if (storagePoolTypeArray[i] == StoragePoolType.Filesystem || storagePoolTypeArray[i] == StoragePoolType.NetworkFilesystem) {
114+
Assert.assertEquals(StrategyPriority.HYPERVISOR, strategyPriority);
115+
} else {
116+
Assert.assertEquals(StrategyPriority.CANT_HANDLE, strategyPriority);
117+
}
118+
}
119+
}
120+
121+
@Test
122+
public void internalCanHandleTestIsManaged() {
123+
StoragePoolType[] storagePoolTypeArray = StoragePoolType.values();
124+
for (int i = 0; i < storagePoolTypeArray.length; i++) {
125+
Map<VolumeInfo, DataStore> volumeMap = configureTestInternalCanHandle(true, storagePoolTypeArray[i]);
126+
StrategyPriority strategyPriority = kvmStorageDataMotionStrategy.internalCanHandle(volumeMap);
127+
Assert.assertEquals(StrategyPriority.CANT_HANDLE, strategyPriority);
128+
}
129+
}
130+
131+
private Map<VolumeInfo, DataStore> configureTestInternalCanHandle(boolean isManagedStorage, StoragePoolType storagePoolType) {
132+
VolumeObject volumeInfo = Mockito.spy(new VolumeObject());
133+
Mockito.doReturn(0l).when(volumeInfo).getPoolId();
134+
DataStore ds = Mockito.spy(new PrimaryDataStoreImpl());
135+
Mockito.doReturn(0l).when(ds).getId();
136+
137+
Map<VolumeInfo, DataStore> volumeMap = new HashMap<>();
138+
volumeMap.put(volumeInfo, ds);
139+
140+
StoragePoolVO storagePool = Mockito.spy(new StoragePoolVO());
141+
Mockito.doReturn(storagePoolType).when(storagePool).getPoolType();
142+
143+
Mockito.doReturn(storagePool).when(primaryDataStoreDao).findById(0l);
144+
Mockito.doReturn(isManagedStorage).when(storagePool).isManaged();
145+
return volumeMap;
146+
}
147+
148+
@Test
149+
public void getTemplateUuidTestTemplateIdNotNull() {
150+
String expectedTemplateUuid = prepareTestGetTemplateUuid();
151+
String templateUuid = kvmStorageDataMotionStrategy.getTemplateUuid(0l);
152+
Assert.assertEquals(expectedTemplateUuid, templateUuid);
153+
}
154+
155+
@Test
156+
public void getTemplateUuidTestTemplateIdNull() {
157+
prepareTestGetTemplateUuid();
158+
String templateUuid = kvmStorageDataMotionStrategy.getTemplateUuid(null);
159+
Assert.assertEquals(null, templateUuid);
160+
}
161+
162+
private String prepareTestGetTemplateUuid() {
163+
TemplateInfo templateImage = Mockito.mock(TemplateInfo.class);
164+
String expectedTemplateUuid = "template uuid";
165+
Mockito.when(templateImage.getUuid()).thenReturn(expectedTemplateUuid);
166+
Mockito.doReturn(templateImage).when(templateFactory).getTemplate(0l, DataStoreRole.Image);
167+
return expectedTemplateUuid;
168+
}
169+
170+
@Test
171+
public void configureMigrateDiskInfoTest() {
172+
VolumeObject srcVolumeInfo = Mockito.spy(new VolumeObject());
173+
Mockito.doReturn("volume path").when(srcVolumeInfo).getPath();
174+
MigrateCommand.MigrateDiskInfo migrateDiskInfo = kvmStorageDataMotionStrategy.configureMigrateDiskInfo(srcVolumeInfo, "destPath");
175+
Assert.assertEquals(MigrateCommand.MigrateDiskInfo.DiskType.FILE, migrateDiskInfo.getDiskType());
176+
Assert.assertEquals(MigrateCommand.MigrateDiskInfo.DriverType.QCOW2, migrateDiskInfo.getDriverType());
177+
Assert.assertEquals(MigrateCommand.MigrateDiskInfo.Source.FILE, migrateDiskInfo.getSource());
178+
Assert.assertEquals("destPath", migrateDiskInfo.getSourceText());
179+
Assert.assertEquals("volume path", migrateDiskInfo.getSerialNumber());
180+
}
181+
182+
@Test
183+
public void generateDestPathTest() {
184+
configureAndVerifygenerateDestPathTest(true, false);
185+
}
186+
187+
@Test(expected = CloudRuntimeException.class)
188+
public void generateDestPathTestExpectCloudRuntimeException() {
189+
configureAndVerifygenerateDestPathTest(false, false);
190+
}
191+
192+
@Test(expected = CloudRuntimeException.class)
193+
public void generateDestPathTestExpectCloudRuntimeException2() {
194+
configureAndVerifygenerateDestPathTest(false, true);
195+
}
196+
197+
private void configureAndVerifygenerateDestPathTest(boolean answerResult, boolean answerIsNull) {
198+
String uuid = "f3d49ecc-870c-475a-89fa-fd0124420a9b";
199+
String destPath = "/var/lib/libvirt/images/";
200+
201+
VirtualMachineTO vmTO = Mockito.mock(VirtualMachineTO.class);
202+
Mockito.when(vmTO.getName()).thenReturn("vmName");
203+
204+
VolumeVO srcVolume = Mockito.spy(new VolumeVO("name", 0l, 0l, 0l, 0l, 0l, "folder", "path", Storage.ProvisioningType.THIN, 0l, Volume.Type.ROOT));
205+
StoragePoolVO destStoragePool = Mockito.spy(new StoragePoolVO());
206+
207+
VolumeInfo destVolumeInfo = Mockito.spy(new VolumeObject());
208+
Mockito.doReturn(0l).when(destVolumeInfo).getTemplateId();
209+
Mockito.doReturn(0l).when(destVolumeInfo).getId();
210+
Mockito.doReturn(Volume.Type.ROOT).when(destVolumeInfo).getVolumeType();
211+
Mockito.doReturn("name").when(destVolumeInfo).getName();
212+
Mockito.doReturn(0l).when(destVolumeInfo).getSize();
213+
Mockito.doReturn(uuid).when(destVolumeInfo).getUuid();
214+
215+
DiskOfferingVO diskOffering = Mockito.spy(new DiskOfferingVO());
216+
Mockito.doReturn(0l).when(diskOffering).getId();
217+
Mockito.doReturn(diskOffering).when(diskOfferingDao).findById(0l);
218+
DiskProfile diskProfile = Mockito.spy(new DiskProfile(destVolumeInfo, diskOffering, HypervisorType.KVM));
219+
220+
String templateUuid = Mockito.doReturn("templateUuid").when(kvmStorageDataMotionStrategy).getTemplateUuid(0l);
221+
CreateCommand rootImageProvisioningCommand = new CreateCommand(diskProfile, templateUuid, destStoragePool, true);
222+
CreateAnswer createAnswer = Mockito.spy(new CreateAnswer(rootImageProvisioningCommand, "details"));
223+
Mockito.doReturn(answerResult).when(createAnswer).getResult();
224+
225+
VolumeTO volumeTo = Mockito.mock(VolumeTO.class);
226+
Mockito.doReturn(destPath).when(volumeTo).getName();
227+
Mockito.doReturn(volumeTo).when(createAnswer).getVolume();
228+
229+
if (answerIsNull) {
230+
Mockito.doReturn(null).when(agentMgr).easySend(0l, rootImageProvisioningCommand);
231+
} else {
232+
Mockito.doReturn(createAnswer).when(agentMgr).easySend(0l, rootImageProvisioningCommand);
233+
}
234+
235+
String generatedDestPath = kvmStorageDataMotionStrategy.generateDestPath(vmTO, srcVolume, new HostVO("sourceHostUuid"), destStoragePool, destVolumeInfo);
236+
237+
Assert.assertEquals(destPath + uuid, generatedDestPath);
238+
}
239+
240+
@Test
241+
public void shouldMigrateVolumeTest() {
242+
StoragePoolVO sourceStoragePool = Mockito.spy(new StoragePoolVO());
243+
HostVO destHost = new HostVO("guid");
244+
StoragePoolVO destStoragePool = new StoragePoolVO();
245+
StoragePoolType[] storagePoolTypes = StoragePoolType.values();
246+
for (int i = 0; i < storagePoolTypes.length; i++) {
247+
Mockito.doReturn(storagePoolTypes[i]).when(sourceStoragePool).getPoolType();
248+
boolean result = kvmStorageDataMotionStrategy.shouldMigrateVolume(sourceStoragePool, destHost, destStoragePool);
249+
if (storagePoolTypes[i] == StoragePoolType.Filesystem) {
250+
Assert.assertTrue(result);
251+
} else {
252+
Assert.assertFalse(result);
253+
}
254+
}
255+
}
256+
}

0 commit comments

Comments
 (0)