Skip to content

Commit 7373a5c

Browse files
João JandreJoaoJandre
João Jandre
authored and
JoaoJandre
committed
Fix snapshot operation selector
1 parent 878350e commit 7373a5c

File tree

4 files changed

+277
-16
lines changed

4 files changed

+277
-16
lines changed

‎engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/EndPointSelector.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
*/
1919
package org.apache.cloudstack.engine.subsystem.api.storage;
2020

21+
import com.cloud.hypervisor.Hypervisor;
22+
2123
import java.util.List;
2224

2325
public interface EndPointSelector {
@@ -39,6 +41,8 @@ public interface EndPointSelector {
3941

4042
EndPoint select(DataObject object, StorageAction action, boolean encryptionSupportRequired);
4143

44+
EndPoint selectRandom(long zoneId, Hypervisor.HypervisorType hypervisorType);
45+
4246
List<EndPoint> selectAll(DataStore store);
4347

4448
List<EndPoint> findAllEndpointsForScope(DataStore store);

‎engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,7 @@ protected DataStore getImageStoreForSnapshot(Long dataCenterId, SnapshotInfo sna
397397
public SnapshotInfo convertSnapshot(SnapshotInfo snapshotInfo) {
398398
SnapshotObject snapObj = (SnapshotObject)snapshotInfo;
399399

400+
logger.debug("Converting snapshot [%s].", snapObj);
400401
Answer answer = null;
401402
try {
402403
snapObj.processEvent(Snapshot.Event.BackupToSecondary);
@@ -409,13 +410,21 @@ public SnapshotInfo convertSnapshot(SnapshotInfo snapshotInfo) {
409410
answer = ep.sendMessage(cmd);
410411

411412
if (answer != null && answer.getResult()) {
412-
snapObj.processEvent(Snapshot.Event.OperationSucceeded);
413413
snapObj.setPath(((ConvertSnapshotAnswer) answer).getSnapshotObjectTO().getPath());
414414
return snapObj;
415415
}
416-
snapObj.processEvent(Snapshot.Event.OperationNotPerformed);
417416
} catch (NoTransitionException e) {
418417
logger.debug("Failed to change snapshot {} state.", snapObj.getUuid(), e);
418+
} finally {
419+
try {
420+
if (answer != null && answer.getResult()) {
421+
snapObj.processEvent(Snapshot.Event.OperationSucceeded);
422+
} else {
423+
snapObj.processEvent(Snapshot.Event.OperationNotPerformed);
424+
}
425+
} catch (NoTransitionException ex) {
426+
logger.debug("Failed to change snapshot {} state.", snapObj.getUuid(), ex);
427+
}
419428
}
420429

421430
throw new CloudRuntimeException(String.format("Failed to convert snapshot [%s]%s.", snapObj.getUuid(), answer != null ? String.format(" due to [%s]", answer.getDetails()) : ""));

‎engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java‎

Lines changed: 62 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -464,23 +464,16 @@ public EndPoint select(DataObject object, StorageAction action, boolean encrypti
464464
switch (action) {
465465
case DELETESNAPSHOT:
466466
case TAKESNAPSHOT:
467-
case CONVERTSNAPSHOT:
468-
case REMOVEBITMAP:
469-
SnapshotInfo snapshotInfo = (SnapshotInfo) object;
467+
case CONVERTSNAPSHOT: {
468+
SnapshotInfo snapshotInfo = (SnapshotInfo)object;
470469
if (Hypervisor.HypervisorType.KVM.equals(snapshotInfo.getHypervisorType())) {
471-
VolumeInfo volumeInfo = snapshotInfo.getBaseVolume();
472-
VirtualMachine vm = volumeInfo.getAttachedVM();
473-
if (vm == null) {
474-
break;
475-
}
476-
if (vm.getState() == VirtualMachine.State.Running) {
477-
Long hostId = vm.getHostId();
478-
return getEndPointFromHostId(hostId);
479-
}
480-
Long hostId = vm.getLastHostId();
481-
return hostId != null ? getEndPointFromHostId(hostId) : select(object, encryptionRequired);
470+
return getEndPointForSnapshotOperationsInKvm(snapshotInfo, encryptionRequired);
482471
}
483472
break;
473+
}
474+
case REMOVEBITMAP: {
475+
return getEndPointForBitmapRemoval(object, encryptionRequired);
476+
}
484477
case MIGRATEVOLUME: {
485478
VolumeInfo volume = (VolumeInfo) object;
486479
if (volume.getHypervisorType() == Hypervisor.HypervisorType.Hyperv || volume.getHypervisorType() == Hypervisor.HypervisorType.VMware) {
@@ -509,11 +502,66 @@ public EndPoint select(DataObject object, StorageAction action, boolean encrypti
509502
return select(object, encryptionRequired);
510503
}
511504

505+
protected EndPoint getEndPointForBitmapRemoval(DataObject object, boolean encryptionRequired) {
506+
SnapshotInfo snapshotInfo = (SnapshotInfo)object;
507+
VolumeInfo volumeInfo = snapshotInfo.getBaseVolume();
508+
509+
logger.debug("Selecting endpoint for bitmap removal of volume [{}].", volumeInfo.getUuid());
510+
if (volumeInfo.isAttachedVM()) {
511+
VirtualMachine attachedVM = volumeInfo.getAttachedVM();
512+
if (attachedVM.getHostId() != null) {
513+
return getEndPointFromHostId(attachedVM.getHostId());
514+
} else if (attachedVM.getLastHostId() != null) {
515+
return getEndPointFromHostId(attachedVM.getLastHostId());
516+
}
517+
}
518+
return select(object, encryptionRequired);
519+
}
520+
521+
protected EndPoint getEndPointForSnapshotOperationsInKvm(SnapshotInfo snapshotInfo, boolean encryptionRequired) {
522+
VolumeInfo volumeInfo = snapshotInfo.getBaseVolume();
523+
DataStoreRole snapshotDataStoreRole = snapshotInfo.getDataStore().getRole();
524+
VirtualMachine vm = volumeInfo.getAttachedVM();
525+
526+
logger.debug("Selecting endpoint for operation on snapshot [{}] with encryptionRequired as [{}].", snapshotInfo, encryptionRequired);
527+
if (vm == null) {
528+
if (snapshotDataStoreRole == DataStoreRole.Image) {
529+
return selectRandom(snapshotInfo.getDataCenterId(), Hypervisor.HypervisorType.KVM);
530+
} else {
531+
return select(snapshotInfo, encryptionRequired);
532+
}
533+
}
534+
535+
if (vm.getState() == VirtualMachine.State.Running) {
536+
return getEndPointFromHostId(vm.getHostId());
537+
}
538+
539+
Long hostId = vm.getLastHostId();
540+
if (hostId != null) {
541+
return getEndPointFromHostId(hostId);
542+
} else if (snapshotDataStoreRole == DataStoreRole.Image) {
543+
return selectRandom(snapshotInfo.getDataCenterId(), Hypervisor.HypervisorType.KVM);
544+
}
545+
546+
return select(snapshotInfo, encryptionRequired);
547+
}
548+
512549
@Override
513550
public EndPoint select(Scope scope, Long storeId) {
514551
return findEndPointInScope(scope, findOneHostOnPrimaryStorage, storeId);
515552
}
516553

554+
@Override
555+
public EndPoint selectRandom(long zoneId, Hypervisor.HypervisorType hypervisorType) {
556+
List<HostVO> hostVOs = hostDao.listByDataCenterIdAndHypervisorType(zoneId, hypervisorType);
557+
558+
if (hostVOs.isEmpty()) {
559+
return null;
560+
}
561+
Collections.shuffle(hostVOs);
562+
return RemoteHostEndPoint.getHypervisorHostEndPoint(hostVOs.get(0));
563+
}
564+
517565
@Override
518566
public List<EndPoint> selectAll(DataStore store) {
519567
List<EndPoint> endPoints = new ArrayList<EndPoint>();

‎engine/storage/src/test/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelectorTest.java‎

Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,200 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
package org.apache.cloudstack.storage.endpoint;
18+
19+
20+
import com.cloud.hypervisor.Hypervisor;
21+
import com.cloud.storage.DataStoreRole;
22+
import com.cloud.vm.VirtualMachine;
23+
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
24+
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
25+
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
26+
import org.junit.Before;
27+
import org.junit.Test;
28+
import org.junit.runner.RunWith;
29+
import org.mockito.Mock;
30+
import org.mockito.Mockito;
31+
import org.mockito.Spy;
32+
import org.mockito.junit.MockitoJUnitRunner;
33+
34+
@RunWith(MockitoJUnitRunner.class)
35+
public class DefaultEndPointSelectorTest {
36+
37+
@Mock
38+
private VirtualMachine virtualMachineMock;
39+
40+
@Mock
41+
private VolumeInfo volumeInfoMock;
42+
43+
@Mock
44+
private SnapshotInfo snapshotInfoMock;
45+
46+
@Mock
47+
private DataStore datastoreMock;
48+
49+
@Spy
50+
private DefaultEndPointSelector defaultEndPointSelectorSpy;
51+
52+
@Before
53+
public void setup() {
54+
Mockito.doReturn(volumeInfoMock).when(snapshotInfoMock).getBaseVolume();
55+
}
56+
57+
@Test
58+
public void getEndPointForBitmapRemovalTestVolumeIsNotAttached() {
59+
Mockito.doReturn(false).when(volumeInfoMock).isAttachedVM();
60+
Mockito.doReturn(null).when(defaultEndPointSelectorSpy).select(snapshotInfoMock, false);
61+
62+
defaultEndPointSelectorSpy.getEndPointForBitmapRemoval(snapshotInfoMock, false);
63+
64+
Mockito.verify(defaultEndPointSelectorSpy, Mockito.times(1)).select(snapshotInfoMock, false);
65+
}
66+
67+
@Test
68+
public void getEndPointForBitmapRemovalTestVolumeIsAttachedHostIdIsSet() {
69+
Mockito.doReturn(true).when(volumeInfoMock).isAttachedVM();
70+
Mockito.doReturn(virtualMachineMock).when(volumeInfoMock).getAttachedVM();
71+
long hostId = 12L;
72+
Mockito.doReturn(hostId).when(virtualMachineMock).getHostId();
73+
74+
Mockito.doReturn(null).when(defaultEndPointSelectorSpy).getEndPointFromHostId(hostId);
75+
76+
defaultEndPointSelectorSpy.getEndPointForBitmapRemoval(snapshotInfoMock, false);
77+
78+
Mockito.verify(defaultEndPointSelectorSpy, Mockito.times(1)).getEndPointFromHostId(hostId);
79+
}
80+
81+
@Test
82+
public void getEndPointForBitmapRemovalTestVolumeIsAttachedLastHostIdIsSet() {
83+
Mockito.doReturn(true).when(volumeInfoMock).isAttachedVM();
84+
Mockito.doReturn(virtualMachineMock).when(volumeInfoMock).getAttachedVM();
85+
86+
Mockito.doReturn(null).when(virtualMachineMock).getHostId();
87+
long lastHostId = 13L;
88+
Mockito.doReturn(lastHostId).when(virtualMachineMock).getLastHostId();
89+
90+
Mockito.doReturn(null).when(defaultEndPointSelectorSpy).getEndPointFromHostId(lastHostId);
91+
92+
defaultEndPointSelectorSpy.getEndPointForBitmapRemoval(snapshotInfoMock, false);
93+
94+
Mockito.verify(defaultEndPointSelectorSpy, Mockito.times(1)).getEndPointFromHostId(lastHostId);
95+
}
96+
97+
@Test
98+
public void getEndPointForBitmapRemovalTestVolumeIsAttachedNoHostIsSet() {
99+
Mockito.doReturn(true).when(volumeInfoMock).isAttachedVM();
100+
Mockito.doReturn(virtualMachineMock).when(volumeInfoMock).getAttachedVM();
101+
102+
Mockito.doReturn(null).when(virtualMachineMock).getHostId();
103+
Mockito.doReturn(null).when(virtualMachineMock).getLastHostId();
104+
105+
Mockito.doReturn(null).when(defaultEndPointSelectorSpy).select(snapshotInfoMock, false);
106+
107+
defaultEndPointSelectorSpy.getEndPointForBitmapRemoval(snapshotInfoMock, false);
108+
109+
Mockito.verify(defaultEndPointSelectorSpy, Mockito.times(1)).select(snapshotInfoMock, false);
110+
}
111+
112+
@Test
113+
public void getEndPointForSnapshotOperationsInKvmTestVolumeIsNotAttachedToVMAndSnapshotOnPrimary() {
114+
Mockito.doReturn(null).when(volumeInfoMock).getAttachedVM();
115+
Mockito.doReturn(datastoreMock).when(snapshotInfoMock).getDataStore();
116+
Mockito.doReturn(DataStoreRole.Primary).when(datastoreMock).getRole();
117+
Mockito.doReturn(null).when(defaultEndPointSelectorSpy).select(snapshotInfoMock, false);
118+
119+
defaultEndPointSelectorSpy.getEndPointForSnapshotOperationsInKvm(snapshotInfoMock, false);
120+
121+
Mockito.verify(defaultEndPointSelectorSpy, Mockito.times(1)).select(snapshotInfoMock, false);
122+
}
123+
124+
@Test
125+
public void getEndPointForSnapshotOperationsInKvmTestVolumeIsNotAttachedToVMAndSnapshotOnSecondary() {
126+
Mockito.doReturn(null).when(volumeInfoMock).getAttachedVM();
127+
Mockito.doReturn(datastoreMock).when(snapshotInfoMock).getDataStore();
128+
Mockito.doReturn(DataStoreRole.Image).when(datastoreMock).getRole();
129+
long zoneId = 1L;
130+
Mockito.doReturn(zoneId).when(snapshotInfoMock).getDataCenterId();
131+
132+
Mockito.doReturn(null).when(defaultEndPointSelectorSpy).selectRandom(zoneId, Hypervisor.HypervisorType.KVM);
133+
134+
defaultEndPointSelectorSpy.getEndPointForSnapshotOperationsInKvm(snapshotInfoMock, false);
135+
136+
Mockito.verify(defaultEndPointSelectorSpy, Mockito.times(1)).selectRandom(zoneId, Hypervisor.HypervisorType.KVM);
137+
}
138+
139+
@Test
140+
public void getEndPointForSnapshotOperationsInKvmTestVolumeAttachedToRunningVm() {
141+
Mockito.doReturn(virtualMachineMock).when(volumeInfoMock).getAttachedVM();
142+
Mockito.doReturn(datastoreMock).when(snapshotInfoMock).getDataStore();
143+
Mockito.doReturn(VirtualMachine.State.Running).when(virtualMachineMock).getState();
144+
long hostId = 12L;
145+
Mockito.doReturn(hostId).when(virtualMachineMock).getHostId();
146+
147+
Mockito.doReturn(null).when(defaultEndPointSelectorSpy).getEndPointFromHostId(hostId);
148+
149+
defaultEndPointSelectorSpy.getEndPointForSnapshotOperationsInKvm(snapshotInfoMock, false);
150+
151+
Mockito.verify(defaultEndPointSelectorSpy, Mockito.times(1)).getEndPointFromHostId(hostId);
152+
}
153+
154+
@Test
155+
public void getEndPointForSnapshotOperationsInKvmTestVolumeAttachedToStoppedVmAndLastHostIdIsSet() {
156+
Mockito.doReturn(virtualMachineMock).when(volumeInfoMock).getAttachedVM();
157+
Mockito.doReturn(datastoreMock).when(snapshotInfoMock).getDataStore();
158+
Mockito.doReturn(VirtualMachine.State.Stopped).when(virtualMachineMock).getState();
159+
long hostId = 13L;
160+
Mockito.doReturn(hostId).when(virtualMachineMock).getLastHostId();
161+
162+
Mockito.doReturn(null).when(defaultEndPointSelectorSpy).getEndPointFromHostId(hostId);
163+
164+
defaultEndPointSelectorSpy.getEndPointForSnapshotOperationsInKvm(snapshotInfoMock, false);
165+
166+
Mockito.verify(defaultEndPointSelectorSpy, Mockito.times(1)).getEndPointFromHostId(hostId);
167+
}
168+
169+
@Test
170+
public void getEndPointForSnapshotOperationsInKvmTestVolumeAttachedToStoppedVmAndLastHostIdIsNotSetAndSnapshotIsOnSecondary() {
171+
Mockito.doReturn(virtualMachineMock).when(volumeInfoMock).getAttachedVM();
172+
Mockito.doReturn(datastoreMock).when(snapshotInfoMock).getDataStore();
173+
Mockito.doReturn(DataStoreRole.Image).when(datastoreMock).getRole();
174+
Mockito.doReturn(VirtualMachine.State.Stopped).when(virtualMachineMock).getState();
175+
Mockito.doReturn(null).when(virtualMachineMock).getLastHostId();
176+
long zoneId = 1L;
177+
Mockito.doReturn(zoneId).when(snapshotInfoMock).getDataCenterId();
178+
Mockito.doReturn(null).when(defaultEndPointSelectorSpy).selectRandom(zoneId, Hypervisor.HypervisorType.KVM);
179+
180+
defaultEndPointSelectorSpy.getEndPointForSnapshotOperationsInKvm(snapshotInfoMock, false);
181+
182+
Mockito.verify(defaultEndPointSelectorSpy, Mockito.times(1)).selectRandom(zoneId, Hypervisor.HypervisorType.KVM);
183+
}
184+
185+
186+
@Test
187+
public void getEndPointForSnapshotOperationsInKvmTestVolumeAttachedToStoppedVmAndLastHostIdIsNotSetAndSnapshotIsOnPrimary() {
188+
Mockito.doReturn(virtualMachineMock).when(volumeInfoMock).getAttachedVM();
189+
Mockito.doReturn(datastoreMock).when(snapshotInfoMock).getDataStore();
190+
Mockito.doReturn(DataStoreRole.Primary).when(datastoreMock).getRole();
191+
Mockito.doReturn(VirtualMachine.State.Stopped).when(virtualMachineMock).getState();
192+
Mockito.doReturn(null).when(virtualMachineMock).getLastHostId();
193+
Mockito.doReturn(null).when(defaultEndPointSelectorSpy).select(snapshotInfoMock, false);
194+
195+
defaultEndPointSelectorSpy.getEndPointForSnapshotOperationsInKvm(snapshotInfoMock, false);
196+
197+
Mockito.verify(defaultEndPointSelectorSpy, Mockito.times(1)).select(snapshotInfoMock, false);
198+
}
199+
200+
}

0 commit comments

Comments
 (0)