From 158d196c056b53002b417c195bd96fcc343c382d Mon Sep 17 00:00:00 2001 From: Mike Tutkowski Date: Tue, 15 Mar 2016 22:56:51 -0600 Subject: [PATCH] CLOUDSTACK-9297: delete snapshot without id is failing with Unable to determine the storage pool of the snapshot --- .../StorageSystemSnapshotStrategy.java | 79 +++---------------- .../snapshot/XenserverSnapshotStrategy.java | 59 +++++++++++++- 2 files changed, 70 insertions(+), 68 deletions(-) diff --git a/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java b/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java index d1470e43596..c6960eb4a1e 100644 --- a/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java +++ b/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java @@ -35,12 +35,10 @@ import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotResult; import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeService; -import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine.Event; import org.apache.cloudstack.storage.command.SnapshotAndCopyAnswer; import org.apache.cloudstack.storage.command.SnapshotAndCopyCommand; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao; -import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import com.cloud.agent.AgentManager; @@ -57,8 +55,6 @@ import com.cloud.storage.DataStoreRole; import com.cloud.storage.Snapshot; import com.cloud.storage.SnapshotVO; import com.cloud.storage.Storage.ImageFormat; -import com.cloud.storage.StoragePool; -import com.cloud.storage.StoragePoolStatus; import com.cloud.storage.Volume; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.SnapshotDao; @@ -157,43 +153,7 @@ public class StorageSystemSnapshotStrategy extends SnapshotStrategyBase { @Override public boolean revertSnapshot(SnapshotInfo snapshot) { - if (canHandle(snapshot,SnapshotOperation.REVERT) == StrategyPriority.CANT_HANDLE) { - throw new UnsupportedOperationException("Reverting not supported. Create a template or volume based on the snapshot instead."); - } - - SnapshotVO snapshotVO = _snapshotDao.acquireInLockTable(snapshot.getId()); - if (snapshotVO == null) { - throw new CloudRuntimeException("Failed to get lock on snapshot:" + snapshot.getId()); - } - - try { - VolumeInfo volumeInfo = snapshot.getBaseVolume(); - StoragePool store = (StoragePool)volumeInfo.getDataStore(); - if (store != null && store.getStatus() != StoragePoolStatus.Up) { - snapshot.processEvent(Event.OperationFailed); - throw new CloudRuntimeException("store is not in up state"); - } - volumeInfo.stateTransit(Volume.Event.RevertSnapshotRequested); - boolean result = false; - try { - result = snapshotSvr.revertSnapshot(snapshot); - if (! result) { - s_logger.debug("Failed to revert snapshot: " + snapshot.getId()); - throw new CloudRuntimeException("Failed to revert snapshot: " + snapshot.getId()); - } - } finally { - if (result) { - volumeInfo.stateTransit(Volume.Event.OperationSucceeded); - } else { - volumeInfo.stateTransit(Volume.Event.OperationFailed); - } - } - return result; - } finally { - if (snapshotVO != null) { - _snapshotDao.releaseFromLockTable(snapshot.getId()); - } - } + throw new UnsupportedOperationException("Reverting not supported. Create a template or volume based on the snapshot instead."); } @Override @@ -440,41 +400,28 @@ public class StorageSystemSnapshotStrategy extends SnapshotStrategyBase { @Override public StrategyPriority canHandle(Snapshot snapshot, SnapshotOperation op) { - long volumeId = snapshot.getVolumeId(); - VolumeVO volumeVO = _volumeDao.findById(volumeId); if (SnapshotOperation.REVERT.equals(op)) { - if (volumeVO != null && ImageFormat.QCOW2.equals(volumeVO.getFormat())) - return StrategyPriority.DEFAULT; - else - return StrategyPriority.CANT_HANDLE; + return StrategyPriority.CANT_HANDLE; } - long storagePoolId; + long volumeId = snapshot.getVolumeId(); - if (volumeVO == null) { - SnapshotDataStoreVO snapshotStore = _snapshotStoreDao.findBySnapshot(snapshot.getId(), DataStoreRole.Primary); + VolumeVO volumeVO = _volumeDao.findByIdIncludingRemoved(volumeId); - if (snapshotStore != null) { - storagePoolId = snapshotStore.getDataStoreId(); - } - else { - throw new CloudRuntimeException("Unable to determine the storage pool of the snapshot"); - } - } - else { - storagePoolId = volumeVO.getPoolId(); - } + long storagePoolId = volumeVO.getPoolId(); DataStore dataStore = _dataStoreMgr.getDataStore(storagePoolId, DataStoreRole.Primary); - Map mapCapabilities = dataStore.getDriver().getCapabilities(); + if (dataStore != null) { + Map mapCapabilities = dataStore.getDriver().getCapabilities(); - if (mapCapabilities != null) { - String value = mapCapabilities.get(DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT.toString()); - Boolean supportsStorageSystemSnapshots = new Boolean(value); + if (mapCapabilities != null) { + String value = mapCapabilities.get(DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT.toString()); + Boolean supportsStorageSystemSnapshots = new Boolean(value); - if (supportsStorageSystemSnapshots) { - return StrategyPriority.HIGHEST; + if (supportsStorageSystemSnapshots) { + return StrategyPriority.HIGHEST; + } } } diff --git a/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java b/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java index 96ea7bbc0c5..edd194e605b 100644 --- a/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java +++ b/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java @@ -45,11 +45,14 @@ import com.cloud.storage.CreateSnapshotPayload; import com.cloud.storage.DataStoreRole; import com.cloud.storage.Snapshot; import com.cloud.storage.SnapshotVO; +import com.cloud.storage.StoragePool; +import com.cloud.storage.StoragePoolStatus; import com.cloud.storage.Volume; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.SnapshotDao; import com.cloud.storage.dao.VolumeDao; import com.cloud.storage.snapshot.SnapshotManager; +import com.cloud.storage.Storage.ImageFormat; import com.cloud.utils.NumbersUtil; import com.cloud.utils.db.DB; import com.cloud.utils.exception.CloudRuntimeException; @@ -279,7 +282,52 @@ public class XenserverSnapshotStrategy extends SnapshotStrategyBase { @Override public boolean revertSnapshot(SnapshotInfo snapshot) { - throw new CloudRuntimeException("revert Snapshot is not supported"); + if (canHandle(snapshot,SnapshotOperation.REVERT) == StrategyPriority.CANT_HANDLE) { + throw new UnsupportedOperationException("Reverting not supported. Create a template or volume based on the snapshot instead."); + } + + SnapshotVO snapshotVO = snapshotDao.acquireInLockTable(snapshot.getId()); + + if (snapshotVO == null) { + throw new CloudRuntimeException("Failed to get lock on snapshot:" + snapshot.getId()); + } + + try { + VolumeInfo volumeInfo = snapshot.getBaseVolume(); + StoragePool store = (StoragePool)volumeInfo.getDataStore(); + + if (store != null && store.getStatus() != StoragePoolStatus.Up) { + snapshot.processEvent(Event.OperationFailed); + + throw new CloudRuntimeException("store is not in up state"); + } + + volumeInfo.stateTransit(Volume.Event.RevertSnapshotRequested); + + boolean result = false; + + try { + result = snapshotSvr.revertSnapshot(snapshot); + + if (!result) { + s_logger.debug("Failed to revert snapshot: " + snapshot.getId()); + + throw new CloudRuntimeException("Failed to revert snapshot: " + snapshot.getId()); + } + } finally { + if (result) { + volumeInfo.stateTransit(Volume.Event.OperationSucceeded); + } else { + volumeInfo.stateTransit(Volume.Event.OperationFailed); + } + } + + return result; + } finally { + if (snapshotVO != null) { + snapshotDao.releaseFromLockTable(snapshot.getId()); + } + } } @Override @@ -353,7 +401,14 @@ public class XenserverSnapshotStrategy extends SnapshotStrategyBase { @Override public StrategyPriority canHandle(Snapshot snapshot, SnapshotOperation op) { - if (op == SnapshotOperation.REVERT) { + if (SnapshotOperation.REVERT.equals(op)) { + long volumeId = snapshot.getVolumeId(); + VolumeVO volumeVO = volumeDao.findById(volumeId); + + if (volumeVO != null && ImageFormat.QCOW2.equals(volumeVO.getFormat())) { + return StrategyPriority.DEFAULT; + } + return StrategyPriority.CANT_HANDLE; }