Skip to content

Commit 4ffb949

Browse files
authored
Fix of revert RBD snapshots (apache#5544)
* Fix of revert RBD snapshots If snapshot is taken only on Primary storage with the option "snapshot.backup.to.secondary" set to true, when you set this option to false the revert will fail. Added check if the snapshot is not on Secondary to check for it on Primary * Check if snapshot is on primary storage Will check first if the snapshot is on Primary storage, if not will return Image as data store * Fix unit tests * removed unused method's params * Formatted error message and added the snapshot ID to it * Return to the old logic, the fix will only apply to RBD * Formatted Exception's messages
1 parent f88f934 commit 4ffb949

File tree

1 file changed

+10
-13
lines changed

1 file changed

+10
-13
lines changed

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

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -308,11 +308,12 @@ public Snapshot revertSnapshot(Long snapshotId) {
308308
}
309309
}
310310

311-
DataStoreRole dataStoreRole = getDataStoreRole(snapshot, _snapshotStoreDao, dataStoreMgr);
311+
DataStoreRole dataStoreRole = getDataStoreRole(snapshot);
312312

313313
SnapshotInfo snapshotInfo = snapshotFactory.getSnapshot(snapshotId, dataStoreRole);
314+
314315
if (snapshotInfo == null) {
315-
throw new CloudRuntimeException("snapshot:" + snapshotId + " not exist in data store");
316+
throw new CloudRuntimeException(String.format("snapshot %s [%s] does not exists in data store", snapshot.getName(), snapshot.getUuid()));
316317
}
317318

318319
SnapshotStrategy snapshotStrategy = _storageStrategyFactory.getSnapshotStrategy(snapshot, SnapshotOperation.REVERT);
@@ -587,7 +588,7 @@ public boolean deleteSnapshot(long snapshotId) {
587588
return false;
588589
}
589590

590-
DataStoreRole dataStoreRole = getDataStoreRole(snapshotCheck, _snapshotStoreDao, dataStoreMgr);
591+
DataStoreRole dataStoreRole = getDataStoreRole(snapshotCheck);
591592

592593
SnapshotDataStoreVO snapshotStoreRef = _snapshotStoreDao.findBySnapshot(snapshotId, dataStoreRole);
593594

@@ -1238,15 +1239,11 @@ public SnapshotInfo takeSnapshot(VolumeInfo volume) throws ResourceAllocationExc
12381239
try {
12391240
postCreateSnapshot(volume.getId(), snapshotId, payload.getSnapshotPolicyId());
12401241

1241-
DataStoreRole dataStoreRole = getDataStoreRole(snapshot, _snapshotStoreDao, dataStoreMgr);
1242+
DataStoreRole dataStoreRole = getDataStoreRole(snapshot);
12421243

12431244
SnapshotDataStoreVO snapshotStoreRef = _snapshotStoreDao.findBySnapshot(snapshotId, dataStoreRole);
12441245
if (snapshotStoreRef == null) {
1245-
// The snapshot was not backed up to secondary. Find the snap on primary
1246-
snapshotStoreRef = _snapshotStoreDao.findBySnapshot(snapshotId, DataStoreRole.Primary);
1247-
if (snapshotStoreRef == null) {
1248-
throw new CloudRuntimeException("Could not find snapshot");
1249-
}
1246+
throw new CloudRuntimeException(String.format("Could not find snapshot %s [%s] on [%s]", snapshot.getName(), snapshot.getUuid(), snapshot.getLocationType()));
12501247
}
12511248
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_SNAPSHOT_CREATE, snapshot.getAccountId(), snapshot.getDataCenterId(), snapshotId, snapshot.getName(), null, null,
12521249
snapshotStoreRef.getPhysicalSize(), volume.getSize(), snapshot.getClass().getName(), snapshot.getUuid());
@@ -1332,8 +1329,8 @@ private void updateSnapshotPayload(long storagePoolId, CreateSnapshotPayload pay
13321329
}
13331330
}
13341331

1335-
private DataStoreRole getDataStoreRole(Snapshot snapshot, SnapshotDataStoreDao snapshotStoreDao, DataStoreManager dataStoreMgr) {
1336-
SnapshotDataStoreVO snapshotStore = snapshotStoreDao.findBySnapshot(snapshot.getId(), DataStoreRole.Primary);
1332+
private DataStoreRole getDataStoreRole(Snapshot snapshot) {
1333+
SnapshotDataStoreVO snapshotStore = _snapshotStoreDao.findBySnapshot(snapshot.getId(), DataStoreRole.Primary);
13371334

13381335
if (snapshotStore == null) {
13391336
return DataStoreRole.Image;
@@ -1346,15 +1343,15 @@ private DataStoreRole getDataStoreRole(Snapshot snapshot, SnapshotDataStoreDao s
13461343

13471344
if (mapCapabilities != null) {
13481345
String value = mapCapabilities.get(DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT.toString());
1349-
Boolean supportsStorageSystemSnapshots = new Boolean(value);
1346+
Boolean supportsStorageSystemSnapshots = Boolean.valueOf(value);
13501347

13511348
if (supportsStorageSystemSnapshots) {
13521349
return DataStoreRole.Primary;
13531350
}
13541351
}
13551352

13561353
StoragePoolVO storagePoolVO = _storagePoolDao.findById(storagePoolId);
1357-
if ((storagePoolVO.getPoolType() == StoragePoolType.RBD || storagePoolVO.getPoolType() == StoragePoolType.PowerFlex) && !BackupSnapshotAfterTakingSnapshot.value()) {
1354+
if (storagePoolVO.getPoolType() == StoragePoolType.RBD) {
13581355
return DataStoreRole.Primary;
13591356
}
13601357

0 commit comments

Comments
 (0)