From 762edb01ef9e9ef32c8034a4fbbaad8f1cb49b50 Mon Sep 17 00:00:00 2001 From: Slavka Peleva Date: Wed, 20 Aug 2025 16:22:52 +0300 Subject: [PATCH 1/3] 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. --- .../orchestration/VolumeOrchestrator.java | 4 +-- .../cloud/template/TemplateManagerImpl.java | 26 ++++++++++--------- .../cloudstack/snapshot/SnapshotHelper.java | 2 +- 3 files changed, 17 insertions(+), 15 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 2668d44f0958..2d641cded8b0 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,7 +579,7 @@ 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); + DataStoreRole dataStoreRole = snapshotHelper.getDataStoreRole(snapshot, zoneId); 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); @@ -587,11 +587,11 @@ public VolumeInfo createVolumeFromSnapshot(Volume volume, Snapshot snapshot, Use boolean storageSupportSnapshotToTemplateEnabled = snapshotHelper.isStorageSupportSnapshotToTemplate(snapInfo); 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); 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 { + dataStoreRole = snapshotHelper.getDataStoreRole(snapshot); 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 925acf98db77..5518a29955be 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 VirtualMachineTemplate createPrivateTemplate(CreateTemplateCmd command) t 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 VirtualMachineTemplate createPrivateTemplate(CreateTemplateCmd command) t 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(); - - if (snapStore != null) { - store = snapStore; // pick snapshot image store to create template + } 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 + } } } 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 b88f8ddb731e..98efe7d1c988 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 void expungeTemporarySnapshot(boolean kvmSnapshotOnlyInPrimaryStorage, Sn } 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())); } From 1202e35077216ffe3608e6bb042134365c2bd5eb Mon Sep 17 00:00:00 2001 From: Slavka Peleva Date: Thu, 21 Aug 2025 14:27:31 +0300 Subject: [PATCH 2/3] update snapshot info depending on the data store role --- .../engine/orchestration/VolumeOrchestrator.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 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 2d641cded8b0..a583ad2986f3 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 @@ -581,17 +581,16 @@ public VolumeInfo createVolumeFromSnapshot(Volume volume, Snapshot snapshot, Use DataStore store = dataStoreMgr.getDataStore(pool.getId(), DataStoreRole.Primary); 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); 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] - 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 { + 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); + snapInfo = snapshotFactory.getSnapshotWithRoleAndZone(snapshot.getId(), dataStoreRole, zoneId); + 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) { From e01a03d69eaa21fc39c386e8dee67ba5612643ce Mon Sep 17 00:00:00 2001 From: Slavka Peleva Date: Thu, 21 Aug 2025 19:39:45 +0300 Subject: [PATCH 3/3] addressed reviews --- .../cloudstack/engine/orchestration/VolumeOrchestrator.java | 4 ++-- 1 file changed, 2 insertions(+), 2 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 a583ad2986f3..ba50d5ff9fac 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 @@ -584,9 +584,9 @@ public VolumeInfo createVolumeFromSnapshot(Volume volume, Snapshot snapshot, Use 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] + if (!storageSupportSnapshotToTemplateEnabled) { dataStoreRole = snapshotHelper.getDataStoreRole(snapshot); snapInfo = snapshotFactory.getSnapshotWithRoleAndZone(snapshot.getId(), dataStoreRole, zoneId); kvmSnapshotOnlyInPrimaryStorage = snapshotHelper.isKvmSnapshotOnlyInPrimaryStorage(snapshot, dataStoreRole);