Skip to content

Commit 64448c3

Browse files
subhash yedugundlasubhash yedugundla
subhash yedugundla
authored and
subhash yedugundla
committed
CLOUDSTACK-9572 Snapshot on primary storage not cleaned up after Storage migration
1 parent 3ee8d83 commit 64448c3

6 files changed

Lines changed: 74 additions & 30 deletions

File tree

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ public interface SnapshotDataFactory {
2929

3030
SnapshotInfo getSnapshot(long snapshotId, DataStoreRole role);
3131

32+
List<SnapshotInfo> getSnapshots(long volumeId, DataStoreRole store);
33+
3234
List<SnapshotInfo> listSnapshotOnCache(long snapshotId);
3335

3436
SnapshotInfo getReadySnapshotOnCache(long snapshotId);

‎engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotDataFactoryImpl.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,24 @@ public SnapshotInfo getSnapshot(DataObject obj, DataStore store) {
6767
return so;
6868
}
6969

70+
@Override
71+
public List<SnapshotInfo> getSnapshots(long volumeId, DataStoreRole role) {
72+
73+
SnapshotDataStoreVO snapshotStore = snapshotStoreDao.findByVolume(volumeId, role);
74+
if (snapshotStore == null) {
75+
return null;
76+
}
77+
DataStore store = storeMgr.getDataStore(snapshotStore.getDataStoreId(), role);
78+
List<SnapshotVO> volSnapShots = snapshotDao.listByVolumeId(volumeId);
79+
List<SnapshotInfo> infos = new ArrayList<>();
80+
for(SnapshotVO snapshot: volSnapShots) {
81+
SnapshotObject info = SnapshotObject.getSnapshotObject(snapshot, store);
82+
infos.add(info);
83+
}
84+
return infos;
85+
}
86+
87+
7088
@Override
7189
public SnapshotInfo getSnapshot(long snapshotId, DataStoreRole role) {
7290
SnapshotVO snapshot = snapshotDao.findById(snapshotId);

‎engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1543,6 +1543,7 @@ protected Void migrateVolumeCallBack(AsyncCallbackDispatcher<VolumeServiceImpl,
15431543
future.complete(res);
15441544
} else {
15451545
srcVolume.processEvent(Event.OperationSuccessed);
1546+
snapshotMgr.cleanupSnapshotsByVolume(srcVolume.getId());
15461547
future.complete(res);
15471548
}
15481549
} catch (Exception e) {
@@ -1624,6 +1625,7 @@ public AsyncCallFuture<CommandResult> migrateVolumes(Map<VolumeInfo, DataStore>
16241625
} else {
16251626
for (Map.Entry<VolumeInfo, DataStore> entry : volumeToPool.entrySet()) {
16261627
VolumeInfo volume = entry.getKey();
1628+
snapshotMgr.cleanupSnapshotsByVolume(volume.getId());
16271629
volume.processEvent(Event.OperationSuccessed);
16281630
}
16291631
future.complete(res);

‎server/src/com/cloud/storage/snapshot/SnapshotManager.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@ public interface SnapshotManager extends Configurable {
7474

7575
boolean canOperateOnVolume(Volume volume);
7676

77+
void cleanupSnapshotsByVolume(Long volumeId);
78+
7779
Answer sendToPool(Volume vol, Command cmd);
7880

7981
SnapshotVO getParentSnapshot(VolumeInfo volume);

‎server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1331,6 +1331,21 @@ public boolean canOperateOnVolume(Volume volume) {
13311331
return true;
13321332
}
13331333

1334+
@Override
1335+
public void cleanupSnapshotsByVolume(Long volumeId) {
1336+
List<SnapshotInfo> infos = snapshotFactory.getSnapshots(volumeId, DataStoreRole.Primary);
1337+
for(SnapshotInfo info: infos) {
1338+
try {
1339+
if(info != null) {
1340+
snapshotSrv.deleteSnapshot(info);
1341+
}
1342+
} catch(CloudRuntimeException e) {
1343+
String msg = "Cleanup of Snapshot with uuid " + info.getUuid() + " in primary storage is failed. Ignoring";
1344+
s_logger.warn(msg);
1345+
}
1346+
}
1347+
}
1348+
13341349
@Override
13351350
public Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName, Snapshot.LocationType locationType) throws ResourceAllocationException {
13361351
Account caller = CallContext.current().getCallingAccount();

‎server/test/com/cloud/storage/snapshot/SnapshotManagerTest.java‎

Lines changed: 35 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,37 @@
1616
// under the License.
1717
package com.cloud.storage.snapshot;
1818

19+
import static org.mockito.Matchers.any;
20+
import static org.mockito.Matchers.anyLong;
21+
import static org.mockito.Mockito.doNothing;
22+
import static org.mockito.Mockito.mock;
23+
import static org.mockito.Mockito.when;
24+
25+
import java.util.List;
26+
import java.util.UUID;
27+
28+
import org.apache.cloudstack.acl.ControlledEntity;
29+
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
30+
import org.apache.cloudstack.context.CallContext;
31+
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
32+
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory;
33+
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
34+
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy;
35+
import org.apache.cloudstack.engine.subsystem.api.storage.StorageStrategyFactory;
36+
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
37+
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
38+
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy.SnapshotOperation;
39+
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
40+
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
41+
import org.junit.After;
42+
import org.junit.Assert;
43+
import org.junit.Before;
44+
import org.junit.Test;
45+
import org.mockito.Mock;
46+
import org.mockito.Mockito;
47+
import org.mockito.MockitoAnnotations;
48+
import org.mockito.Spy;
49+
1950
import com.cloud.configuration.Resource.ResourceType;
2051
import com.cloud.exception.InvalidParameterValueException;
2152
import com.cloud.exception.ResourceAllocationException;
@@ -44,38 +75,10 @@
4475
import com.cloud.vm.snapshot.VMSnapshot;
4576
import com.cloud.vm.snapshot.VMSnapshotVO;
4677
import com.cloud.vm.snapshot.dao.VMSnapshotDao;
47-
import org.apache.cloudstack.acl.ControlledEntity;
48-
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
49-
import org.apache.cloudstack.context.CallContext;
50-
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
51-
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory;
52-
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
53-
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy;
54-
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy.SnapshotOperation;
55-
import org.apache.cloudstack.engine.subsystem.api.storage.StorageStrategyFactory;
56-
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
57-
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
58-
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
5978
import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
6079
import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO;
61-
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
62-
import org.junit.After;
63-
import org.junit.Assert;
64-
import org.junit.Before;
65-
import org.junit.Test;
66-
import org.mockito.Mock;
67-
import org.mockito.Mockito;
68-
import org.mockito.MockitoAnnotations;
69-
import org.mockito.Spy;
80+
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotService;
7081

71-
import java.util.List;
72-
import java.util.UUID;
73-
74-
import static org.mockito.Matchers.any;
75-
import static org.mockito.Matchers.anyLong;
76-
import static org.mockito.Mockito.doNothing;
77-
import static org.mockito.Mockito.mock;
78-
import static org.mockito.Mockito.when;
7982

8083
public class SnapshotManagerTest {
8184
@Spy
@@ -126,6 +129,9 @@ public class SnapshotManagerTest {
126129
SnapshotDataStoreDao _snapshotStoreDao;
127130
@Mock
128131
SnapshotDataStoreVO snapshotStoreMock;
132+
@Mock
133+
SnapshotService snapshotSrv;
134+
129135

130136
private static final long TEST_SNAPSHOT_ID = 3L;
131137
private static final long TEST_VOLUME_ID = 4L;
@@ -330,5 +336,4 @@ public void testBackupSnapshotFromVmSnapshotF3() {
330336
Snapshot snapshot = _snapshotMgr.backupSnapshotFromVmSnapshot(TEST_SNAPSHOT_ID, TEST_VM_ID, TEST_VOLUME_ID, TEST_VM_SNAPSHOT_ID);
331337
Assert.assertNull(snapshot);
332338
}
333-
334339
}

0 commit comments

Comments
 (0)