Skip to content

Commit 61352d6

Browse files
author
Mike Tutkowski
committed
Return StrategyPriority.CANT_HANDLE for SnapshotOperation.REVERT and only handle SnapshotOperation.DELETE for volume snapshots on primary storage. SnapshotOperation.REVERT is then handled by another snapshot strategy.
1 parent a179364 commit 61352d6

2 files changed

Lines changed: 75 additions & 83 deletions

File tree

‎engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java‎

Lines changed: 16 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,10 @@
3838
import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
3939
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
4040
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeService;
41-
import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine.Event;
4241
import org.apache.cloudstack.storage.command.SnapshotAndCopyAnswer;
4342
import org.apache.cloudstack.storage.command.SnapshotAndCopyCommand;
4443
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
4544
import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
46-
import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO;
4745
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
4846

4947
import com.cloud.agent.AgentManager;
@@ -62,8 +60,6 @@
6260
import com.cloud.storage.Snapshot;
6361
import com.cloud.storage.SnapshotVO;
6462
import com.cloud.storage.Storage.ImageFormat;
65-
import com.cloud.storage.StoragePool;
66-
import com.cloud.storage.StoragePoolStatus;
6763
import com.cloud.storage.Volume;
6864
import com.cloud.storage.VolumeVO;
6965
import com.cloud.storage.dao.SnapshotDao;
@@ -163,52 +159,7 @@ public boolean deleteSnapshot(Long snapshotId) {
163159

164160
@Override
165161
public boolean revertSnapshot(SnapshotInfo snapshot) {
166-
if (canHandle(snapshot, SnapshotOperation.REVERT) == StrategyPriority.CANT_HANDLE) {
167-
throw new UnsupportedOperationException("Reverting not supported. Create a template or volume based on the snapshot instead.");
168-
}
169-
170-
SnapshotVO snapshotVO = _snapshotDao.acquireInLockTable(snapshot.getId());
171-
172-
if (snapshotVO == null) {
173-
throw new CloudRuntimeException("Failed to get lock on snapshot:" + snapshot.getId());
174-
}
175-
176-
try {
177-
VolumeInfo volumeInfo = snapshot.getBaseVolume();
178-
StoragePool store = (StoragePool)volumeInfo.getDataStore();
179-
180-
if (store != null && store.getStatus() != StoragePoolStatus.Up) {
181-
snapshot.processEvent(Event.OperationFailed);
182-
183-
throw new CloudRuntimeException("store is not in up state");
184-
}
185-
186-
volumeInfo.stateTransit(Volume.Event.RevertSnapshotRequested);
187-
188-
boolean result = false;
189-
190-
try {
191-
result = snapshotSvr.revertSnapshot(snapshot);
192-
193-
if (!result) {
194-
s_logger.debug("Failed to revert snapshot: " + snapshot.getId());
195-
196-
throw new CloudRuntimeException("Failed to revert snapshot: " + snapshot.getId());
197-
}
198-
} finally {
199-
if (result) {
200-
volumeInfo.stateTransit(Volume.Event.OperationSucceeded);
201-
} else {
202-
volumeInfo.stateTransit(Volume.Event.OperationFailed);
203-
}
204-
}
205-
206-
return result;
207-
} finally {
208-
if (snapshotVO != null) {
209-
_snapshotDao.releaseFromLockTable(snapshot.getId());
210-
}
211-
}
162+
throw new UnsupportedOperationException("Reverting not supported. Create a template or volume based on the snapshot instead.");
212163
}
213164

214165
@Override
@@ -422,7 +373,7 @@ private void performSnapshotAndCopyOnHostSide(VolumeInfo volumeInfo, SnapshotInf
422373
}
423374

424375
private Map<String, String> getSourceDetails(VolumeInfo volumeInfo) {
425-
Map<String, String> sourceDetails = new HashMap<String, String>();
376+
Map<String, String> sourceDetails = new HashMap<>();
426377

427378
VolumeVO volumeVO = _volumeDao.findById(volumeInfo.getId());
428379

@@ -446,7 +397,7 @@ private Map<String, String> getSourceDetails(VolumeInfo volumeInfo) {
446397
}
447398

448399
private Map<String, String> getDestDetails(StoragePoolVO storagePoolVO, SnapshotInfo snapshotInfo) {
449-
Map<String, String> destDetails = new HashMap<String, String>();
400+
Map<String, String> destDetails = new HashMap<>();
450401

451402
destDetails.put(DiskTO.STORAGE_HOST, storagePoolVO.getHostAddress());
452403
destDetails.put(DiskTO.STORAGE_PORT, String.valueOf(storagePoolVO.getPort()));
@@ -517,7 +468,7 @@ private HostVO getHost(long zoneId, Long hostId) {
517468
}
518469

519470
private HostVO getHost(long zoneId, boolean computeClusterMustSupportResign) {
520-
List<? extends Cluster> clusters = _mgr.searchForClusters(zoneId, new Long(0), Long.MAX_VALUE, HypervisorType.XenServer.toString());
471+
List<? extends Cluster> clusters = _mgr.searchForClusters(zoneId, 0L, Long.MAX_VALUE, HypervisorType.XenServer.toString());
521472

522473
if (clusters == null) {
523474
clusters = new ArrayList<>();
@@ -575,44 +526,28 @@ private void markAsBackedUp(SnapshotObject snapshotObj) {
575526

576527
@Override
577528
public StrategyPriority canHandle(Snapshot snapshot, SnapshotOperation op) {
578-
long volumeId = snapshot.getVolumeId();
579-
VolumeVO volumeVO = _volumeDao.findById(volumeId);
580-
581529
if (SnapshotOperation.REVERT.equals(op)) {
582-
if (volumeVO != null && ImageFormat.QCOW2.equals(volumeVO.getFormat())) {
583-
return StrategyPriority.DEFAULT;
584-
}
585-
else {
586-
return StrategyPriority.CANT_HANDLE;
587-
}
530+
return StrategyPriority.CANT_HANDLE;
588531
}
589532

590-
final long storagePoolId;
533+
long volumeId = snapshot.getVolumeId();
591534

592-
if (volumeVO == null) {
593-
SnapshotDataStoreVO snapshotStore = _snapshotStoreDao.findBySnapshot(snapshot.getId(), DataStoreRole.Primary);
535+
VolumeVO volumeVO = _volumeDao.findByIdIncludingRemoved(volumeId);
594536

595-
if (snapshotStore != null) {
596-
storagePoolId = snapshotStore.getDataStoreId();
597-
}
598-
else {
599-
throw new CloudRuntimeException("Unable to determine the storage pool of the snapshot");
600-
}
601-
}
602-
else {
603-
storagePoolId = volumeVO.getPoolId();
604-
}
537+
long storagePoolId = volumeVO.getPoolId();
605538

606539
DataStore dataStore = _dataStoreMgr.getDataStore(storagePoolId, DataStoreRole.Primary);
607540

608-
Map<String, String> mapCapabilities = dataStore.getDriver().getCapabilities();
541+
if (dataStore != null) {
542+
Map<String, String> mapCapabilities = dataStore.getDriver().getCapabilities();
609543

610-
if (mapCapabilities != null) {
611-
String value = mapCapabilities.get(DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT.toString());
612-
Boolean supportsStorageSystemSnapshots = new Boolean(value);
544+
if (mapCapabilities != null) {
545+
String value = mapCapabilities.get(DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT.toString());
546+
Boolean supportsStorageSystemSnapshots = new Boolean(value);
613547

614-
if (supportsStorageSystemSnapshots) {
615-
return StrategyPriority.HIGHEST;
548+
if (supportsStorageSystemSnapshots) {
549+
return StrategyPriority.HIGHEST;
550+
}
616551
}
617552
}
618553

‎engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java‎

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@
4545
import com.cloud.storage.DataStoreRole;
4646
import com.cloud.storage.Snapshot;
4747
import com.cloud.storage.SnapshotVO;
48+
import com.cloud.storage.StoragePool;
49+
import com.cloud.storage.StoragePoolStatus;
50+
import com.cloud.storage.Storage.ImageFormat;
4851
import com.cloud.storage.Volume;
4952
import com.cloud.storage.VolumeVO;
5053
import com.cloud.storage.dao.SnapshotDao;
@@ -73,6 +76,8 @@ public class XenserverSnapshotStrategy extends SnapshotStrategyBase {
7376
VolumeDao volumeDao;
7477
@Inject
7578
SnapshotDataFactory snapshotDataFactory;
79+
@Inject
80+
private SnapshotDao _snapshotDao;
7681

7782
@Override
7883
public SnapshotInfo backupSnapshot(SnapshotInfo snapshot) {
@@ -279,7 +284,52 @@ public boolean deleteSnapshot(Long snapshotId) {
279284

280285
@Override
281286
public boolean revertSnapshot(SnapshotInfo snapshot) {
282-
throw new CloudRuntimeException("revert Snapshot is not supported");
287+
if (canHandle(snapshot,SnapshotOperation.REVERT) == StrategyPriority.CANT_HANDLE) {
288+
throw new UnsupportedOperationException("Reverting not supported. Create a template or volume based on the snapshot instead.");
289+
}
290+
291+
SnapshotVO snapshotVO = _snapshotDao.acquireInLockTable(snapshot.getId());
292+
293+
if (snapshotVO == null) {
294+
throw new CloudRuntimeException("Failed to get lock on snapshot:" + snapshot.getId());
295+
}
296+
297+
try {
298+
VolumeInfo volumeInfo = snapshot.getBaseVolume();
299+
StoragePool store = (StoragePool)volumeInfo.getDataStore();
300+
301+
if (store != null && store.getStatus() != StoragePoolStatus.Up) {
302+
snapshot.processEvent(Event.OperationFailed);
303+
304+
throw new CloudRuntimeException("store is not in up state");
305+
}
306+
307+
volumeInfo.stateTransit(Volume.Event.RevertSnapshotRequested);
308+
309+
boolean result = false;
310+
311+
try {
312+
result = snapshotSvr.revertSnapshot(snapshot);
313+
314+
if (!result) {
315+
s_logger.debug("Failed to revert snapshot: " + snapshot.getId());
316+
317+
throw new CloudRuntimeException("Failed to revert snapshot: " + snapshot.getId());
318+
}
319+
} finally {
320+
if (result) {
321+
volumeInfo.stateTransit(Volume.Event.OperationSucceeded);
322+
} else {
323+
volumeInfo.stateTransit(Volume.Event.OperationFailed);
324+
}
325+
}
326+
327+
return result;
328+
} finally {
329+
if (snapshotVO != null) {
330+
_snapshotDao.releaseFromLockTable(snapshot.getId());
331+
}
332+
}
283333
}
284334

285335
@Override
@@ -353,7 +403,14 @@ public SnapshotInfo takeSnapshot(SnapshotInfo snapshot) {
353403

354404
@Override
355405
public StrategyPriority canHandle(Snapshot snapshot, SnapshotOperation op) {
356-
if (op == SnapshotOperation.REVERT) {
406+
if (SnapshotOperation.REVERT.equals(op)) {
407+
long volumeId = snapshot.getVolumeId();
408+
VolumeVO volumeVO = volumeDao.findById(volumeId);
409+
410+
if (volumeVO != null && ImageFormat.QCOW2.equals(volumeVO.getFormat())) {
411+
return StrategyPriority.DEFAULT;
412+
}
413+
357414
return StrategyPriority.CANT_HANDLE;
358415
}
359416

0 commit comments

Comments
 (0)