You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ga...@apache.org on 2020/03/16 13:18:00 UTC
[cloudstack] 01/06: Fixes snapshot deletion
This is an automated email from the ASF dual-hosted git repository.
gabriel pushed a commit to branch snapshot-deletion-issues
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit afd060b54f48744caf4836e4a6334dc1711b2b17
Author: GabrielBrascher <ga...@apache.org>
AuthorDate: Thu Oct 17 17:16:48 2019 -0300
Fixes snapshot deletion
---
.../src/test/resources/fakeDriverTestContext.xml | 2 +-
.../src/test/resources/storageContext.xml | 2 +-
...tStrategy.java => DefaultSnapshotStrategy.java} | 114 +++++++++++----------
...ing-engine-storage-snapshot-storage-context.xml | 4 +-
4 files changed, 66 insertions(+), 56 deletions(-)
diff --git a/engine/storage/integration-test/src/test/resources/fakeDriverTestContext.xml b/engine/storage/integration-test/src/test/resources/fakeDriverTestContext.xml
index 944196d..b8a2274 100644
--- a/engine/storage/integration-test/src/test/resources/fakeDriverTestContext.xml
+++ b/engine/storage/integration-test/src/test/resources/fakeDriverTestContext.xml
@@ -49,7 +49,7 @@
<bean id="dataStoreProviderManagerImpl" class="org.apache.cloudstack.storage.datastore.provider.DataStoreProviderManagerImpl" />
<bean id="storageCacheManagerImpl" class="org.apache.cloudstack.storage.cache.manager.StorageCacheManagerImpl" />
<bean id="storageCacheRandomAllocator" class="org.apache.cloudstack.storage.cache.allocator.StorageCacheRandomAllocator" />
- <bean id="xenserverSnapshotStrategy" class="org.apache.cloudstack.storage.snapshot.XenserverSnapshotStrategy" />
+ <bean id="defaultSnapshotStrategy" class="org.apache.cloudstack.storage.snapshot.DefaultSnapshotStrategy" />
<bean id="bAREMETAL" class="org.apache.cloudstack.storage.image.format.BAREMETAL" />
<bean id="dataMotionServiceImpl" class="org.apache.cloudstack.storage.motion.DataMotionServiceImpl" />
<bean id="dataObjectManagerImpl" class="org.apache.cloudstack.storage.datastore.DataObjectManagerImpl" />
diff --git a/engine/storage/integration-test/src/test/resources/storageContext.xml b/engine/storage/integration-test/src/test/resources/storageContext.xml
index abf0876..f65e2ac 100644
--- a/engine/storage/integration-test/src/test/resources/storageContext.xml
+++ b/engine/storage/integration-test/src/test/resources/storageContext.xml
@@ -50,7 +50,7 @@
<bean id="ancientDataMotionStrategy" class="org.apache.cloudstack.storage.motion.AncientDataMotionStrategy" />
<bean id="storageCacheManagerImpl" class="org.apache.cloudstack.storage.cache.manager.StorageCacheManagerImpl" />
<bean id="storageCacheRandomAllocator" class="org.apache.cloudstack.storage.cache.allocator.StorageCacheRandomAllocator" />
- <bean id="xenserverSnapshotStrategy" class="org.apache.cloudstack.storage.snapshot.XenserverSnapshotStrategy" />
+ <bean id="defaultSnapshotStrategy" class="org.apache.cloudstack.storage.snapshot.DefaultSnapshotStrategy" />
<bean id="bAREMETAL" class="org.apache.cloudstack.storage.image.format.BAREMETAL" />
<bean id="dataMotionServiceImpl" class="org.apache.cloudstack.storage.motion.DataMotionServiceImpl" />
<bean id="dataObjectManagerImpl" class="org.apache.cloudstack.storage.datastore.DataObjectManagerImpl" />
diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java
similarity index 84%
rename from engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java
rename to engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java
index 3ab2129..2651162 100644
--- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java
+++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java
@@ -34,7 +34,6 @@ import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
import org.apache.cloudstack.framework.jobs.AsyncJob;
-import org.apache.cloudstack.framework.jobs.dao.SyncQueueItemDao;
import org.apache.cloudstack.storage.command.CreateObjectAnswer;
import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO;
@@ -72,8 +71,8 @@ import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.utils.fsm.NoTransitionException;
@Component
-public class XenserverSnapshotStrategy extends SnapshotStrategyBase {
- private static final Logger s_logger = Logger.getLogger(XenserverSnapshotStrategy.class);
+public class DefaultSnapshotStrategy extends SnapshotStrategyBase {
+ private static final Logger s_logger = Logger.getLogger(DefaultSnapshotStrategy.class);
@Inject
SnapshotService snapshotSvr;
@@ -90,12 +89,8 @@ public class XenserverSnapshotStrategy extends SnapshotStrategyBase {
@Inject
SnapshotDataFactory snapshotDataFactory;
@Inject
- private SnapshotDao _snapshotDao;
- @Inject
private SnapshotDetailsDao _snapshotDetailsDao;
@Inject
- private SyncQueueItemDao _syncQueueItemDao;
- @Inject
VolumeDetailsDao _volumeDetailsDaoImpl;
@Override
@@ -269,63 +264,78 @@ public class XenserverSnapshotStrategy extends SnapshotStrategyBase {
throw new InvalidParameterValueException("Can't delete snapshotshot " + snapshotId + " due to it is in " + snapshotVO.getState() + " Status");
}
- // first mark the snapshot as destroyed, so that ui can't see it, but we
- // may not destroy the snapshot on the storage, as other snapshots may
- // depend on it.
SnapshotInfo snapshotOnImage = snapshotDataFactory.getSnapshot(snapshotId, DataStoreRole.Image);
- if (snapshotOnImage == null) {
- s_logger.debug("Can't find snapshot on backup storage, delete it in db");
- snapshotDao.remove(snapshotId);
- return true;
- }
-
- SnapshotObject obj = (SnapshotObject)snapshotOnImage;
- try {
- obj.processEvent(Snapshot.Event.DestroyRequested);
- List<VolumeDetailVO> volumesFromSnapshot;
- volumesFromSnapshot = _volumeDetailsDaoImpl.findDetails("SNAPSHOT_ID", String.valueOf(snapshotId), null);
- if (volumesFromSnapshot.size() > 0) {
- try {
- obj.processEvent(Snapshot.Event.OperationFailed);
- } catch (NoTransitionException e1) {
- s_logger.debug("Failed to change snapshot state: " + e1.toString());
+ boolean deletedOnSecondary = false;
+ if (snapshotOnImage == null) {
+ s_logger.debug(String.format("Can't find snapshot [snapshot id: %d] on backup storage"));
+ } else {
+ SnapshotObject obj = (SnapshotObject)snapshotOnImage;
+ try {
+ deletedOnSecondary = deleteSnapshotOnSecundaryStorage(snapshotId, snapshotOnImage, obj);
+ if (!deletedOnSecondary) {
+ s_logger.debug(
+ String.format("Failed to find/delete snapshot (id: %d) on secondary storage. Still necessary to check and delete snapshot on primary storage.",
+ snapshotId));
+ } else {
+ s_logger.debug(String.format("Snapshot (id: %d) has been deleted on secondary storage.", snapshotId));
}
- throw new InvalidParameterValueException("Unable to perform delete operation, Snapshot with id: " + snapshotId + " is in use ");
+ } catch (NoTransitionException e) {
+ s_logger.debug("Failed to set the state to destroying: ", e);
+ return false;
}
- } catch (NoTransitionException e) {
- s_logger.debug("Failed to set the state to destroying: ", e);
- return false;
}
- try {
- boolean result = deleteSnapshotChain(snapshotOnImage);
- obj.processEvent(Snapshot.Event.OperationSucceeded);
- if (result) {
- //snapshot is deleted on backup storage, need to delete it on primary storage
- SnapshotDataStoreVO snapshotOnPrimary = snapshotStoreDao.findBySnapshot(snapshotId, DataStoreRole.Primary);
- if (snapshotOnPrimary != null) {
- SnapshotInfo snapshotOnPrimaryInfo = snapshotDataFactory.getSnapshot(snapshotId, DataStoreRole.Primary);
- long volumeId = snapshotOnPrimary.getVolumeId();
- VolumeVO volumeVO = volumeDao.findById(volumeId);
- if (((PrimaryDataStoreImpl)snapshotOnPrimaryInfo.getDataStore()).getPoolType() == StoragePoolType.RBD && volumeVO != null) {
- snapshotSvr.deleteSnapshot(snapshotOnPrimaryInfo);
- }
- snapshotOnPrimary.setState(State.Destroyed);
- snapshotStoreDao.update(snapshotOnPrimary.getId(), snapshotOnPrimary);
- }
- }
- } catch (Exception e) {
- s_logger.debug("Failed to delete snapshot: ", e);
+ boolean deletedOnPrimary = deleteSnapshotOnPrimary(snapshotId);
+ if (deletedOnPrimary) {
+ s_logger.debug(String.format("Successfully deleted snapshot (id: %d) on primary storage.", snapshotId));
+ } else if (deletedOnSecondary) {
+ s_logger.debug(String.format("The snapshot was deleted on secondary storage. Could not find/delete snapshot (id: %d) on primary storage.", snapshotId));
+ }
+ return deletedOnSecondary || deletedOnPrimary;
+ }
+
+ /**
+ * Deletes the snapshot on secondary storage.
+ * It can return false when the snapshot was stored on primary storage and not backed up on secondary; therefore, the snapshot should also be deleted on primary storage even when this method returns false.
+ */
+ private boolean deleteSnapshotOnSecundaryStorage(Long snapshotId, SnapshotInfo snapshotOnImage, SnapshotObject obj) throws NoTransitionException {
+ obj.processEvent(Snapshot.Event.DestroyRequested);
+ List<VolumeDetailVO> volumesFromSnapshot;
+ volumesFromSnapshot = _volumeDetailsDaoImpl.findDetails("SNAPSHOT_ID", String.valueOf(snapshotId), null);
+
+ if (volumesFromSnapshot.size() > 0) {
try {
obj.processEvent(Snapshot.Event.OperationFailed);
} catch (NoTransitionException e1) {
- s_logger.debug("Failed to change snapshot state: " + e.toString());
+ s_logger.debug("Failed to change snapshot state: " + e1.toString());
}
- return false;
+ throw new InvalidParameterValueException("Unable to perform delete operation, Snapshot with id: " + snapshotId + " is in use ");
}
- return true;
+ boolean result = deleteSnapshotChain(snapshotOnImage);
+ obj.processEvent(Snapshot.Event.OperationSucceeded);
+ return result;
+ }
+
+ /**
+ * Deletes the snapshot on primary storage. It can return false when the snapshot was not stored on primary storage; however this does not means that it failed to delete the snapshot. </br>
+ * In case of failure, it will throw one of the following exceptions: CloudRuntimeException, InterruptedException, or ExecutionException. </br>
+ */
+ private boolean deleteSnapshotOnPrimary(Long snapshotId) {
+ boolean result = false;
+ SnapshotDataStoreVO snapshotOnPrimary = snapshotStoreDao.findBySnapshot(snapshotId, DataStoreRole.Primary);
+ if (snapshotOnPrimary != null) {
+ SnapshotInfo snapshotOnPrimaryInfo = snapshotDataFactory.getSnapshot(snapshotId, DataStoreRole.Primary);
+ long volumeId = snapshotOnPrimary.getVolumeId();
+ VolumeVO volumeVO = volumeDao.findById(volumeId);
+ if (((PrimaryDataStoreImpl)snapshotOnPrimaryInfo.getDataStore()).getPoolType() == StoragePoolType.RBD && volumeVO != null) {
+ result = snapshotSvr.deleteSnapshot(snapshotOnPrimaryInfo);
+ }
+ snapshotOnPrimary.setState(State.Destroyed);
+ snapshotStoreDao.update(snapshotOnPrimary.getId(), snapshotOnPrimary);
+ }
+ return result;
}
@Override
diff --git a/engine/storage/snapshot/src/main/resources/META-INF/cloudstack/storage/spring-engine-storage-snapshot-storage-context.xml b/engine/storage/snapshot/src/main/resources/META-INF/cloudstack/storage/spring-engine-storage-snapshot-storage-context.xml
index b295398..2bfb3c3 100644
--- a/engine/storage/snapshot/src/main/resources/META-INF/cloudstack/storage/spring-engine-storage-snapshot-storage-context.xml
+++ b/engine/storage/snapshot/src/main/resources/META-INF/cloudstack/storage/spring-engine-storage-snapshot-storage-context.xml
@@ -27,8 +27,8 @@
http://www.springframework.org/schema/context/spring-context.xsd"
>
- <bean id="xenserverSnapshotStrategy"
- class="org.apache.cloudstack.storage.snapshot.XenserverSnapshotStrategy" />
+ <bean id="defaultSnapshotStrategy"
+ class="org.apache.cloudstack.storage.snapshot.DefaultSnapshotStrategy" />
<bean id="storageSystemSnapshotStrategy"
class="org.apache.cloudstack.storage.snapshot.StorageSystemSnapshotStrategy" />