apache · ustcweizhou · Aug 24, 2015 · Aug 28, 2015
diff --git a/api/src/com/cloud/storage/Volume.java b/api/src/com/cloud/storage/Volume.java
@@ -39,6 +39,7 @@ enum State {
         Ready("The volume is ready to be used."),
         Migrating("The volume is migrating to other storage pool"),
         Snapshotting("There is a snapshot created on this volume, not backed up to secondary storage yet"),
+        RevertSnapshotting("There is a snapshot created on this volume, the volume is being reverting from snapshot"),
         Resizing("The volume is being resized"),
         Expunging("The volume is being expunging"),
         Expunged("The volume has been expunged"),
@@ -91,6 +92,9 @@ public String getDescription() {
             s_fsm.addTransition(new StateMachine2.Transition<State, Event>(Ready, Event.SnapshotRequested, Snapshotting, null));
             s_fsm.addTransition(new StateMachine2.Transition<State, Event>(Snapshotting, Event.OperationSucceeded, Ready, null));
             s_fsm.addTransition(new StateMachine2.Transition<State, Event>(Snapshotting, Event.OperationFailed, Ready,null));
+            s_fsm.addTransition(new StateMachine2.Transition<State, Event>(Ready, Event.RevertSnapshotRequested, RevertSnapshotting, null));
+            s_fsm.addTransition(new StateMachine2.Transition<State, Event>(RevertSnapshotting, Event.OperationSucceeded, Ready, null));
+            s_fsm.addTransition(new StateMachine2.Transition<State, Event>(RevertSnapshotting, Event.OperationFailed, Ready,null));
             s_fsm.addTransition(new StateMachine2.Transition<State, Event>(Allocated, Event.MigrationCopyRequested, Creating, null));
             s_fsm.addTransition(new StateMachine2.Transition<State, Event>(Creating, Event.MigrationCopyFailed, Allocated, null));
             s_fsm.addTransition(new StateMachine2.Transition<State, Event>(Creating, Event.MigrationCopySucceeded, Ready, Arrays.asList(new StateMachine2.Transition.Impact[]{StateMachine2.Transition.Impact.USAGE})));
@@ -131,6 +135,7 @@ enum Event {
         MigrationCopySucceeded,
         MigrationCopyFailed,
         SnapshotRequested,
+        RevertSnapshotRequested,
         DestroyRequested,
         ExpungingRequested,
         ResizeRequested,

diff --git a/api/src/com/cloud/storage/snapshot/SnapshotApiService.java b/api/src/com/cloud/storage/snapshot/SnapshotApiService.java
@@ -106,7 +106,7 @@ public interface SnapshotApiService {
      */
     Long getHostIdForSnapshotOperation(Volume vol);
 
-    boolean revertSnapshot(Long snapshotId);
+    Snapshot revertSnapshot(Long snapshotId);
 
     SnapshotPolicy updateSnapshotPolicy(UpdateSnapshotPolicyCmd updateSnapshotPolicyCmd);
 }
diff --git a/api/src/org/apache/cloudstack/api/command/user/snapshot/RevertSnapshotCmd.java b/api/src/org/apache/cloudstack/api/command/user/snapshot/RevertSnapshotCmd.java
@@ -29,8 +29,8 @@
 import org.apache.cloudstack.api.Parameter;
 import org.apache.cloudstack.api.ServerApiException;
 import org.apache.cloudstack.api.response.SnapshotResponse;
-import org.apache.cloudstack.api.response.SuccessResponse;
 import org.apache.cloudstack.context.CallContext;
+import org.apache.log4j.Logger;
 
 import com.cloud.event.EventTypes;
 import com.cloud.storage.Snapshot;
@@ -39,17 +39,27 @@
 @APICommand(name = "revertSnapshot", description = "revert a volume snapshot.", responseObject = SnapshotResponse.class, entityType = {Snapshot.class},
         requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
 public class RevertSnapshotCmd extends BaseAsyncCmd {
+    public static final Logger s_logger = Logger.getLogger(RevertSnapshotCmd.class.getName());
     private static final String s_name = "revertsnapshotresponse";
 
+    /////////////////////////////////////////////////////
+    //////////////// API parameters /////////////////////
+    /////////////////////////////////////////////////////
     @ACL(accessType = AccessType.OperateEntry)
     @Parameter(name= ApiConstants.ID, type= BaseCmd.CommandType.UUID, entityType = SnapshotResponse.class,
             required=true, description="The ID of the snapshot")
     private Long id;
 
+    /////////////////////////////////////////////////////
+    /////////////////// Accessors ///////////////////////
+    /////////////////////////////////////////////////////
     public Long getId() {
         return id;
     }
 
+    /////////////////////////////////////////////////////
+    /////////////// API Implementation///////////////////
+    /////////////////////////////////////////////////////
     @Override
     public String getCommandName() {
         return s_name;
@@ -88,9 +98,9 @@ public Long getInstanceId() {
     @Override
     public void execute() {
         CallContext.current().setEventDetails("Snapshot Id: " + getId());
-        boolean result = _snapshotService.revertSnapshot(getId());
-        if (result) {
-            SuccessResponse response = new SuccessResponse(getCommandName());
+        Snapshot snapshot = _snapshotService.revertSnapshot(getId());
+        if (snapshot != null) {
+            SnapshotResponse response = _responseGenerator.createSnapshotResponse(snapshot);
             response.setResponseName(getCommandName());
             setResponseObject(response);
         } else {

diff --git a/core/src/org/apache/cloudstack/storage/command/RevertSnapshotCommand.java b/core/src/org/apache/cloudstack/storage/command/RevertSnapshotCommand.java
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.cloudstack.storage.command;
+
+import org.apache.cloudstack.storage.to.SnapshotObjectTO;
+
+public final class RevertSnapshotCommand extends StorageSubSystemCommand {
+    private SnapshotObjectTO data;
+    private boolean _executeInSequence = false;
+
+    public RevertSnapshotCommand(SnapshotObjectTO data) {
+        super();
+        this.data = data;
+    }
+
+    protected RevertSnapshotCommand() {
+        super();
+    }
+
+    public SnapshotObjectTO getData() {
+        return this.data;
+    }
+
+    @Override
+    public void setExecuteInSequence(final boolean executeInSequence) {
+        _executeInSequence = executeInSequence;
+    }
+
+    @Override
+    public boolean executeInSequence() {
+        return _executeInSequence;
+    }
+}
diff --git a/...ne/api/src/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreDriver.java b/...ne/api/src/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreDriver.java
@@ -48,5 +48,5 @@ public interface PrimaryDataStoreDriver extends DataStoreDriver {
 
     public void takeSnapshot(SnapshotInfo snapshot, AsyncCompletionCallback<CreateCmdResult> callback);
 
-    public void revertSnapshot(SnapshotInfo snapshot, AsyncCompletionCallback<CommandResult> callback);
+    public void revertSnapshot(SnapshotInfo snapshotOnImageStore, SnapshotInfo snapshotOnPrimaryStore, AsyncCompletionCallback<CommandResult> callback);
 }
diff --git a/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotService.java b/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotService.java
@@ -24,7 +24,7 @@ public interface SnapshotService {
 
     boolean deleteSnapshot(SnapshotInfo snapshot);
 
-    boolean revertSnapshot(Long snapshotId);
+    boolean revertSnapshot(SnapshotInfo snapshot);
 
     void syncVolumeSnapshotsToRegionStore(long volumeId, DataStore store);
 

diff --git a/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotStrategy.java b/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotStrategy.java
@@ -29,7 +29,7 @@ enum SnapshotOperation {
 
     boolean deleteSnapshot(Long snapshotId);
 
-    boolean revertSnapshot(Long snapshotId);
+    boolean revertSnapshot(SnapshotInfo snapshot);
 
     StrategyPriority canHandle(Snapshot snapshot, SnapshotOperation op);
 }
diff --git a/engine/schema/src/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDao.java b/engine/schema/src/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDao.java
@@ -60,4 +60,7 @@ public interface SnapshotDataStoreDao extends GenericDao<SnapshotDataStoreVO, Lo
     SnapshotDataStoreVO findOldestSnapshotForVolume(Long volumeId, DataStoreRole role);
 
     void updateVolumeIds(long oldVolId, long newVolId);
+
+    SnapshotDataStoreVO findByVolume(long volumeId, DataStoreRole role);
+
 }
diff --git a/.../storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotDataFactoryImpl.java b/.../storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotDataFactoryImpl.java
@@ -72,7 +72,10 @@ public SnapshotInfo getSnapshot(long snapshotId, DataStoreRole role) {
         SnapshotVO snapshot = snapshotDao.findById(snapshotId);
         SnapshotDataStoreVO snapshotStore = snapshotStoreDao.findBySnapshot(snapshotId, role);
         if (snapshotStore == null) {
-            return null;
+            snapshotStore = snapshotStoreDao.findByVolume(snapshot.getVolumeId(), role);
+            if (snapshotStore == null) {
+                return null;
+            }
         }
         DataStore store = storeMgr.getDataStore(snapshotStore.getDataStoreId(), role);
         SnapshotObject so = SnapshotObject.getSnapshotObject(snapshot, store);

diff --git a/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java b/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java
@@ -407,16 +407,16 @@ public boolean deleteSnapshot(SnapshotInfo snapInfo) {
     }
 
     @Override
-    public boolean revertSnapshot(Long snapshotId) {
-        SnapshotInfo snapshot = _snapshotFactory.getSnapshot(snapshotId, DataStoreRole.Primary);
-        PrimaryDataStore store = (PrimaryDataStore)snapshot.getDataStore();
+    public boolean revertSnapshot(SnapshotInfo snapshot) {
+        SnapshotInfo snapshotOnPrimaryStore = _snapshotFactory.getSnapshot(snapshot.getId(), DataStoreRole.Primary);
+        PrimaryDataStore store = (PrimaryDataStore)snapshotOnPrimaryStore.getDataStore();
 
         AsyncCallFuture<SnapshotResult> future = new AsyncCallFuture<SnapshotResult>();
         RevertSnapshotContext<CommandResult> context = new RevertSnapshotContext<CommandResult>(null, snapshot, future);
         AsyncCallbackDispatcher<SnapshotServiceImpl, CommandResult> caller = AsyncCallbackDispatcher.create(this);
         caller.setCallback(caller.getTarget().revertSnapshotCallback(null, null)).setContext(context);
 
-        ((PrimaryDataStoreDriver)store.getDriver()).revertSnapshot(snapshot, caller);
+        ((PrimaryDataStoreDriver)store.getDriver()).revertSnapshot(snapshot, snapshotOnPrimaryStore, caller);
 
         SnapshotResult result = null;
         try {

diff --git a/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotStrategyBase.java b/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotStrategyBase.java
@@ -37,7 +37,7 @@ public SnapshotInfo backupSnapshot(SnapshotInfo snapshot) {
     }
 
     @Override
-    public boolean revertSnapshot(Long snapshotId) {
-        return snapshotSvr.revertSnapshot(snapshotId);
+    public boolean revertSnapshot(SnapshotInfo snapshot) {
+        return snapshotSvr.revertSnapshot(snapshot);
     }
 }
diff --git a/...ge/snapshot/src/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java b/...ge/snapshot/src/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java
@@ -35,6 +35,7 @@
 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;
@@ -56,6 +57,8 @@
 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;
@@ -153,8 +156,44 @@ public boolean deleteSnapshot(Long snapshotId) {
     }
 
     @Override
-    public boolean revertSnapshot(Long snapshotId) {
-        throw new UnsupportedOperationException("Reverting not supported. Create a template or volume based on the snapshot instead.");
+    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());
+            }
+        }
     }
 
     @Override
@@ -401,10 +440,6 @@ private void markAsBackedUp(SnapshotObject snapshotObj) {
 
     @Override
     public StrategyPriority canHandle(Snapshot snapshot, SnapshotOperation op) {
-        if (SnapshotOperation.REVERT.equals(op)) {
-            return StrategyPriority.CANT_HANDLE;
-        }
-
         long volumeId = snapshot.getVolumeId();
         VolumeVO volumeVO = _volumeDao.findById(volumeId);
 
@@ -424,6 +459,13 @@ public StrategyPriority canHandle(Snapshot snapshot, SnapshotOperation op) {
             storagePoolId = volumeVO.getPoolId();
         }
 
+        if (SnapshotOperation.REVERT.equals(op)) {
+            if (volumeVO != null && ImageFormat.QCOW2.equals(volumeVO.getFormat()))
+                return StrategyPriority.DEFAULT;
+            else
+                return StrategyPriority.CANT_HANDLE;
+        }
+
         DataStore dataStore = _dataStoreMgr.getDataStore(storagePoolId, DataStoreRole.Primary);
 
         Map<String, String> mapCapabilities = dataStore.getDriver().getCapabilities();

diff --git a/...torage/snapshot/src/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java b/...torage/snapshot/src/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java
@@ -278,7 +278,7 @@ public boolean deleteSnapshot(Long snapshotId) {
     }
 
     @Override
-    public boolean revertSnapshot(Long snapshotId) {
+    public boolean revertSnapshot(SnapshotInfo snapshot) {
         throw new CloudRuntimeException("revert Snapshot is not supported");
     }
 

diff --git a/engine/storage/src/org/apache/cloudstack/storage/image/db/SnapshotDataStoreDaoImpl.java b/engine/storage/src/org/apache/cloudstack/storage/image/db/SnapshotDataStoreDaoImpl.java
@@ -55,6 +55,7 @@ public class SnapshotDataStoreDaoImpl extends GenericDaoBase<SnapshotDataStoreVO
     private SearchBuilder<SnapshotDataStoreVO> storeSnapshotSearch;
     private SearchBuilder<SnapshotDataStoreVO> snapshotIdSearch;
     private SearchBuilder<SnapshotDataStoreVO> volumeIdSearch;
+    private SearchBuilder<SnapshotDataStoreVO> volumeSearch;
 
     private final String parentSearch = "select store_id, store_role, snapshot_id from cloud.snapshot_store_ref where store_id = ? "
         + " and store_role = ? and volume_id = ? and state = 'Ready'" + " order by created DESC " + " limit 1";
@@ -119,6 +120,11 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
         volumeIdSearch.and("volume_id", volumeIdSearch.entity().getVolumeId(), SearchCriteria.Op.EQ);
         volumeIdSearch.done();
 
+        volumeSearch = createSearchBuilder();
+        volumeSearch.and("volume_id", volumeSearch.entity().getVolumeId(), SearchCriteria.Op.EQ);
+        volumeSearch.and("store_role", volumeSearch.entity().getRole(), SearchCriteria.Op.EQ);
+        volumeSearch.done();
+
         return true;
     }
 
@@ -291,6 +297,14 @@ public SnapshotDataStoreVO findBySnapshot(long snapshotId, DataStoreRole role) {
         return findOneBy(sc);
     }
 
+    @Override
+    public SnapshotDataStoreVO findByVolume(long volumeId, DataStoreRole role) {
+        SearchCriteria<SnapshotDataStoreVO> sc = volumeSearch.create();
+        sc.setParameters("volume_id", volumeId);
+        sc.setParameters("store_role", role);
+        return findOneBy(sc);
+    }
+
     @Override
     public List<SnapshotDataStoreVO> findBySnapshotId(long snapshotId) {
         SearchCriteria<SnapshotDataStoreVO> sc = snapshotIdSearch.create();