Skip to content

Commit 661956c

Browse files
yadvryadvr
committed
Merge remote-tracking branch 'origin/4.17'
2 parents 71bc088 + 2c05b63 commit 661956c

File tree

5 files changed

+134
-11
lines changed

5 files changed

+134
-11
lines changed

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotResult;
3737
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotService;
3838
import org.apache.cloudstack.engine.subsystem.api.storage.StorageCacheManager;
39+
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
3940
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
4041
import org.apache.cloudstack.framework.async.AsyncCallFuture;
4142
import org.apache.cloudstack.framework.async.AsyncCallbackDispatcher;
@@ -79,6 +80,8 @@ public class SnapshotServiceImpl implements SnapshotService {
7980
StorageCacheManager _cacheMgr;
8081
@Inject
8182
private SnapshotDetailsDao _snapshotDetailsDao;
83+
@Inject
84+
VolumeDataFactory volFactory;
8285

8386
static private class CreateSnapshotContext<T> extends AsyncRpcContext<T> {
8487
final SnapshotInfo snapshot;
@@ -428,11 +431,15 @@ public boolean deleteSnapshot(SnapshotInfo snapInfo) {
428431

429432
@Override
430433
public boolean revertSnapshot(SnapshotInfo snapshot) {
434+
PrimaryDataStore store = null;
431435
SnapshotInfo snapshotOnPrimaryStore = _snapshotFactory.getSnapshot(snapshot.getId(), DataStoreRole.Primary);
432436
if (snapshotOnPrimaryStore == null) {
433-
throw new CloudRuntimeException("Cannot find an entry for snapshot " + snapshot.getId() + " on primary storage pools");
437+
s_logger.warn("Cannot find an entry for snapshot " + snapshot.getId() + " on primary storage pools, searching with volume's primary storage pool");
438+
VolumeInfo volumeInfo = volFactory.getVolume(snapshot.getVolumeId(), DataStoreRole.Primary);
439+
store = (PrimaryDataStore)volumeInfo.getDataStore();
440+
} else {
441+
store = (PrimaryDataStore)snapshotOnPrimaryStore.getDataStore();
434442
}
435-
PrimaryDataStore store = (PrimaryDataStore)snapshotOnPrimaryStore.getDataStore();
436443

437444
AsyncCallFuture<SnapshotResult> future = new AsyncCallFuture<SnapshotResult>();
438445
RevertSnapshotContext<CommandResult> context = new RevertSnapshotContext<CommandResult>(null, snapshot, future);

‎engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/snapshot/SnapshotServiceImplTest.java‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
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.snapshot;
20+
21+
import com.cloud.storage.DataStoreRole;
22+
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore;
23+
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreDriver;
24+
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory;
25+
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
26+
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotResult;
27+
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
28+
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
29+
import org.apache.cloudstack.framework.async.AsyncCallFuture;
30+
import org.apache.cloudstack.framework.async.AsyncCallbackDispatcher;
31+
import org.apache.cloudstack.storage.command.CommandResult;
32+
import org.junit.Assert;
33+
import org.junit.Before;
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.MockitoAnnotations;
40+
import org.mockito.Spy;
41+
import org.powermock.api.mockito.PowerMockito;
42+
import org.powermock.core.classloader.annotations.PrepareForTest;
43+
import org.powermock.modules.junit4.PowerMockRunner;
44+
import org.springframework.test.context.ContextConfiguration;
45+
import org.springframework.test.context.support.AnnotationConfigContextLoader;
46+
47+
@RunWith(PowerMockRunner.class)
48+
@PrepareForTest({SnapshotServiceImpl.class})
49+
@ContextConfiguration(loader = AnnotationConfigContextLoader.class)
50+
public class SnapshotServiceImplTest {
51+
52+
@Spy
53+
@InjectMocks
54+
private SnapshotServiceImpl snapshotService = new SnapshotServiceImpl();
55+
56+
@Mock
57+
VolumeDataFactory volFactory;
58+
59+
@Mock
60+
SnapshotDataFactory _snapshotFactory;
61+
62+
@Mock
63+
AsyncCallFuture<SnapshotResult> futureMock;
64+
65+
@Mock
66+
AsyncCallbackDispatcher<SnapshotServiceImpl, CommandResult> caller;
67+
68+
@Before
69+
public void testSetUp() throws Exception {
70+
MockitoAnnotations.initMocks(this);
71+
}
72+
73+
@Test
74+
public void testRevertSnapshotWithNoPrimaryStorageEntry() throws Exception {
75+
SnapshotInfo snapshot = Mockito.mock(SnapshotInfo.class);
76+
VolumeInfo volumeInfo = Mockito.mock(VolumeInfo.class);
77+
78+
Mockito.when(snapshot.getId()).thenReturn(1L);
79+
Mockito.when(snapshot.getVolumeId()).thenReturn(1L);
80+
Mockito.when(_snapshotFactory.getSnapshot(1L, DataStoreRole.Primary)).thenReturn(null);
81+
Mockito.when(volFactory.getVolume(1L, DataStoreRole.Primary)).thenReturn(volumeInfo);
82+
83+
PrimaryDataStore store = Mockito.mock(PrimaryDataStore.class);
84+
Mockito.when(volumeInfo.getDataStore()).thenReturn(store);
85+
86+
PrimaryDataStoreDriver driver = Mockito.mock(PrimaryDataStoreDriver.class);
87+
Mockito.when(store.getDriver()).thenReturn(driver);
88+
Mockito.doNothing().when(driver).revertSnapshot(snapshot, null, caller);
89+
90+
SnapshotResult result = Mockito.mock(SnapshotResult.class);
91+
PowerMockito.whenNew(AsyncCallFuture.class).withNoArguments().thenReturn(futureMock);
92+
Mockito.when(futureMock.get()).thenReturn(result);
93+
Mockito.when(result.isFailed()).thenReturn(false);
94+
95+
Assert.assertEquals(true, snapshotService.revertSnapshot(snapshot));
96+
}
97+
98+
}

‎plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertSnapshotCommandWrapper.java‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -178,20 +178,24 @@ protected void revertVolumeToSnapshot(SnapshotObjectTO snapshotOnPrimaryStorage,
178178
* and the snapshot path from the primary storage.
179179
*/
180180
protected Pair<String, SnapshotObjectTO> getSnapshot(SnapshotObjectTO snapshotOnPrimaryStorage, SnapshotObjectTO snapshotOnSecondaryStorage,
181-
KVMStoragePool kvmStoragePoolPrimary, KVMStoragePool kvmStoragePoolSecondary){
182-
String snapshotPath = snapshotOnPrimaryStorage.getPath();
183-
184-
if (Files.exists(Paths.get(snapshotPath))) {
185-
return new Pair<>(snapshotPath, snapshotOnPrimaryStorage);
181+
KVMStoragePool kvmStoragePoolPrimary, KVMStoragePool kvmStoragePoolSecondary) {
182+
String snapshotPath = null;
183+
if (snapshotOnPrimaryStorage != null) {
184+
snapshotPath = snapshotOnPrimaryStorage.getPath();
185+
if (Files.exists(Paths.get(snapshotPath))) {
186+
return new Pair<>(snapshotPath, snapshotOnPrimaryStorage);
187+
}
186188
}
187189

188190
if (kvmStoragePoolSecondary == null) {
189191
throw new CloudRuntimeException(String.format("Snapshot [%s] does not exists on secondary storage, unable to revert volume [%s] to it.",
190192
snapshotOnSecondaryStorage, snapshotOnSecondaryStorage.getVolume()));
191193
}
192194

193-
s_logger.trace(String.format("Snapshot [%s] does not exists on primary storage [%s], searching snapshot [%s] on secondary storage [%s].", snapshotOnPrimaryStorage,
194-
kvmStoragePoolPrimary, snapshotOnSecondaryStorage, kvmStoragePoolSecondary));
195+
if (s_logger.isTraceEnabled()) {
196+
s_logger.trace(String.format("Snapshot does not exists on primary storage [%s], searching snapshot [%s] on secondary storage [%s].",
197+
kvmStoragePoolPrimary, snapshotOnSecondaryStorage, kvmStoragePoolSecondary));
198+
}
195199

196200
String snapshotPathOnSecondaryStorage = snapshotOnSecondaryStorage.getPath();
197201

‎plugins/storage/volume/datera/src/main/java/org/apache/cloudstack/storage/datastore/driver/DateraPrimaryDataStoreDriver.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1536,6 +1536,7 @@ private void deleteTemplate(TemplateInfo templateInfo, long storagePoolId)
15361536

15371537
/**
15381538
* Revert snapshot for a volume
1539+
*
15391540
* @param snapshotInfo Information about volume snapshot
15401541
* @param snapshotOnPrimaryStore Not used
15411542
* @throws CloudRuntimeException

‎plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import org.apache.cloudstack.engine.subsystem.api.storage.StorageAction;
4242
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateDataFactory;
4343
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo;
44+
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
4445
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
4546
import org.apache.cloudstack.framework.async.AsyncCompletionCallback;
4647
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
@@ -121,6 +122,8 @@ public Map<String, String> getCapabilities() {
121122
TemplateManager templateManager;
122123
@Inject
123124
TemplateDataFactory templateDataFactory;
125+
@Inject
126+
VolumeDataFactory volFactory;
124127

125128
@Override
126129
public DataTO getTO(DataObject data) {
@@ -379,11 +382,21 @@ public void takeSnapshot(SnapshotInfo snapshot, AsyncCompletionCallback<CreateCm
379382

380383
@Override
381384
public void revertSnapshot(SnapshotInfo snapshot, SnapshotInfo snapshotOnPrimaryStore, AsyncCompletionCallback<CommandResult> callback) {
382-
RevertSnapshotCommand cmd = new RevertSnapshotCommand((SnapshotObjectTO)snapshot.getTO(), (SnapshotObjectTO)snapshotOnPrimaryStore.getTO());
385+
SnapshotObjectTO dataOnPrimaryStorage = null;
386+
if (snapshotOnPrimaryStore != null) {
387+
dataOnPrimaryStorage = (SnapshotObjectTO)snapshotOnPrimaryStore.getTO();
388+
}
389+
RevertSnapshotCommand cmd = new RevertSnapshotCommand((SnapshotObjectTO)snapshot.getTO(), dataOnPrimaryStorage);
383390

384391
CommandResult result = new CommandResult();
385392
try {
386-
EndPoint ep = epSelector.select(snapshotOnPrimaryStore);
393+
EndPoint ep = null;
394+
if (snapshotOnPrimaryStore != null) {
395+
ep = epSelector.select(snapshotOnPrimaryStore);
396+
} else {
397+
VolumeInfo volumeInfo = volFactory.getVolume(snapshot.getVolumeId(), DataStoreRole.Primary);
398+
ep = epSelector.select(volumeInfo);
399+
}
387400
if ( ep == null ){
388401
String errMsg = "No remote endpoint to send RevertSnapshotCommand, check if host or ssvm is down?";
389402
s_logger.error(errMsg);

0 commit comments

Comments
 (0)