Skip to content

Commit fcfc36c

Browse files
sureshanapartiLocharla, Sandeep
authored andcommitted
Cleanup snapshot files in datastores for Error-ed snapshots, and some code improvements (apache#12347)
1 parent 9547547 commit fcfc36c

File tree

15 files changed

+97
-68
lines changed

15 files changed

+97
-68
lines changed

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

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

3131
SnapshotInfo getSnapshot(long snapshotId, long storeId, DataStoreRole role);
3232

33+
SnapshotInfo getSnapshotIncludingRemoved(long snapshotId, long storeId, DataStoreRole role);
34+
3335
SnapshotInfo getSnapshotWithRoleAndZone(long snapshotId, DataStoreRole role, long zoneId);
3436

3537
SnapshotInfo getSnapshotOnPrimaryStore(long snapshotId);

engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDao.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ public interface SnapshotDao extends GenericDao<SnapshotVO, Long>, StateDao<Snap
4747

4848
List<SnapshotVO> listAllByStatus(Snapshot.State... status);
4949

50+
List<SnapshotVO> listAllByStatusIncludingRemoved(Snapshot.State... status);
51+
5052
void updateVolumeIds(long oldVolId, long newVolId);
5153

5254
List<SnapshotVO> listByStatusNotIn(long volumeId, Snapshot.State... status);

engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDaoImpl.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,13 @@ public List<SnapshotVO> listAllByStatus(Snapshot.State... status) {
265265
return listBy(sc, null);
266266
}
267267

268+
@Override
269+
public List<SnapshotVO> listAllByStatusIncludingRemoved(Snapshot.State... status) {
270+
SearchCriteria<SnapshotVO> sc = StatusSearch.create();
271+
sc.setParameters("status", (Object[])status);
272+
return listIncludingRemovedBy(sc, null);
273+
}
274+
268275
@Override
269276
public List<SnapshotVO> listByIds(Object... ids) {
270277
SearchCriteria<SnapshotVO> sc = snapshotIdsSearch.create();

engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDao.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,8 @@ public interface SnapshotDataStoreDao extends GenericDao<SnapshotDataStoreVO, Lo
7676

7777
List<SnapshotDataStoreVO> findBySnapshotId(long snapshotId);
7878

79+
List<SnapshotDataStoreVO> findBySnapshotIdWithNonDestroyedState(long snapshotId);
80+
7981
void duplicateCacheRecordsOnRegionStore(long storeId);
8082

8183
// delete the snapshot entry on primary data store to make sure that next snapshot will be full snapshot

engine/storage/integration-test/src/test/java/org/apache/cloudstack/storage/test/SnapshotTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ public void setUp() {
269269
to.setSize(1000L);
270270
CopyCmdAnswer answer = new CopyCmdAnswer(to);
271271
templateOnStore.processEvent(Event.CreateOnlyRequested);
272-
templateOnStore.processEvent(Event.OperationSuccessed, answer);
272+
templateOnStore.processEvent(Event.OperationSucceeded, answer);
273273

274274
}
275275

engine/storage/integration-test/src/test/java/org/apache/cloudstack/storage/test/VolumeTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ public void setUp() {
244244
to.setSize(100L);
245245
CopyCmdAnswer answer = new CopyCmdAnswer(to);
246246
templateOnStore.processEvent(Event.CreateOnlyRequested);
247-
templateOnStore.processEvent(Event.OperationSuccessed, answer);
247+
templateOnStore.processEvent(Event.OperationSucceeded, answer);
248248

249249
}
250250

engine/storage/integration-test/src/test/java/org/apache/cloudstack/storage/test/VolumeTestVmware.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ public void setUp() {
247247
to.setPath(this.getImageInstallPath());
248248
CopyCmdAnswer answer = new CopyCmdAnswer(to);
249249
templateOnStore.processEvent(Event.CreateOnlyRequested);
250-
templateOnStore.processEvent(Event.OperationSuccessed, answer);
250+
templateOnStore.processEvent(Event.OperationSucceeded, answer);
251251

252252
}
253253

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

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ public List<SnapshotInfo> getSnapshots(long snapshotId, Long zoneId) {
9494
if (snapshot == null) { //snapshot may have been removed;
9595
return new ArrayList<>();
9696
}
97-
List<SnapshotDataStoreVO> allSnapshotsAndDataStore = snapshotStoreDao.findBySnapshotId(snapshotId);
97+
List<SnapshotDataStoreVO> allSnapshotsAndDataStore = snapshotStoreDao.findBySnapshotIdWithNonDestroyedState(snapshotId);
9898
if (CollectionUtils.isEmpty(allSnapshotsAndDataStore)) {
9999
return new ArrayList<>();
100100
}
@@ -118,7 +118,23 @@ public SnapshotInfo getSnapshot(long snapshotId, long storeId, DataStoreRole rol
118118
if (snapshot == null) {
119119
return null;
120120
}
121-
SnapshotDataStoreVO snapshotStore = snapshotStoreDao.findByStoreSnapshot(role, storeId, snapshotId);
121+
return getSnapshotOnStore(snapshot, storeId, role);
122+
}
123+
124+
@Override
125+
public SnapshotInfo getSnapshotIncludingRemoved(long snapshotId, long storeId, DataStoreRole role) {
126+
SnapshotVO snapshot = snapshotDao.findByIdIncludingRemoved(snapshotId);
127+
if (snapshot == null) {
128+
return null;
129+
}
130+
return getSnapshotOnStore(snapshot, storeId, role);
131+
}
132+
133+
private SnapshotInfo getSnapshotOnStore(SnapshotVO snapshot, long storeId, DataStoreRole role) {
134+
if (snapshot == null) {
135+
return null;
136+
}
137+
SnapshotDataStoreVO snapshotStore = snapshotStoreDao.findByStoreSnapshot(role, storeId, snapshot.getId());
122138
if (snapshotStore == null) {
123139
return null;
124140
}
@@ -207,7 +223,7 @@ public List<SnapshotInfo> listSnapshotOnCache(long snapshotId) {
207223

208224
@Override
209225
public void updateOperationFailed(long snapshotId) throws NoTransitionException {
210-
List<SnapshotDataStoreVO> snapshotStoreRefs = snapshotStoreDao.findBySnapshotId(snapshotId);
226+
List<SnapshotDataStoreVO> snapshotStoreRefs = snapshotStoreDao.findBySnapshotIdWithNonDestroyedState(snapshotId);
211227
for (SnapshotDataStoreVO snapshotStoreRef : snapshotStoreRefs) {
212228
SnapshotInfo snapshotInfo = getSnapshot(snapshotStoreRef.getSnapshotId(), snapshotStoreRef.getDataStoreId(), snapshotStoreRef.getRole());
213229
if (snapshotInfo != null) {

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

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -474,16 +474,14 @@ public SnapshotInfo backupSnapshot(SnapshotInfo snapshot) {
474474
if (res.isFailed()) {
475475
throw new CloudRuntimeException(res.getResult());
476476
}
477-
SnapshotInfo destSnapshot = res.getSnapshot();
478-
return destSnapshot;
477+
return res.getSnapshot();
479478
} catch (InterruptedException e) {
480479
logger.debug("failed copy snapshot", e);
481480
throw new CloudRuntimeException("Failed to copy snapshot", e);
482481
} catch (ExecutionException e) {
483482
logger.debug("Failed to copy snapshot", e);
484483
throw new CloudRuntimeException("Failed to copy snapshot", e);
485484
}
486-
487485
}
488486

489487
protected Void copySnapshotAsyncCallback(AsyncCallbackDispatcher<SnapshotServiceImpl, CopyCommandResult> callback, CopySnapshotContext<CommandResult> context) {
@@ -571,7 +569,6 @@ protected Void prepareCopySnapshotZoneAsyncCallback(AsyncCallbackDispatcher<Snap
571569
}
572570

573571
protected Void deleteSnapshotCallback(AsyncCallbackDispatcher<SnapshotServiceImpl, CommandResult> callback, DeleteSnapshotContext<CommandResult> context) {
574-
575572
CommandResult result = callback.getResult();
576573
AsyncCallFuture<SnapshotResult> future = context.future;
577574
SnapshotInfo snapshot = context.snapshot;
@@ -729,7 +726,7 @@ public void cleanupVolumeDuringSnapshotFailure(Long volumeId, Long snapshotId) {
729726

730727
if (snapshot != null) {
731728
if (snapshot.getState() != Snapshot.State.BackedUp) {
732-
List<SnapshotDataStoreVO> snapshotDataStoreVOs = _snapshotStoreDao.findBySnapshotId(snapshotId);
729+
List<SnapshotDataStoreVO> snapshotDataStoreVOs = _snapshotStoreDao.findBySnapshotIdWithNonDestroyedState(snapshotId);
733730
for (SnapshotDataStoreVO snapshotDataStoreVO : snapshotDataStoreVOs) {
734731
logger.debug("Remove snapshot {}, status {} on snapshot_store_ref table with id: {}", snapshot, snapshotDataStoreVO.getState(), snapshotDataStoreVO.getId());
735732

@@ -834,7 +831,6 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
834831
SnapshotObject srcSnapshot = (SnapshotObject)snapshot;
835832
srcSnapshot.processEvent(Event.DestroyRequested);
836833
srcSnapshot.processEvent(Event.OperationSucceeded);
837-
838834
srcSnapshot.processEvent(Snapshot.Event.OperationFailed);
839835

840836
_snapshotDetailsDao.removeDetail(srcSnapshot.getId(), AsyncJob.Constants.MS_ID);
@@ -845,7 +841,6 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
845841
}
846842
}
847843
});
848-
849844
}
850845

851846
@Override

engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -541,7 +541,6 @@ public void postSnapshotCreation(SnapshotInfo snapshot) {
541541
logger.warn("Failed to clean up snapshot '" + snapshot.getId() + "' on primary storage: " + e.getMessage());
542542
}
543543
}
544-
545544
}
546545

547546
private VMSnapshot takeHypervisorSnapshot(VolumeInfo volumeInfo) {

0 commit comments

Comments
 (0)