Skip to content

Commit f671461

Browse files
sureshanapartisureshanaparti
authored
Fix for create template from snapshot (for snapshots on primary storage and storage doesn't support create snapshot to template directly) (#11452)
* Fix for create template from snapshot * code improvements, for create volume from snapshot
1 parent ba2d70a commit f671461

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
@@ -579,14 +579,19 @@ public VolumeInfo createVolumeFromSnapshot(Volume volume, Snapshot snapshot, Use
579579
VolumeInfo vol = volFactory.getVolume(volume.getId());
580580
long zoneId = volume.getDataCenterId();
581581
DataStore store = dataStoreMgr.getDataStore(pool.getId(), DataStoreRole.Primary);
582-
DataStoreRole dataStoreRole = snapshotHelper.getDataStoreRole(snapshot, zoneId);
582+
DataStoreRole dataStoreRole = snapshotHelper.getDataStoreRole(snapshot);
583583
SnapshotInfo snapInfo = snapshotFactory.getSnapshotWithRoleAndZone(snapshot.getId(), dataStoreRole, zoneId);
584+
boolean kvmSnapshotOnlyInPrimaryStorage = snapshotHelper.isKvmSnapshotOnlyInPrimaryStorage(snapshot, dataStoreRole);
585+
logger.debug("Creating volume from snapshot, with dataStore role {} and on primary storage: {}", dataStoreRole, kvmSnapshotOnlyInPrimaryStorage);
584586

585-
boolean kvmSnapshotOnlyInPrimaryStorage = snapshotHelper.isKvmSnapshotOnlyInPrimaryStorage(snapshot, dataStoreRole, volume.getDataCenterId());
586587
boolean storageSupportSnapshotToTemplateEnabled = snapshotHelper.isStorageSupportSnapshotToTemplate(snapInfo);
587-
588588
try {
589-
if (!storageSupportSnapshotToTemplateEnabled) {
589+
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]
590+
dataStoreRole = snapshotHelper.getDataStoreRole(snapshot, zoneId);
591+
snapInfo = snapshotFactory.getSnapshotWithRoleAndZone(snapshot.getId(), dataStoreRole, zoneId);
592+
kvmSnapshotOnlyInPrimaryStorage = snapshotHelper.isKvmSnapshotOnlyInPrimaryStorage(snapshot, dataStoreRole, zoneId);
593+
logger.debug("Creating volume from snapshot for storage supporting snapshot to template, with dataStore role {} and on primary storage: {}", dataStoreRole, kvmSnapshotOnlyInPrimaryStorage);
594+
} else {
590595
snapInfo = snapshotHelper.backupSnapshotToSecondaryStorageIfNotExists(snapInfo, dataStoreRole, snapshot, kvmSnapshotOnlyInPrimaryStorage);
591596
}
592597
} catch (CloudRuntimeException e) {
@@ -600,7 +605,7 @@ public VolumeInfo createVolumeFromSnapshot(Volume volume, Snapshot snapshot, Use
600605
}
601606

602607
// don't try to perform a sync if the DataStoreRole of the snapshot is equal to DataStoreRole.Primary
603-
if (!DataStoreRole.Primary.equals(dataStoreRole) || !storageSupportSnapshotToTemplateEnabled) {
608+
if (!DataStoreRole.Primary.equals(dataStoreRole) || (kvmSnapshotOnlyInPrimaryStorage && !storageSupportSnapshotToTemplateEnabled)) {
604609
try {
605610
// sync snapshot to region store if necessary
606611
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
@@ -1692,18 +1692,21 @@ public VirtualMachineTemplate createPrivateTemplate(CreateTemplateCmd command) t
16921692
if (store == null) {
16931693
throw new CloudRuntimeException("cannot find an image store for zone " + zoneId);
16941694
}
1695-
AsyncCallFuture<TemplateApiResult> future = null;
1695+
AsyncCallFuture<TemplateApiResult> future;
16961696

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

1704-
boolean skipCopyToSecondary = false;
1705-
boolean keepOnPrimary = snapshotHelper.isStorageSupportSnapshotToTemplate(snapInfo);
1706-
if (keepOnPrimary) {
17071710
ImageStoreVO imageStore = _imgStoreDao.findOneByZoneAndProtocol(zoneId, "nfs");
17081711
if (imageStore == null) {
17091712
throw new CloudRuntimeException(String.format("Could not find an NFS secondary storage pool on zone %s to use as a temporary location " +
@@ -1713,7 +1716,7 @@ public VirtualMachineTemplate createPrivateTemplate(CreateTemplateCmd command) t
17131716
if (dataStore != null) {
17141717
store = dataStore;
17151718
}
1716-
} else if (dataStoreRole == DataStoreRole.Image) {
1719+
} else if (dataStoreRole == DataStoreRole.Image || kvmSnapshotOnlyInPrimaryStorage) {
17171720
snapInfo = snapshotHelper.backupSnapshotToSecondaryStorageIfNotExists(snapInfo, dataStoreRole, snapshot, kvmSnapshotOnlyInPrimaryStorage);
17181721
_accountMgr.checkAccess(caller, null, true, snapInfo);
17191722
DataStore snapStore = snapInfo.getDataStore();
@@ -1722,6 +1725,8 @@ public VirtualMachineTemplate createPrivateTemplate(CreateTemplateCmd command) t
17221725
store = snapStore; // pick snapshot image store to create template
17231726
}
17241727
}
1728+
1729+
boolean kvmIncrementalSnapshot = SnapshotManager.kvmIncrementalSnapshot.valueIn(_hostDao.findClusterIdByVolumeInfo(snapInfo.getBaseVolume()));
17251730
if (kvmIncrementalSnapshot && DataStoreRole.Image.equals(dataStoreRole)) {
17261731
snapInfo = snapshotHelper.convertSnapshotIfNeeded(snapInfo);
17271732
}

‎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)