Skip to content

Commit 3a2bd26

Browse files
sureshanapartidhslove
authored andcommitted
Fix for create template from snapshot (for snapshots on primary storage and storage doesn't support create snapshot to template directly) (apache#11452)
* Fix for create template from snapshot * code improvements, for create volume from snapshot
1 parent b94f914 commit 3a2bd26

File tree

3 files changed

+33
-20
lines changed

3 files changed

+33
-20
lines changed

engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -689,14 +689,19 @@ public VolumeInfo createVolumeFromSnapshot(Volume volume, Snapshot snapshot, Use
689689
VolumeInfo vol = volFactory.getVolume(volume.getId());
690690
long zoneId = volume.getDataCenterId();
691691
DataStore store = dataStoreMgr.getDataStore(pool.getId(), DataStoreRole.Primary);
692-
DataStoreRole dataStoreRole = snapshotHelper.getDataStoreRole(snapshot, zoneId);
692+
DataStoreRole dataStoreRole = snapshotHelper.getDataStoreRole(snapshot);
693693
SnapshotInfo snapInfo = snapshotFactory.getSnapshotWithRoleAndZone(snapshot.getId(), dataStoreRole, zoneId);
694+
boolean kvmSnapshotOnlyInPrimaryStorage = snapshotHelper.isKvmSnapshotOnlyInPrimaryStorage(snapshot, dataStoreRole);
695+
logger.debug("Creating volume from snapshot, with dataStore role {} and on primary storage: {}", dataStoreRole, kvmSnapshotOnlyInPrimaryStorage);
694696

695-
boolean kvmSnapshotOnlyInPrimaryStorage = snapshotHelper.isKvmSnapshotOnlyInPrimaryStorage(snapshot, dataStoreRole, volume.getDataCenterId());
696697
boolean storageSupportSnapshotToTemplateEnabled = snapshotHelper.isStorageSupportSnapshotToTemplate(snapInfo);
697-
698698
try {
699-
if (!storageSupportSnapshotToTemplateEnabled) {
699+
if (storageSupportSnapshotToTemplateEnabled) { // true only for StorPool now [TODO: Update to check storage supports snapshot to volume (DataStoreCapabilities.CAN_CREATE_VOLUME_FROM_SNAPSHOT) - may impact other storages, or StorPool storage type only]
700+
dataStoreRole = snapshotHelper.getDataStoreRole(snapshot, zoneId);
701+
snapInfo = snapshotFactory.getSnapshotWithRoleAndZone(snapshot.getId(), dataStoreRole, zoneId);
702+
kvmSnapshotOnlyInPrimaryStorage = snapshotHelper.isKvmSnapshotOnlyInPrimaryStorage(snapshot, dataStoreRole, zoneId);
703+
logger.debug("Creating volume from snapshot for storage supporting snapshot to template, with dataStore role {} and on primary storage: {}", dataStoreRole, kvmSnapshotOnlyInPrimaryStorage);
704+
} else {
700705
snapInfo = snapshotHelper.backupSnapshotToSecondaryStorageIfNotExists(snapInfo, dataStoreRole, snapshot, kvmSnapshotOnlyInPrimaryStorage);
701706
}
702707
} catch (CloudRuntimeException e) {
@@ -710,7 +715,7 @@ public VolumeInfo createVolumeFromSnapshot(Volume volume, Snapshot snapshot, Use
710715
}
711716

712717
// don't try to perform a sync if the DataStoreRole of the snapshot is equal to DataStoreRole.Primary
713-
if (!DataStoreRole.Primary.equals(dataStoreRole) || !storageSupportSnapshotToTemplateEnabled) {
718+
if (!DataStoreRole.Primary.equals(dataStoreRole) || (kvmSnapshotOnlyInPrimaryStorage && !storageSupportSnapshotToTemplateEnabled)) {
714719
try {
715720
// sync snapshot to region store if necessary
716721
DataStore snapStore = snapInfo.getDataStore();

server/src/main/java/com/cloud/template/TemplateManagerImpl.java

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1696,18 +1696,21 @@ public VirtualMachineTemplate createPrivateTemplate(CreateTemplateCmd command) t
16961696
if (store == null) {
16971697
throw new CloudRuntimeException("cannot find an image store for zone " + zoneId);
16981698
}
1699-
AsyncCallFuture<TemplateApiResult> future = null;
1699+
AsyncCallFuture<TemplateApiResult> future;
17001700

17011701
if (snapshotId != null) {
1702-
DataStoreRole dataStoreRole = snapshotHelper.getDataStoreRole(snapshot, zoneId);
1703-
kvmSnapshotOnlyInPrimaryStorage = snapshotHelper.isKvmSnapshotOnlyInPrimaryStorage(snapshot, dataStoreRole, zoneId);
1704-
1702+
DataStoreRole dataStoreRole = snapshotHelper.getDataStoreRole(snapshot);
1703+
kvmSnapshotOnlyInPrimaryStorage = snapshotHelper.isKvmSnapshotOnlyInPrimaryStorage(snapshot, dataStoreRole);
17051704
snapInfo = _snapshotFactory.getSnapshotWithRoleAndZone(snapshotId, dataStoreRole, zoneId);
1706-
boolean kvmIncrementalSnapshot = SnapshotManager.kvmIncrementalSnapshot.valueIn(_hostDao.findClusterIdByVolumeInfo(snapInfo.getBaseVolume()));
1705+
logger.debug("Creating template from snapshot, with dataStore role {} and on primary storage: {}", dataStoreRole, kvmSnapshotOnlyInPrimaryStorage);
1706+
1707+
boolean storageSupportsSnapshotToTemplate = snapshotHelper.isStorageSupportSnapshotToTemplate(snapInfo);
1708+
if (storageSupportsSnapshotToTemplate) {
1709+
dataStoreRole = snapshotHelper.getDataStoreRole(snapshot, zoneId);
1710+
kvmSnapshotOnlyInPrimaryStorage = snapshotHelper.isKvmSnapshotOnlyInPrimaryStorage(snapshot, dataStoreRole, zoneId);
1711+
snapInfo = _snapshotFactory.getSnapshotWithRoleAndZone(snapshotId, dataStoreRole, zoneId);
1712+
logger.debug("Creating template from snapshot for storage supporting snapshot to template, with dataStore role {} and on primary storage: {}", dataStoreRole, kvmSnapshotOnlyInPrimaryStorage);
17071713

1708-
boolean skipCopyToSecondary = false;
1709-
boolean keepOnPrimary = snapshotHelper.isStorageSupportSnapshotToTemplate(snapInfo);
1710-
if (keepOnPrimary) {
17111714
ImageStoreVO imageStore = _imgStoreDao.findOneByZoneAndProtocol(zoneId, "nfs");
17121715
if (imageStore == null) {
17131716
throw new CloudRuntimeException(String.format("Could not find an NFS secondary storage pool on zone %s to use as a temporary location " +
@@ -1717,7 +1720,7 @@ public VirtualMachineTemplate createPrivateTemplate(CreateTemplateCmd command) t
17171720
if (dataStore != null) {
17181721
store = dataStore;
17191722
}
1720-
} else if (dataStoreRole == DataStoreRole.Image) {
1723+
} else if (dataStoreRole == DataStoreRole.Image || kvmSnapshotOnlyInPrimaryStorage) {
17211724
snapInfo = snapshotHelper.backupSnapshotToSecondaryStorageIfNotExists(snapInfo, dataStoreRole, snapshot, kvmSnapshotOnlyInPrimaryStorage);
17221725
_accountMgr.checkAccess(caller, null, true, snapInfo);
17231726
DataStore snapStore = snapInfo.getDataStore();
@@ -1726,6 +1729,8 @@ public VirtualMachineTemplate createPrivateTemplate(CreateTemplateCmd command) t
17261729
store = snapStore; // pick snapshot image store to create template
17271730
}
17281731
}
1732+
1733+
boolean kvmIncrementalSnapshot = SnapshotManager.kvmIncrementalSnapshot.valueIn(_hostDao.findClusterIdByVolumeInfo(snapInfo.getBaseVolume()));
17291734
if (kvmIncrementalSnapshot && DataStoreRole.Image.equals(dataStoreRole)) {
17301735
snapInfo = snapshotHelper.convertSnapshotIfNeeded(snapInfo);
17311736
}

server/src/main/java/org/apache/cloudstack/snapshot/SnapshotHelper.java

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,8 @@ public void expungeTemporarySnapshot(boolean kvmSnapshotOnlyInPrimaryStorage, Sn
156156
public boolean isStorageSupportSnapshotToTemplate(SnapshotInfo snapInfo) {
157157
if (DataStoreRole.Primary.equals(snapInfo.getDataStore().getRole())) {
158158
Map<String, String> capabilities = snapInfo.getDataStore().getDriver().getCapabilities();
159-
return org.apache.commons.collections4.MapUtils.isNotEmpty(capabilities) && capabilities.containsKey(DataStoreCapabilities.CAN_CREATE_TEMPLATE_FROM_SNAPSHOT.toString());
159+
return org.apache.commons.collections4.MapUtils.isNotEmpty(capabilities)
160+
&& Boolean.parseBoolean(capabilities.get(DataStoreCapabilities.CAN_CREATE_TEMPLATE_FROM_SNAPSHOT.toString()));
160161
}
161162
return false;
162163
}
@@ -214,36 +215,36 @@ protected boolean isSnapshotBackupable(SnapshotInfo snapInfo, DataStoreRole data
214215
* @return true if hypervisor is {@link HypervisorType#KVM} and data store role is {@link DataStoreRole#Primary} and global setting "snapshot.backup.to.secondary" is false,
215216
* else false.
216217
*/
218+
public boolean isKvmSnapshotOnlyInPrimaryStorage(Snapshot snapshot, DataStoreRole dataStoreRole) {
219+
return snapshot.getHypervisorType() == Hypervisor.HypervisorType.KVM && dataStoreRole == DataStoreRole.Primary && !backupSnapshotAfterTakingSnapshot;
220+
}
221+
217222
public boolean isKvmSnapshotOnlyInPrimaryStorage(Snapshot snapshot, DataStoreRole dataStoreRole, Long zoneId){
218223
List<SnapshotJoinVO> snapshots = snapshotJoinDao.listBySnapshotIdAndZoneId(zoneId, snapshot.getSnapshotId());
219-
boolean isKvmSnapshotOnlyInPrimaryStorage = snapshots.stream().filter(s -> s.getStoreRole().equals(DataStoreRole.Image)).count() == 0;
224+
boolean isKvmSnapshotOnlyInPrimaryStorage = snapshots.stream().noneMatch(s -> s.getStoreRole().equals(DataStoreRole.Image));
220225

221226
return snapshot.getHypervisorType() == Hypervisor.HypervisorType.KVM && dataStoreRole == DataStoreRole.Primary && isKvmSnapshotOnlyInPrimaryStorage;
222227
}
223228

224229
public DataStoreRole getDataStoreRole(Snapshot snapshot) {
225230
SnapshotDataStoreVO snapshotStore = snapshotDataStoreDao.findOneBySnapshotAndDatastoreRole(snapshot.getId(), DataStoreRole.Primary);
226-
227231
if (snapshotStore == null) {
228232
return DataStoreRole.Image;
229233
}
230234

231235
long storagePoolId = snapshotStore.getDataStoreId();
232-
233236
StoragePoolVO storagePoolVO = primaryDataStoreDao.findById(storagePoolId);
234237
if ((storagePoolTypesToValidateWithBackupSnapshotAfterTakingSnapshot.contains(storagePoolVO.getPoolType()) || snapshot.getHypervisorType() == HypervisorType.KVM)
235238
&& !backupSnapshotAfterTakingSnapshot) {
236239
return DataStoreRole.Primary;
237240
}
238241

239242
DataStore dataStore = dataStorageManager.getDataStore(storagePoolId, DataStoreRole.Primary);
240-
241243
if (dataStore == null) {
242244
return DataStoreRole.Image;
243245
}
244246

245247
Map<String, String> mapCapabilities = dataStore.getDriver().getCapabilities();
246-
247248
if (MapUtils.isNotEmpty(mapCapabilities) && BooleanUtils.toBoolean(mapCapabilities.get(DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT.toString()))) {
248249
return DataStoreRole.Primary;
249250
}
@@ -255,11 +256,13 @@ public DataStoreRole getDataStoreRole(Snapshot snapshot, Long zoneId) {
255256
if (zoneId == null) {
256257
getDataStoreRole(snapshot);
257258
}
259+
258260
List<SnapshotJoinVO> snapshots = snapshotJoinDao.listBySnapshotIdAndZoneId(zoneId, snapshot.getId());
259261
boolean snapshotOnPrimary = snapshots.stream().anyMatch(s -> s.getStoreRole().equals(DataStoreRole.Primary));
260262
if (snapshotOnPrimary) {
261263
return DataStoreRole.Primary;
262264
}
265+
263266
return DataStoreRole.Image;
264267
}
265268
/**

0 commit comments

Comments
 (0)