From 1272b130878d0d8eb73caad2963343082d4be3fc Mon Sep 17 00:00:00 2001 From: slavkap <51903378+slavkap@users.noreply.github.com> Date: Thu, 21 Aug 2025 22:23:55 +0300 Subject: [PATCH] Fix of create a template from a StorPool snapshot on another zone (#11490) * Fix of create template from snapshot on another zone When a snapshot has a copy on StorPool primary storage in another zone, but the original snapshot resides on secondary storage, creating a template from the copied snapshot results in the template being created in the first zone. If the snapshot.backup.to.secondary setting is disabled, and a user creates a volume or template from a snapshot, the snapshot is temporarily backed up to secondary storage during the operation. After the operation, this backup should be deleted. However, the snapshot currently remains on both primary and secondary storage. * update snapshot info depending on the data store role --- .../orchestration/VolumeOrchestrator.java | 15 ++++++------ .../cloud/template/TemplateManagerImpl.java | 24 ++++++++++--------- .../cloudstack/snapshot/SnapshotHelper.java | 2 +- 3 files changed, 21 insertions(+), 20 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 2668d44f095..ba50d5ff9fa 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,19 +579,18 @@ public class VolumeOrchestrator extends ManagerBase implements VolumeOrchestrati VolumeInfo vol = volFactory.getVolume(volume.getId()); long zoneId = volume.getDataCenterId(); DataStore store = dataStoreMgr.getDataStore(pool.getId(), DataStoreRole.Primary); - DataStoreRole dataStoreRole = snapshotHelper.getDataStoreRole(snapshot); + DataStoreRole dataStoreRole = snapshotHelper.getDataStoreRole(snapshot, zoneId); SnapshotInfo snapInfo = snapshotFactory.getSnapshotWithRoleAndZone(snapshot.getId(), dataStoreRole, zoneId); - boolean kvmSnapshotOnlyInPrimaryStorage = snapshotHelper.isKvmSnapshotOnlyInPrimaryStorage(snapshot, dataStoreRole); + boolean kvmSnapshotOnlyInPrimaryStorage = snapshotHelper.isKvmSnapshotOnlyInPrimaryStorage(snapshot, dataStoreRole, zoneId); logger.debug("Creating volume from snapshot, with dataStore role {} and on primary storage: {}", dataStoreRole, kvmSnapshotOnlyInPrimaryStorage); - boolean storageSupportSnapshotToTemplateEnabled = snapshotHelper.isStorageSupportSnapshotToTemplate(snapInfo); + boolean storageSupportSnapshotToTemplateEnabled = snapshotHelper.isStorageSupportSnapshotToTemplate(snapInfo); // storageSupportSnapshotToTemplateEnabled is 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] try { - 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); + if (!storageSupportSnapshotToTemplateEnabled) { + dataStoreRole = snapshotHelper.getDataStoreRole(snapshot); 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 { + kvmSnapshotOnlyInPrimaryStorage = snapshotHelper.isKvmSnapshotOnlyInPrimaryStorage(snapshot, dataStoreRole); + logger.debug("Creating volume from snapshot, with dataStore role {} and on primary storage: {}", dataStoreRole, kvmSnapshotOnlyInPrimaryStorage); snapInfo = snapshotHelper.backupSnapshotToSecondaryStorageIfNotExists(snapInfo, dataStoreRole, snapshot, kvmSnapshotOnlyInPrimaryStorage); } } catch (CloudRuntimeException e) { diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index 925acf98db7..5518a29955b 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -1695,16 +1695,12 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, AsyncCallFuture future; if (snapshotId != null) { - DataStoreRole dataStoreRole = snapshotHelper.getDataStoreRole(snapshot); - kvmSnapshotOnlyInPrimaryStorage = snapshotHelper.isKvmSnapshotOnlyInPrimaryStorage(snapshot, dataStoreRole); + DataStoreRole dataStoreRole = snapshotHelper.getDataStoreRole(snapshot, zoneId); snapInfo = _snapshotFactory.getSnapshotWithRoleAndZone(snapshotId, dataStoreRole, zoneId); - 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 from snapshot for storage supporting snapshot to template, with dataStore role {} and on primary storage: {}", dataStoreRole, kvmSnapshotOnlyInPrimaryStorage); ImageStoreVO imageStore = _imgStoreDao.findOneByZoneAndProtocol(zoneId, "nfs"); @@ -1716,13 +1712,19 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, if (dataStore != null) { store = dataStore; } - } else if (dataStoreRole == DataStoreRole.Image || kvmSnapshotOnlyInPrimaryStorage) { - snapInfo = snapshotHelper.backupSnapshotToSecondaryStorageIfNotExists(snapInfo, dataStoreRole, snapshot, kvmSnapshotOnlyInPrimaryStorage); - _accountMgr.checkAccess(caller, null, true, snapInfo); - DataStore snapStore = snapInfo.getDataStore(); + } else { + dataStoreRole = snapshotHelper.getDataStoreRole(snapshot); + kvmSnapshotOnlyInPrimaryStorage = snapshotHelper.isKvmSnapshotOnlyInPrimaryStorage(snapshot, dataStoreRole); + snapInfo = _snapshotFactory.getSnapshotWithRoleAndZone(snapshotId, dataStoreRole, zoneId); + logger.debug("Creating template from snapshot, with dataStore role {} and on primary storage: {}", dataStoreRole, kvmSnapshotOnlyInPrimaryStorage); + if (dataStoreRole == DataStoreRole.Image || kvmSnapshotOnlyInPrimaryStorage) { + snapInfo = snapshotHelper.backupSnapshotToSecondaryStorageIfNotExists(snapInfo, dataStoreRole, snapshot, kvmSnapshotOnlyInPrimaryStorage); + _accountMgr.checkAccess(caller, null, true, snapInfo); + DataStore snapStore = snapInfo.getDataStore(); - if (snapStore != null) { - store = snapStore; // pick snapshot image store to create template + if (snapStore != null) { + store = snapStore; // pick snapshot image store to create template + } } } 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 b88f8ddb731..98efe7d1c98 100644 --- a/server/src/main/java/org/apache/cloudstack/snapshot/SnapshotHelper.java +++ b/server/src/main/java/org/apache/cloudstack/snapshot/SnapshotHelper.java @@ -113,7 +113,7 @@ public class SnapshotHelper { } List snapshots = snapshotJoinDao.listBySnapshotIdAndZoneId(zoneId, snapInfo.getSnapshotId()); - if (kvmSnapshotOnlyInPrimaryStorage || snapshots.size() <= 1) { + if (snapshots.size() <= 1) { if (snapInfo != null) { logger.trace(String.format("Snapshot [{}] is not a temporary backup to create a volume from snapshot. Not expunging it.", snapInfo.getId())); }