mirror of
https://github.com/apache/cloudstack.git
synced 2025-11-02 20:02:29 +01:00
Merge pull request #1441 from mike-tutkowski/4.7
CLOUDSTACK-9297 - Reworked logic in StorageSystemSnapshotStrategy and XenserverSnapshotStrategyThe ticket this PR fixes was opened because KVM-specific code had been added to the StorageSystemSnapshotStrategy class and that class' canHandle method was only prepared to handle managed storage being used with XenServer (and a case was hit for KVM that triggered a CloudRuntimeException to be thrown). To solve the problem, I moved the KVM logic to the default snapshot strategy class, which is (unfortunately) named XenserverSnapshotStrategy. I plan to rename XenserverSnapshotStrategy to something like DefaultSnapshotStrategy in 4.9. My guess is that when XenserverSnapshotStrategy was originally written, it was written only for XenServer, but has since that time had its usage increased to support other hypervisors (with non-managed storage). * pr/1441: CLOUDSTACK-9297: delete snapshot without id is failing with Unable to determine the storage pool of the snapshot Signed-off-by: Will Stevens <williamstevens@gmail.com>
This commit is contained in:
commit
f33b4a1b68
@ -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<String, String> mapCapabilities = dataStore.getDriver().getCapabilities();
|
||||
if (dataStore != null) {
|
||||
Map<String, String> 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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@ -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;
|
||||
}
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user