From 691d2619cd168dc07611f84825ccd9e63a66cdae Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Fri, 15 Aug 2025 03:14:26 +0530 Subject: [PATCH 1/3] Fix for create template from snapshot --- .../main/java/com/cloud/template/TemplateManagerImpl.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index 9d9ec4a7c5c3..89b124e1b038 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -1701,9 +1701,8 @@ public VirtualMachineTemplate createPrivateTemplate(CreateTemplateCmd command) t snapInfo = _snapshotFactory.getSnapshotWithRoleAndZone(snapshotId, dataStoreRole, zoneId); boolean kvmIncrementalSnapshot = SnapshotManager.kvmIncrementalSnapshot.valueIn(_hostDao.findClusterIdByVolumeInfo(snapInfo.getBaseVolume())); - boolean skipCopyToSecondary = false; - boolean keepOnPrimary = snapshotHelper.isStorageSupportSnapshotToTemplate(snapInfo); - if (keepOnPrimary) { + boolean storageSupportsSnapshotToTemplate = snapshotHelper.isStorageSupportSnapshotToTemplate(snapInfo); + if (storageSupportsSnapshotToTemplate) { ImageStoreVO imageStore = _imgStoreDao.findOneByZoneAndProtocol(zoneId, "nfs"); if (imageStore == null) { throw new CloudRuntimeException(String.format("Could not find an NFS secondary storage pool on zone %s to use as a temporary location " + @@ -1713,7 +1712,7 @@ public VirtualMachineTemplate createPrivateTemplate(CreateTemplateCmd command) t if (dataStore != null) { store = dataStore; } - } else if (dataStoreRole == DataStoreRole.Image) { + } else if (dataStoreRole == DataStoreRole.Image || kvmSnapshotOnlyInPrimaryStorage) { snapInfo = snapshotHelper.backupSnapshotToSecondaryStorageIfNotExists(snapInfo, dataStoreRole, snapshot, kvmSnapshotOnlyInPrimaryStorage); _accountMgr.checkAccess(caller, null, true, snapInfo); DataStore snapStore = snapInfo.getDataStore(); From 6a781cc5f4911068b12ca2b5515bec4af6c19391 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Fri, 15 Aug 2025 14:59:30 +0530 Subject: [PATCH 2/3] code improvements --- .../com/cloud/template/TemplateManagerImpl.java | 16 +++++++++++----- .../cloudstack/snapshot/SnapshotHelper.java | 15 +++++++++------ 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index 89b124e1b038..51d9249fad79 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -1692,17 +1692,21 @@ public VirtualMachineTemplate createPrivateTemplate(CreateTemplateCmd command) t if (store == null) { throw new CloudRuntimeException("cannot find an image store for zone " + zoneId); } - AsyncCallFuture future = null; + AsyncCallFuture future; if (snapshotId != null) { - DataStoreRole dataStoreRole = snapshotHelper.getDataStoreRole(snapshot, zoneId); - kvmSnapshotOnlyInPrimaryStorage = snapshotHelper.isKvmSnapshotOnlyInPrimaryStorage(snapshot, dataStoreRole, zoneId); - + DataStoreRole dataStoreRole = snapshotHelper.getDataStoreRole(snapshot); + kvmSnapshotOnlyInPrimaryStorage = snapshotHelper.isKvmSnapshotOnlyInPrimaryStorage(snapshot, dataStoreRole); snapInfo = _snapshotFactory.getSnapshotWithRoleAndZone(snapshotId, dataStoreRole, zoneId); - boolean kvmIncrementalSnapshot = SnapshotManager.kvmIncrementalSnapshot.valueIn(_hostDao.findClusterIdByVolumeInfo(snapInfo.getBaseVolume())); + logger.debug("Creating template for snapshot, with dataStore role {} and on primary storage: {}", dataStoreRole, kvmSnapshotOnlyInPrimaryStorage); boolean storageSupportsSnapshotToTemplate = snapshotHelper.isStorageSupportSnapshotToTemplate(snapInfo); if (storageSupportsSnapshotToTemplate) { + dataStoreRole = snapshotHelper.getDataStoreRole(snapshot, zoneId); + kvmSnapshotOnlyInPrimaryStorage = snapshotHelper.isKvmSnapshotOnlyInPrimaryStorage(snapshot, dataStoreRole, zoneId); + snapInfo = _snapshotFactory.getSnapshotWithRoleAndZone(snapshotId, dataStoreRole, zoneId); + logger.debug("Creating template for snapshot for storage supporting snapshot to template, with dataStore role {} and on primary storage: {}", dataStoreRole, kvmSnapshotOnlyInPrimaryStorage); + ImageStoreVO imageStore = _imgStoreDao.findOneByZoneAndProtocol(zoneId, "nfs"); if (imageStore == null) { throw new CloudRuntimeException(String.format("Could not find an NFS secondary storage pool on zone %s to use as a temporary location " + @@ -1721,6 +1725,8 @@ public VirtualMachineTemplate createPrivateTemplate(CreateTemplateCmd command) t store = snapStore; // pick snapshot image store to create template } } + + boolean kvmIncrementalSnapshot = SnapshotManager.kvmIncrementalSnapshot.valueIn(_hostDao.findClusterIdByVolumeInfo(snapInfo.getBaseVolume())); if (kvmIncrementalSnapshot && DataStoreRole.Image.equals(dataStoreRole)) { snapInfo = snapshotHelper.convertSnapshotIfNeeded(snapInfo); } diff --git a/server/src/main/java/org/apache/cloudstack/snapshot/SnapshotHelper.java b/server/src/main/java/org/apache/cloudstack/snapshot/SnapshotHelper.java index e7ec8c8208ff..b88f8ddb731e 100644 --- a/server/src/main/java/org/apache/cloudstack/snapshot/SnapshotHelper.java +++ b/server/src/main/java/org/apache/cloudstack/snapshot/SnapshotHelper.java @@ -156,7 +156,8 @@ public void expungeTemporarySnapshot(boolean kvmSnapshotOnlyInPrimaryStorage, Sn public boolean isStorageSupportSnapshotToTemplate(SnapshotInfo snapInfo) { if (DataStoreRole.Primary.equals(snapInfo.getDataStore().getRole())) { Map capabilities = snapInfo.getDataStore().getDriver().getCapabilities(); - return org.apache.commons.collections4.MapUtils.isNotEmpty(capabilities) && capabilities.containsKey(DataStoreCapabilities.CAN_CREATE_TEMPLATE_FROM_SNAPSHOT.toString()); + return org.apache.commons.collections4.MapUtils.isNotEmpty(capabilities) + && Boolean.parseBoolean(capabilities.get(DataStoreCapabilities.CAN_CREATE_TEMPLATE_FROM_SNAPSHOT.toString())); } return false; } @@ -214,22 +215,24 @@ protected boolean isSnapshotBackupable(SnapshotInfo snapInfo, DataStoreRole data * @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, * else false. */ + public boolean isKvmSnapshotOnlyInPrimaryStorage(Snapshot snapshot, DataStoreRole dataStoreRole) { + return snapshot.getHypervisorType() == Hypervisor.HypervisorType.KVM && dataStoreRole == DataStoreRole.Primary && !backupSnapshotAfterTakingSnapshot; + } + public boolean isKvmSnapshotOnlyInPrimaryStorage(Snapshot snapshot, DataStoreRole dataStoreRole, Long zoneId){ List snapshots = snapshotJoinDao.listBySnapshotIdAndZoneId(zoneId, snapshot.getSnapshotId()); - boolean isKvmSnapshotOnlyInPrimaryStorage = snapshots.stream().filter(s -> s.getStoreRole().equals(DataStoreRole.Image)).count() == 0; + boolean isKvmSnapshotOnlyInPrimaryStorage = snapshots.stream().noneMatch(s -> s.getStoreRole().equals(DataStoreRole.Image)); return snapshot.getHypervisorType() == Hypervisor.HypervisorType.KVM && dataStoreRole == DataStoreRole.Primary && isKvmSnapshotOnlyInPrimaryStorage; } public DataStoreRole getDataStoreRole(Snapshot snapshot) { SnapshotDataStoreVO snapshotStore = snapshotDataStoreDao.findOneBySnapshotAndDatastoreRole(snapshot.getId(), DataStoreRole.Primary); - if (snapshotStore == null) { return DataStoreRole.Image; } long storagePoolId = snapshotStore.getDataStoreId(); - StoragePoolVO storagePoolVO = primaryDataStoreDao.findById(storagePoolId); if ((storagePoolTypesToValidateWithBackupSnapshotAfterTakingSnapshot.contains(storagePoolVO.getPoolType()) || snapshot.getHypervisorType() == HypervisorType.KVM) && !backupSnapshotAfterTakingSnapshot) { @@ -237,13 +240,11 @@ public DataStoreRole getDataStoreRole(Snapshot snapshot) { } DataStore dataStore = dataStorageManager.getDataStore(storagePoolId, DataStoreRole.Primary); - if (dataStore == null) { return DataStoreRole.Image; } Map mapCapabilities = dataStore.getDriver().getCapabilities(); - if (MapUtils.isNotEmpty(mapCapabilities) && BooleanUtils.toBoolean(mapCapabilities.get(DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT.toString()))) { return DataStoreRole.Primary; } @@ -255,11 +256,13 @@ public DataStoreRole getDataStoreRole(Snapshot snapshot, Long zoneId) { if (zoneId == null) { getDataStoreRole(snapshot); } + List snapshots = snapshotJoinDao.listBySnapshotIdAndZoneId(zoneId, snapshot.getId()); boolean snapshotOnPrimary = snapshots.stream().anyMatch(s -> s.getStoreRole().equals(DataStoreRole.Primary)); if (snapshotOnPrimary) { return DataStoreRole.Primary; } + return DataStoreRole.Image; } /** From 1aef01f64f47c4d4cd8930761d86e47ccc76ccc9 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Fri, 15 Aug 2025 17:27:35 +0530 Subject: [PATCH 3/3] code improvements, for create volume from snapshot --- .../engine/orchestration/VolumeOrchestrator.java | 15 ++++++++++----- .../com/cloud/template/TemplateManagerImpl.java | 4 ++-- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java index e98a5b78a94c..2668d44f0958 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java @@ -579,14 +579,19 @@ public VolumeInfo createVolumeFromSnapshot(Volume volume, Snapshot snapshot, Use VolumeInfo vol = volFactory.getVolume(volume.getId()); long zoneId = volume.getDataCenterId(); DataStore store = dataStoreMgr.getDataStore(pool.getId(), DataStoreRole.Primary); - DataStoreRole dataStoreRole = snapshotHelper.getDataStoreRole(snapshot, zoneId); + DataStoreRole dataStoreRole = snapshotHelper.getDataStoreRole(snapshot); SnapshotInfo snapInfo = snapshotFactory.getSnapshotWithRoleAndZone(snapshot.getId(), dataStoreRole, zoneId); + boolean kvmSnapshotOnlyInPrimaryStorage = snapshotHelper.isKvmSnapshotOnlyInPrimaryStorage(snapshot, dataStoreRole); + logger.debug("Creating volume from snapshot, with dataStore role {} and on primary storage: {}", dataStoreRole, kvmSnapshotOnlyInPrimaryStorage); - boolean kvmSnapshotOnlyInPrimaryStorage = snapshotHelper.isKvmSnapshotOnlyInPrimaryStorage(snapshot, dataStoreRole, volume.getDataCenterId()); boolean storageSupportSnapshotToTemplateEnabled = snapshotHelper.isStorageSupportSnapshotToTemplate(snapInfo); - try { - if (!storageSupportSnapshotToTemplateEnabled) { + 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] + dataStoreRole = snapshotHelper.getDataStoreRole(snapshot, zoneId); + snapInfo = snapshotFactory.getSnapshotWithRoleAndZone(snapshot.getId(), dataStoreRole, zoneId); + kvmSnapshotOnlyInPrimaryStorage = snapshotHelper.isKvmSnapshotOnlyInPrimaryStorage(snapshot, dataStoreRole, zoneId); + logger.debug("Creating volume from snapshot for storage supporting snapshot to template, with dataStore role {} and on primary storage: {}", dataStoreRole, kvmSnapshotOnlyInPrimaryStorage); + } else { snapInfo = snapshotHelper.backupSnapshotToSecondaryStorageIfNotExists(snapInfo, dataStoreRole, snapshot, kvmSnapshotOnlyInPrimaryStorage); } } catch (CloudRuntimeException e) { @@ -600,7 +605,7 @@ public VolumeInfo createVolumeFromSnapshot(Volume volume, Snapshot snapshot, Use } // don't try to perform a sync if the DataStoreRole of the snapshot is equal to DataStoreRole.Primary - if (!DataStoreRole.Primary.equals(dataStoreRole) || !storageSupportSnapshotToTemplateEnabled) { + if (!DataStoreRole.Primary.equals(dataStoreRole) || (kvmSnapshotOnlyInPrimaryStorage && !storageSupportSnapshotToTemplateEnabled)) { try { // sync snapshot to region store if necessary DataStore snapStore = snapInfo.getDataStore(); diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index 51d9249fad79..925acf98db77 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -1698,14 +1698,14 @@ public VirtualMachineTemplate createPrivateTemplate(CreateTemplateCmd command) t DataStoreRole dataStoreRole = snapshotHelper.getDataStoreRole(snapshot); kvmSnapshotOnlyInPrimaryStorage = snapshotHelper.isKvmSnapshotOnlyInPrimaryStorage(snapshot, dataStoreRole); snapInfo = _snapshotFactory.getSnapshotWithRoleAndZone(snapshotId, dataStoreRole, zoneId); - logger.debug("Creating template for snapshot, with dataStore role {} and on primary storage: {}", dataStoreRole, kvmSnapshotOnlyInPrimaryStorage); + logger.debug("Creating template from snapshot, with dataStore role {} and on primary storage: {}", dataStoreRole, kvmSnapshotOnlyInPrimaryStorage); boolean storageSupportsSnapshotToTemplate = snapshotHelper.isStorageSupportSnapshotToTemplate(snapInfo); if (storageSupportsSnapshotToTemplate) { dataStoreRole = snapshotHelper.getDataStoreRole(snapshot, zoneId); kvmSnapshotOnlyInPrimaryStorage = snapshotHelper.isKvmSnapshotOnlyInPrimaryStorage(snapshot, dataStoreRole, zoneId); snapInfo = _snapshotFactory.getSnapshotWithRoleAndZone(snapshotId, dataStoreRole, zoneId); - logger.debug("Creating template for snapshot for storage supporting snapshot to template, with dataStore role {} and on primary storage: {}", dataStoreRole, kvmSnapshotOnlyInPrimaryStorage); + logger.debug("Creating template from snapshot for storage supporting snapshot to template, with dataStore role {} and on primary storage: {}", dataStoreRole, kvmSnapshotOnlyInPrimaryStorage); ImageStoreVO imageStore = _imgStoreDao.findOneByZoneAndProtocol(zoneId, "nfs"); if (imageStore == null) {